[LLDB] [PECOFF] Use FindSectionByID to associate symbols to sections

[mstorsjo]

Summary

The virtual container/header section caused the section list to be offset by one, but by using FindSectionByID, the layout of the section list shouldn't matter.

[mstorsjo]

Updated the testcase to check for symbols in more than one section.

[labath]

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

691

You should use FindSectionByID here. That's the API used by both ELF and MachO object files and SymbolFilePDB classes for this sort of thing. This would obviate the need for the long comment and make this patch independent of the header stuff (which I'm going to commit as soon as I figure out how that's done).

lldb/test/Shell/ObjectFile/PECOFF/symbol.yaml

3

At some point we'll probably want to add some kind of a symtab dumping mechanism to the object-file subcommand, but this is fine until then.

5

Could you also add a CHECK line with the table header, just so one gets the idea of what is the meaning of individual fields..

[amccarth]

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

691

+1 to labath's suggestion. It seems unfortunate that this already had a coupling to implementation details of the section list.

[mstorsjo]

Using FindSectionByID, added a table header for readability to the test.

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

if (m_symtab_up == nullptr) {
             }
             symbol.value = symtab_data.GetU32(&offset);
             symbol.sect = symtab_data.GetU16(&offset);
             symbol.type = symtab_data.GetU16(&offset);
             symbol.storage = symtab_data.GetU8(&offset);
             symbol.naux = symtab_data.GetU8(&offset);
             symbols[i].GetMangled().SetValue(ConstString(symbol_name.c_str()));
             if ((int16_t)symbol.sect >= 1) {
-              Address symbol_addr(sect_list->GetSectionAtIndex(symbol.sect - 1),
+              Address symbol_addr(sect_list->FindSectionByID(symbol.sect),
                                   symbol.value);
               symbols[i].GetAddressRef() = symbol_addr;
               symbols[i].SetType(MapSymbolType(symbol.type));
             }
 
             if (symbol.naux > 0) {
               i += symbol.naux;
               offset += symbol_size;
             }
           }
         }

lldb/test/Shell/ObjectFile/PECOFF/symbol.yaml

+# RUN: yaml2obj %s > %t
+# RUN: lldb-test symbols %t | FileCheck %s
+
+# CHECK: Type File Address/Value {{.*}} Size            Flags           Name
+# CHECK: Code 0x0000000040001000        0x{{[0-9a-f]+}} 0x{{[0-9a-f]+}} entry
+# CHECK:      0x0000000040002000        0x{{[0-9a-f]+}} 0x{{[0-9a-f]+}} variable
+
+--- !COFF
+OptionalHeader:
+  AddressOfEntryPoint: 4096
+  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
+  ExportTable:
+    RelativeVirtualAddress: 0
+    Size:            0
+  ImportTable:
+    RelativeVirtualAddress: 0
+    Size:            0
+  ResourceTable:
+    RelativeVirtualAddress: 0
+    Size:            0
+  ExceptionTable:
+    RelativeVirtualAddress: 0
+    Size:            0
+  CertificateTable:
+    RelativeVirtualAddress: 0
+    Size:            0
+  BaseRelocationTable:
+    RelativeVirtualAddress: 0
+    Size:            0
+  Debug:
+    RelativeVirtualAddress: 0
+    Size:            0
+  Architecture:
+    RelativeVirtualAddress: 0
+    Size:            0
+  GlobalPtr:
+    RelativeVirtualAddress: 0
+    Size:            0
+  TlsTable:
+    RelativeVirtualAddress: 0
+    Size:            0
+  LoadConfigTable:
+    RelativeVirtualAddress: 0
+    Size:            0
+  BoundImport:
+    RelativeVirtualAddress: 0
+    Size:            0
+  IAT:
+    RelativeVirtualAddress: 0
+    Size:            0
+  DelayImportDescriptor:
+    RelativeVirtualAddress: 0
+    Size:            0
+  ClrRuntimeHeader:
+    RelativeVirtualAddress: 0
+    Size:            0
+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:     1
+    SectionData:     C3
+  - Name:            .data
+    Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_READ, IMAGE_SCN_MEM_WRITE ]
+    VirtualAddress:  8192
+    VirtualSize:     4
+    SectionData:     ''
+symbols:
+  - Name:            .text
+    Value:           0
+    SectionNumber:   1
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_NULL
+    StorageClass:    IMAGE_SYM_CLASS_STATIC
+  - Name:            .data
+    Value:           0
+    SectionNumber:   2
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_NULL
+    StorageClass:    IMAGE_SYM_CLASS_STATIC
+  - Name:            .bss
+    Value:           0
+    SectionNumber:   2
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_NULL
+    StorageClass:    IMAGE_SYM_CLASS_STATIC
+  - Name:            entry
+    Value:           0
+    SectionNumber:   1
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_FUNCTION
+    StorageClass:    IMAGE_SYM_CLASS_EXTERNAL
+  - Name:            variable
+    Value:           0
+    SectionNumber:   2
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_NULL
+    StorageClass:    IMAGE_SYM_CLASS_EXTERNAL
+...