search

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

read/macho: fix data for segments in dyld caches #673

Closed philipc closed 7 months ago

philipc commented 7 months ago

Note that this is only for operations on the segment itself. Sections within the segment were already using the correct data.

philipc commented 7 months ago

@mstange Can you review this, and do you know if the following code needs fixing too? Edit: I've fixed this too now. https://github.com/gimli-rs/object/blob/9650d6429667938a01b41a19e6de32f8f36b1aab/src/read/macho/section.rs#L196

mstange commented 7 months ago

I'll take a look. I'm a bit surprised because I thought I was using segment data for the assembly view, here: https://github.com/mstange/samply/blob/6ec2c01f8e4f30c570321b8516fba975e62ab135/samply-symbols/src/binary_image.rs#L279-L280

philipc commented 7 months ago

The code looks wrong to me, but I haven't tested it. I'll see if I can put together a test.

philipc commented 7 months ago

I obtained a login to a macOS 13.6.6 system. I changed data_and_offset_for_address to also return the index of the subcache, and added an assert in parse_dyld_cache_image to check that the image and segment had the same subcache index. I then ran objdump on the cache, and the assert never triggered. Conclusion: I need a different macOS version in order to be able to test this.

mstange commented 7 months ago

I then ran objdump on the cache, and the assert never triggered.

I see the same on macOS 14.4.1. So maybe what I observed in the original commit on macOS 12.0.1 no longer happens:

For example, on macOS 12.0.1, the image for libsystem_malloc.dylib for the arm64e architecture has its TEXT segment in the root cache and the LINKEDIT segment in the .1 subcache - there's a single __LINKEDIT segment which is shared between all images across both files. The remaining libsystem_malloc.dylib segments are in the same file as the __TEXT segment.

I've sent you an email with the macOS 12.0.1 arm64e dyld shared cache.

mstange commented 7 months ago

Ok, so I think the one case where the segment data would have been wrong is if you had asked for the data of the __LINKEDIT segment on a cache from macOS 12, for any image that's stored in the root cache. I never noticed anything wrong because I only look at the data of the __TEXT segment, which is always located in the same subcache as the image (almost by definition, because the load commands are always at the start of the __TEXT segment, IIRC).

Anyway, I think your fix makes sense.

philipc commented 7 months ago

Thanks for looking into this, and for the email. I did some manual testing to check that this had an effect.