[Lldb-commits] [PATCH] D51566: Add a relocation to ObjectFileELF::ApplyRelocations and a test
lanza added a comment. LGTM and the test case passes. Thanks a lot, Davide! Repository: rLLDB LLDB https://reviews.llvm.org/D51566 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D51566: Add a relocation to ObjectFileELF::ApplyRelocations and a test
davide added a comment. In https://reviews.llvm.org/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 0x61f01526 for type > 'uint32_t' (aka 'unsigned int'), which requires 4 byte alignment > 0x61f01526: 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 https://reviews.llvm.org/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 0x61f01526 for type > 'uint32_t' (aka 'unsigned int'), which requires 4 byte alignment > 0x61f01526: 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 Repository: rLLDB LLDB https://reviews.llvm.org/D51566 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D51566: Add a relocation to ObjectFileELF::ApplyRelocations and a test
davide added a comment. @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 0x61f01526 for type 'uint32_t' (aka 'unsigned int'), which requires 4 byte alignment 0x61f01526: 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 Repository: rLLDB LLDB https://reviews.llvm.org/D51566 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D51566: Add a relocation to ObjectFileELF::ApplyRelocations and a test
This revision was automatically updated to reflect the committed changes.
Closed by commit rLLDB346171: Add a relocation to
ObjectFileELF::ApplyRelocations and a test (authored by lanza, committed by ).
Herald added a subscriber: lldb-commits.
Changed prior to commit:
https://reviews.llvm.org/D51566?vs=163617=172654#toc
Repository:
rLLDB LLDB
https://reviews.llvm.org/D51566
Files:
source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
unittests/ObjectFile/ELF/CMakeLists.txt
unittests/ObjectFile/ELF/Inputs/debug-info-relocations.pcm.yaml
unittests/ObjectFile/ELF/TestObjectFileELF.cpp
Index: source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
===
--- source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
+++ source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
@@ -2703,6 +2703,7 @@
}
} else {
switch (reloc_type(rel)) {
+ case R_AARCH64_ABS64:
case R_X86_64_64: {
symbol = symtab->FindSymbolByID(reloc_symbol(rel));
if (symbol) {
@@ -2722,13 +2723,15 @@
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 & 0x);
DataBufferSP _buffer_sp = debug_data.GetSharedDataBuffer();
Index: unittests/ObjectFile/ELF/TestObjectFileELF.cpp
===
--- unittests/ObjectFile/ELF/TestObjectFileELF.cpp
+++ unittests/ObjectFile/ELF/TestObjectFileELF.cpp
@@ -16,6 +16,7 @@
#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"
@@ -141,3 +142,64 @@
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 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(spec);
+
+ auto objfile = static_cast(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 _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);
+
