Skip to content

fix: COMMON/ABS symbols are now defined in .dynsym #1685

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

Open
ostylk wants to merge 2 commits into
base: main
Choose a base branch
from
Open

fix: COMMON/ABS symbols are now defined in .dynsym #1685

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e6a1d5a
fix: COMMON symbols are now defined in .dynsym
ostylk Mar 11, 2026
f30b4fb
fix: ABS symbols are now defined in .dynsym
ostylk Mar 11, 2026
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
36 changes: 36 additions & 0 deletions libwild/src/elf_writer.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3857,6 +3857,42 @@ fn write_regular_object_dynamic_symbol_definition<'data>(
format!("Failed to copy dynamic {}", layout.symbol_debug(symbol_id))
})?;
}
} else if sym.is_common(LittleEndian) {
let symbol_id = sym_def.symbol_id;
let resolution = layout.local_symbol_resolution(symbol_id).with_context(|| {
format!(
"Tried to write dynamic symbol definition without a resolution: {}",
layout.symbol_debug(symbol_id)
)
})?;

let mut sym_value = resolution.value();

// As common symbols are denoted by setting shndx=SHN_COMMON which is a special section,
// we need to put them manually into BSS/TBSS sections depending on whether they are thread
// local or not.
let section_id = if sym.st_type() == STT_TLS {
sym_value -= layout.tls_start_address();
output_section_id::TBSS
} else {
output_section_id::BSS
};
let section_id = layout.output_sections.primary_output_section(section_id);

dynamic_symbol_writer
.copy_symbol(sym, name, section_id, sym_value, ValueFlags::empty())
.with_context(|| {
format!("Failed to copy dynamic {}", layout.symbol_debug(symbol_id))
})?;
Comment on lines +3860 to +3886
Copy link
Member

@mati865 mati865 Mar 12, 2026

Choose a reason for hiding this comment

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

This branch seems to duplicate a lot of code from write_symbols. Would it be possible to extract common parts?

Copy link
Author

@ostylk ostylk Mar 12, 2026

Choose a reason for hiding this comment

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

Hmm, this refactoring looked a bit tricky for me. I attempted it here: ostylk@00abc82 . I tried extracting the section_id/symbol_value resolution logic.

Do you think this approach looks good? I am currently a bit unsure (as some special case logic is now also applied for COMMON symbols but that may be desirable?) so I didn't include that commit in this PR.
At least the unit tests pass.

Copy link
Member

@davidlattimore davidlattimore Mar 12, 2026

Choose a reason for hiding this comment

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

It seems to me that there's probably already a bit of duplication between writing of dynamic symbols and writing of debug symbols even prior to this PR. Given that, if we're going to attempt to deduplicate it, it'd probably make sense to do that as a separate PR. So it's fine with me to leave this bit as-is for now.

Copy link
Member

@mati865 mati865 Mar 12, 2026

Choose a reason for hiding this comment

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

Thanks for the attempt. Since this proves hard we can do it separately.

} else if sym.is_absolute(LittleEndian) {
dynamic_symbol_writer
.copy_absolute_symbol(sym, name, ValueFlags::empty())
.with_context(|| {
format!(
"Failed to absolute {}",
layout.symbol_debug(sym_def.symbol_id)
)
})?;
} else {
dynamic_symbol_writer
.copy_symbol_shndx(sym, name, 0, 0, ValueFlags::empty())
Expand Down
2 changes: 2 additions & 0 deletions wild/tests/integration_tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3547,6 +3547,8 @@ fn integration_test(
"non-alloc.s",
"gnu-unique.c",
"symbol-versions.c",
"common-shared.c",
"absolute-symbol.s",
"simple-version-script.c",
"mixed-verdef-verneed.c",
"copy-relocations.c",
Expand Down
15 changes: 15 additions & 0 deletions wild/tests/sources/absolute-symbol.s
View file
Open in desktop
Copy link
Member

@lapla-cogito lapla-cogito Mar 13, 2026

Choose a reason for hiding this comment

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

You can simply write this test as an assembly file (there are other test files with *.s).

Copy link
Author

@ostylk ostylk Mar 14, 2026

Choose a reason for hiding this comment

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

done

Copy link
Member

@lapla-cogito lapla-cogito Mar 14, 2026

Choose a reason for hiding this comment

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

Assembly tests that also run on RISC-V must enclose comments in /* */ (see https://github.com/wild-linker/wild/blob/main/wild/tests/sources/exclude-section.s).

Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
//#LinkArgs:-shared -z now
//#RunEnabled:false
//#DiffIgnore:section.got
//#ExpectSym:abs_sym address=0xCAFECAFE
//#ExpectDynSym:abs_sym address=0xCAFECAFE

// TODO: checkout those differences later
//#DiffIgnore:segment.RISCV_ATTRIBUTES.alignment
//#DiffIgnore:segment.RISCV_ATTRIBUTES.flags
//#DiffIgnore:riscv_attributes..riscv.attributes
//#DiffIgnore:riscv_attributes.arch
//#DiffIgnore:riscv_attributes.stack_align

.global abs_sym
.set abs_sym, 0xCAFECAFE
5 changes: 5 additions & 0 deletions wild/tests/sources/common-shared-1.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
extern int data[];
int data[100];

extern __thread int tvar[];
__thread int tvar[100] __attribute__((common));
15 changes: 15 additions & 0 deletions wild/tests/sources/common-shared.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
//#CompArgs:-fcommon
//#Object:common-shared-1.c
//#LinkArgs:-shared -z now
//#RunEnabled:false
//#DiffIgnore:section.got
//#ExpectSym:data section=".bss",size=400
//#ExpectDynSym:data section=".bss",size=400
//#ExpectSym:tvar section=".tbss",size=400,address=0
//#ExpectDynSym:tvar section=".tbss",size=400,address=0

extern int data[];
int data[10];

extern __thread int tvar[];
__thread int tvar[10] __attribute__((common));
Loading