Change image list -r so that it actually shows the ref count and whether the image is in the shared cache.

[jimingham]

The help for the -r option to image list says:

   -r[<width>] ( --ref-count=[<width>] )
        Display the reference count if the module is still in the shared module cache.

but that's not what it actually does. It unconditionally shows the use_count for all Module shared pointers, regardless of whether they are still in the shared module cache or whether they are just in the ModuleCollection and other entities are keeping them alive. That seems like a more useful behavior, but then it is also useful to know what's in the shared cache, so I changed this to:

   -r[<width>] ( --ref-count=[<width>] )
        Display whether the module is still in the the shared module cache (Y/N), and its shared pointer use_count.

So instead of just {5} you will see {Y 5} if it is in the shared cache and {N 5} if not.

I didn't add tests for this because I'm not sure how much we want to fix shared cache behavior in the testsuite.

[llvmbot]

@llvm/pr-subscribers-lldb

Author: None (jimingham)

Changes

The help for the -r option to image list says:

   -r[<width>] ( --ref-count=[<width>] )
        Display the reference count if the module is still in the shared module cache.

but that's not what it actually does. It unconditionally shows the use_count for all Module shared pointers, regardless of whether they are still in the shared module cache or whether they are just in the ModuleCollection and other entities are keeping them alive. That seems like a more useful behavior, but then it is also useful to know what's in the shared cache, so I changed this to:

   -r[<width>] ( --ref-count=[<width>] )
        Display whether the module is still in the the shared module cache (Y/N), and its shared pointer use_count.

So instead of just {5} you will see {Y 5} if it is in the shared cache and {N 5} if not.

I didn't add tests for this because I'm not sure how much we want to fix shared cache behavior in the testsuite.

---

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

2 Files Affected:

• (modified) lldb/source/Commands/CommandObjectTarget.cpp (+6-2)
• (modified) lldb/source/Commands/Options.td (+2-2)

diff --git a/lldb/source/Commands/CommandObjectTarget.cpp b/lldb/source/Commands/CommandObjectTarget.cpp
index 45265577e8b61c..b2346c2402a81d 100644
--- a/lldb/source/Commands/CommandObjectTarget.cpp
+++ b/lldb/source/Commands/CommandObjectTarget.cpp
@@ -3376,15 +3376,19 @@ class CommandObjectTargetModulesList : public CommandObjectParsed {
 
       case 'r': {
         size_t ref_count = 0;
+        char in_shared_cache = 'Y';
+        
         ModuleSP module_sp(module->shared_from_this());
+        if (!ModuleList::ModuleIsInCache(module))
+          in_shared_cache = 'N';
         if (module_sp) {
           // Take one away to make sure we don't count our local "module_sp"
           ref_count = module_sp.use_count() - 1;
         }
         if (width)
-          strm.Printf("{%*" PRIu64 "}", width, (uint64_t)ref_count);
+          strm.Printf("{%c %*" PRIu64 "}", in_shared_cache, width, (uint64_t)ref_count);
         else
-          strm.Printf("{%" PRIu64 "}", (uint64_t)ref_count);
+          strm.Printf("{%c %" PRIu64 "}", in_shared_cache, (uint64_t)ref_count);
       } break;
 
       case 's':
diff --git a/lldb/source/Commands/Options.td b/lldb/source/Commands/Options.td
index ad4321d9a386cc..91d5eea830dedf 100644
--- a/lldb/source/Commands/Options.td
+++ b/lldb/source/Commands/Options.td
@@ -936,8 +936,8 @@ let Command = "target modules list" in {
     OptionalArg<"Width">, Desc<"Display the modification time with optional "
     "width of the module.">;
   def target_modules_list_ref_count : Option<"ref-count", "r">, Group<1>,
-    OptionalArg<"Width">, Desc<"Display the reference count if the module is "
-    "still in the shared module cache.">;
+    OptionalArg<"Width">, Desc<"Display whether the module is still in the "
+    "the shared module cache (Y/N), and its shared pointer use_count.">;
   def target_modules_list_pointer : Option<"pointer", "p">, Group<1>,
     OptionalArg<"None">, Desc<"Display the module pointer.">;
   def target_modules_list_global : Option<"global", "g">, Group<1>,

[github-actions]

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:

git-clang-format --diff e427e934f677567f8184ff900cb4cbdb8cf21a21 b95ffa364ecfc8593dc48a1986385d81cbeb05c2 -- lldb/source/Commands/CommandObjectTarget.cpp

View the diff from clang-format here.

diff --git a/lldb/source/Commands/CommandObjectTarget.cpp b/lldb/source/Commands/CommandObjectTarget.cpp
index b2346c2402..0d52fde961 100644
--- a/lldb/source/Commands/CommandObjectTarget.cpp
+++ b/lldb/source/Commands/CommandObjectTarget.cpp
@@ -3377,7 +3377,7 @@ protected:
       case 'r': {
         size_t ref_count = 0;
         char in_shared_cache = 'Y';
-        
+
         ModuleSP module_sp(module->shared_from_this());
         if (!ModuleList::ModuleIsInCache(module))
           in_shared_cache = 'N';
@@ -3386,7 +3386,8 @@ protected:
           ref_count = module_sp.use_count() - 1;
         }
         if (width)
-          strm.Printf("{%c %*" PRIu64 "}", in_shared_cache, width, (uint64_t)ref_count);
+          strm.Printf("{%c %*" PRIu64 "}", in_shared_cache, width,
+                      (uint64_t)ref_count);
         else
           strm.Printf("{%c %" PRIu64 "}", in_shared_cache, (uint64_t)ref_count);
       } break;

[medismailben]

Does it make sense to have an image multiple times in the shared cache ? If not, instead of saying {Y/N count} what about printing {location(shared cache/module collection)} and only print it ref count if it's not in the shared cache ?

[medismailben]

Does it make sense to have an image multiple times in the shared cache ? If not, instead of saying {Y/N count} what about printing {location(shared cache/module collection)} and only print it ref count if it's not in the shared cache ?

My bad, I thought you were talking about the dyld shared cache as opposed to the lldb shared module cache. LGTM then.