This is an archive of the discontinued LLVM Phabricator instance.

[Driver] add -lresolv for all but Android.
ClosedPublic

Authored by kda on Jun 6 2022, 1:14 PM.

Details

Summary

As there 3 intercepts that depend on libresolv, link tests in ./configure scripts may be confuse by the presence of resolv symbols (i.e. dn_expand) even with -lresolv and get a runtime error.

Android provides the functionality in libc.

https://reviews.llvm.org/D122849
https://reviews.llvm.org/D126851

Diff Detail

Event Timeline

kda created this revision.Jun 6 2022, 1:14 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 6 2022, 1:14 PM
kda requested review of this revision.Jun 6 2022, 1:14 PM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptJun 6 2022, 1:14 PM
Herald added subscribers: Restricted Project, cfe-commits, MaskRay. · View Herald Transcript
MaskRay added a comment.EditedJun 6 2022, 1:19 PM

The description needs to mention the motivation. Is this meant for isOSLinux() && isGNUEnvironment() targets? Unless there is a case that sanitizer runtime uses libresolve.so while user code may not use -lresolv, this is not needed.

kda edited the summary of this revision. (Show Details)Jun 6 2022, 1:24 PM

yes, however I have no idea if we need !TC.getTriple().isOSFreeBSD() !TC.getTriple().isOSNetBSD() !TC.getTriple().isOSOpenBSD()) etc.
I guess if make mistake it will be noticed and fixed.

Please update clang/test/Driver/sanitizer-ld.c

MaskRay accepted this revision.Jun 6 2022, 1:57 PM
MaskRay added inline comments.
clang/lib/Driver/ToolChains/CommonArgs.cpp
843

Omit brace for simple single-line body.

This revision is now accepted and ready to land.Jun 6 2022, 1:57 PM

The interceptor (dn_expand) is only defined on linux && !android, while this code links libresolv on *bsd and others, too. I think it's ok, as long as all platforms with the interceptor are covered.

It would be great to mention the actual reason we do this in the commit message: because otherwise link tests in ./configure scripts may be confused by the presence of the dn_expand symbol even without -lresolv (because of the interceptor) and get a runtime error later.

kda edited the summary of this revision. (Show Details)Jun 6 2022, 3:40 PM

The interceptor (dn_expand) is only defined on linux && !android, while this code links libresolv on *bsd and others, too. I think it's ok, as long as all platforms with the interceptor are covered.

It would be great to mention the actual reason we do this in the commit message: because otherwise link tests in ./configure scripts may be confused by the presence of the dn_expand symbol even without -lresolv (because of the interceptor) and get a runtime error later.

Updated summary.

eugenis accepted this revision.Jun 6 2022, 3:41 PM

Typo: "runttime"

LGTM

kda edited the summary of this revision. (Show Details)Jun 6 2022, 3:42 PM
kda updated this revision to Diff 434634.Jun 6 2022, 3:43 PM
kda marked an inline comment as done.

removed braces on single statement from conditional

kda edited the summary of this revision. (Show Details)Jun 6 2022, 3:48 PM
This revision was landed with ongoing or failed builds.Jun 6 2022, 3:50 PM
This revision was automatically updated to reflect the committed changes.