larmel / lacc

A simple, self-hosting C compiler
MIT License
872 stars 64 forks source link

Full OpenBSD support #6

Closed ibara closed 6 years ago

ibara commented 6 years ago

Hi --

This patch adds complete support for OpenBSD. It does so by doing the following:

  1. Registers an __OpenBSD__ preprocessor macro on OpenBSD, like lacc does with __linux__ and __unix__.
  2. Disables adding the default include path of /usr/include/x86_64-linux-gnu when not on Linux. On all the BSDs, the proper includes are simply in /usr/include.
  3. Sets up a proper environment when on OpenBSD. OpenBSD defaults to -fPIC unless explicitly disabled. On other systems, there is no change.
  4. Adds an -fno-PIC flag to explicitly disable pic generation.
  5. Adds support for the __restrict and __restrict__ synonyms for C99 restrict keyword (see: https://gcc.gnu.org/onlinedocs/gcc/Restricted-Pointers.html). All the BSDs use the __restrict form in their headers (for example, in stdio.h) and gcc, clang, pcc, and cparser all support these synonyms, even though it is technically a GNU extension.
  6. Defaults OpenBSD to -std=c99 because of the __restrict keyword use.

This is good enough to run gmake bin/selfhost successfully on OpenBSD. But not all is 100%, as there is a code generation bug in the selfhost binary. When building the selfhost binary with the bootstrap binary, this warning is produced:

bin/bootstrap -fPIC -I include/ -D'LACC_STDLIB_PATH="/home/brian/lacc/include/stdlib"' -c src/backend/x86_64/instr.c -o bin/backend/x86_64/instr-selfhost.o
(src/backend/x86_64/instr.c, 187) warning: Conversion of decimal constant to unsigned.

This is the only warning generated in the entire build process; the warning is not emitted in the lacc binary building the bootstrap binary stage. This appears to matter greatly, as it triggers an assertion for some tests when running gmake test-selfhost. For example:

assertion "0" failed: file "src/backend/x86_64/instr.c", line 427, function "encode_sub"
Abort trap (core dumped)
assertion "0" failed: file "src/backend/x86_64/instr.c", line 427, function "encode_sub"
Abort trap (core dumped)
[-E: Ok!] [-S: Ok!] [-c: Compilation failed!] [-c -O1: Compilation failed!] :: test/c99/flexible-array.c

Let me know if there's anything else you need from me.

Additionally, there is a tiny bug fix: in src/preprocessor/tokenize.c there was an off by 1 in the Static_assert strncmp check (12 instead of 13). Test still passes: [-E: Ok!] [-S: Ok!] [-c: Ok!] [-c -O1: Ok!] :: test/c11/static-assert.c

larmel commented 6 years ago

Hi,

I have some concerns regarding restrict: I think __restrict and __restrict__ should not be known as actual keywords in the tokenizer, but be supported through macros. It seems strange that this is not handled in OpenBSD headers, while f.ex inline is disabled if not `GNUC__`. (looking at https://github.com/openbsd/src/blob/7757c787c46f10e9276a4eef51b897fe6f6e9e23/sys/sys/cdefs.h#L73).

A better fix would be to add register_macro("__restrict", "restrict") in register_builtin_definitions when on OpenBSD (and >= C99). For C89 and OpenBSD, we can remove __restrict with register_macro("__restrict", ""). Then it should not be necessary to set default to C99 either. Otherwise looks good. If you do those changes, I am fine merging this.

Not sure yet what is going on with the codegen bug, but I have reproduced it and will investigate further.

Thanks for taking the time to improve lacc :)

ibara commented 6 years ago

Hi I think I got it all now. This new version creates an identical selfhosted lacc on OpenBSD as the original version. Note that I had to add __ISO_C_VISIBLE=1990 to register_macro on OpenBSD, as OpenBSD's sys/cdefs.h defaults to __ISO_C_VISIBLE=1999 if not defined, and this breaks assertions as it will then try to use the __func__ C99 macro unconditionally.

larmel commented 6 years ago

FYI @ibara: I fixed the codegen issue in this commit. The issue was that LONG_MAX and ULONGMAX had 32-bit values when compiled with lacc, due to missing definition of __LP64_\. Self-hosting and tests should now work on OpenBSD.

Also note that I did some changes to the Makefile to no longer depend on GNU make. It should now be possible to use default make on both platforms.

ibara commented 6 years ago

Thanks a lot! Yup, I can confirm that self-hosting and all tests pass on my laptop. And it's happy with our BSD make.