lldb: Add support for R_386_32 relocations to ObjectFileELF

[dmlary]

Summary

I encountered an issue where p &variable was finding an incorrect address for
32-bit PIC ELF files loaded into a running process. The problem was that the
R_386_32 ELF relocations were not being applied to the DWARF section, so all
variables in that file were reporting as being at the start of their respective
section. There is an assert that catches this on debug builds, but silently
ignores the issue on non-debug builds.

In this changeset, I added handling for the R_386_32 relocation type to
ObjectFileELF, and a supporting function to ELFRelocation to differentiate
between DT_REL & DT_RELA in ObjectFileELF::ApplyRelocations().

Demonstration of issue:

# build a relocatable object file;
[dmlary@host work]$ cat rel.c
volatile char padding[32] = "make sure var isnt at .data+0";
volatile char var[] = "test";
[dmlary@host work]$ gcc -c rel.c -FPIC -fpic -g -m32

# start lldb debugging any binary;
[dmlary@host work]$ lldb ./exec
(lldb) target create "./exec"
Current executable set to '/home/dmlary/src/work/exec' (i386).
(lldb) process launch --stop-at-entry
Process 21278 stopped
* thread #1, name = 'exec', stop reason = signal SIGSTOP
    frame #0: 0xf7fdb150 ld-2.17.so`_start
ld-2.17.so`_start:
->  0xf7fdb150 <+0>: movl   %esp, %eax
    0xf7fdb152 <+2>: calll  0xf7fdb990                ; _dl_start

ld-2.17.so`_dl_start_user:
    0xf7fdb157 <+0>: movl   %eax, %edi
    0xf7fdb159 <+2>: calll  0xf7fdb140
Process 21278 launched: '/home/dmlary/src/work/exec' (i386)

# add the .o file, and load it at an arbitrary address
(lldb) image add ./rel.o
(lldb) image load --file rel.o .text 0x40000000 .data 0x50000000
section '.text' loaded at 0x40000000
section '.data' loaded at 0x50000000

# look where the `var` symbol should be loaded (0x50000020)
(lldb) image dump symtab rel.o
Symtab, file = rel.o, num_symbols = 13:
               Debug symbol
               |Synthetic symbol
               ||Externally Visible
               |||
Index   UserID DSX Type            File Address/Value Load Address       Size               Flags      Name
------- ------ --- --------------- ------------------ ------------------ ------------------ ---------- ----------------------------------
[    0]      1     SourceFile      0x0000000000000000                    0x0000000000000000 0x00000004 rel.c
[    1]      2     Invalid         0x0000000000000000                    0x0000000000000020 0x00000003
[    2]      3     Invalid         0x0000000000000000 0x50000000 0x0000000000000020 0x00000003
[    3]      4     Invalid         0x0000000000000025                    0x0000000000000000 0x00000003
[    4]      5     Invalid         0x0000000000000000                    0x0000000000000020 0x00000003
[    5]      6     Invalid         0x0000000000000000                    0x0000000000000020 0x00000003
[    6]      7     Invalid         0x0000000000000000                    0x0000000000000020 0x00000003
[    7]      8     Invalid         0x0000000000000000                    0x0000000000000020 0x00000003
[    8]      9     Invalid         0x0000000000000000                    0x0000000000000020 0x00000003
[    9]     10     Invalid         0x0000000000000000                    0x0000000000000020 0x00000003
[   10]     11     Invalid         0x0000000000000000                    0x0000000000000020 0x00000003
[   11]     12   X Data            0x0000000000000000 0x50000000 0x0000000000000020 0x00000011 padding
[   12]     13   X Data            0x0000000000000020 0x50000020 0x0000000000000005 0x00000011 var

# eval reports it at the start of the section instead
(lldb) p &var
(volatile char (*)[5]) $1 = 0x50000000

[dmlary]

I'm looking for any suggestions of how to test this. I can create a simple object file with the needed relocation types with yaml2obj, but I don't see an easy way to get lldb to apply those relocations (in ..text for example).

I wanted to see if that was possible before making a far more complex test with both a generic return binary, and a second object file to be loaded. If that's the direction I need to go in, any pointer to an existing test that compiles a relocatable object file (not executable) would be appreciated.

[labath]

In D132954#3758856, @dmlary wrote:

I'm looking for any suggestions of how to test this. I can create a simple object file with the needed relocation types with yaml2obj, but I don't see an easy way to get lldb to apply those relocations (in ..text for example).

IIRC, lldb explicitly skips any relocations on non-debug sections. Could you check if the relocations are being reflected in the output of lldb-test object-file --contents %t? If so, then you could use that command to print the contents of a dummy debug info section (it doesn't even have to be valid DWARF).

lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp

2650

Maybe just remove this assertion now.

[dmlary]

@labath it doesn't look like the relocations are applied during lldb-test object-file --contents:

from obj2yaml for rel.c described in summary (with added spacing for readabilty):

[...]
  - Name:            .debug_info
    Type:            SHT_PROGBITS
    AddressAlign:    0x1
    Content:
      74000000 04000000 00000401 09000000 01890000 00620000 00000000 00023400
      00002D00 0000032D 0000001F 00040407 00000000 04010684 00000005 7C000000
      01024C00 00000503 00000000 061D0000 00023400 00006100 0000032D 00000004
      00077661 72000103 72000000 05030000 00000651 00000000
[...]
  - Name:            .rel.debug_info
    Type:            SHT_REL
    Flags:           [ SHF_INFO_LINK ]
    Link:            .symtab
    AddressAlign:    0x4
    Info:            .debug_info
    Relocations:
      - Offset:          0x6
        Symbol:          .debug_abbrev
        Type:            R_386_32
      - Offset:          0xC
        Symbol:          .debug_str
        Type:            R_386_32
      - Offset:          0x11
[...]

output for .debug_info from lldb-test object-file --contents rel.o:

txt
  Index: 3
  ID: 0x4
  Name: .debug_info
  Type: dwarf-info
  Permissions: ---
  Thread specific: no
  VM address: 0x0
  VM size: 0
  File size: 120
  Data:  (
      0000: 74000000 04000000 00000401 09000000 01890000 00620000 00000000 00023400  |t....................b........4.|
      0020: 00002D00 0000032D 0000001F 00040407 00000000 04010684 00000005 7C000000  |..-....-....................|...|
      0040: 01024C00 00000503 00000000 061D0000 00023400 00006100 0000032D 00000004  |..L...............4...a....-....|
      0060: 00077661 72000103 72000000 05030000 00000651 00000000                    |..var...r..........Q....|
  )

I would expect at least one byte in the output to be changed if relocations were being applied to .debug_info. To ensure it's not just relocating using a base address of 0, I modified one relocation to use var as the symbol for relocation; it has a non-zero offset within its section. Even in that case there were no modifications made to the data in .debug_info.

It looks like I'll need to build a binary and a separate object file in the test case. Any chance you know of a test off-hand building multiple things, with specific flags?

[labath]

In D132954#3767083, @dmlary wrote:

@labath it doesn't look like the relocations are applied during lldb-test object-file --contents:

I'm definitely seeing ObjectFileELF::RelocateSection be invoked with this command (I'm using it on a random .o file from my lldb build). Could you step through the function to see why that does not happen for you?

* frame #0: 0x0000555556916576 lldb-test`ObjectFileELF::RelocateSection(this=0x0000555564473a60, section=0x00005555645310e0) at ObjectFileELF.cpp:2857:26
  frame #1: 0x00005555564b50de lldb-test`lldb_private::ObjectFile::ReadSectionData(this=0x0000555564473a60, section=0x00005555645310e0, section_data=0x00007fffffffce80) at ObjectFile.cpp:530:20
  frame #2: 0x00005555569187e1 lldb-test`ObjectFileELF::ReadSectionData(this=0x0000555564473a60, section=0x00005555645310e0, section_data=0x00007fffffffce80) at ObjectFileELF.cpp:3350:46
  frame #3: 0x00005555562f0c7c lldb-test`lldb_private::Section::GetSectionData(this=0x00005555645310e0, section_data=0x00007fffffffce80) at Section.cpp:381:39

from obj2yaml for rel.c described in summary (with added spacing for readabilty):

[...]
  - Name:            .debug_info
    Type:            SHT_PROGBITS
    AddressAlign:    0x1
    Content:
      74000000 04000000 00000401 09000000 01890000 00620000 00000000 00023400
      00002D00 0000032D 0000001F 00040407 00000000 04010684 00000005 7C000000
      01024C00 00000503 00000000 061D0000 00023400 00006100 0000032D 00000004
      00077661 72000103 72000000 05030000 00000651 00000000
[...]
  - Name:            .rel.debug_info
    Type:            SHT_REL
    Flags:           [ SHF_INFO_LINK ]
    Link:            .symtab
    AddressAlign:    0x4
    Info:            .debug_info
    Relocations:
      - Offset:          0x6
        Symbol:          .debug_abbrev
        Type:            R_386_32
      - Offset:          0xC
        Symbol:          .debug_str
        Type:            R_386_32
      - Offset:          0x11
[...]

output for .debug_info from lldb-test object-file --contents rel.o:

txt
  Index: 3
  ID: 0x4
  Name: .debug_info
  Type: dwarf-info
  Permissions: ---
  Thread specific: no
  VM address: 0x0
  VM size: 0
  File size: 120
  Data:  (
      0000: 74000000 04000000 00000401 09000000 01890000 00620000 00000000 00023400  |t....................b........4.|
      0020: 00002D00 0000032D 0000001F 00040407 00000000 04010684 00000005 7C000000  |..-....-....................|...|
      0040: 01024C00 00000503 00000000 061D0000 00023400 00006100 0000032D 00000004  |..L...............4...a....-....|
      0060: 00077661 72000103 72000000 05030000 00000651 00000000                    |..var...r..........Q....|
  )

I would expect at least one byte in the output to be changed if relocations were being applied to .debug_info. To ensure it's not just relocating using a base address of 0, I modified one relocation to use var as the symbol for relocation; it has a non-zero offset within its section. Even in that case there were no modifications made to the data in .debug_info.

It looks like I'll need to build a binary and a separate object file in the test case. Any chance you know of a test off-hand building multiple things, with specific flags?

That will be a bit messy. I'd rather figure out what's going on with the lldb-test command..

[dmlary]

@labath you are correct, lldb-test is performing the relocations. I'm digging into why I'm not seeing the changes I would expect in the section. I'll update the diff with tests when I get that resolved. Thank you.

[dmlary]

• Added test cases for REL and RELA cases
• Removed assert & fixme comment

[labath]

Cool. Thanks for figuring this out.

[dmlary]

Could someone please merge this? I do not have commit access.

[dmlary]

I saw the build bot failures; looking into them now.

[dmlary]

I'm not sure what to do about the test cases that now unexpectedly pass. The removal of the assert() in ObjectFileELF::ApplyRelocations means each of these cases is now passing on what looks like 32-bit arm, but I don't know enough about the three tests to say that it's right to remove the XPASS. I think the buildbot is reporting that they do pass, so is it safe?

Unexpectedly Passed Tests (3):
  lldb-shell :: SymbolFile/DWARF/anon_class_w_and_wo_export_symbols.ll
  lldb-shell :: SymbolFile/DWARF/clang-ast-from-dwarf-unamed-and-anon-structs.cpp
  lldb-shell :: SymbolFile/DWARF/split-optimized.c

[labath]

Yeah, we can enable them. I've done that now in d079bf33.

lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp

public:
   static unsigned RelocOffset32(const ELFRelocation &rel);
 
   static unsigned RelocOffset64(const ELFRelocation &rel);
 
   static unsigned RelocAddend32(const ELFRelocation &rel);
 
   static unsigned RelocAddend64(const ELFRelocation &rel);
 
+  bool IsRela() { return (reloc.is<ELFRela *>()); }
+
 private:
   typedef llvm::PointerUnion<ELFRel *, ELFRela *> RelocUnion;
 
   RelocUnion reloc;
 };
 } // end anonymous namespace
 
 ELFRelocation::ELFRelocation(unsigned type) {
if (hdr->Is32Bit()) {
     reloc_type = ELFRelocation::RelocType32;
     reloc_symbol = ELFRelocation::RelocSymbol32;
   } else {
     reloc_type = ELFRelocation::RelocType64;
     reloc_symbol = ELFRelocation::RelocSymbol64;
   }
 
   for (unsigned i = 0; i < num_relocations; ++i) {
-    if (!rel.Parse(rel_data, &offset))
+    if (!rel.Parse(rel_data, &offset)) {
+      GetModule()->ReportError(".rel%s[%d] failed to parse relocation",
+                               rel_section->GetName().AsCString(), i);
       break;
-
+    }
     Symbol *symbol = nullptr;
 
     if (hdr->Is32Bit()) {
       switch (reloc_type(rel)) {
       case R_386_32:
+        symbol = symtab->FindSymbolByID(reloc_symbol(rel));
+        if (symbol) {
+          addr_t f_offset =
+              rel_section->GetFileOffset() + ELFRelocation::RelocOffset32(rel);
+          DataBufferSP &data_buffer_sp = debug_data.GetSharedDataBuffer();
+          // ObjectFileELF creates a WritableDataBuffer in CreateInstance.
+          WritableDataBuffer *data_buffer =
+              llvm::cast<WritableDataBuffer>(data_buffer_sp.get());
+          uint32_t *dst = reinterpret_cast<uint32_t *>(
+              data_buffer->GetBytes() + f_offset);
+
+          addr_t value = symbol->GetAddressRef().GetFileAddress();
+          if (rel.IsRela()) {
+            value += ELFRelocation::RelocAddend32(rel);
+          } else {
+            value += *dst;
+          }
+          *dst = value;
+        } else {
+          GetModule()->ReportError(".rel%s[%u] unknown symbol id: %d",
+                                   rel_section->GetName().AsCString(), i,
+                                   reloc_symbol(rel));
+        }
+        break;
       case R_386_PC32:
       default:
         // FIXME: This asserts with this input:
         //
         // foo.cpp
         // int main(int argc, char **argv) { return 0; }
         //
         // clang++.exe --target=i686-unknown-linux-gnu -g -c foo.cpp -o foo.o
         //
         // and running this on the foo.o module.
+        GetModule()->ReportError("unsupported 32-bit relocation:"
+                                 " .rel%s[%u], type %u",
+                                 rel_section->GetName().AsCString(), i,
+                                 reloc_type(rel));
         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();