PE inter-section relocations #4128
Conversation
crates/linker/src/elf.rs
Outdated
| @@ -1244,6 +1232,10 @@ fn surgery_elf_help( | |||
| RelocationKind::Relative | RelocationKind::PltRelative => { | |||
| target_offset - virt_base as i64 + rel.1.addend() | |||
| } | |||
| RelocationKind::Absolute => { | |||
There was a problem hiding this comment.
This shouldn't work. It is missing the need for a dynamic relocation. An absolute relocation is a pointer to some offest. That offset is not truly known until the app is loaded into memory with address space layout randomization. So this will just point to an offset based on the file and not the true offset in memory.
There was a problem hiding this comment.
hmm, it does work, but maybe just for this example? it points to a fixed virtual address, so it's not quite based on the file, but yeah if the whole thing is relocated then that might cause issues.
So, how do I add a dynamic relocation (conceptually)? do we do that anywhere yet?
There was a problem hiding this comment.
Interesting that it works. I remember testing something similar in a different PR (#3623) and that is when I realized all of this. This is the main info about what I think needs to be done: #3609 (comment). We do not do anything with dynamic relocations currently. They are stored in a sligtly compressed format where you essentially have to run a basic instruction set to generate a table. We would just be adding a new entry to the table. So minorly annoying, but shouldn't be super hard. The painful part is that adding new bytes may shift a lot of stuff around in the binary.
There was a problem hiding this comment.
Ok. I just look at the zig example, there is no absolute relocation in the app. It is a PC relative relocation. So that should work. Yeah, the test passes with the absolute relocation code being commented out.
There was a problem hiding this comment.
If you take that zig example and put multiple strings in the list, it will be a real reproduction of the issue that requires absolute relocations to be turned into dynamic relocations.
There was a problem hiding this comment.
hmm right. I was hoping that we could do this more incrementally. Also with the two strings there is still no absolute relocation for me, just a relative one with the familiar empty string symbol name. I'll remove the absolute relocation bit, and then for elf the only change is the renaming of LittleEndian to LE. For windows having just the single string requires some real changes. Having 2 strings there also does not yet work so that will also be fun to figure out (later)
functional changes are small
but I renamed some imports, and cleaned up the windows tests we it's easier to add more zig host/app tests