[Lldb-commits] [lldb] c1df376 - [LLDB] Finish implementing support for DW_FORM_data16 (#113508)
Author: Walter Erquinigo
Date: 2024-11-01T17:33:08-04:00
New Revision: c1df376b0c8b3edbb6c5ad3fb6ebf39e75b5ad88
URL:
https://github.com/llvm/llvm-project/commit/c1df376b0c8b3edbb6c5ad3fb6ebf39e75b5ad88
DIFF:
https://github.com/llvm/llvm-project/commit/c1df376b0c8b3edbb6c5ad3fb6ebf39e75b5ad88.diff
LOG: [LLDB] Finish implementing support for DW_FORM_data16 (#113508)
This FORM already has support within LLDB to be parsed as a 16-byte
BLOCK, and all that is left to properly support it in the DWARFParser is
to add it to some enums.
With this, I can debug programs that use libstdc++.so.6.0.33 built with
GCC.
Added:
Modified:
lldb/source/Plugins/SymbolFile/DWARF/DWARFFormValue.cpp
lldb/test/Shell/SymbolFile/DWARF/x86/DW_AT_const_value.s
Removed:
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFFormValue.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFFormValue.cpp
index f58c6262349c6f..404e50d57a9251 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFFormValue.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFFormValue.cpp
@@ -306,6 +306,11 @@ bool DWARFFormValue::SkipValue(dw_form_t form,
*offset_ptr += 8;
return true;
+// 16 byte values
+case DW_FORM_data16:
+ *offset_ptr += 16;
+ return true;
+
// signed or unsigned LEB 128 values
case DW_FORM_addrx:
case DW_FORM_loclistx:
@@ -578,6 +583,7 @@ bool DWARFFormValue::IsBlockForm(const dw_form_t form) {
case DW_FORM_block1:
case DW_FORM_block2:
case DW_FORM_block4:
+ case DW_FORM_data16:
return true;
default:
return false;
@@ -611,6 +617,7 @@ bool DWARFFormValue::FormIsSupported(dw_form_t form) {
case DW_FORM_data2:
case DW_FORM_data4:
case DW_FORM_data8:
+case DW_FORM_data16:
case DW_FORM_string:
case DW_FORM_block:
case DW_FORM_block1:
diff --git a/lldb/test/Shell/SymbolFile/DWARF/x86/DW_AT_const_value.s
b/lldb/test/Shell/SymbolFile/DWARF/x86/DW_AT_const_value.s
index 720684c19beebc..306afd32d92235 100644
--- a/lldb/test/Shell/SymbolFile/DWARF/x86/DW_AT_const_value.s
+++ b/lldb/test/Shell/SymbolFile/DWARF/x86/DW_AT_const_value.s
@@ -4,6 +4,7 @@
# RUN: llvm-mc -filetype=obj -triple x86_64-pc-linux %s -o %t
# RUN: %lldb %t \
# RUN: -o "target variable udata data1 data2 data4 data8 string strp ref4
udata_ptr" \
+# RUN: -o "target variable --format x data16" \
# RUN: -o exit | FileCheck %s
# CHECK-LABEL: target variable
@@ -22,6 +23,8 @@
# CHECK: (char[7]) ref4 =
## A variable of pointer type.
# CHECK: (unsigned long *) udata_ptr = 0xdeadbeefbaadf00d
+# CHECK: (unsigned __int128) data16 = 0xdeadbeefbaadf00ddeadbeefbaadf00d
+
.section.debug_abbrev,"",@progbits
.byte 1 # Abbreviation Code
@@ -88,6 +91,7 @@
var 15, 0x8 # DW_FORM_string
var 16, 0xe # DW_FORM_strp
var 17, 0x13# DW_FORM_ref4
+var 18, 0x1e# DW_FORM_data16
.byte 0 # EOM(3)
.section.debug_info,"",@progbits
.Lcu_begin0:
@@ -119,6 +123,11 @@
.Lulong_ptr:
.byte 2 # Abbrev DW_TAG_pointer_type
.long .Lulong-.Lcu_begin0 # DW_AT_type
+.Lu128:
+.byte 6 # Abbrev DW_TAG_base_type
+.asciz "Lu128" # DW_AT_name
+.byte 16 # DW_AT_byte_size
+.byte 7 # DW_AT_encoding
.byte 10 # Abbrev DW_TAG_variable
.asciz "udata" # DW_AT_name
@@ -165,6 +174,12 @@
.long .Lulong_ptr-.Lcu_begin0 # DW_AT_type
.uleb128 0xdeadbeefbaadf00d # DW_AT_const_value
+.byte 18 # Abbrev DW_TAG_variable
+.asciz "data16"# DW_AT_name
+.long .Lu128-.Lcu_begin0 # DW_AT_type
+.quad 0xdeadbeefbaadf00d # DW_AT_const_value
+.quad 0xdeadbeefbaadf00d # DW_AT_const_value
+
.byte 0 # End Of Children Mark
.Ldebug_info_end0:
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB] Finish implementing support for DW_FORM_data16 (PR #113508)
https://github.com/walter-erquinigo closed https://github.com/llvm/llvm-project/pull/113508 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Avoid unnecessary regex check in dwim-print (PR #114608)
llvmbot wrote:
@llvm/pr-subscribers-lldb
Author: Dave Lee (kastiglione)
Changes
---
Full diff: https://github.com/llvm/llvm-project/pull/114608.diff
1 Files Affected:
- (modified) lldb/source/Commands/CommandObjectDWIMPrint.cpp (+8-8)
``diff
diff --git a/lldb/source/Commands/CommandObjectDWIMPrint.cpp
b/lldb/source/Commands/CommandObjectDWIMPrint.cpp
index 76bed100dc7291..ae35f6e43f6c62 100644
--- a/lldb/source/Commands/CommandObjectDWIMPrint.cpp
+++ b/lldb/source/Commands/CommandObjectDWIMPrint.cpp
@@ -101,6 +101,10 @@ void CommandObjectDWIMPrint::DoExecute(StringRef command,
// Add a hint if object description was requested, but no description
// function was implemented.
auto maybe_add_hint = [&](llvm::StringRef output) {
+static bool note_shown = false;
+if (note_shown)
+ return;
+
// Identify the default output of object description for Swift and
// Objective-C
// ". The regex is:
@@ -110,20 +114,16 @@ void CommandObjectDWIMPrint::DoExecute(StringRef command,
// - Followed by 5 or more hex digits.
// - Followed by ">".
// - End with zero or more whitespace characters.
-const std::regex swift_class_regex("^<\\S+: 0x[[:xdigit:]]{5,}>\\s*$");
+static const std::regex class_po_regex("^<\\S+: 0x[[:xdigit:]]{5,}>\\s*$");
if (GetDebugger().GetShowDontUsePoHint() && target_ptr &&
(language == lldb::eLanguageTypeSwift ||
language == lldb::eLanguageTypeObjC) &&
-std::regex_match(output.data(), swift_class_regex)) {
-
- static bool note_shown = false;
- if (note_shown)
-return;
+std::regex_match(output.data(), class_po_regex)) {
result.GetOutputStream()
- << "note: object description requested, but type doesn't implement "
- "a custom object description. Consider using \"p\" instead of "
+ << "note: object description requested, but type doesn't implement a
"
+ "custom object description. Consider using \"p\" instead of "
"\"po\" (this note will only be shown once per debug session).\n";
note_shown = true;
}
``
https://github.com/llvm/llvm-project/pull/114608
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Avoid unnecessary regex check in dwim-print (PR #114608)
https://github.com/kastiglione created
https://github.com/llvm/llvm-project/pull/114608
None
>From e244e4557a618c6d532f9d369624234ad739cf11 Mon Sep 17 00:00:00 2001
From: Dave Lee
Date: Fri, 1 Nov 2024 14:38:19 -0700
Subject: [PATCH] [lldb] Avoid unnecessary regex check in dwim-print
---
lldb/source/Commands/CommandObjectDWIMPrint.cpp | 16
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/lldb/source/Commands/CommandObjectDWIMPrint.cpp
b/lldb/source/Commands/CommandObjectDWIMPrint.cpp
index 76bed100dc7291..ae35f6e43f6c62 100644
--- a/lldb/source/Commands/CommandObjectDWIMPrint.cpp
+++ b/lldb/source/Commands/CommandObjectDWIMPrint.cpp
@@ -101,6 +101,10 @@ void CommandObjectDWIMPrint::DoExecute(StringRef command,
// Add a hint if object description was requested, but no description
// function was implemented.
auto maybe_add_hint = [&](llvm::StringRef output) {
+static bool note_shown = false;
+if (note_shown)
+ return;
+
// Identify the default output of object description for Swift and
// Objective-C
// ". The regex is:
@@ -110,20 +114,16 @@ void CommandObjectDWIMPrint::DoExecute(StringRef command,
// - Followed by 5 or more hex digits.
// - Followed by ">".
// - End with zero or more whitespace characters.
-const std::regex swift_class_regex("^<\\S+: 0x[[:xdigit:]]{5,}>\\s*$");
+static const std::regex class_po_regex("^<\\S+: 0x[[:xdigit:]]{5,}>\\s*$");
if (GetDebugger().GetShowDontUsePoHint() && target_ptr &&
(language == lldb::eLanguageTypeSwift ||
language == lldb::eLanguageTypeObjC) &&
-std::regex_match(output.data(), swift_class_regex)) {
-
- static bool note_shown = false;
- if (note_shown)
-return;
+std::regex_match(output.data(), class_po_regex)) {
result.GetOutputStream()
- << "note: object description requested, but type doesn't implement "
- "a custom object description. Consider using \"p\" instead of "
+ << "note: object description requested, but type doesn't implement a
"
+ "custom object description. Consider using \"p\" instead of "
"\"po\" (this note will only be shown once per debug session).\n";
note_shown = true;
}
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Create dependent modules in parallel (PR #114507)
https://github.com/JDevlieghere updated
https://github.com/llvm/llvm-project/pull/114507
>From 9a269fa83cea529f704c9eba339eac9987032155 Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere
Date: Thu, 31 Oct 2024 21:42:38 -0700
Subject: [PATCH 1/3] [lldb] Create dependent modules in parallel
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Create dependent modules in parallel in Target::SetExecutableModule.
This change was inspired by #110646 which takes the same approach when
attaching. Jason suggested we could use the same approach when you
create a target in LLDB.
I used Slack for benchmarking, which loads 902 images.
Benchmark 1: ./bin/lldb /Applications/Slack.app/Contents/MacOS/Slack
Time (mean ± σ): 1.225 s ± 0.003 s[User: 3.977 s, System: 1.521 s]
Range (min … max):1.220 s … 1.229 s10 runs
Benchmark 2: ./bin/lldb /Applications/Slack.app/Contents/MacOS/Slack
Time (mean ± σ): 3.253 s ± 0.037 s[User: 3.013 s, System: 0.248 s]
Range (min … max):3.211 s … 3.310 s10 runs
We see about a 2x speedup, which matches what Jason saw for the attach
scenario. I also ran this under TSan to confirm this doesn't introduce
any races or deadlocks.
---
lldb/source/Target/Target.cpp | 33 -
1 file changed, 28 insertions(+), 5 deletions(-)
diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp
index 199efae8a728cc..ef5d38fc796b08 100644
--- a/lldb/source/Target/Target.cpp
+++ b/lldb/source/Target/Target.cpp
@@ -68,6 +68,7 @@
#include "llvm/ADT/ScopeExit.h"
#include "llvm/ADT/SetVector.h"
+#include "llvm/Support/ThreadPool.h"
#include
#include
@@ -1575,7 +1576,6 @@ void Target::SetExecutableModule(ModuleSP &executable_sp,
m_arch.GetSpec().GetTriple().getTriple());
}
-FileSpecList dependent_files;
ObjectFile *executable_objfile = executable_sp->GetObjectFile();
bool load_dependents = true;
switch (load_dependent_files) {
@@ -1591,10 +1591,14 @@ void Target::SetExecutableModule(ModuleSP
&executable_sp,
}
if (executable_objfile && load_dependents) {
+ // FileSpecList is not thread safe and needs to be synchronized.
+ FileSpecList dependent_files;
+ std::mutex dependent_files_mutex;
+
+ // ModuleList is thread safe.
ModuleList added_modules;
- executable_objfile->GetDependentModules(dependent_files);
- for (uint32_t i = 0; i < dependent_files.GetSize(); i++) {
-FileSpec dependent_file_spec(dependent_files.GetFileSpecAtIndex(i));
+
+ auto GetDependentModules = [&](FileSpec dependent_file_spec) {
FileSpec platform_dependent_file_spec;
if (m_platform_sp)
m_platform_sp->GetFileWithUUID(dependent_file_spec, nullptr,
@@ -1608,9 +1612,28 @@ void Target::SetExecutableModule(ModuleSP &executable_sp,
if (image_module_sp) {
added_modules.AppendIfNeeded(image_module_sp, false);
ObjectFile *objfile = image_module_sp->GetObjectFile();
- if (objfile)
+ if (objfile) {
+std::lock_guard guard(dependent_files_mutex);
objfile->GetDependentModules(dependent_files);
+ }
+}
+ };
+
+ executable_objfile->GetDependentModules(dependent_files);
+
+ llvm::ThreadPoolTaskGroup task_group(Debugger::GetThreadPool());
+ for (uint32_t i = 0; i < dependent_files.GetSize(); i++) {
+// Process all currently known dependencies in parallel in the
innermost
+// loop. This may create newly discovered dependencies to be appended
to
+// dependent_files. We'll deal with these files during the next
+// iteration of the outermost loop.
+{
+ std::lock_guard guard(dependent_files_mutex);
+ for (; i < dependent_files.GetSize(); i++)
+task_group.async(GetDependentModules,
+dependent_files.GetFileSpecAtIndex(i));
}
+task_group.wait();
}
ModulesDidLoad(added_modules);
}
>From 0bba5f799aadb4bd987e7aa29ca727562364b915 Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere
Date: Thu, 31 Oct 2024 21:54:26 -0700
Subject: [PATCH 2/3] Fix formatting
---
lldb/source/Target/Target.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp
index ef5d38fc796b08..ad38e6138cf0c6 100644
--- a/lldb/source/Target/Target.cpp
+++ b/lldb/source/Target/Target.cpp
@@ -1631,7 +1631,7 @@ void Target::SetExecutableModule(ModuleSP &executable_sp,
std::lock_guard guard(dependent_files_mutex);
for (; i < dependent_files.GetSize(); i++)
task_group.async(GetDependentModules,
-dependent_files.GetFileSpecAtIndex(i));
+ dependent_files.GetFileSpecAtIndex(i));
}
task_group.wait();
}
>Fro
[Lldb-commits] [lldb] [lldb] Create dependent modules in parallel (PR #114507)
https://github.com/jasonmolenda approved this pull request. LGTM. https://github.com/llvm/llvm-project/pull/114507 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] More refinement of call site handling in stepping. (PR #114628)
https://github.com/jimingham created
https://github.com/llvm/llvm-project/pull/114628
When you set a "next branch breakpoint" and run to it while stepping, you have
to claim the stop at that breakpoint to be the top of the inlined call stack,
or you will seem to "step in" and then plans might try to step back out again.
This records the PrefferedLineEntry for next branch breakpoints and adds a test
to make sure this works.
>From 042ac07ed67a5465aaf5c2dc8c4396adf5da2948 Mon Sep 17 00:00:00 2001
From: Jim Ingham
Date: Fri, 1 Nov 2024 17:23:12 -0700
Subject: [PATCH] More refinement of call site handling in stepping.
When you set a "next branch breakpoint" and run to it while
stepping, you have to claim the stop at that breakpoint to be
the top of the inlined call stack, or you will seem to "step in"
and then plans might try to step back out again.
This records the PrefferedLineEntry for next branch breakpoints and
adds a test to make sure this works.
---
lldb/source/Target/ThreadPlanStepRange.cpp| 42 +--
.../inline-stepping/TestInlineStepping.py | 20 -
2 files changed, 58 insertions(+), 4 deletions(-)
diff --git a/lldb/source/Target/ThreadPlanStepRange.cpp
b/lldb/source/Target/ThreadPlanStepRange.cpp
index 3c825058b8c375..e53b189d49be44 100644
--- a/lldb/source/Target/ThreadPlanStepRange.cpp
+++ b/lldb/source/Target/ThreadPlanStepRange.cpp
@@ -379,10 +379,9 @@ bool ThreadPlanStepRange::SetNextBranchBreakpoint() {
!m_next_branch_bp_sp->HasResolvedLocations())
m_could_not_resolve_hw_bp = true;
+BreakpointLocationSP bp_loc =
m_next_branch_bp_sp->GetLocationAtIndex(0);
if (log) {
lldb::break_id_t bp_site_id = LLDB_INVALID_BREAK_ID;
- BreakpointLocationSP bp_loc =
- m_next_branch_bp_sp->GetLocationAtIndex(0);
if (bp_loc) {
BreakpointSiteSP bp_site = bp_loc->GetBreakpointSite();
if (bp_site) {
@@ -395,7 +394,44 @@ bool ThreadPlanStepRange::SetNextBranchBreakpoint() {
m_next_branch_bp_sp->GetID(), bp_site_id,
run_to_address.GetLoadAddress(&m_process.GetTarget()));
}
-
+// The "next branch breakpoint might land on an virtual inlined call
+// stack. If that's true, we should always stop at the top of the
+// inlined call stack. Only virtual steps should walk deeper into the
+// inlined call stack.
+Block *block = run_to_address.CalculateSymbolContextBlock();
+if (bp_loc && block) {
+ LineEntry top_most_line_entry;
+ lldb::addr_t run_to_addr = run_to_address.GetFileAddress();
+ for (Block *inlined_parent = block->GetContainingInlinedBlock();
inlined_parent;
+ inlined_parent = inlined_parent->GetInlinedParent()) {
+AddressRange range;
+if (!inlined_parent->GetRangeContainingAddress(run_to_address,
range))
+ break;
+Address range_start_address = range.GetBaseAddress();
+// Only compare addresses here, we may have different symbol
+// contexts (for virtual inlined stacks), but we just want to know
+// that they are all at the same address.
+if (range_start_address.GetFileAddress() != run_to_addr)
+ break;
+const InlineFunctionInfo *inline_info =
inlined_parent->GetInlinedFunctionInfo();
+if (!inline_info)
+ break;
+const Declaration &call_site = inline_info->GetCallSite();
+top_most_line_entry.line = call_site.GetLine();
+top_most_line_entry.column = call_site.GetColumn();
+FileSpec call_site_file_spec = call_site.GetFile();
+top_most_line_entry.original_file_sp.reset(new
SupportFile(call_site_file_spec));
+top_most_line_entry.range = range;
+top_most_line_entry.file_sp.reset();
+
top_most_line_entry.ApplyFileMappings(GetThread().CalculateTarget());
+if (!top_most_line_entry.file_sp)
+ top_most_line_entry.file_sp =
top_most_line_entry.original_file_sp;
+ }
+ if (top_most_line_entry.IsValid()) {
+LLDB_LOG(log, "Setting preferred line entry: {0}:{1}",
top_most_line_entry.GetFile(), top_most_line_entry.line);
+bp_loc->SetPreferredLineEntry(top_most_line_entry);
+ }
+}
m_next_branch_bp_sp->SetThreadID(m_tid);
m_next_branch_bp_sp->SetBreakpointKind("next-branch-location");
diff --git
a/lldb/test/API/functionalities/inline-stepping/TestInlineStepping.py
b/lldb/test/API/functionalities/inline-stepping/TestInlineStepping.py
index 4e2d908e63b81c..b43bc71243f259 100644
--- a/lldb/test/API/functionalities/inline-stepping/TestInlineStepping.py
+++ b/lldb/test/API/functionalities/inline-stepping/TestInlineStepping.py
@@ -291,7 +291,7 @@ def inline_stepping_step_over(self):
break
[Lldb-commits] [lldb] More refinement of call site handling in stepping. (PR #114628)
llvmbot wrote:
@llvm/pr-subscribers-lldb
Author: None (jimingham)
Changes
When you set a "next branch breakpoint" and run to it while stepping, you have
to claim the stop at that breakpoint to be the top of the inlined call stack,
or you will seem to "step in" and then plans might try to step back out again.
This records the PrefferedLineEntry for next branch breakpoints and adds a test
to make sure this works.
---
Full diff: https://github.com/llvm/llvm-project/pull/114628.diff
2 Files Affected:
- (modified) lldb/source/Target/ThreadPlanStepRange.cpp (+39-3)
- (modified)
lldb/test/API/functionalities/inline-stepping/TestInlineStepping.py (+19-1)
``diff
diff --git a/lldb/source/Target/ThreadPlanStepRange.cpp
b/lldb/source/Target/ThreadPlanStepRange.cpp
index 3c825058b8c375..e53b189d49be44 100644
--- a/lldb/source/Target/ThreadPlanStepRange.cpp
+++ b/lldb/source/Target/ThreadPlanStepRange.cpp
@@ -379,10 +379,9 @@ bool ThreadPlanStepRange::SetNextBranchBreakpoint() {
!m_next_branch_bp_sp->HasResolvedLocations())
m_could_not_resolve_hw_bp = true;
+BreakpointLocationSP bp_loc =
m_next_branch_bp_sp->GetLocationAtIndex(0);
if (log) {
lldb::break_id_t bp_site_id = LLDB_INVALID_BREAK_ID;
- BreakpointLocationSP bp_loc =
- m_next_branch_bp_sp->GetLocationAtIndex(0);
if (bp_loc) {
BreakpointSiteSP bp_site = bp_loc->GetBreakpointSite();
if (bp_site) {
@@ -395,7 +394,44 @@ bool ThreadPlanStepRange::SetNextBranchBreakpoint() {
m_next_branch_bp_sp->GetID(), bp_site_id,
run_to_address.GetLoadAddress(&m_process.GetTarget()));
}
-
+// The "next branch breakpoint might land on an virtual inlined call
+// stack. If that's true, we should always stop at the top of the
+// inlined call stack. Only virtual steps should walk deeper into the
+// inlined call stack.
+Block *block = run_to_address.CalculateSymbolContextBlock();
+if (bp_loc && block) {
+ LineEntry top_most_line_entry;
+ lldb::addr_t run_to_addr = run_to_address.GetFileAddress();
+ for (Block *inlined_parent = block->GetContainingInlinedBlock();
inlined_parent;
+ inlined_parent = inlined_parent->GetInlinedParent()) {
+AddressRange range;
+if (!inlined_parent->GetRangeContainingAddress(run_to_address,
range))
+ break;
+Address range_start_address = range.GetBaseAddress();
+// Only compare addresses here, we may have different symbol
+// contexts (for virtual inlined stacks), but we just want to know
+// that they are all at the same address.
+if (range_start_address.GetFileAddress() != run_to_addr)
+ break;
+const InlineFunctionInfo *inline_info =
inlined_parent->GetInlinedFunctionInfo();
+if (!inline_info)
+ break;
+const Declaration &call_site = inline_info->GetCallSite();
+top_most_line_entry.line = call_site.GetLine();
+top_most_line_entry.column = call_site.GetColumn();
+FileSpec call_site_file_spec = call_site.GetFile();
+top_most_line_entry.original_file_sp.reset(new
SupportFile(call_site_file_spec));
+top_most_line_entry.range = range;
+top_most_line_entry.file_sp.reset();
+
top_most_line_entry.ApplyFileMappings(GetThread().CalculateTarget());
+if (!top_most_line_entry.file_sp)
+ top_most_line_entry.file_sp =
top_most_line_entry.original_file_sp;
+ }
+ if (top_most_line_entry.IsValid()) {
+LLDB_LOG(log, "Setting preferred line entry: {0}:{1}",
top_most_line_entry.GetFile(), top_most_line_entry.line);
+bp_loc->SetPreferredLineEntry(top_most_line_entry);
+ }
+}
m_next_branch_bp_sp->SetThreadID(m_tid);
m_next_branch_bp_sp->SetBreakpointKind("next-branch-location");
diff --git
a/lldb/test/API/functionalities/inline-stepping/TestInlineStepping.py
b/lldb/test/API/functionalities/inline-stepping/TestInlineStepping.py
index 4e2d908e63b81c..b43bc71243f259 100644
--- a/lldb/test/API/functionalities/inline-stepping/TestInlineStepping.py
+++ b/lldb/test/API/functionalities/inline-stepping/TestInlineStepping.py
@@ -291,7 +291,7 @@ def inline_stepping_step_over(self):
break_1_in_main = target.BreakpointCreateBySourceRegex(
"// At second call of caller_ref_1 in main.", self.main_source_spec
)
-self.assertTrue(break_1_in_main, VALID_BREAKPOINT)
+self.assertGreater(break_1_in_main.GetNumLocations(), 0,
VALID_BREAKPOINT)
# Now launch the process, and do not stop at entry point.
self.process = target.LaunchSimple(
@@ -317,6 +317,24 @@ def inline_stepping_step_over(self):
]
[Lldb-commits] [lldb] More refinement of call site handling in stepping. (PR #114628)
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
80b9f07436617d650bdab7035394705f643d1b19...042ac07ed67a5465aaf5c2dc8c4396adf5da2948
lldb/test/API/functionalities/inline-stepping/TestInlineStepping.py
``
View the diff from darker here.
``diff
--- TestInlineStepping.py 2024-11-02 00:23:12.00 +
+++ TestInlineStepping.py 2024-11-02 00:30:29.551403 +
@@ -327,15 +327,15 @@
self.assertEqual(len(threads), 1, "Hit our second breakpoint")
self.assertEqual(threads[0].id, self.thread.id, "Stopped at right
thread")
self.thread.StepOver()
frame_0 = self.thread.frames[0]
line_entry = frame_0.line_entry
-self.assertEqual(line_entry.file.basename,
self.main_source_spec.basename, "File matches")
+self.assertEqual(
+line_entry.file.basename, self.main_source_spec.basename, "File
matches"
+)
target_line = line_number("calling.cpp", "// At
caller_trivial_inline_1")
self.assertEqual(line_entry.line, target_line, "Lines match as well.")
-
-
def step_in_template(self):
"""Use Python APIs to test stepping in to templated functions."""
exe = self.getBuildArtifact("a.out")
``
https://github.com/llvm/llvm-project/pull/114628
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] More refinement of call site handling in stepping. (PR #114628)
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 80b9f07436617d650bdab7035394705f643d1b19
042ac07ed67a5465aaf5c2dc8c4396adf5da2948 --extensions cpp --
lldb/source/Target/ThreadPlanStepRange.cpp
``
View the diff from clang-format here.
``diff
diff --git a/lldb/source/Target/ThreadPlanStepRange.cpp
b/lldb/source/Target/ThreadPlanStepRange.cpp
index e53b189d49..6a0d8257fe 100644
--- a/lldb/source/Target/ThreadPlanStepRange.cpp
+++ b/lldb/source/Target/ThreadPlanStepRange.cpp
@@ -379,7 +379,8 @@ bool ThreadPlanStepRange::SetNextBranchBreakpoint() {
!m_next_branch_bp_sp->HasResolvedLocations())
m_could_not_resolve_hw_bp = true;
-BreakpointLocationSP bp_loc =
m_next_branch_bp_sp->GetLocationAtIndex(0);
+BreakpointLocationSP bp_loc =
+m_next_branch_bp_sp->GetLocationAtIndex(0);
if (log) {
lldb::break_id_t bp_site_id = LLDB_INVALID_BREAK_ID;
if (bp_loc) {
@@ -402,33 +403,40 @@ bool ThreadPlanStepRange::SetNextBranchBreakpoint() {
if (bp_loc && block) {
LineEntry top_most_line_entry;
lldb::addr_t run_to_addr = run_to_address.GetFileAddress();
- for (Block *inlined_parent = block->GetContainingInlinedBlock();
inlined_parent;
- inlined_parent = inlined_parent->GetInlinedParent()) {
+ for (Block *inlined_parent = block->GetContainingInlinedBlock();
+ inlined_parent;
+ inlined_parent = inlined_parent->GetInlinedParent()) {
AddressRange range;
-if (!inlined_parent->GetRangeContainingAddress(run_to_address,
range))
- break;
+if (!inlined_parent->GetRangeContainingAddress(run_to_address,
+ range))
+ break;
Address range_start_address = range.GetBaseAddress();
// Only compare addresses here, we may have different symbol
// contexts (for virtual inlined stacks), but we just want to know
// that they are all at the same address.
-if (range_start_address.GetFileAddress() != run_to_addr)
+if (range_start_address.GetFileAddress() != run_to_addr)
break;
-const InlineFunctionInfo *inline_info =
inlined_parent->GetInlinedFunctionInfo();
+const InlineFunctionInfo *inline_info =
+inlined_parent->GetInlinedFunctionInfo();
if (!inline_info)
break;
const Declaration &call_site = inline_info->GetCallSite();
top_most_line_entry.line = call_site.GetLine();
top_most_line_entry.column = call_site.GetColumn();
FileSpec call_site_file_spec = call_site.GetFile();
-top_most_line_entry.original_file_sp.reset(new
SupportFile(call_site_file_spec));
+top_most_line_entry.original_file_sp.reset(
+new SupportFile(call_site_file_spec));
top_most_line_entry.range = range;
top_most_line_entry.file_sp.reset();
-
top_most_line_entry.ApplyFileMappings(GetThread().CalculateTarget());
+top_most_line_entry.ApplyFileMappings(
+GetThread().CalculateTarget());
if (!top_most_line_entry.file_sp)
- top_most_line_entry.file_sp =
top_most_line_entry.original_file_sp;
+ top_most_line_entry.file_sp =
+ top_most_line_entry.original_file_sp;
}
if (top_most_line_entry.IsValid()) {
-LLDB_LOG(log, "Setting preferred line entry: {0}:{1}",
top_most_line_entry.GetFile(), top_most_line_entry.line);
+LLDB_LOG(log, "Setting preferred line entry: {0}:{1}",
+ top_most_line_entry.GetFile(), top_most_line_entry.line);
bp_loc->SetPreferredLineEntry(top_most_line_entry);
}
}
``
https://github.com/llvm/llvm-project/pull/114628
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 2bd21b2 - [lldb] Improve command status when dwim-print has no result (#114478)
Author: Dave Lee Date: 2024-11-01T13:39:14-07:00 New Revision: 2bd21b26e26e5a4b962307d30c8e6279b464bf02 URL: https://github.com/llvm/llvm-project/commit/2bd21b26e26e5a4b962307d30c8e6279b464bf02 DIFF: https://github.com/llvm/llvm-project/commit/2bd21b26e26e5a4b962307d30c8e6279b464bf02.diff LOG: [lldb] Improve command status when dwim-print has no result (#114478) When an expression produces no result, set `dwim-print` status to `eReturnStatusSuccessFinishNoResult`. Added: Modified: lldb/source/Commands/CommandObjectDWIMPrint.cpp Removed: diff --git a/lldb/source/Commands/CommandObjectDWIMPrint.cpp b/lldb/source/Commands/CommandObjectDWIMPrint.cpp index f5aa6a287b6f46..76bed100dc7291 100644 --- a/lldb/source/Commands/CommandObjectDWIMPrint.cpp +++ b/lldb/source/Commands/CommandObjectDWIMPrint.cpp @@ -231,7 +231,7 @@ void CommandObjectDWIMPrint::DoExecute(StringRef command, if (valobj_sp->GetError().GetError() != UserExpression::kNoResult) dump_val_object(*valobj_sp); else - result.SetStatus(eReturnStatusSuccessFinishResult); + result.SetStatus(eReturnStatusSuccessFinishNoResult); if (suppress_result) if (auto result_var_sp = ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Avoid unnecessary regex check in dwim-print (PR #114608)
https://github.com/kastiglione edited https://github.com/llvm/llvm-project/pull/114608 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Avoid unnecessary regex check in dwim-print (PR #114608)
https://github.com/augusto2112 approved this pull request. Nice! This code lives upstream though so you'll need to open a patch there too https://github.com/llvm/llvm-project/pull/114608 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Avoid unnecessary regex check in dwim-print (PR #114608)
kastiglione wrote: I will cherry-pick this downstream to swiftlang/llvm-project too. https://github.com/llvm/llvm-project/pull/114608 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Implement a formatter bytecode interpreter in C++ (PR #114333)
https://github.com/adrian-prantl updated
https://github.com/llvm/llvm-project/pull/114333
>From c9a4d24f222a70c7c108deebb6c25222893d7159 Mon Sep 17 00:00:00 2001
From: Dave Lee
Date: Wed, 24 Jan 2024 12:42:45 -0800
Subject: [PATCH 1/2] [lldb] Load embedded type summary section (#7859) (#8040)
Add support for type summaries embedded into the binary.
These embedded summaries will typically be generated by Swift macros,
but can also be generated by any other means.
rdar://115184658
---
lldb/include/lldb/lldb-enumerations.h | 1 +
lldb/source/Core/Section.cpp | 3 +
.../Plugins/ObjectFile/ELF/ObjectFileELF.cpp | 1 +
.../ObjectFile/Mach-O/ObjectFileMachO.cpp | 4 ++
.../ObjectFile/PECOFF/ObjectFilePECOFF.cpp| 1 +
lldb/source/Symbol/ObjectFile.cpp | 1 +
lldb/source/Target/Target.cpp | 72 +++
.../data-formatter/embedded-summary/Makefile | 2 +
.../TestEmbeddedTypeSummary.py| 12
.../data-formatter/embedded-summary/main.c| 22 ++
10 files changed, 119 insertions(+)
create mode 100644
lldb/test/API/functionalities/data-formatter/embedded-summary/Makefile
create mode 100644
lldb/test/API/functionalities/data-formatter/embedded-summary/TestEmbeddedTypeSummary.py
create mode 100644
lldb/test/API/functionalities/data-formatter/embedded-summary/main.c
diff --git a/lldb/include/lldb/lldb-enumerations.h
b/lldb/include/lldb/lldb-enumerations.h
index 938f6e3abe8f2a..1ca4aa62218c09 100644
--- a/lldb/include/lldb/lldb-enumerations.h
+++ b/lldb/include/lldb/lldb-enumerations.h
@@ -761,6 +761,7 @@ enum SectionType {
eSectionTypeDWARFDebugLocListsDwo,
eSectionTypeDWARFDebugTuIndex,
eSectionTypeCTF,
+ eSectionTypeLLDBTypeSummaries,
eSectionTypeSwiftModules,
};
diff --git a/lldb/source/Core/Section.cpp b/lldb/source/Core/Section.cpp
index 0763e88d4608f4..ee01b4ce06ca1e 100644
--- a/lldb/source/Core/Section.cpp
+++ b/lldb/source/Core/Section.cpp
@@ -147,6 +147,8 @@ const char *Section::GetTypeAsCString() const {
return "dwarf-gnu-debugaltlink";
case eSectionTypeCTF:
return "ctf";
+ case eSectionTypeLLDBTypeSummaries:
+return "lldb-type-summaries";
case eSectionTypeOther:
return "regular";
case eSectionTypeSwiftModules:
@@ -457,6 +459,7 @@ bool Section::ContainsOnlyDebugInfo() const {
case eSectionTypeDWARFAppleObjC:
case eSectionTypeDWARFGNUDebugAltLink:
case eSectionTypeCTF:
+ case eSectionTypeLLDBTypeSummaries:
case eSectionTypeSwiftModules:
return true;
}
diff --git a/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
b/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
index 10d09662c0a47a..ad4a84ef02bf72 100644
--- a/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
+++ b/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
@@ -1678,6 +1678,7 @@ static SectionType GetSectionTypeFromName(llvm::StringRef
Name) {
.Case(".gnu_debugaltlink", eSectionTypeDWARFGNUDebugAltLink)
.Case(".gosymtab", eSectionTypeGoSymtab)
.Case(".text", eSectionTypeCode)
+ .Case(".lldbsummaries", lldb::eSectionTypeLLDBTypeSummaries)
.Case(".swift_ast", eSectionTypeSwiftModules)
.Default(eSectionTypeOther);
}
diff --git a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
index b542e237f023d4..d6bec5d84aea19 100644
--- a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
+++ b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
@@ -1209,6 +1209,7 @@ AddressClass
ObjectFileMachO::GetAddressClass(lldb::addr_t file_addr) {
case eSectionTypeDWARFAppleObjC:
case eSectionTypeDWARFGNUDebugAltLink:
case eSectionTypeCTF:
+case eSectionTypeLLDBTypeSummaries:
case eSectionTypeSwiftModules:
return AddressClass::eDebug;
@@ -1484,6 +1485,7 @@ static lldb::SectionType GetSectionType(uint32_t flags,
static ConstString g_sect_name_data("__data");
static ConstString g_sect_name_go_symtab("__gosymtab");
static ConstString g_sect_name_ctf("__ctf");
+ static ConstString g_sect_name_lldb_summaries("__lldbsummaries");
static ConstString g_sect_name_swift_ast("__swift_ast");
if (section_name == g_sect_name_dwarf_debug_abbrev)
@@ -1564,6 +1566,8 @@ static lldb::SectionType GetSectionType(uint32_t flags,
return eSectionTypeGoSymtab;
if (section_name == g_sect_name_ctf)
return eSectionTypeCTF;
+ if (section_name == g_sect_name_lldb_summaries)
+return lldb::eSectionTypeLLDBTypeSummaries;
if (section_name == g_sect_name_swift_ast)
return eSectionTypeSwiftModules;
if (section_name == g_sect_name_objc_data ||
diff --git a/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
b/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
index 8d9c919bc9b101..bb712da7f6d67d 100644
--- a/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
+++ b/lldb
[Lldb-commits] [lldb] More refinement of call site handling in stepping. (PR #114628)
https://github.com/adrian-prantl approved this pull request. https://github.com/llvm/llvm-project/pull/114628 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] More refinement of call site handling in stepping. (PR #114628)
@@ -395,7 +394,44 @@ bool ThreadPlanStepRange::SetNextBranchBreakpoint() {
m_next_branch_bp_sp->GetID(), bp_site_id,
run_to_address.GetLoadAddress(&m_process.GetTarget()));
}
-
+// The "next branch breakpoint might land on an virtual inlined call
adrian-prantl wrote:
```suggestion
// The "next branch breakpoint might land on a virtual inlined call
```
https://github.com/llvm/llvm-project/pull/114628
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] d6a1501 - [lldb] Skip TestDAP_completions on older versions of libcxx
Author: Felipe de Azevedo Piovezan
Date: 2024-11-01T08:55:23-07:00
New Revision: d6a150137773bd1d104aa5a1847863a5138f14d9
URL:
https://github.com/llvm/llvm-project/commit/d6a150137773bd1d104aa5a1847863a5138f14d9
DIFF:
https://github.com/llvm/llvm-project/commit/d6a150137773bd1d104aa5a1847863a5138f14d9.diff
LOG: [lldb] Skip TestDAP_completions on older versions of libcxx
Added:
Modified:
lldb/test/API/tools/lldb-dap/completions/TestDAP_completions.py
Removed:
diff --git a/lldb/test/API/tools/lldb-dap/completions/TestDAP_completions.py
b/lldb/test/API/tools/lldb-dap/completions/TestDAP_completions.py
index 8ad12b3fc2f6b1..210e591bff426f 100644
--- a/lldb/test/API/tools/lldb-dap/completions/TestDAP_completions.py
+++ b/lldb/test/API/tools/lldb-dap/completions/TestDAP_completions.py
@@ -32,6 +32,9 @@
variable_var1_completion = {"text": "var1", "label": "var1 -- int &"}
variable_var2_completion = {"text": "var2", "label": "var2 -- int &"}
+# Older version of libcxx produce slightly
diff erent typename strings for
+# templates like vector.
+@skipIf(compiler="clang", compiler_version=["<", "16.0"])
class TestDAP_completions(lldbdap_testcase.DAPTestCaseBase):
def verify_completions(self, actual_list, expected_list,
not_expected_list=[]):
for expected_item in expected_list:
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] f33b5b7 - [lldb] INSTANTIATE_TEST_CASE_P -> INSTANTIATE_TEST_SUITE_P
Author: Jonas Devlieghere
Date: 2024-11-01T11:03:19-07:00
New Revision: f33b5b79372c7a46e2e6b89a314bce0009bf4ab7
URL:
https://github.com/llvm/llvm-project/commit/f33b5b79372c7a46e2e6b89a314bce0009bf4ab7
DIFF:
https://github.com/llvm/llvm-project/commit/f33b5b79372c7a46e2e6b89a314bce0009bf4ab7.diff
LOG: [lldb] INSTANTIATE_TEST_CASE_P -> INSTANTIATE_TEST_SUITE_P
INSTANTIATE_TEST_CASE_P is deprecated in favor of
INSTANTIATE_TEST_SUITE_P.
Added:
Modified:
lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp
Removed:
diff --git a/lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp
b/lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp
index dc5680522e1836..f1998a92f19b05 100644
--- a/lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp
+++ b/lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp
@@ -312,6 +312,6 @@ SDKPathParsingTestData sdkPathParsingTestCases[] = {
.expect_sdk_path_pattern = "iPhoneOS14.1.sdk"},
};
-INSTANTIATE_TEST_CASE_P(SDKPathParsingTests, SDKPathParsingMultiparamTests,
-::testing::ValuesIn(sdkPathParsingTestCases));
+INSTANTIATE_TEST_SUITE_P(SDKPathParsingTests, SDKPathParsingMultiparamTests,
+ ::testing::ValuesIn(sdkPathParsingTestCases));
#endif
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 16a6c10 - [lldb] Fix warning: comparison of integers of different signs
Author: Jonas Devlieghere
Date: 2024-11-01T11:06:18-07:00
New Revision: 16a6c10bd485979ba2edf4b487d633230a9df01f
URL:
https://github.com/llvm/llvm-project/commit/16a6c10bd485979ba2edf4b487d633230a9df01f
DIFF:
https://github.com/llvm/llvm-project/commit/16a6c10bd485979ba2edf4b487d633230a9df01f.diff
LOG: [lldb] Fix warning: comparison of integers of different signs
Added:
Modified:
lldb/unittests/ScriptInterpreter/Lua/ScriptInterpreterTests.cpp
Removed:
diff --git a/lldb/unittests/ScriptInterpreter/Lua/ScriptInterpreterTests.cpp
b/lldb/unittests/ScriptInterpreter/Lua/ScriptInterpreterTests.cpp
index 214de14f73ff9e..5f8cd5aef56b62 100644
--- a/lldb/unittests/ScriptInterpreter/Lua/ScriptInterpreterTests.cpp
+++ b/lldb/unittests/ScriptInterpreter/Lua/ScriptInterpreterTests.cpp
@@ -49,5 +49,6 @@ TEST_F(ScriptInterpreterTest, ExecuteOneLine) {
EXPECT_TRUE(script_interpreter.ExecuteOneLine("foo = 1", &result));
EXPECT_FALSE(script_interpreter.ExecuteOneLine("nil = foo", &result));
EXPECT_EQ(result.GetErrorString().find(
- "error: lua failed attempting to evaluate 'nil = foo'"), 0 );
+"error: lua failed attempting to evaluate 'nil = foo'"),
+0U);
}
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] c3782f6 - [lldb] Disable automatically opening editor for TestSessionSave (#114469)
Author: Alex Langford Date: 2024-11-01T11:12:29-07:00 New Revision: c3782f67daf462f6b289b5898cb65c61f64c197d URL: https://github.com/llvm/llvm-project/commit/c3782f67daf462f6b289b5898cb65c61f64c197d DIFF: https://github.com/llvm/llvm-project/commit/c3782f67daf462f6b289b5898cb65c61f64c197d.diff LOG: [lldb] Disable automatically opening editor for TestSessionSave (#114469) Added: Modified: lldb/test/API/commands/session/save/TestSessionSave.py Removed: diff --git a/lldb/test/API/commands/session/save/TestSessionSave.py b/lldb/test/API/commands/session/save/TestSessionSave.py index 0f064e60844fe2..e5913f8edfe8fc 100644 --- a/lldb/test/API/commands/session/save/TestSessionSave.py +++ b/lldb/test/API/commands/session/save/TestSessionSave.py @@ -103,6 +103,7 @@ def test_session_save_no_transcript_warning(self): # These commands won't be saved, so are arbitrary. commands = [ +"settings set interpreter.open-transcript-in-editor false", "p 1", "settings set interpreter.save-session-on-quit true", "fr v", ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Disable automatically opening editor for TestSessionSave (PR #114469)
https://github.com/bulbazord closed https://github.com/llvm/llvm-project/pull/114469 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 0cfcd38 - [lldb][NativePDB] Parse global variables. (#114303)
Author: Zequan Wu
Date: 2024-11-01T14:15:54-04:00
New Revision: 0cfcd387f968f2c9de0648673b5db9e221e5c84e
URL:
https://github.com/llvm/llvm-project/commit/0cfcd387f968f2c9de0648673b5db9e221e5c84e
DIFF:
https://github.com/llvm/llvm-project/commit/0cfcd387f968f2c9de0648673b5db9e221e5c84e.diff
LOG: [lldb][NativePDB] Parse global variables. (#114303)
This doesn't parse S_CONSTANT case yet, because I found that the global
variable `std::strong_ordering::equal` is a S_CONSTANT and has type of
LF_STRUCTURE which is not currently handled when creating dwarf
expression for the variable. Left a TODO for it to finish later.
This makes `lldb/test/Shell/SymbolFile/PDB/ast-restore.test` and
`lldb/test/Shell/SymbolFile/PDB/calling-conventions-x86.test` pass on
windows with native pdb plugin only.
Added:
Modified:
lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
lldb/test/Shell/SymbolFile/NativePDB/ast-methods.cpp
lldb/test/Shell/SymbolFile/NativePDB/global-ctor-dtor.cpp
lldb/test/Shell/SymbolFile/PDB/ast-restore.test
Removed:
diff --git a/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
b/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
index 7fded6a31a3af5..c784c2e28f6452 100644
--- a/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
+++ b/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
@@ -888,9 +888,9 @@ VariableSP
SymbolFileNativePDB::CreateGlobalVariable(PdbGlobalSymId var_id) {
CompUnitSP comp_unit;
std::optional modi = m_index->GetModuleIndexForVa(addr);
- if (!modi) {
+ // Some globals has modi points to the linker module, ignore them.
+ if (!modi || modi >= GetNumCompileUnits())
return nullptr;
- }
CompilandIndexItem &cci = m_index->compilands().GetOrCreateCompiland(*modi);
comp_unit = GetOrCreateCompileUnit(cci);
@@ -1810,7 +1810,27 @@
SymbolFileNativePDB::ParseVariablesForCompileUnit(CompileUnit &comp_unit,
VariableList &variables) {
PdbSymUid sym_uid(comp_unit.GetID());
lldbassert(sym_uid.kind() == PdbSymUidKind::Compiland);
- return 0;
+ for (const uint32_t gid : m_index->globals().getGlobalsTable()) {
+PdbGlobalSymId global{gid, false};
+CVSymbol sym = m_index->ReadSymbolRecord(global);
+// TODO: S_CONSTANT is not handled here to prevent a possible crash in
+// lldb_private::npdb::MakeConstantLocationExpression when it's a record
+// type (e.g. std::strong_ordering::equal). That function needs to be
+// updated to handle this case when we add S_CONSTANT case here.
+switch (sym.kind()) {
+case SymbolKind::S_GDATA32:
+case SymbolKind::S_LDATA32:
+case SymbolKind::S_GTHREAD32:
+case SymbolKind::S_LTHREAD32: {
+ if (VariableSP var = GetOrCreateGlobalVariable(global))
+variables.AddVariable(var);
+ break;
+}
+default:
+ break;
+}
+ }
+ return variables.GetSize();
}
VariableSP SymbolFileNativePDB::CreateLocalVariable(PdbCompilandSymId scope_id,
diff --git a/lldb/test/Shell/SymbolFile/NativePDB/ast-methods.cpp
b/lldb/test/Shell/SymbolFile/NativePDB/ast-methods.cpp
index 91bd5bb810c8e3..c90eaefe298035 100644
--- a/lldb/test/Shell/SymbolFile/NativePDB/ast-methods.cpp
+++ b/lldb/test/Shell/SymbolFile/NativePDB/ast-methods.cpp
@@ -44,8 +44,7 @@ int main(int argc, char **argv) {
// AST: | |-ParmVarDecl {{.*}} 'char'
// AST: | `-ParmVarDecl {{.*}} 'int'
-// SYMBOL: int main(int argc, char **argv);
-// SYMBOL-NEXT: struct Struct {
+// SYMBOL: struct Struct {
// SYMBOL-NEXT: void simple_method();
// SYMBOL-NEXT: static void static_method();
// SYMBOL-NEXT: virtual void virtual_method();
@@ -53,3 +52,5 @@ int main(int argc, char **argv) {
// SYMBOL-NEXT: int overloaded_method(char);
// SYMBOL-NEXT: int overloaded_method(char, int, ...);
// SYMBOL-NEXT: };
+// SYMBOL-NEXT: Struct s;
+// SYMBOL-NEXT: int main(int argc, char **argv);
diff --git a/lldb/test/Shell/SymbolFile/NativePDB/global-ctor-dtor.cpp
b/lldb/test/Shell/SymbolFile/NativePDB/global-ctor-dtor.cpp
index 5f6c68d69023ef..e34e6eb7bf54a1 100644
--- a/lldb/test/Shell/SymbolFile/NativePDB/global-ctor-dtor.cpp
+++ b/lldb/test/Shell/SymbolFile/NativePDB/global-ctor-dtor.cpp
@@ -18,13 +18,14 @@ int main() {
return 0;
}
-// CHECK: static void B::`dynamic initializer for 'glob'();
+// CHECK: struct A {
+// CHECK-NEXT: ~A();
+// CHECK-NEXT: };
+// CHECK-NEXT: A B::glob;
+// CHECK-NEXT: static void B::`dynamic initializer for 'glob'();
// CHECK-NEXT: static void B::`dynamic atexit destructor for 'glob'();
// CHECK-NEXT: int main();
// CHECK-NEXT: static void _GLOBAL__sub_I_global_ctor_dtor.cpp();
-// CHECK-NEXT: struct A {
-// CHECK-NEXT: ~A();
-// CHECK-NEXT: };
// CHECK-NEXT: struct B {
// CHECK-NEXT: static A glob;
// CHE
[Lldb-commits] [lldb] [lldb][NativePDB] Parse global variables. (PR #114303)
https://github.com/ZequanWu edited https://github.com/llvm/llvm-project/pull/114303 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][NativePDB] Parse global variables. (PR #114303)
https://github.com/ZequanWu closed https://github.com/llvm/llvm-project/pull/114303 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][test] Fix `LibCxxInternalsRecognizerTestCase` on clang <= 17 (PR #114122)
felipepiovezan wrote:
It's interesting that the 17.0 bot is still not happy:
```
File
"/Users/ec2-user/jenkins/workspace/llvm.org/lldb-cmake-matrix/llvm-project/lldb/packages/Python/lldbsuite/test/lldbtest.py",
line 321, in check_value
test_base.assertEqual(
AssertionError: '"Hello"' != None : Checking child with index 0:
(std::__lldb::optional) maybe_string = Has Value=true {
Value = 0x00010a2f3f9e
}
Checking SBValue: (std::__1::remove_cv_t) Value = 0x00010a2f3f9e
Config=x86_64-/Users/ec2-user/jenkins/workspace/llvm.org/lldb-cmake-matrix/clang_1706_build/bin/clang
```
https://green.lab.llvm.org/job/llvm.org/view/LLDB/job/lldb-cmake-matrix/925/execution/node/135/log/
https://github.com/llvm/llvm-project/pull/114122
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Improve locking in PathMappingLists (NFC) (PR #114576)
llvmbot wrote:
@llvm/pr-subscribers-lldb
Author: Jonas Devlieghere (JDevlieghere)
Changes
In D148380 [1], Alex added locking to PathMappingLists. The current
implementation runs the callback under the lock, which I don't believe is
necessary. As far as I can tell, no users of the callback are relying on the
list not having been modified until the callback is handled.
This patch implements my suggestion to unlock the mutex before the callback. I
also switched to a non-recursive mutex as I don't believe the recursive
property is needed. To make the class fully thread safe, I did have to
introduce another mutex to protect the callback members.
The motivation for this change is #114507. Specifically,
Target::SetExecutableModule calls Target::GetOrCreateModule, which potentially
performs path remapping, which has a callback to Target::SetExecutableModule.
[1] https://reviews.llvm.org/D148380
---
Full diff: https://github.com/llvm/llvm-project/pull/114576.diff
2 Files Affected:
- (modified) lldb/include/lldb/Target/PathMappingList.h (+13-6)
- (modified) lldb/source/Target/PathMappingList.cpp (+94-69)
``diff
diff --git a/lldb/include/lldb/Target/PathMappingList.h
b/lldb/include/lldb/Target/PathMappingList.h
index 1c0ff564739c5a..a59539eb0e4bc6 100644
--- a/lldb/include/lldb/Target/PathMappingList.h
+++ b/lldb/include/lldb/Target/PathMappingList.h
@@ -25,7 +25,6 @@ class PathMappingList {
typedef void (*ChangedCallback)(const PathMappingList &path_list,
void *baton);
- // Constructors and Destructors
PathMappingList();
PathMappingList(ChangedCallback callback, void *callback_baton);
@@ -53,12 +52,12 @@ class PathMappingList {
llvm::json::Value ToJSON();
bool IsEmpty() const {
-std::lock_guard lock(m_mutex);
+std::lock_guard lock(m_pairs_mutex);
return m_pairs.empty();
}
size_t GetSize() const {
-std::lock_guard lock(m_mutex);
+std::lock_guard lock(m_pairs_mutex);
return m_pairs.size();
}
@@ -137,25 +136,33 @@ class PathMappingList {
uint32_t FindIndexForPath(llvm::StringRef path) const;
uint32_t GetModificationID() const {
-std::lock_guard lock(m_mutex);
+std::lock_guard lock(m_pairs_mutex);
return m_mod_id;
}
protected:
- mutable std::recursive_mutex m_mutex;
typedef std::pair pair;
typedef std::vector collection;
typedef collection::iterator iterator;
typedef collection::const_iterator const_iterator;
+ void AppendImpl(llvm::StringRef path, llvm::StringRef replacement);
+ void Notify(bool notify) const;
+
iterator FindIteratorForPath(ConstString path);
const_iterator FindIteratorForPath(ConstString path) const;
collection m_pairs;
+ mutable std::mutex m_pairs_mutex;
+
ChangedCallback m_callback = nullptr;
void *m_callback_baton = nullptr;
- uint32_t m_mod_id = 0; // Incremented anytime anything is added or removed.
+ mutable std::mutex m_callback_mutex;
+
+ /// Incremented anytime anything is added to or removed from m_pairs. Also
+ /// protected by m_pairs_mutex.
+ uint32_t m_mod_id = 0;
};
} // namespace lldb_private
diff --git a/lldb/source/Target/PathMappingList.cpp
b/lldb/source/Target/PathMappingList.cpp
index 9c283b0146fe07..7343522fafef20 100644
--- a/lldb/source/Target/PathMappingList.cpp
+++ b/lldb/source/Target/PathMappingList.cpp
@@ -48,7 +48,10 @@ PathMappingList::PathMappingList(const PathMappingList &rhs)
const PathMappingList &PathMappingList::operator=(const PathMappingList &rhs) {
if (this != &rhs) {
-std::scoped_lock
locks(m_mutex, rhs.m_mutex);
+std::scoped_lock pairs_locks(m_pairs_mutex,
+ rhs.m_pairs_mutex);
+std::scoped_lock callback_locks(
+m_callback_mutex, rhs.m_callback_mutex);
m_pairs = rhs.m_pairs;
m_callback = nullptr;
m_callback_baton = nullptr;
@@ -59,85 +62,105 @@ const PathMappingList &PathMappingList::operator=(const
PathMappingList &rhs) {
PathMappingList::~PathMappingList() = default;
-void PathMappingList::Append(llvm::StringRef path, llvm::StringRef replacement,
- bool notify) {
- std::lock_guard lock(m_mutex);
+void PathMappingList::AppendImpl(llvm::StringRef path,
+ llvm::StringRef replacement) {
++m_mod_id;
m_pairs.emplace_back(pair(NormalizePath(path), NormalizePath(replacement)));
+}
+
+void PathMappingList::Notify(bool notify) const {
+ std::lock_guard lock(m_callback_mutex);
if (notify && m_callback)
m_callback(*this, m_callback_baton);
}
+void PathMappingList::Append(llvm::StringRef path, llvm::StringRef replacement,
+ bool notify) {
+ {
+std::lock_guard lock(m_pairs_mutex);
+AppendImpl(path, replacement);
+ }
+ Notify(notify);
+}
+
void PathMappingList::Append(const PathMappingList &rhs, bool notify) {
- std::scoped_lock locks(m_mutex,
rhs.m
[Lldb-commits] [lldb] [lldb] Create dependent modules in parallel (PR #114507)
JDevlieghere wrote: The test failures were both caused by the locking in `PathMappingList`, which I'm addressing in #114576. https://github.com/llvm/llvm-project/pull/114507 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Improve locking in PathMappingLists (NFC) (PR #114576)
https://github.com/JDevlieghere created
https://github.com/llvm/llvm-project/pull/114576
In D148380 [1], Alex added locking to PathMappingLists. The current
implementation runs the callback under the lock, which I don't believe is
necessary. As far as I can tell, no users of the callback are relying on the
list not having been modified until the callback is handled.
This patch implements my suggestion to unlock the mutex before the callback. I
also switched to a non-recursive mutex as I don't believe the recursive
property is needed. To make the class fully thread safe, I did have to
introduce another mutex to protect the callback members.
The motivation for this change is #114507. Specifically,
Target::SetExecutableModule calls Target::GetOrCreateModule, which potentially
performs path remapping, which has a callback to Target::SetExecutableModule.
[1] https://reviews.llvm.org/D148380
>From 9ebfdc84c46a795d35b4dd157573b97a57b232ce Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere
Date: Fri, 1 Nov 2024 09:58:28 -0700
Subject: [PATCH] [lldb] Improve locking in PathMappingLists (NFC)
In D148380 [1], Alex added locking to PathMappingLists. The current
implementation runs the callback under the lock, which I don't believe
is necessary. As far as I can tell, no users of the callback are relying
on the list not having been modified until the callback is handled.
This patch implements my suggestion to unlock the mutex before the
callback. I also switched to a non-recursive mutex as I don't believe
the recursive property is needed. To make the class fully thread safe, I
did have to introduce another mutex to protect the callback members.
The motivation for this change is #114507. Specifically,
Target::SetExecutableModule calls Target::GetOrCreateModule, which
potentially performs path remapping, which has a callback to
Target::SetExecutableModule.
[1] https://reviews.llvm.org/D148380
---
lldb/include/lldb/Target/PathMappingList.h | 19 ++-
lldb/source/Target/PathMappingList.cpp | 163 -
2 files changed, 107 insertions(+), 75 deletions(-)
diff --git a/lldb/include/lldb/Target/PathMappingList.h
b/lldb/include/lldb/Target/PathMappingList.h
index 1c0ff564739c5a..a59539eb0e4bc6 100644
--- a/lldb/include/lldb/Target/PathMappingList.h
+++ b/lldb/include/lldb/Target/PathMappingList.h
@@ -25,7 +25,6 @@ class PathMappingList {
typedef void (*ChangedCallback)(const PathMappingList &path_list,
void *baton);
- // Constructors and Destructors
PathMappingList();
PathMappingList(ChangedCallback callback, void *callback_baton);
@@ -53,12 +52,12 @@ class PathMappingList {
llvm::json::Value ToJSON();
bool IsEmpty() const {
-std::lock_guard lock(m_mutex);
+std::lock_guard lock(m_pairs_mutex);
return m_pairs.empty();
}
size_t GetSize() const {
-std::lock_guard lock(m_mutex);
+std::lock_guard lock(m_pairs_mutex);
return m_pairs.size();
}
@@ -137,25 +136,33 @@ class PathMappingList {
uint32_t FindIndexForPath(llvm::StringRef path) const;
uint32_t GetModificationID() const {
-std::lock_guard lock(m_mutex);
+std::lock_guard lock(m_pairs_mutex);
return m_mod_id;
}
protected:
- mutable std::recursive_mutex m_mutex;
typedef std::pair pair;
typedef std::vector collection;
typedef collection::iterator iterator;
typedef collection::const_iterator const_iterator;
+ void AppendImpl(llvm::StringRef path, llvm::StringRef replacement);
+ void Notify(bool notify) const;
+
iterator FindIteratorForPath(ConstString path);
const_iterator FindIteratorForPath(ConstString path) const;
collection m_pairs;
+ mutable std::mutex m_pairs_mutex;
+
ChangedCallback m_callback = nullptr;
void *m_callback_baton = nullptr;
- uint32_t m_mod_id = 0; // Incremented anytime anything is added or removed.
+ mutable std::mutex m_callback_mutex;
+
+ /// Incremented anytime anything is added to or removed from m_pairs. Also
+ /// protected by m_pairs_mutex.
+ uint32_t m_mod_id = 0;
};
} // namespace lldb_private
diff --git a/lldb/source/Target/PathMappingList.cpp
b/lldb/source/Target/PathMappingList.cpp
index 9c283b0146fe07..7343522fafef20 100644
--- a/lldb/source/Target/PathMappingList.cpp
+++ b/lldb/source/Target/PathMappingList.cpp
@@ -48,7 +48,10 @@ PathMappingList::PathMappingList(const PathMappingList &rhs)
const PathMappingList &PathMappingList::operator=(const PathMappingList &rhs) {
if (this != &rhs) {
-std::scoped_lock
locks(m_mutex, rhs.m_mutex);
+std::scoped_lock pairs_locks(m_pairs_mutex,
+ rhs.m_pairs_mutex);
+std::scoped_lock callback_locks(
+m_callback_mutex, rhs.m_callback_mutex);
m_pairs = rhs.m_pairs;
m_callback = nullptr;
m_callback_baton = nullptr;
@@ -59,85 +62,105 @@ const PathMappingList &PathMappingList::operator=(const
PathMappingList &rhs)
[Lldb-commits] [lldb] [lldb] Improve locking in PathMappingLists (NFC) (PR #114576)
https://github.com/JDevlieghere edited https://github.com/llvm/llvm-project/pull/114576 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Create dependent modules in parallel (PR #114507)
https://github.com/JDevlieghere updated
https://github.com/llvm/llvm-project/pull/114507
>From 9a269fa83cea529f704c9eba339eac9987032155 Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere
Date: Thu, 31 Oct 2024 21:42:38 -0700
Subject: [PATCH 1/3] [lldb] Create dependent modules in parallel
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Create dependent modules in parallel in Target::SetExecutableModule.
This change was inspired by #110646 which takes the same approach when
attaching. Jason suggested we could use the same approach when you
create a target in LLDB.
I used Slack for benchmarking, which loads 902 images.
Benchmark 1: ./bin/lldb /Applications/Slack.app/Contents/MacOS/Slack
Time (mean ± σ): 1.225 s ± 0.003 s[User: 3.977 s, System: 1.521 s]
Range (min … max):1.220 s … 1.229 s10 runs
Benchmark 2: ./bin/lldb /Applications/Slack.app/Contents/MacOS/Slack
Time (mean ± σ): 3.253 s ± 0.037 s[User: 3.013 s, System: 0.248 s]
Range (min … max):3.211 s … 3.310 s10 runs
We see about a 2x speedup, which matches what Jason saw for the attach
scenario. I also ran this under TSan to confirm this doesn't introduce
any races or deadlocks.
---
lldb/source/Target/Target.cpp | 33 -
1 file changed, 28 insertions(+), 5 deletions(-)
diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp
index 199efae8a728cc..ef5d38fc796b08 100644
--- a/lldb/source/Target/Target.cpp
+++ b/lldb/source/Target/Target.cpp
@@ -68,6 +68,7 @@
#include "llvm/ADT/ScopeExit.h"
#include "llvm/ADT/SetVector.h"
+#include "llvm/Support/ThreadPool.h"
#include
#include
@@ -1575,7 +1576,6 @@ void Target::SetExecutableModule(ModuleSP &executable_sp,
m_arch.GetSpec().GetTriple().getTriple());
}
-FileSpecList dependent_files;
ObjectFile *executable_objfile = executable_sp->GetObjectFile();
bool load_dependents = true;
switch (load_dependent_files) {
@@ -1591,10 +1591,14 @@ void Target::SetExecutableModule(ModuleSP
&executable_sp,
}
if (executable_objfile && load_dependents) {
+ // FileSpecList is not thread safe and needs to be synchronized.
+ FileSpecList dependent_files;
+ std::mutex dependent_files_mutex;
+
+ // ModuleList is thread safe.
ModuleList added_modules;
- executable_objfile->GetDependentModules(dependent_files);
- for (uint32_t i = 0; i < dependent_files.GetSize(); i++) {
-FileSpec dependent_file_spec(dependent_files.GetFileSpecAtIndex(i));
+
+ auto GetDependentModules = [&](FileSpec dependent_file_spec) {
FileSpec platform_dependent_file_spec;
if (m_platform_sp)
m_platform_sp->GetFileWithUUID(dependent_file_spec, nullptr,
@@ -1608,9 +1612,28 @@ void Target::SetExecutableModule(ModuleSP &executable_sp,
if (image_module_sp) {
added_modules.AppendIfNeeded(image_module_sp, false);
ObjectFile *objfile = image_module_sp->GetObjectFile();
- if (objfile)
+ if (objfile) {
+std::lock_guard guard(dependent_files_mutex);
objfile->GetDependentModules(dependent_files);
+ }
+}
+ };
+
+ executable_objfile->GetDependentModules(dependent_files);
+
+ llvm::ThreadPoolTaskGroup task_group(Debugger::GetThreadPool());
+ for (uint32_t i = 0; i < dependent_files.GetSize(); i++) {
+// Process all currently known dependencies in parallel in the
innermost
+// loop. This may create newly discovered dependencies to be appended
to
+// dependent_files. We'll deal with these files during the next
+// iteration of the outermost loop.
+{
+ std::lock_guard guard(dependent_files_mutex);
+ for (; i < dependent_files.GetSize(); i++)
+task_group.async(GetDependentModules,
+dependent_files.GetFileSpecAtIndex(i));
}
+task_group.wait();
}
ModulesDidLoad(added_modules);
}
>From 0bba5f799aadb4bd987e7aa29ca727562364b915 Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere
Date: Thu, 31 Oct 2024 21:54:26 -0700
Subject: [PATCH 2/3] Fix formatting
---
lldb/source/Target/Target.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp
index ef5d38fc796b08..ad38e6138cf0c6 100644
--- a/lldb/source/Target/Target.cpp
+++ b/lldb/source/Target/Target.cpp
@@ -1631,7 +1631,7 @@ void Target::SetExecutableModule(ModuleSP &executable_sp,
std::lock_guard guard(dependent_files_mutex);
for (; i < dependent_files.GetSize(); i++)
task_group.async(GetDependentModules,
-dependent_files.GetFileSpecAtIndex(i));
+ dependent_files.GetFileSpecAtIndex(i));
}
task_group.wait();
}
>Fro
[Lldb-commits] [lldb] [lldb] Create dependent modules in parallel (PR #114507)
@@ -1608,9 +1612,28 @@ void Target::SetExecutableModule(ModuleSP &executable_sp,
if (image_module_sp) {
added_modules.AppendIfNeeded(image_module_sp, false);
ObjectFile *objfile = image_module_sp->GetObjectFile();
- if (objfile)
+ if (objfile) {
+std::lock_guard guard(dependent_files_mutex);
objfile->GetDependentModules(dependent_files);
JDevlieghere wrote:
Yeah fair enough. Even the Mach-O one is doing some filesystem operations that
really shouldn't be happening while holding the lock. I don't think we need to
make every FileSpecList thread safe, and passing an optional `mutex*` was
messy, so I went with the local copy as Jason suggested.
https://github.com/llvm/llvm-project/pull/114507
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Fix pointer to reference type (PR #113596)
vogelsgesang wrote: FWIW, codelldb also dereferences pointers and references when displaying values. They [provide a user-facing setting](https://github.com/vadimcn/codelldb/blob/v1.10.0/MANUAL.md#pointers) for it and [implement it in the debug adapter](https://github.com/vadimcn/codelldb/blob/fe108b53a0f276c10f02d57d85b93991bbb6e30b/adapter/codelldb/src/debug_session/variables.rs#L290), instead of at the core of LLDB https://github.com/llvm/llvm-project/pull/113596 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][test] Fix `LibCxxInternalsRecognizerTestCase` on clang <= 17 (PR #114122)
Michael137 wrote:
> It's interesting that the 17.0 bot is still not happy:
>
>
>
> ```
>
> File
> "/Users/ec2-user/jenkins/workspace/llvm.org/lldb-cmake-matrix/llvm-project/lldb/packages/Python/lldbsuite/test/lldbtest.py",
> line 321, in check_value
>
> test_base.assertEqual(
>
> AssertionError: '"Hello"' != None : Checking child with index 0:
>
> (std::__lldb::optional) maybe_string = Has Value=true {
>
> Value = 0x00010a2f3f9e
>
> }
>
> Checking SBValue: (std::__1::remove_cv_t) Value =
> 0x00010a2f3f9e
>
> Config=x86_64-/Users/ec2-user/jenkins/workspace/llvm.org/lldb-cmake-matrix/clang_1706_build/bin/clang
>
> ```
>
>
>
> https://green.lab.llvm.org/job/llvm.org/view/LLDB/job/lldb-cmake-matrix/925/execution/node/135/log/
i think that's a different test though (meant to look into it but havent yet :D
)
https://github.com/llvm/llvm-project/pull/114122
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][test] Fix `LibCxxInternalsRecognizerTestCase` on clang <= 17 (PR #114122)
felipepiovezan wrote:
> > It's interesting that the 17.0 bot is still not happy:
> > ```
> >
> > File
> > "/Users/ec2-user/jenkins/workspace/llvm.org/lldb-cmake-matrix/llvm-project/lldb/packages/Python/lldbsuite/test/lldbtest.py",
> > line 321, in check_value
> >
> > test_base.assertEqual(
> >
> > AssertionError: '"Hello"' != None : Checking child with index 0:
> >
> > (std::__lldb::optional) maybe_string = Has Value=true {
> >
> > Value = 0x00010a2f3f9e
> >
> > }
> >
> > Checking SBValue: (std::__1::remove_cv_t) Value =
> > 0x00010a2f3f9e
> >
> > Config=x86_64-/Users/ec2-user/jenkins/workspace/llvm.org/lldb-cmake-matrix/clang_1706_build/bin/clang
> > ```
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >
> > https://green.lab.llvm.org/job/llvm.org/view/LLDB/job/lldb-cmake-matrix/925/execution/node/135/log/
>
> i think that's a different test though (meant to look into it but havent yet
> :D )
oh oops, sorry, didn't notice that
https://github.com/llvm/llvm-project/pull/114122
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [lldb] [llvm] [WIP][lldb][Expression] More reliable function call resolution (PR #114529)
https://github.com/Michael137 edited https://github.com/llvm/llvm-project/pull/114529 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [lldb] [llvm] [WIP][lldb][Expression] More reliable function call resolution (PR #114529)
https://github.com/Michael137 edited https://github.com/llvm/llvm-project/pull/114529 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [lldb-dap] Support column breakpoints (PR #113787)
https://github.com/vogelsgesang updated
https://github.com/llvm/llvm-project/pull/113787
>From af45bc2e24623d7225d24a4680a28630d67d636e Mon Sep 17 00:00:00 2001
From: Adrian Vogelsgesang
Date: Sat, 26 Oct 2024 14:34:31 +
Subject: [PATCH 1/5] [lldb-dap] Support column breakpoints
This commit adds support for column breakpoints to lldb-dap.
To do so, support for `breakpointLocations` was added. To find all
available breakpoint positions, we iterate over the line table.
The `setBreakpoints` request already forwarded the column correctly to
`SBTarget::BreakpointCreateByLocation`. However, `SourceBreakpointMap`
did not keep track of multiple breakpoints in the same line. To do so,
the `SourceBreakpointMap` is now indexed by line+column instead of by
line only.
See http://jonasdevlieghere.com/post/lldb-column-breakpoints/ for a
high-level introduction to column breakpoints.
---
.../test/tools/lldb-dap/dap_server.py | 30 ++-
.../API/tools/lldb-dap/breakpoint/Makefile| 2 +-
.../breakpoint/TestDAP_breakpointLocations.py | 74 +++
.../breakpoint/TestDAP_setBreakpoints.py | 172 +--
lldb/tools/lldb-dap/DAP.h | 2 +-
lldb/tools/lldb-dap/lldb-dap.cpp | 197 +-
.../llvm/DebugInfo/DWARF/DWARFDebugLine.h | 2 +-
7 files changed, 393 insertions(+), 86 deletions(-)
create mode 100644
lldb/test/API/tools/lldb-dap/breakpoint/TestDAP_breakpointLocations.py
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 63748a71f1122d..659408c709a249 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
@@ -612,6 +612,22 @@ def request_attach(
command_dict = {"command": "attach", "type": "request", "arguments":
args_dict}
return self.send_recv(command_dict)
+def request_breakpointLocations(self, file_path, line, end_line=None,
column=None, end_column=None):
+(dir, base) = os.path.split(file_path)
+source_dict = {"name": base, "path": file_path}
+args_dict = {}
+args_dict["source"] = source_dict
+if line is not None:
+ args_dict["line"] = line
+if end_line is not None:
+args_dict["endLine"] = end_line
+if column is not None:
+args_dict["column"] = column
+if end_column is not None:
+args_dict["endColumn"] = end_column
+command_dict = {"command": "breakpointLocations", "type": "request",
"arguments": args_dict}
+return self.send_recv(command_dict)
+
def request_configurationDone(self):
command_dict = {
"command": "configurationDone",
@@ -912,18 +928,14 @@ def request_setBreakpoints(self, file_path, line_array,
data=None):
breakpoint_data = data[i]
bp = {"line": line}
if breakpoint_data is not None:
-if "condition" in breakpoint_data and
breakpoint_data["condition"]:
+if breakpoint_data.get("condition"):
bp["condition"] = breakpoint_data["condition"]
-if (
-"hitCondition" in breakpoint_data
-and breakpoint_data["hitCondition"]
-):
+if breakpoint_data.get("hitCondition"):
bp["hitCondition"] = breakpoint_data["hitCondition"]
-if (
-"logMessage" in breakpoint_data
-and breakpoint_data["logMessage"]
-):
+if breakpoint_data.get("logMessage"):
bp["logMessage"] = breakpoint_data["logMessage"]
+if breakpoint_data.get("column"):
+bp["column"] = breakpoint_data["column"]
breakpoints.append(bp)
args_dict["breakpoints"] = breakpoints
diff --git a/lldb/test/API/tools/lldb-dap/breakpoint/Makefile
b/lldb/test/API/tools/lldb-dap/breakpoint/Makefile
index 7634f513e85233..06438b3e6e3139 100644
--- a/lldb/test/API/tools/lldb-dap/breakpoint/Makefile
+++ b/lldb/test/API/tools/lldb-dap/breakpoint/Makefile
@@ -16,4 +16,4 @@ main-copy.cpp: main.cpp
# The following shared library will be used to test breakpoints under dynamic
loading
libother: other-copy.c
"$(MAKE)" -f $(MAKEFILE_RULES) \
- DYLIB_ONLY=YES DYLIB_C_SOURCES=other-copy.c DYLIB_NAME=other
+ DYLIB_ONLY=YES DYLIB_C_SOURCES=other-copy.c DYLIB_NAME=other
diff --git
a/lldb/test/API/tools/lldb-dap/breakpoint/TestDAP_breakpointLocations.py
b/lldb/test/API/tools/lldb-dap/breakpoint/TestDAP_breakpointLocations.py
new file mode 100644
index 00..d84ce843e3fba8
--- /dev/null
+++ b/lldb/test/API/tools/lldb-dap/breakpoint/TestDAP_b
[Lldb-commits] [clang] [lldb] [llvm] [WIP][lldb][Expression] More reliable function call resolution (PR #114529)
Michael137 wrote: Example constructor decl would look like the following: ``` | `-CXXConstructorDecl 0x246e6f8 <> Bar 'void ()' | `-StructorMangledNamesAttr 0x246e7c0 <> Implicit 2:$__lldb_func_0x014B6560:257 1:$__lldb_func_0x014B6560:294 ``` https://github.com/llvm/llvm-project/pull/114529 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Highlight "note:" in CommandReturnObject (PR #114610)
https://github.com/JDevlieghere updated
https://github.com/llvm/llvm-project/pull/114610
>From ef1aaf94b52726c77e5569c84c09ecdd73811abd Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere
Date: Fri, 1 Nov 2024 15:02:59 -0700
Subject: [PATCH] [lldb] Highlight "note:" in CommandReturnObject
We have helpers to emit warnings and errors. Do the same thing for notes
to they stand out more.
---
.../lldb/Interpreter/CommandReturnObject.h| 10
.../Commands/CommandObjectDWIMPrint.cpp | 15 ++--
.../Interpreter/CommandReturnObject.cpp | 24 +++
3 files changed, 41 insertions(+), 8 deletions(-)
diff --git a/lldb/include/lldb/Interpreter/CommandReturnObject.h
b/lldb/include/lldb/Interpreter/CommandReturnObject.h
index a491a6c1535b11..9fef59337016df 100644
--- a/lldb/include/lldb/Interpreter/CommandReturnObject.h
+++ b/lldb/include/lldb/Interpreter/CommandReturnObject.h
@@ -110,6 +110,11 @@ class CommandReturnObject {
void AppendMessageWithFormat(const char *format, ...)
__attribute__((format(printf, 2, 3)));
+ void AppendNote(llvm::StringRef in_string);
+
+ void AppendNoteWithFormat(const char *format, ...)
+ __attribute__((format(printf, 2, 3)));
+
void AppendWarning(llvm::StringRef in_string);
void AppendWarningWithFormat(const char *format, ...)
@@ -127,6 +132,11 @@ class CommandReturnObject {
AppendMessage(llvm::formatv(format, std::forward(args)...).str());
}
+ template
+ void AppendNoteWithFormatv(const char *format, Args &&...args) {
+AppendNote(llvm::formatv(format, std::forward(args)...).str());
+ }
+
template
void AppendWarningWithFormatv(const char *format, Args &&... args) {
AppendWarning(llvm::formatv(format, std::forward(args)...).str());
diff --git a/lldb/source/Commands/CommandObjectDWIMPrint.cpp
b/lldb/source/Commands/CommandObjectDWIMPrint.cpp
index 76bed100dc7291..62c4e74d853ad1 100644
--- a/lldb/source/Commands/CommandObjectDWIMPrint.cpp
+++ b/lldb/source/Commands/CommandObjectDWIMPrint.cpp
@@ -121,10 +121,10 @@ void CommandObjectDWIMPrint::DoExecute(StringRef command,
if (note_shown)
return;
- result.GetOutputStream()
- << "note: object description requested, but type doesn't implement "
- "a custom object description. Consider using \"p\" instead of "
- "\"po\" (this note will only be shown once per debug session).\n";
+ result.AppendNote(
+ "object description requested, but type doesn't implement "
+ "a custom object description. Consider using \"p\" instead of "
+ "\"po\" (this note will only be shown once per debug session).\n");
note_shown = true;
}
};
@@ -164,8 +164,8 @@ void CommandObjectDWIMPrint::DoExecute(StringRef command,
StringRef flags;
if (args.HasArgs())
flags = args.GetArgString();
-result.AppendMessageWithFormatv("note: ran `frame variable {0}{1}`",
-flags, expr);
+result.AppendNoteWithFormatv("ran `frame variable {0}{1}`", flags,
+ expr);
}
dump_val_object(*valobj_sp);
@@ -224,8 +224,7 @@ void CommandObjectDWIMPrint::DoExecute(StringRef command,
StringRef flags;
if (args.HasArgs())
flags = args.GetArgStringWithDelimiter();
- result.AppendMessageWithFormatv("note: ran `expression {0}{1}`", flags,
- expr);
+ result.AppendNoteWithFormatv("ran `expression {0}{1}`", flags, expr);
}
if (valobj_sp->GetError().GetError() != UserExpression::kNoResult)
diff --git a/lldb/source/Interpreter/CommandReturnObject.cpp
b/lldb/source/Interpreter/CommandReturnObject.cpp
index 94f5ff608b2aea..2776efbb5ee36d 100644
--- a/lldb/source/Interpreter/CommandReturnObject.cpp
+++ b/lldb/source/Interpreter/CommandReturnObject.cpp
@@ -27,6 +27,12 @@ static llvm::raw_ostream &warning(Stream &strm) {
<< "warning: ";
}
+static llvm::raw_ostream ¬e(Stream &strm) {
+ return llvm::WithColor(strm.AsRawOstream(), llvm::HighlightColor::Note,
+ llvm::ColorMode::Enable)
+ << "note: ";
+}
+
static void DumpStringToStreamWithNewline(Stream &strm, const std::string &s) {
bool add_newline = false;
if (!s.empty()) {
@@ -74,6 +80,18 @@ void CommandReturnObject::AppendMessageWithFormat(const char
*format, ...) {
GetOutputStream() << sstrm.GetString();
}
+void CommandReturnObject::AppendNoteWithFormat(const char *format, ...) {
+ if (!format)
+return;
+ va_list args;
+ va_start(args, format);
+ StreamString sstrm;
+ sstrm.PrintfVarArg(format, args);
+ va_end(args);
+
+ note(GetOutputStream()) << sstrm.GetString();
+}
+
void CommandReturnObject::AppendWarningWithFormat(const char *format, ...) {
if (!format)
return;
@@ -92,6 +110,12 @@ void CommandReturnObject::AppendMessage(llvm::StringRef
in_string) {
GetOutputStream(
[Lldb-commits] [lldb] [lldb] Improve locking in PathMappingLists (NFC) (PR #114576)
https://github.com/JDevlieghere updated
https://github.com/llvm/llvm-project/pull/114576
>From 9ebfdc84c46a795d35b4dd157573b97a57b232ce Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere
Date: Fri, 1 Nov 2024 09:58:28 -0700
Subject: [PATCH 1/2] [lldb] Improve locking in PathMappingLists (NFC)
In D148380 [1], Alex added locking to PathMappingLists. The current
implementation runs the callback under the lock, which I don't believe
is necessary. As far as I can tell, no users of the callback are relying
on the list not having been modified until the callback is handled.
This patch implements my suggestion to unlock the mutex before the
callback. I also switched to a non-recursive mutex as I don't believe
the recursive property is needed. To make the class fully thread safe, I
did have to introduce another mutex to protect the callback members.
The motivation for this change is #114507. Specifically,
Target::SetExecutableModule calls Target::GetOrCreateModule, which
potentially performs path remapping, which has a callback to
Target::SetExecutableModule.
[1] https://reviews.llvm.org/D148380
---
lldb/include/lldb/Target/PathMappingList.h | 19 ++-
lldb/source/Target/PathMappingList.cpp | 163 -
2 files changed, 107 insertions(+), 75 deletions(-)
diff --git a/lldb/include/lldb/Target/PathMappingList.h
b/lldb/include/lldb/Target/PathMappingList.h
index 1c0ff564739c5a..a59539eb0e4bc6 100644
--- a/lldb/include/lldb/Target/PathMappingList.h
+++ b/lldb/include/lldb/Target/PathMappingList.h
@@ -25,7 +25,6 @@ class PathMappingList {
typedef void (*ChangedCallback)(const PathMappingList &path_list,
void *baton);
- // Constructors and Destructors
PathMappingList();
PathMappingList(ChangedCallback callback, void *callback_baton);
@@ -53,12 +52,12 @@ class PathMappingList {
llvm::json::Value ToJSON();
bool IsEmpty() const {
-std::lock_guard lock(m_mutex);
+std::lock_guard lock(m_pairs_mutex);
return m_pairs.empty();
}
size_t GetSize() const {
-std::lock_guard lock(m_mutex);
+std::lock_guard lock(m_pairs_mutex);
return m_pairs.size();
}
@@ -137,25 +136,33 @@ class PathMappingList {
uint32_t FindIndexForPath(llvm::StringRef path) const;
uint32_t GetModificationID() const {
-std::lock_guard lock(m_mutex);
+std::lock_guard lock(m_pairs_mutex);
return m_mod_id;
}
protected:
- mutable std::recursive_mutex m_mutex;
typedef std::pair pair;
typedef std::vector collection;
typedef collection::iterator iterator;
typedef collection::const_iterator const_iterator;
+ void AppendImpl(llvm::StringRef path, llvm::StringRef replacement);
+ void Notify(bool notify) const;
+
iterator FindIteratorForPath(ConstString path);
const_iterator FindIteratorForPath(ConstString path) const;
collection m_pairs;
+ mutable std::mutex m_pairs_mutex;
+
ChangedCallback m_callback = nullptr;
void *m_callback_baton = nullptr;
- uint32_t m_mod_id = 0; // Incremented anytime anything is added or removed.
+ mutable std::mutex m_callback_mutex;
+
+ /// Incremented anytime anything is added to or removed from m_pairs. Also
+ /// protected by m_pairs_mutex.
+ uint32_t m_mod_id = 0;
};
} // namespace lldb_private
diff --git a/lldb/source/Target/PathMappingList.cpp
b/lldb/source/Target/PathMappingList.cpp
index 9c283b0146fe07..7343522fafef20 100644
--- a/lldb/source/Target/PathMappingList.cpp
+++ b/lldb/source/Target/PathMappingList.cpp
@@ -48,7 +48,10 @@ PathMappingList::PathMappingList(const PathMappingList &rhs)
const PathMappingList &PathMappingList::operator=(const PathMappingList &rhs) {
if (this != &rhs) {
-std::scoped_lock
locks(m_mutex, rhs.m_mutex);
+std::scoped_lock pairs_locks(m_pairs_mutex,
+ rhs.m_pairs_mutex);
+std::scoped_lock callback_locks(
+m_callback_mutex, rhs.m_callback_mutex);
m_pairs = rhs.m_pairs;
m_callback = nullptr;
m_callback_baton = nullptr;
@@ -59,85 +62,105 @@ const PathMappingList &PathMappingList::operator=(const
PathMappingList &rhs) {
PathMappingList::~PathMappingList() = default;
-void PathMappingList::Append(llvm::StringRef path, llvm::StringRef replacement,
- bool notify) {
- std::lock_guard lock(m_mutex);
+void PathMappingList::AppendImpl(llvm::StringRef path,
+ llvm::StringRef replacement) {
++m_mod_id;
m_pairs.emplace_back(pair(NormalizePath(path), NormalizePath(replacement)));
+}
+
+void PathMappingList::Notify(bool notify) const {
+ std::lock_guard lock(m_callback_mutex);
if (notify && m_callback)
m_callback(*this, m_callback_baton);
}
+void PathMappingList::Append(llvm::StringRef path, llvm::StringRef replacement,
+ bool notify) {
+ {
+std::lock_guard lock(m_pairs_mutex);
+AppendImpl(path, replacement);
+ }
[Lldb-commits] [lldb] [lldb] Improve locking in PathMappingLists (NFC) (PR #114576)
https://github.com/bulbazord approved this pull request. https://github.com/llvm/llvm-project/pull/114576 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Create dependent modules in parallel (PR #114507)
@@ -1608,9 +1612,48 @@ void Target::SetExecutableModule(ModuleSP &executable_sp,
if (image_module_sp) {
added_modules.AppendIfNeeded(image_module_sp, false);
ObjectFile *objfile = image_module_sp->GetObjectFile();
- if (objfile)
-objfile->GetDependentModules(dependent_files);
+ if (objfile) {
+// Create a local copy of the dependent file list so we don't have
+// to lock for the whole duration of GetDependentModules.
+FileSpecList dependent_files_copy;
+{
+ std::lock_guard guard(dependent_files_mutex);
+ dependent_files_copy = dependent_files;
+}
+
+// Remember the size of the local copy so we can append only the
+// modules that have been added by GetDependentModules.
+const size_t previous_dependent_files =
+dependent_files_copy.GetSize();
+
+objfile->GetDependentModules(dependent_files_copy);
+
+{
+ std::lock_guard guard(dependent_files_mutex);
+ for (size_t i = previous_dependent_files;
+ i < dependent_files_copy.GetSize(); ++i)
+dependent_files.AppendIfUnique(
+dependent_files_copy.GetFileSpecAtIndex(i));
+}
+ }
+}
+ };
+
+ executable_objfile->GetDependentModules(dependent_files);
+
+ llvm::ThreadPoolTaskGroup task_group(Debugger::GetThreadPool());
+ for (uint32_t i = 0; i < dependent_files.GetSize(); i++) {
+// Process all currently known dependencies in parallel in the
innermost
+// loop. This may create newly discovered dependencies to be appended
to
+// dependent_files. We'll deal with these files during the next
+// iteration of the outermost loop.
+{
+ std::lock_guard guard(dependent_files_mutex);
+ for (; i < dependent_files.GetSize(); i++)
+task_group.async(GetDependentModules,
adrian-prantl wrote:
async inside a lock_guard looks dangerous. I assume you're sure this won't
recurse into this function? :-)
https://github.com/llvm/llvm-project/pull/114507
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Improve locking in PathMappingLists (NFC) (PR #114576)
@@ -48,7 +48,10 @@ PathMappingList::PathMappingList(const PathMappingList &rhs)
const PathMappingList &PathMappingList::operator=(const PathMappingList &rhs) {
if (this != &rhs) {
-std::scoped_lock
locks(m_mutex, rhs.m_mutex);
+std::scoped_lock pairs_locks(m_pairs_mutex,
+ rhs.m_pairs_mutex);
+std::scoped_lock callback_locks(
+m_callback_mutex, rhs.m_callback_mutex);
JDevlieghere wrote:
Right, it took me a while to convince myself that wouldn't just cause the same
problem. The only way to avoid a deadlock is always acquire the lock in the
same order and because we always acquire the `m_callback_mutex` before doing
callback, and that's the only one that could be locked while acquiring the
`m_pairs_mutex` (i.e. there's no code path besides that and `operator= ` that
acquires both locks), that both mutex are always acquired in order.
https://github.com/llvm/llvm-project/pull/114576
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] c820994 - [lldb] Improve locking in PathMappingLists (NFC) (#114576)
Author: Jonas Devlieghere
Date: 2024-11-01T16:38:14-07:00
New Revision: c8209943faeead43c6932c5ddcc69c9e9d1e4262
URL:
https://github.com/llvm/llvm-project/commit/c8209943faeead43c6932c5ddcc69c9e9d1e4262
DIFF:
https://github.com/llvm/llvm-project/commit/c8209943faeead43c6932c5ddcc69c9e9d1e4262.diff
LOG: [lldb] Improve locking in PathMappingLists (NFC) (#114576)
In [D148380](https://reviews.llvm.org/D148380), Alex added locking to
PathMappingLists. The current implementation runs the callback under the
lock, which I don't believe is necessary. As far as I can tell, no users
of the callback are relying on the list not having been modified until
the callback is handled.
This patch implements my suggestion to unlock the mutex before the
callback. I also switched to a non-recursive mutex as I don't believe
the recursive property is needed. To make the class fully thread safe, I
did have to introduce another mutex to protect the callback members.
The motivation for this change is #114507. Specifically,
Target::SetExecutableModule calls Target::GetOrCreateModule, which
potentially performs path remapping, which in turns has a callback to
Target::SetExecutableModule.
Added:
Modified:
lldb/include/lldb/Target/PathMappingList.h
lldb/source/Target/PathMappingList.cpp
Removed:
diff --git a/lldb/include/lldb/Target/PathMappingList.h
b/lldb/include/lldb/Target/PathMappingList.h
index 1c0ff564739c5a..a59539eb0e4bc6 100644
--- a/lldb/include/lldb/Target/PathMappingList.h
+++ b/lldb/include/lldb/Target/PathMappingList.h
@@ -25,7 +25,6 @@ class PathMappingList {
typedef void (*ChangedCallback)(const PathMappingList &path_list,
void *baton);
- // Constructors and Destructors
PathMappingList();
PathMappingList(ChangedCallback callback, void *callback_baton);
@@ -53,12 +52,12 @@ class PathMappingList {
llvm::json::Value ToJSON();
bool IsEmpty() const {
-std::lock_guard lock(m_mutex);
+std::lock_guard lock(m_pairs_mutex);
return m_pairs.empty();
}
size_t GetSize() const {
-std::lock_guard lock(m_mutex);
+std::lock_guard lock(m_pairs_mutex);
return m_pairs.size();
}
@@ -137,25 +136,33 @@ class PathMappingList {
uint32_t FindIndexForPath(llvm::StringRef path) const;
uint32_t GetModificationID() const {
-std::lock_guard lock(m_mutex);
+std::lock_guard lock(m_pairs_mutex);
return m_mod_id;
}
protected:
- mutable std::recursive_mutex m_mutex;
typedef std::pair pair;
typedef std::vector collection;
typedef collection::iterator iterator;
typedef collection::const_iterator const_iterator;
+ void AppendImpl(llvm::StringRef path, llvm::StringRef replacement);
+ void Notify(bool notify) const;
+
iterator FindIteratorForPath(ConstString path);
const_iterator FindIteratorForPath(ConstString path) const;
collection m_pairs;
+ mutable std::mutex m_pairs_mutex;
+
ChangedCallback m_callback = nullptr;
void *m_callback_baton = nullptr;
- uint32_t m_mod_id = 0; // Incremented anytime anything is added or removed.
+ mutable std::mutex m_callback_mutex;
+
+ /// Incremented anytime anything is added to or removed from m_pairs. Also
+ /// protected by m_pairs_mutex.
+ uint32_t m_mod_id = 0;
};
} // namespace lldb_private
diff --git a/lldb/source/Target/PathMappingList.cpp
b/lldb/source/Target/PathMappingList.cpp
index 9c283b0146fe07..81d10bdd53f920 100644
--- a/lldb/source/Target/PathMappingList.cpp
+++ b/lldb/source/Target/PathMappingList.cpp
@@ -48,7 +48,10 @@ PathMappingList::PathMappingList(const PathMappingList &rhs)
const PathMappingList &PathMappingList::operator=(const PathMappingList &rhs) {
if (this != &rhs) {
-std::scoped_lock
locks(m_mutex, rhs.m_mutex);
+std::scoped_lock callback_locks(
+m_callback_mutex, rhs.m_callback_mutex);
+std::scoped_lock pairs_locks(m_pairs_mutex,
+ rhs.m_pairs_mutex);
m_pairs = rhs.m_pairs;
m_callback = nullptr;
m_callback_baton = nullptr;
@@ -59,85 +62,105 @@ const PathMappingList &PathMappingList::operator=(const
PathMappingList &rhs) {
PathMappingList::~PathMappingList() = default;
-void PathMappingList::Append(llvm::StringRef path, llvm::StringRef replacement,
- bool notify) {
- std::lock_guard lock(m_mutex);
+void PathMappingList::AppendImpl(llvm::StringRef path,
+ llvm::StringRef replacement) {
++m_mod_id;
m_pairs.emplace_back(pair(NormalizePath(path), NormalizePath(replacement)));
+}
+
+void PathMappingList::Notify(bool notify) const {
+ std::lock_guard lock(m_callback_mutex);
if (notify && m_callback)
m_callback(*this, m_callback_baton);
}
+void PathMappingList::Append(llvm::StringRef path, llvm::StringRef replacement,
+
[Lldb-commits] [lldb] [lldb] Improve locking in PathMappingLists (NFC) (PR #114576)
https://github.com/JDevlieghere closed https://github.com/llvm/llvm-project/pull/114576 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Avoid unnecessary regex check in dwim-print (PR #114608)
https://github.com/kastiglione edited https://github.com/llvm/llvm-project/pull/114608 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Create dependent modules in parallel (PR #114507)
@@ -1608,9 +1612,28 @@ void Target::SetExecutableModule(ModuleSP &executable_sp,
if (image_module_sp) {
added_modules.AppendIfNeeded(image_module_sp, false);
ObjectFile *objfile = image_module_sp->GetObjectFile();
- if (objfile)
+ if (objfile) {
+std::lock_guard guard(dependent_files_mutex);
objfile->GetDependentModules(dependent_files);
DmT021 wrote:
Just in case, did you check for a regression in performance after the latest
patch?
https://github.com/llvm/llvm-project/pull/114507
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Improve command status when dwim-print has no result (PR #114478)
https://github.com/kastiglione closed https://github.com/llvm/llvm-project/pull/114478 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Highlight "note:" in CommandReturnObject (PR #114610)
@@ -27,6 +27,12 @@ static llvm::raw_ostream &warning(Stream &strm) {
<< "warning: ";
}
+static llvm::raw_ostream ¬e(Stream &strm) {
augusto2112 wrote:
Actually `error` and `warning` are not capitalized, I guess we can either keep
all three lower case or capitalize all of them, up to you
https://github.com/llvm/llvm-project/pull/114610
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Highlight "note:" in CommandReturnObject (PR #114610)
@@ -110,6 +110,11 @@ class CommandReturnObject {
void AppendMessageWithFormat(const char *format, ...)
__attribute__((format(printf, 2, 3)));
+ void AppendNote(llvm::StringRef in_string);
+
+ void AppendNoteWithFormat(const char *format, ...)
+ __attribute__((format(printf, 2, 3)));
augusto2112 wrote:
Nice, didn't know about `__attribute__((format(printf`
https://github.com/llvm/llvm-project/pull/114610
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Highlight "note:" in CommandReturnObject (PR #114610)
@@ -27,6 +27,12 @@ static llvm::raw_ostream &warning(Stream &strm) {
<< "warning: ";
}
+static llvm::raw_ostream ¬e(Stream &strm) {
augusto2112 wrote:
Since `note` is a function should it be capitalized to follow the LLDB style?
https://github.com/llvm/llvm-project/pull/114610
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Highlight "note:" in CommandReturnObject (PR #114610)
https://github.com/augusto2112 approved this pull request. https://github.com/llvm/llvm-project/pull/114610 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Highlight "note:" in CommandReturnObject (PR #114610)
https://github.com/kastiglione approved this pull request. https://github.com/llvm/llvm-project/pull/114610 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Highlight "note:" in CommandReturnObject (PR #114610)
https://github.com/adrian-prantl approved this pull request. I was going to say this is not the right way to do this, but I suppose there isn't really a better mechanism available, since this isn't an error. Otherwise you'd be better off joining a diagnostic error with the note so the UI layer can choose how to render the note. https://github.com/llvm/llvm-project/pull/114610 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Avoid unnecessary regex check in dwim-print (PR #114608)
@@ -101,6 +101,10 @@ void CommandObjectDWIMPrint::DoExecute(StringRef command,
// Add a hint if object description was requested, but no description
// function was implemented.
auto maybe_add_hint = [&](llvm::StringRef output) {
+static bool note_shown = false;
+if (note_shown)
+ return;
adrian-prantl wrote:
If we tie it to a module, we could use the mechanism introduced in
https://github.com/llvm/llvm-project/pull/112801/files to automatically show it
once per session.
> I'd expect this to show up once per debugger
You mean once per type _and_ debugger, right?
https://github.com/llvm/llvm-project/pull/114608
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Avoid unnecessary regex check in dwim-print (PR #114608)
@@ -101,6 +101,10 @@ void CommandObjectDWIMPrint::DoExecute(StringRef command,
// Add a hint if object description was requested, but no description
// function was implemented.
auto maybe_add_hint = [&](llvm::StringRef output) {
+static bool note_shown = false;
+if (note_shown)
+ return;
kastiglione wrote:
I think @augusto2112 and I discussed and decided not to do once per type. We
figured that might make the hint too noisy.
https://github.com/llvm/llvm-project/pull/114608
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB] Finish implementing support for DW_FORM_data16 (PR #113508)
walter-erquinigo wrote: I was able to make it work using @labath 's suggestion. Fortunately it just works. Please give it another look :) https://github.com/llvm/llvm-project/pull/113508 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB] Finish implementing support for DW_FORM_data16 (PR #113508)
https://github.com/walter-erquinigo updated
https://github.com/llvm/llvm-project/pull/113508
>From dce501d494a3d559d490f8d58f6918b63ed9d175 Mon Sep 17 00:00:00 2001
From: walter erquinigo
Date: Wed, 23 Oct 2024 19:49:24 -0400
Subject: [PATCH 1/2] [LLDB] Finish implementing support for DW_FORM_data16
This FORM already has support within LLDB to be parsed as a 16-byte
BLOCK, and all that is left to properly support it in the DWARFParser is
to add it to some enums.
With this, I can debug programs that use libstdc++.so.6.0.33 built with
GCC.
---
.../Plugins/SymbolFile/DWARF/DWARFFormValue.cpp| 7 +++
.../Shell/SymbolFile/DWARF/x86/DW_AT_const_value.s | 14 +-
2 files changed, 20 insertions(+), 1 deletion(-)
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFFormValue.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFFormValue.cpp
index f58c6262349c6f..404e50d57a9251 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFFormValue.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFFormValue.cpp
@@ -306,6 +306,11 @@ bool DWARFFormValue::SkipValue(dw_form_t form,
*offset_ptr += 8;
return true;
+// 16 byte values
+case DW_FORM_data16:
+ *offset_ptr += 16;
+ return true;
+
// signed or unsigned LEB 128 values
case DW_FORM_addrx:
case DW_FORM_loclistx:
@@ -578,6 +583,7 @@ bool DWARFFormValue::IsBlockForm(const dw_form_t form) {
case DW_FORM_block1:
case DW_FORM_block2:
case DW_FORM_block4:
+ case DW_FORM_data16:
return true;
default:
return false;
@@ -611,6 +617,7 @@ bool DWARFFormValue::FormIsSupported(dw_form_t form) {
case DW_FORM_data2:
case DW_FORM_data4:
case DW_FORM_data8:
+case DW_FORM_data16:
case DW_FORM_string:
case DW_FORM_block:
case DW_FORM_block1:
diff --git a/lldb/test/Shell/SymbolFile/DWARF/x86/DW_AT_const_value.s
b/lldb/test/Shell/SymbolFile/DWARF/x86/DW_AT_const_value.s
index 720684c19beebc..731a558f3e572d 100644
--- a/lldb/test/Shell/SymbolFile/DWARF/x86/DW_AT_const_value.s
+++ b/lldb/test/Shell/SymbolFile/DWARF/x86/DW_AT_const_value.s
@@ -3,7 +3,7 @@
# RUN: llvm-mc -filetype=obj -triple x86_64-pc-linux %s -o %t
# RUN: %lldb %t \
-# RUN: -o "target variable udata data1 data2 data4 data8 string strp ref4
udata_ptr" \
+# RUN: -o "target variable udata data1 data2 data4 data8 data16 string strp
ref4 udata_ptr" \
# RUN: -o exit | FileCheck %s
# CHECK-LABEL: target variable
@@ -14,6 +14,7 @@
# CHECK: (unsigned long) data2 = 4742
# CHECK: (unsigned long) data4 = 47424742
# CHECK: (unsigned long) data8 = 4742474247424742
+# CHECK: (unsigned __int128) data16 = 129440743495415807670381713415221633377
## Variables specified using string forms. This behavior purely speculative --
I
## don't know of any compiler that would represent character strings this way.
# CHECK: (char[7]) string = "string"
@@ -88,6 +89,7 @@
var 15, 0x8 # DW_FORM_string
var 16, 0xe # DW_FORM_strp
var 17, 0x13# DW_FORM_ref4
+var 18, 0x1e# DW_FORM_data16
.byte 0 # EOM(3)
.section.debug_info,"",@progbits
.Lcu_begin0:
@@ -119,6 +121,11 @@
.Lulong_ptr:
.byte 2 # Abbrev DW_TAG_pointer_type
.long .Lulong-.Lcu_begin0 # DW_AT_type
+.Lu128:
+.byte 6 # Abbrev DW_TAG_base_type
+.asciz "Lu128" # DW_AT_name
+.byte 16 # DW_AT_byte_size
+.byte 7 # DW_AT_encoding
.byte 10 # Abbrev DW_TAG_variable
.asciz "udata" # DW_AT_name
@@ -165,6 +172,11 @@
.long .Lulong_ptr-.Lcu_begin0 # DW_AT_type
.uleb128 0xdeadbeefbaadf00d # DW_AT_const_value
+.byte 18 # Abbrev DW_TAG_variable
+.asciz "data16"# DW_AT_name
+.long .Lu128-.Lcu_begin0 # DW_AT_type
+.asciz
""
# DW_AT_const_value
+
.byte 0 # End Of Children Mark
.Ldebug_info_end0:
>From 403bbcaddc40d011cbd262381c3bc4e6768c234a Mon Sep 17 00:00:00 2001
From: walter erquinigo
Date: Fri, 1 Nov 2024 13:14:59 -0400
Subject: [PATCH 2/2] fix fix
---
lldb/test/Shell/SymbolFile/DWARF/x86/DW_AT_const_value.s | 9 ++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/lldb/test/Shell/SymbolFile/DWARF/x86/DW_AT_const_value.s
b/lldb/test/Shell/SymbolFile/DWARF/x86/DW_AT_const_value.s
index 731a558f3e572d..306afd32d92235 100644
--- a/lldb/test/Shell/SymbolFile/DWARF/x86/DW_AT_const_value.s
+++ b/lldb/test/Shell/SymbolFile/DWARF/x86/DW_AT_const_value.s
@@ -3,7 +3,8 @@
# RUN: llvm-mc -filety
[Lldb-commits] [lldb] [lldb] Avoid unnecessary regex check in dwim-print (PR #114608)
@@ -101,6 +101,10 @@ void CommandObjectDWIMPrint::DoExecute(StringRef command,
// Add a hint if object description was requested, but no description
// function was implemented.
auto maybe_add_hint = [&](llvm::StringRef output) {
+static bool note_shown = false;
+if (note_shown)
+ return;
kastiglione wrote:
I am indifferent about per-debugger. I don't consider this note as being
critical, as long as it's shown once in a while, I'm good.
https://github.com/llvm/llvm-project/pull/114608
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Avoid unnecessary regex check in dwim-print (PR #114608)
https://github.com/augusto2112 edited https://github.com/llvm/llvm-project/pull/114608 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Avoid unnecessary regex check in dwim-print (PR #114608)
@@ -101,6 +101,10 @@ void CommandObjectDWIMPrint::DoExecute(StringRef command,
// Add a hint if object description was requested, but no description
// function was implemented.
auto maybe_add_hint = [&](llvm::StringRef output) {
+static bool note_shown = false;
+if (note_shown)
+ return;
adrian-prantl wrote:
If we already landed on only once per session I'm fine with that too.
You can also manually allocate a std::once and pass it to the Debugger-wide
function. Or just do the call_once here.
https://github.com/llvm/llvm-project/pull/114608
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Avoid unnecessary regex check in dwim-print (PR #114608)
@@ -101,6 +101,10 @@ void CommandObjectDWIMPrint::DoExecute(StringRef command,
// Add a hint if object description was requested, but no description
// function was implemented.
auto maybe_add_hint = [&](llvm::StringRef output) {
+static bool note_shown = false;
+if (note_shown)
+ return;
augusto2112 wrote:
Yes the idea was for it not to show up too frequently, I'd be ok if it's once
per debugger too.
https://github.com/llvm/llvm-project/pull/114608
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Improve command status when dwim-print has no result (PR #114478)
https://github.com/adrian-prantl commented: @kastiglione Is there a way to turn this into a test? Apparently this works: ``` (lldb) p ; (lldb) ``` https://github.com/llvm/llvm-project/pull/114478 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Avoid unnecessary regex check in dwim-print (PR #114608)
https://github.com/kastiglione edited https://github.com/llvm/llvm-project/pull/114608 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [LLDB] Add a target.launch-working-dir setting (PR #113521)
https://github.com/walter-erquinigo updated
https://github.com/llvm/llvm-project/pull/113521
>From 819380f93381eff8200c7eb9c55372f9a840c379 Mon Sep 17 00:00:00 2001
From: walter erquinigo
Date: Thu, 24 Oct 2024 00:17:48 -0400
Subject: [PATCH] [LLDB] Add a target.launch-working-dir setting
Internally we use bazel in a way in which it can drop you in a LLDB session
with the target launched in a particular cwd, which is needed for things to
work. We've been making this automation work via `process launch -w`. However,
if later the user wants to restart the process with `r`, then they end up using
a different cwd for relaunching the process.
As a way to fix this, I'm adding a target-level setting that allows setting a
default cwd used for launching the process without needing the user to specify
it manually.
---
lldb/include/lldb/Target/Target.h | 3 +
lldb/source/Commands/CommandObjectProcess.cpp | 7 +++
lldb/source/Commands/Options.td | 5 +-
lldb/source/Target/Target.cpp | 5 ++
lldb/source/Target/TargetProperties.td| 6 ++
.../process/launch/TestProcessLaunch.py | 57 +++
llvm/docs/ReleaseNotes.md | 2 +
7 files changed, 84 insertions(+), 1 deletion(-)
diff --git a/lldb/include/lldb/Target/Target.h
b/lldb/include/lldb/Target/Target.h
index e4848f19e64d62..cab21c29a7486f 100644
--- a/lldb/include/lldb/Target/Target.h
+++ b/lldb/include/lldb/Target/Target.h
@@ -37,6 +37,7 @@
#include "lldb/Utility/RealpathPrefixes.h"
#include "lldb/Utility/Timeout.h"
#include "lldb/lldb-public.h"
+#include "llvm/ADT/StringRef.h"
namespace lldb_private {
@@ -114,6 +115,8 @@ class TargetProperties : public Properties {
void SetDisableSTDIO(bool b);
+ llvm::StringRef GetLaunchWorkingDirectory() const;
+
const char *GetDisassemblyFlavor() const;
InlineStrategy GetInlineStrategy() const;
diff --git a/lldb/source/Commands/CommandObjectProcess.cpp
b/lldb/source/Commands/CommandObjectProcess.cpp
index e7c7d07ad47722..7444e46aa729e7 100644
--- a/lldb/source/Commands/CommandObjectProcess.cpp
+++ b/lldb/source/Commands/CommandObjectProcess.cpp
@@ -201,6 +201,13 @@ class CommandObjectProcessLaunch : public
CommandObjectProcessLaunchOrAttach {
if (target->GetDisableSTDIO())
m_options.launch_info.GetFlags().Set(eLaunchFlagDisableSTDIO);
+if (!m_options.launch_info.GetWorkingDirectory()) {
+ if (llvm::StringRef wd = target->GetLaunchWorkingDirectory();
+ !wd.empty()) {
+m_options.launch_info.SetWorkingDirectory(FileSpec(wd));
+ }
+}
+
// Merge the launch info environment with the target environment.
Environment target_env = target->GetEnvironment();
m_options.launch_info.GetEnvironment().insert(target_env.begin(),
diff --git a/lldb/source/Commands/Options.td b/lldb/source/Commands/Options.td
index 4276d9e7f9c8b0..94d1219b82b87e 100644
--- a/lldb/source/Commands/Options.td
+++ b/lldb/source/Commands/Options.td
@@ -691,7 +691,10 @@ let Command = "process launch" in {
def process_launch_plugin : Option<"plugin", "P">, Arg<"Plugin">,
Desc<"Name of the process plugin you want to use.">;
def process_launch_working_dir : Option<"working-dir", "w">,
Arg<"DirectoryName">,
-Desc<"Set the current working directory to when running the
inferior.">;
+Desc<"Set the current working directory to when running the
inferior. This setting "
+ "applies only to the current `process launch` invocation. You can use
the "
+ "`target.launch-working-dir` setting to set a working-dir that is
persistent "
+ "across launches.">;
def process_launch_arch : Option<"arch", "a">, Arg<"Architecture">,
Desc<"Set the architecture for the process to launch when ambiguous.">;
def process_launch_environment : Option<"environment", "E">,
diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp
index 199efae8a728cc..b6d4b28b03a5b3 100644
--- a/lldb/source/Target/Target.cpp
+++ b/lldb/source/Target/Target.cpp
@@ -4428,6 +4428,11 @@ void TargetProperties::SetDisableSTDIO(bool b) {
const uint32_t idx = ePropertyDisableSTDIO;
SetPropertyAtIndex(idx, b);
}
+llvm::StringRef TargetProperties::GetLaunchWorkingDirectory() const {
+ const uint32_t idx = ePropertyLaunchWorkingDir;
+ return GetPropertyAtIndexAs(
+ idx, g_target_properties[idx].default_cstr_value);
+}
const char *TargetProperties::GetDisassemblyFlavor() const {
const uint32_t idx = ePropertyDisassemblyFlavor;
diff --git a/lldb/source/Target/TargetProperties.td
b/lldb/source/Target/TargetProperties.td
index fb61478fb752dc..3aa7cc41c4d5ca 100644
--- a/lldb/source/Target/TargetProperties.td
+++ b/lldb/source/Target/TargetProperties.td
@@ -201,6 +201,12 @@ let Definition = "target" in {
def DebugUtilityExpression: Property<"debug-utility-expression", "Boolean">,
DefaultFalse,
Desc<"Enable debugging of LLDB-internal utility
[Lldb-commits] [lldb] [lldb] Highlight "note:" in CommandReturnObject (PR #114610)
JDevlieghere wrote: Motivated by #114608 https://github.com/llvm/llvm-project/pull/114610 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Highlight "note:" in CommandReturnObject (PR #114610)
https://github.com/JDevlieghere created
https://github.com/llvm/llvm-project/pull/114610
We have helpers to emit warnings and errors. Do the same thing for notes to
they stand out more.
>From 155396aa89fd418268cfcf8d054849c67bf1207e Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere
Date: Fri, 1 Nov 2024 15:02:59 -0700
Subject: [PATCH] [lldb] Highlight "note:" in CommandReturnObject
We have helpers to emit warnings and errors. Do the same thing for notes
to they stand out more.
---
.../lldb/Interpreter/CommandReturnObject.h| 5
.../Commands/CommandObjectDWIMPrint.cpp | 11 -
.../Interpreter/CommandReturnObject.cpp | 24 +++
3 files changed, 34 insertions(+), 6 deletions(-)
diff --git a/lldb/include/lldb/Interpreter/CommandReturnObject.h
b/lldb/include/lldb/Interpreter/CommandReturnObject.h
index a491a6c1535b11..14f2c4a62f7cbd 100644
--- a/lldb/include/lldb/Interpreter/CommandReturnObject.h
+++ b/lldb/include/lldb/Interpreter/CommandReturnObject.h
@@ -110,6 +110,11 @@ class CommandReturnObject {
void AppendMessageWithFormat(const char *format, ...)
__attribute__((format(printf, 2, 3)));
+ void AppendNote(llvm::StringRef in_string);
+
+ void AppendNoteWithFormat(const char *format, ...)
+ __attribute__((format(printf, 2, 3)));
+
void AppendWarning(llvm::StringRef in_string);
void AppendWarningWithFormat(const char *format, ...)
diff --git a/lldb/source/Commands/CommandObjectDWIMPrint.cpp
b/lldb/source/Commands/CommandObjectDWIMPrint.cpp
index 76bed100dc7291..dc0f98ed06f7fe 100644
--- a/lldb/source/Commands/CommandObjectDWIMPrint.cpp
+++ b/lldb/source/Commands/CommandObjectDWIMPrint.cpp
@@ -121,8 +121,8 @@ void CommandObjectDWIMPrint::DoExecute(StringRef command,
if (note_shown)
return;
- result.GetOutputStream()
- << "note: object description requested, but type doesn't implement "
+ result.AppendNote(
+ << "object description requested, but type doesn't implement "
"a custom object description. Consider using \"p\" instead of "
"\"po\" (this note will only be shown once per debug session).\n";
note_shown = true;
@@ -164,8 +164,8 @@ void CommandObjectDWIMPrint::DoExecute(StringRef command,
StringRef flags;
if (args.HasArgs())
flags = args.GetArgString();
-result.AppendMessageWithFormatv("note: ran `frame variable {0}{1}`",
-flags, expr);
+result.AppendNoteWithFormatv("ran `frame variable {0}{1}`", flags,
+ expr);
}
dump_val_object(*valobj_sp);
@@ -224,8 +224,7 @@ void CommandObjectDWIMPrint::DoExecute(StringRef command,
StringRef flags;
if (args.HasArgs())
flags = args.GetArgStringWithDelimiter();
- result.AppendMessageWithFormatv("note: ran `expression {0}{1}`", flags,
- expr);
+ result.AppendNoteWithFormatv("ran `expression {0}{1}`", flags, expr);
}
if (valobj_sp->GetError().GetError() != UserExpression::kNoResult)
diff --git a/lldb/source/Interpreter/CommandReturnObject.cpp
b/lldb/source/Interpreter/CommandReturnObject.cpp
index 94f5ff608b2aea..2776efbb5ee36d 100644
--- a/lldb/source/Interpreter/CommandReturnObject.cpp
+++ b/lldb/source/Interpreter/CommandReturnObject.cpp
@@ -27,6 +27,12 @@ static llvm::raw_ostream &warning(Stream &strm) {
<< "warning: ";
}
+static llvm::raw_ostream ¬e(Stream &strm) {
+ return llvm::WithColor(strm.AsRawOstream(), llvm::HighlightColor::Note,
+ llvm::ColorMode::Enable)
+ << "note: ";
+}
+
static void DumpStringToStreamWithNewline(Stream &strm, const std::string &s) {
bool add_newline = false;
if (!s.empty()) {
@@ -74,6 +80,18 @@ void CommandReturnObject::AppendMessageWithFormat(const char
*format, ...) {
GetOutputStream() << sstrm.GetString();
}
+void CommandReturnObject::AppendNoteWithFormat(const char *format, ...) {
+ if (!format)
+return;
+ va_list args;
+ va_start(args, format);
+ StreamString sstrm;
+ sstrm.PrintfVarArg(format, args);
+ va_end(args);
+
+ note(GetOutputStream()) << sstrm.GetString();
+}
+
void CommandReturnObject::AppendWarningWithFormat(const char *format, ...) {
if (!format)
return;
@@ -92,6 +110,12 @@ void CommandReturnObject::AppendMessage(llvm::StringRef
in_string) {
GetOutputStream() << in_string.rtrim() << '\n';
}
+void CommandReturnObject::AppendNote(llvm::StringRef in_string) {
+ if (in_string.empty())
+return;
+ note(GetOutputStream()) << in_string.rtrim() << '\n';
+}
+
void CommandReturnObject::AppendWarning(llvm::StringRef in_string) {
if (in_string.empty())
return;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Highlight "note:" in CommandReturnObject (PR #114610)
llvmbot wrote:
@llvm/pr-subscribers-lldb
Author: Jonas Devlieghere (JDevlieghere)
Changes
We have helpers to emit warnings and errors. Do the same thing for notes to
they stand out more.
---
Full diff: https://github.com/llvm/llvm-project/pull/114610.diff
3 Files Affected:
- (modified) lldb/include/lldb/Interpreter/CommandReturnObject.h (+5)
- (modified) lldb/source/Commands/CommandObjectDWIMPrint.cpp (+5-6)
- (modified) lldb/source/Interpreter/CommandReturnObject.cpp (+24)
``diff
diff --git a/lldb/include/lldb/Interpreter/CommandReturnObject.h
b/lldb/include/lldb/Interpreter/CommandReturnObject.h
index a491a6c1535b11..14f2c4a62f7cbd 100644
--- a/lldb/include/lldb/Interpreter/CommandReturnObject.h
+++ b/lldb/include/lldb/Interpreter/CommandReturnObject.h
@@ -110,6 +110,11 @@ class CommandReturnObject {
void AppendMessageWithFormat(const char *format, ...)
__attribute__((format(printf, 2, 3)));
+ void AppendNote(llvm::StringRef in_string);
+
+ void AppendNoteWithFormat(const char *format, ...)
+ __attribute__((format(printf, 2, 3)));
+
void AppendWarning(llvm::StringRef in_string);
void AppendWarningWithFormat(const char *format, ...)
diff --git a/lldb/source/Commands/CommandObjectDWIMPrint.cpp
b/lldb/source/Commands/CommandObjectDWIMPrint.cpp
index 76bed100dc7291..dc0f98ed06f7fe 100644
--- a/lldb/source/Commands/CommandObjectDWIMPrint.cpp
+++ b/lldb/source/Commands/CommandObjectDWIMPrint.cpp
@@ -121,8 +121,8 @@ void CommandObjectDWIMPrint::DoExecute(StringRef command,
if (note_shown)
return;
- result.GetOutputStream()
- << "note: object description requested, but type doesn't implement "
+ result.AppendNote(
+ << "object description requested, but type doesn't implement "
"a custom object description. Consider using \"p\" instead of "
"\"po\" (this note will only be shown once per debug session).\n";
note_shown = true;
@@ -164,8 +164,8 @@ void CommandObjectDWIMPrint::DoExecute(StringRef command,
StringRef flags;
if (args.HasArgs())
flags = args.GetArgString();
-result.AppendMessageWithFormatv("note: ran `frame variable {0}{1}`",
-flags, expr);
+result.AppendNoteWithFormatv("ran `frame variable {0}{1}`", flags,
+ expr);
}
dump_val_object(*valobj_sp);
@@ -224,8 +224,7 @@ void CommandObjectDWIMPrint::DoExecute(StringRef command,
StringRef flags;
if (args.HasArgs())
flags = args.GetArgStringWithDelimiter();
- result.AppendMessageWithFormatv("note: ran `expression {0}{1}`", flags,
- expr);
+ result.AppendNoteWithFormatv("ran `expression {0}{1}`", flags, expr);
}
if (valobj_sp->GetError().GetError() != UserExpression::kNoResult)
diff --git a/lldb/source/Interpreter/CommandReturnObject.cpp
b/lldb/source/Interpreter/CommandReturnObject.cpp
index 94f5ff608b2aea..2776efbb5ee36d 100644
--- a/lldb/source/Interpreter/CommandReturnObject.cpp
+++ b/lldb/source/Interpreter/CommandReturnObject.cpp
@@ -27,6 +27,12 @@ static llvm::raw_ostream &warning(Stream &strm) {
<< "warning: ";
}
+static llvm::raw_ostream ¬e(Stream &strm) {
+ return llvm::WithColor(strm.AsRawOstream(), llvm::HighlightColor::Note,
+ llvm::ColorMode::Enable)
+ << "note: ";
+}
+
static void DumpStringToStreamWithNewline(Stream &strm, const std::string &s) {
bool add_newline = false;
if (!s.empty()) {
@@ -74,6 +80,18 @@ void CommandReturnObject::AppendMessageWithFormat(const char
*format, ...) {
GetOutputStream() << sstrm.GetString();
}
+void CommandReturnObject::AppendNoteWithFormat(const char *format, ...) {
+ if (!format)
+return;
+ va_list args;
+ va_start(args, format);
+ StreamString sstrm;
+ sstrm.PrintfVarArg(format, args);
+ va_end(args);
+
+ note(GetOutputStream()) << sstrm.GetString();
+}
+
void CommandReturnObject::AppendWarningWithFormat(const char *format, ...) {
if (!format)
return;
@@ -92,6 +110,12 @@ void CommandReturnObject::AppendMessage(llvm::StringRef
in_string) {
GetOutputStream() << in_string.rtrim() << '\n';
}
+void CommandReturnObject::AppendNote(llvm::StringRef in_string) {
+ if (in_string.empty())
+return;
+ note(GetOutputStream()) << in_string.rtrim() << '\n';
+}
+
void CommandReturnObject::AppendWarning(llvm::StringRef in_string) {
if (in_string.empty())
return;
``
https://github.com/llvm/llvm-project/pull/114610
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Avoid unnecessary regex check in dwim-print (PR #114608)
@@ -101,6 +101,10 @@ void CommandObjectDWIMPrint::DoExecute(StringRef command,
// Add a hint if object description was requested, but no description
// function was implemented.
auto maybe_add_hint = [&](llvm::StringRef output) {
+static bool note_shown = false;
+if (note_shown)
+ return;
JDevlieghere wrote:
I realize this code was already there, but this isn't great. Can we store this
at the debugger level, for example? I'd expect this to show up once per
debugger, and again after having called SBDebugger::Terminate/Initialize.
https://github.com/llvm/llvm-project/pull/114608
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Improve locking in PathMappingLists (NFC) (PR #114576)
@@ -48,7 +48,10 @@ PathMappingList::PathMappingList(const PathMappingList &rhs)
const PathMappingList &PathMappingList::operator=(const PathMappingList &rhs) {
if (this != &rhs) {
-std::scoped_lock
locks(m_mutex, rhs.m_mutex);
+std::scoped_lock pairs_locks(m_pairs_mutex,
+ rhs.m_pairs_mutex);
+std::scoped_lock callback_locks(
+m_callback_mutex, rhs.m_callback_mutex);
bulbazord wrote:
To future-proof this, we should probably swap the ordering of lock acquisition
here. Imagine this scenario:
1. Thread 1 mutates the list. `m_pairs_mutex` is acquired, the work is done,
and then it releases the lock.
2. Thread 1 begins the notification process so it grabs `m_callback_mutex` and
executes the callback. Simultaneously, the same PathMappingList is used for
`operator=` on Thread 2. It will grab the `m_pairs_mutex` and stall on
acquiring `m_callback_mutex`.
3. Thread 1's executing a callback that attempts to mutate the underlying
PathMappingList, attempting to grab `m_pairs_mutex`. Thread 1 is holding onto
`m_callback_mutex` and wants `m_pairs_mutex` and Thread 2 is holding onto
`m_pairs_mutex` and wants `m_callback_mutex`. Deadlock.
https://github.com/llvm/llvm-project/pull/114576
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Improve locking in PathMappingLists (NFC) (PR #114576)
https://github.com/bulbazord edited https://github.com/llvm/llvm-project/pull/114576 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Improve locking in PathMappingLists (NFC) (PR #114576)
https://github.com/bulbazord commented: It took me some time to read this and think about the implications, but I think this is generally okay. I left 1 comment about the order of lock acquisition for `operator=`. The only way I can see this going wrong is if a callback can mutate the list in a way that triggers yet another callback? Not sure we should be too concerned about that right now though. https://github.com/llvm/llvm-project/pull/114576 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Fix call site breakpoint patch (PR #114158)
slackito wrote:
Repro was pretty straightforward from the description above.
common.cc:
```c++
#include
#define XSTR(x) STR(x)
#define STR(x) #x
namespace NAMESPACE {
void DoSomeStuff() {
printf("%s::DoSomeStuff()\n", XSTR(NAMESPACE));
}
} // end NAMESPACE
```
main.cc:
```c++
namespace ns1 {
extern void DoSomeStuff();
} // namespace ns1
namespace ns2 {
extern void DoSomeStuff();
} // namespace ns2
namespace ns3 {
extern void DoSomeStuff();
} // namespace ns3
namespace ns4 {
extern void DoSomeStuff();
} // namespace ns4
int main(int argc, char* argv[]) {
ns1::DoSomeStuff();
ns2::DoSomeStuff();
ns3::DoSomeStuff();
ns4::DoSomeStuff();
return 0;
}
```
build.sh
```bash
#!/bin/bash
g++ -c -g -DNAMESPACE=ns1 -o ns1.o common.cc
g++ -c -g -DNAMESPACE=ns2 -o ns2.o common.cc
g++ -c -g -DNAMESPACE=ns3 -o ns3.o common.cc
g++ -c -g -DNAMESPACE=ns4 -o ns4.o common.cc
g++ -g -o main ns1.o ns2.o ns3.o ns4.o main.cc
```
with an lldb built at the revision prior to this commit:
```
$ lldb --no-lldbinit main
(lldb) target create "main"
Current executable set to '/usr/local/google/home/jgorbe/lldb-repro/main'
(x86_64).
(lldb) b common.cc:9
Breakpoint 1: 4 locations.
```
at this revision (and also at HEAD as of 5 minutes ago):
```
$ lldb --no-lldbinit main
(lldb) target create "main"
Current executable set to '/usr/local/google/home/jgorbe/lldb-repro/main'
(x86_64).
(lldb) b common.cc:9
Breakpoint 1: where = main`ns1::DoSomeStuff() + 4 at common.cc:9:9, address =
0x113d
```
https://github.com/llvm/llvm-project/pull/114158
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Fix call site breakpoint patch (PR #114158)
slackito wrote:
Hi,
I'm having some problems with one of our tests, that also reproduce after
[#114337](https://github.com/llvm/llvm-project/pull/114337).
I don't have a repro yet but I'll describe what I know of the problem so far in
case it rings a bell.
The test in question has a common source file with the following structure:
```
#if defined(PROTO_V2)
#include "test_proto_v2.pb.h"
namespace proto_v2 {
#elif defined(PROTO_V3)
#include "test_proto_v3.proto.h"
namespace proto_v3 {
#elif...
// some more cases
#endif
void DoSomeStuff() {
// common logic goes here
}
} // end whatever namespace got selected
```
This file gets compiled multiple times with `-DPROTO_V2`, `-DPROTO_V3` and so
on, and all of them get linked into a binary. Each version goes into a
different namespace so there are no duplicate definitions of anything, it's
just a little hack to avoid having four separate files with mostly the same
code.
Before this commit, running `b common.cc:123` results in a breakpoint with N
locations. After this commit, only one of them gets selected.
I'll do my best to share a repro case later today.
https://github.com/llvm/llvm-project/pull/114158
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB] Finish implementing support for DW_FORM_data16 (PR #113508)
https://github.com/clayborg approved this pull request. https://github.com/llvm/llvm-project/pull/113508 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Create dependent modules in parallel (PR #114507)
@@ -1608,9 +1612,28 @@ void Target::SetExecutableModule(ModuleSP &executable_sp,
if (image_module_sp) {
added_modules.AppendIfNeeded(image_module_sp, false);
ObjectFile *objfile = image_module_sp->GetObjectFile();
- if (objfile)
+ if (objfile) {
+std::lock_guard guard(dependent_files_mutex);
objfile->GetDependentModules(dependent_files);
jasonmolenda wrote:
good question. for mach-o this is a simple iteration over the load commands,
but we might want to to pass a temporaryfile list object to this method, and
then acquire the lock and append its entries to `dependency_files`, if we
didn't want to assume that.
https://github.com/llvm/llvm-project/pull/114507
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Create dependent modules in parallel (PR #114507)
https://github.com/jasonmolenda edited https://github.com/llvm/llvm-project/pull/114507 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][test] Skip libc++ tests if it is linked statically (PR #113935)
Michael137 wrote: > > Agree with Pavel. > > I'm not sure you do :P > > > > When libcxx is linked with a program as an archive of static libraries, > > > functions that aren't used in the program are omitted. > > > > > > This really isn't the problem. The tests _want_ those symbols to not be > > present and make sure that we can find those in the `std` Clang module. So > > I'm pretty sure the issue isn't about what symbols get stripped from the > > library > > There are different kinds of symbols going around here. The symbol you don't > want to be in the binary is `std::vector::at()` (because it can be > generated from the c++ module and stuff). However, the implementation of > `at()` will call (on the failure path) __libcxx_throw_out_of_range (or > something, I forgot it's name), which is a non-template function defined only > in some libc++ source (not header) file. The libc++ module will not contain a > definition of that. And if the program never calls it, the symbol won't be > extracted from the archive. I think the fix is to make sure this function is > present (by depending on it in some way), without pulling in std::vector and > friends. Ah I see, thanks for elaborating! Yea that makes sense. Referencing `__throw_out_of_range` somehow might work. Or just change the function we're calling from `at` to something that doesn't pull in the symbol from the dylib? That should still provide us with coverage of calling functions from the `std` module https://github.com/llvm/llvm-project/pull/113935 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [lldb] [llvm] [WIP][lldb][Expression] More reliable function call resolution (PR #114529)
https://github.com/Michael137 created
https://github.com/llvm/llvm-project/pull/114529
Naive implementation of all the parts of following RFC:
https://discourse.llvm.org/t/rfc-lldb-handling-abi-tagged-constructors-destructors-in-expression-evaluator/82816
Main changes:
1. Instead of relying on linkage names to resolve function symbols, encode the
exact function DIE and module in the `AsmLabelAttr`. Currently we just put the
`Module` pointer value into the label, which may be not sufficient if those
could disappear under the expression evaluator's feet.
2. Teach the LLDB symbol resolve about (1)
3. Introduce new Clang attribute to allow specifying multiple `asm` labels for
ctors/dtors (one for each variant)
4. Attach the new attribute in (3), where the mangled names use the format from
(1). To determine which variant a DIE corresponds to we add a new API to the
`ItaniumPartialDemangler` (though could be made into a DWARF attribute for
quicker determination).
>From 6b3b5ef0e50dd2e575922c47696ccffcd71460ea Mon Sep 17 00:00:00 2001
From: Michael Buch
Date: Sun, 20 Oct 2024 11:35:15 +0100
Subject: [PATCH] [WIP][lldb][Expression] More reliable function call
resolution
Implements all the parts of following RFC:
https://discourse.llvm.org/t/rfc-lldb-handling-abi-tagged-constructors-destructors-in-expression-evaluator/82816
Main changes:
1. Instead of relying on linkage names to resolve function symbols,
encode the exact function DIE and module in the `AsmLabelAttr`
2. Teach the LLDB symbol resolve about (1)
3. Introduce new Clang attribute to allow specifying multiple `asm`
labels for ctors/dtors (one for each variant)
4. Attach the new attribute in (3), where the mangled names use the
format from (1). To determine which variant a DIE corresponds to we
add a new API to the `ItaniumPartialDemangler` (though could be made
into a DWARF attribute for quicker determination).
---
clang/include/clang/Basic/Attr.td | 8 ++
clang/include/clang/Basic/AttrDocs.td | 5 ++
clang/lib/AST/Mangle.cpp | 65 ++-
clang/lib/Sema/SemaDeclAttr.cpp | 22 +
lldb/source/Expression/IRExecutionUnit.cpp| 36 +
.../SymbolFile/DWARF/DWARFASTParserClang.cpp | 80 +++
.../TypeSystem/Clang/TypeSystemClang.cpp | 15 +++-
.../TypeSystem/Clang/TypeSystemClang.h| 3 +-
lldb/test/Shell/Expr/TestAbiTagCtorsDtors.cpp | 28 +++
llvm/include/llvm/Demangle/Demangle.h | 3 +
llvm/include/llvm/Demangle/ItaniumDemangle.h | 2 +
llvm/lib/Demangle/ItaniumDemangle.cpp | 18 +++--
12 files changed, 260 insertions(+), 25 deletions(-)
create mode 100644 lldb/test/Shell/Expr/TestAbiTagCtorsDtors.cpp
diff --git a/clang/include/clang/Basic/Attr.td
b/clang/include/clang/Basic/Attr.td
index 70fad60d4edbb5..407eece2a728a2 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -784,6 +784,14 @@ def AbiTag : Attr {
let Documentation = [AbiTagsDocs];
}
+def StructorMangledNames : Attr {
+ let Spellings = [Clang<"structor_names">];
+ let Args = [VariadicStringArgument<"MangledNames">];
+ let Subjects = SubjectList<[Function], ErrorDiag>;
+ let Documentation = [StructorMangledNamesDocs];
+}
+
+
def AddressSpace : TypeAttr {
let Spellings = [Clang<"address_space">];
let Args = [IntArgument<"AddressSpace">];
diff --git a/clang/include/clang/Basic/AttrDocs.td
b/clang/include/clang/Basic/AttrDocs.td
index 546e5100b79dd9..2b886aecd193de 100644
--- a/clang/include/clang/Basic/AttrDocs.td
+++ b/clang/include/clang/Basic/AttrDocs.td
@@ -3568,6 +3568,11 @@ manipulating bits of the enumerator when issuing
warnings.
}];
}
+def StructorMangledNamesDocs : Documentation {
+ let Category = DocCatDecl;
+ let Content = [{ TODO }];
+}
+
def AsmLabelDocs : Documentation {
let Category = DocCatDecl;
let Content = [{
diff --git a/clang/lib/AST/Mangle.cpp b/clang/lib/AST/Mangle.cpp
index 4875e8537b3c11..c784fca053062c 100644
--- a/clang/lib/AST/Mangle.cpp
+++ b/clang/lib/AST/Mangle.cpp
@@ -9,19 +9,20 @@
// Implements generic name mangling support for blocks and Objective-C.
//
//===--===//
-#include "clang/AST/Attr.h"
+#include "clang/AST/Mangle.h"
#include "clang/AST/ASTContext.h"
+#include "clang/AST/Attr.h"
#include "clang/AST/Decl.h"
#include "clang/AST/DeclCXX.h"
#include "clang/AST/DeclObjC.h"
#include "clang/AST/DeclTemplate.h"
#include "clang/AST/ExprCXX.h"
-#include "clang/AST/Mangle.h"
#include "clang/AST/VTableBuilder.h"
#include "clang/Basic/ABI.h"
#include "clang/Basic/SourceManager.h"
#include "clang/Basic/TargetInfo.h"
#include "llvm/ADT/StringExtras.h"
+#include "llvm/ADT/StringSwitch.h"
#include "llvm/IR/DataLayout.h"
#include "llvm/IR/Mangler.h"
#include "llvm/Support/ErrorHandling.h"
@@ -126,7 +127,7 @@ bool MangleContext::shouldMangleDeclName(const NamedDecl
*
[Lldb-commits] [clang] [lldb] [llvm] [WIP][lldb][Expression] More reliable function call resolution (PR #114529)
@@ -140,6 +141,64 @@ void MangleContext::mangleName(GlobalDecl GD, raw_ostream
&Out) {
const ASTContext &ASTContext = getASTContext();
const NamedDecl *D = cast(GD.getDecl());
+ if (const StructorMangledNamesAttr *SMA =
+ D->getAttr()) {
+CXXConstructorDecl const *Ctor = dyn_cast(D);
+CXXDestructorDecl const *Dtor = dyn_cast(D);
+assert(Ctor || Dtor);
Michael137 wrote:
Ideally this won't look like this. But shows where I intend the mangler changes
to go for the new attribute.
https://github.com/llvm/llvm-project/pull/114529
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][test] Skip one inline stepping test for arm-ubuntu. (PR #114295)
DavidSpickett wrote:
dwarfdump:
[arm_dwarfdump.txt](https://github.com/user-attachments/files/17598949/arm_dwarfdump.txt)
(this doesn't seem different to what AArch64 produces)
objdump:
[arm_objdump.txt](https://github.com/user-attachments/files/17598953/arm_objdump.txt)
stepping log:
[arm_step_log.txt](https://github.com/user-attachments/files/17598983/arm_step_log.txt)
In the log file I break on main then step (`n`) once. It might be relevant that
we don't use hardware single step on Arm 32 bit (it exists we just don't use
it). So perhaps the combination of software single step and this inlined
function is a problem.
I noticed that although you can step into (`s`) and out of (`fin`) the inlined
function, if I did instruction steps (`ni`), lldb would not recognise that it
was inside the inlined function:
```
(lldb) ni
Process 1792462 stopped
* thread #1, name = 'a.out', stop reason = instruction step over
frame #0: 0x00400558 a.out`main(argc=1, argv=0xfffef3d4) at calling.cpp:11:5
8
9int main (int argc, char **argv) {
10 inline_value = 0;// Stop here and step over to set up stepping
over.
-> 11 inline_trivial_1 (); // At inline_trivial_1 called from main.
12 return 0;
13 }
(lldb) dis
a.out`main:
0x40052c <+0>: push {r11, lr}
0x400530 <+4>: movr11, sp
0x400534 <+8>: subsp, sp, #12
0x400538 <+12>: movw r2, #0x0
0x40053c <+16>: strr2, [r11, #-0x4]
0x400540 <+20>: strr0, [sp, #0x4]
0x400544 <+24>: strr1, [sp]
0x400548 <+28>: movw r0, #0x0
0x40054c <+32>: ldrr1, [pc, #0x14] ; <+60> at calling.cpp
0x400550 <+36>: addr1, pc, r1
0x400554 <+40>: strr0, [r1]
-> 0x400558 <+44>: nop
0x40055c <+48>: movw r0, #0x0
0x400560 <+52>: movsp, r11
0x400564 <+56>: pop{r11, pc}
0x400568 <+60>: andeq r0, r1, r4, ror #21
(lldb) bt
* thread #1, name = 'a.out', stop reason = instruction step over
* frame #0: 0x00400558 a.out`main(argc=1, argv=0xfffef3d4) at calling.cpp:11:5
frame #1: 0xf7c8d7d6 libc.so.6`__libc_start_call_main(main=(a.out`main at
calling.cpp:9), argc=1, argv=0xfffef3d4) at libc_start_call_main.h:58:16
frame #2: 0xf7c8d886 libc.so.6`__libc_start_main_impl(main=(a.out`main at
calling.cpp:9), argc=0, argv=0xf7d8e000, init=, fini=0x,
rtld_fini=(ld-linux-armhf.so.3`_dl_fini + 1 at dl-fini.c:51:20),
stack_end=0xfffef3d4) at libc-start.c:392:3
frame #3: 0x00400458 a.out`_start + 40
```
If I add a second nop, it will realise it's in the inlined function when it
gets to the second nop.
https://github.com/llvm/llvm-project/pull/114295
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [lldb] [llvm] [WIP][lldb][Expression] More reliable function call resolution (PR #114529)
https://github.com/Michael137 updated
https://github.com/llvm/llvm-project/pull/114529
>From 9337e170d920eaabe2b59a25622f0c554ca5afcf Mon Sep 17 00:00:00 2001
From: Michael Buch
Date: Sun, 20 Oct 2024 11:35:15 +0100
Subject: [PATCH] [WIP][lldb][Expression] More reliable function call
resolution
Implements all the parts of following RFC:
https://discourse.llvm.org/t/rfc-lldb-handling-abi-tagged-constructors-destructors-in-expression-evaluator/82816
Main changes:
1. Instead of relying on linkage names to resolve function symbols,
encode the exact function DIE and module in the `AsmLabelAttr`
2. Teach the LLDB symbol resolve about (1)
3. Introduce new Clang attribute to allow specifying multiple `asm`
labels for ctors/dtors (one for each variant)
4. Attach the new attribute in (3), where the mangled names use the
format from (1). To determine which variant a DIE corresponds to we
add a new API to the `ItaniumPartialDemangler` (though could be made
into a DWARF attribute for quicker determination).
---
clang/include/clang/Basic/Attr.td | 8 ++
clang/include/clang/Basic/AttrDocs.td | 5 ++
clang/lib/AST/Mangle.cpp | 69 +++-
clang/lib/Sema/SemaDeclAttr.cpp | 22 +
lldb/source/Expression/IRExecutionUnit.cpp| 36 +
.../SymbolFile/DWARF/DWARFASTParserClang.cpp | 80 +++
.../TypeSystem/Clang/TypeSystemClang.cpp | 15 +++-
.../TypeSystem/Clang/TypeSystemClang.h| 3 +-
lldb/test/Shell/Expr/TestAbiTagCtorsDtors.cpp | 28 +++
llvm/include/llvm/Demangle/Demangle.h | 3 +
llvm/include/llvm/Demangle/ItaniumDemangle.h | 2 +
llvm/lib/Demangle/ItaniumDemangle.cpp | 18 +++--
12 files changed, 264 insertions(+), 25 deletions(-)
create mode 100644 lldb/test/Shell/Expr/TestAbiTagCtorsDtors.cpp
diff --git a/clang/include/clang/Basic/Attr.td
b/clang/include/clang/Basic/Attr.td
index 70fad60d4edbb5..407eece2a728a2 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -784,6 +784,14 @@ def AbiTag : Attr {
let Documentation = [AbiTagsDocs];
}
+def StructorMangledNames : Attr {
+ let Spellings = [Clang<"structor_names">];
+ let Args = [VariadicStringArgument<"MangledNames">];
+ let Subjects = SubjectList<[Function], ErrorDiag>;
+ let Documentation = [StructorMangledNamesDocs];
+}
+
+
def AddressSpace : TypeAttr {
let Spellings = [Clang<"address_space">];
let Args = [IntArgument<"AddressSpace">];
diff --git a/clang/include/clang/Basic/AttrDocs.td
b/clang/include/clang/Basic/AttrDocs.td
index 546e5100b79dd9..2b886aecd193de 100644
--- a/clang/include/clang/Basic/AttrDocs.td
+++ b/clang/include/clang/Basic/AttrDocs.td
@@ -3568,6 +3568,11 @@ manipulating bits of the enumerator when issuing
warnings.
}];
}
+def StructorMangledNamesDocs : Documentation {
+ let Category = DocCatDecl;
+ let Content = [{ TODO }];
+}
+
def AsmLabelDocs : Documentation {
let Category = DocCatDecl;
let Content = [{
diff --git a/clang/lib/AST/Mangle.cpp b/clang/lib/AST/Mangle.cpp
index 4875e8537b3c11..9b304d28113625 100644
--- a/clang/lib/AST/Mangle.cpp
+++ b/clang/lib/AST/Mangle.cpp
@@ -9,19 +9,20 @@
// Implements generic name mangling support for blocks and Objective-C.
//
//===--===//
-#include "clang/AST/Attr.h"
+#include "clang/AST/Mangle.h"
#include "clang/AST/ASTContext.h"
+#include "clang/AST/Attr.h"
#include "clang/AST/Decl.h"
#include "clang/AST/DeclCXX.h"
#include "clang/AST/DeclObjC.h"
#include "clang/AST/DeclTemplate.h"
#include "clang/AST/ExprCXX.h"
-#include "clang/AST/Mangle.h"
#include "clang/AST/VTableBuilder.h"
#include "clang/Basic/ABI.h"
#include "clang/Basic/SourceManager.h"
#include "clang/Basic/TargetInfo.h"
#include "llvm/ADT/StringExtras.h"
+#include "llvm/ADT/StringSwitch.h"
#include "llvm/IR/DataLayout.h"
#include "llvm/IR/Mangler.h"
#include "llvm/Support/ErrorHandling.h"
@@ -126,7 +127,7 @@ bool MangleContext::shouldMangleDeclName(const NamedDecl
*D) {
// Any decl can be declared with __asm("foo") on it, and this takes
precedence
// over all other naming in the .o file.
- if (D->hasAttr())
+ if (D->hasAttr() || D->hasAttr())
return true;
// Declarations that don't have identifier names always need to be mangled.
@@ -140,6 +141,68 @@ void MangleContext::mangleName(GlobalDecl GD, raw_ostream
&Out) {
const ASTContext &ASTContext = getASTContext();
const NamedDecl *D = cast(GD.getDecl());
+ if (const StructorMangledNamesAttr *SMA =
+ D->getAttr()) {
+CXXConstructorDecl const *Ctor = dyn_cast(D);
+CXXDestructorDecl const *Dtor = dyn_cast(D);
+assert(Ctor || Dtor);
+enum CtorDtor {
+ None = -1,
+ Deleting = 0,
+ Base,
+ Complete,
+ Allocating
+} CtorDtorVariant = None;
+
+// Map ctor/dtor variant to a the variant that LLDB encoded in the
+
[Lldb-commits] [clang] [lldb] [llvm] [WIP][lldb][Expression] More reliable function call resolution (PR #114529)
https://github.com/Michael137 edited https://github.com/llvm/llvm-project/pull/114529 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [lldb] [llvm] [WIP][lldb][Expression] More reliable function call resolution (PR #114529)
https://github.com/Michael137 updated
https://github.com/llvm/llvm-project/pull/114529
>From 7f18d96b7117e83c4ed246ac498ee4c9a72064ff Mon Sep 17 00:00:00 2001
From: Michael Buch
Date: Sun, 20 Oct 2024 11:35:15 +0100
Subject: [PATCH] [WIP][lldb][Expression] More reliable function call
resolution
Implements all the parts of following RFC:
https://discourse.llvm.org/t/rfc-lldb-handling-abi-tagged-constructors-destructors-in-expression-evaluator/82816
Main changes:
1. Instead of relying on linkage names to resolve function symbols,
encode the exact function DIE and module in the `AsmLabelAttr`
2. Teach the LLDB symbol resolve about (1)
3. Introduce new Clang attribute to allow specifying multiple `asm`
labels for ctors/dtors (one for each variant)
4. Attach the new attribute in (3), where the mangled names use the
format from (1). To determine which variant a DIE corresponds to we
add a new API to the `ItaniumPartialDemangler` (though could be made
into a DWARF attribute for quicker determination).
---
clang/include/clang/Basic/Attr.td | 8 ++
clang/include/clang/Basic/AttrDocs.td | 5 ++
clang/lib/AST/Mangle.cpp | 65 ++-
clang/lib/Sema/SemaDeclAttr.cpp | 22 +
lldb/source/Expression/IRExecutionUnit.cpp| 36 +
.../SymbolFile/DWARF/DWARFASTParserClang.cpp | 80 +++
.../TypeSystem/Clang/TypeSystemClang.cpp | 15 +++-
.../TypeSystem/Clang/TypeSystemClang.h| 3 +-
lldb/test/Shell/Expr/TestAbiTagCtorsDtors.cpp | 28 +++
llvm/include/llvm/Demangle/Demangle.h | 3 +
llvm/include/llvm/Demangle/ItaniumDemangle.h | 2 +
llvm/lib/Demangle/ItaniumDemangle.cpp | 18 +++--
12 files changed, 260 insertions(+), 25 deletions(-)
create mode 100644 lldb/test/Shell/Expr/TestAbiTagCtorsDtors.cpp
diff --git a/clang/include/clang/Basic/Attr.td
b/clang/include/clang/Basic/Attr.td
index 70fad60d4edbb5..407eece2a728a2 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -784,6 +784,14 @@ def AbiTag : Attr {
let Documentation = [AbiTagsDocs];
}
+def StructorMangledNames : Attr {
+ let Spellings = [Clang<"structor_names">];
+ let Args = [VariadicStringArgument<"MangledNames">];
+ let Subjects = SubjectList<[Function], ErrorDiag>;
+ let Documentation = [StructorMangledNamesDocs];
+}
+
+
def AddressSpace : TypeAttr {
let Spellings = [Clang<"address_space">];
let Args = [IntArgument<"AddressSpace">];
diff --git a/clang/include/clang/Basic/AttrDocs.td
b/clang/include/clang/Basic/AttrDocs.td
index 546e5100b79dd9..2b886aecd193de 100644
--- a/clang/include/clang/Basic/AttrDocs.td
+++ b/clang/include/clang/Basic/AttrDocs.td
@@ -3568,6 +3568,11 @@ manipulating bits of the enumerator when issuing
warnings.
}];
}
+def StructorMangledNamesDocs : Documentation {
+ let Category = DocCatDecl;
+ let Content = [{ TODO }];
+}
+
def AsmLabelDocs : Documentation {
let Category = DocCatDecl;
let Content = [{
diff --git a/clang/lib/AST/Mangle.cpp b/clang/lib/AST/Mangle.cpp
index 4875e8537b3c11..c784fca053062c 100644
--- a/clang/lib/AST/Mangle.cpp
+++ b/clang/lib/AST/Mangle.cpp
@@ -9,19 +9,20 @@
// Implements generic name mangling support for blocks and Objective-C.
//
//===--===//
-#include "clang/AST/Attr.h"
+#include "clang/AST/Mangle.h"
#include "clang/AST/ASTContext.h"
+#include "clang/AST/Attr.h"
#include "clang/AST/Decl.h"
#include "clang/AST/DeclCXX.h"
#include "clang/AST/DeclObjC.h"
#include "clang/AST/DeclTemplate.h"
#include "clang/AST/ExprCXX.h"
-#include "clang/AST/Mangle.h"
#include "clang/AST/VTableBuilder.h"
#include "clang/Basic/ABI.h"
#include "clang/Basic/SourceManager.h"
#include "clang/Basic/TargetInfo.h"
#include "llvm/ADT/StringExtras.h"
+#include "llvm/ADT/StringSwitch.h"
#include "llvm/IR/DataLayout.h"
#include "llvm/IR/Mangler.h"
#include "llvm/Support/ErrorHandling.h"
@@ -126,7 +127,7 @@ bool MangleContext::shouldMangleDeclName(const NamedDecl
*D) {
// Any decl can be declared with __asm("foo") on it, and this takes
precedence
// over all other naming in the .o file.
- if (D->hasAttr())
+ if (D->hasAttr() || D->hasAttr())
return true;
// Declarations that don't have identifier names always need to be mangled.
@@ -140,6 +141,64 @@ void MangleContext::mangleName(GlobalDecl GD, raw_ostream
&Out) {
const ASTContext &ASTContext = getASTContext();
const NamedDecl *D = cast(GD.getDecl());
+ if (const StructorMangledNamesAttr *SMA =
+ D->getAttr()) {
+CXXConstructorDecl const *Ctor = dyn_cast(D);
+CXXDestructorDecl const *Dtor = dyn_cast(D);
+assert(Ctor || Dtor);
+enum CtorDtor {
+ None = -1,
+ Deleting = 0,
+ Base,
+ Complete,
+ Allocating
+} CtorDtorVariant = None;
+
+if (Dtor) {
+ switch (GD.getDtorType()) {
+ case Dtor_Complete
