Skip to content

Conversation

@lhoward
Copy link
Contributor

@lhoward lhoward commented Jun 28, 2025

limits.h must be included before stdlib.h when building with glibc and having _FORTIFY_SOURCE set to a non-zero value.

When building with _FORTIFY_SOURCE, realpath() is inlined, and its definition depends on whether limits.h has been included or not (clearly, this is a terrible idea in terms of interacting with Clang modules and should probably be fixed upstream). If the definition differs from the one in SwiftGlibc, then _TestingInternals will not build.

lhoward added a commit to PADL/meta-swift that referenced this pull request Jun 28, 2025
`limits.h` must be included before `stdlib.h` when building with glibc and
having `_FORTIFY_SOURCE` set to a non-zero value.

When building with `_FORTIFY_SOURCE`, `realpath()` is inlined, and its
definition depends on whether `limits.h` has been included or not (clearly,
this is a terrible idea in terms of interacting with Clang modules and should
probably be fixed upstream). If the definition differs from the one in
SwiftGlibc, then _TestingInternals will not build.

swiftlang/swift-testing#1184
Copy link
Contributor

@grynspan grynspan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we put it in alphabetical position? Also, can you include a URL to whatever issue tracks the upstream fix?

@grynspan grynspan added bug 🪲 Something isn't working linux 🐧 Linux support (all distros) workaround Workaround for an issue in another component (may need to revert later) build 🧱 Affects the project's build configuration or process labels Jun 28, 2025
@grynspan grynspan added this to the Swift 6.x (main) milestone Jun 28, 2025
@grynspan
Copy link
Contributor

@swift-ci test

@lhoward
Copy link
Contributor Author

lhoward commented Jun 28, 2025

Obviously we could also conditionalise this based on platform but I presume it's generally safe to include <limits.h> first.

@lhoward
Copy link
Contributor Author

lhoward commented Jun 28, 2025

Can we put it in alphabetical position? Also, can you include a URL to whatever issue tracks the upstream fix?

Yes, will do.

@grynspan
Copy link
Contributor

Is there a URL we can point to tracking the issue in glibc?

@lhoward
Copy link
Contributor Author

lhoward commented Jun 29, 2025

Is there a URL we can point to tracking the issue in glibc?

I haven't filed a bug yet, will do (sorry, it's Sunday, negotiating bug fixing around family life!).

@grynspan
Copy link
Contributor

Understood. This can certainly wait until Monday!

@lhoward
Copy link
Contributor Author

lhoward commented Jun 29, 2025

Understood. This can certainly wait until Monday!

It appears applying for a glibc bug tracker account is a manual process so, I'll have to wait until I hear back...

@grynspan
Copy link
Contributor

Okay, fair enough. When you get the issue filed please comment here so we can refer back to it in the future.

It's not a blocking issue, so we can merge this change without it.

@grynspan
Copy link
Contributor

@swift-ci test

@lhoward
Copy link
Contributor Author

lhoward commented Jun 30, 2025

Updated.

glibc bug here: https://sourceware.org/bugzilla/show_bug.cgi?id=33120

@grynspan
Copy link
Contributor

@swift-ci test

1 similar comment
@grynspan
Copy link
Contributor

@swift-ci test

@lhoward
Copy link
Contributor Author

lhoward commented Jun 30, 2025

FYI upstream bug is a duplicate, first report was in 2023:

https://sourceware.org/bugzilla/show_bug.cgi?id=30516

@grynspan
Copy link
Contributor

grynspan commented Jul 1, 2025

That's okay, we just want a paper trail. Want to update the link? We can then merge the PR I think.

`limits.h` must be included before `stdlib.h` when building with glibc and
having `_FORTIFY_SOURCE` set to a non-zero value.

When building with `_FORTIFY_SOURCE`, `realpath()` is inlined, and its
definition depends on whether `limits.h` has been included or not (clearly,
this is a terrible idea in terms of interacting with Clang modules and should
probably be fixed upstream). If the definition differs from the one in
SwiftGlibc, then _TestingInternals will not build.

glibc bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30516
@grynspan
Copy link
Contributor

grynspan commented Jul 2, 2025

@lhoward If you need this in our 6.2 release, please create a new PR targetting release/6.2 including this PR's commit and following this template. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug 🪲 Something isn't working build 🧱 Affects the project's build configuration or process linux 🐧 Linux support (all distros) workaround Workaround for an issue in another component (may need to revert later)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants