Merge pull request #7160 from hjyamauchi/lldb

[lldb][PECOFF] Exclude alignment padding when reading section data

Author: compnerd

lldb/include/lldb/Symbol/ObjectFile.h

@@ -680,6 +680,12 @@ class ObjectFile : public std::enable_shared_from_this<ObjectFile>,
   const char *GetCStrFromSection(Section *section,
                                  lldb::offset_t section_offset) const;
 
+  // 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; }
 

lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp

@@ -1033,6 +1033,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;

lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h

@@ -261,6 +261,7 @@ class ObjectFilePECOFF : public lldb_private::ObjectFile {
   llvm::StringRef GetSectionName(const section_header_t &sect);
   static lldb::SectionType GetSectionType(llvm::StringRef sect_name,
                                           const section_header_t &sect);
+  size_t GetSectionDataSize(lldb_private::Section *section) override;
 
   llvm::StringRef
   GetReflectionSectionIdentifier(swift::ReflectionSectionKind section) override;

lldb/source/Symbol/ObjectFile.cpp

@@ -552,8 +552,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);
 }
 
 const char *

lldb/unittests/ObjectFile/PECOFF/CMakeLists.txt

@@ -1,5 +1,6 @@
 add_lldb_unittest(ObjectFilePECOFFTests
   TestPECallFrameInfo.cpp
+  TestSectionSize.cpp
 
   LINK_LIBS
     lldbUtilityHelpers

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);
+}
+

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