From f32b83eb08e9ee158d3037b2114357187fd45a05 Mon Sep 17 00:00:00 2001 From: Nelson Billing Date: Tue, 3 Dec 2019 15:00:30 -0800 Subject: [PATCH] Enable reading DWARF4 CIEs with 32 bit addresses. - Reading DWARF4 CIEs was added in https://chromium-review.googlesource.com/c/breakpad/breakpad/+/406012 but it was only enabled for 64bit builds, since it would error out if the CIE address size was not 8 bytes. - Added a unit test to ensure that 32bit continues to work. Change-Id: I824bb40cdf12056d39da335adb55ed315970fb88 Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/1941034 Reviewed-by: Ivan Penkov Reviewed-by: Mark Mentovai --- src/common/dwarf/dwarf2reader.cc | 27 +++++++------ src/common/dwarf/dwarf2reader.h | 5 +++ src/common/dwarf/dwarf2reader_cfi_unittest.cc | 39 ++++++++++++++++--- 3 files changed, 53 insertions(+), 18 deletions(-) diff --git a/src/common/dwarf/dwarf2reader.cc b/src/common/dwarf/dwarf2reader.cc index 27f3a83e..3e6a3e89 100644 --- a/src/common/dwarf/dwarf2reader.cc +++ b/src/common/dwarf/dwarf2reader.cc @@ -2330,21 +2330,15 @@ bool CallFrameInfo::ReadCIEFields(CIE *cie) { } if (cie->version >= 4) { - uint8_t address_size = *cursor++; - if (address_size != 8) { - // TODO(scottmg): Only supporting x64 for now. - reporter_->UnexpectedAddressSize(cie->offset, address_size); + cie->address_size = *cursor++; + if (cie->address_size != 8 && cie->address_size != 4) { + reporter_->UnexpectedAddressSize(cie->offset, cie->address_size); return false; } - uint8_t segment_size = *cursor++; - if (segment_size != 0) { - // TODO(scottmg): Only supporting x64 for now. - // I would have perhaps expected 4 here, but LLVM emits a 0, near - // http://llvm.org/docs/doxygen/html/MCDwarf_8cpp_source.html#l00606. As - // we are not using the value, only succeed for now if it's the expected - // 0. - reporter_->UnexpectedSegmentSize(cie->offset, segment_size); + cie->segment_size = *cursor++; + if (cie->segment_size != 0) { + reporter_->UnexpectedSegmentSize(cie->offset, cie->segment_size); return false; } } @@ -2606,6 +2600,15 @@ bool CallFrameInfo::Start() { if (!ReadCIEFields(&cie)) continue; + // TODO(nbilling): This could lead to strange behavior if a single buffer + // contained a mixture of DWARF versions as well as address sizes. Not + // sure if it's worth handling such a case. + + // DWARF4 CIE specifies address_size, so use it for this call frame. + if (cie.version >= 4) { + reader_->SetAddressSize(cie.address_size); + } + // We now have the values that govern both the CIE and the FDE. cie.cie = &cie; fde.cie = &cie; diff --git a/src/common/dwarf/dwarf2reader.h b/src/common/dwarf/dwarf2reader.h index cf3ba3cd..902d9ef1 100644 --- a/src/common/dwarf/dwarf2reader.h +++ b/src/common/dwarf/dwarf2reader.h @@ -999,6 +999,11 @@ class CallFrameInfo { // or not we saw a 'z' augmentation string; its default value is // DW_EH_PE_absptr, which is what normal DWARF CFI uses. DwarfPointerEncoding pointer_encoding; + + // These were only introduced in DWARF4, so will not be set in older + // versions. + uint8 address_size; + uint8 segment_size; }; // A frame description entry (FDE). diff --git a/src/common/dwarf/dwarf2reader_cfi_unittest.cc b/src/common/dwarf/dwarf2reader_cfi_unittest.cc index 38cc7ea3..35d4f340 100644 --- a/src/common/dwarf/dwarf2reader_cfi_unittest.cc +++ b/src/common/dwarf/dwarf2reader_cfi_unittest.cc @@ -608,11 +608,11 @@ TEST_F(CFI, CIEVersion3ReturnColumn) { } TEST_F(CFI, CIEVersion4AdditionalFields) { - CFISection section(kBigEndian, 4); + CFISection section(kBigEndian, 8); Label cie; section .Mark(&cie) - // CIE version 4 with expected address and segment size. + // CIE version 4 with expected address (64bit) and segment size. .CIEHeader(0x0ab4758d, 0xc010fdf7, 0x89, 4, "", true, 8, 0) .FinishEntry() // FDE, citing that CIE. @@ -631,7 +631,36 @@ TEST_F(CFI, CIEVersion4AdditionalFields) { string contents; EXPECT_TRUE(section.GetContents(&contents)); ByteReader byte_reader(ENDIANNESS_BIG); - byte_reader.SetAddressSize(4); + CallFrameInfo parser(reinterpret_cast(contents.data()), + contents.size(), + &byte_reader, &handler, &reporter); + EXPECT_TRUE(parser.Start()); +} + +TEST_F(CFI, CIEVersion4AdditionalFields32BitAddress) { + CFISection section(kBigEndian, 4); + Label cie; + section + .Mark(&cie) + // CIE version 4 with expected address (32bit) and segment size. + .CIEHeader(0x0ab4758d, 0xc010fdf7, 0x89, 4, "", true, 4, 0) + .FinishEntry() + // FDE, citing that CIE. + .FDEHeader(cie, 0x86763f2b, 0x2a66dc23) + .FinishEntry(); + + PERHAPS_WRITE_DEBUG_FRAME_FILE("CIEVersion3ReturnColumn", section); + + { + InSequence s; + EXPECT_CALL(handler, Entry(_, 0x86763f2b, 0x2a66dc23, 4, "", 0x89)) + .WillOnce(Return(true)); + EXPECT_CALL(handler, End()).WillOnce(Return(true)); + } + + string contents; + EXPECT_TRUE(section.GetContents(&contents)); + ByteReader byte_reader(ENDIANNESS_BIG); CallFrameInfo parser(reinterpret_cast(contents.data()), contents.size(), &byte_reader, &handler, &reporter); @@ -659,7 +688,6 @@ TEST_F(CFI, CIEVersion4AdditionalFieldsUnexpectedAddressSize) { string contents; EXPECT_TRUE(section.GetContents(&contents)); ByteReader byte_reader(ENDIANNESS_BIG); - byte_reader.SetAddressSize(8); CallFrameInfo parser(reinterpret_cast(contents.data()), contents.size(), &byte_reader, &handler, &reporter); @@ -667,7 +695,7 @@ TEST_F(CFI, CIEVersion4AdditionalFieldsUnexpectedAddressSize) { } TEST_F(CFI, CIEVersion4AdditionalFieldsUnexpectedSegmentSize) { - CFISection section(kBigEndian, 4); + CFISection section(kBigEndian, 8); Label cie; section @@ -685,7 +713,6 @@ TEST_F(CFI, CIEVersion4AdditionalFieldsUnexpectedSegmentSize) { string contents; EXPECT_TRUE(section.GetContents(&contents)); ByteReader byte_reader(ENDIANNESS_BIG); - byte_reader.SetAddressSize(8); CallFrameInfo parser(reinterpret_cast(contents.data()), contents.size(), &byte_reader, &handler, &reporter);