Skip to content

[ELF] Make shouldAddProvideSym return values consistent when demoted to Undefined #111945

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Oct 11, 2024
Merged

[ELF] Make shouldAddProvideSym return values consistent when demoted to Undefined #111945

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6ca4990
[𝘀𝗽𝗿] initial version
MaskRay Oct 11, 2024
b66a6bc
update comment
MaskRay Oct 11, 2024
665ccc4
add common symbol test
MaskRay Oct 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion lld/ELF/LinkerScript.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1811,6 +1811,11 @@ void LinkerScript::addScriptReferencedSymbolsToSymTable() {
}

bool LinkerScript::shouldAddProvideSym(StringRef symName) {
// A Defined may be demoted to Undefined but the return value must stay the
Copy link
Collaborator

@smithp35 smithp35 Oct 11, 2024

Choose a reason for hiding this comment

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

I got a bit confused about whether the return value was this function's return value or the symbol's value. After thinking it through it is definitely the former.

A suggestion:

This function is called before and after garbage collection. To prevent undefined references the result of this function for a symbol must be the same for each call. We use isUsedInRegularObj to
prevent garbage collection demoting a Defined to an Undefnined. The exportDynamic condition, while not so accurate, allows PROVIDE to define a symbol referenced by a DSO.

// same. Use isUsedInRegularObj to ensure this. The exportDynamic condition,
// while not so accurate, allows PROVIDE to define a symbol referenced by a
// DSO.
Symbol *sym = elf::ctx.symtab->find(symName);
return sym && !sym->isDefined() && !sym->isCommon();
return sym && !sym->isDefined() && !sym->isCommon() &&
(sym->isUsedInRegularObj || sym->exportDynamic);
}
33 changes: 33 additions & 0 deletions lld/test/ELF/linkerscript/provide-defined.s
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
# REQUIRES: x86
## Test the GC behavior when the PROVIDE symbol is defined by a relocatable file.

# RUN: rm -rf %t && split-file %s %t && cd %t
# RUN: llvm-mc -filetype=obj -triple=x86_64 a.s -o a.o
# RUN: llvm-mc -filetype=obj -triple=x86_64 b.s -o b.o
# RUN: ld.lld -T a.t --gc-sections a.o b.o -o a
# RUN: llvm-readelf -s a | FileCheck %s

# CHECK: 1: {{.*}} 0 NOTYPE GLOBAL DEFAULT 1 _start
# CHECK-NEXT:2: {{.*}} 0 NOTYPE GLOBAL DEFAULT 2 f3
# CHECK-NOT: {{.}}

#--- a.s
.global _start, f1, f2, f3, bar
_start:
call f3

.section .text.f1,"ax"; f1:
.section .text.f2,"ax"; f2:
.section .text.f3,"ax"; f3:
.section .text.bar,"ax"; bar:

#--- b.s
call f2

#--- a.t
SECTIONS {
. = . + SIZEOF_HEADERS;
PROVIDE(f1 = bar+1);
PROVIDE(f2 = bar+2);
PROVIDE(f3 = bar+3);
}
Loading