#133071 modified this handling to minimise a lot of those usages; but on platforms that usedlopen() (macOS and iOS), there was still some duplicated use. This PR cleans up that usage.

Issue:Add a way of printing a C backtrace tofaulthandler #127604

Optimize -ldl usage on platforms that use dlopen for libFFI.

297f242

bedevere-appbot mentioned this pull request

Apr 28, 2025

Add a way of printing a C backtrace tofaulthandler#127604

Closed

Copy link

ContributorAuthor

freakboy3742 commentedApr 28, 2025

!buildbot iOS

Copy link

bedevere-bot commentedApr 28, 2025

🤖 New build scheduled with the buildbot fleet by@freakboy3742 for commit297f242 🤖

Results will be shown at:

https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F133081%2Fmerge

The command will test the builders whose names match following regular expression:iOS

The builders matched are:

iOS ARM64 Simulator PR

freakboy3742 added the skip news label

Apr 28, 2025

freakboy3742 mentioned this pull request

Apr 28, 2025

gh-127604: ensure-ldl is passed only once to the linker#133071

Merged

Copy link

ContributorAuthor

freakboy3742 commentedApr 28, 2025•
edited
Loading

Nope... looks like that hasn't done it. I'll take another look in the morning if nobody beats me to it.

picnixz previously approved these changes

Apr 28, 2025

View reviewed changes

Copy link

Member

picnixz left a comment

Choose a reason for hiding this comment

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

It looks fine and I'm pretty sure there is a cleaner and alternative way to do it but I'm also pretty sure that it would require more steps and a more complicate configuration so I think we should live with this first.

More generally, I think we need to have a function that cleans up the flags to remove duplicate ones.

bedevere-appbot added the awaiting merge label

Apr 28, 2025

Copy link

Member

picnixz commentedApr 28, 2025

Nope... looks like that hasn't done it. I'll take another look in the morning if nobody beats me to it.

Wait how come? then where is the one coming from...?

picnixz dismissed theirstale review

April 28, 2025 12:14

Apparently it doesn't work.

bedevere-appbot added awaiting review and removed awaiting merge labels

Apr 28, 2025

Copy link

Member

picnixz commentedApr 28, 2025

Actually, it looks like it worked because before we had (in the "Compile build Python" step):

ld: warning: ignoring duplicate libraries: '-ldl'ld: warning: ignoring duplicate libraries: '-ldl'ld: warning: ignoring duplicate libraries: '-ldl'ld: warning: ignoring duplicate libraries: '-ldl'ld: warning: ignoring duplicate libraries: '-ldl'ld: warning: ignoring duplicate libraries: '-ldl'

and now we have

ld: warning: ignoring duplicate libraries: '-ldl'ld: warning: ignoring duplicate libraries: '-ldl'ld: warning: ignoring duplicate libraries: '-ldl'ld: warning: ignoring duplicate libraries: '-ldl'

So two warnings go eliminated. It remains to check what the others are. We also have 2 warnings less in the "Compile host Python" step.

Copy link

Member

picnixz commentedApr 28, 2025

Note that the duplicate-ldl appear in only one command:

gcc -ldl    -o _bootstrap_python Modules/getbuildinfo.o [...] \Programs/_bootstrap_python.o Modules/getpath.o -ldl  -framework CoreFoundation

Rework addition of -ldl.

f6a117a

Copy link

ContributorAuthor

freakboy3742 commentedApr 29, 2025

@picnixz After a little more digging, Ithink I've found the source of the problem.

The code added by#133040 is adding-ldl to the top-levelLDFLAGS. This effectively means that every link command gets-ldl injected.

Apple platforms then do a similar check; appending toLIBFFI_LDFLAGS. The initial form of this PR removes that addition, removing most of the duplicate library usage if theLDFLAGS version was used, avoiding most of the duplicates.

However there's alsothis check fordl on L3714, which looks for dlopen; on success, this adds-ldl toLIBS.

Having-ldl in bothLIBS andLDFLAGS leads to the the remaining handful of duplicated cases, becauseLDFLAGS ends up being part ofPY_LDFLAGS, which is part ofPY_CORE_LDFLAGS - and many of the places that includePY_CORE_LDFLAGS also includeLIBS.

So - one fix is to move theLDFLAGS addition until after L3714, and only append toLDFLAGS if thedl check failed. I've done that in the updated version of the PR, but I'm not completely convinced that's the right approach.

It seems like the pre-existing check fordl (that is documented to exist to support SunOS/Solaris, but is also picked up by macOS/iOS) seems incomplete/incompatible with how it's being added in service offaulthandler. It feels like it might be more appropriate to add-ldl only to the linking commands forfaulthandler - which, by the Makefile, would beMODULE_FAULTHANDLER_LDFLAGS - but at that point I'm into the weeds of autoconf macros and it's not clear to me how that would be injected cleanly.

Copy link

ContributorAuthor

freakboy3742 commentedApr 29, 2025

!buildbot iOS

Copy link

bedevere-bot commentedApr 29, 2025

🤖 New build scheduled with the buildbot fleet by@freakboy3742 for commitf6a117a 🤖

Results will be shown at:

https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F133081%2Fmerge

The command will test the builders whose names match following regular expression:iOS

The builders matched are:

iOS ARM64 Simulator PR

freakboy3742 marked this pull request as ready for review

April 29, 2025 06:47

freakboy3742 requested review fromerlend-aasland andcorona10 ascode owners

April 29, 2025 06:47

bedevere-appbot added awaiting core review and removed awaiting review labels

Apr 29, 2025

freakboy3742 requested a review frompicnixz

April 29, 2025 06:47

picnixz approved these changes

Apr 30, 2025

View reviewed changes

Copy link

Member

picnixz left a comment

Choose a reason for hiding this comment

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

It works but I don't know if there is a better autoconf-way to do it

configure.ac

		dnl only add -ldl to LDFLAGS if it isn't already part of LIBS (GH-133081)
		AS_VAR_IF([ac_cv_require_ldl], [yes], [
		AS_VAR_IF([ac_cv_lib_dl_dlopen], [yes], [], [
		AS_VAR_APPEND([LDFLAGS], [" -ldl"])

Copy link

Member

picnixzApr 30, 2025

Choose a reason for hiding this comment

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

Would autoconf be smart enough if we were to just use AC_CHECK_LIB for dladdr1 as we did for dlopen in that it wouldn't readd the -ldl flag or is it the only way to write these extra checks?

Copy link

ContributorAuthor

freakboy3742Apr 30, 2025

Choose a reason for hiding this comment

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

Honestly, I don't know - I know enough autoconf to get by, but not much more.

Unfortunately, I don't have ready access to a development platform that would use thedladdr1 check (at least, I don't think I do), so it's difficult for me to experiment with this.

My concern would be whether it would add the flag toLIBS orLDFLAGS though - if it's added toLIBS, we'll be back in the same situation as before. Based on what I'm seeing in otherAC_CHECK_LIB usage, it looks like it would get added toLIBS by default, and any strategy that got it intoLDFLAGS would essentially be no better thanAS_VAR_APPEND.