labath created this revision. labath added reviewers: zturner, amccarth, stella.stamenova, clayborg. Herald added subscribers: abidh, JDevlieghere.
This is coming from the discussion in D55356 <https://reviews.llvm.org/D55356> (the most interesting part happened on the mailing list, so it isn't reflected on the review page). In short the issue is that lldb assumes that all bytes of a module image in memory will be backed by a "section". This isn't the case for PECOFF files because the initial bytes of the module image will contain the file header, which does not correspond to any normal section in the file. In particular, this means it is not possible to implement GetBaseAddress function for PECOFF files, because that's supposed point to the first byte of that header. If my (limited) understanding of how PECOFF files work is correct, then the OS is expecded to load the entire module into one continuous chunk of memory. The address of that chunk (+/- ASLR) is given by the "image base" field in the COFF header, and it's size by "image size". All of the COFF sections are then loaded into this range. If that's true, then we can model this behavior in lldb by creating a "container" section to represent the entire module image, and then place other sections inside that. This would make be consistent with how MachO and ELF files are modelled (except that those can have multiple top-level containers as they can be loaded into multiple discontinuous chunks of memory). This change required a small number of fixups in the PDB plugins, which assumed a certain order of sections within the object file (which obivously changes now). I fix this by changing the lookup code to use section IDs (which are unchanged) instead of indexes. This has the nice benefit of removing spurious -1s in the plugins as the section IDs in the pdbs match the 1-based section IDs in the COFF plugin. Besides making the implementation of GetBaseAddress possible, this also improves the lookup of addresses in the gaps between the object file sections, which will now be correctly resolved as belonging to the object file. Repository: rLLDB LLDB https://reviews.llvm.org/D56537 Files: lit/Modules/PECOFF/export-dllfunc.yaml lit/Modules/PECOFF/subsections.yaml source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp source/Plugins/SymbolFile/NativePDB/DWARFLocationExpression.cpp source/Plugins/SymbolFile/PDB/PDBLocationToDWARFExpression.cpp source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
Index: source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp =================================================================== --- source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp +++ source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp @@ -1365,11 +1365,9 @@ return; while (auto pub_symbol = results->getNext()) { - auto section_idx = pub_symbol->getAddressSection() - 1; - if (section_idx >= section_list->GetSize()) - continue; + auto section_id = pub_symbol->getAddressSection(); - auto section = section_list->GetSectionAtIndex(section_idx); + auto section = section_list->FindSectionByID(section_id); if (!section) continue; Index: source/Plugins/SymbolFile/PDB/PDBLocationToDWARFExpression.cpp =================================================================== --- source/Plugins/SymbolFile/PDB/PDBLocationToDWARFExpression.cpp +++ source/Plugins/SymbolFile/PDB/PDBLocationToDWARFExpression.cpp @@ -514,11 +514,9 @@ if (!section_list) return DWARFExpression(nullptr); - uint32_t section_idx = symbol.getAddressSection() - 1; - if (section_idx >= section_list->GetSize()) - return DWARFExpression(nullptr); + uint32_t section_id = symbol.getAddressSection(); - auto section = section_list->GetSectionAtIndex(section_idx); + auto section = section_list->FindSectionByID(section_id); if (!section) return DWARFExpression(nullptr); Index: source/Plugins/SymbolFile/NativePDB/DWARFLocationExpression.cpp =================================================================== --- source/Plugins/SymbolFile/NativePDB/DWARFLocationExpression.cpp +++ source/Plugins/SymbolFile/NativePDB/DWARFLocationExpression.cpp @@ -622,13 +622,7 @@ SectionList *section_list = module->GetSectionList(); assert(section_list); - // Section indices in PDB are 1-based, but in DWARF they are 0-based, so - // we need to subtract 1. - uint32_t section_idx = section - 1; - if (section_idx >= section_list->GetSize()) - return false; - - auto section_ptr = section_list->GetSectionAtIndex(section_idx); + auto section_ptr = section_list->FindSectionByID(section); if (!section_ptr) return false; Index: source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp =================================================================== --- source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp +++ source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp @@ -707,6 +707,15 @@ ModuleSP module_sp(GetModule()); if (module_sp) { std::lock_guard<std::recursive_mutex> guard(module_sp->GetMutex()); + + SectionSP image_sp = std::make_shared<Section>( + module_sp, this, ~user_id_t(0), ConstString(), eSectionTypeContainer, + m_coff_header_opt.image_base, m_coff_header_opt.image_size, + /*file_offset*/ 0, /*file_size*/ 0, m_coff_header_opt.sect_alignment, + /*flags*/ 0); + m_sections_ap->AddSection(image_sp); + unified_section_list.AddSection(image_sp); + const uint32_t nsects = m_sect_headers.size(); ModuleSP module_sp(GetModule()); for (uint32_t idx = 0; idx < nsects; ++idx) { @@ -809,20 +818,16 @@ section_type = eSectionTypeData; } - // Use a segment ID of the segment index shifted left by 8 so they - // never conflict with any of the sections. SectionSP section_sp(new Section( - module_sp, // Module to which this section belongs - this, // Object file to which this section belongs - idx + 1, // Section ID is the 1 based segment index shifted right by - // 8 bits as not to collide with any of the 256 section IDs - // that are possible + image_sp, // Parent section + module_sp, // Module to which this section belongs + this, // Object file to which this section belongs + idx + 1, // Section ID is the 1 based section index. const_sect_name, // Name of this section - section_type, // This section is a container of other sections. - m_coff_header_opt.image_base + - m_sect_headers[idx].vmaddr, // File VM address == addresses as - // they are found in the object file - m_sect_headers[idx].vmsize, // VM size in bytes of this section + section_type, + m_sect_headers[idx].vmaddr, // File VM address == addresses as + // they are found in the object file + m_sect_headers[idx].vmsize, // VM size in bytes of this section m_sect_headers[idx] .offset, // Offset to the data for this section in the file m_sect_headers[idx] @@ -830,10 +835,7 @@ m_coff_header_opt.sect_alignment, // Section alignment m_sect_headers[idx].flags)); // Flags for this section - // section_sp->SetIsEncrypted (segment_is_encrypted); - - unified_section_list.AddSection(section_sp); - m_sections_ap->AddSection(section_sp); + image_sp->GetChildren().AddSection(std::move(section_sp)); } } } Index: lit/Modules/PECOFF/subsections.yaml =================================================================== --- /dev/null +++ lit/Modules/PECOFF/subsections.yaml @@ -0,0 +1,70 @@ +# RUN: yaml2obj %s > %t +# RUN: lldb-test object-file %t | FileCheck %s + + +# CHECK: Showing 1 sections +# CHECK-NEXT: Index: 0 +# CHECK-NEXT: ID: 0xffffffffffffffff +# CHECK-NEXT: Name: +# CHECK-NEXT: Type: container +# CHECK-NEXT: Permissions: --- +# CHECK-NEXT: Thread specific: no +# CHECK-NEXT: VM address: 0x40000000 +# CHECK-NEXT: VM size: 12288 +# CHECK-NEXT: File size: 0 +# CHECK-NEXT: Showing 2 subsections +# CHECK-NEXT: Index: 0 +# CHECK-NEXT: ID: 0x1 +# CHECK-NEXT: Name: .text +# CHECK-NEXT: Type: code +# CHECK-NEXT: Permissions: --- +# CHECK-NEXT: Thread specific: no +# CHECK-NEXT: VM address: 0x40001000 +# CHECK-NEXT: VM size: 64 +# CHECK-NEXT: File size: 512 +# CHECK-EMPTY: +# CHECK-NEXT: Index: 1 +# CHECK-NEXT: ID: 0x2 +# CHECK-NEXT: Name: .data +# CHECK-NEXT: Type: data +# CHECK-NEXT: Permissions: --- +# CHECK-NEXT: Thread specific: no +# CHECK-NEXT: VM address: 0x40002000 +# CHECK-NEXT: VM size: 64 +# CHECK-NEXT: File size: 512 + + +--- !COFF +OptionalHeader: + AddressOfEntryPoint: 4616 + ImageBase: 1073741824 + SectionAlignment: 4096 + FileAlignment: 512 + MajorOperatingSystemVersion: 6 + MinorOperatingSystemVersion: 0 + MajorImageVersion: 0 + MinorImageVersion: 0 + MajorSubsystemVersion: 6 + MinorSubsystemVersion: 0 + Subsystem: IMAGE_SUBSYSTEM_WINDOWS_CUI + DLLCharacteristics: [ IMAGE_DLL_CHARACTERISTICS_HIGH_ENTROPY_VA, IMAGE_DLL_CHARACTERISTICS_DYNAMIC_BASE, IMAGE_DLL_CHARACTERISTICS_NX_COMPAT, IMAGE_DLL_CHARACTERISTICS_TERMINAL_SERVER_AWARE ] + SizeOfStackReserve: 1048576 + SizeOfStackCommit: 4096 + SizeOfHeapReserve: 1048576 + SizeOfHeapCommit: 4096 +header: + Machine: IMAGE_FILE_MACHINE_AMD64 + Characteristics: [ IMAGE_FILE_EXECUTABLE_IMAGE, IMAGE_FILE_LARGE_ADDRESS_AWARE ] +sections: + - Name: .text + Characteristics: [ IMAGE_SCN_CNT_CODE, IMAGE_SCN_MEM_EXECUTE, IMAGE_SCN_MEM_READ ] + VirtualAddress: 4096 + VirtualSize: 64 + SectionData: DEADBEEFBAADF00D + - Name: .data + Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_READ ] + VirtualAddress: 8192 + VirtualSize: 64 + SectionData: DEADBEEFBAADF00D +symbols: [] +... Index: lit/Modules/PECOFF/export-dllfunc.yaml =================================================================== --- lit/Modules/PECOFF/export-dllfunc.yaml +++ lit/Modules/PECOFF/export-dllfunc.yaml @@ -7,7 +7,7 @@ # RUN: lldb-test object-file -dep-modules %t.dll | FileCheck -check-prefix=DEPS %s -# BASIC-CHECK: Showing 3 sections +# BASIC-CHECK: Showing 3 subsections # BASIC-CHECK: Index: 0 # BASIC-CHECK: Name: .text # BASIC-CHECK: Type: code
_______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits