Skip to content
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.

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

write/elf: want_section_symbol is a hack #587

Closed
philipc opened this issue Nov 2, 2023 · 2 comments · Fixed by #590
Closed

write/elf: want_section_symbol is a hack #587

philipc opened this issue Nov 2, 2023 · 2 comments · Fixed by #590

Comments

@philipc
Copy link
Contributor

philipc commented Nov 2, 2023

As part of working on #585, it would be nice to delete this hack:

// Use section symbols for relocations where required to avoid preemption.
// Otherwise, the linker will fail with:
// relocation R_X86_64_PC32 against symbol `SomeSymbolName' can not be used when
// making a shared object; recompile with -fPIC
let symbol = &self.symbols[relocation.symbol.0];
if want_section_symbol(relocation, symbol) {
if let Some(section) = symbol.section.id() {
relocation.addend += symbol.value as i64;
relocation.symbol = self.section_symbol(section);
}
}

The problem that this hack works around is that cranelift is emitting R_X86_64_PC32 relocations for preemptible symbols. The proper solution is that either cranelift should not emit these relocations (and use things like PLT relocations instead), or cranelift should tell object that the symbol is not preemptible (although there's no way to do that currently).

cranelift already has Linkage::Preemptible to control whether it emits these relocations. However, this is currently being passed on to object as a request for a weak symbol, which is a different concept:
https://github.com/bytecodealliance/wasmtime/blob/e0bfa7336de20f76048edbdc0157ee637a2c5fea/cranelift/object/src/backend.rs#L902-L903

and the reason that code was written like that is because rustc_codegen_cranelift translates rust's weak linkage into Linkage::Preemptible, which I think is wrong:
https://github.com/rust-lang/rustc_codegen_cranelift/blob/48ca2d9703742149aa33b3f84ae933d063213d19/src/linkage.rs#L16

An initial solution would be to delete the hack and change object to set STV_PROTECTED for all exported symbols so that they are never preemptible, which matches what cranelift is expecting. We can then later add a way to control this, and fix cranelift and rustc_codegen_cranelift to agree with each other about weak and preemptible symbols.

cc @bjorn3

@philipc
Copy link
Contributor Author

philipc commented Nov 2, 2023

Using STV_PROTECTED isn't a solution either, since ld.bfd still generates the error (because function pointers should go via the GOT even for protected symbols so that pointer comparisons work).

@philipc
Copy link
Contributor Author

philipc commented Nov 2, 2023

For x86-64, reducing the use of section symbols to just the following case is enough for rustc_codegen_cranelift's test.sh:

                (RelocationKind::Relative, RelocationEncoding::X86Branch, 32) => {
                    if symbol.scope == SymbolScope::Dynamic && !symbol.is_undefined() {
                        want_section_symbol = true;
                    }
                    elf::R_X86_64_PC32
                }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
1 participant