Add a relocation to ObjectFileELF::ApplyRelocations and a test

[lanza]

Summary

pcm files can end up being processed by lldb with relocations to be
made for the .debug_info section. When a R_AARCH64_ABS64 relocation
was required lldb would hit an assert(false) and die.

Add R_AARCH64_ABS64 relocations to the S+A 64 bit width code path. Add
a test for R_AARCH64_ABS64 and R_AARCH64_ABS32 .rela.debug_info
relocations in a pcm file.

[lanza]

move macros

[lanza]

@davide Hey Davide, could you take a look at this?

[davide]

LGTM. @zturner / @labath might have opinions.

[davide]

Also, thanks for taking the time to do this right :)

[labath]

I am not sure which thread it was, but I think I mentioned somewhere that lldb-test would be better suited to write this kind of a test. bin/lldb-test object-file -contents should print the contents of the sections, which should reflect the relocations that have been applied to them. Then you could write the test as:
yaml2obj | lldb-test object-file -contents | FileCheck

Have you tried this approach?

[lanza]

Also, thanks for taking the time to do this right :)

No problem, it was a great learning experience, too.

I am not sure which thread it was, but I think I mentioned somewhere that lldb-test would be better suited to write this kind of a test. bin/lldb-test object-file -contents should print the contents of the sections, which should reflect the relocations that have been applied to them. Then you could write the test as:
yaml2obj | lldb-test object-file -contents | FileCheck

Have you tried this approach?

Just from a quick scan that seems like it would work. I'll make sure look there first from now on.

[davide]

@lanza Hey Nathan, it looks like your test exposed an UB in ELFObjectFileReader. May I ask you to take a look?

******************** TEST 'lldb-Unit :: ObjectFile/ELF/./ObjectFileELFTests/ObjectFileELFTest.TestAARCH64Relocations' FAILED ********************
Note: Google Test filter = ObjectFileELFTest.TestAARCH64Relocations
[==========] Running 1 test from 1 test case.
[----------] Global test environment set-up.
[----------] 1 test from ObjectFileELFTest
[ RUN      ] ObjectFileELFTest.TestAARCH64Relocations
/Users/buildslave/jenkins/workspace/lldb-sanitized/llvm/tools/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp:2741:11: runtime error: store to misaligned address 0x61f000001526 for type 'uint32_t' (aka 'unsigned int'), which requires 4 byte alignment
0x61f000001526: note: pointer points here
 00 00 04 00 00 00  00 00 08 01 00 00 00 00  0c 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  00 00
             ^ 
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /Users/buildslave/jenkins/workspace/lldb-sanitized/llvm/tools/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp:2741:11 in

[davide]

In D51566#1288792, @davide wrote:

@lanza Hey Nathan, it looks like your test exposed an UB in ELFObjectFileReader. May I ask you to take a look?

• TEST 'lldb-Unit :: ObjectFile/ELF/./ObjectFileELFTests/ObjectFileELFTest.TestAARCH64Relocations' FAILED ****

Note: Google Test filter = ObjectFileELFTest.TestAARCH64Relocations
[==========] Running 1 test from 1 test case.
[----------] Global test environment set-up.
[----------] 1 test from ObjectFileELFTest
[ RUN ] ObjectFileELFTest.TestAARCH64Relocations
/Users/buildslave/jenkins/workspace/lldb-sanitized/llvm/tools/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp:2741:11: runtime error: store to misaligned address 0x61f000001526 for type 'uint32_t' (aka 'unsigned int'), which requires 4 byte alignment
0x61f000001526: note: pointer points here
00 00 04 00 00 00 00 00 08 01 00 00 00 00 0c 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00

^

SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /Users/buildslave/jenkins/workspace/lldb-sanitized/llvm/tools/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp:2741:11 in

In D51566#1288792, @davide wrote:

@lanza Hey Nathan, it looks like your test exposed an UB in ELFObjectFileReader. May I ask you to take a look?

******************** TEST 'lldb-Unit :: ObjectFile/ELF/./ObjectFileELFTests/ObjectFileELFTest.TestAARCH64Relocations' FAILED ********************
Note: Google Test filter = ObjectFileELFTest.TestAARCH64Relocations
[==========] Running 1 test from 1 test case.
[----------] Global test environment set-up.
[----------] 1 test from ObjectFileELFTest
[ RUN      ] ObjectFileELFTest.TestAARCH64Relocations
/Users/buildslave/jenkins/workspace/lldb-sanitized/llvm/tools/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp:2741:11: runtime error: store to misaligned address 0x61f000001526 for type 'uint32_t' (aka 'unsigned int'), which requires 4 byte alignment
0x61f000001526: note: pointer points here
 00 00 04 00 00 00  00 00 08 01 00 00 00 00  0c 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  00 00
             ^ 
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /Users/buildslave/jenkins/workspace/lldb-sanitized/llvm/tools/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp:2741:11 in

Went ahead and fixed this in r346244. A post commit review would be appreciated.

Davide

[lanza]

LGTM and the test case passes. Thanks a lot, Davide!

source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp

if (hdr->Is32Bit()) {
         //
         // clang++.exe --target=i686-unknown-linux-gnu -g -c foo.cpp -o foo.o
         //
         // and running this on the foo.o module.
         assert(false && "unexpected relocation type");
       }
     } else {
       switch (reloc_type(rel)) {
+      case R_AARCH64_ABS64:
       case R_X86_64_64: {
         symbol = symtab->FindSymbolByID(reloc_symbol(rel));
         if (symbol) {
           addr_t value = symbol->GetAddressRef().GetFileAddress();
           DataBufferSP &data_buffer_sp = debug_data.GetSharedDataBuffer();
           uint64_t *dst = reinterpret_cast<uint64_t *>(
               data_buffer_sp->GetBytes() + rel_section->GetFileOffset() +
               ELFRelocation::RelocOffset64(rel));
           *dst = value + ELFRelocation::RelocAddend64(rel);
         }
         break;
       }
       case R_X86_64_32:
       case R_X86_64_32S:
       case R_AARCH64_ABS32: {
         symbol = symtab->FindSymbolByID(reloc_symbol(rel));
         if (symbol) {
           addr_t value = symbol->GetAddressRef().GetFileAddress();
           value += ELFRelocation::RelocAddend32(rel);
-          if ((reloc_type(rel) == R_X86_64_32 && (value <= UINT32_MAX)) ||
+          if ((reloc_type(rel) == R_X86_64_32 && (value > UINT32_MAX)) ||
               (reloc_type(rel) == R_X86_64_32S &&
-               ((int64_t)value <= INT32_MAX && (int64_t)value >= INT32_MIN)) ||
-              (reloc_type(rel) == R_AARCH64_ABS32 && (value <= UINT32_MAX))) {
+               ((int64_t)value > INT32_MAX && (int64_t)value < INT32_MIN)) ||
+              (reloc_type(rel) == R_AARCH64_ABS32 &&
+               ((int64_t)value > INT32_MAX && (int64_t)value < INT32_MIN))) {
             Log *log =
                 lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_MODULES);
             log->Printf("Failed to apply debug info relocations");
+            break;
           }
           uint32_t truncated_addr = (value & 0xFFFFFFFF);
           DataBufferSP &data_buffer_sp = debug_data.GetSharedDataBuffer();
           uint32_t *dst = reinterpret_cast<uint32_t *>(
               data_buffer_sp->GetBytes() + rel_section->GetFileOffset() +
               ELFRelocation::RelocOffset32(rel));
           *dst = truncated_addr;
         }

unittests/ObjectFile/ELF/CMakeLists.txt

 add_lldb_unittest(ObjectFileELFTests
   TestObjectFileELF.cpp
 
   LINK_LIBS
     lldbPluginObjectFileELF
     lldbPluginSymbolVendorELF
     lldbCore
     lldbUtilityHelpers
   )
 
 add_dependencies(ObjectFileELFTests yaml2obj)
 add_definitions(-DYAML2OBJ="$<TARGET_FILE:yaml2obj>")
 
 set(test_inputs
   early-section-headers.so
+  debug-info-relocations.pcm.yaml
   sections-resolve-consistently.yaml
   )
 add_unittest_inputs(ObjectFileELFTests "${test_inputs}")

unittests/ObjectFile/ELF/Inputs/debug-info-relocations.pcm.yaml

+--- !ELF
+FileHeader:      
+  Class:           ELFCLASS64
+  Data:            ELFDATA2LSB
+  Type:            ET_REL
+  Machine:         EM_AARCH64
+Sections:        
+  - Name:            .text
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_ALLOC, SHF_EXECINSTR ]
+    AddressAlign:    0x0000000000000004
+    Content:         E0031B32C0035FD6
+  - Name:            .debug_str
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_MERGE, SHF_STRINGS ]
+    AddressAlign:    0x0000000000000001
+    Content:         636C616E672076657273696F6E20362E302E312028746167732F52454C454153455F3630312F66696E616C2900726564756365642E68002F746D702F617364666173646673646661006600696E7400
+  - Name:            .debug_abbrev
+    Type:            SHT_PROGBITS
+    AddressAlign:    0x0000000000000001
+    Content:         011101250E1305030E10171B0EB44219110112060000022E00110112064018030E3A0B3B0B271949133F190000032400030E3E0B0B0B000000
+  - Name:            .debug_info
+    Type:            SHT_PROGBITS
+    AddressAlign:    0x0000000000000001
+    Content:         470000000400000000000801000000000C0000000000000000000000000000000000000000000800000002000000000000000008000000016F000000000102430000000300000000050400
+  - Name:            .rela.debug_info
+    Type:            SHT_RELA
+    Link:            .symtab
+    AddressAlign:    0x0000000000000008
+    Info:            .debug_info
+    Relocations:     
+      - Offset:          0x0000000000000006
+        Symbol:          .debug_abbrev
+        Type:            R_AARCH64_ABS32
+      - Offset:          0x000000000000000C
+        Symbol:          .debug_str
+        Type:            R_AARCH64_ABS32
+      - Offset:          0x0000000000000012
+        Symbol:          .debug_str
+        Type:            R_AARCH64_ABS32
+        Addend:          45
+      - Offset:          0x0000000000000016
+        Symbol:          .debug_line
+        Type:            R_AARCH64_ABS32
+      - Offset:          0x000000000000001A
+        Symbol:          .debug_str
+        Type:            R_AARCH64_ABS32
+        Addend:          55
+      - Offset:          0x000000000000001E
+        Symbol:          .text
+        Type:            R_AARCH64_ABS64
+      - Offset:          0x000000000000002B
+        Symbol:          .text
+        Type:            R_AARCH64_ABS64
+      - Offset:          0x0000000000000039
+        Symbol:          .debug_str
+        Type:            R_AARCH64_ABS32
+        Addend:          73
+      - Offset:          0x0000000000000044
+        Symbol:          .debug_str
+        Type:            R_AARCH64_ABS32
+        Addend:          75
+  - Name:            .debug_ranges
+    Type:            SHT_PROGBITS
+    AddressAlign:    0x0000000000000001
+    Content:         ''
+  - Name:            .debug_macinfo
+    Type:            SHT_PROGBITS
+    AddressAlign:    0x0000000000000001
+    Content:         '00'
+  - Name:            .debug_pubnames
+    Type:            SHT_PROGBITS
+    AddressAlign:    0x0000000000000001
+    Content:         140000000200000000004B0000002A000000660000000000
+  - Name:            .rela.debug_pubnames
+    Type:            SHT_RELA
+    Link:            .symtab
+    AddressAlign:    0x0000000000000008
+    Info:            .debug_pubnames
+    Relocations:     
+      - Offset:          0x0000000000000006
+        Symbol:          .debug_info
+        Type:            R_AARCH64_ABS32
+  - Name:            .debug_pubtypes
+    Type:            SHT_PROGBITS
+    AddressAlign:    0x0000000000000001
+    Content:         160000000200000000004B00000043000000696E740000000000
+  - Name:            .rela.debug_pubtypes
+    Type:            SHT_RELA
+    Link:            .symtab
+    AddressAlign:    0x0000000000000008
+    Info:            .debug_pubtypes
+    Relocations:     
+      - Offset:          0x0000000000000006
+        Symbol:          .debug_info
+        Type:            R_AARCH64_ABS32
+  - Name:            .comment
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_MERGE, SHF_STRINGS ]
+    AddressAlign:    0x0000000000000001
+    Content:         00636C616E672076657273696F6E20362E302E312028746167732F52454C454153455F3630312F66696E616C2900
+  - Name:            .note.GNU-stack
+    Type:            SHT_PROGBITS
+    AddressAlign:    0x0000000000000001
+    Content:         ''
+  - Name:            .debug_frame
+    Type:            SHT_PROGBITS
+    AddressAlign:    0x0000000000000008
+    Content:         14000000FFFFFFFF04000800017C1E0C1F00000000000000140000000000000000000000000000000800000000000000
+  - Name:            .rela.debug_frame
+    Type:            SHT_RELA
+    Link:            .symtab
+    AddressAlign:    0x0000000000000008
+    Info:            .debug_frame
+    Relocations:     
+      - Offset:          0x000000000000001C
+        Symbol:          .debug_frame
+        Type:            R_AARCH64_ABS32
+      - Offset:          0x0000000000000020
+        Symbol:          .text
+        Type:            R_AARCH64_ABS64
+  - Name:            .debug_line
+    Type:            SHT_PROGBITS
+    AddressAlign:    0x0000000000000001
+    Content:         3C000000040021000000010101FB0E0D00010101010000000100000100726564756365642E68000000000000090200000000000000001305030A4B0204000101
+  - Name:            .rela.debug_line
+    Type:            SHT_RELA
+    Link:            .symtab
+    AddressAlign:    0x0000000000000008
+    Info:            .debug_line
+    Relocations:     
+      - Offset:          0x000000000000002E
+        Symbol:          .text
+        Type:            R_AARCH64_ABS64
+Symbols:         
+  Local:           
+    - Name:            reduced.h
+      Type:            STT_FILE
+    - Name:            '$d.1'
+      Section:         .debug_str
+    - Name:            '$d.2'
+      Section:         .debug_abbrev
+    - Name:            '$d.3'
+      Section:         .debug_info
+    - Name:            '$d.4'
+      Section:         .debug_macinfo
+    - Name:            '$d.5'
+      Section:         .debug_pubnames
+    - Name:            '$d.6'
+      Section:         .debug_pubtypes
+    - Name:            '$d.7'
+      Section:         .comment
+    - Name:            '$d.8'
+      Section:         .debug_frame
+    - Name:            '$d.9'
+      Section:         .debug_line
+    - Name:            '$x.0'
+      Section:         .text
+    - Name:            .text
+      Type:            STT_SECTION
+      Section:         .text
+    - Name:            .debug_str
+      Type:            STT_SECTION
+      Section:         .debug_str
+    - Name:            .debug_abbrev
+      Type:            STT_SECTION
+      Section:         .debug_abbrev
+    - Name:            .debug_info
+      Type:            STT_SECTION
+      Section:         .debug_info
+    - Name:            .debug_frame
+      Type:            STT_SECTION
+      Section:         .debug_frame
+    - Name:            .debug_line
+      Type:            STT_SECTION
+      Section:         .debug_line
+  Global:          
+    - Name:            f
+      Type:            STT_FUNC
+      Section:         .text
+      Size:            0x0000000000000008
+DynamicSymbols:  
+...

unittests/ObjectFile/ELF/TestObjectFileELF.cpp

 #include "Plugins/ObjectFile/ELF/ObjectFileELF.h"
 #include "Plugins/SymbolVendor/ELF/SymbolVendorELF.h"
 #include "TestingSupport/TestUtilities.h"
 #include "lldb/Core/Module.h"
 #include "lldb/Core/ModuleSpec.h"
 #include "lldb/Core/Section.h"
 #include "lldb/Host/FileSystem.h"
 #include "lldb/Host/HostInfo.h"
+#include "lldb/Utility/DataBufferHeap.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/Support/Compression.h"
 #include "llvm/Support/FileUtilities.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/Program.h"
 #include "llvm/Support/raw_ostream.h"
 #include "gtest/gtest.h"
 
TEST_F(ObjectFileELFTest, GetModuleSpecifications_EarlySectionHeaders) {
   ModuleSpecList Specs;
   ASSERT_EQ(1u, ObjectFile::GetModuleSpecifications(FileSpec(SO), 0, 0, Specs));
   ModuleSpec Spec;
   ASSERT_TRUE(Specs.GetModuleSpecAtIndex(0, Spec)) ;
   UUID Uuid;
   Uuid.SetFromStringRef("1b8a73ac238390e32a7ff4ac8ebe4d6a41ecf5c9", 20);
   EXPECT_EQ(Spec.GetUUID(), Uuid);
 }
+
+#define CHECK_ABS32(offset, addend)                                            \
+  ASSERT_EQ((uint32_t)addend, *(uint32_t *)(bytes + offset))
+#define CHECK_ABS64(offset, addend)                                            \
+  ASSERT_EQ((uint64_t)addend, *(uint64_t *)(bytes + offset))
+
+TEST_F(ObjectFileELFTest, TestAARCH64Relocations) {
+  std::string yaml = GetInputFilePath("debug-info-relocations.pcm.yaml");
+  llvm::SmallString<128> obj;
+  ASSERT_NO_ERROR(llvm::sys::fs::createTemporaryFile(
+      "debug-info-relocations-%%%%%%", "obj", obj));
+
+  llvm::FileRemover remover(obj);
+  llvm::StringRef args[] = {YAML2OBJ, yaml};
+  llvm::StringRef obj_ref = obj;
+  const llvm::Optional<llvm::StringRef> redirects[] = {llvm::None, obj_ref,
+                                                       llvm::None};
+  ASSERT_EQ(0,
+            llvm::sys::ExecuteAndWait(YAML2OBJ, args, llvm::None, redirects));
+  uint64_t size;
+  ASSERT_NO_ERROR(llvm::sys::fs::file_size(obj, size));
+  ASSERT_GT(size, 0u);
+
+  ModuleSpec spec{FileSpec(obj)};
+  spec.GetSymbolFileSpec().SetFile(obj, FileSpec::Style::native);
+  auto module_sp = std::make_shared<Module>(spec);
+
+  auto objfile = static_cast<ObjectFileELF *>(module_sp->GetObjectFile());
+  SectionList *section_list = objfile->GetSectionList();
+  ASSERT_NE(nullptr, section_list);
+
+  auto debug_info_sp =
+      section_list->FindSectionByName(ConstString(".debug_info"));
+  ASSERT_NE(nullptr, debug_info_sp);
+  objfile->RelocateSection(debug_info_sp.get());
+
+  DataExtractor data;
+  // length of 0x10 is not needed but length 0x0 crashes
+  objfile->GetData(0x00, 0x10, data);
+  DataBufferSP &data_buffer_sp = data.GetSharedDataBuffer();
+  uint8_t *bytes = data_buffer_sp->GetBytes();
+
+  addr_t debug_info_offset = debug_info_sp->GetFileOffset();
+  bytes += debug_info_offset;
+
+  // Sanity check - The first byte from the yaml file is 0x47
+  ASSERT_EQ(0x47, *bytes);
+
+  // .rela.debug_info contains 9 relocations:
+  // 7 R_AARCH64_ABS32 - 2 R_AARCH64_ABS64
+  // None have a value. Four have addends.
+  CHECK_ABS32(0x6, 0);
+  CHECK_ABS32(0xC, 0);
+  CHECK_ABS32(0x12, 45);
+  CHECK_ABS32(0x16, 0);
+  CHECK_ABS32(0x1A, 55);
+  CHECK_ABS64(0x1E, 0);
+  CHECK_ABS64(0x2B, 0);
+  CHECK_ABS32(0x39, 73);
+  CHECK_ABS32(0x44, 75);
+}