Skip to content

Commit 2e99f09

Browse files
committed
Remove any uses of file ranges.
The text bytes are only used by macOS "Compact Unwind Info"-guided unwinding. Before #11, we were computing `offset_from_base` in that code based on the text segment/section start SVMA and the base SVMA. In #11 this code was changed to use the file range, but this is actually incorrect; file offsets are not always equivalent to relative-to-base addresses, for example in the dyld shared cache. Furthermore, in samply's case, we're getting these bytes straight from the other process's memory, so we're not interested in any information about the file anyway. This PR changes things back to deal in terms of SVMAs.
1 parent f8dddda commit 2e99f09

File tree

5 files changed

+30
-47
lines changed

5 files changed

+30
-47
lines changed

Readme.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ let module = Module::new(
8686
unwind_info: Some(vec![/* __unwind_info */]),
8787
eh_frame_svma: Some(0x100237f80..0x100237ffc),
8888
eh_frame: Some(vec![/* __eh_frame */]),
89-
text_segment_file_range: Some(0x1003fc000..0x100634000),
89+
text_segment_svma: Some(0x1003fc000..0x100634000),
9090
text_segment: Some(vec![/* __TEXT */]),
9191
..Default::default()
9292
},

src/lib.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@
7070
//! unwind_info: Some(vec![/* __unwind_info */]),
7171
//! eh_frame_svma: Some(0x100237f80..0x100237ffc),
7272
//! eh_frame: Some(vec![/* __eh_frame */]),
73-
//! text_segment_file_range: Some(0x1003fc000..0x100634000),
73+
//! text_segment_svma: Some(0x1003fc000..0x100634000),
7474
//! text_segment: Some(vec![/* __TEXT */]),
7575
//! ..Default::default()
7676
//! },

src/unwinder.rs

Lines changed: 25 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -395,7 +395,8 @@ impl<
395395
} => {
396396
// eprintln!("unwinding with cui and eh_frame in module {}", module.name);
397397
let text_bytes = text_data.as_ref().and_then(|data| {
398-
let offset_from_base = u32::try_from(data.svma_range.start).ok()?;
398+
let offset_from_base =
399+
u32::try_from(data.svma_range.start.checked_sub(module.base_svma)?).ok()?;
399400
Some(TextBytes::new(offset_from_base, &data.bytes[..]))
400401
});
401402
let stubs_range = if let Some(stubs_range) = stubs {
@@ -618,15 +619,19 @@ impl<D: Deref<Target = [u8]>> ModuleUnwindDataInternal<D> {
618619
// multiple executable sections such as `__text`, `__stubs`, and `__stub_helper`. If we
619620
// don't have the full `__TEXT` segment contents, we can fall back to the contents of
620621
// just the `__text` section.
621-
let text_data = section_info
622-
.segment_data(b"__TEXT")
623-
.zip(section_info.segment_file_range(b"__TEXT"))
624-
.or_else(|| {
625-
section_info
626-
.section_data(b"__text")
627-
.zip(section_info.section_file_range(b"__text"))
628-
})
629-
.map(|(bytes, svma_range)| TextByteData { bytes, svma_range });
622+
let text_data = if let (Some(bytes), Some(svma_range)) = (
623+
section_info.segment_data(b"__TEXT"),
624+
section_info.segment_svma_range(b"__TEXT"),
625+
) {
626+
Some(TextByteData { bytes, svma_range })
627+
} else if let (Some(bytes), Some(svma_range)) = (
628+
section_info.section_data(b"__text"),
629+
section_info.section_svma_range(b"__text"),
630+
) {
631+
Some(TextByteData { bytes, svma_range })
632+
} else {
633+
None
634+
};
630635
ModuleUnwindDataInternal::CompactUnwindInfoAndEhFrame {
631636
unwind_info,
632637
eh_frame: eh_frame.map(Arc::new),
@@ -763,14 +768,11 @@ pub trait ModuleSectionInfo<D> {
763768
/// Get the given section's memory range, as stated in the module.
764769
fn section_svma_range(&mut self, name: &[u8]) -> Option<Range<u64>>;
765770

766-
/// Get the given section's file range in the module.
767-
fn section_file_range(&mut self, name: &[u8]) -> Option<Range<u64>>;
768-
769771
/// Get the given section's data.
770772
fn section_data(&mut self, name: &[u8]) -> Option<D>;
771773

772-
/// Get the given segment's file range in the module.
773-
fn segment_file_range(&mut self, _name: &[u8]) -> Option<Range<u64>> {
774+
/// Get the given segment's memory range, as stated in the module.
775+
fn segment_svma_range(&mut self, _name: &[u8]) -> Option<Range<u64>> {
774776
None
775777
}
776778

@@ -804,7 +806,7 @@ pub struct ExplicitModuleSectionInfo<D> {
804806
/// This is used to detect whether we need to do instruction analysis for an address.
805807
pub text_svma: Option<Range<u64>>,
806808
/// The data of the `__text` or `.text` section. This is where most of the compiled code is
807-
/// stored.
809+
/// stored. For mach-O binaries, this does not need to be supplied if `text_segment` is supplied.
808810
///
809811
/// This is used to handle function prologues and epilogues in some cases.
810812
pub text: Option<D>,
@@ -844,11 +846,9 @@ pub struct ExplicitModuleSectionInfo<D> {
844846
pub eh_frame_hdr: Option<D>,
845847
/// The data of the `.debug_frame` section. The related address range is not needed.
846848
pub debug_frame: Option<D>,
847-
/// The file range of the `__TEXT` segment of mach-O binaries, or the `__text` section if the
848-
/// segment is unavailable.
849-
pub text_segment_file_range: Option<Range<u64>>,
850-
/// The data of the `__TEXT` segment of mach-O binaries, or the `__text` section if the segment
851-
/// is unavailable.
849+
/// The address range of the `__TEXT` segment of mach-O binaries, if available.
850+
pub text_segment_svma: Option<Range<u64>>,
851+
/// The data of the `__TEXT` segment of mach-O binaries, if available.
852852
pub text_segment: Option<D>,
853853
}
854854

@@ -871,9 +871,6 @@ where
871871
_ => None,
872872
}
873873
}
874-
fn section_file_range(&mut self, _name: &[u8]) -> Option<Range<u64>> {
875-
None
876-
}
877874
fn section_data(&mut self, name: &[u8]) -> Option<D> {
878875
match name {
879876
b"__text" | b".text" => self.text.take(),
@@ -884,9 +881,9 @@ where
884881
_ => None,
885882
}
886883
}
887-
fn segment_file_range(&mut self, name: &[u8]) -> Option<Range<u64>> {
884+
fn segment_svma_range(&mut self, name: &[u8]) -> Option<Range<u64>> {
888885
match name {
889-
b"__TEXT" => self.text_segment_file_range.take(),
886+
b"__TEXT" => self.text_segment_svma.take(),
890887
_ => None,
891888
}
892889
}
@@ -926,21 +923,14 @@ mod object {
926923
Some(section.address()..section.address() + section.size())
927924
}
928925

929-
fn section_file_range(&mut self, name: &[u8]) -> Option<Range<u64>> {
930-
let section = self.section_by_name_bytes(name)?;
931-
let (start, size) = section.file_range()?;
932-
Some(start..start + size)
933-
}
934-
935926
fn section_data(&mut self, name: &[u8]) -> Option<D> {
936927
let section = self.section_by_name_bytes(name)?;
937928
section.data().ok().map(|data| data.into())
938929
}
939930

940-
fn segment_file_range(&mut self, name: &[u8]) -> Option<Range<u64>> {
931+
fn segment_svma_range(&mut self, name: &[u8]) -> Option<Range<u64>> {
941932
let segment = self.segments().find(|s| s.name_bytes() == Ok(Some(name)))?;
942-
let (start, size) = segment.file_range();
943-
Some(start..start + size)
933+
Some(segment.address()..segment.address() + segment.size())
944934
}
945935

946936
fn segment_data(&mut self, name: &[u8]) -> Option<D> {

tests/integration_tests/common.rs

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -26,12 +26,6 @@ where
2626
Some(section.address()..section.address() + section.size())
2727
}
2828

29-
fn section_file_range(&mut self, name: &[u8]) -> Option<Range<u64>> {
30-
let section = self.0.section_by_name_bytes(name)?;
31-
let (start, size) = section.file_range()?;
32-
Some(start..start + size)
33-
}
34-
3529
fn section_data(&mut self, name: &[u8]) -> Option<Vec<u8>> {
3630
match self.0.section_by_name_bytes(name) {
3731
Some(section) => section.data().ok().map(|data| data.to_owned()),
@@ -43,13 +37,12 @@ where
4337
}
4438
}
4539

46-
fn segment_file_range(&mut self, name: &[u8]) -> Option<Range<u64>> {
40+
fn segment_svma_range(&mut self, name: &[u8]) -> Option<Range<u64>> {
4741
let segment = self
4842
.0
4943
.segments()
5044
.find(|s| s.name_bytes() == Ok(Some(name)))?;
51-
let (start, size) = segment.file_range();
52-
Some(start..start + size)
45+
Some(segment.address()..segment.address() + segment.size())
5346
}
5447

5548
fn segment_data(&mut self, name: &[u8]) -> Option<Vec<u8>> {

tests/integration_tests/macos.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ fn test_root_doc_comment() {
9292
stub_helper_svma: Some(0x1001d309c..0x1001d3438),
9393
eh_frame_svma: Some(0x100237f80..0x100237ffc),
9494
got_svma: Some(0x100238000..0x100238010),
95-
text_segment_file_range: Some(0x1003fc000..0x100634000),
95+
text_segment_svma: Some(0x1003fc000..0x100634000),
9696
text_segment: Some(vec![]),
9797
..Default::default()
9898
},

0 commit comments

Comments
 (0)