aadsm created this revision.
aadsm added reviewers: clayborg, labath, wallace.
Herald added subscribers: lldb-commits, MaskRay, kristof.beyls, arichardson, 
emaste, srhines.
Herald added a reviewer: espindola.
Herald added a project: LLDB.

I found a case where the main android binary (app_process32) had thumb code at 
its entry point but no entry in the symbol table indicating this. This made 
lldb set a 4 byte breakpoint at that address (we default to arm code) instead 
of a 2 byte one (like we should for thumb).
The big deal with this is that the expression evaluator uses the entry point as 
a way to know when a JITed expression has finished executing by putting a 
breakpoint there. Because of this, evaluating expressions on certain android 
devices (Google Pixel something) made the process crash.
This was fixed by checking this specific situation when we parse the symbol 
table and add an artificial symbol for this 2 byte range and indicating that 
it's arm thumb.

I created 2 unit tests for this, one to check that now we know that the entry 
point is arm thumb, and the other to make sure we didn't change the behaviour 
for arm code.

I also run the following on the command line with the `app_process32` where I 
found the issue:
**Before:**

  (lldb) dis -s 0x1640 -e 0x1644
  app_process32[0x1640]: .long  0xf0004668                ; unknown opcode

**After:**

  (lldb) dis -s 0x1640 -e 0x1644
  app_process32`:
  app_process32[0x1640] <+0>: mov    r0, sp
  app_process32[0x1642]:      andeq  r0, r0, r0


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D68069

Files:
  lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
  lldb/unittests/ObjectFile/ELF/TestObjectFileELF.cpp

Index: lldb/unittests/ObjectFile/ELF/TestObjectFileELF.cpp
===================================================================
--- lldb/unittests/ObjectFile/ELF/TestObjectFileELF.cpp
+++ lldb/unittests/ObjectFile/ELF/TestObjectFileELF.cpp
@@ -172,3 +172,131 @@
   Uuid.SetFromStringRef("1b8a73ac238390e32a7ff4ac8ebe4d6a41ecf5c9", 20);
   EXPECT_EQ(Spec.GetUUID(), Uuid);
 }
+
+TEST_F(ObjectFileELFTest, GetSymtab_NoSymEntryPointArmThumbAddressClass) {
+  /*
+  // nosym-entrypoint-arm-thumb.s
+  .global _Start
+  .thumb_func
+  _start:
+      mov r0, #42
+      mov r7, #1
+      svc #0
+  // arm-linux-androideabi-as nosym-entrypoint-arm-thumb.s
+  //   -o nosym-entrypoint-arm-thumb.o
+  // arm-linux-androideabi-ld nosym-entrypoint-arm-thumb.o
+  //   -o nosym-entrypoint-arm-thumb -e 0x8075 -s
+  */
+  auto ExpectedFile = TestFile::fromYaml(R"(
+--- !ELF
+FileHeader:
+  Class:           ELFCLASS32
+  Data:            ELFDATA2LSB
+  Type:            ET_EXEC
+  Machine:         EM_ARM
+  Flags:           [ EF_ARM_SOFT_FLOAT, EF_ARM_EABI_VER5 ]
+  Entry:           0x0000000000008075
+Sections:
+  - Name:            .text
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_ALLOC, SHF_EXECINSTR ]
+    Address:         0x0000000000008074
+    AddressAlign:    0x0000000000000002
+    Content:         2A20012700DF
+  - Name:            .data
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_WRITE, SHF_ALLOC ]
+    Address:         0x0000000000009000
+    AddressAlign:    0x0000000000000001
+    Content:         ''
+  - Name:            .bss
+    Type:            SHT_NOBITS
+    Flags:           [ SHF_WRITE, SHF_ALLOC ]
+    Address:         0x0000000000009000
+    AddressAlign:    0x0000000000000001
+  - Name:            .note.gnu.gold-version
+    Type:            SHT_NOTE
+    AddressAlign:    0x0000000000000004
+    Content:         040000000900000004000000474E5500676F6C6420312E3131000000
+  - Name:            .ARM.attributes
+    Type:            SHT_ARM_ATTRIBUTES
+    AddressAlign:    0x0000000000000001
+    Content:         '4113000000616561626900010900000006020901'
+...
+)");
+  ASSERT_THAT_EXPECTED(ExpectedFile, llvm::Succeeded());
+
+  ModuleSpec spec{FileSpec(ExpectedFile->name())};
+  spec.GetSymbolFileSpec().SetFile(ExpectedFile->name(),
+                                   FileSpec::Style::native);
+  auto module_sp = std::make_shared<Module>(spec);
+
+  auto entry_point_addr = module_sp->GetObjectFile()->GetEntryPointAddress();
+  ASSERT_TRUE(entry_point_addr.GetOffset() & 1);
+  // Decrease the offsite by 1 to make it into a breakable address since this
+  // is Thumb.
+  entry_point_addr.SetOffset(entry_point_addr.GetOffset() - 1);
+  ASSERT_EQ(entry_point_addr.GetAddressClass(),
+            AddressClass::eCodeAlternateISA);
+}
+
+TEST_F(ObjectFileELFTest, GetSymtab_NoSymEntryPointArmAddressClass) {
+  /*
+  // nosym-entrypoint-arm.s
+  .global _Start
+  _start:
+      mov r0, #42
+      mov r7, #1
+      svc #0
+  // arm-linux-androideabi-as nosym-entrypoint-arm.s
+  //   -o nosym-entrypoint-arm.o
+  // arm-linux-androideabi-ld nosym-entrypoint-arm.o
+  //   -o nosym-entrypoint-arm -e 0x8074 -s
+  */
+  auto ExpectedFile = TestFile::fromYaml(R"(
+--- !ELF
+FileHeader:
+  Class:           ELFCLASS32
+  Data:            ELFDATA2LSB
+  Type:            ET_EXEC
+  Machine:         EM_ARM
+  Flags:           [ EF_ARM_SOFT_FLOAT, EF_ARM_EABI_VER5 ]
+  Entry:           0x0000000000008074
+Sections:
+  - Name:            .text
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_ALLOC, SHF_EXECINSTR ]
+    Address:         0x0000000000008074
+    AddressAlign:    0x0000000000000004
+    Content:         2A00A0E30170A0E3000000EF
+  - Name:            .data
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_WRITE, SHF_ALLOC ]
+    Address:         0x0000000000009000
+    AddressAlign:    0x0000000000000001
+    Content:         ''
+  - Name:            .bss
+    Type:            SHT_NOBITS
+    Flags:           [ SHF_WRITE, SHF_ALLOC ]
+    Address:         0x0000000000009000
+    AddressAlign:    0x0000000000000001
+  - Name:            .note.gnu.gold-version
+    Type:            SHT_NOTE
+    AddressAlign:    0x0000000000000004
+    Content:         040000000900000004000000474E5500676F6C6420312E3131000000
+  - Name:            .ARM.attributes
+    Type:            SHT_ARM_ATTRIBUTES
+    AddressAlign:    0x0000000000000001
+    Content:         '4113000000616561626900010900000006010801'
+...
+)");
+  ASSERT_THAT_EXPECTED(ExpectedFile, llvm::Succeeded());
+
+  ModuleSpec spec{FileSpec(ExpectedFile->name())};
+  spec.GetSymbolFileSpec().SetFile(ExpectedFile->name(),
+                                   FileSpec::Style::native);
+  auto module_sp = std::make_shared<Module>(spec);
+
+  auto entry_point_addr = module_sp->GetObjectFile()->GetEntryPointAddress();
+  ASSERT_EQ(entry_point_addr.GetAddressClass(), AddressClass::eUnknown);
+}
\ No newline at end of file
Index: lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
===================================================================
--- lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
+++ lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
@@ -2703,6 +2703,47 @@
     if (m_symtab_up == nullptr)
       m_symtab_up.reset(new Symtab(this));
 
+    // When the entry point is arm thumb and there's no symbol entry for it in
+    // the table then we don't know its address class. This makes it impossible
+    // to correctly set a breakpoint there since we'll assume arm code. I found
+    // this to be the case in an android binary (app_process32).
+    // Since this is the mechanism the expression evaluator uses to know when a
+    // JITed expression has finished executing we need to know the correct
+    // address class.
+    // We get around this by checking if the entry point is arm thumb (by means
+    // of its least significant bit), when no entry symbol is found for it we
+    // create an articial one and set the correct address class for that
+    // address.
+    ArchSpec arch = GetArchitecture();
+    if (arch.GetMachine() == llvm::Triple::arm) {
+      auto entry_point_addr = GetEntryPointAddress().GetFileAddress();
+      if (entry_point_addr != LLDB_INVALID_ADDRESS && (entry_point_addr & 1)) {
+        auto opcode_addr = entry_point_addr ^ 1;
+        if (!m_symtab_up->FindSymbolContainingFileAddress(opcode_addr)) {
+          uint64_t symbol_id = m_symtab_up->GetNumSymbols();
+          SectionSP section_sp =
+              GetSectionList()->FindSectionContainingFileAddress(opcode_addr);
+          Symbol symbol(
+              symbol_id,
+              "",              // Symbol name.
+              false,           // Is the symbol name mangled?
+              eSymbolTypeCode, // Type of this symbol.
+              true,            // Is this globally visible?
+              false,           // Is this symbol debug info?
+              false,           // Is this symbol a trampoline?
+              true,            // Is this symbol artificial?
+              section_sp, // Section in which this symbol is defined or null.
+              0,          // Offset in section or symbol value.
+              2,          // Size.
+              true,       // Size is valid.
+              false,      // Contains linker annotations?
+              0);         // Symbol flags.
+          m_symtab_up->AddSymbol(symbol);
+          m_address_class_map[opcode_addr] = AddressClass::eCodeAlternateISA;
+        }
+      }
+    }
+
     m_symtab_up->CalculateSymbolSizes();
   }
 
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to