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