Bug 24389 – importC: Building zlib in Phobos with importC fails on FreeBSD 14

This is not an issue on the latest release of dmd - 2.070 - but it is an issue on master. It looks like with 2.070, we're still building zlib in Phobos without importC, whereas with master, it's currently set up to use importC. Presumably, that works on some systems, but it does not work on FreeBSD 14. The relevant error in the Phobos build is --- /tmp/building_dmd/bin/dmd -c -conf= -I../dmd/druntime/import -w -de -preview=dip1000 -preview=dtorfields -preview=fieldwise -m64 -fPIC -O -release -P=-Ietc/c/zlib -P=-DHAVE_UNISTD_H -ofgenerated/freebsd/release/64/zlib.o etc/c/zlib/adler32.c etc/c/zlib/compress.c etc/c/zlib/crc32.c etc/c/zlib/deflate.c etc/c/zlib/gzclose.c etc/c/zlib/gzlib.c etc/c/zlib/gzread.c etc/c/zlib/gzwrite.c etc/c/zlib/infback.c etc/c/zlib/inffast.c etc/c/zlib/inflate.c etc/c/zlib/inftrees.c etc/c/zlib/trees.c etc/c/zlib/uncompr.c etc/c/zlib/zutil.c /usr/include/stdlib.h(352): Error: no type for declarator before `asm` /usr/include/stdlib.h(352): Error: no type for declarator before `asm` /usr/include/stdlib.h(352): Error: no type for declarator before `asm` /usr/include/stdlib.h(352): Error: no type for declarator before `asm` /usr/include/stdlib.h(352): Error: no type for declarator before `asm` /usr/include/stdlib.h(352): Error: no type for declarator before `asm` /usr/include/stdlib.h(352): Error: no type for declarator before `asm` /usr/include/stdlib.h(352): Error: no type for declarator before `asm` /usr/include/stdlib.h(352): Error: no type for declarator before `asm` /usr/include/stdlib.h(352): Error: no type for declarator before `asm` /usr/include/stdlib.h(352): Error: no type for declarator before `asm` /usr/include/stdlib.h(352): Error: no type for declarator before `asm` /usr/include/stdlib.h(352): Error: no type for declarator before `asm` gmake[1]: *** [Makefile:328: generated/freebsd/release/64/zlib.o] Error 1 --- From looking at stdlib.h, I'm pretty sure that this is not a problem in FreeBSD 13 (and it looks like the current dlang CI stuff tests FreeBSD 13.2), but it is a problem in FreeBSD 14 (FreeBSD 14.0 currently being the latest release of FreeBSD). The FreeBSD 13 version of stdlib.h can be seen here: https://cgit.freebsd.org/src/tree/include/stdlib.h?h=stable/13 The FreeBSD 14 version of stdlib.h can be seen here: https://cgit.freebsd.org/src/tree/include/stdlib.h?h=stable/14 The section that's causing a problem does not exist in the FreeBSD 13 version, and it's --- /* * In FreeBSD 14, the prototype of qsort_r() was modified to comply with * POSIX. The standardized qsort_r()'s order of last two parameters was * changed, and the comparator function is now taking thunk as its last * parameter, and both are different from the ones expected by the historical * FreeBSD qsort_r() interface. * * Apply a workaround where we explicitly link against the historical * interface, qsort_r@FBSD_1.0, in case when qsort_r() is called with * the last parameter with a function pointer that exactly matches the * historical FreeBSD qsort_r() comparator signature, so applications * written for the historical interface can continue to work without * modification. */ #if defined(__generic) || defined(__cplusplus) void __qsort_r_compat(void *, size_t, size_t, void *, int (*)(void *, const void *, const void *)); __sym_compat(qsort_r, __qsort_r_compat, FBSD_1.0); #endif #if defined(__generic) && !defined(__cplusplus) #define qsort_r(base, nel, width, arg4, arg5) \ __generic(arg5, int (*)(void *, const void *, const void *), \ __qsort_r_compat, qsort_r)(base, nel, width, arg4, arg5) #elif defined(__cplusplus) __END_DECLS extern "C++" { static inline void qsort_r(void *base, size_t nmemb, size_t size, void *thunk, int (*compar)(void *, const void *, const void *)) { __qsort_r_compat(base, nmemb, size, thunk, compar); } } __BEGIN_DECLS #endif --- Specifically, it's this line --- __sym_compat(qsort_r, __qsort_r_compat, FBSD_1.0); --- __sym_compat is a macro that comes from cdefs.h, which can be seen here: https://cgit.freebsd.org/src/tree/sys/sys/cdefs.h?h=stable/14 Specifically, the macro can be found in this block of code --- #if defined(__GNUC__) #define __strong_reference(sym,aliassym) \ extern __typeof (sym) aliassym __attribute__ ((__alias__ (#sym))) #ifdef __STDC__ #define __weak_reference(sym,alias) \ __asm__(".weak " #alias); \ __asm__(".equ " #alias ", " #sym) #define __warn_references(sym,msg) \ __asm__(".section .gnu.warning." #sym); \ __asm__(".asciz \"" msg "\""); \ __asm__(".previous") #define __sym_compat(sym,impl,verid) \ __asm__(".symver " #impl ", " #sym "@" #verid) #define __sym_default(sym,impl,verid) \ __asm__(".symver " #impl ", " #sym "@@@" #verid) #else #define __weak_reference(sym,alias) \ __asm__(".weak alias"); \ __asm__(".equ alias, sym") #define __warn_references(sym,msg) \ __asm__(".section .gnu.warning.sym"); \ __asm__(".asciz \"msg\""); \ __asm__(".previous") #define __sym_compat(sym,impl,verid) \ __asm__(".symver impl, sym@verid") #define __sym_default(impl,sym,verid) \ __asm__(".symver impl, sym@@@verid") #endif /* __STDC__ */ #endif /* __GNUC__ */ --- I don't know if __GNUC__ is being defined when zlib.c is being compiled, but if it is, then the macro is --- #define __sym_compat(sym,impl,verid) \ __asm__(".symver " #impl ", " #sym "@" #verid) --- whereas if it isn't, then it's --- #define __sym_compat(sym,impl,verid) \ __asm__(".symver impl, sym@verid") --- However, I'm not sure that it really matters which version is being used with regards to importC's complaint. It seems to dislike how __asm__ is being used, but I don't know enough about __asm__ to really comment on what could be wrong here. But regardless of what importC needs to do here, until it's fixed, zlib in Phobos cannot be built with importC on FreeBSD 14, and while that may not be a problem with the CI at the moment, it will be eventually once it's testing FreeBSD 14, and it's certainly a problem for me, since that's what I currently run on my main machine. And since this is a macro that FreeBSD uses periodically to deal with backwards compatibility issues, I'd expect it to pop up again at some point - and it can probably be hit with FreeBSD 13 as well, just with a different function somewhere that doesn't affect building zlib.

There isn't quite enough information there. If you could compile with cpp to capture the preprocessor output, we can pinpoint the exact line of failure.

Created attachment 1905 cpp output on .c file containing just #include <stdlib.h> Okay. My experience with importC at this point is pretty much zero, so my experience with debugging it is zero, and I could be going about this the wrong way, but rather than try to fight the Phobos build to try to run specific build commands on stuff, I tried to reduce this to something simple that I can easily run build commands on, and given that the problem seems to be with stdlib.h, that should be straightforward. So, I created a file called a.c that was just --- #include <stdlib.h> --- and ran dmd a.c which results in basically the same error as the Phobos build (except only once): --- /usr/include/stdlib.h(352): Error: no type for declarator before `asm` --- I ran cpp a.c > b.c and attached b.c here. However running dmd b.c results in exactly the same error. --- /usr/include/stdlib.h(352): Error: no type for declarator before `asm` --- So, I don't know if that's helpful (since I would have thought that it would then show an error inside of b.c instead), but b.c does end up with --- void __qsort_r_compat(void *, size_t, size_t, void *, int (*)(void *, const void *, const void *)); __asm__(".symver " "__qsort_r_compat" ", " "qsort_r" "@" "FBSD_1.0"); --- inside of it, which presumably is what you were looking for. Sticking just that in a file called c.c and running dmd c.c results in --- c.c(3): Error: no type for declarator before `asm --- Hopefully that's generated from the same version of the macro that's used when zlib imports it in the Phobos build. I know that the dmd / Phobos build at least used to need gcc on FreeBSD for some stuff, but I don't know if it's currently required or used, and running cpp is definitely running clang. Either way, the error is the same. So, hopefully that helps. If you need me to run something else, just ask. I don't have much experience trying to debug preprocessor issues (and what experience I do have is about a decade old at this point), though I guess that that's technically more experience than I have with importC.

(In reply to Jonathan M Davis from comment #0) > This is not an issue on the latest release of dmd - 2.070 - but it is an > issue on master. It looks like with 2.070, we're still building zlib in You mean 2.107 is ok? 2.108-dev is not?

(In reply to Jonathan M Davis from comment #2) > --- > void __qsort_r_compat(void *, size_t, size_t, void *, > int (*)(void *, const void *, const void *)); > __asm__(".symver " "__qsort_r_compat" ", " "qsort_r" "@" "FBSD_1.0"); > --- I know what that is - as far as I'm aware as of writing, the ImportC parser does not support top-level asm declarations.

(In reply to Iain Buclaw from comment #4) > (In reply to Jonathan M Davis from comment #2) > > --- > > void __qsort_r_compat(void *, size_t, size_t, void *, > > int (*)(void *, const void *, const void *)); > > __asm__(".symver " "__qsort_r_compat" ", " "qsort_r" "@" "FBSD_1.0"); > > --- > I know what that is - as far as I'm aware as of writing, the ImportC parser > does not support top-level asm declarations. FYI https://gcc.gnu.org/onlinedocs/gcc/Basic-Asm.html#Remarks """ Extended asm statements have to be inside a C function, so to write inline assembly language at file scope (“top-level”), outside of C functions, you must use basic asm. You can use this technique to emit assembler directives, define assembly language macros that can be invoked elsewhere in the file, or write entire functions in assembly language. Basic asm statements outside of functions may not use any qualifiers. """

Support can be simply added to the parser, but I can't see DMD doing anything but ignore or error about it being there.

@ibuclaw created dlang/dmd pull request #16184 "partial bugzilla 24389 - importC: Parser support for top-level asm definitions" mentioning this issue: - partial bugzilla 24389 - importC: Parser support for top-level asm definitions https://github.com/dlang/dmd/pull/16184

(In reply to Iain Buclaw from comment #3) > (In reply to Jonathan M Davis from comment #0) > > This is not an issue on the latest release of dmd - 2.070 - but it is an > > issue on master. It looks like with 2.070, we're still building zlib in > You mean 2.107 is ok? 2.108-dev is not? Yes, sorry. 2.107 is fine - for the Phobos build - whereas dmd master is not - presumably because dmd master is using importC for zlib, whereas 2.107 is not. However, 2.107 fails the simplified test case of just #including stdlib.h, so the importC issue is there regardless. (In reply to Iain Buclaw from comment #6) > Support can be simply added to the parser, but I can't see DMD doing > anything but ignore or error about it being there. I'm not sure what dmd should be doing here, but as far as the Phobos build goes, either dmd needs to be able to handle the __asm__ that FreeBSD 14 is using in stdlib.h and do the correct thing for zlib to work, or we're not going to be able to build zlib with importC on FreeBSD 14 (and thus can't build it with importC in general unless we're doing something different for FreeBSD). It doesn't look like qsort_r is being used by anything in Phobos right now, so I don't think that the __asm__ has to actually work in this case in order to build zlib, though __asm__ like this could definitely pop up again in the future, since this is something that FreeBSD will sometimes do to change what's being linked (usually to fix compatibility issues). I don't know if other OSes do the same. Either way, obviously, the closer that we can get to actually using the __asm__ properly, the better, and gdc and ldc are clearly in a much better position for that.

(In reply to Jonathan M Davis from comment #8) > It doesn't look like qsort_r is being used by anything in Phobos right now, > so I don't think that the __asm__ has to actually work in this case in order > to build zlib, though __asm__ like this could definitely pop up again in the > future, since this is something that FreeBSD will sometimes do to change > what's being linked (usually to fix compatibility issues). I don't know if > other OSes do the same. > > Either way, obviously, the closer that we can get to actually using the > __asm__ properly, the better, and gdc and ldc are clearly in a much better > position for that. Alternatively, there's reverting the change that introduced using DMD to build zlib. https://github.com/dlang/phobos/pull/8873 If not resorting back to using $CC for FreeBSD, maybe the zlib sources could be altered to filter out the bad code in the header? https://reviews.freebsd.org/source/src/browse/main/include/stdlib.h$347 #undef __generic?

(In reply to Iain Buclaw from comment #9) > Alternatively, there's reverting the change that introduced using DMD to > build zlib. Yeah, I was talking about what would be required for zlib to build with importC. Either way, ultimately, if we can't get dmd to work with importC and the way that FreeBSD is using __asm__ in stdlib.h, then we can't build zlib in Phobos with importC. > If not resorting back to using $CC for FreeBSD, maybe the zlib sources could > be altered to filter out the bad code in the header? > > https://reviews.freebsd.org/source/src/browse/main/include/stdlib.h$347 > > #undef __generic? Maybe? If it were done carefully, then we could probably make it work, but it also could cause problems down the line as the headers change with different OS versions, and depending on what the changes were, we might end up with silent breakage. Personally, I'd be more inclined to just not use importC for zlib, since we clearly don't need to. Of course, it would be nice if dmd could just handle the __asm__ stuff properly, but I guess that that would require that dmd be able to handle another kind of assembly language.

I agree that being able to robustly build Phobos (on updated or exotic targets) is way more important than slightly simplifying the druntime build and testing importC for free while at it. [That's why I'll keep using the C compiler for LDC, where exotic targets are much more common than with DMD.] If any of these common system headers use unsupported syntax/features, importC is basically unusable on that target, and adding support might be non-trivial as in this case. It might be worth trying to make importC/CC configurable via make option, so that we don't lose the extra test coverage for the common targets, where importC works sufficiently; I guess that wouldn't come with too many extra makefile lines.

@kinke created dlang/phobos pull request #8914 "Build: Add option USE_IMPORTC=0 to switch back to compiling zlib with C compiler" mentioning this issue: - Build: Add option USE_IMPORTC=0 to switch back to compiling zlib with C compiler Wrt. Bugzilla Issue 24389. https://github.com/dlang/phobos/pull/8914

dlang/dmd pull request #16184 "partial bugzilla 24389 - importC: Parser support for top-level asm definitions" was merged into master: - 78043dabcba037adf9de12b629745415e42609e2 by Iain Buclaw: partial bugzilla 24389 - importC: Parser support for top-level asm definitions https://github.com/dlang/dmd/pull/16184

dlang/phobos pull request #8914 "Build: Add option USE_IMPORTC=0 to switch back to compiling zlib with C compiler" was merged into master: - c06255411a1a51b51aea24dd88bcf133f00c584e by Martin Kinkelin: Build: Add option USE_IMPORTC=0 to switch back to compiling zlib with C compiler Wrt. Bugzilla Issue 24389. https://github.com/dlang/phobos/pull/8914

@jmdavis created dlang/phobos pull request #8918 "Fix bugzilla issue 24389 - Make Phobos compile on FreeBSD 14 again." fixing this issue: - Fix bugzilla issue 24389 - Make Phobos compile on FreeBSD 14 again. zlib can't be built with importC on FreeBSD 14, because qsort_r in stdlib.h uses some asm instructions, which dmd can't handle. https://github.com/dlang/phobos/pull/8914 fixed it so that we got a proper error message about it (and theoretically making it so that zlib could be compiled on FreeBSD 14 with gdc or ldc, though I haven't tried that), but it didn't fix it so that it could build on dmd, since that would require that dmd be able to handle GNU assembly code, which isn't a planned feature AFAIK. https://github.com/dlang/phobos/pull/8914 fixed it so that it's possible to build Phobos without using importC by providing a make variable, but it didn't do anything for FreeBSD specifically. This commit changes it so that FreeBSD sets that make variable in the makefile so that you don't have to do it manually to get FreeBSD 14 to build. It's not necessary for FreeBSD 13.2 (which is what the auto testers currently use), but it will be necessary for FreeBSD 14 (14.0 currently being the latest release of FreeBSD). I can confirm from testing that explicitly setting USE_IMPORTC=1 on the command line will overrides this change, so that variable can still be set one way or the other on FreeBSD. It's just now 0 by default so that it will build by default on FreeBSD 14. https://github.com/dlang/phobos/pull/8918

dlang/phobos pull request #8918 "Fix bugzilla issue 24389 - Make Phobos compile on FreeBSD 14 again." was merged into master: - e7779f64d119ad71f6fad3a4578d6fc9dd7dbf0a by Jonathan M Davis: Fix bugzilla issue 24389 - Make Phobos compile on FreeBSD 14 again. zlib can't be built with importC on FreeBSD 14, because qsort_r in stdlib.h uses some asm instructions, which dmd can't handle. https://github.com/dlang/dmd/pull/16184 fixed it so that we got a proper error message about it (and theoretically making it so that zlib could be compiled on FreeBSD 14 with gdc or ldc, though I haven't tried that), but it didn't fix it so that it could build on dmd, since that would require that dmd be able to handle GNU assembly code, which isn't a planned feature AFAIK. https://github.com/dlang/phobos/pull/8914 fixed it so that it's possible to build Phobos without using importC by providing a make variable, but it didn't do anything for FreeBSD specifically. This commit changes it so that FreeBSD sets that make variable in the makefile so that you don't have to do it manually to get FreeBSD 14 to build. It's not necessary for FreeBSD 13.2 (which is what the auto testers currently use), but it will be necessary for FreeBSD 14 (14.0 currently being the latest release of FreeBSD). I can confirm from testing that explicitly setting USE_IMPORTC=1 on the command line will overrides this change, so that variable can still be set one way or the other on FreeBSD. It's just now 0 by default so that it will build by default on FreeBSD 14. https://github.com/dlang/phobos/pull/8918