[Lldb-commits] [lldb] e9040e8 - [lldb][PECOFF] Exclude alignment padding when reading section data
Hiroshi Yamauchi via lldb-commits
lldb-commits at lists.llvm.org
Fri Aug 4 13:45:24 PDT 2023
Author: Hiroshi Yamauchi
Date: 2023-08-04T13:38:30-07:00
New Revision: e9040e875d9252f726c41579f70663154718c3c6
URL: https://github.com/llvm/llvm-project/commit/e9040e875d9252f726c41579f70663154718c3c6
DIFF: https://github.com/llvm/llvm-project/commit/e9040e875d9252f726c41579f70663154718c3c6.diff
LOG: [lldb][PECOFF] Exclude alignment padding when reading section data
There can be zero padding bytes at a section end for file alignment in
PECOFF. Exclude those padding bytes when reading the section data.
Differential Revision: https://reviews.llvm.org/D157059
Added:
lldb/unittests/ObjectFile/PECOFF/TestSectionSize.cpp
Modified:
lldb/include/lldb/Symbol/ObjectFile.h
lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
lldb/source/Symbol/ObjectFile.cpp
lldb/unittests/ObjectFile/PECOFF/CMakeLists.txt
llvm/lib/ObjectYAML/COFFYAML.cpp
llvm/test/tools/yaml2obj/COFF/invalid-raw-data.yaml
llvm/test/tools/yaml2obj/COFF/xrelocs.yaml
Removed:
################################################################################
diff --git a/lldb/include/lldb/Symbol/ObjectFile.h b/lldb/include/lldb/Symbol/ObjectFile.h
index 9b006dfe05e7e4..2fc1cc582d2ceb 100644
--- a/lldb/include/lldb/Symbol/ObjectFile.h
+++ b/lldb/include/lldb/Symbol/ObjectFile.h
@@ -654,6 +654,12 @@ class ObjectFile : public std::enable_shared_from_this<ObjectFile>,
virtual size_t ReadSectionData(Section *section,
DataExtractor §ion_data);
+ // Returns the section data size. This is special-cased for PECOFF
+ // due to file alignment.
+ virtual size_t GetSectionDataSize(Section *section) {
+ return section->GetFileSize();
+ }
+
/// Returns true if the object file exists only in memory.
bool IsInMemory() const { return m_memory_addr != LLDB_INVALID_ADDRESS; }
diff --git a/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp b/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
index 77506b40ff5c36..3941d6be2457b9 100644
--- a/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
+++ b/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
@@ -1027,6 +1027,16 @@ SectionType ObjectFilePECOFF::GetSectionType(llvm::StringRef sect_name,
return eSectionTypeOther;
}
+size_t ObjectFilePECOFF::GetSectionDataSize(Section *section) {
+ // For executables, SizeOfRawData (getFileSize()) is aligned by
+ // FileAlignment and the actual section size is in VirtualSize
+ // (getByteSize()). See the comment on
+ // llvm::object::COFFObjectFile::getSectionSize().
+ if (m_binary->getPE32Header() || m_binary->getPE32PlusHeader())
+ return std::min(section->GetByteSize(), section->GetFileSize());
+ return section->GetFileSize();
+}
+
void ObjectFilePECOFF::CreateSections(SectionList &unified_section_list) {
if (m_sections_up)
return;
diff --git a/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h b/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
index 5d0465b4ee44f6..c59701fa65ec3f 100644
--- a/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
+++ b/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
@@ -263,6 +263,7 @@ class ObjectFilePECOFF : public lldb_private::ObjectFile {
llvm::StringRef GetSectionName(const section_header_t §);
static lldb::SectionType GetSectionType(llvm::StringRef sect_name,
const section_header_t §);
+ size_t GetSectionDataSize(lldb_private::Section *section) override;
typedef std::vector<section_header_t> SectionHeaderColl;
typedef SectionHeaderColl::iterator SectionHeaderCollIter;
diff --git a/lldb/source/Symbol/ObjectFile.cpp b/lldb/source/Symbol/ObjectFile.cpp
index bebc9589418cbb..364cb850a28a48 100644
--- a/lldb/source/Symbol/ObjectFile.cpp
+++ b/lldb/source/Symbol/ObjectFile.cpp
@@ -550,8 +550,8 @@ size_t ObjectFile::ReadSectionData(Section *section,
// The object file now contains a full mmap'ed copy of the object file
// data, so just use this
- return GetData(section->GetFileOffset(), section->GetFileSize(),
- section_data);
+ return GetData(section->GetFileOffset(), GetSectionDataSize(section),
+ section_data);
}
bool ObjectFile::SplitArchivePathWithObject(llvm::StringRef path_with_object,
diff --git a/lldb/unittests/ObjectFile/PECOFF/CMakeLists.txt b/lldb/unittests/ObjectFile/PECOFF/CMakeLists.txt
index 3ce5a7b9739cc4..cdcbdc8a745380 100644
--- a/lldb/unittests/ObjectFile/PECOFF/CMakeLists.txt
+++ b/lldb/unittests/ObjectFile/PECOFF/CMakeLists.txt
@@ -1,5 +1,6 @@
add_lldb_unittest(ObjectFilePECOFFTests
TestPECallFrameInfo.cpp
+ TestSectionSize.cpp
LINK_LIBS
lldbUtilityHelpers
diff --git a/lldb/unittests/ObjectFile/PECOFF/TestSectionSize.cpp b/lldb/unittests/ObjectFile/PECOFF/TestSectionSize.cpp
new file mode 100644
index 00000000000000..3ce1e20265f4d9
--- /dev/null
+++ b/lldb/unittests/ObjectFile/PECOFF/TestSectionSize.cpp
@@ -0,0 +1,78 @@
+//===-- TestSectionFileSize.cpp -------------------------------------------===//
+//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "gtest/gtest.h"
+
+#include "Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h"
+#include "Plugins/Process/Utility/lldb-x86-register-enums.h"
+#include "TestingSupport/SubsystemRAII.h"
+#include "TestingSupport/TestUtilities.h"
+
+#include "lldb/Core/Module.h"
+#include "lldb/Symbol/CallFrameInfo.h"
+#include "lldb/Symbol/UnwindPlan.h"
+#include "llvm/Testing/Support/Error.h"
+
+using namespace lldb_private;
+using namespace lldb;
+
+class SectionSizeTest : public testing::Test {
+ SubsystemRAII<FileSystem, ObjectFilePECOFF> subsystems;
+};
+
+TEST_F(SectionSizeTest, NoAlignmentPadding) {
+ llvm::Expected<TestFile> ExpectedFile = TestFile::fromYaml(
+ R"(
+--- !COFF
+OptionalHeader:
+ SectionAlignment: 4096
+ FileAlignment: 512
+header:
+ Machine: IMAGE_FILE_MACHINE_AMD64
+ Characteristics: [ IMAGE_FILE_EXECUTABLE_IMAGE, IMAGE_FILE_LARGE_ADDRESS_AWARE ]
+sections:
+ - Name: swiftast
+ VirtualSize: 496
+ SizeOfRawData: 512
+ Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_READ ]
+ SectionData: 11111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111110000
+
+symbols: []
+...
+)");
+ ASSERT_THAT_EXPECTED(ExpectedFile, llvm::Succeeded());
+
+ ModuleSP module_sp = std::make_shared<Module>(ExpectedFile->moduleSpec());
+ ObjectFile *object_file = module_sp->GetObjectFile();
+ ASSERT_NE(object_file, nullptr);
+
+ SectionList *section_list = object_file->GetSectionList();
+ ASSERT_NE(section_list, nullptr);
+
+ SectionSP swiftast_section;
+ size_t section_count = section_list->GetNumSections(0);
+ for (size_t i = 0; i < section_count; ++i) {
+ SectionSP section_sp = section_list->GetSectionAtIndex(i);
+ if (section_sp->GetName() == "swiftast") {
+ swiftast_section = section_sp;
+ break;
+ }
+ }
+ ASSERT_NE(swiftast_section.get(), nullptr);
+
+ DataExtractor section_data;
+ ASSERT_NE(object_file->ReadSectionData(swiftast_section.get(),
+ section_data),
+ 0);
+
+ // Check that the section data size is equal to VirtualSize (496)
+ // without the zero padding, instead of SizeOfRawData (512).
+ EXPECT_EQ(section_data.GetByteSize(), 496);
+}
+
diff --git a/llvm/lib/ObjectYAML/COFFYAML.cpp b/llvm/lib/ObjectYAML/COFFYAML.cpp
index 3fe2ea5af08f25..cd1db24f0d5d13 100644
--- a/llvm/lib/ObjectYAML/COFFYAML.cpp
+++ b/llvm/lib/ObjectYAML/COFFYAML.cpp
@@ -689,11 +689,12 @@ void MappingTraits<COFFYAML::Section>::mapping(IO &IO, COFFYAML::Section &Sec) {
return;
}
- // Uninitialized sections, such as .bss, typically have no data, but the size
- // is carried in SizeOfRawData, even though PointerToRawData is zero.
- if (Sec.SectionData.binary_size() == 0 && Sec.StructuredData.empty() &&
- NC->Characteristics & COFF::IMAGE_SCN_CNT_UNINITIALIZED_DATA)
- IO.mapOptional("SizeOfRawData", Sec.Header.SizeOfRawData);
+ IO.mapOptional("SizeOfRawData", Sec.Header.SizeOfRawData, 0U);
+
+ if (!Sec.StructuredData.empty() && Sec.Header.SizeOfRawData) {
+ IO.setError("StructuredData and SizeOfRawData can't be used together");
+ return;
+ }
IO.mapOptional("Relocations", Sec.Relocations);
}
diff --git a/llvm/test/tools/yaml2obj/COFF/invalid-raw-data.yaml b/llvm/test/tools/yaml2obj/COFF/invalid-raw-data.yaml
index 62445fa1858b86..1691d4a85dfa8b 100644
--- a/llvm/test/tools/yaml2obj/COFF/invalid-raw-data.yaml
+++ b/llvm/test/tools/yaml2obj/COFF/invalid-raw-data.yaml
@@ -1,5 +1,5 @@
# RUN: not yaml2obj %s -o %t 2>&1 | FileCheck %s
-# CHECK: YAML:18:5: error: unknown key 'SizeOfRawData'
+# CHECK: YAML:14:5: error: StructuredData and SizeOfRawData can't be used together
--- !COFF
OptionalHeader:
diff --git a/llvm/test/tools/yaml2obj/COFF/xrelocs.yaml b/llvm/test/tools/yaml2obj/COFF/xrelocs.yaml
index fbc22a1e99caed..cecef3b0fdad99 100644
--- a/llvm/test/tools/yaml2obj/COFF/xrelocs.yaml
+++ b/llvm/test/tools/yaml2obj/COFF/xrelocs.yaml
@@ -30,6 +30,7 @@
# CHECK-YAML-NEXT: Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_LNK_NRELOC_OVFL, IMAGE_SCN_MEM_READ ]
# CHECK-YAML-NEXT: Alignment: 16
# CHECK-YAML-NEXT: SectionData: '00000000000000000000000000000000'
+# CHECK-YAML-NEXT: SizeOfRawData: 16
# CHECK-YAML-NEXT: Relocations:
# CHECK-YAML-NEXT: - VirtualAddress: 0
# CHECK-YAML-NEXT: SymbolName: foo
More information about the lldb-commits
mailing list