gimli-rs / object

A unified interface for reading and writing object file formats
https://docs.rs/object/
Apache License 2.0
647 stars 144 forks source link

write/elf: want_section_symbol is a hack #587

Closed philipc closed 10 months ago

philipc commented 10 months ago

As part of working on #585, it would be nice to delete this hack: https://github.com/gimli-rs/object/blob/32888b446a6c1caf376b5d3cef6a2bb1e1b58262/src/write/elf/object.rs#L188-L198

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 commented 10 months ago

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 commented 10 months ago

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
                }