[Lldb-commits] [lldb] [lldb-dap] Stop the process for the threads request (PR #134456)

2025-04-17 Thread Jonas Devlieghere via lldb-commits

https://github.com/JDevlieghere closed 
https://github.com/llvm/llvm-project/pull/134456
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [clang] [clang-tools-extra] [lldb] Reland [clang] Unify `SourceLocation` and `IdentifierInfo*` pair-like data structures to `IdentifierLoc` (PR #136077)

2025-04-17 Thread Erich Keane via lldb-commits

https://github.com/erichkeane approved this pull request.


https://github.com/llvm/llvm-project/pull/136077
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [clang] [clang-tools-extra] [lldb] Reland [clang] Unify `SourceLocation` and `IdentifierInfo*` pair-like data structures to `IdentifierLoc` (PR #136077)

2025-04-17 Thread via lldb-commits

https://github.com/yronglin closed 
https://github.com/llvm/llvm-project/pull/136077
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb][Telemetry] Fix unit test compile failure with LLVM_ENABLE_TELEMETRY=0 (PR #136115)

2025-04-17 Thread Jonas Devlieghere via lldb-commits

https://github.com/JDevlieghere approved this pull request.


https://github.com/llvm/llvm-project/pull/136115
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb][nfc] Factor out code from ThreadPlanStepOut ctor (PR #136159)

2025-04-17 Thread via lldb-commits

https://github.com/jimingham approved this pull request.

LGTM.

https://github.com/llvm/llvm-project/pull/136159
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb][nfc] Add customization flags for ThreadPlanStepOut (PR #135866)

2025-04-17 Thread Felipe de Azevedo Piovezan via lldb-commits

felipepiovezan wrote:

Closing in favor of the stack:
[[lldb][nfc] Factor out code from ThreadPlanStepOut ctor 
#136159](https://github.com/llvm/llvm-project/pull/136159)
[[lldb][nfc] Split the constructor of ThreadPlanStepOut 
#136160](https://github.com/llvm/llvm-project/pull/136160)
[[lldb][nfc] Remove unused parameters in ThreadPlanStepOut ctor 
#136161](https://github.com/llvm/llvm-project/pull/136161)
[[lldb] Create ThreadPlanStepOut ctor that never skips frames 
#136163](https://github.com/llvm/llvm-project/pull/136163)

https://github.com/llvm/llvm-project/pull/135866
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb][Mach-O corefiles] Don't init Target arch to corefile (PR #136065)

2025-04-17 Thread via lldb-commits

github-actions[bot] wrote:




:warning: C/C++ code formatter, clang-format found issues in your code. 
:warning:



You can test this locally with the following command:


``bash
git-clang-format --diff HEAD~1 HEAD --extensions cpp -- 
lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp
``





View the diff from clang-format here.


``diff
diff --git a/lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp 
b/lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp
index 4f57c9ade..39584bad2 100644
--- a/lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp
+++ b/lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp
@@ -429,7 +429,7 @@ void ProcessMachCore::LoadBinariesViaExhaustiveSearch() {
   // To do an exhaustive search, we'll need to create data extractors
   // to get correctly sized/endianness fields.  If we had a main binary
   // already, we would have set the Target to that - so here we'll use
-  // the corefile's cputype/cpusubtype as the best guess. 
+  // the corefile's cputype/cpusubtype as the best guess.
   if (!GetTarget().GetArchitecture().IsValid()) {
 // The corefile's architecture is our best starting point.
 ArchSpec arch(m_core_module_sp->GetArchitecture());

``




https://github.com/llvm/llvm-project/pull/136065
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] 1042d99 - disable test on older compilers (#136186)

2025-04-17 Thread via lldb-commits

Author: Shubham Sandeep Rastogi
Date: 2025-04-17T12:33:30-07:00
New Revision: 1042d9988764e16e5f6fd984d362042b2fadd0c6

URL: 
https://github.com/llvm/llvm-project/commit/1042d9988764e16e5f6fd984d362042b2fadd0c6
DIFF: 
https://github.com/llvm/llvm-project/commit/1042d9988764e16e5f6fd984d362042b2fadd0c6.diff

LOG: disable test on older compilers (#136186)

Added: 


Modified: 

lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/invalid-vector/TestDataFormatterLibcxxInvalidVectorSimulator.py

Removed: 




diff  --git 
a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/invalid-vector/TestDataFormatterLibcxxInvalidVectorSimulator.py
 
b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/invalid-vector/TestDataFormatterLibcxxInvalidVectorSimulator.py
index 3f58018b0fbd9..1a23d9a19fe19 100644
--- 
a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/invalid-vector/TestDataFormatterLibcxxInvalidVectorSimulator.py
+++ 
b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/invalid-vector/TestDataFormatterLibcxxInvalidVectorSimulator.py
@@ -14,7 +14,7 @@ class 
LibcxxInvalidVectorDataFormatterSimulatorTestCase(TestBase):
 NO_DEBUG_INFO_TESTCASE = True
 
 
-@skipIf(compiler="clang", compiler_version=['<', '15.0.1'])
+@skipIf(compiler="clang", compiler_version=['<', '18.0'])
 def test(self):
 self.build()
 lldbutil.run_to_source_breakpoint(self, "return 0", 
lldb.SBFileSpec("main.cpp"))



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] disable test on older compilers (PR #136186)

2025-04-17 Thread Adrian Prantl via lldb-commits

https://github.com/adrian-prantl approved this pull request.


https://github.com/llvm/llvm-project/pull/136186
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb][nfc] Remove unused parameters in ThreadPlanStepOut ctor (PR #136161)

2025-04-17 Thread Felipe de Azevedo Piovezan via lldb-commits

https://github.com/felipepiovezan edited 
https://github.com/llvm/llvm-project/pull/136161
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] disable test on older compilers (PR #136186)

2025-04-17 Thread Shubham Sandeep Rastogi via lldb-commits

https://github.com/rastogishubham created 
https://github.com/llvm/llvm-project/pull/136186

None

>From fc70ee11fe2122c646ec20a61ae4b691902b1ce9 Mon Sep 17 00:00:00 2001
From: Shubham Sandeep Rastogi 
Date: Thu, 17 Apr 2025 12:31:03 -0700
Subject: [PATCH] disable test on older compilers

---
 .../TestDataFormatterLibcxxInvalidVectorSimulator.py| 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git 
a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/invalid-vector/TestDataFormatterLibcxxInvalidVectorSimulator.py
 
b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/invalid-vector/TestDataFormatterLibcxxInvalidVectorSimulator.py
index 3f58018b0fbd9..1a23d9a19fe19 100644
--- 
a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/invalid-vector/TestDataFormatterLibcxxInvalidVectorSimulator.py
+++ 
b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/invalid-vector/TestDataFormatterLibcxxInvalidVectorSimulator.py
@@ -14,7 +14,7 @@ class 
LibcxxInvalidVectorDataFormatterSimulatorTestCase(TestBase):
 NO_DEBUG_INFO_TESTCASE = True
 
 
-@skipIf(compiler="clang", compiler_version=['<', '15.0.1'])
+@skipIf(compiler="clang", compiler_version=['<', '18.0'])
 def test(self):
 self.build()
 lldbutil.run_to_source_breakpoint(self, "return 0", 
lldb.SBFileSpec("main.cpp"))

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] disable test on older compilers (PR #136186)

2025-04-17 Thread LLVM Continuous Integration via lldb-commits

llvm-ci wrote:

LLVM Buildbot has detected a new failure on builder `lldb-x86_64-debian` 
running on `lldb-x86_64-debian` while building `lldb` at step 6 "test".

Full details are available at: 
https://lab.llvm.org/buildbot/#/builders/162/builds/20449


Here is the relevant piece of the build log for the reference

```
Step 6 (test) failure: build (failure)
...
PASS: lldb-api :: lang/cpp/global_operators/TestCppGlobalOperators.py (230 of 
2829)
PASS: lldb-api :: 
functionalities/thread/thread_specific_break_plus_condition/TestThreadSpecificBpPlusCondition.py
 (231 of 2829)
PASS: lldb-api :: 
functionalities/breakpoint/breakpoint_ids/TestBreakpointIDs.py (232 of 2829)
PASS: lldb-api :: functionalities/scripted_process/TestScriptedProcess.py (233 
of 2829)
PASS: lldb-api :: 
functionalities/thread/concurrent_events/TestConcurrentManyWatchpoints.py (234 
of 2829)
PASS: lldb-api :: commands/process/attach-resume/TestAttachResume.py (235 of 
2829)
PASS: lldb-api :: functionalities/gdb_remote_client/TestTargetXMLArch.py (236 
of 2829)
PASS: lldb-api :: tools/lldb-dap/breakpoint/TestDAP_setExceptionBreakpoints.py 
(237 of 2829)
PASS: lldb-api :: lang/cpp/operators/TestCppOperators.py (238 of 2829)
UNRESOLVED: lldb-api :: tools/lldb-dap/optimized/TestDAP_optimized.py (239 of 
2829)
 TEST 'lldb-api :: 
tools/lldb-dap/optimized/TestDAP_optimized.py' FAILED 
Script:
--
/usr/bin/python3 
/home/worker/2.0.1/lldb-x86_64-debian/llvm-project/lldb/test/API/dotest.py -u 
CXXFLAGS -u CFLAGS --env 
LLVM_LIBS_DIR=/home/worker/2.0.1/lldb-x86_64-debian/build/./lib --env 
LLVM_INCLUDE_DIR=/home/worker/2.0.1/lldb-x86_64-debian/build/include --env 
LLVM_TOOLS_DIR=/home/worker/2.0.1/lldb-x86_64-debian/build/./bin --arch x86_64 
--build-dir /home/worker/2.0.1/lldb-x86_64-debian/build/lldb-test-build.noindex 
--lldb-module-cache-dir 
/home/worker/2.0.1/lldb-x86_64-debian/build/lldb-test-build.noindex/module-cache-lldb/lldb-api
 --clang-module-cache-dir 
/home/worker/2.0.1/lldb-x86_64-debian/build/lldb-test-build.noindex/module-cache-clang/lldb-api
 --executable /home/worker/2.0.1/lldb-x86_64-debian/build/./bin/lldb --compiler 
/home/worker/2.0.1/lldb-x86_64-debian/build/./bin/clang --dsymutil 
/home/worker/2.0.1/lldb-x86_64-debian/build/./bin/dsymutil --make 
/usr/bin/gmake --llvm-tools-dir 
/home/worker/2.0.1/lldb-x86_64-debian/build/./bin --lldb-obj-root 
/home/worker/2.0.1/lldb-x86_64-debian/build/tools/lldb --lldb-libs-dir 
/home/worker/2.0.1/lldb-x86_64-debian/build/./lib -t 
/home/worker/2.0.1/lldb-x86_64-debian/llvm-project/lldb/test/API/tools/lldb-dap/optimized
 -p TestDAP_optimized.py
--
Exit Code: 1

Command Output (stdout):
--
lldb version 21.0.0git (https://github.com/llvm/llvm-project.git revision 
1042d9988764e16e5f6fd984d362042b2fadd0c6)
  clang revision 1042d9988764e16e5f6fd984d362042b2fadd0c6
  llvm revision 1042d9988764e16e5f6fd984d362042b2fadd0c6
Skipping the following test categories: ['libc++', 'dsym', 'gmodules', 
'debugserver', 'objc']

--
Command Output (stderr):
--
Change dir to: 
/home/worker/2.0.1/lldb-x86_64-debian/llvm-project/lldb/test/API/tools/lldb-dap/optimized
runCmd: settings clear -all

output: 

runCmd: settings set symbols.enable-external-lookup false

output: 

runCmd: settings set target.inherit-tcc true

output: 

runCmd: settings set target.disable-aslr false

output: 

runCmd: settings set target.detach-on-error false

output: 

runCmd: settings set target.auto-apply-fixits false

```



https://github.com/llvm/llvm-project/pull/136186
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb][Minidump Parser] Implement a range data vector for minidump memory ranges (PR #136040)

2025-04-17 Thread Miro Bucko via lldb-commits


@@ -429,62 +429,64 @@ MinidumpParser::GetExceptionStreams() {
 
 std::optional
 MinidumpParser::FindMemoryRange(lldb::addr_t addr) {
-  Log *log = GetLog(LLDBLog::Modules);
+  if (m_memory_ranges.IsEmpty())
+PopulateMemoryRanges();
+
+  MemoryRangeVector::Entry *entry = 
m_memory_ranges.FindEntryThatContains(addr);
+  if (!entry)
+return std::nullopt;
+
+  return entry->data;
+}
 
+void MinidumpParser::PopulateMemoryRanges() {
+  Log *log = GetLog(LLDBLog::Modules);
   auto ExpectedMemory = GetMinidumpFile().getMemoryList();

mbucko wrote:

const

https://github.com/llvm/llvm-project/pull/136040
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb] returning command completions up to a maximum (PR #135565)

2025-04-17 Thread Ely Ronnen via lldb-commits

https://github.com/eronnen updated 
https://github.com/llvm/llvm-project/pull/135565

>From 04e503abe160bc6a9214ad1f345ef90be1e8c7e0 Mon Sep 17 00:00:00 2001
From: Ely Ronnen 
Date: Sun, 13 Apr 2025 20:46:56 +0200
Subject: [PATCH 1/3] [lldb] returning command completions up to a maximum

- Adding `max_return_elements` field to `CompletionRequest`.
- adding maximum checks to `SymbolCompleter` and `SourceFileCompleter`.
---
 lldb/include/lldb/Utility/CompletionRequest.h | 24 +++
 lldb/source/API/SBCommandInterpreter.cpp  |  2 ++
 lldb/source/Commands/CommandCompletions.cpp   | 14 ---
 3 files changed, 37 insertions(+), 3 deletions(-)

diff --git a/lldb/include/lldb/Utility/CompletionRequest.h 
b/lldb/include/lldb/Utility/CompletionRequest.h
index 865d6db576298..2d3f0a8a44a0a 100644
--- a/lldb/include/lldb/Utility/CompletionRequest.h
+++ b/lldb/include/lldb/Utility/CompletionRequest.h
@@ -115,6 +115,11 @@ class CompletionRequest {
   CompletionRequest(llvm::StringRef command_line, unsigned raw_cursor_pos,
 CompletionResult &result);
 
+  /// Sets the maximum number of completions that should be returned.
+  void SetMaxReturnElements(size_t max_return_elements) {
+m_max_return_elements = max_return_elements;
+  }
+
   /// Returns the raw user input used to create this CompletionRequest cut off
   /// at the cursor position. The cursor will be at the end of the raw line.
   llvm::StringRef GetRawLine() const {
@@ -157,6 +162,23 @@ class CompletionRequest {
 
   size_t GetCursorIndex() const { return m_cursor_index; }
 
+  size_t GetMaxReturnElements() const { return m_max_return_elements; }
+
+  /// Returns true if the maximum number of completions has been reached
+  /// already.
+  bool ShouldStopAddingResults() const {
+return m_result.GetNumberOfResults() >= m_max_return_elements;
+  }
+
+  /// Returns the maximum number of completions that need to be added
+  /// until reaching the maximum
+  size_t GetMaxNumberOfResultsToAdd() const {
+const size_t number_of_results = m_result.GetNumberOfResults();
+if (number_of_results >= m_max_return_elements)
+  return 0;
+return m_max_return_elements - number_of_results;
+  }
+
   /// Adds a possible completion string. If the completion was already
   /// suggested before, it will not be added to the list of results. A copy of
   /// the suggested completion is stored, so the given string can be free'd
@@ -231,6 +253,8 @@ class CompletionRequest {
   size_t m_cursor_index;
   /// The cursor position in the argument indexed by m_cursor_index.
   size_t m_cursor_char_position;
+  /// The maximum number of completions that should be returned.
+  size_t m_max_return_elements = SIZE_MAX;
 
   /// The result this request is supposed to fill out.
   /// We keep this object private to ensure that no backend can in any way
diff --git a/lldb/source/API/SBCommandInterpreter.cpp 
b/lldb/source/API/SBCommandInterpreter.cpp
index de22a9dd96bd8..ad3cc3c556fd4 100644
--- a/lldb/source/API/SBCommandInterpreter.cpp
+++ b/lldb/source/API/SBCommandInterpreter.cpp
@@ -266,6 +266,8 @@ int SBCommandInterpreter::HandleCompletionWithDescriptions(
   lldb_private::StringList lldb_matches, lldb_descriptions;
   CompletionResult result;
   CompletionRequest request(current_line, cursor - current_line, result);
+  if (max_return_elements >= 0)
+request.SetMaxReturnElements(max_return_elements);
   m_opaque_ptr->HandleCompletion(request);
   result.GetMatches(lldb_matches);
   result.GetDescriptions(lldb_descriptions);
diff --git a/lldb/source/Commands/CommandCompletions.cpp 
b/lldb/source/Commands/CommandCompletions.cpp
index 216aaf9abce6c..11cb94d4eda15 100644
--- a/lldb/source/Commands/CommandCompletions.cpp
+++ b/lldb/source/Commands/CommandCompletions.cpp
@@ -91,7 +91,7 @@ bool CommandCompletions::InvokeCommonCompletionCallbacks(
nullptr} // This one has to be last in the list.
   };
 
-  for (int i = 0;; i++) {
+  for (int i = 0; !request.ShouldStopAddingResults(); i++) {
 if (common_completions[i].type == lldb::eTerminatorCompletion)
   break;
 else if ((common_completions[i].type & completion_mask) ==
@@ -167,7 +167,9 @@ class SourceFileCompleter : public Completer {
 m_matching_files.AppendIfUnique(context.comp_unit->GetPrimaryFile());
   }
 }
-return Searcher::eCallbackReturnContinue;
+return m_matching_files.GetSize() >= m_request.GetMaxNumberOfResultsToAdd()
+   ? Searcher::eCallbackReturnStop
+   : Searcher::eCallbackReturnContinue;
   }
 
   void DoCompletion(SearchFilter *filter) override {
@@ -230,6 +232,10 @@ class SymbolCompleter : public Completer {
 
   // Now add the functions & symbols to the list - only add if unique:
   for (const SymbolContext &sc : sc_list) {
+if (m_match_set.size() >= m_request.GetMaxNumberOfResultsToAdd()) {
+  break;
+}
+
 ConstString func_name = sc.GetFunctionName(Mang

[Lldb-commits] [lldb] [lldb] returning command completions up to a maximum (PR #135565)

2025-04-17 Thread Ely Ronnen via lldb-commits


@@ -230,6 +232,9 @@ class SymbolCompleter : public Completer {
 
   // Now add the functions & symbols to the list - only add if unique:
   for (const SymbolContext &sc : sc_list) {
+if (m_match_set.size() >= m_request.GetMaxNumberOfResultsToAdd())

eronnen wrote:

In this case at the last iteration `m_match_set.size() == 254`  before adding 
the last completion, so it will still be added...
if `m_match_set.size() > m_request.GetMaxNumberOfResultsToAdd()` it means that 
the number of completions added at the end will be bigger than the max number

https://github.com/llvm/llvm-project/pull/135565
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb] Add symbol/table count into statistics (PR #136226)

2025-04-17 Thread David Peixotto via lldb-commits


@@ -168,23 +170,34 @@ def test_default_no_run(self):
 "totalSymbolTableIndexTime",
 "totalSymbolTablesLoadedFromCache",
 "totalSymbolTablesSavedToCache",
+"totalSymbolsLoaded",
 "totalDebugInfoByteSize",
 "totalDebugInfoIndexTime",
 "totalDebugInfoIndexLoadedFromCache",
 "totalDebugInfoIndexSavedToCache",
 "totalDebugInfoParseTime",
 ]
 self.verify_keys(debug_stats, '"debug_stats"', debug_stat_keys, None)
-stats = debug_stats["targets"][0]
-keys_exist = [
+
+# Verify target stats keys.
+target_stats = debug_stats["targets"][0]
+target_stat_keys_exist = [
 "expressionEvaluation",
 "frameVariable",
 "moduleIdentifiers",
 "targetCreateTime",
 ]
-keys_missing = ["firstStopTime", "launchOrAttachTime"]
-self.verify_keys(stats, '"stats"', keys_exist, keys_missing)
-self.assertGreater(stats["targetCreateTime"], 0.0)
+target_stat_keys_missing = ["firstStopTime", "launchOrAttachTime"]
+self.verify_keys(target_stats, '"target_stats"', 
target_stat_keys_exist, target_stat_keys_missing)
+self.assertGreater(target_stats["targetCreateTime"], 0.0)
+
+# Verify module stats keys.
+for module_stats in debug_stats["modules"]:
+module_stat_keys_exist = [
+"symbolsLoaded",
+]
+self.verify_keys(module_stats, '"module_stats"', 
module_stat_keys_exist, None)

dmpots wrote:

Is there anything we can check about the expected value of these new stats? 
Like do we expect them to be > 0?

https://github.com/llvm/llvm-project/pull/136226
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb] Avoid force loading symbols in statistics collection (PR #136236)

2025-04-17 Thread via lldb-commits

https://github.com/royitaqi updated 
https://github.com/llvm/llvm-project/pull/136236

>From 85cb8cca4fe41cf4080b3dbf7ce4f98c4b15a3c7 Mon Sep 17 00:00:00 2001
From: Roy Shi 
Date: Thu, 17 Apr 2025 15:03:00 -0700
Subject: [PATCH 1/2] [lldb] Do not parse symtab during statistics dump

Summary:

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:

Differential Revision: https://phabricator.intern.facebook.com/D73218490
---
 lldb/include/lldb/Symbol/ObjectFile.h | 2 +-
 lldb/include/lldb/Symbol/SymbolFile.h | 4 ++--
 lldb/include/lldb/Symbol/SymbolFileOnDemand.h | 2 +-
 lldb/source/Core/Module.cpp   | 2 +-
 lldb/source/Symbol/ObjectFile.cpp | 4 ++--
 lldb/source/Symbol/SymbolFile.cpp | 4 ++--
 6 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/lldb/include/lldb/Symbol/ObjectFile.h 
b/lldb/include/lldb/Symbol/ObjectFile.h
index cfcca04a76de8..7fca6383fa9f3 100644
--- a/lldb/include/lldb/Symbol/ObjectFile.h
+++ b/lldb/include/lldb/Symbol/ObjectFile.h
@@ -319,7 +319,7 @@ class ObjectFile : public 
std::enable_shared_from_this,
   ///
   /// \return
   /// The symbol table for this object file.
-  Symtab *GetSymtab();
+  Symtab *GetSymtab(bool can_create = true);
 
   /// Parse the symbol table into the provides symbol table object.
   ///
diff --git a/lldb/include/lldb/Symbol/SymbolFile.h 
b/lldb/include/lldb/Symbol/SymbolFile.h
index f35d3ee9f22ae..df2f263b18e17 100644
--- a/lldb/include/lldb/Symbol/SymbolFile.h
+++ b/lldb/include/lldb/Symbol/SymbolFile.h
@@ -144,7 +144,7 @@ class SymbolFile : public PluginInterface {
   virtual uint32_t GetNumCompileUnits() = 0;
   virtual lldb::CompUnitSP GetCompileUnitAtIndex(uint32_t idx) = 0;
 
-  virtual Symtab *GetSymtab() = 0;
+  virtual Symtab *GetSymtab(bool can_create = true) = 0;
 
   virtual lldb::LanguageType ParseLanguage(CompileUnit &comp_unit) = 0;
   /// Return the Xcode SDK comp_unit was compiled against.
@@ -533,7 +533,7 @@ class SymbolFileCommon : public SymbolFile {
 return m_abilities;
   }
 
-  Symtab *GetSymtab() override;
+  Symtab *GetSymtab(bool can_create = true) override;
 
   ObjectFile *GetObjectFile() override { return m_objfile_sp.get(); }
   const ObjectFile *GetObjectFile() const override {
diff --git a/lldb/include/lldb/Symbol/SymbolFileOnDemand.h 
b/lldb/include/lldb/Symbol/SymbolFileOnDemand.h
index 7a366bfabec86..ce9fc8626ac34 100644
--- a/lldb/include/lldb/Symbol/SymbolFileOnDemand.h
+++ b/lldb/include/lldb/Symbol/SymbolFileOnDemand.h
@@ -186,7 +186,7 @@ class SymbolFileOnDemand : public lldb_private::SymbolFile {
 
   uint32_t GetAbilities() override;
 
-  Symtab *GetSymtab() override { return m_sym_file_impl->GetSymtab(); }
+  Symtab *GetSymtab(bool can_create = true) override { return 
m_sym_file_impl->GetSymtab(can_create); }
 
   ObjectFile *GetObjectFile() override {
 return m_sym_file_impl->GetObjectFile();
diff --git a/lldb/source/Core/Module.cpp b/lldb/source/Core/Module.cpp
index ad7046e596278..625c14e4a2153 100644
--- a/lldb/source/Core/Module.cpp
+++ b/lldb/source/Core/Module.cpp
@@ -997,7 +997,7 @@ SymbolFile *Module::GetSymbolFile(bool can_create, Stream 
*feedback_strm) {
 
 Symtab *Module::GetSymtab(bool can_create) {
   if (SymbolFile *symbols = GetSymbolFile(can_create))
-return symbols->GetSymtab();
+return symbols->GetSymtab(can_create);
   return nullptr;
 }
 
diff --git a/lldb/source/Symbol/ObjectFile.cpp 
b/lldb/source/Symbol/ObjectFile.cpp
index 2f2c59d6af620..292c00b9ac166 100644
--- a/lldb/source/Symbol/ObjectFile.cpp
+++ b/lldb/source/Symbol/ObjectFile.cpp
@@ -735,9 +735,9 @@ void llvm::format_provider::format(
 }
 
 
-Symtab *ObjectFile::GetSymtab() {
+Symtab *ObjectFile::GetSymtab(bool can_create) {
   ModuleSP module_sp(GetModule());
-  if (module_sp) {
+  if (module_sp && can_create) {
 // We can't take the module lock in ObjectFile::GetSymtab() or we can
 // deadlock in DWARF indexing when any file asks for the symbol table from
 // an object file. This currently happens in the preloading of symbols in
diff --git a/lldb/source/Symbol/SymbolFile.cpp 
b/lldb/source/Symbol/SymbolFile.cpp
index 94e32b55572dd..870d778dca740 100644
--- a/lldb/source/Symbol/SymbolFile.cpp
+++ b/lldb/source/Symbol/SymbolFile.cpp
@@ -152,10 +152,10 @@ void SymbolFile::AssertModuleLock() {
 
 SymbolFile::RegisterInfoResolver::~RegisterInfoResolver() = default;
 
-Symtab *SymbolFileCommon::GetSymtab() {
+Symtab *SymbolFileCommon::GetSymtab(bool can_create) {
   std::lock_guard guard(GetModuleMutex());
   // Fetch the symtab from the main object file.
-  auto *symtab = GetMainObjectFile()->GetSymtab();
+  auto *symtab = GetMainObjectFile()->GetSymtab(can_create);
   if (m_symtab != symtab) {
 m_symtab = symtab;
 

>From 1fccd09dde2d1f8da6f8253516c5c908049eb270 Mon Sep 17 00:00:00 2001
From: Roy Shi 
Date: Thu, 17 Apr 2025 21:45:48 -0700
Subject: [PATCH 2/2] Fix compilation of a unit test

---
 lldb/unittests/Symbol/LineTableTest.cpp

[Lldb-commits] [lldb] [lldb][Mach-O corefiles] Don't init Target arch to corefile (PR #136065)

2025-04-17 Thread Jason Molenda via lldb-commits

https://github.com/jasonmolenda updated 
https://github.com/llvm/llvm-project/pull/136065

>From 41274f7632d13ab7ec83a420c6ad6258e6fe8c36 Mon Sep 17 00:00:00 2001
From: Jason Molenda 
Date: Wed, 16 Apr 2025 17:18:02 -0700
Subject: [PATCH 1/3] [lldb][Mach-O corefiles] Don't init Target arch to
 corefile

This patch is making three changes, when loading a Mach-O
corefile:

1. At the start of `DoLoadCore`, if a binary was provided in addition
to the corefile, initialize the Target's ArchSpec.

2. Before ProcessMachCore does its "exhaustive search" fallback,
looking through the corefile contents for a userland dyld or mach
kernel, we must make sure the Target has an ArchSpec, or methods
that check the address word size, or initialize a DataExtractor
based on the Target arch will not succeed.

3. Add logging when setting the Target's arch listing exactly what
that setting was based on -- the corefile itself, or the main binary.

Jonas landed a change last August (started with a patch from me)
which removed the Target ArchSpec initialization at the start of
DoLoadCore, in a scenario where the corefile had arch armv7 and
the main binary had arch armv7em (Cortex-M), and there was python
code in the main binary's dSYM which sets the operating system threads
provider based on the Target arch.  It did different things for
armv7 or armv7em, and so it would fail.

Jonas' patch removed any ArchSpec setting at the start of DoLoadCore,
so we wouldn't have an incorrect arch value, but that broke the
exhaustive search for kernel binaries, because we didn't have an
address word size or endianness.

This patch should navigate the needs of both use cases.

I spent a good bit of time trying to construct a test to capture all
of these requirements -- but it turns out to be a good bit difficult,
encompassing both a genuine kernel corefiles and a microcontroller
firmware corefiles.

rdar://146821929
---
 .../Process/mach-core/ProcessMachCore.cpp | 58 ++-
 1 file changed, 56 insertions(+), 2 deletions(-)

diff --git a/lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp 
b/lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp
index 281f3a0db8f69..17362b2d2855d 100644
--- a/lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp
+++ b/lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp
@@ -426,6 +426,32 @@ void ProcessMachCore::LoadBinariesViaExhaustiveSearch() {
   std::vector dylds_found;
   std::vector kernels_found;
 
+  // To do an exhaustive search, we'll need to create data extractors
+  // to get correctly sized/endianness fields, and if the Target still
+  // doesn't have an architecture set, we need to seed it from either
+  // the main binary (if we were given one) or the corefile cputype/cpusubtype.
+  if (!GetTarget().GetArchitecture().IsValid()) {
+Log *log(GetLog(LLDBLog::DynamicLoader | LLDBLog::Target));
+ModuleSP exe_module_sp = GetTarget().GetExecutableModule();
+if (exe_module_sp && exe_module_sp->GetArchitecture().IsValid()) {
+  LLDB_LOGF(log,
+"ProcessMachCore::%s: Was given binary + corefile, setting "
+"target ArchSpec to binary to start",
+__FUNCTION__);
+  GetTarget().SetArchitecture(exe_module_sp->GetArchitecture());
+} else {
+  // The corefile's architecture is our best starting point.
+  ArchSpec arch(m_core_module_sp->GetArchitecture());
+  if (arch.IsValid()) {
+LLDB_LOGF(log,
+  "ProcessMachCore::%s: Setting target ArchSpec based on "
+  "corefile mach-o cputype/cpusubtype",
+  __FUNCTION__);
+GetTarget().SetArchitecture(arch);
+  }
+}
+  }
+
   const size_t num_core_aranges = m_core_aranges.GetSize();
   for (size_t i = 0; i < num_core_aranges; ++i) {
 const VMRangeToFileOffset::Entry *entry = 
m_core_aranges.GetEntryAtIndex(i);
@@ -569,6 +595,7 @@ Status ProcessMachCore::DoLoadCore() {
 error = Status::FromErrorString("invalid core module");
 return error;
   }
+  Log *log(GetLog(LLDBLog::DynamicLoader | LLDBLog::Target));
 
   ObjectFile *core_objfile = m_core_module_sp->GetObjectFile();
   if (core_objfile == nullptr) {
@@ -578,20 +605,47 @@ Status ProcessMachCore::DoLoadCore() {
 
   SetCanJIT(false);
 
+  // If we have an executable binary in the Target already,
+  // use that to set the Target's ArchSpec.
+  //
+  // Don't initialize the ArchSpec based on the corefile's cputype/cpusubtype
+  // here, the corefile creator may not know the correct subtype of the code
+  // that is executing, initialize the Target to that, and if the
+  // main binary has Python code which initializes based on the Target arch,
+  // get the wrong subtype value.
+  ModuleSP exe_module_sp = GetTarget().GetExecutableModule();
+  if (exe_module_sp && exe_module_sp->GetArchitecture().IsValid()) {
+LLDB_LOGF(log,
+  "ProcessMachCore::%s: Was given binary + corefile, setting "
+  "target A

[Lldb-commits] [lldb] [lldb] Create ThreadPlanStepOut ctor that never skips frames (PR #136163)

2025-04-17 Thread Felipe de Azevedo Piovezan via lldb-commits

https://github.com/felipepiovezan updated 
https://github.com/llvm/llvm-project/pull/136163

>From d1f15f7a23b75d7b68989ac84b1c4b5991fb120d Mon Sep 17 00:00:00 2001
From: Felipe de Azevedo Piovezan 
Date: Thu, 17 Apr 2025 09:36:22 -0700
Subject: [PATCH 1/2] [lldb] Create ThreadPlanStepOut ctor that never skips
 frames

The function QueueThreadPlanForStepOutNoShouldStop has the semantics of
"go this parent frame"; ThreadPlanStepOut needs to respect that, not
skipping over any frames it finds uninteresting. This commit creates a
constructor that respects such instruction.
---
 lldb/include/lldb/Target/ThreadPlanStepOut.h |  9 
 lldb/source/Target/Thread.cpp|  5 ++---
 lldb/source/Target/ThreadPlanStepOut.cpp | 22 
 3 files changed, 33 insertions(+), 3 deletions(-)

diff --git a/lldb/include/lldb/Target/ThreadPlanStepOut.h 
b/lldb/include/lldb/Target/ThreadPlanStepOut.h
index e414a6e0a2d49..9a62f6102d368 100644
--- a/lldb/include/lldb/Target/ThreadPlanStepOut.h
+++ b/lldb/include/lldb/Target/ThreadPlanStepOut.h
@@ -17,6 +17,9 @@ namespace lldb_private {
 
 class ThreadPlanStepOut : public ThreadPlan, public ThreadPlanShouldStopHere {
 public:
+  /// Creates a thread plan to step out from frame_idx, skipping parent frames
+  /// that artificial and hidden frames. Also skips frames without debug info
+  /// based on step_out_avoids_code_without_debug_info.
   ThreadPlanStepOut(Thread &thread, SymbolContext *addr_context,
 bool first_insn, bool stop_others, Vote report_stop_vote,
 Vote report_run_vote, uint32_t frame_idx,
@@ -24,6 +27,12 @@ class ThreadPlanStepOut : public ThreadPlan, public 
ThreadPlanShouldStopHere {
 bool continue_to_next_branch = false,
 bool gather_return_value = true);
 
+  /// Creates a thread plan to step out from frame_idx to frame_idx + 1.
+  ThreadPlanStepOut(Thread &thread, bool stop_others, Vote report_stop_vote,
+Vote report_run_vote, uint32_t frame_idx,
+bool continue_to_next_branch = false,
+bool gather_return_value = true);
+
   ~ThreadPlanStepOut() override;
 
   void GetDescription(Stream *s, lldb::DescriptionLevel level) override;
diff --git a/lldb/source/Target/Thread.cpp b/lldb/source/Target/Thread.cpp
index accc4708c24e1..b0e0f1e67c060 100644
--- a/lldb/source/Target/Thread.cpp
+++ b/lldb/source/Target/Thread.cpp
@@ -1360,9 +1360,8 @@ ThreadPlanSP 
Thread::QueueThreadPlanForStepOutNoShouldStop(
   const bool calculate_return_value =
   false; // No need to calculate the return value here.
   ThreadPlanSP thread_plan_sp(new ThreadPlanStepOut(
-  *this, addr_context, first_insn, stop_other_threads, report_stop_vote,
-  report_run_vote, frame_idx, eLazyBoolNo, continue_to_next_branch,
-  calculate_return_value));
+  *this, stop_other_threads, report_stop_vote, report_run_vote, frame_idx,
+  continue_to_next_branch, calculate_return_value));
 
   ThreadPlanStepOut *new_plan =
   static_cast(thread_plan_sp.get());
diff --git a/lldb/source/Target/ThreadPlanStepOut.cpp 
b/lldb/source/Target/ThreadPlanStepOut.cpp
index f2606403016a6..0628451a5abf9 100644
--- a/lldb/source/Target/ThreadPlanStepOut.cpp
+++ b/lldb/source/Target/ThreadPlanStepOut.cpp
@@ -83,6 +83,28 @@ ThreadPlanStepOut::ThreadPlanStepOut(
  continue_to_next_branch);
 }
 
+ThreadPlanStepOut::ThreadPlanStepOut(Thread &thread, bool stop_others,
+ Vote report_stop_vote,
+ Vote report_run_vote, uint32_t frame_idx,
+ bool continue_to_next_branch,
+ bool gather_return_value)
+: ThreadPlan(ThreadPlan::eKindStepOut, "Step out", thread, 
report_stop_vote,
+ report_run_vote),
+  ThreadPlanShouldStopHere(this), m_return_bp_id(LLDB_INVALID_BREAK_ID),
+  m_return_addr(LLDB_INVALID_ADDRESS), m_stop_others(stop_others),
+  m_immediate_step_from_function(nullptr),
+  m_calculate_return_value(gather_return_value) {
+  SetFlagsToDefault();
+  m_step_from_insn = thread.GetRegisterContext()->GetPC(0);
+
+  StackFrameSP return_frame_sp = thread.GetStackFrameAtIndex(frame_idx + 1);
+  StackFrameSP immediate_return_from_sp =
+  thread.GetStackFrameAtIndex(frame_idx);
+
+  SetupReturnAddress(return_frame_sp, immediate_return_from_sp, frame_idx,
+ continue_to_next_branch);
+}
+
 void ThreadPlanStepOut::SetupReturnAddress(
 StackFrameSP return_frame_sp, StackFrameSP immediate_return_from_sp,
 uint32_t frame_idx, bool continue_to_next_branch) {

>From b1665da9637d16d91ba7a08ff9ad884c0b87c490 Mon Sep 17 00:00:00 2001
From: Felipe de Azevedo Piovezan 
Date: Thu, 17 Apr 2025 13:48:08 -0700
Subject: [PATCH 2/2] fixup! Update comment

---
 lldb/include/lldb/Target/ThreadPlanStepOut.h | 4 ++--
 1 file changed,

[Lldb-commits] [lldb] [lldb] Create ThreadPlanStepOut ctor that never skips frames (PR #136163)

2025-04-17 Thread Felipe de Azevedo Piovezan via lldb-commits

https://github.com/felipepiovezan edited 
https://github.com/llvm/llvm-project/pull/136163
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb] Create ThreadPlanStepOut ctor that never skips frames (PR #136163)

2025-04-17 Thread Felipe de Azevedo Piovezan via lldb-commits

felipepiovezan wrote:

Addressed review comments, rebased on top of main

https://github.com/llvm/llvm-project/pull/136163
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb] returning command completions up to a maximum (PR #135565)

2025-04-17 Thread Jacob Lalonde via lldb-commits


@@ -230,6 +232,9 @@ class SymbolCompleter : public Completer {
 
   // Now add the functions & symbols to the list - only add if unique:
   for (const SymbolContext &sc : sc_list) {
+if (m_match_set.size() >= m_request.GetMaxNumberOfResultsToAdd())

Jlalond wrote:

`>`? If I have 255 completions, and the max number of completions is 255, 
shouldn't that be valid?

https://github.com/llvm/llvm-project/pull/135565
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb][nfc] Remove unused parameters in ThreadPlanStepOut ctor (PR #136161)

2025-04-17 Thread Felipe de Azevedo Piovezan via lldb-commits

https://github.com/felipepiovezan closed 
https://github.com/llvm/llvm-project/pull/136161
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb] Do not create/parse symtab during `DebuggerStats::ReportStatistics()` (PR #136236)

2025-04-17 Thread via lldb-commits

llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: None (royitaqi)


Changes

# The change

Currently, `DebuggerStats::ReportStatistics()` calls `Module::GetSymtab(/* 
can_create */ false)`, but then the latter calls `SymbolFile::GetSymtab()`. 
This will load symbols if haven't yet.

The problem is that `DebuggerStats::ReportStatistics` should be read-only. This 
is especially important because it reports stats for symtab parsing/indexing 
time, which could be affected by the reporting itself if it's not read-only.

This patch fixes this problem by adding an optional parameter 
`SymbolFile::GetSymtab(can_create = true)` and receive the `false` value passed 
down from `Module::GetSymtab()` when the call was initiated from 
`DebuggerStats::ReportStatistics()`.

# Tests

**Manually tested** with a helloworld program, that:
1. Without this patch, a breakpoint in `ObjectFileMachO::ParseSymtab()` is hit 
when we do `lldb a.out` then `statistics dump`.
2. With this patch, said breakpoint is not hit.

**Unit test**: Still looking for a good unit test file to add a new test. Will 
update here.

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


6 Files Affected:

- (modified) lldb/include/lldb/Symbol/ObjectFile.h (+1-1) 
- (modified) lldb/include/lldb/Symbol/SymbolFile.h (+2-2) 
- (modified) lldb/include/lldb/Symbol/SymbolFileOnDemand.h (+1-1) 
- (modified) lldb/source/Core/Module.cpp (+1-1) 
- (modified) lldb/source/Symbol/ObjectFile.cpp (+2-2) 
- (modified) lldb/source/Symbol/SymbolFile.cpp (+2-2) 


``diff
diff --git a/lldb/include/lldb/Symbol/ObjectFile.h 
b/lldb/include/lldb/Symbol/ObjectFile.h
index cfcca04a76de8..7fca6383fa9f3 100644
--- a/lldb/include/lldb/Symbol/ObjectFile.h
+++ b/lldb/include/lldb/Symbol/ObjectFile.h
@@ -319,7 +319,7 @@ class ObjectFile : public 
std::enable_shared_from_this,
   ///
   /// \return
   /// The symbol table for this object file.
-  Symtab *GetSymtab();
+  Symtab *GetSymtab(bool can_create = true);
 
   /// Parse the symbol table into the provides symbol table object.
   ///
diff --git a/lldb/include/lldb/Symbol/SymbolFile.h 
b/lldb/include/lldb/Symbol/SymbolFile.h
index f35d3ee9f22ae..df2f263b18e17 100644
--- a/lldb/include/lldb/Symbol/SymbolFile.h
+++ b/lldb/include/lldb/Symbol/SymbolFile.h
@@ -144,7 +144,7 @@ class SymbolFile : public PluginInterface {
   virtual uint32_t GetNumCompileUnits() = 0;
   virtual lldb::CompUnitSP GetCompileUnitAtIndex(uint32_t idx) = 0;
 
-  virtual Symtab *GetSymtab() = 0;
+  virtual Symtab *GetSymtab(bool can_create = true) = 0;
 
   virtual lldb::LanguageType ParseLanguage(CompileUnit &comp_unit) = 0;
   /// Return the Xcode SDK comp_unit was compiled against.
@@ -533,7 +533,7 @@ class SymbolFileCommon : public SymbolFile {
 return m_abilities;
   }
 
-  Symtab *GetSymtab() override;
+  Symtab *GetSymtab(bool can_create = true) override;
 
   ObjectFile *GetObjectFile() override { return m_objfile_sp.get(); }
   const ObjectFile *GetObjectFile() const override {
diff --git a/lldb/include/lldb/Symbol/SymbolFileOnDemand.h 
b/lldb/include/lldb/Symbol/SymbolFileOnDemand.h
index 7a366bfabec86..ce9fc8626ac34 100644
--- a/lldb/include/lldb/Symbol/SymbolFileOnDemand.h
+++ b/lldb/include/lldb/Symbol/SymbolFileOnDemand.h
@@ -186,7 +186,7 @@ class SymbolFileOnDemand : public lldb_private::SymbolFile {
 
   uint32_t GetAbilities() override;
 
-  Symtab *GetSymtab() override { return m_sym_file_impl->GetSymtab(); }
+  Symtab *GetSymtab(bool can_create = true) override { return 
m_sym_file_impl->GetSymtab(can_create); }
 
   ObjectFile *GetObjectFile() override {
 return m_sym_file_impl->GetObjectFile();
diff --git a/lldb/source/Core/Module.cpp b/lldb/source/Core/Module.cpp
index ad7046e596278..625c14e4a2153 100644
--- a/lldb/source/Core/Module.cpp
+++ b/lldb/source/Core/Module.cpp
@@ -997,7 +997,7 @@ SymbolFile *Module::GetSymbolFile(bool can_create, Stream 
*feedback_strm) {
 
 Symtab *Module::GetSymtab(bool can_create) {
   if (SymbolFile *symbols = GetSymbolFile(can_create))
-return symbols->GetSymtab();
+return symbols->GetSymtab(can_create);
   return nullptr;
 }
 
diff --git a/lldb/source/Symbol/ObjectFile.cpp 
b/lldb/source/Symbol/ObjectFile.cpp
index 2f2c59d6af620..292c00b9ac166 100644
--- a/lldb/source/Symbol/ObjectFile.cpp
+++ b/lldb/source/Symbol/ObjectFile.cpp
@@ -735,9 +735,9 @@ void llvm::format_provider::format(
 }
 
 
-Symtab *ObjectFile::GetSymtab() {
+Symtab *ObjectFile::GetSymtab(bool can_create) {
   ModuleSP module_sp(GetModule());
-  if (module_sp) {
+  if (module_sp && can_create) {
 // We can't take the module lock in ObjectFile::GetSymtab() or we can
 // deadlock in DWARF indexing when any file asks for the symbol table from
 // an object file. This currently happens in the preloading of symbols in
diff --git a/lldb/source/Symbol/SymbolFile.cpp 
b/lldb/source/Symbol/SymbolFile.cpp
index 94e32b55572dd..870d778dca740 100644
--- a/lldb/source/Symbol/SymbolFile.cpp
+++ b

[Lldb-commits] [lldb] [lldb] Add symbol/table count into statistics (PR #136226)

2025-04-17 Thread David Peixotto via lldb-commits

https://github.com/dmpots approved this pull request.

LGTM

https://github.com/llvm/llvm-project/pull/136226
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb] Try to fix TestExprDiagnostics test (PR #136269)

2025-04-17 Thread Timm Baeder via lldb-commits

https://github.com/tbaederr created 
https://github.com/llvm/llvm-project/pull/136269

None

>From 3af741ab0f57822e46be79b7d426d8b225755dbe Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Timm=20B=C3=A4der?= 
Date: Fri, 18 Apr 2025 08:43:37 +0200
Subject: [PATCH] [lldb] Try to fix TestExprDiagnostics test

---
 .../expression/diagnostics/TestExprDiagnostics.py | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git 
a/lldb/test/API/commands/expression/diagnostics/TestExprDiagnostics.py 
b/lldb/test/API/commands/expression/diagnostics/TestExprDiagnostics.py
index b476a807c6dc0..428dbd0a85234 100644
--- a/lldb/test/API/commands/expression/diagnostics/TestExprDiagnostics.py
+++ b/lldb/test/API/commands/expression/diagnostics/TestExprDiagnostics.py
@@ -33,7 +33,7 @@ def test_source_and_caret_printing(self):
 self.assertIn(
 """
 1 | unknown_identifier
-  | ^
+  | ^~
 """,
 value.GetError().GetCString(),
 )
@@ -45,7 +45,7 @@ def test_source_and_caret_printing(self):
 self.assertIn(
 """
 1 | 1 + unknown_identifier
-  | ^
+  | ^~
 """,
 value.GetError().GetCString(),
 )
@@ -57,7 +57,7 @@ def test_source_and_caret_printing(self):
 self.assertIn(
 """
 2 | foobar +=1;
-  | ^
+  | ^~
 """,
 value.GetError().GetCString(),
 )
@@ -74,7 +74,7 @@ def test_source_and_caret_printing(self):
 self.assertIn(
 """
 1 | void foo(unknown_type x) {}
-  |  ^
+  |  ^~~~
 """,
 value.GetError().GetCString(),
 )

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb] Try to fix TestExprDiagnostics test (PR #136269)

2025-04-17 Thread via lldb-commits

llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: Timm Baeder (tbaederr)


Changes



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


1 Files Affected:

- (modified) 
lldb/test/API/commands/expression/diagnostics/TestExprDiagnostics.py (+4-4) 


``diff
diff --git 
a/lldb/test/API/commands/expression/diagnostics/TestExprDiagnostics.py 
b/lldb/test/API/commands/expression/diagnostics/TestExprDiagnostics.py
index b476a807c6dc0..428dbd0a85234 100644
--- a/lldb/test/API/commands/expression/diagnostics/TestExprDiagnostics.py
+++ b/lldb/test/API/commands/expression/diagnostics/TestExprDiagnostics.py
@@ -33,7 +33,7 @@ def test_source_and_caret_printing(self):
 self.assertIn(
 """
 1 | unknown_identifier
-  | ^
+  | ^~
 """,
 value.GetError().GetCString(),
 )
@@ -45,7 +45,7 @@ def test_source_and_caret_printing(self):
 self.assertIn(
 """
 1 | 1 + unknown_identifier
-  | ^
+  | ^~
 """,
 value.GetError().GetCString(),
 )
@@ -57,7 +57,7 @@ def test_source_and_caret_printing(self):
 self.assertIn(
 """
 2 | foobar +=1;
-  | ^
+  | ^~
 """,
 value.GetError().GetCString(),
 )
@@ -74,7 +74,7 @@ def test_source_and_caret_printing(self):
 self.assertIn(
 """
 1 | void foo(unknown_type x) {}
-  |  ^
+  |  ^~~~
 """,
 value.GetError().GetCString(),
 )

``




https://github.com/llvm/llvm-project/pull/136269
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb-dap] Remove an incorrect assumption on reverse requests. (PR #136210)

2025-04-17 Thread John Harrison via lldb-commits

https://github.com/ashgti created 
https://github.com/llvm/llvm-project/pull/136210

Reverse requests do have a 'seq' set still from VSCode. I incorrectly 
interpreted 
https://github.com/microsoft/vscode/blob/dede7bb4b7e9c9ec69155a243bb84037a40588fe/src/vs/workbench/contrib/debug/common/abstractDebugAdapter.ts#L65
 to mean they have a 'seq' of '0', however the 'seq' is set in 'internalSend' 
here 
https://github.com/microsoft/vscode/blob/dede7bb4b7e9c9ec69155a243bb84037a40588fe/src/vs/workbench/contrib/debug/common/abstractDebugAdapter.ts#L178.

Removing the check that 'seq=0' on reverse requests and updating the 
dap_server.py impl to also set the seq.

>From 80e07264a2695796ebcd7b63ed882ba263de2c26 Mon Sep 17 00:00:00 2001
From: John Harrison 
Date: Thu, 17 Apr 2025 14:40:13 -0700
Subject: [PATCH] [lldb-dap] Remove an incorrect assumption on reverse
 requests.

Reverse requests do have a 'seq' set still from VSCode. I incorrectly 
interpreted 
https://github.com/microsoft/vscode/blob/dede7bb4b7e9c9ec69155a243bb84037a40588fe/src/vs/workbench/contrib/debug/common/abstractDebugAdapter.ts#L65
 to mean they have a 'seq' of '0', however the 'seq' is set in 'internalSend' 
here 
https://github.com/microsoft/vscode/blob/dede7bb4b7e9c9ec69155a243bb84037a40588fe/src/vs/workbench/contrib/debug/common/abstractDebugAdapter.ts#L178.

Removing the check that 'seq=0' on reverse requests and updating the 
dap_server.py impl to also set the seq.
---
 .../Python/lldbsuite/test/tools/lldb-dap/dap_server.py   | 4 
 lldb/tools/lldb-dap/Protocol/ProtocolBase.cpp| 5 -
 2 files changed, 9 deletions(-)

diff --git a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py 
b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py
index 61d7fa94479b8..a9915ba2f6de6 100644
--- a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py
+++ b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py
@@ -343,25 +343,21 @@ def send_recv(self, command):
 self.send_packet(
 {
 "type": "response",
-"seq": 0,
 "request_seq": response_or_request["seq"],
 "success": True,
 "command": "runInTerminal",
 "body": {},
 },
-set_sequence=False,
 )
 elif response_or_request["command"] == "startDebugging":
 self.send_packet(
 {
 "type": "response",
-"seq": 0,
 "request_seq": response_or_request["seq"],
 "success": True,
 "command": "startDebugging",
 "body": {},
 },
-set_sequence=False,
 )
 else:
 desc = 'unknown reverse request "%s"' % (
diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolBase.cpp 
b/lldb/tools/lldb-dap/Protocol/ProtocolBase.cpp
index bfd68448fb483..bc4fee4aa8b8d 100644
--- a/lldb/tools/lldb-dap/Protocol/ProtocolBase.cpp
+++ b/lldb/tools/lldb-dap/Protocol/ProtocolBase.cpp
@@ -163,11 +163,6 @@ bool fromJSON(json::Value const &Params, Response &R, 
json::Path P) {
 return false;
   }
 
-  if (seq != 0) {
-P.field("seq").report("expected to be '0'");
-return false;
-  }
-
   if (R.command.empty()) {
 P.field("command").report("expected to not be ''");
 return false;

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb] Avoid force loading symbols in statistics collection (PR #136236)

2025-04-17 Thread via lldb-commits


@@ -749,10 +749,20 @@ TEST_F(SymtabTest, TestSymtabCreatedOnDemand) {
   ASSERT_THAT_EXPECTED(ExpectedFile, llvm::Succeeded());
   auto module_sp = std::make_shared(ExpectedFile->moduleSpec());
 
-  // The symbol table should not be loaded by default.
+  // The symbol file should not be created by default.
   Symtab *module_symtab = module_sp->GetSymtab(/*can_create=*/false);
   ASSERT_EQ(module_symtab, nullptr);
 
+  // Even if the symbol file is created, the symbol table should not be 
created by default.
+
+  // TODO:
+  // I need to create a symbol file here, but without causing it to parse the 
symbol table.
+  // See next line as a failed attempt.
+
+  // module_sp->GetSymbolFile(/*can_create=*/true); // Cannot do this because 
it will parse the symbol table.

royitaqi wrote:

@clayborg and @dmpots: Any thoughts on how to make this work?

https://github.com/llvm/llvm-project/pull/136236
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [clang] [lldb] Fix filename parsing in clang-format-diff.py for paths with spaces (PR #135779)

2025-04-17 Thread Selim Keles via lldb-commits

https://github.com/selimkeles updated 
https://github.com/llvm/llvm-project/pull/135779

>From 235ef7b9d0e5f8cb9329400a01fa1b51c74626e7 Mon Sep 17 00:00:00 2001
From: Vy Nguyen 
Date: Tue, 15 Apr 2025 11:40:07 +0200
Subject: [PATCH] Fix filename parsing in clang-format-diff.py for paths with
 spaces

---
 clang/tools/clang-format/clang-format-diff.py | 2 +-
 lldb/source/Target/Process.cpp| 9 +
 lldb/source/Target/Target.cpp | 2 +-
 3 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/clang/tools/clang-format/clang-format-diff.py 
b/clang/tools/clang-format/clang-format-diff.py
index c82b41e8bd031..3059982ba231f 100755
--- a/clang/tools/clang-format/clang-format-diff.py
+++ b/clang/tools/clang-format/clang-format-diff.py
@@ -102,7 +102,7 @@ def main():
 filename = None
 lines_by_file = {}
 for line in sys.stdin:
-match = re.search(r"^\+\+\+\ (.*?/){%s}(\S*)" % args.p, line)
+match = re.search(r"^\+\+\+\ (.*?/){%s}(.+)" % args.p, line.rstrip())
 if match:
 filename = match.group(2)
 if filename is None:
diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp
index 633f7488dc76a..73557eb767c72 100644
--- a/lldb/source/Target/Process.cpp
+++ b/lldb/source/Target/Process.cpp
@@ -1047,10 +1047,11 @@ bool Process::SetExitStatus(int status, llvm::StringRef 
exit_string) {
 info->exit_desc = {status, exit_string.str()};
   });
 
-  helper.DispatchOnExit([&](telemetry::ProcessExitInfo *info) {
-info->module_uuid = module_uuid;
-info->pid = m_pid;
-  });
+  helper.DispatchOnExit(
+  [module_uuid, pid = m_pid](telemetry::ProcessExitInfo *info) {
+info->module_uuid = module_uuid;
+info->pid = pid;
+  });
 
   m_exit_status = status;
   if (!exit_string.empty())
diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp
index 42b1561fb2993..b6186b76d6236 100644
--- a/lldb/source/Target/Target.cpp
+++ b/lldb/source/Target/Target.cpp
@@ -1578,7 +1578,7 @@ void Target::SetExecutableModule(ModuleSP &executable_sp,
   info->is_start_entry = true;
 });
 
-helper.DispatchOnExit([&](telemetry::ExecutableModuleInfo *info) {
+helper.DispatchOnExit([&, pid](telemetry::ExecutableModuleInfo *info) {
   info->exec_mod = executable_sp;
   info->uuid = executable_sp->GetUUID();
   info->pid = pid;

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb] Avoid force loading symbols in statistics collection (PR #136236)

2025-04-17 Thread via lldb-commits

https://github.com/royitaqi updated 
https://github.com/llvm/llvm-project/pull/136236

>From 85cb8cca4fe41cf4080b3dbf7ce4f98c4b15a3c7 Mon Sep 17 00:00:00 2001
From: Roy Shi 
Date: Thu, 17 Apr 2025 15:03:00 -0700
Subject: [PATCH 1/3] [lldb] Do not parse symtab during statistics dump

Summary:

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:

Differential Revision: https://phabricator.intern.facebook.com/D73218490
---
 lldb/include/lldb/Symbol/ObjectFile.h | 2 +-
 lldb/include/lldb/Symbol/SymbolFile.h | 4 ++--
 lldb/include/lldb/Symbol/SymbolFileOnDemand.h | 2 +-
 lldb/source/Core/Module.cpp   | 2 +-
 lldb/source/Symbol/ObjectFile.cpp | 4 ++--
 lldb/source/Symbol/SymbolFile.cpp | 4 ++--
 6 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/lldb/include/lldb/Symbol/ObjectFile.h 
b/lldb/include/lldb/Symbol/ObjectFile.h
index cfcca04a76de8..7fca6383fa9f3 100644
--- a/lldb/include/lldb/Symbol/ObjectFile.h
+++ b/lldb/include/lldb/Symbol/ObjectFile.h
@@ -319,7 +319,7 @@ class ObjectFile : public 
std::enable_shared_from_this,
   ///
   /// \return
   /// The symbol table for this object file.
-  Symtab *GetSymtab();
+  Symtab *GetSymtab(bool can_create = true);
 
   /// Parse the symbol table into the provides symbol table object.
   ///
diff --git a/lldb/include/lldb/Symbol/SymbolFile.h 
b/lldb/include/lldb/Symbol/SymbolFile.h
index f35d3ee9f22ae..df2f263b18e17 100644
--- a/lldb/include/lldb/Symbol/SymbolFile.h
+++ b/lldb/include/lldb/Symbol/SymbolFile.h
@@ -144,7 +144,7 @@ class SymbolFile : public PluginInterface {
   virtual uint32_t GetNumCompileUnits() = 0;
   virtual lldb::CompUnitSP GetCompileUnitAtIndex(uint32_t idx) = 0;
 
-  virtual Symtab *GetSymtab() = 0;
+  virtual Symtab *GetSymtab(bool can_create = true) = 0;
 
   virtual lldb::LanguageType ParseLanguage(CompileUnit &comp_unit) = 0;
   /// Return the Xcode SDK comp_unit was compiled against.
@@ -533,7 +533,7 @@ class SymbolFileCommon : public SymbolFile {
 return m_abilities;
   }
 
-  Symtab *GetSymtab() override;
+  Symtab *GetSymtab(bool can_create = true) override;
 
   ObjectFile *GetObjectFile() override { return m_objfile_sp.get(); }
   const ObjectFile *GetObjectFile() const override {
diff --git a/lldb/include/lldb/Symbol/SymbolFileOnDemand.h 
b/lldb/include/lldb/Symbol/SymbolFileOnDemand.h
index 7a366bfabec86..ce9fc8626ac34 100644
--- a/lldb/include/lldb/Symbol/SymbolFileOnDemand.h
+++ b/lldb/include/lldb/Symbol/SymbolFileOnDemand.h
@@ -186,7 +186,7 @@ class SymbolFileOnDemand : public lldb_private::SymbolFile {
 
   uint32_t GetAbilities() override;
 
-  Symtab *GetSymtab() override { return m_sym_file_impl->GetSymtab(); }
+  Symtab *GetSymtab(bool can_create = true) override { return 
m_sym_file_impl->GetSymtab(can_create); }
 
   ObjectFile *GetObjectFile() override {
 return m_sym_file_impl->GetObjectFile();
diff --git a/lldb/source/Core/Module.cpp b/lldb/source/Core/Module.cpp
index ad7046e596278..625c14e4a2153 100644
--- a/lldb/source/Core/Module.cpp
+++ b/lldb/source/Core/Module.cpp
@@ -997,7 +997,7 @@ SymbolFile *Module::GetSymbolFile(bool can_create, Stream 
*feedback_strm) {
 
 Symtab *Module::GetSymtab(bool can_create) {
   if (SymbolFile *symbols = GetSymbolFile(can_create))
-return symbols->GetSymtab();
+return symbols->GetSymtab(can_create);
   return nullptr;
 }
 
diff --git a/lldb/source/Symbol/ObjectFile.cpp 
b/lldb/source/Symbol/ObjectFile.cpp
index 2f2c59d6af620..292c00b9ac166 100644
--- a/lldb/source/Symbol/ObjectFile.cpp
+++ b/lldb/source/Symbol/ObjectFile.cpp
@@ -735,9 +735,9 @@ void llvm::format_provider::format(
 }
 
 
-Symtab *ObjectFile::GetSymtab() {
+Symtab *ObjectFile::GetSymtab(bool can_create) {
   ModuleSP module_sp(GetModule());
-  if (module_sp) {
+  if (module_sp && can_create) {
 // We can't take the module lock in ObjectFile::GetSymtab() or we can
 // deadlock in DWARF indexing when any file asks for the symbol table from
 // an object file. This currently happens in the preloading of symbols in
diff --git a/lldb/source/Symbol/SymbolFile.cpp 
b/lldb/source/Symbol/SymbolFile.cpp
index 94e32b55572dd..870d778dca740 100644
--- a/lldb/source/Symbol/SymbolFile.cpp
+++ b/lldb/source/Symbol/SymbolFile.cpp
@@ -152,10 +152,10 @@ void SymbolFile::AssertModuleLock() {
 
 SymbolFile::RegisterInfoResolver::~RegisterInfoResolver() = default;
 
-Symtab *SymbolFileCommon::GetSymtab() {
+Symtab *SymbolFileCommon::GetSymtab(bool can_create) {
   std::lock_guard guard(GetModuleMutex());
   // Fetch the symtab from the main object file.
-  auto *symtab = GetMainObjectFile()->GetSymtab();
+  auto *symtab = GetMainObjectFile()->GetSymtab(can_create);
   if (m_symtab != symtab) {
 m_symtab = symtab;
 

>From 1fccd09dde2d1f8da6f8253516c5c908049eb270 Mon Sep 17 00:00:00 2001
From: Roy Shi 
Date: Thu, 17 Apr 2025 21:45:48 -0700
Subject: [PATCH 2/3] Fix compilation of a unit test

---
 lldb/unittests/Symbol/LineTableTest.cpp

[Lldb-commits] [lldb] [lldb][Mach-O corefiles] Don't init Target arch to corefile (PR #136065)

2025-04-17 Thread via lldb-commits

https://github.com/jimingham approved this pull request.

Okay, that makes sense.

https://github.com/llvm/llvm-project/pull/136065
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb] Add symbol/table count into statistics (PR #136226)

2025-04-17 Thread via lldb-commits

https://github.com/royitaqi updated 
https://github.com/llvm/llvm-project/pull/136226

>From 4157b0d1426830a4c19af7401728ef86dbc2d8c9 Mon Sep 17 00:00:00 2001
From: Roy Shi 
Date: Wed, 16 Apr 2025 09:19:04 -0700
Subject: [PATCH 1/2] [lldb] Add new stats (# symbols loaded, # symbol tables
 loaded) to module and target

Summary:

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:

Differential Revision: https://phabricator.intern.facebook.com/D73117709
---
 lldb/include/lldb/Target/Statistics.h |  1 +
 lldb/source/Target/Statistics.cpp | 18 +++
 .../commands/statistics/basic/TestStats.py| 23 +++
 3 files changed, 32 insertions(+), 10 deletions(-)

diff --git a/lldb/include/lldb/Target/Statistics.h 
b/lldb/include/lldb/Target/Statistics.h
index ee365357fcf31..b87a12a8ab9cd 100644
--- a/lldb/include/lldb/Target/Statistics.h
+++ b/lldb/include/lldb/Target/Statistics.h
@@ -120,6 +120,7 @@ struct ModuleStats {
   llvm::StringMap type_system_stats;
   double symtab_parse_time = 0.0;
   double symtab_index_time = 0.0;
+  uint32_t num_symbols_loaded = 0;
   double debug_parse_time = 0.0;
   double debug_index_time = 0.0;
   uint64_t debug_info_size = 0;
diff --git a/lldb/source/Target/Statistics.cpp 
b/lldb/source/Target/Statistics.cpp
index b5d2e7bda1edf..f70fb529544a5 100644
--- a/lldb/source/Target/Statistics.cpp
+++ b/lldb/source/Target/Statistics.cpp
@@ -71,6 +71,7 @@ json::Value ModuleStats::ToJSON() const {
   module.try_emplace("debugInfoHadIncompleteTypes",
  debug_info_had_incomplete_types);
   module.try_emplace("symbolTableStripped", symtab_stripped);
+  module.try_emplace("symbolsLoaded", num_symbols_loaded);
   if (!symfile_path.empty())
 module.try_emplace("symbolFilePath", symfile_path);
 
@@ -293,7 +294,8 @@ llvm::json::Value DebuggerStats::ReportStatistics(
   double debug_parse_time = 0.0;
   double debug_index_time = 0.0;
   uint32_t symtabs_loaded = 0;
-  uint32_t symtabs_saved = 0;
+  uint32_t symtabs_loaded_from_cache = 0;
+  uint32_t symtabs_saved_to_cache = 0;
   uint32_t debug_index_loaded = 0;
   uint32_t debug_index_saved = 0;
   uint64_t debug_info_size = 0;
@@ -309,6 +311,7 @@ llvm::json::Value DebuggerStats::ReportStatistics(
   uint32_t num_modules_with_variable_errors = 0;
   uint32_t num_modules_with_incomplete_types = 0;
   uint32_t num_stripped_modules = 0;
+  uint32_t num_symbols_loaded = 0;
   for (size_t image_idx = 0; image_idx < num_modules; ++image_idx) {
 Module *module = target != nullptr
  ? 
target->GetImages().GetModuleAtIndex(image_idx).get()
@@ -318,12 +321,15 @@ llvm::json::Value DebuggerStats::ReportStatistics(
 module_stat.symtab_index_time = module->GetSymtabIndexTime().get().count();
 Symtab *symtab = module->GetSymtab(/*can_create=*/false);
 if (symtab) {
+  module_stat.num_symbols_loaded = symtab->GetNumSymbols();
+  num_symbols_loaded += module_stat.num_symbols_loaded;
+  symtabs_loaded++;
   module_stat.symtab_loaded_from_cache = symtab->GetWasLoadedFromCache();
   if (module_stat.symtab_loaded_from_cache)
-++symtabs_loaded;
+++symtabs_loaded_from_cache;
   module_stat.symtab_saved_to_cache = symtab->GetWasSavedToCache();
   if (module_stat.symtab_saved_to_cache)
-++symtabs_saved;
+++symtabs_saved_to_cache;
 }
 SymbolFile *sym_file = module->GetSymbolFile(/*can_create=*/false);
 if (sym_file) {
@@ -393,8 +399,9 @@ llvm::json::Value DebuggerStats::ReportStatistics(
   json::Object global_stats{
   {"totalSymbolTableParseTime", symtab_parse_time},
   {"totalSymbolTableIndexTime", symtab_index_time},
-  {"totalSymbolTablesLoadedFromCache", symtabs_loaded},
-  {"totalSymbolTablesSavedToCache", symtabs_saved},
+  {"totalSymbolTablesLoaded", symtabs_loaded},
+  {"totalSymbolTablesLoadedFromCache", symtabs_loaded_from_cache},
+  {"totalSymbolTablesSavedToCache", symtabs_saved_to_cache},
   {"totalDebugInfoParseTime", debug_parse_time},
   {"totalDebugInfoIndexTime", debug_index_time},
   {"totalDebugInfoIndexLoadedFromCache", debug_index_loaded},
@@ -407,6 +414,7 @@ llvm::json::Value DebuggerStats::ReportStatistics(
num_modules_with_incomplete_types},
   {"totalDebugInfoEnabled", num_debug_info_enabled_modules},
   {"totalSymbolTableStripped", num_stripped_modules},
+  {"totalSymbolsLoaded", num_symbols_loaded},
   };
 
   if (include_targets) {
diff --git a/lldb/test/API/commands/statistics/basic/TestStats.py 
b/lldb/test/API/commands/statistics/basic/TestStats.py
index 54881c13bcb68..1ecbe89e3b3d4 100644
--- a/lldb/test/API/commands/statistics/basic/TestStats.py
+++ b/lldb/test/API/commands/statistics/basic/TestStats.py
@@ -159,6 +159,8 @@ def test_default_no_run(self):
 """
 self.build()
 target = self.createTestTarget()
+
+# Verify top-level keys.
 debug_stats = self.get_stats(

[Lldb-commits] [lldb] Add download time for each module in statistics (PR #134563)

2025-04-17 Thread via lldb-commits


@@ -1217,28 +1217,36 @@ 
PluginManager::GetSymbolLocatorCreateCallbackAtIndex(uint32_t idx) {
 }
 
 ModuleSpec
-PluginManager::LocateExecutableObjectFile(const ModuleSpec &module_spec) {
+PluginManager::LocateExecutableObjectFile(const ModuleSpec &module_spec,
+  std::string *locator_name) {
   auto instances = GetSymbolLocatorInstances().GetSnapshot();
   for (auto &instance : instances) {
 if (instance.locate_executable_object_file) {
   std::optional result =
   instance.locate_executable_object_file(module_spec);
-  if (result)
+  if (result) {

GeorgeHuyubo wrote:

Not possible here, since it's a loop. We will try all symbol locator.

https://github.com/llvm/llvm-project/pull/134563
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb] Do not create/parse symtab during `DebuggerStats::ReportStatistics()` (PR #136236)

2025-04-17 Thread via lldb-commits

https://github.com/royitaqi created 
https://github.com/llvm/llvm-project/pull/136236

# The change

Currently, `DebuggerStats::ReportStatistics()` calls `Module::GetSymtab(/* 
can_create */ false)`, but then the latter calls `SymbolFile::GetSymtab()`. 
This will load symbols if haven't yet.

The problem is that `DebuggerStats::ReportStatistics` should be read-only. This 
is especially important because it reports stats for symtab parsing/indexing 
time, which could be affected by the reporting itself if it's not read-only.

This patch fixes this problem by adding an optional parameter 
`SymbolFile::GetSymtab(can_create = true)` and receive the `false` value passed 
down from `Module::GetSymtab()` when the call was initiated from 
`DebuggerStats::ReportStatistics()`.

# Tests

**Manually tested** with a helloworld program, that:
1. Without this patch, a breakpoint in `ObjectFileMachO::ParseSymtab()` is hit 
when we do `lldb a.out` then `statistics dump`.
2. With this patch, said breakpoint is not hit.

**Unit test**: Still looking for a good unit test file to add a new test. Will 
update here.

>From 85cb8cca4fe41cf4080b3dbf7ce4f98c4b15a3c7 Mon Sep 17 00:00:00 2001
From: Roy Shi 
Date: Thu, 17 Apr 2025 15:03:00 -0700
Subject: [PATCH] [lldb] Do not parse symtab during statistics dump

Summary:

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:

Differential Revision: https://phabricator.intern.facebook.com/D73218490
---
 lldb/include/lldb/Symbol/ObjectFile.h | 2 +-
 lldb/include/lldb/Symbol/SymbolFile.h | 4 ++--
 lldb/include/lldb/Symbol/SymbolFileOnDemand.h | 2 +-
 lldb/source/Core/Module.cpp   | 2 +-
 lldb/source/Symbol/ObjectFile.cpp | 4 ++--
 lldb/source/Symbol/SymbolFile.cpp | 4 ++--
 6 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/lldb/include/lldb/Symbol/ObjectFile.h 
b/lldb/include/lldb/Symbol/ObjectFile.h
index cfcca04a76de8..7fca6383fa9f3 100644
--- a/lldb/include/lldb/Symbol/ObjectFile.h
+++ b/lldb/include/lldb/Symbol/ObjectFile.h
@@ -319,7 +319,7 @@ class ObjectFile : public 
std::enable_shared_from_this,
   ///
   /// \return
   /// The symbol table for this object file.
-  Symtab *GetSymtab();
+  Symtab *GetSymtab(bool can_create = true);
 
   /// Parse the symbol table into the provides symbol table object.
   ///
diff --git a/lldb/include/lldb/Symbol/SymbolFile.h 
b/lldb/include/lldb/Symbol/SymbolFile.h
index f35d3ee9f22ae..df2f263b18e17 100644
--- a/lldb/include/lldb/Symbol/SymbolFile.h
+++ b/lldb/include/lldb/Symbol/SymbolFile.h
@@ -144,7 +144,7 @@ class SymbolFile : public PluginInterface {
   virtual uint32_t GetNumCompileUnits() = 0;
   virtual lldb::CompUnitSP GetCompileUnitAtIndex(uint32_t idx) = 0;
 
-  virtual Symtab *GetSymtab() = 0;
+  virtual Symtab *GetSymtab(bool can_create = true) = 0;
 
   virtual lldb::LanguageType ParseLanguage(CompileUnit &comp_unit) = 0;
   /// Return the Xcode SDK comp_unit was compiled against.
@@ -533,7 +533,7 @@ class SymbolFileCommon : public SymbolFile {
 return m_abilities;
   }
 
-  Symtab *GetSymtab() override;
+  Symtab *GetSymtab(bool can_create = true) override;
 
   ObjectFile *GetObjectFile() override { return m_objfile_sp.get(); }
   const ObjectFile *GetObjectFile() const override {
diff --git a/lldb/include/lldb/Symbol/SymbolFileOnDemand.h 
b/lldb/include/lldb/Symbol/SymbolFileOnDemand.h
index 7a366bfabec86..ce9fc8626ac34 100644
--- a/lldb/include/lldb/Symbol/SymbolFileOnDemand.h
+++ b/lldb/include/lldb/Symbol/SymbolFileOnDemand.h
@@ -186,7 +186,7 @@ class SymbolFileOnDemand : public lldb_private::SymbolFile {
 
   uint32_t GetAbilities() override;
 
-  Symtab *GetSymtab() override { return m_sym_file_impl->GetSymtab(); }
+  Symtab *GetSymtab(bool can_create = true) override { return 
m_sym_file_impl->GetSymtab(can_create); }
 
   ObjectFile *GetObjectFile() override {
 return m_sym_file_impl->GetObjectFile();
diff --git a/lldb/source/Core/Module.cpp b/lldb/source/Core/Module.cpp
index ad7046e596278..625c14e4a2153 100644
--- a/lldb/source/Core/Module.cpp
+++ b/lldb/source/Core/Module.cpp
@@ -997,7 +997,7 @@ SymbolFile *Module::GetSymbolFile(bool can_create, Stream 
*feedback_strm) {
 
 Symtab *Module::GetSymtab(bool can_create) {
   if (SymbolFile *symbols = GetSymbolFile(can_create))
-return symbols->GetSymtab();
+return symbols->GetSymtab(can_create);
   return nullptr;
 }
 
diff --git a/lldb/source/Symbol/ObjectFile.cpp 
b/lldb/source/Symbol/ObjectFile.cpp
index 2f2c59d6af620..292c00b9ac166 100644
--- a/lldb/source/Symbol/ObjectFile.cpp
+++ b/lldb/source/Symbol/ObjectFile.cpp
@@ -735,9 +735,9 @@ void llvm::format_provider::format(
 }
 
 
-Symtab *ObjectFile::GetSymtab() {
+Symtab *ObjectFile::GetSymtab(bool can_create) {
   ModuleSP module_sp(GetModule());
-  if (module_sp) {
+  if (module_sp && can_create) {
 // We can't take the module lock in ObjectFile::GetSymtab() or we can
 // deadlock in DWARF indexing when any file asks for the

[Lldb-commits] [lldb] [lldb][nfc] Factor out code from ThreadPlanStepOut ctor (PR #136159)

2025-04-17 Thread Felipe de Azevedo Piovezan via lldb-commits

https://github.com/felipepiovezan created 
https://github.com/llvm/llvm-project/pull/136159

A future patch will need to create a new constructor for this class, and 
extracting code out of its sole existing constructor will make this easier.

This commit creates a helper function for the code computing the target frame 
to step out to.

>From 42850d46b705c239b099be8d102a3756a8fd5283 Mon Sep 17 00:00:00 2001
From: Felipe de Azevedo Piovezan 
Date: Tue, 15 Apr 2025 10:20:41 -0700
Subject: [PATCH] [lldb][nfc] Factor out code from ThreadPlanStepOut ctor

A future patch will need to create a new constructor for this class, and
extracting code out of its sole existing constructor will make this
easier.

This commit creates a helper function for the code computing the target
frame to step out to.
---
 lldb/source/Target/ThreadPlanStepOut.cpp | 47 +++-
 1 file changed, 29 insertions(+), 18 deletions(-)

diff --git a/lldb/source/Target/ThreadPlanStepOut.cpp 
b/lldb/source/Target/ThreadPlanStepOut.cpp
index a05c46db6b8ca..26c1abe710319 100644
--- a/lldb/source/Target/ThreadPlanStepOut.cpp
+++ b/lldb/source/Target/ThreadPlanStepOut.cpp
@@ -31,6 +31,32 @@ using namespace lldb_private;
 
 uint32_t ThreadPlanStepOut::s_default_flag_values = 0;
 
+/// Computes the target frame this plan should step out to.
+static StackFrameSP
+ComputeTargetFrame(Thread &thread, uint32_t start_frame_idx,
+   std::vector &skipped_frames) {
+  uint32_t frame_idx = start_frame_idx + 1;
+  StackFrameSP return_frame_sp = thread.GetStackFrameAtIndex(frame_idx);
+  if (!return_frame_sp)
+return nullptr;
+
+  while (return_frame_sp->IsArtificial() || return_frame_sp->IsHidden()) {
+skipped_frames.push_back(return_frame_sp);
+
+frame_idx++;
+return_frame_sp = thread.GetStackFrameAtIndex(frame_idx);
+
+// We never expect to see an artificial frame without a regular ancestor.
+// Defensively refuse to step out.
+if (!return_frame_sp) {
+  LLDB_LOG(GetLog(LLDBLog::Step),
+   "Can't step out of frame with artificial ancestors");
+  return nullptr;
+}
+  }
+  return return_frame_sp;
+}
+
 // ThreadPlanStepOut: Step out of the current frame
 ThreadPlanStepOut::ThreadPlanStepOut(
 Thread &thread, SymbolContext *context, bool first_insn, bool stop_others,
@@ -44,34 +70,18 @@ ThreadPlanStepOut::ThreadPlanStepOut(
   m_return_addr(LLDB_INVALID_ADDRESS), m_stop_others(stop_others),
   m_immediate_step_from_function(nullptr),
   m_calculate_return_value(gather_return_value) {
-  Log *log = GetLog(LLDBLog::Step);
   SetFlagsToDefault();
   SetupAvoidNoDebug(step_out_avoids_code_without_debug_info);
 
   m_step_from_insn = thread.GetRegisterContext()->GetPC(0);
 
-  uint32_t return_frame_index = frame_idx + 1;
-  StackFrameSP 
return_frame_sp(thread.GetStackFrameAtIndex(return_frame_index));
+  StackFrameSP return_frame_sp =
+  ComputeTargetFrame(thread, frame_idx, m_stepped_past_frames);
   StackFrameSP 
immediate_return_from_sp(thread.GetStackFrameAtIndex(frame_idx));
 
   if (!return_frame_sp || !immediate_return_from_sp)
 return; // we can't do anything here.  ValidatePlan() will return false.
 
-  // While stepping out, behave as-if artificial frames are not present.
-  while (return_frame_sp->IsArtificial() || return_frame_sp->IsHidden()) {
-m_stepped_past_frames.push_back(return_frame_sp);
-
-++return_frame_index;
-return_frame_sp = thread.GetStackFrameAtIndex(return_frame_index);
-
-// We never expect to see an artificial frame without a regular ancestor.
-// If this happens, log the issue and defensively refuse to step out.
-if (!return_frame_sp) {
-  LLDB_LOG(log, "Can't step out of frame with artificial ancestors");
-  return;
-}
-  }
-
   m_step_out_to_id = return_frame_sp->GetStackID();
   m_immediate_step_from_id = immediate_return_from_sp->GetStackID();
 
@@ -125,6 +135,7 @@ ThreadPlanStepOut::ThreadPlanStepOut(
 
 // Perform some additional validation on the return address.
 uint32_t permissions = 0;
+Log *log = GetLog(LLDBLog::Step);
 if (!m_process.GetLoadAddressPermissions(m_return_addr, permissions)) {
   LLDB_LOGF(log, "ThreadPlanStepOut(%p): Return address (0x%" PRIx64
 ") permissions not found.", static_cast(this),

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb][Mach-O corefiles] Don't init Target arch to corefile (PR #136065)

2025-04-17 Thread Jason Molenda via lldb-commits

https://github.com/jasonmolenda updated 
https://github.com/llvm/llvm-project/pull/136065

>From 41274f7632d13ab7ec83a420c6ad6258e6fe8c36 Mon Sep 17 00:00:00 2001
From: Jason Molenda 
Date: Wed, 16 Apr 2025 17:18:02 -0700
Subject: [PATCH 1/2] [lldb][Mach-O corefiles] Don't init Target arch to
 corefile

This patch is making three changes, when loading a Mach-O
corefile:

1. At the start of `DoLoadCore`, if a binary was provided in addition
to the corefile, initialize the Target's ArchSpec.

2. Before ProcessMachCore does its "exhaustive search" fallback,
looking through the corefile contents for a userland dyld or mach
kernel, we must make sure the Target has an ArchSpec, or methods
that check the address word size, or initialize a DataExtractor
based on the Target arch will not succeed.

3. Add logging when setting the Target's arch listing exactly what
that setting was based on -- the corefile itself, or the main binary.

Jonas landed a change last August (started with a patch from me)
which removed the Target ArchSpec initialization at the start of
DoLoadCore, in a scenario where the corefile had arch armv7 and
the main binary had arch armv7em (Cortex-M), and there was python
code in the main binary's dSYM which sets the operating system threads
provider based on the Target arch.  It did different things for
armv7 or armv7em, and so it would fail.

Jonas' patch removed any ArchSpec setting at the start of DoLoadCore,
so we wouldn't have an incorrect arch value, but that broke the
exhaustive search for kernel binaries, because we didn't have an
address word size or endianness.

This patch should navigate the needs of both use cases.

I spent a good bit of time trying to construct a test to capture all
of these requirements -- but it turns out to be a good bit difficult,
encompassing both a genuine kernel corefiles and a microcontroller
firmware corefiles.

rdar://146821929
---
 .../Process/mach-core/ProcessMachCore.cpp | 58 ++-
 1 file changed, 56 insertions(+), 2 deletions(-)

diff --git a/lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp 
b/lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp
index 281f3a0db8f69..17362b2d2855d 100644
--- a/lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp
+++ b/lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp
@@ -426,6 +426,32 @@ void ProcessMachCore::LoadBinariesViaExhaustiveSearch() {
   std::vector dylds_found;
   std::vector kernels_found;
 
+  // To do an exhaustive search, we'll need to create data extractors
+  // to get correctly sized/endianness fields, and if the Target still
+  // doesn't have an architecture set, we need to seed it from either
+  // the main binary (if we were given one) or the corefile cputype/cpusubtype.
+  if (!GetTarget().GetArchitecture().IsValid()) {
+Log *log(GetLog(LLDBLog::DynamicLoader | LLDBLog::Target));
+ModuleSP exe_module_sp = GetTarget().GetExecutableModule();
+if (exe_module_sp && exe_module_sp->GetArchitecture().IsValid()) {
+  LLDB_LOGF(log,
+"ProcessMachCore::%s: Was given binary + corefile, setting "
+"target ArchSpec to binary to start",
+__FUNCTION__);
+  GetTarget().SetArchitecture(exe_module_sp->GetArchitecture());
+} else {
+  // The corefile's architecture is our best starting point.
+  ArchSpec arch(m_core_module_sp->GetArchitecture());
+  if (arch.IsValid()) {
+LLDB_LOGF(log,
+  "ProcessMachCore::%s: Setting target ArchSpec based on "
+  "corefile mach-o cputype/cpusubtype",
+  __FUNCTION__);
+GetTarget().SetArchitecture(arch);
+  }
+}
+  }
+
   const size_t num_core_aranges = m_core_aranges.GetSize();
   for (size_t i = 0; i < num_core_aranges; ++i) {
 const VMRangeToFileOffset::Entry *entry = 
m_core_aranges.GetEntryAtIndex(i);
@@ -569,6 +595,7 @@ Status ProcessMachCore::DoLoadCore() {
 error = Status::FromErrorString("invalid core module");
 return error;
   }
+  Log *log(GetLog(LLDBLog::DynamicLoader | LLDBLog::Target));
 
   ObjectFile *core_objfile = m_core_module_sp->GetObjectFile();
   if (core_objfile == nullptr) {
@@ -578,20 +605,47 @@ Status ProcessMachCore::DoLoadCore() {
 
   SetCanJIT(false);
 
+  // If we have an executable binary in the Target already,
+  // use that to set the Target's ArchSpec.
+  //
+  // Don't initialize the ArchSpec based on the corefile's cputype/cpusubtype
+  // here, the corefile creator may not know the correct subtype of the code
+  // that is executing, initialize the Target to that, and if the
+  // main binary has Python code which initializes based on the Target arch,
+  // get the wrong subtype value.
+  ModuleSP exe_module_sp = GetTarget().GetExecutableModule();
+  if (exe_module_sp && exe_module_sp->GetArchitecture().IsValid()) {
+LLDB_LOGF(log,
+  "ProcessMachCore::%s: Was given binary + corefile, setting "
+  "target A

[Lldb-commits] [lldb] [lldb][nfc] Factor out code from ThreadPlanStepOut ctor (PR #136159)

2025-04-17 Thread via lldb-commits

llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: Felipe de Azevedo Piovezan (felipepiovezan)


Changes

A future patch will need to create a new constructor for this class, and 
extracting code out of its sole existing constructor will make this easier.

This commit creates a helper function for the code computing the target frame 
to step out to.

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


1 Files Affected:

- (modified) lldb/source/Target/ThreadPlanStepOut.cpp (+29-18) 


``diff
diff --git a/lldb/source/Target/ThreadPlanStepOut.cpp 
b/lldb/source/Target/ThreadPlanStepOut.cpp
index a05c46db6b8ca..26c1abe710319 100644
--- a/lldb/source/Target/ThreadPlanStepOut.cpp
+++ b/lldb/source/Target/ThreadPlanStepOut.cpp
@@ -31,6 +31,32 @@ using namespace lldb_private;
 
 uint32_t ThreadPlanStepOut::s_default_flag_values = 0;
 
+/// Computes the target frame this plan should step out to.
+static StackFrameSP
+ComputeTargetFrame(Thread &thread, uint32_t start_frame_idx,
+   std::vector &skipped_frames) {
+  uint32_t frame_idx = start_frame_idx + 1;
+  StackFrameSP return_frame_sp = thread.GetStackFrameAtIndex(frame_idx);
+  if (!return_frame_sp)
+return nullptr;
+
+  while (return_frame_sp->IsArtificial() || return_frame_sp->IsHidden()) {
+skipped_frames.push_back(return_frame_sp);
+
+frame_idx++;
+return_frame_sp = thread.GetStackFrameAtIndex(frame_idx);
+
+// We never expect to see an artificial frame without a regular ancestor.
+// Defensively refuse to step out.
+if (!return_frame_sp) {
+  LLDB_LOG(GetLog(LLDBLog::Step),
+   "Can't step out of frame with artificial ancestors");
+  return nullptr;
+}
+  }
+  return return_frame_sp;
+}
+
 // ThreadPlanStepOut: Step out of the current frame
 ThreadPlanStepOut::ThreadPlanStepOut(
 Thread &thread, SymbolContext *context, bool first_insn, bool stop_others,
@@ -44,34 +70,18 @@ ThreadPlanStepOut::ThreadPlanStepOut(
   m_return_addr(LLDB_INVALID_ADDRESS), m_stop_others(stop_others),
   m_immediate_step_from_function(nullptr),
   m_calculate_return_value(gather_return_value) {
-  Log *log = GetLog(LLDBLog::Step);
   SetFlagsToDefault();
   SetupAvoidNoDebug(step_out_avoids_code_without_debug_info);
 
   m_step_from_insn = thread.GetRegisterContext()->GetPC(0);
 
-  uint32_t return_frame_index = frame_idx + 1;
-  StackFrameSP 
return_frame_sp(thread.GetStackFrameAtIndex(return_frame_index));
+  StackFrameSP return_frame_sp =
+  ComputeTargetFrame(thread, frame_idx, m_stepped_past_frames);
   StackFrameSP 
immediate_return_from_sp(thread.GetStackFrameAtIndex(frame_idx));
 
   if (!return_frame_sp || !immediate_return_from_sp)
 return; // we can't do anything here.  ValidatePlan() will return false.
 
-  // While stepping out, behave as-if artificial frames are not present.
-  while (return_frame_sp->IsArtificial() || return_frame_sp->IsHidden()) {
-m_stepped_past_frames.push_back(return_frame_sp);
-
-++return_frame_index;
-return_frame_sp = thread.GetStackFrameAtIndex(return_frame_index);
-
-// We never expect to see an artificial frame without a regular ancestor.
-// If this happens, log the issue and defensively refuse to step out.
-if (!return_frame_sp) {
-  LLDB_LOG(log, "Can't step out of frame with artificial ancestors");
-  return;
-}
-  }
-
   m_step_out_to_id = return_frame_sp->GetStackID();
   m_immediate_step_from_id = immediate_return_from_sp->GetStackID();
 
@@ -125,6 +135,7 @@ ThreadPlanStepOut::ThreadPlanStepOut(
 
 // Perform some additional validation on the return address.
 uint32_t permissions = 0;
+Log *log = GetLog(LLDBLog::Step);
 if (!m_process.GetLoadAddressPermissions(m_return_addr, permissions)) {
   LLDB_LOGF(log, "ThreadPlanStepOut(%p): Return address (0x%" PRIx64
 ") permissions not found.", static_cast(this),

``




https://github.com/llvm/llvm-project/pull/136159
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] 12becff - [lldb] Create ThreadPlanStepOut ctor that never skips frames (#136163)

2025-04-17 Thread via lldb-commits

Author: Felipe de Azevedo Piovezan
Date: 2025-04-17T14:13:28-07:00
New Revision: 12becfff035a33141a0b2fb3ea5d5558738ce7eb

URL: 
https://github.com/llvm/llvm-project/commit/12becfff035a33141a0b2fb3ea5d5558738ce7eb
DIFF: 
https://github.com/llvm/llvm-project/commit/12becfff035a33141a0b2fb3ea5d5558738ce7eb.diff

LOG: [lldb] Create ThreadPlanStepOut ctor that never skips frames (#136163)

The function QueueThreadPlanForStepOutNoShouldStop has the semantics of
"go this parent frame"; ThreadPlanStepOut needs to respect that, not
skipping over any frames it finds uninteresting. This commit creates a
constructor that respects such instruction.

Added: 


Modified: 
lldb/include/lldb/Target/ThreadPlanStepOut.h
lldb/source/Target/Thread.cpp
lldb/source/Target/ThreadPlanStepOut.cpp

Removed: 




diff  --git a/lldb/include/lldb/Target/ThreadPlanStepOut.h 
b/lldb/include/lldb/Target/ThreadPlanStepOut.h
index e414a6e0a2d49..ad4434be14120 100644
--- a/lldb/include/lldb/Target/ThreadPlanStepOut.h
+++ b/lldb/include/lldb/Target/ThreadPlanStepOut.h
@@ -17,6 +17,9 @@ namespace lldb_private {
 
 class ThreadPlanStepOut : public ThreadPlan, public ThreadPlanShouldStopHere {
 public:
+  /// Creates a thread plan to step out from frame_idx, skipping parent frames
+  /// if they are artificial or hidden frames. Also skips frames without debug
+  /// info based on step_out_avoids_code_without_debug_info.
   ThreadPlanStepOut(Thread &thread, SymbolContext *addr_context,
 bool first_insn, bool stop_others, Vote report_stop_vote,
 Vote report_run_vote, uint32_t frame_idx,
@@ -24,6 +27,12 @@ class ThreadPlanStepOut : public ThreadPlan, public 
ThreadPlanShouldStopHere {
 bool continue_to_next_branch = false,
 bool gather_return_value = true);
 
+  /// Creates a thread plan to step out from frame_idx to frame_idx + 1.
+  ThreadPlanStepOut(Thread &thread, bool stop_others, Vote report_stop_vote,
+Vote report_run_vote, uint32_t frame_idx,
+bool continue_to_next_branch = false,
+bool gather_return_value = true);
+
   ~ThreadPlanStepOut() override;
 
   void GetDescription(Stream *s, lldb::DescriptionLevel level) override;

diff  --git a/lldb/source/Target/Thread.cpp b/lldb/source/Target/Thread.cpp
index accc4708c24e1..b0e0f1e67c060 100644
--- a/lldb/source/Target/Thread.cpp
+++ b/lldb/source/Target/Thread.cpp
@@ -1360,9 +1360,8 @@ ThreadPlanSP 
Thread::QueueThreadPlanForStepOutNoShouldStop(
   const bool calculate_return_value =
   false; // No need to calculate the return value here.
   ThreadPlanSP thread_plan_sp(new ThreadPlanStepOut(
-  *this, addr_context, first_insn, stop_other_threads, report_stop_vote,
-  report_run_vote, frame_idx, eLazyBoolNo, continue_to_next_branch,
-  calculate_return_value));
+  *this, stop_other_threads, report_stop_vote, report_run_vote, frame_idx,
+  continue_to_next_branch, calculate_return_value));
 
   ThreadPlanStepOut *new_plan =
   static_cast(thread_plan_sp.get());

diff  --git a/lldb/source/Target/ThreadPlanStepOut.cpp 
b/lldb/source/Target/ThreadPlanStepOut.cpp
index f2606403016a6..0628451a5abf9 100644
--- a/lldb/source/Target/ThreadPlanStepOut.cpp
+++ b/lldb/source/Target/ThreadPlanStepOut.cpp
@@ -83,6 +83,28 @@ ThreadPlanStepOut::ThreadPlanStepOut(
  continue_to_next_branch);
 }
 
+ThreadPlanStepOut::ThreadPlanStepOut(Thread &thread, bool stop_others,
+ Vote report_stop_vote,
+ Vote report_run_vote, uint32_t frame_idx,
+ bool continue_to_next_branch,
+ bool gather_return_value)
+: ThreadPlan(ThreadPlan::eKindStepOut, "Step out", thread, 
report_stop_vote,
+ report_run_vote),
+  ThreadPlanShouldStopHere(this), m_return_bp_id(LLDB_INVALID_BREAK_ID),
+  m_return_addr(LLDB_INVALID_ADDRESS), m_stop_others(stop_others),
+  m_immediate_step_from_function(nullptr),
+  m_calculate_return_value(gather_return_value) {
+  SetFlagsToDefault();
+  m_step_from_insn = thread.GetRegisterContext()->GetPC(0);
+
+  StackFrameSP return_frame_sp = thread.GetStackFrameAtIndex(frame_idx + 1);
+  StackFrameSP immediate_return_from_sp =
+  thread.GetStackFrameAtIndex(frame_idx);
+
+  SetupReturnAddress(return_frame_sp, immediate_return_from_sp, frame_idx,
+ continue_to_next_branch);
+}
+
 void ThreadPlanStepOut::SetupReturnAddress(
 StackFrameSP return_frame_sp, StackFrameSP immediate_return_from_sp,
 uint32_t frame_idx, bool continue_to_next_branch) {



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb] Create ThreadPlanStepOut ctor that never skips frames (PR #136163)

2025-04-17 Thread Felipe de Azevedo Piovezan via lldb-commits

https://github.com/felipepiovezan closed 
https://github.com/llvm/llvm-project/pull/136163
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb][Minidump Parser] Implement a range data vector for minidump memory ranges (PR #136040)

2025-04-17 Thread Miro Bucko via lldb-commits


@@ -35,6 +36,9 @@ namespace minidump {
 
 // Describes a range of memory captured in the Minidump
 struct Range {
+  // Default constructor required for range data vector
+  // but unusued.
+  Range() {}

mbucko wrote:

Range() = default;

https://github.com/llvm/llvm-project/pull/136040
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb][nfc] Factor out code from ThreadPlanStepOut ctor (PR #136159)

2025-04-17 Thread Felipe de Azevedo Piovezan via lldb-commits

https://github.com/felipepiovezan closed 
https://github.com/llvm/llvm-project/pull/136159
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] 9d5f163 - [lldb][nfc] Factor out code from ThreadPlanStepOut ctor (#136159)

2025-04-17 Thread via lldb-commits

Author: Felipe de Azevedo Piovezan
Date: 2025-04-17T11:33:07-07:00
New Revision: 9d5f16308a2a1d778b9022c3a06a3cecc6d5e066

URL: 
https://github.com/llvm/llvm-project/commit/9d5f16308a2a1d778b9022c3a06a3cecc6d5e066
DIFF: 
https://github.com/llvm/llvm-project/commit/9d5f16308a2a1d778b9022c3a06a3cecc6d5e066.diff

LOG: [lldb][nfc] Factor out code from ThreadPlanStepOut ctor (#136159)

A future patch will need to create a new constructor for this class, and
extracting code out of its sole existing constructor will make this
easier.

This commit creates a helper function for the code computing the target
frame to step out to.

Added: 


Modified: 
lldb/source/Target/ThreadPlanStepOut.cpp

Removed: 




diff  --git a/lldb/source/Target/ThreadPlanStepOut.cpp 
b/lldb/source/Target/ThreadPlanStepOut.cpp
index a05c46db6b8ca..26c1abe710319 100644
--- a/lldb/source/Target/ThreadPlanStepOut.cpp
+++ b/lldb/source/Target/ThreadPlanStepOut.cpp
@@ -31,6 +31,32 @@ using namespace lldb_private;
 
 uint32_t ThreadPlanStepOut::s_default_flag_values = 0;
 
+/// Computes the target frame this plan should step out to.
+static StackFrameSP
+ComputeTargetFrame(Thread &thread, uint32_t start_frame_idx,
+   std::vector &skipped_frames) {
+  uint32_t frame_idx = start_frame_idx + 1;
+  StackFrameSP return_frame_sp = thread.GetStackFrameAtIndex(frame_idx);
+  if (!return_frame_sp)
+return nullptr;
+
+  while (return_frame_sp->IsArtificial() || return_frame_sp->IsHidden()) {
+skipped_frames.push_back(return_frame_sp);
+
+frame_idx++;
+return_frame_sp = thread.GetStackFrameAtIndex(frame_idx);
+
+// We never expect to see an artificial frame without a regular ancestor.
+// Defensively refuse to step out.
+if (!return_frame_sp) {
+  LLDB_LOG(GetLog(LLDBLog::Step),
+   "Can't step out of frame with artificial ancestors");
+  return nullptr;
+}
+  }
+  return return_frame_sp;
+}
+
 // ThreadPlanStepOut: Step out of the current frame
 ThreadPlanStepOut::ThreadPlanStepOut(
 Thread &thread, SymbolContext *context, bool first_insn, bool stop_others,
@@ -44,34 +70,18 @@ ThreadPlanStepOut::ThreadPlanStepOut(
   m_return_addr(LLDB_INVALID_ADDRESS), m_stop_others(stop_others),
   m_immediate_step_from_function(nullptr),
   m_calculate_return_value(gather_return_value) {
-  Log *log = GetLog(LLDBLog::Step);
   SetFlagsToDefault();
   SetupAvoidNoDebug(step_out_avoids_code_without_debug_info);
 
   m_step_from_insn = thread.GetRegisterContext()->GetPC(0);
 
-  uint32_t return_frame_index = frame_idx + 1;
-  StackFrameSP 
return_frame_sp(thread.GetStackFrameAtIndex(return_frame_index));
+  StackFrameSP return_frame_sp =
+  ComputeTargetFrame(thread, frame_idx, m_stepped_past_frames);
   StackFrameSP 
immediate_return_from_sp(thread.GetStackFrameAtIndex(frame_idx));
 
   if (!return_frame_sp || !immediate_return_from_sp)
 return; // we can't do anything here.  ValidatePlan() will return false.
 
-  // While stepping out, behave as-if artificial frames are not present.
-  while (return_frame_sp->IsArtificial() || return_frame_sp->IsHidden()) {
-m_stepped_past_frames.push_back(return_frame_sp);
-
-++return_frame_index;
-return_frame_sp = thread.GetStackFrameAtIndex(return_frame_index);
-
-// We never expect to see an artificial frame without a regular ancestor.
-// If this happens, log the issue and defensively refuse to step out.
-if (!return_frame_sp) {
-  LLDB_LOG(log, "Can't step out of frame with artificial ancestors");
-  return;
-}
-  }
-
   m_step_out_to_id = return_frame_sp->GetStackID();
   m_immediate_step_from_id = immediate_return_from_sp->GetStackID();
 
@@ -125,6 +135,7 @@ ThreadPlanStepOut::ThreadPlanStepOut(
 
 // Perform some additional validation on the return address.
 uint32_t permissions = 0;
+Log *log = GetLog(LLDBLog::Step);
 if (!m_process.GetLoadAddressPermissions(m_return_addr, permissions)) {
   LLDB_LOGF(log, "ThreadPlanStepOut(%p): Return address (0x%" PRIx64
 ") permissions not found.", static_cast(this),



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb][Minidump Parser] Implement a range data vector for minidump memory ranges (PR #136040)

2025-04-17 Thread Miro Bucko via lldb-commits


@@ -429,62 +429,64 @@ MinidumpParser::GetExceptionStreams() {
 
 std::optional
 MinidumpParser::FindMemoryRange(lldb::addr_t addr) {
-  Log *log = GetLog(LLDBLog::Modules);
+  if (m_memory_ranges.IsEmpty())
+PopulateMemoryRanges();
+
+  MemoryRangeVector::Entry *entry = 
m_memory_ranges.FindEntryThatContains(addr);

mbucko wrote:

const

https://github.com/llvm/llvm-project/pull/136040
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb][nfc] Split the constructor of ThreadPlanStepOut (PR #136160)

2025-04-17 Thread Felipe de Azevedo Piovezan via lldb-commits

https://github.com/felipepiovezan edited 
https://github.com/llvm/llvm-project/pull/136160
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb][nfc] Split the constructor of ThreadPlanStepOut (PR #136160)

2025-04-17 Thread Felipe de Azevedo Piovezan via lldb-commits

https://github.com/felipepiovezan updated 
https://github.com/llvm/llvm-project/pull/136160

>From 42850d46b705c239b099be8d102a3756a8fd5283 Mon Sep 17 00:00:00 2001
From: Felipe de Azevedo Piovezan 
Date: Tue, 15 Apr 2025 10:20:41 -0700
Subject: [PATCH 1/3] [lldb][nfc] Factor out code from ThreadPlanStepOut ctor

A future patch will need to create a new constructor for this class, and
extracting code out of its sole existing constructor will make this
easier.

This commit creates a helper function for the code computing the target
frame to step out to.
---
 lldb/source/Target/ThreadPlanStepOut.cpp | 47 +++-
 1 file changed, 29 insertions(+), 18 deletions(-)

diff --git a/lldb/source/Target/ThreadPlanStepOut.cpp 
b/lldb/source/Target/ThreadPlanStepOut.cpp
index a05c46db6b8ca..26c1abe710319 100644
--- a/lldb/source/Target/ThreadPlanStepOut.cpp
+++ b/lldb/source/Target/ThreadPlanStepOut.cpp
@@ -31,6 +31,32 @@ using namespace lldb_private;
 
 uint32_t ThreadPlanStepOut::s_default_flag_values = 0;
 
+/// Computes the target frame this plan should step out to.
+static StackFrameSP
+ComputeTargetFrame(Thread &thread, uint32_t start_frame_idx,
+   std::vector &skipped_frames) {
+  uint32_t frame_idx = start_frame_idx + 1;
+  StackFrameSP return_frame_sp = thread.GetStackFrameAtIndex(frame_idx);
+  if (!return_frame_sp)
+return nullptr;
+
+  while (return_frame_sp->IsArtificial() || return_frame_sp->IsHidden()) {
+skipped_frames.push_back(return_frame_sp);
+
+frame_idx++;
+return_frame_sp = thread.GetStackFrameAtIndex(frame_idx);
+
+// We never expect to see an artificial frame without a regular ancestor.
+// Defensively refuse to step out.
+if (!return_frame_sp) {
+  LLDB_LOG(GetLog(LLDBLog::Step),
+   "Can't step out of frame with artificial ancestors");
+  return nullptr;
+}
+  }
+  return return_frame_sp;
+}
+
 // ThreadPlanStepOut: Step out of the current frame
 ThreadPlanStepOut::ThreadPlanStepOut(
 Thread &thread, SymbolContext *context, bool first_insn, bool stop_others,
@@ -44,34 +70,18 @@ ThreadPlanStepOut::ThreadPlanStepOut(
   m_return_addr(LLDB_INVALID_ADDRESS), m_stop_others(stop_others),
   m_immediate_step_from_function(nullptr),
   m_calculate_return_value(gather_return_value) {
-  Log *log = GetLog(LLDBLog::Step);
   SetFlagsToDefault();
   SetupAvoidNoDebug(step_out_avoids_code_without_debug_info);
 
   m_step_from_insn = thread.GetRegisterContext()->GetPC(0);
 
-  uint32_t return_frame_index = frame_idx + 1;
-  StackFrameSP 
return_frame_sp(thread.GetStackFrameAtIndex(return_frame_index));
+  StackFrameSP return_frame_sp =
+  ComputeTargetFrame(thread, frame_idx, m_stepped_past_frames);
   StackFrameSP 
immediate_return_from_sp(thread.GetStackFrameAtIndex(frame_idx));
 
   if (!return_frame_sp || !immediate_return_from_sp)
 return; // we can't do anything here.  ValidatePlan() will return false.
 
-  // While stepping out, behave as-if artificial frames are not present.
-  while (return_frame_sp->IsArtificial() || return_frame_sp->IsHidden()) {
-m_stepped_past_frames.push_back(return_frame_sp);
-
-++return_frame_index;
-return_frame_sp = thread.GetStackFrameAtIndex(return_frame_index);
-
-// We never expect to see an artificial frame without a regular ancestor.
-// If this happens, log the issue and defensively refuse to step out.
-if (!return_frame_sp) {
-  LLDB_LOG(log, "Can't step out of frame with artificial ancestors");
-  return;
-}
-  }
-
   m_step_out_to_id = return_frame_sp->GetStackID();
   m_immediate_step_from_id = immediate_return_from_sp->GetStackID();
 
@@ -125,6 +135,7 @@ ThreadPlanStepOut::ThreadPlanStepOut(
 
 // Perform some additional validation on the return address.
 uint32_t permissions = 0;
+Log *log = GetLog(LLDBLog::Step);
 if (!m_process.GetLoadAddressPermissions(m_return_addr, permissions)) {
   LLDB_LOGF(log, "ThreadPlanStepOut(%p): Return address (0x%" PRIx64
 ") permissions not found.", static_cast(this),

>From 662baea555175e39ca4f0f170f2ef197c53af60d Mon Sep 17 00:00:00 2001
From: Felipe de Azevedo Piovezan 
Date: Thu, 17 Apr 2025 07:52:19 -0700
Subject: [PATCH 2/3] [lldb][nfc] Split the constructor of ThreadPlanStepOut

A subsequent commit will create a new constructor for ThreadPlanStepOut,
which needs to reuse much of the same logic of the existing constructor.
This commit places all of that reusable logic into a separate function.
---
 lldb/include/lldb/Target/ThreadPlanStepOut.h | 4 
 lldb/source/Target/ThreadPlanStepOut.cpp | 9 -
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/lldb/include/lldb/Target/ThreadPlanStepOut.h 
b/lldb/include/lldb/Target/ThreadPlanStepOut.h
index 013c675afc33d..e414a6e0a2d49 100644
--- a/lldb/include/lldb/Target/ThreadPlanStepOut.h
+++ b/lldb/include/lldb/Target/ThreadPlanStepOut.h
@@ -82,6 +82,10 @@ class ThreadPlan

[Lldb-commits] [lldb] [lldb] Add symbol/table count into statistics (PR #136226)

2025-04-17 Thread via lldb-commits

royitaqi wrote:

Will wait until tomorrow morning/noon before landing, so that:
1. Give other folks a chance to review before merge.
2. Avoid failing tests after working hour so that folks can sleep.

https://github.com/llvm/llvm-project/pull/136226
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb] Add symbol/table count into statistics (PR #136226)

2025-04-17 Thread via lldb-commits

https://github.com/royitaqi created 
https://github.com/llvm/llvm-project/pull/136226

These stats are added or changed:
1. In summary:
1. Add `totalSymbolsLoaded`. The total number of symbols loaded in all 
modules.
2. Add `totalSymbolTablesLoaded `. The total number symbol tables loaded in 
all modules.
2. In each module's stats:
1. Add `symbolsLoaded`. The number of symbols loaded in the current module.

--

Still running tests. Will update here.

>From 23800e3979ff321530f98a61e9d2375b3426ddda Mon Sep 17 00:00:00 2001
From: Roy Shi 
Date: Wed, 16 Apr 2025 09:19:04 -0700
Subject: [PATCH] [lldb] Add new stats (# symbols loaded, # symbol tables
 loaded) to module and target

Summary:

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:

Differential Revision: https://phabricator.intern.facebook.com/D73117709
---
 lldb/include/lldb/Target/Statistics.h |  1 +
 lldb/source/Target/Statistics.cpp | 18 +++
 .../commands/statistics/basic/TestStats.py| 23 +++
 3 files changed, 32 insertions(+), 10 deletions(-)

diff --git a/lldb/include/lldb/Target/Statistics.h 
b/lldb/include/lldb/Target/Statistics.h
index ee365357fcf31..b87a12a8ab9cd 100644
--- a/lldb/include/lldb/Target/Statistics.h
+++ b/lldb/include/lldb/Target/Statistics.h
@@ -120,6 +120,7 @@ struct ModuleStats {
   llvm::StringMap type_system_stats;
   double symtab_parse_time = 0.0;
   double symtab_index_time = 0.0;
+  uint32_t num_symbols_loaded = 0;
   double debug_parse_time = 0.0;
   double debug_index_time = 0.0;
   uint64_t debug_info_size = 0;
diff --git a/lldb/source/Target/Statistics.cpp 
b/lldb/source/Target/Statistics.cpp
index b5d2e7bda1edf..f70fb529544a5 100644
--- a/lldb/source/Target/Statistics.cpp
+++ b/lldb/source/Target/Statistics.cpp
@@ -71,6 +71,7 @@ json::Value ModuleStats::ToJSON() const {
   module.try_emplace("debugInfoHadIncompleteTypes",
  debug_info_had_incomplete_types);
   module.try_emplace("symbolTableStripped", symtab_stripped);
+  module.try_emplace("symbolsLoaded", num_symbols_loaded);
   if (!symfile_path.empty())
 module.try_emplace("symbolFilePath", symfile_path);
 
@@ -293,7 +294,8 @@ llvm::json::Value DebuggerStats::ReportStatistics(
   double debug_parse_time = 0.0;
   double debug_index_time = 0.0;
   uint32_t symtabs_loaded = 0;
-  uint32_t symtabs_saved = 0;
+  uint32_t symtabs_loaded_from_cache = 0;
+  uint32_t symtabs_saved_to_cache = 0;
   uint32_t debug_index_loaded = 0;
   uint32_t debug_index_saved = 0;
   uint64_t debug_info_size = 0;
@@ -309,6 +311,7 @@ llvm::json::Value DebuggerStats::ReportStatistics(
   uint32_t num_modules_with_variable_errors = 0;
   uint32_t num_modules_with_incomplete_types = 0;
   uint32_t num_stripped_modules = 0;
+  uint32_t num_symbols_loaded = 0;
   for (size_t image_idx = 0; image_idx < num_modules; ++image_idx) {
 Module *module = target != nullptr
  ? 
target->GetImages().GetModuleAtIndex(image_idx).get()
@@ -318,12 +321,15 @@ llvm::json::Value DebuggerStats::ReportStatistics(
 module_stat.symtab_index_time = module->GetSymtabIndexTime().get().count();
 Symtab *symtab = module->GetSymtab(/*can_create=*/false);
 if (symtab) {
+  module_stat.num_symbols_loaded = symtab->GetNumSymbols();
+  num_symbols_loaded += module_stat.num_symbols_loaded;
+  symtabs_loaded++;
   module_stat.symtab_loaded_from_cache = symtab->GetWasLoadedFromCache();
   if (module_stat.symtab_loaded_from_cache)
-++symtabs_loaded;
+++symtabs_loaded_from_cache;
   module_stat.symtab_saved_to_cache = symtab->GetWasSavedToCache();
   if (module_stat.symtab_saved_to_cache)
-++symtabs_saved;
+++symtabs_saved_to_cache;
 }
 SymbolFile *sym_file = module->GetSymbolFile(/*can_create=*/false);
 if (sym_file) {
@@ -393,8 +399,9 @@ llvm::json::Value DebuggerStats::ReportStatistics(
   json::Object global_stats{
   {"totalSymbolTableParseTime", symtab_parse_time},
   {"totalSymbolTableIndexTime", symtab_index_time},
-  {"totalSymbolTablesLoadedFromCache", symtabs_loaded},
-  {"totalSymbolTablesSavedToCache", symtabs_saved},
+  {"totalSymbolTablesLoaded", symtabs_loaded},
+  {"totalSymbolTablesLoadedFromCache", symtabs_loaded_from_cache},
+  {"totalSymbolTablesSavedToCache", symtabs_saved_to_cache},
   {"totalDebugInfoParseTime", debug_parse_time},
   {"totalDebugInfoIndexTime", debug_index_time},
   {"totalDebugInfoIndexLoadedFromCache", debug_index_loaded},
@@ -407,6 +414,7 @@ llvm::json::Value DebuggerStats::ReportStatistics(
num_modules_with_incomplete_types},
   {"totalDebugInfoEnabled", num_debug_info_enabled_modules},
   {"totalSymbolTableStripped", num_stripped_modules},
+  {"totalSymbolsLoaded", num_symbols_loaded},
   };
 
   if (include_targets) {
diff --git a/lldb/test/API/commands/statistics/basic/TestStats.py 
b/lldb/test/API/commands/statistics/basic

[Lldb-commits] [lldb] [lldb] returning command completions up to a maximum (PR #135565)

2025-04-17 Thread Jacob Lalonde via lldb-commits


@@ -157,6 +162,23 @@ class CompletionRequest {
 
   size_t GetCursorIndex() const { return m_cursor_index; }
 
+  size_t GetMaxReturnElements() const { return m_max_return_elements; }
+
+  /// Returns true if the maximum number of completions has not been reached
+  /// yet, hence we should keep adding completions.
+  bool ShouldAddCompletions() const {
+return m_result.GetNumberOfResults() < m_max_return_elements;
+  }
+
+  /// Returns the maximum number of completions that need to be added
+  /// until reaching the maximum
+  size_t GetMaxNumberOfResultsToAdd() const {
+const size_t number_of_results = m_result.GetNumberOfResults();
+if (number_of_results >= m_max_return_elements)

Jlalond wrote:

Shouldn't this just call `ShouldAddCompletions()` and return 0 if false?

https://github.com/llvm/llvm-project/pull/135565
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb] Add symbol/table count into statistics (PR #136226)

2025-04-17 Thread via lldb-commits

https://github.com/royitaqi edited 
https://github.com/llvm/llvm-project/pull/136226
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] disable test on older compilers (PR #136186)

2025-04-17 Thread Shubham Sandeep Rastogi via lldb-commits

https://github.com/rastogishubham closed 
https://github.com/llvm/llvm-project/pull/136186
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb] Add symbol/table count into statistics (PR #136226)

2025-04-17 Thread David Peixotto via lldb-commits


@@ -318,12 +321,15 @@ llvm::json::Value DebuggerStats::ReportStatistics(
 module_stat.symtab_index_time = module->GetSymtabIndexTime().get().count();
 Symtab *symtab = module->GetSymtab(/*can_create=*/false);
 if (symtab) {
+  module_stat.num_symbols_loaded = symtab->GetNumSymbols();
+  num_symbols_loaded += module_stat.num_symbols_loaded;
+  symtabs_loaded++;

dmpots wrote:

Nit: The other increments use a pre-increment. Would be nice to be consistent 
and use it here too.

https://github.com/llvm/llvm-project/pull/136226
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb] Add symbol/table count into statistics (PR #136226)

2025-04-17 Thread via lldb-commits

https://github.com/royitaqi edited 
https://github.com/llvm/llvm-project/pull/136226
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] Add download time for each module in statistics (PR #134563)

2025-04-17 Thread via lldb-commits

https://github.com/GeorgeHuyubo updated 
https://github.com/llvm/llvm-project/pull/134563

>From 28e52ec264f088625410b7202b5de1e7f601f164 Mon Sep 17 00:00:00 2001
From: George Hu 
Date: Tue, 8 Apr 2025 18:12:29 -0700
Subject: [PATCH] Add symbol locator time for each module in statistics

Summary:

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:

Differential Revision: https://phabricator.intern.facebook.com/D73224269
---
 lldb/include/lldb/Core/Module.h   | 21 +++
 lldb/include/lldb/Core/PluginManager.h|  7 +++--
 lldb/include/lldb/Target/Statistics.h |  1 +
 lldb/source/Core/DynamicLoader.cpp| 26 ---
 lldb/source/Core/ModuleList.cpp   | 13 +++---
 lldb/source/Core/PluginManager.cpp| 16 +---
 .../Platform/MacOSX/PlatformDarwinKernel.cpp  | 12 +++--
 .../Process/MacOSX-Kernel/ProcessKDP.cpp  | 26 +++
 .../SymbolFile/DWARF/SymbolFileDWARF.cpp  | 18 ++---
 .../SymbolVendor/ELF/SymbolVendorELF.cpp  | 25 ++
 .../MacOSX/SymbolVendorMacOSX.cpp | 12 +++--
 .../PECOFF/SymbolVendorPECOFF.cpp | 12 +++--
 .../SymbolVendor/wasm/SymbolVendorWasm.cpp| 12 +++--
 lldb/source/Target/Statistics.cpp | 20 ++
 14 files changed, 186 insertions(+), 35 deletions(-)

diff --git a/lldb/include/lldb/Core/Module.h b/lldb/include/lldb/Core/Module.h
index 1ad67d6747850..1c872f922e30a 100644
--- a/lldb/include/lldb/Core/Module.h
+++ b/lldb/include/lldb/Core/Module.h
@@ -89,6 +89,7 @@ class Module : public std::enable_shared_from_this,
public SymbolContextScope {
 public:
   class LookupInfo;
+  class StatisticsMap;
   // Static functions that can track the lifetime of module objects. This is
   // handy because we might have Module objects that are in shared pointers
   // that aren't in the global module list (from ModuleList). If this is the
@@ -885,6 +886,10 @@ class Module : public std::enable_shared_from_this,
   /// ElapsedTime RAII object.
   StatsDuration &GetSymtabIndexTime() { return m_symtab_index_time; }
 
+  StatisticsMap &GetSymbolLocatorStatistics() {
+return m_symbol_locator_duration_map;
+  }
+
   void ResetStatistics();
 
   /// \class LookupInfo Module.h "lldb/Core/Module.h"
@@ -956,6 +961,20 @@ class Module : public std::enable_shared_from_this,
 bool m_match_name_after_lookup = false;
   };
 
+  class StatisticsMap {
+  public:
+void add(const std::string &key, double value) {
+  if (key.empty())
+return;
+  auto it = map.find(key);
+  if (it == map.end())
+map.try_emplace(key, value);
+  else
+it->second += value;
+}
+std::unordered_map map;
+  };
+
   /// Get a unique hash for this module.
   ///
   /// The hash should be enough to identify the file on disk and the
@@ -1064,6 +1083,8 @@ class Module : public 
std::enable_shared_from_this,
   /// time for the symbol tables can be aggregated here.
   StatsDuration m_symtab_index_time;
 
+  StatisticsMap m_symbol_locator_duration_map;
+
   /// A set of hashes of all warnings and errors, to avoid reporting them
   /// multiple times to the same Debugger.
   llvm::DenseMap>
diff --git a/lldb/include/lldb/Core/PluginManager.h 
b/lldb/include/lldb/Core/PluginManager.h
index a6dab045adf27..dfa7bfa6eb9da 100644
--- a/lldb/include/lldb/Core/PluginManager.h
+++ b/lldb/include/lldb/Core/PluginManager.h
@@ -377,11 +377,14 @@ class PluginManager {
   static SymbolLocatorCreateInstance
   GetSymbolLocatorCreateCallbackAtIndex(uint32_t idx);
 
-  static ModuleSpec LocateExecutableObjectFile(const ModuleSpec &module_spec);
+  static ModuleSpec
+  LocateExecutableObjectFile(const ModuleSpec &module_spec,
+ std::string *locator_name = nullptr);
 
   static FileSpec
   LocateExecutableSymbolFile(const ModuleSpec &module_spec,
- const FileSpecList &default_search_paths);
+ const FileSpecList &default_search_paths,
+ std::string *locator_name = nullptr);
 
   static bool DownloadObjectAndSymbolFile(ModuleSpec &module_spec,
   Status &error,
diff --git a/lldb/include/lldb/Target/Statistics.h 
b/lldb/include/lldb/Target/Statistics.h
index ee365357fcf31..2b60931f065f7 100644
--- a/lldb/include/lldb/Target/Statistics.h
+++ b/lldb/include/lldb/Target/Statistics.h
@@ -122,6 +122,7 @@ struct ModuleStats {
   double symtab_index_time = 0.0;
   double debug_parse_time = 0.0;
   double debug_index_time = 0.0;
+  llvm::StringMap symbol_locator_time;
   uint64_t debug_info_size = 0;
   bool symtab_loaded_from_cache = false;
   bool symtab_saved_to_cache = false;
diff --git a/lldb/source/Core/DynamicLoader.cpp 
b/lldb/source/Core/DynamicLoader.cpp
index 76c71d2a49a48..9bf5df7f86d25 100644
--- a/lldb/source/Core/DynamicLoader.cpp
+++ b/lldb/source/Core

[Lldb-commits] [lldb] Add download time for each module in statistics (PR #134563)

2025-04-17 Thread via lldb-commits

https://github.com/GeorgeHuyubo reopened 
https://github.com/llvm/llvm-project/pull/134563
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] Add symbol locator time for each module in statistics (PR #134563)

2025-04-17 Thread via lldb-commits

https://github.com/GeorgeHuyubo edited 
https://github.com/llvm/llvm-project/pull/134563
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb][Mach-O corefiles] Don't init Target arch to corefile (PR #136065)

2025-04-17 Thread Jonas Devlieghere via lldb-commits

https://github.com/JDevlieghere approved this pull request.

LGTM modulo Jim's questions/suggestions. 

https://github.com/llvm/llvm-project/pull/136065
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb][Mach-O corefiles] Don't init Target arch to corefile (PR #136065)

2025-04-17 Thread Jonas Devlieghere via lldb-commits


@@ -578,20 +605,47 @@ Status ProcessMachCore::DoLoadCore() {
 
   SetCanJIT(false);
 
+  // If we have an executable binary in the Target already,
+  // use that to set the Target's ArchSpec.
+  //
+  // Don't initialize the ArchSpec based on the corefile's cputype/cpusubtype
+  // here, the corefile creator may not know the correct subtype of the code
+  // that is executing, initialize the Target to that, and if the
+  // main binary has Python code which initializes based on the Target arch,
+  // get the wrong subtype value.
+  ModuleSP exe_module_sp = GetTarget().GetExecutableModule();
+  if (exe_module_sp && exe_module_sp->GetArchitecture().IsValid()) {
+LLDB_LOGF(log,
+  "ProcessMachCore::%s: Was given binary + corefile, setting "
+  "target ArchSpec to binary to start",
+  __FUNCTION__);
+GetTarget().SetArchitecture(exe_module_sp->GetArchitecture());
+  }
+
   CreateMemoryRegions();
 
   LoadBinariesAndSetDYLD();
 
   CleanupMemoryRegionPermissions();
 
-  ModuleSP exe_module_sp = GetTarget().GetExecutableModule();
+  exe_module_sp = GetTarget().GetExecutableModule();

JDevlieghere wrote:

Plus, if we really need to do it twice, better to move this into a helper 
method. 

https://github.com/llvm/llvm-project/pull/136065
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb] Fix lock inversion between statusline mutex and output mutex (PR #135956)

2025-04-17 Thread Jonas Devlieghere via lldb-commits

https://github.com/JDevlieghere closed 
https://github.com/llvm/llvm-project/pull/135956
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb] Consider "hidden" frames in ThreadPlanShouldStopHere (PR #131800)

2025-04-17 Thread Felipe de Azevedo Piovezan via lldb-commits

https://github.com/felipepiovezan closed 
https://github.com/llvm/llvm-project/pull/131800
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb][Mach-O corefiles] Don't init Target arch to corefile (PR #136065)

2025-04-17 Thread Jason Molenda via lldb-commits


@@ -578,20 +605,47 @@ Status ProcessMachCore::DoLoadCore() {
 
   SetCanJIT(false);
 
+  // If we have an executable binary in the Target already,
+  // use that to set the Target's ArchSpec.
+  //
+  // Don't initialize the ArchSpec based on the corefile's cputype/cpusubtype
+  // here, the corefile creator may not know the correct subtype of the code
+  // that is executing, initialize the Target to that, and if the
+  // main binary has Python code which initializes based on the Target arch,
+  // get the wrong subtype value.
+  ModuleSP exe_module_sp = GetTarget().GetExecutableModule();
+  if (exe_module_sp && exe_module_sp->GetArchitecture().IsValid()) {
+LLDB_LOGF(log,
+  "ProcessMachCore::%s: Was given binary + corefile, setting "
+  "target ArchSpec to binary to start",
+  __FUNCTION__);
+GetTarget().SetArchitecture(exe_module_sp->GetArchitecture());
+  }
+
   CreateMemoryRegions();
 
   LoadBinariesAndSetDYLD();
 
   CleanupMemoryRegionPermissions();
 
-  ModuleSP exe_module_sp = GetTarget().GetExecutableModule();
+  exe_module_sp = GetTarget().GetExecutableModule();

jasonmolenda wrote:

Before we call `LoadBinariesAndSetDYLD()`, the only way we have an executable 
module is if the user provided it on the commandline along with the corefile.  
After that method call, we may have found it by metadata in the corefile or by 
an exhaustive search for a properly-aligned dyld or kernel.  I'm open to having 
this in a method, but it's also a tiny method "if we have an executable binary, 
set the Target's arch to that ArchSpec" - the logging I added here is the 
largest part.  

The real opportunity for reducing a copy of code would be in 
ProcessMachCore::LoadBinariesViaExhaustiveSearch where it does the "if we have 
an executable module, set arch, else use the corefile's arch" but we already 
know we don't have an executable module.  We checked it before we got here in 
DoLoadCore, and if we'd added an executable module at this point, we wouldn't 
be doing the exhaustive search.  This bit could be reduced to simply "if target 
has no arch, set it to the corefile's arch", I thought of that last night.

https://github.com/llvm/llvm-project/pull/136065
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb][nfc] Add customization flags for ThreadPlanStepOut (PR #135866)

2025-04-17 Thread Felipe de Azevedo Piovezan via lldb-commits

https://github.com/felipepiovezan closed 
https://github.com/llvm/llvm-project/pull/135866
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb] Add symbol/table count into statistics (PR #136226)

2025-04-17 Thread via lldb-commits

github-actions[bot] wrote:




:warning: Python code formatter, darker found issues in your code. :warning:



You can test this locally with the following command:


``bash
darker --check --diff -r HEAD~1...HEAD 
lldb/test/API/commands/statistics/basic/TestStats.py
``





View the diff from darker here.


``diff
--- TestStats.py2025-04-17 22:57:56.00 +
+++ TestStats.py2025-04-17 23:37:50.537203 +
@@ -176,30 +176,33 @@
 "totalDebugInfoIndexLoadedFromCache",
 "totalDebugInfoIndexSavedToCache",
 "totalDebugInfoParseTime",
 ]
 self.verify_keys(debug_stats, '"debug_stats"', debug_stat_keys, None)
-
+
 # Verify target stats keys.
 target_stats = debug_stats["targets"][0]
 target_stat_keys_exist = [
 "expressionEvaluation",
 "frameVariable",
 "moduleIdentifiers",
 "targetCreateTime",
 ]
 target_stat_keys_missing = ["firstStopTime", "launchOrAttachTime"]
-self.verify_keys(target_stats, '"target_stats"', target_stat_keys, 
target_stat_keys_missing)
+self.verify_keys(
+target_stats, '"target_stats"', target_stat_keys, 
target_stat_keys_missing
+)
 self.assertGreater(target_stats["targetCreateTime"], 0.0)
 
 # Verify module stats keys.
 for module_stats in debug_stats["modules"]:
 module_stat_keys_exist = [
 "symbolsLoaded",
 ]
-self.verify_keys(module_stats, '"module_stats"', 
module_stat_keys_exist, None)
-
+self.verify_keys(
+module_stats, '"module_stats"', module_stat_keys_exist, None
+)
 
 def test_default_with_run(self):
 """Test "statistics dump" when running the target to a breakpoint.
 
 When we run the target, we expect to see 'launchOrAttachTime' and

``




https://github.com/llvm/llvm-project/pull/136226
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb] Add symbol/table count into statistics (PR #136226)

2025-04-17 Thread via lldb-commits

https://github.com/royitaqi edited 
https://github.com/llvm/llvm-project/pull/136226
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb] Add symbol/table count into statistics (PR #136226)

2025-04-17 Thread via lldb-commits

https://github.com/royitaqi updated 
https://github.com/llvm/llvm-project/pull/136226

>From 4157b0d1426830a4c19af7401728ef86dbc2d8c9 Mon Sep 17 00:00:00 2001
From: Roy Shi 
Date: Wed, 16 Apr 2025 09:19:04 -0700
Subject: [PATCH] [lldb] Add new stats (# symbols loaded, # symbol tables
 loaded) to module and target

Summary:

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:

Differential Revision: https://phabricator.intern.facebook.com/D73117709
---
 lldb/include/lldb/Target/Statistics.h |  1 +
 lldb/source/Target/Statistics.cpp | 18 +++
 .../commands/statistics/basic/TestStats.py| 23 +++
 3 files changed, 32 insertions(+), 10 deletions(-)

diff --git a/lldb/include/lldb/Target/Statistics.h 
b/lldb/include/lldb/Target/Statistics.h
index ee365357fcf31..b87a12a8ab9cd 100644
--- a/lldb/include/lldb/Target/Statistics.h
+++ b/lldb/include/lldb/Target/Statistics.h
@@ -120,6 +120,7 @@ struct ModuleStats {
   llvm::StringMap type_system_stats;
   double symtab_parse_time = 0.0;
   double symtab_index_time = 0.0;
+  uint32_t num_symbols_loaded = 0;
   double debug_parse_time = 0.0;
   double debug_index_time = 0.0;
   uint64_t debug_info_size = 0;
diff --git a/lldb/source/Target/Statistics.cpp 
b/lldb/source/Target/Statistics.cpp
index b5d2e7bda1edf..f70fb529544a5 100644
--- a/lldb/source/Target/Statistics.cpp
+++ b/lldb/source/Target/Statistics.cpp
@@ -71,6 +71,7 @@ json::Value ModuleStats::ToJSON() const {
   module.try_emplace("debugInfoHadIncompleteTypes",
  debug_info_had_incomplete_types);
   module.try_emplace("symbolTableStripped", symtab_stripped);
+  module.try_emplace("symbolsLoaded", num_symbols_loaded);
   if (!symfile_path.empty())
 module.try_emplace("symbolFilePath", symfile_path);
 
@@ -293,7 +294,8 @@ llvm::json::Value DebuggerStats::ReportStatistics(
   double debug_parse_time = 0.0;
   double debug_index_time = 0.0;
   uint32_t symtabs_loaded = 0;
-  uint32_t symtabs_saved = 0;
+  uint32_t symtabs_loaded_from_cache = 0;
+  uint32_t symtabs_saved_to_cache = 0;
   uint32_t debug_index_loaded = 0;
   uint32_t debug_index_saved = 0;
   uint64_t debug_info_size = 0;
@@ -309,6 +311,7 @@ llvm::json::Value DebuggerStats::ReportStatistics(
   uint32_t num_modules_with_variable_errors = 0;
   uint32_t num_modules_with_incomplete_types = 0;
   uint32_t num_stripped_modules = 0;
+  uint32_t num_symbols_loaded = 0;
   for (size_t image_idx = 0; image_idx < num_modules; ++image_idx) {
 Module *module = target != nullptr
  ? 
target->GetImages().GetModuleAtIndex(image_idx).get()
@@ -318,12 +321,15 @@ llvm::json::Value DebuggerStats::ReportStatistics(
 module_stat.symtab_index_time = module->GetSymtabIndexTime().get().count();
 Symtab *symtab = module->GetSymtab(/*can_create=*/false);
 if (symtab) {
+  module_stat.num_symbols_loaded = symtab->GetNumSymbols();
+  num_symbols_loaded += module_stat.num_symbols_loaded;
+  symtabs_loaded++;
   module_stat.symtab_loaded_from_cache = symtab->GetWasLoadedFromCache();
   if (module_stat.symtab_loaded_from_cache)
-++symtabs_loaded;
+++symtabs_loaded_from_cache;
   module_stat.symtab_saved_to_cache = symtab->GetWasSavedToCache();
   if (module_stat.symtab_saved_to_cache)
-++symtabs_saved;
+++symtabs_saved_to_cache;
 }
 SymbolFile *sym_file = module->GetSymbolFile(/*can_create=*/false);
 if (sym_file) {
@@ -393,8 +399,9 @@ llvm::json::Value DebuggerStats::ReportStatistics(
   json::Object global_stats{
   {"totalSymbolTableParseTime", symtab_parse_time},
   {"totalSymbolTableIndexTime", symtab_index_time},
-  {"totalSymbolTablesLoadedFromCache", symtabs_loaded},
-  {"totalSymbolTablesSavedToCache", symtabs_saved},
+  {"totalSymbolTablesLoaded", symtabs_loaded},
+  {"totalSymbolTablesLoadedFromCache", symtabs_loaded_from_cache},
+  {"totalSymbolTablesSavedToCache", symtabs_saved_to_cache},
   {"totalDebugInfoParseTime", debug_parse_time},
   {"totalDebugInfoIndexTime", debug_index_time},
   {"totalDebugInfoIndexLoadedFromCache", debug_index_loaded},
@@ -407,6 +414,7 @@ llvm::json::Value DebuggerStats::ReportStatistics(
num_modules_with_incomplete_types},
   {"totalDebugInfoEnabled", num_debug_info_enabled_modules},
   {"totalSymbolTableStripped", num_stripped_modules},
+  {"totalSymbolsLoaded", num_symbols_loaded},
   };
 
   if (include_targets) {
diff --git a/lldb/test/API/commands/statistics/basic/TestStats.py 
b/lldb/test/API/commands/statistics/basic/TestStats.py
index 54881c13bcb68..1ecbe89e3b3d4 100644
--- a/lldb/test/API/commands/statistics/basic/TestStats.py
+++ b/lldb/test/API/commands/statistics/basic/TestStats.py
@@ -159,6 +159,8 @@ def test_default_no_run(self):
 """
 self.build()
 target = self.createTestTarget()
+
+# Verify top-level keys.
 debug_stats = self.get_stats()
  

[Lldb-commits] [lldb] [lldb] Add symbol/table count into statistics (PR #136226)

2025-04-17 Thread David Peixotto via lldb-commits


@@ -168,23 +170,34 @@ def test_default_no_run(self):
 "totalSymbolTableIndexTime",
 "totalSymbolTablesLoadedFromCache",
 "totalSymbolTablesSavedToCache",
+"totalSymbolsLoaded",

dmpots wrote:

We should also check for `totalSymbolTablesLoaded` too, right?

https://github.com/llvm/llvm-project/pull/136226
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb] Try to fix TestExprDiagnostics test (PR #136269)

2025-04-17 Thread Timm Baeder via lldb-commits

https://github.com/tbaederr updated 
https://github.com/llvm/llvm-project/pull/136269

>From 080087a0b5b8a399aa633d1c1433f28ae83398c3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Timm=20B=C3=A4der?= 
Date: Fri, 18 Apr 2025 08:43:37 +0200
Subject: [PATCH] [lldb] Try to fix TestExprDiagnostics test

---
 .../commands/expression/diagnostics/TestExprDiagnostics.py| 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git 
a/lldb/test/API/commands/expression/diagnostics/TestExprDiagnostics.py 
b/lldb/test/API/commands/expression/diagnostics/TestExprDiagnostics.py
index b476a807c6dc0..06cb758bc62d4 100644
--- a/lldb/test/API/commands/expression/diagnostics/TestExprDiagnostics.py
+++ b/lldb/test/API/commands/expression/diagnostics/TestExprDiagnostics.py
@@ -57,7 +57,7 @@ def test_source_and_caret_printing(self):
 self.assertIn(
 """
 2 | foobar +=1;
-  | ^
+  | ^~
 """,
 value.GetError().GetCString(),
 )
@@ -74,7 +74,7 @@ def test_source_and_caret_printing(self):
 self.assertIn(
 """
 1 | void foo(unknown_type x) {}
-  |  ^
+  |  ^~~~
 """,
 value.GetError().GetCString(),
 )

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb][Telemetry] Fix unit test compile failure with LLVM_ENABLE_TELEMETRY=0 (PR #136115)

2025-04-17 Thread via lldb-commits

llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: Raul Tambre (tambry)


Changes

It needs to be `TEST_F` to access `received_entries`.
Disabling also works based on the test not the fixture name.

Build failure:
```
lldb/unittests/Core/TelemetryTest.cpp:110:17: error: use of undeclared 
identifier 'received_entries'
  110 |   ASSERT_EQ(1U, received_entries.size());
  | ^
lldb/unittests/Core/TelemetryTest.cpp:112:61: error: use of undeclared 
identifier 'received_entries'
  112 | 
llvm::dyn_cast(received_entries[0])
  | ^
```

Fixes: 159b872b37363511a359c800bcc9230bb09f2457

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


1 Files Affected:

- (modified) lldb/unittests/Core/TelemetryTest.cpp (+1-1) 


``diff
diff --git a/lldb/unittests/Core/TelemetryTest.cpp 
b/lldb/unittests/Core/TelemetryTest.cpp
index 1e41424bac3ce..910149d865c13 100644
--- a/lldb/unittests/Core/TelemetryTest.cpp
+++ b/lldb/unittests/Core/TelemetryTest.cpp
@@ -96,7 +96,7 @@ class TelemetryTest : public testing::Test {
 #if LLVM_ENABLE_TELEMETRY
 #define TELEMETRY_TEST(suite, test) TEST_F(suite, test)
 #else
-#define TELEMETRY_TEST(suite, test) TEST(DISABLED_##suite, test)
+#define TELEMETRY_TEST(suite, test) TEST_F(suite, DISABLED_##test)
 #endif
 
 TELEMETRY_TEST(TelemetryTest, PluginTest) {

``




https://github.com/llvm/llvm-project/pull/136115
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] f135ce6 - [lldb] Remove CompilerType::GetIndexOfFieldWithName (#135963)

2025-04-17 Thread via lldb-commits

Author: Charles Zablit
Date: 2025-04-17T12:55:18+01:00
New Revision: f135ce6a9329124f1e508fcad2eedf5326a668fa

URL: 
https://github.com/llvm/llvm-project/commit/f135ce6a9329124f1e508fcad2eedf5326a668fa
DIFF: 
https://github.com/llvm/llvm-project/commit/f135ce6a9329124f1e508fcad2eedf5326a668fa.diff

LOG: [lldb] Remove CompilerType::GetIndexOfFieldWithName (#135963)

This patch removes the unused `CompilerType::GetIndexOfFieldWithName` API (it 
wasn't used apart from in a single testcase). Given we have so many similarly 
named APIs already, it's best not to maintain this API that's not really used 
(and isnt tested).

Added: 


Modified: 
lldb/include/lldb/Symbol/CompilerType.h
lldb/source/Symbol/CompilerType.cpp
lldb/unittests/Platform/PlatformSiginfoTest.cpp

Removed: 




diff  --git a/lldb/include/lldb/Symbol/CompilerType.h 
b/lldb/include/lldb/Symbol/CompilerType.h
index 41a1676dabd76..3561bc70887e6 100644
--- a/lldb/include/lldb/Symbol/CompilerType.h
+++ b/lldb/include/lldb/Symbol/CompilerType.h
@@ -433,12 +433,6 @@ class CompilerType {
 
   CompilerDecl GetStaticFieldWithName(llvm::StringRef name) const;
 
-  uint32_t GetIndexOfFieldWithName(const char *name,
-   CompilerType *field_compiler_type = nullptr,
-   uint64_t *bit_offset_ptr = nullptr,
-   uint32_t *bitfield_bit_size_ptr = nullptr,
-   bool *is_bitfield_ptr = nullptr) const;
-
   llvm::Expected GetChildCompilerTypeAtIndex(
   ExecutionContext *exe_ctx, size_t idx, bool transparent_pointers,
   bool omit_empty_base_classes, bool ignore_array_bounds,

diff  --git a/lldb/source/Symbol/CompilerType.cpp 
b/lldb/source/Symbol/CompilerType.cpp
index 22fdd24fc7cd5..8e89d006d08d3 100644
--- a/lldb/source/Symbol/CompilerType.cpp
+++ b/lldb/source/Symbol/CompilerType.cpp
@@ -893,25 +893,6 @@ CompilerDecl 
CompilerType::GetStaticFieldWithName(llvm::StringRef name) const {
   return CompilerDecl();
 }
 
-uint32_t CompilerType::GetIndexOfFieldWithName(
-const char *name, CompilerType *field_compiler_type_ptr,
-uint64_t *bit_offset_ptr, uint32_t *bitfield_bit_size_ptr,
-bool *is_bitfield_ptr) const {
-  unsigned count = GetNumFields();
-  std::string field_name;
-  for (unsigned index = 0; index < count; index++) {
-CompilerType field_compiler_type(
-GetFieldAtIndex(index, field_name, bit_offset_ptr,
-bitfield_bit_size_ptr, is_bitfield_ptr));
-if (strcmp(field_name.c_str(), name) == 0) {
-  if (field_compiler_type_ptr)
-*field_compiler_type_ptr = field_compiler_type;
-  return index;
-}
-  }
-  return UINT32_MAX;
-}
-
 llvm::Expected CompilerType::GetChildCompilerTypeAtIndex(
 ExecutionContext *exe_ctx, size_t idx, bool transparent_pointers,
 bool omit_empty_base_classes, bool ignore_array_bounds,

diff  --git a/lldb/unittests/Platform/PlatformSiginfoTest.cpp 
b/lldb/unittests/Platform/PlatformSiginfoTest.cpp
index 4b2c93a68a94a..b9f1fe4bbaadf 100644
--- a/lldb/unittests/Platform/PlatformSiginfoTest.cpp
+++ b/lldb/unittests/Platform/PlatformSiginfoTest.cpp
@@ -60,9 +60,11 @@ class PlatformSiginfoTest : public ::testing::Test {
 uint64_t total_offset = 0;
 for (auto field_name : llvm::split(path, '.')) {
   uint64_t bit_offset;
-  ASSERT_NE(field_type.GetIndexOfFieldWithName(field_name.str().c_str(),
-   &field_type, &bit_offset),
-UINT32_MAX);
+  std::string name;
+  field_type = field_type.GetFieldAtIndex(
+  field_type.GetIndexOfChildWithName(field_name, false), name,
+  &bit_offset, nullptr, nullptr);
+  ASSERT_TRUE(field_type);
   total_offset += bit_offset;
 }
 



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb] Remove CompilerType::GetIndexOfFieldWithName (PR #135963)

2025-04-17 Thread via lldb-commits

github-actions[bot] wrote:



@charles-zablit Congratulations on having your first Pull Request (PR) merged 
into the LLVM Project!

Your changes will be combined with recent changes from other authors, then 
tested by our [build bots](https://lab.llvm.org/buildbot/). If there is a 
problem with a build, you may receive a report in an email or a comment on this 
PR.

Please check whether problems have been caused by your change specifically, as 
the builds can include changes from many authors. It is not uncommon for your 
change to be included in a build that fails due to someone else's changes, or 
infrastructure issues.

How to do this, and the rest of the post-merge process, is covered in detail 
[here](https://llvm.org/docs/MyFirstTypoFix.html#myfirsttypofix-issues-after-landing-your-pr).

If your change does cause a problem, it may be reverted, or you can revert it 
yourself. This is a normal part of [LLVM 
development](https://llvm.org/docs/DeveloperPolicy.html#patch-reversion-policy).
 You can fix your changes and open a new PR to merge them again.

If you don't get any reports, no action is required from you. Your changes are 
working as expected, well done!


https://github.com/llvm/llvm-project/pull/135963
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb] Remove CompilerType::GetIndexOfFieldWithName (PR #135963)

2025-04-17 Thread Michael Buch via lldb-commits

https://github.com/Michael137 closed 
https://github.com/llvm/llvm-project/pull/135963
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [clang] [clang-tools-extra] [lldb] Reland [clang] Unify `SourceLocation` and `IdentifierInfo*` pair-like data structures to `IdentifierLoc` (PR #136077)

2025-04-17 Thread via lldb-commits

yronglin wrote:

Thanks for the review!

https://github.com/llvm/llvm-project/pull/136077
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb] Remove CompilerType::GetIndexOfFieldWithName (PR #135963)

2025-04-17 Thread Charles Zablit via lldb-commits

charles-zablit wrote:

> I think the test can just do this:
> 
> ```
>   uint64_t bit_offset;
>   std::string name;
>   field_type = field_type.GetFieldAtIndex(
>   field_type.GetIndexOfChildWithName(field_name, 
> /*omit_empty_base_classes=*/false),
>   name, &bit_offset, nullptr, nullptr);
>   ASSERT_TRUE(field_type);
> ```

Tested this locally and the tests pass, I have updated the commit.

> Instead of using `CompilerType::GetIndexOfFieldWithName` (though I haven't 
> actually tried to compile/run this)
> 
> Don't have a strong opinion on whether to remove or extend the API. 
> Personally I prefer removing it just because we already have so many 
> similarly named APIs across CompilerType/TypeSystemClang that do things 
> slightly differently, that it would be nice to get rid of at least one of 
> them.

Ended up removing the API and used your suggestion to define the test without 
the API. Thanks!



https://github.com/llvm/llvm-project/pull/135963
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb][Telemetry] Fix unit test compile failure with LLVM_ENABLE_TELEMETRY=0 (PR #136115)

2025-04-17 Thread Raul Tambre via lldb-commits

https://github.com/tambry created 
https://github.com/llvm/llvm-project/pull/136115

It needs to be `TEST_F` to access `received_entries`.
Disabling also works based on the test not the fixture name.

Build failure:
```
lldb/unittests/Core/TelemetryTest.cpp:110:17: error: use of undeclared 
identifier 'received_entries'
  110 |   ASSERT_EQ(1U, received_entries.size());
  | ^
lldb/unittests/Core/TelemetryTest.cpp:112:61: error: use of undeclared 
identifier 'received_entries'
  112 | 
llvm::dyn_cast(received_entries[0])
  | ^
```

Fixes: 159b872b37363511a359c800bcc9230bb09f2457

>From 2c93219ad894416e87458bfed2d2745e29b5bbe0 Mon Sep 17 00:00:00 2001
From: Raul Tambre 
Date: Thu, 17 Apr 2025 12:21:32 +0300
Subject: [PATCH] [lldb][Telemetry] Fix unit test compile failure with
 LLVM_ENABLE_TELEMETRY=0

It needs to be `TEST_F` to access `received_entries`.
Disabling also works based on the test not the fixture name.

Build failure:
```
lldb/unittests/Core/TelemetryTest.cpp:110:17: error: use of undeclared 
identifier 'received_entries'
  110 |   ASSERT_EQ(1U, received_entries.size());
  | ^
lldb/unittests/Core/TelemetryTest.cpp:112:61: error: use of undeclared 
identifier 'received_entries'
  112 | 
llvm::dyn_cast(received_entries[0])
  | ^
```

Fixes: 159b872b37363511a359c800bcc9230bb09f2457
---
 lldb/unittests/Core/TelemetryTest.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lldb/unittests/Core/TelemetryTest.cpp 
b/lldb/unittests/Core/TelemetryTest.cpp
index 1e41424bac3ce..910149d865c13 100644
--- a/lldb/unittests/Core/TelemetryTest.cpp
+++ b/lldb/unittests/Core/TelemetryTest.cpp
@@ -96,7 +96,7 @@ class TelemetryTest : public testing::Test {
 #if LLVM_ENABLE_TELEMETRY
 #define TELEMETRY_TEST(suite, test) TEST_F(suite, test)
 #else
-#define TELEMETRY_TEST(suite, test) TEST(DISABLED_##suite, test)
+#define TELEMETRY_TEST(suite, test) TEST_F(suite, DISABLED_##test)
 #endif
 
 TELEMETRY_TEST(TelemetryTest, PluginTest) {

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb] Remove CompilerType::GetIndexOfFieldWithName (PR #135963)

2025-04-17 Thread Charles Zablit via lldb-commits

charles-zablit wrote:

Sorry I forgot to run `clang-format` I will configure a pre-commit hook.

https://github.com/llvm/llvm-project/pull/135963
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb][Mach-O corefiles] Don't init Target arch to corefile (PR #136065)

2025-04-17 Thread via lldb-commits

llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: Jason Molenda (jasonmolenda)


Changes

This patch is making three changes, when loading a Mach-O corefile:

1. At the start of `DoLoadCore`, if a binary was provided in addition to the 
corefile, initialize the Target's ArchSpec.

2. Before ProcessMachCore does its "exhaustive search" fallback, looking 
through the corefile contents for a userland dyld or mach kernel, we must make 
sure the Target has an ArchSpec, or methods that check the address word size, 
or initialize a DataExtractor based on the Target arch will not succeed.

3. Add logging when setting the Target's arch listing exactly what that setting 
was based on -- the corefile itself, or the main binary.

Jonas landed a change last August (started with a patch from me) which removed 
the Target ArchSpec initialization at the start of DoLoadCore, in a scenario 
where the corefile had arch armv7 and the main binary had arch armv7em 
(Cortex-M), and there was python code in the main binary's dSYM which sets the 
operating system threads provider based on the Target arch.  It did different 
things for armv7 or armv7em, and so it would fail.

Jonas' patch removed any ArchSpec setting at the start of DoLoadCore, so we 
wouldn't have an incorrect arch value, but that broke the exhaustive search for 
kernel binaries, because we didn't have an address word size or endianness.

This patch should navigate the needs of both use cases.

I spent a good bit of time trying to construct a test to capture all of these 
requirements -- but it turns out to be a good bit difficult, encompassing both 
a genuine kernel corefiles and a microcontroller firmware corefiles.

rdar://146821929

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


1 Files Affected:

- (modified) lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp (+56-2) 


``diff
diff --git a/lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp 
b/lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp
index 281f3a0db8f69..17362b2d2855d 100644
--- a/lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp
+++ b/lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp
@@ -426,6 +426,32 @@ void ProcessMachCore::LoadBinariesViaExhaustiveSearch() {
   std::vector dylds_found;
   std::vector kernels_found;
 
+  // To do an exhaustive search, we'll need to create data extractors
+  // to get correctly sized/endianness fields, and if the Target still
+  // doesn't have an architecture set, we need to seed it from either
+  // the main binary (if we were given one) or the corefile cputype/cpusubtype.
+  if (!GetTarget().GetArchitecture().IsValid()) {
+Log *log(GetLog(LLDBLog::DynamicLoader | LLDBLog::Target));
+ModuleSP exe_module_sp = GetTarget().GetExecutableModule();
+if (exe_module_sp && exe_module_sp->GetArchitecture().IsValid()) {
+  LLDB_LOGF(log,
+"ProcessMachCore::%s: Was given binary + corefile, setting "
+"target ArchSpec to binary to start",
+__FUNCTION__);
+  GetTarget().SetArchitecture(exe_module_sp->GetArchitecture());
+} else {
+  // The corefile's architecture is our best starting point.
+  ArchSpec arch(m_core_module_sp->GetArchitecture());
+  if (arch.IsValid()) {
+LLDB_LOGF(log,
+  "ProcessMachCore::%s: Setting target ArchSpec based on "
+  "corefile mach-o cputype/cpusubtype",
+  __FUNCTION__);
+GetTarget().SetArchitecture(arch);
+  }
+}
+  }
+
   const size_t num_core_aranges = m_core_aranges.GetSize();
   for (size_t i = 0; i < num_core_aranges; ++i) {
 const VMRangeToFileOffset::Entry *entry = 
m_core_aranges.GetEntryAtIndex(i);
@@ -569,6 +595,7 @@ Status ProcessMachCore::DoLoadCore() {
 error = Status::FromErrorString("invalid core module");
 return error;
   }
+  Log *log(GetLog(LLDBLog::DynamicLoader | LLDBLog::Target));
 
   ObjectFile *core_objfile = m_core_module_sp->GetObjectFile();
   if (core_objfile == nullptr) {
@@ -578,20 +605,47 @@ Status ProcessMachCore::DoLoadCore() {
 
   SetCanJIT(false);
 
+  // If we have an executable binary in the Target already,
+  // use that to set the Target's ArchSpec.
+  //
+  // Don't initialize the ArchSpec based on the corefile's cputype/cpusubtype
+  // here, the corefile creator may not know the correct subtype of the code
+  // that is executing, initialize the Target to that, and if the
+  // main binary has Python code which initializes based on the Target arch,
+  // get the wrong subtype value.
+  ModuleSP exe_module_sp = GetTarget().GetExecutableModule();
+  if (exe_module_sp && exe_module_sp->GetArchitecture().IsValid()) {
+LLDB_LOGF(log,
+  "ProcessMachCore::%s: Was given binary + corefile, setting "
+  "target ArchSpec to binary to start",
+  __FUNCTION__);
+GetTarget().SetArchitecture(exe_module_sp->GetArchitecture());
+  }
+
  

[Lldb-commits] [lldb] [lldb] Remove CompilerType::GetIndexOfFieldWithName (PR #135963)

2025-04-17 Thread Charles Zablit via lldb-commits

https://github.com/charles-zablit updated 
https://github.com/llvm/llvm-project/pull/135963

>From 6dd67fe4ad03f0ec0623717715b8cfcc9537ab3f Mon Sep 17 00:00:00 2001
From: Charles Zablit 
Date: Wed, 16 Apr 2025 11:28:54 +0100
Subject: [PATCH 1/3] [lldb] Remove unused API
 CompilerType::GetIndexOfFieldWithName

---
 lldb/include/lldb/Symbol/CompilerType.h   |  6 --
 lldb/source/Symbol/CompilerType.cpp   | 19 -
 .../Platform/PlatformSiginfoTest.cpp  | 21 ---
 3 files changed, 46 deletions(-)

diff --git a/lldb/include/lldb/Symbol/CompilerType.h 
b/lldb/include/lldb/Symbol/CompilerType.h
index 41a1676dabd76..3561bc70887e6 100644
--- a/lldb/include/lldb/Symbol/CompilerType.h
+++ b/lldb/include/lldb/Symbol/CompilerType.h
@@ -433,12 +433,6 @@ class CompilerType {
 
   CompilerDecl GetStaticFieldWithName(llvm::StringRef name) const;
 
-  uint32_t GetIndexOfFieldWithName(const char *name,
-   CompilerType *field_compiler_type = nullptr,
-   uint64_t *bit_offset_ptr = nullptr,
-   uint32_t *bitfield_bit_size_ptr = nullptr,
-   bool *is_bitfield_ptr = nullptr) const;
-
   llvm::Expected GetChildCompilerTypeAtIndex(
   ExecutionContext *exe_ctx, size_t idx, bool transparent_pointers,
   bool omit_empty_base_classes, bool ignore_array_bounds,
diff --git a/lldb/source/Symbol/CompilerType.cpp 
b/lldb/source/Symbol/CompilerType.cpp
index 22fdd24fc7cd5..8e89d006d08d3 100644
--- a/lldb/source/Symbol/CompilerType.cpp
+++ b/lldb/source/Symbol/CompilerType.cpp
@@ -893,25 +893,6 @@ CompilerDecl 
CompilerType::GetStaticFieldWithName(llvm::StringRef name) const {
   return CompilerDecl();
 }
 
-uint32_t CompilerType::GetIndexOfFieldWithName(
-const char *name, CompilerType *field_compiler_type_ptr,
-uint64_t *bit_offset_ptr, uint32_t *bitfield_bit_size_ptr,
-bool *is_bitfield_ptr) const {
-  unsigned count = GetNumFields();
-  std::string field_name;
-  for (unsigned index = 0; index < count; index++) {
-CompilerType field_compiler_type(
-GetFieldAtIndex(index, field_name, bit_offset_ptr,
-bitfield_bit_size_ptr, is_bitfield_ptr));
-if (strcmp(field_name.c_str(), name) == 0) {
-  if (field_compiler_type_ptr)
-*field_compiler_type_ptr = field_compiler_type;
-  return index;
-}
-  }
-  return UINT32_MAX;
-}
-
 llvm::Expected CompilerType::GetChildCompilerTypeAtIndex(
 ExecutionContext *exe_ctx, size_t idx, bool transparent_pointers,
 bool omit_empty_base_classes, bool ignore_array_bounds,
diff --git a/lldb/unittests/Platform/PlatformSiginfoTest.cpp 
b/lldb/unittests/Platform/PlatformSiginfoTest.cpp
index 4b2c93a68a94a..a1f55bdd926db 100644
--- a/lldb/unittests/Platform/PlatformSiginfoTest.cpp
+++ b/lldb/unittests/Platform/PlatformSiginfoTest.cpp
@@ -50,27 +50,6 @@ class PlatformSiginfoTest : public ::testing::Test {
 
   typedef std::tuple field_tuple;
 
-  void ExpectField(const CompilerType &siginfo_type, field_tuple field) {
-const char *path;
-uint64_t offset, size;
-std::tie(path, offset, size) = field;
-
-SCOPED_TRACE(path);
-CompilerType field_type = siginfo_type;
-uint64_t total_offset = 0;
-for (auto field_name : llvm::split(path, '.')) {
-  uint64_t bit_offset;
-  ASSERT_NE(field_type.GetIndexOfFieldWithName(field_name.str().c_str(),
-   &field_type, &bit_offset),
-UINT32_MAX);
-  total_offset += bit_offset;
-}
-
-EXPECT_EQ(total_offset, offset * 8);
-EXPECT_EQ(llvm::expectedToOptional(field_type.GetByteSize(nullptr)),
-  std::optional(size));
-  }
-
   void ExpectFields(const CompilerType &container,
 std::initializer_list fields) {
 for (auto x : fields)

>From 77faa748f436cd28ea95854296c476a1be04e5d3 Mon Sep 17 00:00:00 2001
From: Charles Zablit 
Date: Wed, 16 Apr 2025 19:04:56 +0100
Subject: [PATCH 2/3] fixup! [lldb] Remove unused API
 CompilerType::GetIndexOfFieldWithName

---
 lldb/unittests/Platform/CMakeLists.txt|   1 -
 .../Platform/PlatformSiginfoTest.cpp  | 288 --
 2 files changed, 289 deletions(-)
 delete mode 100644 lldb/unittests/Platform/PlatformSiginfoTest.cpp

diff --git a/lldb/unittests/Platform/CMakeLists.txt 
b/lldb/unittests/Platform/CMakeLists.txt
index 5c0ef5ca6ef22..7d57f633d89c3 100644
--- a/lldb/unittests/Platform/CMakeLists.txt
+++ b/lldb/unittests/Platform/CMakeLists.txt
@@ -2,7 +2,6 @@ add_lldb_unittest(LLDBPlatformTests
   PlatformAppleSimulatorTest.cpp
   PlatformDarwinTest.cpp
   PlatformMacOSXTest.cpp
-  PlatformSiginfoTest.cpp
   PlatformTest.cpp
 
   LINK_LIBS
diff --git a/lldb/unittests/Platform/PlatformSiginfoTest.cpp 
b/lldb/unittests/Platform/PlatformSiginfoTest.cpp
deleted file mode 100644
index a1f55bdd926db..0
--- a/lldb/

[Lldb-commits] [lldb] [llvm] Modify the localCache API to require an explicit commit on CachedFile… (PR #115331)

2025-04-17 Thread via lldb-commits

anjenner wrote:

New PR for this: https://github.com/llvm/llvm-project/pull/136121 .

https://github.com/llvm/llvm-project/pull/115331
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb] Remove CompilerType::GetIndexOfFieldWithName (PR #135963)

2025-04-17 Thread Michael Buch via lldb-commits

https://github.com/Michael137 approved this pull request.

LGTM thanks!

https://github.com/llvm/llvm-project/pull/135963
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb] Remove CompilerType::GetIndexOfFieldWithName (PR #135963)

2025-04-17 Thread Charles Zablit via lldb-commits

https://github.com/charles-zablit updated 
https://github.com/llvm/llvm-project/pull/135963

>From 6dd67fe4ad03f0ec0623717715b8cfcc9537ab3f Mon Sep 17 00:00:00 2001
From: Charles Zablit 
Date: Wed, 16 Apr 2025 11:28:54 +0100
Subject: [PATCH 1/3] [lldb] Remove unused API
 CompilerType::GetIndexOfFieldWithName

---
 lldb/include/lldb/Symbol/CompilerType.h   |  6 --
 lldb/source/Symbol/CompilerType.cpp   | 19 -
 .../Platform/PlatformSiginfoTest.cpp  | 21 ---
 3 files changed, 46 deletions(-)

diff --git a/lldb/include/lldb/Symbol/CompilerType.h 
b/lldb/include/lldb/Symbol/CompilerType.h
index 41a1676dabd76..3561bc70887e6 100644
--- a/lldb/include/lldb/Symbol/CompilerType.h
+++ b/lldb/include/lldb/Symbol/CompilerType.h
@@ -433,12 +433,6 @@ class CompilerType {
 
   CompilerDecl GetStaticFieldWithName(llvm::StringRef name) const;
 
-  uint32_t GetIndexOfFieldWithName(const char *name,
-   CompilerType *field_compiler_type = nullptr,
-   uint64_t *bit_offset_ptr = nullptr,
-   uint32_t *bitfield_bit_size_ptr = nullptr,
-   bool *is_bitfield_ptr = nullptr) const;
-
   llvm::Expected GetChildCompilerTypeAtIndex(
   ExecutionContext *exe_ctx, size_t idx, bool transparent_pointers,
   bool omit_empty_base_classes, bool ignore_array_bounds,
diff --git a/lldb/source/Symbol/CompilerType.cpp 
b/lldb/source/Symbol/CompilerType.cpp
index 22fdd24fc7cd5..8e89d006d08d3 100644
--- a/lldb/source/Symbol/CompilerType.cpp
+++ b/lldb/source/Symbol/CompilerType.cpp
@@ -893,25 +893,6 @@ CompilerDecl 
CompilerType::GetStaticFieldWithName(llvm::StringRef name) const {
   return CompilerDecl();
 }
 
-uint32_t CompilerType::GetIndexOfFieldWithName(
-const char *name, CompilerType *field_compiler_type_ptr,
-uint64_t *bit_offset_ptr, uint32_t *bitfield_bit_size_ptr,
-bool *is_bitfield_ptr) const {
-  unsigned count = GetNumFields();
-  std::string field_name;
-  for (unsigned index = 0; index < count; index++) {
-CompilerType field_compiler_type(
-GetFieldAtIndex(index, field_name, bit_offset_ptr,
-bitfield_bit_size_ptr, is_bitfield_ptr));
-if (strcmp(field_name.c_str(), name) == 0) {
-  if (field_compiler_type_ptr)
-*field_compiler_type_ptr = field_compiler_type;
-  return index;
-}
-  }
-  return UINT32_MAX;
-}
-
 llvm::Expected CompilerType::GetChildCompilerTypeAtIndex(
 ExecutionContext *exe_ctx, size_t idx, bool transparent_pointers,
 bool omit_empty_base_classes, bool ignore_array_bounds,
diff --git a/lldb/unittests/Platform/PlatformSiginfoTest.cpp 
b/lldb/unittests/Platform/PlatformSiginfoTest.cpp
index 4b2c93a68a94a..a1f55bdd926db 100644
--- a/lldb/unittests/Platform/PlatformSiginfoTest.cpp
+++ b/lldb/unittests/Platform/PlatformSiginfoTest.cpp
@@ -50,27 +50,6 @@ class PlatformSiginfoTest : public ::testing::Test {
 
   typedef std::tuple field_tuple;
 
-  void ExpectField(const CompilerType &siginfo_type, field_tuple field) {
-const char *path;
-uint64_t offset, size;
-std::tie(path, offset, size) = field;
-
-SCOPED_TRACE(path);
-CompilerType field_type = siginfo_type;
-uint64_t total_offset = 0;
-for (auto field_name : llvm::split(path, '.')) {
-  uint64_t bit_offset;
-  ASSERT_NE(field_type.GetIndexOfFieldWithName(field_name.str().c_str(),
-   &field_type, &bit_offset),
-UINT32_MAX);
-  total_offset += bit_offset;
-}
-
-EXPECT_EQ(total_offset, offset * 8);
-EXPECT_EQ(llvm::expectedToOptional(field_type.GetByteSize(nullptr)),
-  std::optional(size));
-  }
-
   void ExpectFields(const CompilerType &container,
 std::initializer_list fields) {
 for (auto x : fields)

>From 77faa748f436cd28ea95854296c476a1be04e5d3 Mon Sep 17 00:00:00 2001
From: Charles Zablit 
Date: Wed, 16 Apr 2025 19:04:56 +0100
Subject: [PATCH 2/3] fixup! [lldb] Remove unused API
 CompilerType::GetIndexOfFieldWithName

---
 lldb/unittests/Platform/CMakeLists.txt|   1 -
 .../Platform/PlatformSiginfoTest.cpp  | 288 --
 2 files changed, 289 deletions(-)
 delete mode 100644 lldb/unittests/Platform/PlatformSiginfoTest.cpp

diff --git a/lldb/unittests/Platform/CMakeLists.txt 
b/lldb/unittests/Platform/CMakeLists.txt
index 5c0ef5ca6ef22..7d57f633d89c3 100644
--- a/lldb/unittests/Platform/CMakeLists.txt
+++ b/lldb/unittests/Platform/CMakeLists.txt
@@ -2,7 +2,6 @@ add_lldb_unittest(LLDBPlatformTests
   PlatformAppleSimulatorTest.cpp
   PlatformDarwinTest.cpp
   PlatformMacOSXTest.cpp
-  PlatformSiginfoTest.cpp
   PlatformTest.cpp
 
   LINK_LIBS
diff --git a/lldb/unittests/Platform/PlatformSiginfoTest.cpp 
b/lldb/unittests/Platform/PlatformSiginfoTest.cpp
deleted file mode 100644
index a1f55bdd926db..0
--- a/lldb/

[Lldb-commits] [lldb] [lldb][Mach-O corefiles] Don't init Target arch to corefile (PR #136065)

2025-04-17 Thread via lldb-commits

jimingham wrote:

Beyond the question about the duplicate work, the approach seems reasonable to 
me.

https://github.com/llvm/llvm-project/pull/136065
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb] Remove CompilerType::GetIndexOfFieldWithName (PR #135963)

2025-04-17 Thread Charles Zablit via lldb-commits

https://github.com/charles-zablit updated 
https://github.com/llvm/llvm-project/pull/135963

>From 6dd67fe4ad03f0ec0623717715b8cfcc9537ab3f Mon Sep 17 00:00:00 2001
From: Charles Zablit 
Date: Wed, 16 Apr 2025 11:28:54 +0100
Subject: [PATCH 1/4] [lldb] Remove unused API
 CompilerType::GetIndexOfFieldWithName

---
 lldb/include/lldb/Symbol/CompilerType.h   |  6 --
 lldb/source/Symbol/CompilerType.cpp   | 19 -
 .../Platform/PlatformSiginfoTest.cpp  | 21 ---
 3 files changed, 46 deletions(-)

diff --git a/lldb/include/lldb/Symbol/CompilerType.h 
b/lldb/include/lldb/Symbol/CompilerType.h
index 41a1676dabd76..3561bc70887e6 100644
--- a/lldb/include/lldb/Symbol/CompilerType.h
+++ b/lldb/include/lldb/Symbol/CompilerType.h
@@ -433,12 +433,6 @@ class CompilerType {
 
   CompilerDecl GetStaticFieldWithName(llvm::StringRef name) const;
 
-  uint32_t GetIndexOfFieldWithName(const char *name,
-   CompilerType *field_compiler_type = nullptr,
-   uint64_t *bit_offset_ptr = nullptr,
-   uint32_t *bitfield_bit_size_ptr = nullptr,
-   bool *is_bitfield_ptr = nullptr) const;
-
   llvm::Expected GetChildCompilerTypeAtIndex(
   ExecutionContext *exe_ctx, size_t idx, bool transparent_pointers,
   bool omit_empty_base_classes, bool ignore_array_bounds,
diff --git a/lldb/source/Symbol/CompilerType.cpp 
b/lldb/source/Symbol/CompilerType.cpp
index 22fdd24fc7cd5..8e89d006d08d3 100644
--- a/lldb/source/Symbol/CompilerType.cpp
+++ b/lldb/source/Symbol/CompilerType.cpp
@@ -893,25 +893,6 @@ CompilerDecl 
CompilerType::GetStaticFieldWithName(llvm::StringRef name) const {
   return CompilerDecl();
 }
 
-uint32_t CompilerType::GetIndexOfFieldWithName(
-const char *name, CompilerType *field_compiler_type_ptr,
-uint64_t *bit_offset_ptr, uint32_t *bitfield_bit_size_ptr,
-bool *is_bitfield_ptr) const {
-  unsigned count = GetNumFields();
-  std::string field_name;
-  for (unsigned index = 0; index < count; index++) {
-CompilerType field_compiler_type(
-GetFieldAtIndex(index, field_name, bit_offset_ptr,
-bitfield_bit_size_ptr, is_bitfield_ptr));
-if (strcmp(field_name.c_str(), name) == 0) {
-  if (field_compiler_type_ptr)
-*field_compiler_type_ptr = field_compiler_type;
-  return index;
-}
-  }
-  return UINT32_MAX;
-}
-
 llvm::Expected CompilerType::GetChildCompilerTypeAtIndex(
 ExecutionContext *exe_ctx, size_t idx, bool transparent_pointers,
 bool omit_empty_base_classes, bool ignore_array_bounds,
diff --git a/lldb/unittests/Platform/PlatformSiginfoTest.cpp 
b/lldb/unittests/Platform/PlatformSiginfoTest.cpp
index 4b2c93a68a94a..a1f55bdd926db 100644
--- a/lldb/unittests/Platform/PlatformSiginfoTest.cpp
+++ b/lldb/unittests/Platform/PlatformSiginfoTest.cpp
@@ -50,27 +50,6 @@ class PlatformSiginfoTest : public ::testing::Test {
 
   typedef std::tuple field_tuple;
 
-  void ExpectField(const CompilerType &siginfo_type, field_tuple field) {
-const char *path;
-uint64_t offset, size;
-std::tie(path, offset, size) = field;
-
-SCOPED_TRACE(path);
-CompilerType field_type = siginfo_type;
-uint64_t total_offset = 0;
-for (auto field_name : llvm::split(path, '.')) {
-  uint64_t bit_offset;
-  ASSERT_NE(field_type.GetIndexOfFieldWithName(field_name.str().c_str(),
-   &field_type, &bit_offset),
-UINT32_MAX);
-  total_offset += bit_offset;
-}
-
-EXPECT_EQ(total_offset, offset * 8);
-EXPECT_EQ(llvm::expectedToOptional(field_type.GetByteSize(nullptr)),
-  std::optional(size));
-  }
-
   void ExpectFields(const CompilerType &container,
 std::initializer_list fields) {
 for (auto x : fields)

>From 77faa748f436cd28ea95854296c476a1be04e5d3 Mon Sep 17 00:00:00 2001
From: Charles Zablit 
Date: Wed, 16 Apr 2025 19:04:56 +0100
Subject: [PATCH 2/4] fixup! [lldb] Remove unused API
 CompilerType::GetIndexOfFieldWithName

---
 lldb/unittests/Platform/CMakeLists.txt|   1 -
 .../Platform/PlatformSiginfoTest.cpp  | 288 --
 2 files changed, 289 deletions(-)
 delete mode 100644 lldb/unittests/Platform/PlatformSiginfoTest.cpp

diff --git a/lldb/unittests/Platform/CMakeLists.txt 
b/lldb/unittests/Platform/CMakeLists.txt
index 5c0ef5ca6ef22..7d57f633d89c3 100644
--- a/lldb/unittests/Platform/CMakeLists.txt
+++ b/lldb/unittests/Platform/CMakeLists.txt
@@ -2,7 +2,6 @@ add_lldb_unittest(LLDBPlatformTests
   PlatformAppleSimulatorTest.cpp
   PlatformDarwinTest.cpp
   PlatformMacOSXTest.cpp
-  PlatformSiginfoTest.cpp
   PlatformTest.cpp
 
   LINK_LIBS
diff --git a/lldb/unittests/Platform/PlatformSiginfoTest.cpp 
b/lldb/unittests/Platform/PlatformSiginfoTest.cpp
deleted file mode 100644
index a1f55bdd926db..0
--- a/lldb/

[Lldb-commits] [lldb] [llvm] Modify the localCache API to require an explicit commit on CachedFile… (PR #136121)

2025-04-17 Thread via lldb-commits

https://github.com/anjenner created 
https://github.com/llvm/llvm-project/pull/136121

…Stream.

CachedFileStream has previously performed the commit step in its destructor, 
but this means its only recourse for error handling is report_fatal_error. 
Modify this to add an explicit commit() method, and call this in the 
appropriate places with appropriate error handling for the location.

Currently the destructor of CacheStream gives an assert failure in Debug builds 
if commit() was not called. This will help track down any remaining uses of the 
API that assume the old destructior behaviour. In Release builds we fall back 
to the previous behaviour and call report_fatal_error if the commit fails.

This is version 2 of this PR, superseding reverted PR 
https://github.com/llvm/llvm-project/pull/115331 . I have incorporated a change 
to the testcase to make it more reliable on Windows, as well as two follow-up 
changes 
(https://github.com/llvm/llvm-project/commit/df79000896101acc9b8d7435e59f767b36c00ac8
 and 
https://github.com/llvm/llvm-project/commit/b0baa1d8bd68a2ce2f7c5f2b62333e410e9122a1)
 that were also reverted when 115331 was reverted.

>From 3fdba46d41ad9668a114766fe892af497f121cd5 Mon Sep 17 00:00:00 2001
From: Andrew Jenner 
Date: Thu, 7 Nov 2024 10:47:42 -0500
Subject: [PATCH 01/10] Modify the localCache API to require an explicit commit
 on CachedFileStream.

CachedFileStream has previously performed the commit step in its destructor,
but this means its only recourse for error handling is report_fatal_error.
Modify this to add an explicit commit() method, and call this in the
appropriate places with appropriate error handling for the location.

Currently the destructor of CacheStream gives an assert failure in Debug builds
if commit() was not called. This will help track down any remaining uses of the
API that assume the old destructior behaviour. In Release builds we fall back
to the previous behaviour and call report_fatal_error if the commit fails.
---
 lldb/source/Core/DataFileCache.cpp  |  5 
 llvm/include/llvm/Support/Caching.h |  1 +
 llvm/lib/Debuginfod/Debuginfod.cpp  |  9 +++
 llvm/lib/LTO/LTOBackend.cpp |  3 +++
 llvm/lib/Support/Caching.cpp| 40 +
 llvm/tools/gold/gold-plugin.cpp |  4 ++-
 llvm/tools/llvm-lto2/llvm-lto2.cpp  |  4 ++-
 7 files changed, 53 insertions(+), 13 deletions(-)

diff --git a/lldb/source/Core/DataFileCache.cpp 
b/lldb/source/Core/DataFileCache.cpp
index ef0e07a8b0342..9109269711231 100644
--- a/lldb/source/Core/DataFileCache.cpp
+++ b/lldb/source/Core/DataFileCache.cpp
@@ -132,6 +132,11 @@ bool DataFileCache::SetCachedData(llvm::StringRef key,
   if (file_or_err) {
 llvm::CachedFileStream *cfs = file_or_err->get();
 cfs->OS->write((const char *)data.data(), data.size());
+if (llvm::Error err = cfs->commit()) {
+  Log *log = GetLog(LLDBLog::Modules);
+  LLDB_LOG_ERROR(log, std::move(err),
+ "failed to commit to the cache for key: {0}");
+}
 return true;
   } else {
 Log *log = GetLog(LLDBLog::Modules);
diff --git a/llvm/include/llvm/Support/Caching.h 
b/llvm/include/llvm/Support/Caching.h
index cf45145619d95..120bcd9da02ed 100644
--- a/llvm/include/llvm/Support/Caching.h
+++ b/llvm/include/llvm/Support/Caching.h
@@ -30,6 +30,7 @@ class CachedFileStream {
   CachedFileStream(std::unique_ptr OS,
std::string OSPath = "")
   : OS(std::move(OS)), ObjectPathName(OSPath) {}
+  virtual Error commit() { return Error::success(); }
   std::unique_ptr OS;
   std::string ObjectPathName;
   virtual ~CachedFileStream() = default;
diff --git a/llvm/lib/Debuginfod/Debuginfod.cpp 
b/llvm/lib/Debuginfod/Debuginfod.cpp
index 4c785117ae8ef..6ff889d3a8cad 100644
--- a/llvm/lib/Debuginfod/Debuginfod.cpp
+++ b/llvm/lib/Debuginfod/Debuginfod.cpp
@@ -188,6 +188,7 @@ class StreamedHTTPResponseHandler : public 
HTTPResponseHandler {
 public:
   StreamedHTTPResponseHandler(CreateStreamFn CreateStream, HTTPClient &Client)
   : CreateStream(CreateStream), Client(Client) {}
+  Error commit();
   virtual ~StreamedHTTPResponseHandler() = default;
 
   Error handleBodyChunk(StringRef BodyChunk) override;
@@ -210,6 +211,12 @@ Error 
StreamedHTTPResponseHandler::handleBodyChunk(StringRef BodyChunk) {
   return Error::success();
 }
 
+Error StreamedHTTPResponseHandler::commit() {
+  if (FileStream)
+return FileStream->commit();
+  return Error::success();
+}
+
 // An over-accepting simplification of the HTTP RFC 7230 spec.
 static bool isHeader(StringRef S) {
   StringRef Name;
@@ -298,6 +305,8 @@ Expected getCachedOrDownloadArtifact(
   Error Err = Client.perform(Request, Handler);
   if (Err)
 return std::move(Err);
+  if (Err = Handler.commit())
+return std::move(Err);
 
   unsigned Code = Client.responseCode();
   if (Code && Code != 200)
diff --git a/llvm/lib/LTO/LTOBackend.cpp