sgraenitz created this revision.
sgraenitz added reviewers: labath, clayborg, bkoropoff, loladiro, lhames.
Herald added a project: LLDB.
sgraenitz requested review of this revision.
Herald added a subscriber: JDevlieghere.

Part 2 of a fix for JITed code debugging. This has been a regression from 5.0 
to 6.0 and it's still reproducible on current master: 
https://bugs.llvm.org/show_bug.cgi?id=36209 Part 1 was D61611 
<https://reviews.llvm.org/D61611> a while ago.

The in-memory object files we obtain from JITLoaderGDB are not yet relocated. 
It looks like this used to happen on the LLDB side and my guess is that it 
broke with D38142 <https://reviews.llvm.org/D38142>. (However, it's hard to 
tell because the whole thing was broken already due to the bug in part 1.) The 
patch moved relocation resolution to a later point in time and didn't apply it 
to in-memory objects. I am not aware of any reason why we wouldn't resolve 
relocations per-se, so I made it unconditional here. On Debian, it fixes the 
bug for me and all tests in `check-lldb` are still fine.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D90769

Files:
  lldb/source/Symbol/ObjectFile.cpp
  lldb/test/Shell/Breakpoint/jitbp_elf.test


Index: lldb/test/Shell/Breakpoint/jitbp_elf.test
===================================================================
--- lldb/test/Shell/Breakpoint/jitbp_elf.test
+++ lldb/test/Shell/Breakpoint/jitbp_elf.test
@@ -7,5 +7,8 @@
 # CHECK: Breakpoint 1: no locations (pending).
 # CHECK: (lldb) run -jit-kind=mcjit {{.*}}/jitbp_elf.test.tmp.ll
 # CHECK: Process {{.*}} stopped
-# CHECK: JIT(0x{{.*}})`jitbp:
+# CHECK: JIT(0x{{.*}})`jitbp() at jitbp.cpp:1:15
+# CHECK: -> 1    int jitbp() { return 0; }
+# CHECK:                       ^
+# CHECK:    2    int main() { return jitbp(); }
 # CHECK: Process {{.*}} launched: {{.*}}
Index: lldb/source/Symbol/ObjectFile.cpp
===================================================================
--- lldb/source/Symbol/ObjectFile.cpp
+++ lldb/source/Symbol/ObjectFile.cpp
@@ -503,6 +503,9 @@
     return section->GetObjectFile()->ReadSectionData(section, section_offset,
                                                      dst, dst_len);
 
+  if (!section->IsRelocated())
+    RelocateSection(section);
+
   if (IsInMemory()) {
     ProcessSP process_sp(m_process_wp.lock());
     if (process_sp) {
@@ -514,9 +517,6 @@
                                       dst_len, error);
     }
   } else {
-    if (!section->IsRelocated())
-      RelocateSection(section);
-
     const lldb::offset_t section_file_size = section->GetFileSize();
     if (section_offset < section_file_size) {
       const size_t section_bytes_left = section_file_size - section_offset;
@@ -547,6 +547,9 @@
   if (section->GetObjectFile() != this)
     return section->GetObjectFile()->ReadSectionData(section, section_data);
 
+  if (!section->IsRelocated())
+    RelocateSection(section);
+
   if (IsInMemory()) {
     ProcessSP process_sp(m_process_wp.lock());
     if (process_sp) {
@@ -563,17 +566,12 @@
         }
       }
     }
-    return GetData(section->GetFileOffset(), section->GetFileSize(),
-                   section_data);
-  } else {
-    // The object file now contains a full mmap'ed copy of the object file
-    // data, so just use this
-    if (!section->IsRelocated())
-      RelocateSection(section);
-
-    return GetData(section->GetFileOffset(), section->GetFileSize(),
-                   section_data);
   }
+
+  // The object file now contains a full mmap'ed copy of the object file
+  // data, so just use this
+  return GetData(section->GetFileOffset(), section->GetFileSize(),
+                  section_data);
 }
 
 bool ObjectFile::SplitArchivePathWithObject(llvm::StringRef path_with_object,


Index: lldb/test/Shell/Breakpoint/jitbp_elf.test
===================================================================
--- lldb/test/Shell/Breakpoint/jitbp_elf.test
+++ lldb/test/Shell/Breakpoint/jitbp_elf.test
@@ -7,5 +7,8 @@
 # CHECK: Breakpoint 1: no locations (pending).
 # CHECK: (lldb) run -jit-kind=mcjit {{.*}}/jitbp_elf.test.tmp.ll
 # CHECK: Process {{.*}} stopped
-# CHECK: JIT(0x{{.*}})`jitbp:
+# CHECK: JIT(0x{{.*}})`jitbp() at jitbp.cpp:1:15
+# CHECK: -> 1    int jitbp() { return 0; }
+# CHECK:                       ^
+# CHECK:    2    int main() { return jitbp(); }
 # CHECK: Process {{.*}} launched: {{.*}}
Index: lldb/source/Symbol/ObjectFile.cpp
===================================================================
--- lldb/source/Symbol/ObjectFile.cpp
+++ lldb/source/Symbol/ObjectFile.cpp
@@ -503,6 +503,9 @@
     return section->GetObjectFile()->ReadSectionData(section, section_offset,
                                                      dst, dst_len);
 
+  if (!section->IsRelocated())
+    RelocateSection(section);
+
   if (IsInMemory()) {
     ProcessSP process_sp(m_process_wp.lock());
     if (process_sp) {
@@ -514,9 +517,6 @@
                                       dst_len, error);
     }
   } else {
-    if (!section->IsRelocated())
-      RelocateSection(section);
-
     const lldb::offset_t section_file_size = section->GetFileSize();
     if (section_offset < section_file_size) {
       const size_t section_bytes_left = section_file_size - section_offset;
@@ -547,6 +547,9 @@
   if (section->GetObjectFile() != this)
     return section->GetObjectFile()->ReadSectionData(section, section_data);
 
+  if (!section->IsRelocated())
+    RelocateSection(section);
+
   if (IsInMemory()) {
     ProcessSP process_sp(m_process_wp.lock());
     if (process_sp) {
@@ -563,17 +566,12 @@
         }
       }
     }
-    return GetData(section->GetFileOffset(), section->GetFileSize(),
-                   section_data);
-  } else {
-    // The object file now contains a full mmap'ed copy of the object file
-    // data, so just use this
-    if (!section->IsRelocated())
-      RelocateSection(section);
-
-    return GetData(section->GetFileOffset(), section->GetFileSize(),
-                   section_data);
   }
+
+  // The object file now contains a full mmap'ed copy of the object file
+  // data, so just use this
+  return GetData(section->GetFileOffset(), section->GetFileSize(),
+                  section_data);
 }
 
 bool ObjectFile::SplitArchivePathWithObject(llvm::StringRef path_with_object,
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to