gbdev / rgbds

Rednex Game Boy Development System - An assembly toolchain for the Nintendo Game Boy and Game Boy Color
https://rgbds.gbdev.io
MIT License
1.35k stars 172 forks source link

FreeBSD: v0.6.0 clang build fails #1091

Closed nunotexbsd closed 1 year ago

nunotexbsd commented 2 years ago

Hello,

v0.6.0 doesn't build with clang 14.0.5 and 13.0.0:

cc -O2 -pipe  -fstack-protector-strong -fno-strict-aliasing  -Wall -pedantic -std=gnu11 -I include  -D_POSIX_C_SOURCE=200809L -D_ISOC11_SOURCE -c -o src/asm/main.o src/asm/main.c
--- src/gfx/pal_spec.o ---
In file included from src/gfx/pal_spec.cpp:18:
In file included from /usr/include/c++/v1/fstream:185:
/usr/include/c++/v1/__locale:637:16: error: use of undeclared identifier 'isascii'
        return isascii(__c) ? (__tab_[static_cast<int>(__c)] & __m) !=0 : false;
               ^
/usr/include/c++/v1/__locale:644:22: error: use of undeclared identifier 'isascii'
            *__vec = isascii(*__low) ? __tab_[static_cast<int>(*__low)] : 0;
                     ^
/usr/include/c++/v1/__locale:652:17: error: use of undeclared identifier 'isascii'
            if (isascii(*__low) && (__tab_[static_cast<int>(*__low)] & __m))
                ^
/usr/include/c++/v1/__locale:661:19: error: use of undeclared identifier 'isascii'
            if (!(isascii(*__low) && (__tab_[static_cast<int>(*__low)] & __m)))

14.0.5 log 13.0.0 log

But it build fine with clang 10.0.1: log

Any hints?

Thanks

ISSOtm commented 2 years ago

I'm not sure, this requires digging in the FreeBSD C library to check why it's not defined. The -D on that CLI should expose that function (POSIX.1-2008 specifies isascii, it's "just" marked as obsolete).

...that said, the fact that the error originates from template code in a C++ header hints at a problem entirely contained within the FreeBSD library. You should probably pester them :S

Additionally, the compiler differences are likely because the compilers expose different "__" #defines, or some other benign side effect. This is just speculation at this point, but it's likely not the root cause.

nunotexbsd commented 2 years ago

I don't have more skills to inverstigate this but it builds fine with clang 10.0.0 and gcc 11.3.0. On FreeBSD ports, emulators/sameboy ( https://github.com/LIJI32/SameBoy ) uses rgbds as a build dependency and it gives the error with 0.6.0:

rgbgfx -h -u -o build/obj/BootROMs/SameBoyLogo.2bpp BootROMs/SameBoyLogo.png
warning: `-h` is deprecated, use `-Z` instead
cc -std=c99 -Wall -Werror BootROMs/pb12.c -o build/pb12
cp -f LICENSE build/bin/SDL/LICENSE
cp -f Misc/registers.sym build/bin/SDL/registers.sym
cp -f SDL/background.bmp build/bin/SDL/background.bmp
cp -rf Shaders/*.fsh build/bin/SDL/Shaders
cp -rf Misc/Palettes/*.sbp build/bin/SDL/Palettes
/wrkdirs/usr/ports/emulators/sameboy/work/SameBoy-0.15.6/build/pb12 < build/obj/BootROMs/SameBoyLogo.2bpp > build/obj/BootROMs/SameBoyLogo.pb12
Assertion failed: (outctl != 1), function main, file BootROMs/pb12.c, line 90.
gmake[1]: *** [Makefile:473: build/obj/BootROMs/SameBoyLogo.pb12] Abort trap (core dumped)
gmake[1]: *** Deleting file 'build/obj/BootROMs/SameBoyLogo.pb12'

So I will wait for a sameboy new release to see if they found some problems with this version.

Thanks

ISSOtm commented 2 years ago

I can also reproduce that error with my local RGBDS v0.6.0. That's problematic.

ISSOtm commented 2 years ago

I submitted LIJI32/SameBoy#501 to fix the build error; it boils down to passing -c embedded to the rgbgfx invocation.

orbea commented 2 years ago

The API break in rgbgfx is very unfortunate how its impossible to support both older and newer versions of rgbds in SameBoy. Could a v0.6.1 be released without this issue?

I made a SameBoy issue already.

https://github.com/LIJI32/SameBoy/issues/500

evie-calico commented 2 years ago

API breaks are common in RGBDS; it's still in its 0.x stage after all.

ISSOtm commented 2 years ago

Strictly speaking, yes; "unfortunately", we also have a fairly large amount of depending projects, so we are somewhere in limbo, and should try to play nice with them.

orbea commented 2 years ago

API breaks are common in RGBDS; it's still in its 0.x stage after all.

This is a fair point, but perhaps a slower transition to the new default would of been better for downstream projects?

For example add -c now while keeping the current default with a deprecation message, then later for 0.7.0 where the default is changed?

ISSOtm commented 2 years ago

This was the case before the commit you bisected to (d86d24bdc14fb2fc5c1ca9729495e1a26c2d6d08); external discussion prior to that ruled that the hack was not necessary for downstream projects, i.e. we weren't expecting reliance on colour order as strong as this.

nunotexbsd commented 1 year ago

Same problem with 0.6.1:

Fails: 14.0.5 log 13.0.0 log OK: 10.0.1 log All OK with gcc12

Rebuilded sameboy and build/test run OK

This port uses a configure knob that without it, it fails too on 10.0.1:

do-configure:
# yank _POSIX_C_SOURCE when there is no support for _ISOC11_SOURCE
        @cd ${WRKSRC}; ${PRINTF} '#include <assert.h>\nint main(){static_assert(1, "");}' | \
                ${CC} -std=gnu11 -D_POSIX_C_SOURCE=200809L -D_ISOC11_SOURCE -xc - 2>/dev/null || \
                ${REINPLACE_CMD} 's,-D_POSIX_C_SOURCE=200809L,,' ${WRKSRC}/Makefile

Thanks

ISSOtm commented 1 year ago

Why does it delete the -D_POSIX_C_SOURCE=200809L from the Makefile?

nunotexbsd commented 1 year ago

Why does it delete the -D_POSIX_C_SOURCE=200809L from the Makefile?

Good question!

I'm not the creator of this port, I'm a maintainer but I've never had an answer of why this configure tweak is needed.

I will test it without OR || part.

nunotexbsd commented 1 year ago

Without || ${REINPLACE_CMD} 's,-D_POSIX_C_SOURCE=200809L,,' ${WRKSRC}/Makefile, builds with clang 14 and 13 continues to fail and got a configure stage error on jail running with clang 10.

nunotexbsd commented 1 year ago

Fixing build on clang >10 removing -D_POSIX_C_SOURCE=200809L

https://cgit.freebsd.org/ports/commit/?id=e4d330bbdc82f346bc31f109ebff0d71154362d0

ISSOtm commented 1 year ago

I don't think this bug is on our end? At least, I don't think there's anything we can do; therefore, closing this.