gimli-rs / object

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

Reverse the order of emitting relocations on MachO #702

Closed bjorn3 closed 4 months ago

bjorn3 commented 4 months ago

This prevents a crash of Apple's new linker.

philipc commented 4 months ago

This probably should be treated as a breaking change. What do you think about trying to avoid that by detecting if it needs to be reversed? Too magical?

                let need_reverse = |relocs: &[Relocation]| {
                    let Some(first) = relocs.first() else { return false; };
                    let Some(last) = relocs.last() else { return false; };
                    first.offset < last.offset
                }; 
                if need_reverse(&section.relocations) {
                    for reloc in section.relocations.iter().rev() {
                        write_reloc(reloc)?;
                    }
                } else {
                    for reloc in &section.relocations {
                        write_reloc(reloc)?;
                    }
                }

Also, out of interest, I checked that just swapping the ARM64_RELOC_GOT_LOAD_PAGE21/ ARM64_RELOC_GOT_LOAD_PAGEOFF12 pairs is enough for the cg_clif tests to pass. I think it is safer to reverse everything though.

bjorn3 commented 4 months ago

Too magical?

Maybe? I applied your suggestion anyway.

bjorn3 commented 4 months ago

Would it be possible to get a patch release for this? That would allow me to fix cg_clif on arm64 macOS quicker as I wouldn't need to get a Cranelift PR for bumping the object version merged and wait a couple of weeks for the next Cranelift release.

philipc commented 4 months ago

Yes, I'll do a release tomorrow.

philipc commented 4 months ago

Released 0.36.1