IAIK/sweb

-gstabs2 build is compiler specific

Closed this issue · 12 comments

While it is apparently needed for stack traces (https://swebwiki.student.iaik.tugraz.at/tdevel/stacktrace) this currently limits sweb in the default configuration to be built only by GCC. Stallman would be proud... but please consider wrapping the cflag lines in CMAKE_COMPILER_IS_GNUCC/GNUCXX (http://www.cmake.org/Wiki/CMake_Useful_Variables). If it would be accepted upstream, I can try to patch it myself too.

does it build and boot using another compiler without -gstabs2?

if it does i would find it useful to wrapping it in CMAKE_COMPILER_IS_GNUCC/GNUCXX
otherwise i would rather put an assertion somewhere in the beginning that the compiler is GNUCC/GNUCXX

I built it with clang last semester with minimal changes(removing gstabs2).
Booting did not work though. The only change I can remember was some
varargs stuff in kprintf ( sizeof(void) or something like that ).
On 14 Feb 2015 01:26, "Daniel Gruss" notifications@github.com wrote:

does it build and boot using another compiler without -gstabs2?

if it does i would find it useful to wrapping it in
CMAKE_COMPILER_IS_GNUCC/GNUCXX
otherwise i would rather put an assertion somewhere in the beginning that
the compiler is GNUCC/GNUCXX


Reply to this email directly or view it on GitHub
#41 (comment).

it seems since then we added some more gcc/g++ dependencies...

/d/arbeit/sweb/arch/x86/32/common/source/boot.cpp:28:3: error: default initialization of an object of const type 'const struct (anonymous struct at /d/arbeit/sweb/arch/x86/32/common/source/boot.cpp:20:14)'
      requires a user-provided default constructor
} mboot __attribute__ ((section (".mboot")));

well, that one is easily fixed like this:

struct mboot_t {
  mboot_t() {}
  uint32 magic = MULTIBOOT_HEADER_MAGIC;
  uint32 flags = MULTIBOOT_HEADER_FLAGS;
  uint32 checksum = MULTIBOOT_CHECKSUM;
  uint32 mode = 0;
  uint32 widht = 800;
  uint32 height = 600;
  uint32 depth = 32;
};
const static mboot_t mboot __attribute__ ((section (".mboot")));   

It seems if you want to make it const, you need a constructor.

The only other fix needed is in:
userspace/libc/printf.c:420

va_arg(args,void);  

This needs to be handled anyways since it is undefined behaviour. I have no idea how you want to handle unknown format strings in printf - I would just kill the program with an error or something.

It's "width", not "widht" by the way, doesn't seem the name is important in that case apparently... Also it's more of a compiler bug in GCC (a bit simplified, this writing style might be in the standard in the future but it currently isn't: http://stackoverflow.com/questions/26077807/why-does-gcc-allow-a-const-object-without-a-user-declared-default-constructor-bu).

I'll try to get it at least to compile with at least clang and provide a pull request against master.

there are a lot of typos in sweb ;)

i will merge it if it does not change development experience with gcc/g++

arch/x86/32/CMakeLists.compiler also look suspiciously locked in.

It's kinda weird where these flags get set, set again, then set again... especially for some more globally important settings (-Wall) next to some that are only needed in some local areas.

To make it really mean next semester, just put -Wfatal-errors in the root. :-P

the current cmake files work on different linux versions/different osx versions/different windows versions...

feel free to suggest changes which work on all platforms...

Apparently pull requests really suck hard if your local master is ahead of upstream master... I'll just send the patch file via mail.

yeah, i would keep it in a separate branch anyway until it is functional for another compiler, it makes no sense to give anyone the impression it would work...

new branch is clang_compat_devel, feel free to submit further patches or pull requests on that branch @MarkusTeufelberger / @skogler ;)