iains / gcc-darwin-arm64

GCC master branch for Darwin with experimental support for Arm64. Currently GCC-15.0.0 [September 2024]
GNU General Public License v2.0
268 stars 33 forks source link

Build failure with Xcode 16 on some usages of the `NULL` macro #135

Closed BertalanD closed 1 month ago

BertalanD commented 2 months ago

Already posted a patch here: https://gcc.gnu.org/pipermail/gcc-patches/2024-July/656779.html

As of Xcode 16 beta 2 with the macOS 15 SDK, each re-inclusion of the stddef.h header causes the NULL macro in C++ to be re-defined to an integral constant (__null). This makes the workaround in d59a576b8 ("Redefine NULL to nullptr") ineffective, as other headers that are typically included after system.h (such as obstack.h) do include stddef.h too.

This can be seen by running the sample below through clang++ -E

include

define NULL nullptr

include

NULL

The relevant libc++ change is here: https://github.com/llvm/llvm-project/commit/2950283dddab03c183c1be2d7de9d4999cc86131

This commit fixes the instances where NULL being an integral constant instead of a null pointer literal (such as no longer implicitly converting to a pointer when used as a template function's argument).

   gcc/value-pointer-equiv.cc:65:43: error: no viable conversion from `pair<typename __unwrap_ref_decay<long>::type, typename __unwrap_ref_decay<long>::type>' to 'const pair<tree, tree>'
   65 |   const std::pair <tree, tree> m_marker = std::make_pair (NULL, NULL);
      |                                           ^~~~~~~~~~~~~~~~~~~~~~~~~~~

As noted in the previous commit though, the proper solution would be to phase out the usages of NULL in GCC's C++ source code.

Looks like this needs to be solved on GCC's side; this response on the LLVM Discord indicates that the stddef.h change likely won't be reverted:

Working around a workaround is almost never a good idea, and since you say yourself that the fix is simple, I'd just fix the code instead of doing error-prone trickery with macros.

iains commented 2 months ago

I saw your patch on gcc-patches@ (thanks for doing this!), and the follow-on (which is correct). I suggest you repost with that fixed and cc the analyser maintainer (since most changes are there).

It would generally be preferable to pull in an upstream patch once it is applied; although I guess we probably will need to re-issue the darwin branches for the GCC-14, 13, 12 and 11 when that is done. If we do not get any traction with the patch in a reasonable timeframe - I will put it on the darwin branches here.

BertalanD commented 2 months ago

I'm currently fighting with git-send-email as I'm not too familiar with that workflow, but will try to post the updated patch soon.

I would like to have this backported for all active release branches upstream, too. I originally found this because SerenityOS contributors were reporting that they were failing to build our 13.2.0-based toolchain.

iains commented 2 months ago

re-git send-email - you can attach a patch to the response to a review, if needed.

... we want to avoid too much code churn from an as yet unreleased OS and Xcode pair; tracking issues in betas is better done with Feedbacks to Apple (although if the response in this case is that this is not likely to be altered, we can proceed). Persevere with upstream - but I will apply here at the weekend to the Arm64 branch rebase - i'd really really prefer to wait for the upstream patch to back port because it means making another release (and my hardware is all tied up with the GCC-11.5 release for the next week) - but, again, if we get no upstream traction - then fine, we can do this.

iains commented 2 months ago

OK .. so now it's approved upstream

if no to either - please add a signed-off-by line to your patch header and send me the git-format output directly - I can apply it for you.

BertalanD commented 2 months ago

I do not have commit access; sent you the patch directly via email with the DCO sign-off added.

Thanks for all the help!

iains commented 1 month ago

this is fixed on trunk and I back ported to gcc-14 (for gcc-14.2) and 11.5 (darwin branch only) - I guess we could do an r2 for 13.3 and/or r1 for 12.4 if anyone has a pressing need - but 11.5 would provide a bootstrap compiler for D - so it does not seem too pressing to me.