Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix open_memstream/fmemopen feature detection with GCC >= 14 #544

Merged
merged 1 commit into from
Sep 26, 2024

Conversation

nmeum
Copy link
Contributor

@nmeum nmeum commented Sep 22, 2024

GCC 14 has enabled various warnings as errors by default, e.g. -Wimplicit-function-declaration. This causes the current feature detection code for open_memstream(3) and fmemopen(3) to fail with GCC 14.

This commit restores compatibility with GCC 14 in this regard.

Note that it may also be beneficial to pass a feature test macro such as -D_POSIX_C_SOURCE. See the feature test macro requirements for open_memstream(3)andfmemopen(3)`.

See also https://gitlab.alpinelinux.org/alpine/aports/-/merge_requests/72328

GCC 14 has enabled various warnings as errors by default, e.g.
-Wimplicit-function-declaration. This causes the current feature
detection code for `open_memstream(3)` and `fmemopen(3)` to fail
with GCC 14.

This commit restores compatibility with GCC 14 in this regard.

Note that it may also be beneficial to pass a feature test macro
such as -D_POSIX_C_SOURCE. See the feature test macro requirements
for open_memstream(3)` and `fmemopen(3)`.
@justinethier
Copy link
Owner

Appreciate the report and PR @nmeum!

Unfortunately this change breaks the build:

$ make
Makefile.config:94: *** unterminated call to function 'shell': missing ')'.  Stop.

@nmeum
Copy link
Contributor Author

nmeum commented Sep 24, 2024

Hmmm, what Make implementation are you using? Works for me with GNU make 4.4.1.

Will happily adjust the patch but just can't reproduce this right now.

@justinethier
Copy link
Owner

Using make 4.2.1 over here:

justin@ubuntu:~/Documents/cyclone/tests$ make --version
GNU Make 4.2.1

That said, this works in my environment:

CYC_PLATFORM_HAS_MEMSTREAM := $(shell echo "\#include \<stdio.h\> int main(void){char *buf; size_t len; open_memstream(&buf, &len);}" | $(CC) -xc - >/dev/null 2>/dev/null && echo 1 || echo 0)
CYC_PLATFORM_HAS_FMEMOPEN := $(shell echo "\#include \<stdio.h\> int main(void){char *buf; fmemopen(&buf, 0, \"r\");}" | $(CC) -xc - >/dev/null 2>/dev/null && echo 1 || echo 0)

@justinethier
Copy link
Owner

Also, merged in more CI for this repo that will build the C runtime. If you want to repro, you can merge the master branch back into your PR branch and you should see the build failure.

@nmeum nmeum force-pushed the gcc-14 branch 2 times, most recently from 08bc86b to abfaeb8 Compare September 25, 2024 07:25
@nmeum
Copy link
Contributor Author

nmeum commented Sep 25, 2024

Ok, I looked into it. This is caused by the following change in GNU Make 4.3:

  • WARNING: Backward-incompatibility!
    Number signs (#) appearing inside a macro reference or function invocation
    no longer introduce comments and should not be escaped with backslashes:
    thus a call such as:
    foo := $(shell echo '#')
    is legal. Previously the number sign needed to be escaped, for example:
    foo := $(shell echo '#')
    Now this latter will resolve to "#". If you want to write makefiles
    portable to both versions, assign the number sign to a variable:
    H := #
    foo := $(shell echo '$H')
    This was claimed to be fixed in 3.81, but wasn't, for some reason.
    To detect this change search for 'nocomment' in the .FEATURES variable.

@nmeum
Copy link
Contributor Author

nmeum commented Sep 25, 2024

I added the H := \# hack proposed in that release announcement.

Let me know if that works for you.

@justinethier
Copy link
Owner

Alright, this works great on my side as well. Appreciate you researching/adding the backwards compatibility and thanks again for the PR!

@justinethier justinethier merged commit 26d0e1f into justinethier:master Sep 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants