Skip to content

Conversation

clayborg
Copy link
Collaborator

@clayborg clayborg commented Mar 4, 2025

ObjectFileJSON sections didn't work, they were set to zero all of the time. Fixed the bug and fixed the test to ensure it was testing real values.

ObjectFileJSON sections didn't work, they were set to zero all of the time. Fixed the bug and fixed the test to ensure it was testing real values.
@clayborg clayborg requested a review from JDevlieghere as a code owner March 4, 2025 05:49
@llvmbot llvmbot added the lldb label Mar 4, 2025
@llvmbot
Copy link
Member

llvmbot commented Mar 4, 2025

@llvm/pr-subscribers-lldb

Author: Greg Clayton (clayborg)

Changes

ObjectFileJSON sections didn't work, they were set to zero all of the time. Fixed the bug and fixed the test to ensure it was testing real values.


Full diff: https://github.com/llvm/llvm-project/pull/129648.diff

3 Files Affected:

  • (modified) lldb/source/Core/Section.cpp (+1-1)
  • (modified) lldb/source/Plugins/ObjectFile/JSON/ObjectFileJSON.cpp (+10-3)
  • (modified) lldb/test/API/functionalities/json/object-file/TestObjectFileJSON.py (+59-18)
diff --git a/lldb/source/Core/Section.cpp b/lldb/source/Core/Section.cpp
index a17f43fe89033..96410a1ad497c 100644
--- a/lldb/source/Core/Section.cpp
+++ b/lldb/source/Core/Section.cpp
@@ -690,7 +690,7 @@ bool fromJSON(const llvm::json::Value &value,
               lldb_private::JSONSection &section, llvm::json::Path path) {
   llvm::json::ObjectMapper o(value, path);
   return o && o.map("name", section.name) && o.map("type", section.type) &&
-         o.map("size", section.address) && o.map("size", section.size);
+         o.map("address", section.address) && o.map("size", section.size);
 }
 
 bool fromJSON(const llvm::json::Value &value, lldb::SectionType &type,
diff --git a/lldb/source/Plugins/ObjectFile/JSON/ObjectFileJSON.cpp b/lldb/source/Plugins/ObjectFile/JSON/ObjectFileJSON.cpp
index ffbd87714242c..5638cea33f4f5 100644
--- a/lldb/source/Plugins/ObjectFile/JSON/ObjectFileJSON.cpp
+++ b/lldb/source/Plugins/ObjectFile/JSON/ObjectFileJSON.cpp
@@ -182,9 +182,16 @@ void ObjectFileJSON::CreateSections(SectionList &unified_section_list) {
   lldb::user_id_t id = 1;
   for (const auto &section : m_sections) {
     auto section_sp = std::make_shared<Section>(
-        GetModule(), this, id++, ConstString(section.name),
-        section.type.value_or(eSectionTypeCode), 0, section.size.value_or(0), 0,
-        section.size.value_or(0), /*log2align*/ 0, /*flags*/ 0);
+        GetModule(), this,
+        /*sect_id=*/id++,
+        /*name=*/ConstString(section.name),
+        /*sect_type=*/section.type.value_or(eSectionTypeCode),
+        /*file_vm_addr*/section.address.value_or(0),
+        /*vm_size*/section.size.value_or(0),
+        /*file_offset=*/0,
+        /*file_size=*/0,
+        /*log2align*/0,
+        /*flags*/0);
     m_sections_up->AddSection(section_sp);
     unified_section_list.AddSection(section_sp);
   }
diff --git a/lldb/test/API/functionalities/json/object-file/TestObjectFileJSON.py b/lldb/test/API/functionalities/json/object-file/TestObjectFileJSON.py
index efb1aa2c3ad8a..40dab2bf2bcb0 100644
--- a/lldb/test/API/functionalities/json/object-file/TestObjectFileJSON.py
+++ b/lldb/test/API/functionalities/json/object-file/TestObjectFileJSON.py
@@ -59,7 +59,15 @@ def test_module(self):
 
         module = target.AddModule(self.toModuleSpec(json_object_file_b))
         self.assertFalse(module.IsValid())
-
+        TEXT_file_addr = 0x100000000
+        DATA_file_addr = 0x100001000
+        foo_file_addr = TEXT_file_addr + 0x100
+        bar_file_addr = DATA_file_addr + 0x10
+        TEXT_size = 0x222
+        DATA_size = 0x333
+        foo_size = 0x11
+        bar_size = 0x22
+        slide = 0x100000000
         data = {
             "triple": target.GetTriple(),
             "uuid": str(uuid.uuid4()),
@@ -68,36 +76,69 @@ def test_module(self):
                 {
                     "name": "__TEXT",
                     "type": "code",
-                    "address": 0,
-                    "size": 0x222,
+                    "address": TEXT_file_addr,
+                    "size": TEXT_size,
+                },
+                {
+                    "name": "__DATA",
+                    "type": "code",
+                    "address": DATA_file_addr,
+                    "size": DATA_size,
                 }
             ],
             "symbols": [
                 {
                     "name": "foo",
-                    "address": 0x100,
-                    "size": 0x11,
+                    "type": "code",
+                    "address": foo_file_addr,
+                    "size": foo_size,
+                },
+                {
+                    "name": "bar",
+                    "type": "data",
+                    "address": bar_file_addr,
+                    "size": bar_size,
                 }
+
             ],
         }
 
         json_object_file_c = self.getBuildArtifact("c.json")
+        print('EXE = %s' % (json_object_file_c))
         self.emitJSON(data, json_object_file_c)
 
         module = target.AddModule(self.toModuleSpec(json_object_file_c))
         self.assertTrue(module.IsValid())
 
-        section = module.GetSectionAtIndex(0)
-        self.assertTrue(section.IsValid())
-        self.assertEqual(section.GetName(), "__TEXT")
-        self.assertEqual(section.file_addr, 0x0)
-        self.assertEqual(section.size, 0x222)
-
-        symbol = module.FindSymbol("foo")
-        self.assertTrue(symbol.IsValid())
-        self.assertEqual(symbol.addr.GetFileAddress(), 0x100)
-        self.assertEqual(symbol.GetSize(), 0x11)
-
-        error = target.SetSectionLoadAddress(section, 0x1000)
+        text_section = module.GetSectionAtIndex(0)
+        self.assertTrue(text_section.IsValid())
+        self.assertEqual(text_section.GetName(), "__TEXT")
+        self.assertEqual(text_section.file_addr, TEXT_file_addr)
+        self.assertEqual(text_section.size, TEXT_size)
+
+        data_section = module.GetSectionAtIndex(1)
+        self.assertTrue(data_section.IsValid())
+        self.assertEqual(data_section.GetName(), "__DATA")
+        self.assertEqual(data_section.file_addr, DATA_file_addr)
+        self.assertEqual(data_section.size, DATA_size)
+
+        foo_symbol = module.FindSymbol("foo")
+        self.assertTrue(foo_symbol.IsValid())
+        self.assertEqual(foo_symbol.addr.GetFileAddress(), foo_file_addr)
+        self.assertEqual(foo_symbol.GetSize(), foo_size)
+
+        bar_symbol = module.FindSymbol("bar")
+        self.assertTrue(bar_symbol.IsValid())
+        self.assertEqual(bar_symbol.addr.GetFileAddress(), bar_file_addr)
+        self.assertEqual(bar_symbol.GetSize(), bar_size)
+
+        error = target.SetSectionLoadAddress(text_section,
+                                             TEXT_file_addr + slide)
+        self.assertSuccess(error)
+        error = target.SetSectionLoadAddress(data_section,
+                                             DATA_file_addr + slide)
         self.assertSuccess(error)
-        self.assertEqual(symbol.addr.GetLoadAddress(target), 0x1100)
+        self.assertEqual(foo_symbol.addr.GetLoadAddress(target),
+                         foo_file_addr + slide)
+        self.assertEqual(bar_symbol.addr.GetLoadAddress(target),
+                         bar_file_addr + slide)

Copy link

github-actions bot commented Mar 4, 2025

✅ With the latest revision this PR passed the Python code formatter.

Copy link

github-actions bot commented Mar 4, 2025

✅ With the latest revision this PR passed the C/C++ code formatter.

@DavidSpickett
Copy link
Collaborator

FYI there are still Python formatting changes to fix (took me a bit to realise that those are two separate checks).

@DavidSpickett DavidSpickett changed the title Fix ObjectFileJSON to section addresses. [lldb] Fix ObjectFileJSON to section addresses. Mar 4, 2025
Copy link
Collaborator

@DavidSpickett DavidSpickett left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks ok to me but assuming this is https://lldb.llvm.org/use/symbolfilejson.html then @JDevlieghere should also take a look, iirc they documented that recently.

}

json_object_file_c = self.getBuildArtifact("c.json")
print('EXE = %s' % (json_object_file_c))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove debug print.

Copy link
Member

@JDevlieghere JDevlieghere left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! LGTM with the print removed.

@clayborg clayborg merged commit 7b596ce into llvm:main Mar 4, 2025
8 of 9 checks passed
@clayborg clayborg deleted the objfile-json-fix branch March 4, 2025 22:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants