[LLDB] added getName method in SBModule #150331

barsolo2000 · 2025-07-23T22:35:45Z

added getName method in SBModule.h and .cpp in order to get the name of the module from m_object_name.

github-actions · 2025-07-23T22:36:01Z

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

llvmbot · 2025-07-23T22:36:37Z

@llvm/pr-subscribers-lldb

Author: None (barsolo2000)

Changes

added getName method in SBModule.h and .cpp in order to get the name of the module from m_object_name.

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

2 Files Affected:

(modified) lldb/include/lldb/API/SBModule.h (+3)
(modified) lldb/source/API/SBModule.cpp (+8)

diff --git a/lldb/include/lldb/API/SBModule.h b/lldb/include/lldb/API/SBModule.h
index 85332066ee687..ed90c48849699 100644
--- a/lldb/include/lldb/API/SBModule.h
+++ b/lldb/include/lldb/API/SBModule.h
@@ -296,6 +296,9 @@ class LLDB_API SBModule {
   /// Remove any global modules which are no longer needed.
   static void GarbageCollectAllocatedModules();
 
+  /// Return the name of the module.
+  const char *GetName() const;
+
 private:
   friend class SBAddress;
   friend class SBFrame;
diff --git a/lldb/source/API/SBModule.cpp b/lldb/source/API/SBModule.cpp
index 985107ec68efd..9eb0ae3cb3dba 100644
--- a/lldb/source/API/SBModule.cpp
+++ b/lldb/source/API/SBModule.cpp
@@ -671,3 +671,11 @@ void SBModule::GarbageCollectAllocatedModules() {
   const bool mandatory = false;
   ModuleList::RemoveOrphanSharedModules(mandatory);
 }
+
+const char *SBModule::GetName() const {
+  LLDB_INSTRUMENT_VA(this);
+  if (!m_opaque_sp) {
+    return nullptr;
+  }
+  return m_opaque_sp->GetObjectName().AsCString();
+}
\ No newline at end of file

JDevlieghere

TL;DR:

const char *SBModule::GetName() const {
  LLDB_INSTRUMENT_VA(this);

  if (!m_opaque_sp) 
    return nullptr;

  return m_opaque_sp->GetObjectName().AsCString();
}

This also needs a test.

JDevlieghere · 2025-07-23T23:52:01Z

lldb/source/API/SBModule.cpp

+  if (!m_opaque_sp) {
+    return nullptr;
+  }


No braces around single-line ifs.

Suggested change

if (!m_opaque_sp) {

return nullptr;

}

if (!m_opaque_sp)

return nullptr;

JDevlieghere · 2025-07-23T23:52:27Z

lldb/source/API/SBModule.cpp

+  if (!m_opaque_sp) {
+    return nullptr;
+  }
+  auto mod_name = m_opaque_sp->GetObjectName();


Don't use auto if the type isn't obvious from the right hand side.

Suggested change

auto mod_name = m_opaque_sp->GetObjectName();

ConstString mod_name = m_opaque_sp->GetObjectName();

JDevlieghere · 2025-07-23T23:52:49Z

lldb/source/API/SBModule.cpp

+  if (!mod_name) {
+    return nullptr;
+  }
+  return mod_name.AsCString();


AsCString will return nullptr if the ConstString is empty, so the previous check is redundant.

JDevlieghere · 2025-07-23T23:53:12Z

lldb/source/API/SBModule.cpp

@@ -671,3 +671,15 @@ void SBModule::GarbageCollectAllocatedModules() {
  const bool mandatory = false;
  ModuleList::RemoveOrphanSharedModules(mandatory);
 }
+
+const char *SBModule::GetName() const {
+  LLDB_INSTRUMENT_VA(this);


There should be a newline after LLDB_INSTRUMENT_VA

Suggested change

LLDB_INSTRUMENT_VA(this);

LLDB_INSTRUMENT_VA(this);

JDevlieghere · 2025-07-23T23:53:35Z

lldb/source/API/SBModule.cpp

+    return nullptr;
+  }
+  return mod_name.AsCString();
+}


Missing newline.

Jlalond · 2025-07-24T21:08:20Z

This also needs a test.

@JDevlieghere I'm working with Bar on this, one expected issue we're experiencing is almost every module has a null name. Even trying on Darwin if we loop over all the modules all the names are none.

The original rationale behind this patch was there are sometimes modules without a UUID, and I wanted to log the name of these modules, especially given Module itself exposes a name. However it appears my hypothesis was wrong, any idea or context why that is?

CC: @clayborg

jimingham · 2025-07-24T23:04:11Z

This also needs a test.

@JDevlieghere I'm working with Bar on this, one expected issue we're experiencing is almost every module has a null name. Even trying on Darwin if we loop over all the modules all the names are none.

The original rationale behind this patch was there are sometimes modules without a UUID, and I wanted to log the name of these modules, especially given Module itself exposes a name. However it appears my hypothesis was wrong, any idea or context why that is?

CC: @clayborg

That name is actually derived from the ObjectFile, not from the module. In some of the Module constructors we don't make the ObjectFile at construction time, but wait till someone calls Module::GetObjectFile. But from what I can see GetObjectFile doesn't actually set m_object_name. That's probably what you are seeing.

But also, if you are on the lazy code path, you might be in a state where we haven't made the object file yet - in which case we haven't set m_object_name. So your GetName probably has to call GetObjectFile first - that will be a no-op if the ObjectFile has been made - and then access m_object_name.

labath · 2025-07-25T10:03:08Z

I don't think that m_object_name is what you're looking for. It's supposed to be used for disambiguation when a single physical file contains multiple logical object files, such as with BSD archives. It being empty for "normal" files is kind of expected:

  ConstString m_object_name; ///< The name an object within this module that is
                             /// selected, or empty of the module is represented
                             /// by \a m_file.

Though I guess that brings up the question of what is the expected result of GetName for files within BSD archives...

bulbazord · 2025-07-28T20:32:08Z

lldb/source/API/SBModule.cpp

+
+  if (!m_opaque_sp)
+    return nullptr; 
+  m_opaque_sp->GetObjectFile();


This feels like a code smell. GetObjectName should probably fill in the object file for you instead of manually needing to call this before GetObjectName.

labath · 2025-07-29T08:56:25Z

lldb/test/API/python_api/sbmodule/TestSBModule.py

+#If no valid file, expect GetName() to be a non - empty string
+                self.assertIsInstance(name, str)
+                self.assertTrue(name, "Expected a non-empty name for module without a valid file")


Why should that be true? I think we need to start by defining what exactly do you expect this function to return.

@labath

I think the best case for this API would be returning a valid string identifier when there is no backing file. VDSO is the example I have in mind but a module jitted in memory only could also make sense.

Per the code you linked

ConstString m_object_name; ///< The name an object within this module that is /// selected, or empty of the module is represented /// by \a m_file.

Shouldn't this be the case? I'm very open to how we implement this but my goals align with the comments, which is obtaining a valid identifier when I can't fall back to the file for identification

Shouldn't this be the case?

I don't think so. This fields is really meant for the case where a single file represents multiple logical modules (in which case it disambiguates them), as is the case with BSD archives. See ObjectContainerBSDArchive::GetObjectFile to see how it's used.

In the case of vdso, the "name" ends up in the Module::m_file field, if I'm following the code in DynamicLoaderPOSIXDYLD::LoadVDSO correctly. The same thing happens with the JIT modules (JITLoaderGDB::ReadJITDescriptorImpl). Now, you could argue that's not the right place to put this information, and that it should go somehwere else... However, I don't think m_object_name is that place.

BTW, if you're looking for a truly unique identifier (and not just a name), you may want to look at #148877, which adds an integer ID for that purpose. I think it would be okay to expose that through the SB API if there's a use case for it.

What do you intend to use this for? Just show it to the user, or use in some sort of a critical capacity?

Thanks for the great context on this Pavel

What do you intend to use this for? Just show it to the user, or use in some sort of a critical capacity?

I'm currently trying to generate some metrics around Modules, and there are some cases where I don't have access to the UUID. I think the identifier would be great, but a generic name field would also be nice for logging.

Should we pivot this change to expose the m_file name if m_object_name is not present? If I follow correct the precedence of 'uniqueness' is

UUID

The numerical ID you linked

Object name

File name

Having a 'helper' accessor exposing the object name or the file name would solve my needs.

If I follow correct the precedence of 'uniqueness' is

I don't think you can really order these, as they all have different properties. The UUID (build-id) is assigned at build time, and it's supposed to be unique -- but it may not be assigned, and you can't really guarantee its uniqueness, which makes it unsuitable for some uses. The numeric ID uniquely identifies a loaded copy of a specific module, but it's a purely internal thing, so you can't use it to link two sessions together. The object name is only meaningful within an archive (you can have two archives with the same object), so it's best to think of these two as a single entity -- the file+object name pair uniquely identifies an object file on a specific file system at a specific point in time.

Having a 'helper' accessor exposing the object name or the file name would solve my needs.

The file name is available already through GetFileSpec and it shows the vdso "name":

(lldb) script for m in lldb.target.modules: print(m.GetFileSpec()) /bin/ls /lib64/ld-linux-x86-64.so.2 [vdso]

Is that enough? If you want to also access the object name, I think the simplest approach would be to expose m_object_name via Get*Object*Name() as that makes it clear what it's doing and it matches the function name of SBModuleSpec. Then you can combine these however you see fit on your end.

labath

The code looks good, but I have a problem with the docstring, as it captures none of the subtlety around this function (which was discussed in the other thread).

I'd recommend something like:
"If this Module represents a part of a larger file, returns the name of that part. Otherwise, returns nullptr."

The test is also pretty weak. Basically, all it does is verify that the result is a string. I'd suggest looking at TestBSDArchives.py. I think it should be fairly easy to extend that to cover this function as well -- just take one of the SBModuleSpecs floating around and use that to create SBModule (then call this function).

github-actions · 2025-08-04T19:54:53Z

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

github-actions · 2025-08-04T19:54:53Z

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

Summary: Test Plan: Reviewers: Subscribers: Tasks: Tags:

labath

Looks good, with a couple of adjustments to the test. You can construct a SBModule without adding it to a target. Adding the .o file to the target is unnecessary, and somewhat strange.

labath · 2025-08-08T10:16:26Z

lldb/test/API/python_api/sbmodule/TestSBModule.py

+            #create a module from the arhive using the sepc
+            module = target.AddModule(spec)


Suggested change

#create a module from the arhive using the sepc

module = target.AddModule(spec)

module = lldb.SBModule(spec)

labath · 2025-08-08T10:16:40Z

lldb/test/API/python_api/sbmodule/TestSBModule.py

+        target = self.dbg.CreateTarget(None)
+        self.assertTrue(target.IsValid(), "Target is valid")


Suggested change

target = self.dbg.CreateTarget(None)

self.assertTrue(target.IsValid(), "Target is valid")

barsolo2000 requested a review from JDevlieghere as a code owner July 23, 2025 22:35

llvmbot added the lldb label Jul 23, 2025

JDevlieghere requested changes Jul 23, 2025

View reviewed changes

bulbazord reviewed Jul 28, 2025

View reviewed changes

labath reviewed Jul 29, 2025

View reviewed changes

labath reviewed Aug 4, 2025

View reviewed changes

Bar Soloveychik added 9 commits August 5, 2025 13:35

[LLDB] added getName method in SBModule

140f405

added nullptr check

8241fcd

changed format and added a test to run darwin

fcfe476

added GetObjectfile and upgraded test to expect None

e741ae6

added changes to SBModule GetName

35c8da6

changed to GetObjectName()

4a085c3

added fixed description of function

f1b1ac5

format errors

c1d0e30

added test that check archive names

24b22ed

barsolo2000 force-pushed the SBmoduleNametask branch from 5e15dc5 to 24b22ed Compare August 5, 2025 20:35

fixed small issues.

94543de

Summary: Test Plan: Reviewers: Subscribers: Tasks: Tags:

labath approved these changes Aug 8, 2025

View reviewed changes

Bar Soloveychik added 2 commits August 8, 2025 11:52

fixed comments

0e70052

format fix

088fb76

	auto mod_name = m_opaque_sp->GetObjectName();
	ConstString mod_name = m_opaque_sp->GetObjectName();

		#create a module from the arhive using the sepc
		module = target.AddModule(spec)

	#create a module from the arhive using the sepc
	module = target.AddModule(spec)
	module = lldb.SBModule(spec)

		target = self.dbg.CreateTarget(None)
		self.assertTrue(target.IsValid(), "Target is valid")

[LLDB] added getName method in SBModule #150331

Are you sure you want to change the base?

[LLDB] added getName method in SBModule #150331

Uh oh!

Conversation

barsolo2000 commented Jul 23, 2025

Uh oh!

github-actions bot commented Jul 23, 2025

Uh oh!

llvmbot commented Jul 23, 2025

Uh oh!

JDevlieghere left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Jlalond commented Jul 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jimingham commented Jul 24, 2025

Uh oh!

labath commented Jul 25, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

labath left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Aug 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Aug 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

labath left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Jlalond commented Jul 24, 2025 •

edited

Loading

github-actions bot commented Aug 4, 2025 •

edited

Loading

github-actions bot commented Aug 4, 2025 •

edited

Loading