Warn when --version-script has no effect - #1587
Warn when --version-script has no effect#1587Rachit Mehta (rachitmeht) wants to merge 2 commits into
Conversation
9351ccd to
f9337dd
Compare
| @@ -0,0 +1,34 @@ | |||
| # Test that version scripts emit warnings when they have no effect | |||
| # (i.e., when no dynamic symbol table will be created) | |||
There was a problem hiding this comment.
nit : use the test format for new tests.
| if (ThisConfig.isLinkPartial()) { | ||
| // Relocatable output (-r) never emits .dynsym. | ||
| ThisConfig.raise(Diag::warn_version_script_no_effect_relocatable) | ||
| << VersionScriptInput->decoratedPath(); |
There was a problem hiding this comment.
Should this be a warning under -Wlinker-script category ?
There was a problem hiding this comment.
Yes, it should, but there is -Wversion-script also ( not documented ), should it be warning under both category ?
| # RUN 4: Shared library should NOT warn (has .dynsym) | ||
| RUN: %link %linkopts -shared --version-script=%p/Inputs/vs1 -o %t_shared.so %t1_pic.o 2>&1 | %filecheck --allow-empty --check-prefix=NO-WARN-SHARED %s | ||
|
|
||
| NO-WARN-SHARED-NOT: warning: version script has no effect |
There was a problem hiding this comment.
no version script filename is printed ?
what about if you have a linker script that has a version block ?
There was a problem hiding this comment.
I think, the case of version block in linkerscript will be solved automatically if this PR #1672 is correct. Otherwise we have to add same check in two different paths.
| "emit a dynamic symbol table") | ||
| DIAG(warn_version_script_no_effect_relocatable, DiagnosticEngine::Warning, | ||
| "%0: warning: version script has no effect: relocatable output (-r) does " | ||
| "not emit a dynamic symbol table") |
There was a problem hiding this comment.
do we need does not emit a dynamic symbol table ? remove the reason ?
| "Error parsing version script %0") | ||
| DIAG(warn_version_script_no_dynsym, DiagnosticEngine::Warning, | ||
| "%0: warning: version script has no effect: static executable does not " | ||
| "emit a dynamic symbol table") |
There was a problem hiding this comment.
not sure about static executables and version script. We should check
ld.eld <object> .... <object> -static -pie
There was a problem hiding this comment.
Verified: static pie do have .dynsym table but it doesn't contain any user defined symbols, so, version-script has no effect on it. We should warn in this case also.
There was a problem hiding this comment.
Actually, for every pie combination, it is not affected by version script.
except for -pie --export-dynamic.
| # RUN 6: Verify the warnings include the version script path | ||
| RUN: %link %linkopts -static --entry=main --version-script=%p/Inputs/vs1 -o %t_path.out %t1.o 2>&1 | %filecheck --check-prefix=CHECK-PATH %s | ||
|
|
||
| CHECK-PATH: {{.*}}/Inputs/vs1: warning: version script has no effect |
There was a problem hiding this comment.
will this test pass on windows ?
Emit warnings when version scripts are specified but will be ignored because no dynamic symbol table (.dynsym) will be created. Warns in these cases: - Static executable (without -pie or --force-dynamic) - Relocatable output (-r) No warning for PIE executables, shared libraries, or static executables with --force-dynamic, as these correctly have effect of version-script. Adds test coverage in VersionScriptNoEffect.test with 6 test cases. Resolves qualcomm#1444 Signed-off-by: Rachit Mehta <rachmeht@qti.qualcomm.com>
f9337dd to
b5769bc
Compare
Version scripts control symbol visibility in .dynsym, but have no effect when no user symbols are exported. Add warning for: - PIE links: symbols suppressed unless --export-dynamic overrides - Static/relocatable: no .dynsym unless --force-dynamic Warning fires at version script parse time, guarded by -Wlinker-script. Resolves qualcomm#1444
b5769bc to
19d8036
Compare
4 participants
Version scripts control symbol visibility in the dynamic symbol table (
.dynsym). When output types don't create.dynsym, version scripts are silently ignored, So, analyzed when.dynsymis created:isShared() || isPIE() || (isStatic() && forceDynamic())Negated these conditions to warn when version scripts have no effect:
isCodeStatic() && !isPIE() && !forceDynamic()isLinkPartial()(in this case also no.dynsymis created)Implemented in
ObjectLinker::parseVersionScript()after version script is recorded but before parsing.Changes:
DiagLDScript.inc: Added 2 warning diagnosticsObjectLinker.cpp: Added warning checksVersionScriptNoEffect.test: 6 test casesResolves #1444