[Lldb-commits] [lldb] Fix saving minidump from lldb-dap (PR #89113)

2024-04-17 Thread via lldb-commits

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


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


[Lldb-commits] [lldb] Summarize std::string's when created from data. (PR #89110)

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

https://github.com/Jlalond updated 
https://github.com/llvm/llvm-project/pull/89110

>From e0316188d22605c670079e37855d3d8b5c944cee Mon Sep 17 00:00:00 2001
From: Jacob John Lalonde 
Date: Wed, 10 Apr 2024 14:33:40 -0700
Subject: [PATCH 1/2] Fix bug where an sbvalue containing a std::string created
 from data would not use the built in summary provider, but default to the
 valueobjectprinter

std::string children are normally formatted as their summary [my_string_prop] = 
"hello world".
Sbvalues created from data do not follow the same summary formatter path and 
instead hit the value object printer.
This change fixes that and adds tests for the future.

Added tests in TestDataFormatterStdString.py and a formatter for a custom 
struct.
---
 .../Plugins/Language/CPlusPlus/LibStdcpp.cpp  | 31 ++--
 .../string/ConvertToDataFormatter.py  | 50 +++
 .../string/TestDataFormatterStdString.py  | 28 +++
 .../libstdcpp/string/main.cpp | 13 +
 4 files changed, 118 insertions(+), 4 deletions(-)
 create mode 100644 
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/ConvertToDataFormatter.py

diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp 
b/lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp
index 86bb575af5ca34..0da01e9ca07fb6 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp
@@ -254,13 +254,13 @@ bool 
lldb_private::formatters::LibStdcppStringSummaryProvider(
   } else
 addr_of_string =
 valobj.GetAddressOf(scalar_is_load_addr, _type);
-  if (addr_of_string != LLDB_INVALID_ADDRESS) {
+  if (addr_of_string != LLDB_INVALID_ADDRESS ||
+addr_type == eAddressTypeHost) {
 switch (addr_type) {
 case eAddressTypeLoad: {
   ProcessSP process_sp(valobj.GetProcessSP());
   if (!process_sp)
 return false;
-
   StringPrinter::ReadStringAndDumpToStreamOptions options(valobj);
   Status error;
   lldb::addr_t addr_of_data =
@@ -287,8 +287,31 @@ bool 
lldb_private::formatters::LibStdcppStringSummaryProvider(
   } else
 return true;
 } break;
-case eAddressTypeHost:
-  break;
+case eAddressTypeHost: {
+  // We have the host address of our std::string
+  // But we need to read the pointee data from the debugged process.
+  ProcessSP process_sp(valobj.GetProcessSP());
+  DataExtractor data;
+  Status error;
+  valobj.GetData(data, error);
+  if (error.Fail())
+return false;
+  // We want to read the address from std::string, which is the first 8 
bytes.
+  lldb::offset_t offset = 0;
+  lldb::addr_t addr = data.GetAddress();
+  if (!addr)
+  {
+stream.Printf("nullptr");
+return true;
+  }
+
+  std::string contents;
+  process_sp->ReadCStringFromMemory(addr, contents, error);
+  if (error.Fail())
+return false;
+  stream.Printf("%s", contents.c_str());
+  return true;
+} break;
 case eAddressTypeInvalid:
 case eAddressTypeFile:
   break;
diff --git 
a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/ConvertToDataFormatter.py
 
b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/ConvertToDataFormatter.py
new file mode 100644
index 00..57e42c920f8294
--- /dev/null
+++ 
b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/ConvertToDataFormatter.py
@@ -0,0 +1,50 @@
+# coding=utf8
+"""
+Helper formmater to verify Std::String by created via SBData
+"""
+
+import lldb
+
+class SyntheticFormatter:
+def __init__(self, valobj, dict):
+self.valobj = valobj
+
+def num_children(self):
+return 6
+
+def has_children(self):
+return True
+
+def get_child_at_index(self, index):
+name = None
+match index:
+case 0:
+name = "short_string"
+case 1:
+name = "long_string"
+case 2:
+name = "short_string_ptr"
+case 3:
+name = "long_string_ptr"
+case 4:
+name = "short_string_ref"
+case 5:
+name = "long_string_ref"
+case _:
+return None
+
+child = self.valobj.GetChildMemberWithName(name)
+valType = child.GetType()
+return self.valobj.CreateValueFromData(name,
+child.GetData(),
+valType)
+
+
+def __lldb_init_module(debugger, dict):
+typeName = "string_container"
+debugger.HandleCommand(
+'type synthetic add -x "'
++ typeName
++ '" --python-class '
++ f"{__name__}.SyntheticFormatter"
+)
diff --git 
a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/TestDataFormatterStdString.py
 

[Lldb-commits] [lldb] [lldb][MinidumpFileBuilder] Fix addition of MemoryList steam (PR #88564)

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

https://github.com/mbucko updated 
https://github.com/llvm/llvm-project/pull/88564

>From 8877be23ad4d342e7f9b61896581707005f76d4e Mon Sep 17 00:00:00 2001
From: Miro Bucko 
Date: Fri, 12 Apr 2024 09:55:46 -0700
Subject: [PATCH] [lldb][MinidumpFileBuilder] Fix addition of MemoryList steam

Summary:
AddMemoryList() was returning the last error status returned by ReadMemory(). 
So if an invalid memory region was read last, the function would return an 
error. Also, one of the reasons why the invalid memory region was read was 
because the check for reading permission in AddRegion() was incorrect.

Test Plan:
./bin/llvm-lit -sv 
~/src/llvm-project/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py

Reviewers:

Subscribers:

Tasks:

Tags:
---
 .../ObjectFile/Minidump/MinidumpFileBuilder.cpp  | 16 +---
 lldb/source/Target/Process.cpp   |  7 +--
 2 files changed, 18 insertions(+), 5 deletions(-)

diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp 
b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
index 50d1b563f469cf..25a57c6a81b091 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
+++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
@@ -20,6 +20,8 @@
 #include "lldb/Target/StopInfo.h"
 #include "lldb/Target/ThreadList.h"
 #include "lldb/Utility/DataExtractor.h"
+#include "lldb/Utility/LLDBLog.h"
+#include "lldb/Utility/Log.h"
 #include "lldb/Utility/RegisterValue.h"
 
 #include "llvm/ADT/StringRef.h"
@@ -649,16 +651,24 @@ MinidumpFileBuilder::AddMemoryList(const lldb::ProcessSP 
_sp,
   DataBufferHeap helper_data;
   std::vector mem_descriptors;
   for (const auto _range : core_ranges) {
-// Skip empty memory regions or any regions with no permissions.
-if (core_range.range.empty() || core_range.lldb_permissions == 0)
+// Skip empty memory regions.
+if (core_range.range.empty())
   continue;
 const addr_t addr = core_range.range.start();
 const addr_t size = core_range.range.size();
 auto data_up = std::make_unique(size, 0);
 const size_t bytes_read =
 process_sp->ReadMemory(addr, data_up->GetBytes(), size, error);
-if (bytes_read == 0)
+if (error.Fail()) {
+  Log *log = GetLog(LLDBLog::Object);
+  LLDB_LOGF(log, "Failed to read memory region. Bytes read: %zu, error: 
%s",
+bytes_read, error.AsCString());
+  error.Clear();
+}
+if (bytes_read == 0) {
   continue;
+}
+
 // We have a good memory region with valid bytes to store.
 LocationDescriptor memory_dump;
 memory_dump.DataSize = static_cast(bytes_read);
diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp
index f02ec37cb0f08f..606518ca541267 100644
--- a/lldb/source/Target/Process.cpp
+++ b/lldb/source/Target/Process.cpp
@@ -6325,8 +6325,11 @@ static bool AddDirtyPages(const MemoryRegionInfo ,
 // ranges.
 static void AddRegion(const MemoryRegionInfo , bool try_dirty_pages,
   Process::CoreFileMemoryRanges ) {
-  // Don't add empty ranges or ranges with no permissions.
-  if (region.GetRange().GetByteSize() == 0 || region.GetLLDBPermissions() == 0)
+  // Don't add empty ranges.
+  if (region.GetRange().GetByteSize() == 0)
+return;
+  // Don't add ranges with no read permissions.
+  if ((region.GetLLDBPermissions() & lldb::ePermissionsReadable) == 0)
 return;
   if (try_dirty_pages && AddDirtyPages(region, ranges))
 return;

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


[Lldb-commits] [lldb] Summarize std::string's when created from data. (PR #89110)

2024-04-17 Thread via lldb-commits


@@ -254,13 +254,13 @@ bool 
lldb_private::formatters::LibStdcppStringSummaryProvider(
   } else
 addr_of_string =
 valobj.GetAddressOf(scalar_is_load_addr, _type);
-  if (addr_of_string != LLDB_INVALID_ADDRESS) {
+  if (addr_of_string != LLDB_INVALID_ADDRESS ||
+addr_type == eAddressTypeHost) {

kusmour wrote:

Ohh wow, so for this specific situation we will have `addr_of_string == 
LLDB_INVALID_ADDRESS`?
Would you mind adding a comment for that?

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


[Lldb-commits] [lldb] Summarize std::string's when created from data. (PR #89110)

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

Jlalond wrote:

@jimingham

What should we do if the child address type is File? I don't know how that 
could ever happen, and should we support it? Because currently we didn't 
support summarizing host or file.


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


[Lldb-commits] [lldb] Summarize std::string's when created from data. (PR #89110)

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


@@ -254,13 +254,13 @@ bool 
lldb_private::formatters::LibStdcppStringSummaryProvider(
   } else
 addr_of_string =
 valobj.GetAddressOf(scalar_is_load_addr, _type);
-  if (addr_of_string != LLDB_INVALID_ADDRESS) {
+  if (addr_of_string != LLDB_INVALID_ADDRESS ||
+addr_type == eAddressTypeHost) {

Jlalond wrote:

Yes, the switch statement is within the if block, if we don't add this 
exception we will return false and fall back to the ValueObjectPrinter

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


[Lldb-commits] [lldb] [lldb][DynamicLoader] Fix lldb unable to stop at _dl_debug_state if user set it before the process launched. (PR #88792)

2024-04-17 Thread Zequan Wu via lldb-commits

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


[Lldb-commits] [lldb] 60b90b5 - [lldb][DynamicLoader] Fix lldb unable to stop at _dl_debug_state if user set it before the process launched. (#88792)

2024-04-17 Thread via lldb-commits

Author: Zequan Wu
Date: 2024-04-17T16:09:38-04:00
New Revision: 60b90b523323f8196a9e4a68b1f33358624c09eb

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

LOG: [lldb][DynamicLoader] Fix lldb unable to stop at _dl_debug_state if user 
set it before the process launched. (#88792)

If user sets a breakpoint at `_dl_debug_state` before the process
launched, the breakpoint is not resolved yet. When lldb loads dynamic
loader module, it's created with `Target::GetOrCreateModule` which
notifies any pending breakpoint to resolve. However, the module's
sections are not loaded at this time. They are loaded after returned
from
[Target::GetOrCreateModule](https://github.com/llvm/llvm-project/blob/0287a5cc4e2a5ded1ae2e4079f91052e6a6b8d9b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp#L574-L577).
This change fixes it by manually resolving breakpoints after creating
dynamic loader module.

Added: 


Modified: 
lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp

lldb/test/API/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py

Removed: 




diff  --git 
a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp 
b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
index 9baf86da4dc799..9fa245fc41d40c 100644
--- a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
+++ b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
@@ -571,10 +571,17 @@ ModuleSP DynamicLoaderPOSIXDYLD::LoadInterpreterModule() {
   FileSpec file(info.GetName().GetCString());
   ModuleSpec module_spec(file, target.GetArchitecture());
 
-  if (ModuleSP module_sp = target.GetOrCreateModule(module_spec,
-true /* notify */)) {
+  // Don't notify that module is added here because its loading section
+  // addresses are not updated yet. We manually notify it below.
+  if (ModuleSP module_sp =
+  target.GetOrCreateModule(module_spec, /*notify=*/false)) {
 UpdateLoadedSections(module_sp, LLDB_INVALID_ADDRESS, m_interpreter_base,
  false);
+// Manually notify that dynamic linker is loaded after updating load 
section
+// addersses so that breakpoints can be resolved.
+ModuleList module_list;
+module_list.Append(module_sp);
+target.ModulesDidLoad(module_list);
 m_interpreter_module = module_sp;
 return module_sp;
   }

diff  --git 
a/lldb/test/API/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py
 
b/lldb/test/API/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py
index ea242952e409ec..850235fdcefa70 100644
--- 
a/lldb/test/API/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py
+++ 
b/lldb/test/API/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py
@@ -671,3 +671,20 @@ def test_breakpoint_statistics_hitcount(self):
 self.assertNotEqual(breakpoints_stats, None)
 for breakpoint_stats in breakpoints_stats:
 self.assertIn("hitCount", breakpoint_stats)
+
+@skipIf(oslist=no_match(["linux"]))
+def test_break_at__dl_debug_state(self):
+"""
+Test lldb is able to stop at _dl_debug_state if it is set before the
+process is launched.
+"""
+self.build()
+exe = self.getBuildArtifact("a.out")
+self.runCmd("target create %s" % exe)
+bpid = lldbutil.run_break_set_by_symbol(
+self, "_dl_debug_state", num_expected_locations=0
+)
+self.runCmd("run")
+self.assertIsNotNone(
+lldbutil.get_one_thread_stopped_at_breakpoint_id(self.process(), 
bpid)
+)



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


[Lldb-commits] [lldb] [lldb][DynamicLoader] Fix lldb unable to stop at _dl_debug_state if user set it before the process launched. (PR #88792)

2024-04-17 Thread Zequan Wu via lldb-commits


@@ -572,9 +572,14 @@ ModuleSP DynamicLoaderPOSIXDYLD::LoadInterpreterModule() {
   ModuleSpec module_spec(file, target.GetArchitecture());
 
   if (ModuleSP module_sp = target.GetOrCreateModule(module_spec,
-true /* notify */)) {
+false /* notify */)) {

ZequanWu wrote:

Updated coding style and added a comment here.

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


[Lldb-commits] [lldb] [lldb][DynamicLoader] Fix lldb unable to stop at _dl_debug_state if user set it before the process launched. (PR #88792)

2024-04-17 Thread Zequan Wu via lldb-commits

https://github.com/ZequanWu updated 
https://github.com/llvm/llvm-project/pull/88792

>From 26528cd679478448edf446e0e82e5f207ffd6113 Mon Sep 17 00:00:00 2001
From: Zequan Wu 
Date: Mon, 15 Apr 2024 16:30:38 -0400
Subject: [PATCH 1/5] [lldb][DynamicLoader] Fix lldb unable to stop at
 _dl_debug_state if user set it before the process launched.

---
 lldb/include/lldb/Target/Target.h   |  3 +++
 .../POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp   |  5 +
 lldb/source/Target/Target.cpp   | 17 ++---
 .../breakpoint_command/TestBreakpointCommand.py | 17 +
 4 files changed, 35 insertions(+), 7 deletions(-)

diff --git a/lldb/include/lldb/Target/Target.h 
b/lldb/include/lldb/Target/Target.h
index 2c2e6b2831ccee..3554ef0ea5a140 100644
--- a/lldb/include/lldb/Target/Target.h
+++ b/lldb/include/lldb/Target/Target.h
@@ -878,6 +878,9 @@ class Target : public std::enable_shared_from_this,
   // the address of its previous instruction and return that address.
   lldb::addr_t GetBreakableLoadAddress(lldb::addr_t addr);
 
+  void UpdateBreakpoints(ModuleList _list, bool added,
+ bool delete_locations);
+
   void ModulesDidLoad(ModuleList _list);
 
   void ModulesDidUnload(ModuleList _list, bool delete_locations);
diff --git 
a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp 
b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
index 9baf86da4dc799..901fa53682da8e 100644
--- a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
+++ b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
@@ -576,6 +576,11 @@ ModuleSP DynamicLoaderPOSIXDYLD::LoadInterpreterModule() {
 UpdateLoadedSections(module_sp, LLDB_INVALID_ADDRESS, m_interpreter_base,
  false);
 m_interpreter_module = module_sp;
+// Manually update breakpoints after updating loaded sections, because they
+// cannot be resolve at the time when creating this module.
+ModuleList module_list;
+module_list.Append(module_sp);
+m_process->GetTarget().UpdateBreakpoints(module_list, true, false);
 return module_sp;
   }
   return nullptr;
diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp
index 09b0ac42631d1a..ec2dea5cc238d3 100644
--- a/lldb/source/Target/Target.cpp
+++ b/lldb/source/Target/Target.cpp
@@ -1687,6 +1687,13 @@ void 
Target::NotifyModulesRemoved(lldb_private::ModuleList _list) {
   ModulesDidUnload(module_list, false);
 }
 
+void Target::UpdateBreakpoints(ModuleList _list, bool added,
+   bool delete_locations) {
+  m_breakpoint_list.UpdateBreakpoints(module_list, added, delete_locations);
+  m_internal_breakpoint_list.UpdateBreakpoints(module_list, added,
+   delete_locations);
+}
+
 void Target::ModulesDidLoad(ModuleList _list) {
   const size_t num_images = module_list.GetSize();
   if (m_valid && num_images) {
@@ -1694,8 +1701,7 @@ void Target::ModulesDidLoad(ModuleList _list) {
   ModuleSP module_sp(module_list.GetModuleAtIndex(idx));
   LoadScriptingResourceForModule(module_sp, this);
 }
-m_breakpoint_list.UpdateBreakpoints(module_list, true, false);
-m_internal_breakpoint_list.UpdateBreakpoints(module_list, true, false);
+UpdateBreakpoints(module_list, true, false);
 if (m_process_sp) {
   m_process_sp->ModulesDidLoad(module_list);
 }
@@ -1713,8 +1719,7 @@ void Target::SymbolsDidLoad(ModuleList _list) {
   }
 }
 
-m_breakpoint_list.UpdateBreakpoints(module_list, true, false);
-m_internal_breakpoint_list.UpdateBreakpoints(module_list, true, false);
+UpdateBreakpoints(module_list, true, false);
 auto data_sp =
 std::make_shared(shared_from_this(), module_list);
 BroadcastEvent(eBroadcastBitSymbolsLoaded, data_sp);
@@ -1727,9 +1732,7 @@ void Target::ModulesDidUnload(ModuleList _list, 
bool delete_locations) {
 auto data_sp =
 std::make_shared(shared_from_this(), module_list);
 BroadcastEvent(eBroadcastBitModulesUnloaded, data_sp);
-m_breakpoint_list.UpdateBreakpoints(module_list, false, delete_locations);
-m_internal_breakpoint_list.UpdateBreakpoints(module_list, false,
- delete_locations);
+UpdateBreakpoints(module_list, false, delete_locations);
 
 // If a module was torn down it will have torn down the 'TypeSystemClang's
 // that we used as source 'ASTContext's for the persistent variables in
diff --git 
a/lldb/test/API/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py
 
b/lldb/test/API/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py
index ea242952e409ec..a7e23fae14a21f 100644
--- 
a/lldb/test/API/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py
+++ 

[Lldb-commits] [lldb] adds additional information to the ProcessInfo object for elf processes (PR #88995)

2024-04-17 Thread Fred Grim via lldb-commits

feg208 wrote:

> Yes, you'd have to put this test into a host test for a system that doesn't 
> support this, like use the Posix one or make one for the Windows or Darwin 
> Hosts.

Done!

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


[Lldb-commits] [lldb] adds additional information to the ProcessInfo object for elf processes (PR #88995)

2024-04-17 Thread Fred Grim via lldb-commits

https://github.com/feg208 updated 
https://github.com/llvm/llvm-project/pull/88995

>From 9b8ec4d0c31ad1b228add56bc27cd79457e515c7 Mon Sep 17 00:00:00 2001
From: Fred Grim 
Date: Tue, 16 Apr 2024 14:46:37 -0700
Subject: [PATCH 1/3] adds additional information to the ProcessInfo object for
 elf processes

---
 lldb/include/lldb/Utility/ProcessInfo.h |  71 ++
 lldb/source/Host/linux/Host.cpp | 125 
 lldb/unittests/Host/linux/HostTest.cpp  |   6 ++
 3 files changed, 182 insertions(+), 20 deletions(-)

diff --git a/lldb/include/lldb/Utility/ProcessInfo.h 
b/lldb/include/lldb/Utility/ProcessInfo.h
index 7fb5b37be0f48f..e9fe71e1b851d1 100644
--- a/lldb/include/lldb/Utility/ProcessInfo.h
+++ b/lldb/include/lldb/Utility/ProcessInfo.h
@@ -139,6 +139,11 @@ class ProcessInfo {
 // to that process.
 class ProcessInstanceInfo : public ProcessInfo {
 public:
+  struct timespec {
+time_t tv_sec = 0;
+long int tv_usec = 0;
+  };
+
   ProcessInstanceInfo() = default;
 
   ProcessInstanceInfo(const char *name, const ArchSpec , lldb::pid_t pid)
@@ -172,6 +177,66 @@ class ProcessInstanceInfo : public ProcessInfo {
 return m_parent_pid != LLDB_INVALID_PROCESS_ID;
   }
 
+  lldb::pid_t GetProcessGroupID() const { return m_process_group_id; }
+
+  void SetProcessGroupID(lldb::pid_t pgrp) { m_process_group_id = pgrp; }
+
+  bool ProcessGroupIDIsValid() const {
+return m_process_group_id != LLDB_INVALID_PROCESS_ID;
+  }
+
+  lldb::pid_t GetProcessSessionID() const { return m_process_session_id; }
+
+  void SetProcessSessionID(lldb::pid_t session) {
+m_process_session_id = session;
+  }
+
+  bool ProcessSessionIDIsValid() const {
+return m_process_session_id != LLDB_INVALID_PROCESS_ID;
+  }
+
+  struct timespec GetUserTime() const { return m_user_time; }
+
+  void SetUserTime(struct timespec utime) { m_user_time = utime; }
+
+  bool UserTimeIsValid() const {
+return m_user_time.tv_sec > 0 || m_user_time.tv_usec > 0;
+  }
+
+  struct timespec GetSystemTime() const { return m_system_time; }
+
+  void SetSystemTime(struct timespec stime) { m_system_time = stime; }
+
+  bool SystemTimeIsValid() const {
+return m_system_time.tv_sec > 0 || m_system_time.tv_usec > 0;
+  }
+
+  struct timespec GetCumulativeUserTime() const {
+return m_cumulative_user_time;
+  }
+
+  void SetCumulativeUserTime(struct timespec cutime) {
+m_cumulative_user_time = cutime;
+  }
+
+  bool CumulativeUserTimeIsValid() const {
+return m_cumulative_user_time.tv_sec > 0 ||
+   m_cumulative_user_time.tv_usec > 0;
+  }
+
+  struct timespec GetCumulativeSystemTime() const {
+return m_cumulative_system_time;
+  }
+
+  void SetCumulativeSystemTime(struct timespec cstime) {
+m_cumulative_system_time = cstime;
+  }
+
+  bool CumulativeSystemTimeIsValid() const {
+return m_cumulative_system_time.tv_sec > 0 ||
+   m_cumulative_system_time.tv_sec > 0;
+  }
+
   void Dump(Stream , UserIDResolver ) const;
 
   static void DumpTableHeader(Stream , bool show_args, bool verbose);
@@ -183,6 +248,12 @@ class ProcessInstanceInfo : public ProcessInfo {
   uint32_t m_euid = UINT32_MAX;
   uint32_t m_egid = UINT32_MAX;
   lldb::pid_t m_parent_pid = LLDB_INVALID_PROCESS_ID;
+  lldb::pid_t m_process_group_id = LLDB_INVALID_PROCESS_ID;
+  lldb::pid_t m_process_session_id = LLDB_INVALID_PROCESS_ID;
+  struct timespec m_user_time {};
+  struct timespec m_system_time {};
+  struct timespec m_cumulative_user_time {};
+  struct timespec m_cumulative_system_time {};
 };
 
 typedef std::vector ProcessInstanceInfoList;
diff --git a/lldb/source/Host/linux/Host.cpp b/lldb/source/Host/linux/Host.cpp
index 6c57384aa38a13..c6490f2fc9e2f5 100644
--- a/lldb/source/Host/linux/Host.cpp
+++ b/lldb/source/Host/linux/Host.cpp
@@ -49,6 +49,29 @@ enum class ProcessState {
   TracedOrStopped,
   Zombie,
 };
+
+constexpr int task_comm_len = 16;
+
+struct StatFields {
+  ::pid_t pid = LLDB_INVALID_PROCESS_ID;
+  char comm[task_comm_len];
+  char state;
+  ::pid_t ppid = LLDB_INVALID_PROCESS_ID;
+  ::pid_t pgrp = LLDB_INVALID_PROCESS_ID;
+  ::pid_t session = LLDB_INVALID_PROCESS_ID;
+  int tty_nr;
+  int tpgid;
+  unsigned flags;
+  long unsigned minflt;
+  long unsigned cminflt;
+  long unsigned majflt;
+  long unsigned cmajflt;
+  long unsigned utime;
+  long unsigned stime;
+  long cutime;
+  long cstime;
+  //  other things. We don't need them below
+};
 }
 
 namespace lldb_private {
@@ -60,11 +83,92 @@ static bool GetStatusInfo(::pid_t Pid, ProcessInstanceInfo 
,
   ::pid_t ) {
   Log *log = GetLog(LLDBLog::Host);
 
-  auto BufferOrError = getProcFile(Pid, "status");
+  auto BufferOrError = getProcFile(Pid, "stat");
   if (!BufferOrError)
 return false;
 
   llvm::StringRef Rest = BufferOrError.get()->getBuffer();
+  if (Rest.empty())
+return false;
+  StatFields stat_fields;
+  if (sscanf(Rest.data(),
+ "%d %s %c %d %d %d %d %d %u %lu %lu %lu %lu 

[Lldb-commits] [lldb] Summarize std::string's when created from data. (PR #89110)

2024-04-17 Thread via lldb-commits

jimingham wrote:

Most of this change makes sense.  However, you are assuming that if the address 
type of a ValueObject is Host, its children are necessarily going to be in the 
target.  But there's actually an API that answers that question: 
ValueObject::GetAddressTypeOfChildren. You should consult that rather than 
assuming the children live in the target.

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


[Lldb-commits] [lldb] [lldb][DynamicLoader] Fix lldb unable to stop at _dl_debug_state if user set it before the process launched. (PR #88792)

2024-04-17 Thread via lldb-commits

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


[Lldb-commits] [lldb] [lldb][DynamicLoader] Fix lldb unable to stop at _dl_debug_state if user set it before the process launched. (PR #88792)

2024-04-17 Thread via lldb-commits


@@ -572,9 +572,14 @@ ModuleSP DynamicLoaderPOSIXDYLD::LoadInterpreterModule() {
   ModuleSpec module_spec(file, target.GetArchitecture());
 
   if (ModuleSP module_sp = target.GetOrCreateModule(module_spec,
-true /* notify */)) {
+false /* notify */)) {

jimingham wrote:

Can you put a comment in here saying why you pass false here.  The next person 
who comes to read this won't have the context of this discussion and will 
likely be confused.

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


[Lldb-commits] [lldb] adds additional information to the ProcessInfo object for elf processes (PR #88995)

2024-04-17 Thread via lldb-commits

jimingham wrote:

Yes, you'd have to put this test into a host test for a system that doesn't 
support this, like use the Posix one or make one for the Windows or Darwin 
Hosts.

Jim


> On Apr 17, 2024, at 10:43 AM, Fred Grim ***@***.***> wrote:
> 
> 
> That test looks good. It seems unlikely that you would buggily always produce 
> monotonically increasing times...
> 
> Not to be a pest, but since this info is only provided on Linux, we should 
> maybe add a test to make sure that asking for this information on systems 
> that don't provide it fails gracefully?
> 
> Actually looking at lldb/unittest/Host/CMakeLists.txt I see:
> 
> if (CMAKE_SYSTEM_NAME MATCHES "Linux|Android")
> list(APPEND FILES
>   linux/HostTest.cpp
>   linux/SupportTest.cpp
>  )
> endif()
> which should ensure the test is never built nor executed outside of linux
> 
> —
> Reply to this email directly, view it on GitHub 
> , or 
> unsubscribe 
> .
> You are receiving this because you are on a team that was mentioned.
> 



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


[Lldb-commits] [lldb] Summarize std::string's when created from data. (PR #89110)

2024-04-17 Thread via lldb-commits


@@ -254,13 +254,13 @@ bool 
lldb_private::formatters::LibStdcppStringSummaryProvider(
   } else
 addr_of_string =
 valobj.GetAddressOf(scalar_is_load_addr, _type);
-  if (addr_of_string != LLDB_INVALID_ADDRESS) {
+  if (addr_of_string != LLDB_INVALID_ADDRESS ||
+addr_type == eAddressTypeHost) {

kusmour wrote:

Do we need this since we iterate all cases anyway?

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


[Lldb-commits] [lldb] [lldb][MinidumpFileBuilder] Fix addition of MemoryList steam (PR #88564)

2024-04-17 Thread via lldb-commits


@@ -3857,8 +3857,8 @@ thread_result_t Process::RunPrivateStateThread(bool 
is_secondary_thread) {
 // case we should tell it to stop doing that.  Normally, we don't NEED
 // to do that because we will next close the communication to the stub
 // and that will get it to shut down.  But there are remote debugging
-// cases where relying on that side-effect causes the shutdown to be 
-// flakey, so we should send a positive signal to interrupt the wait. 
+// cases where relying on that side-effect causes the shutdown to be

jeffreytan81 wrote:

Undo these space changes. I believe there is a VSCode setting for this.

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


[Lldb-commits] [lldb] [lldb][MinidumpFileBuilder] Fix addition of MemoryList steam (PR #88564)

2024-04-17 Thread via lldb-commits


@@ -649,16 +651,25 @@ MinidumpFileBuilder::AddMemoryList(const lldb::ProcessSP 
_sp,
   DataBufferHeap helper_data;
   std::vector mem_descriptors;
   for (const auto _range : core_ranges) {
-// Skip empty memory regions or any regions with no permissions.
-if (core_range.range.empty() || core_range.lldb_permissions == 0)
+// Skip empty memory regions.
+if (core_range.range.empty())
   continue;
 const addr_t addr = core_range.range.start();
 const addr_t size = core_range.range.size();
 auto data_up = std::make_unique(size, 0);
 const size_t bytes_read =
 process_sp->ReadMemory(addr, data_up->GetBytes(), size, error);
-if (bytes_read == 0)
+if (error.Fail()) {
+  Log *log = GetLog(LLDBLog::SystemRuntime);

jeffreytan81 wrote:

I still think we should emit this to end users as public messages instead of as 
an internal log. I am ok to do this in future follow ups.

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


[Lldb-commits] [lldb] [lldb][MinidumpFileBuilder] Fix addition of MemoryList steam (PR #88564)

2024-04-17 Thread via lldb-commits


@@ -649,16 +651,25 @@ MinidumpFileBuilder::AddMemoryList(const lldb::ProcessSP 
_sp,
   DataBufferHeap helper_data;
   std::vector mem_descriptors;
   for (const auto _range : core_ranges) {
-// Skip empty memory regions or any regions with no permissions.
-if (core_range.range.empty() || core_range.lldb_permissions == 0)
+// Skip empty memory regions.
+if (core_range.range.empty())
   continue;
 const addr_t addr = core_range.range.start();
 const addr_t size = core_range.range.size();
 auto data_up = std::make_unique(size, 0);
 const size_t bytes_read =
 process_sp->ReadMemory(addr, data_up->GetBytes(), size, error);
-if (bytes_read == 0)
+if (error.Fail()) {
+  Log *log = GetLog(LLDBLog::SystemRuntime);

jeffreytan81 wrote:

Since this is part of ObjectFile plugin, let's use "LLDBLog::Object"

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


[Lldb-commits] [lldb] [lldb][MinidumpFileBuilder] Fix addition of MemoryList steam (PR #88564)

2024-04-17 Thread via lldb-commits


@@ -649,16 +651,25 @@ MinidumpFileBuilder::AddMemoryList(const lldb::ProcessSP 
_sp,
   DataBufferHeap helper_data;
   std::vector mem_descriptors;
   for (const auto _range : core_ranges) {
-// Skip empty memory regions or any regions with no permissions.
-if (core_range.range.empty() || core_range.lldb_permissions == 0)
+// Skip empty memory regions.
+if (core_range.range.empty())
   continue;
 const addr_t addr = core_range.range.start();
 const addr_t size = core_range.range.size();
 auto data_up = std::make_unique(size, 0);
 const size_t bytes_read =
 process_sp->ReadMemory(addr, data_up->GetBytes(), size, error);
-if (bytes_read == 0)
+if (error.Fail()) {
+  Log *log = GetLog(LLDBLog::SystemRuntime);
+  LLDB_LOGF(log, "Failed to read memory region. Bytes read: %zu, error: 
%s",
+bytes_read, error.AsCString());
+  error.Clear();
+}
+
+if (bytes_read == 0) {

jeffreytan81 wrote:

Nit: undo, llvm/lldb guideline does not quoting single line block.

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


[Lldb-commits] [lldb] Fix saving minidump from lldb-dap (PR #89113)

2024-04-17 Thread via lldb-commits

https://github.com/jeffreytan81 updated 
https://github.com/llvm/llvm-project/pull/89113

>From 6cccde22723157260e7c0b19bf8372aae8d1afaa Mon Sep 17 00:00:00 2001
From: jeffreytan81 
Date: Wed, 6 Mar 2024 12:07:03 -0800
Subject: [PATCH 1/3] Fix strcmp build error on buildbot

---
 lldb/test/API/functionalities/fork/concurrent_vfork/main.cpp | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lldb/test/API/functionalities/fork/concurrent_vfork/main.cpp 
b/lldb/test/API/functionalities/fork/concurrent_vfork/main.cpp
index b0a4446ba01581..40cb63755ee8a5 100644
--- a/lldb/test/API/functionalities/fork/concurrent_vfork/main.cpp
+++ b/lldb/test/API/functionalities/fork/concurrent_vfork/main.cpp
@@ -1,6 +1,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 

>From 4453a9cb876fe4ed3c5a3ea57a03a428c5d847fc Mon Sep 17 00:00:00 2001
From: jeffreytan81 
Date: Wed, 17 Apr 2024 10:59:51 -0700
Subject: [PATCH 2/3] Fix saving minidump from lldb-dap

---
 .../Minidump/MinidumpFileBuilder.cpp  | 20 -
 .../Minidump/ObjectFileMinidump.cpp   | 18 -
 .../API/tools/lldb-dap/save-core/Makefile |  5 ++
 .../lldb-dap/save-core/TestDAP_save_core.py   | 81 +++
 .../API/tools/lldb-dap/save-core/main.cpp |  8 ++
 5 files changed, 125 insertions(+), 7 deletions(-)
 create mode 100644 lldb/test/API/tools/lldb-dap/save-core/Makefile
 create mode 100644 lldb/test/API/tools/lldb-dap/save-core/TestDAP_save_core.py
 create mode 100644 lldb/test/API/tools/lldb-dap/save-core/main.cpp

diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp 
b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
index 50d1b563f469cf..cefd4cb22b6bae 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
+++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
@@ -20,6 +20,7 @@
 #include "lldb/Target/StopInfo.h"
 #include "lldb/Target/ThreadList.h"
 #include "lldb/Utility/DataExtractor.h"
+#include "lldb/Utility/LLDBLog.h"
 #include "lldb/Utility/RegisterValue.h"
 
 #include "llvm/ADT/StringRef.h"
@@ -154,8 +155,16 @@ Status WriteString(const std::string _write,
 
 llvm::Expected getModuleFileSize(Target ,
const ModuleSP ) {
-  SectionSP sect_sp = mod->GetObjectFile()->GetBaseAddress().GetSection();
+  // JIT module has the same vm and file size.
   uint64_t SizeOfImage = 0;
+  if (mod->GetObjectFile()->CalculateType() == ObjectFile::Type::eTypeJIT) {
+for (const auto  : *mod->GetObjectFile()->GetSectionList()) {
+  SizeOfImage += section->GetByteSize();
+}
+return SizeOfImage;
+  }
+
+  SectionSP sect_sp = mod->GetObjectFile()->GetBaseAddress().GetSection();
 
   if (!sect_sp) {
 return llvm::createStringError(std::errc::operation_not_supported,
@@ -226,8 +235,13 @@ Status MinidumpFileBuilder::AddModuleList(Target ) {
 std::string module_name = mod->GetSpecificationDescription();
 auto maybe_mod_size = getModuleFileSize(target, mod);
 if (!maybe_mod_size) {
-  error.SetErrorStringWithFormat("Unable to get the size of module %s.",
- module_name.c_str());
+  llvm::Error mod_size_err = maybe_mod_size.takeError();
+  llvm::handleAllErrors(std::move(mod_size_err),
+[&](const llvm::ErrorInfoBase ) {
+  error.SetErrorStringWithFormat(
+  "Unable to get the size of module %s: %s.",
+  module_name.c_str(), E.message().c_str());
+});
   return error;
 }
 
diff --git a/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp 
b/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp
index fe609c7f3d2001..1af5d99f0b1604 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp
+++ b/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp
@@ -14,6 +14,7 @@
 #include "lldb/Core/PluginManager.h"
 #include "lldb/Core/Section.h"
 #include "lldb/Target/Process.h"
+#include "lldb/Utility/LLDBLog.h"
 
 #include "llvm/Support/FileSystem.h"
 
@@ -68,26 +69,35 @@ bool ObjectFileMinidump::SaveCore(const lldb::ProcessSP 
_sp,
 
   Target  = process_sp->GetTarget();
 
+  Log *log = GetLog(LLDBLog::Object);
   error = builder.AddSystemInfo(target.GetArchitecture().GetTriple());
-  if (error.Fail())
+  if (error.Fail()) {
+LLDB_LOG(log, "AddSystemInfo failed: %s", error.AsCString());
 return false;
+  }
 
   error = builder.AddModuleList(target);
-  if (error.Fail())
+  if (error.Fail()) {
+LLDB_LOG(log, "AddModuleList failed: %s", error.AsCString());
 return false;
+  }
 
   builder.AddMiscInfo(process_sp);
 
   error = builder.AddThreadList(process_sp);
-  if (error.Fail())
+  if (error.Fail()) {
+LLDB_LOG(log, "AddThreadList failed: %s", error.AsCString());
 return false;
+  }
 
   // Add any exceptions but 

[Lldb-commits] [lldb] Fix saving minidump from lldb-dap (PR #89113)

2024-04-17 Thread via lldb-commits

github-actions[bot] wrote:




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



You can test this locally with the following command:


``bash
darker --check --diff -r 
93f9fb2c825dba48db64d5f726b54bcbd4766009...4453a9cb876fe4ed3c5a3ea57a03a428c5d847fc
 lldb/test/API/tools/lldb-dap/save-core/TestDAP_save_core.py
``





View the diff from darker here.


``diff
--- TestDAP_save_core.py2024-04-17 17:59:51.00 +
+++ TestDAP_save_core.py2024-04-17 18:13:59.115018 +
@@ -27,55 +27,54 @@
 breakpoint_ids = self.set_source_breakpoints(source, lines)
 self.assertEqual(
 len(breakpoint_ids), len(lines), "expect correct number of 
breakpoints"
 )
 self.continue_to_breakpoints(breakpoint_ids)
-
+
 # Getting dap stack trace may trigger __lldb_caller_function JIT 
module to be created.
 self.get_stackFrames(startFrame=0)
-
+
 # Evaluating an expression that cause "_$__lldb_valid_pointer_check" 
JIT module to be created.
 expression = 'printf("this is a test")'
 self.dap_server.request_evaluate(expression, context="watch")
-
+
 # Verify "_$__lldb_valid_pointer_check" JIT module is created.
 modules = self.dap_server.get_modules()
 self.assertTrue(modules["_$__lldb_valid_pointer_check"])
 thread_count = len(self.dap_server.get_threads())
-
+
 core_stack = self.getBuildArtifact("core.stack.dmp")
 core_dirty = self.getBuildArtifact("core.dirty.dmp")
 core_full = self.getBuildArtifact("core.full.dmp")
-
+
 base_command = "`process save-core --plugin-name=minidump "
-self.dap_server.request_evaluate(base_command + " --style=stack '%s'" 
% (core_stack), context="repl")
-
-self.assertTrue(os.path.isfile(core_stack))
-self.verify_core_file(
-core_stack, len(modules), thread_count
+self.dap_server.request_evaluate(
+base_command + " --style=stack '%s'" % (core_stack), context="repl"
 )
 
-self.dap_server.request_evaluate(base_command + " 
--style=modified-memory '%s'" % (core_dirty), context="repl")
+self.assertTrue(os.path.isfile(core_stack))
+self.verify_core_file(core_stack, len(modules), thread_count)
+
+self.dap_server.request_evaluate(
+base_command + " --style=modified-memory '%s'" % (core_dirty),
+context="repl",
+)
 self.assertTrue(os.path.isfile(core_dirty))
-self.verify_core_file(
-core_dirty, len(modules), thread_count
+self.verify_core_file(core_dirty, len(modules), thread_count)
+
+self.dap_server.request_evaluate(
+base_command + " --style=full '%s'" % (core_full), context="repl"
 )
+self.assertTrue(os.path.isfile(core_full))
+self.verify_core_file(core_full, len(modules), thread_count)
 
-self.dap_server.request_evaluate(base_command + " --style=full '%s'" % 
(core_full), context="repl")
-self.assertTrue(os.path.isfile(core_full))
-self.verify_core_file(
-core_full, len(modules), thread_count
-)
+def verify_core_file(self, core_path, expected_module_count, 
expected_thread_count):
+# To verify, we'll launch with the mini dump
+target = self.dbg.CreateTarget(None)
+process = target.LoadCore(core_path)
 
-def verify_core_file(
-  self, core_path, expected_module_count, expected_thread_count
-  ):
-  # To verify, we'll launch with the mini dump
-  target = self.dbg.CreateTarget(None)
-  process = target.LoadCore(core_path)
-
-  # check if the core is in desired state
-  self.assertTrue(process, PROCESS_IS_VALID)
-  self.assertTrue(process.GetProcessInfo().IsValid())
-  self.assertNotEqual(target.GetTriple().find("linux"), -1)
-  self.assertTrue(target.GetNumModules(), expected_module_count)
-  self.assertEqual(process.GetNumThreads(), expected_thread_count)
+# check if the core is in desired state
+self.assertTrue(process, PROCESS_IS_VALID)
+self.assertTrue(process.GetProcessInfo().IsValid())
+self.assertNotEqual(target.GetTriple().find("linux"), -1)
+self.assertTrue(target.GetNumModules(), expected_module_count)
+self.assertEqual(process.GetNumThreads(), expected_thread_count)

``




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


[Lldb-commits] [lldb] Fix saving minidump from lldb-dap (PR #89113)

2024-04-17 Thread via lldb-commits

llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: None (jeffreytan81)


Changes

There are users reporting saving minidump from lldb-dap does not work.

Turns out our stack trace request always evaluate a function call which caused 
JIT object file like "__lldb_caller_function" to be created which can fail 
minidump builder to get its module size.

This patch fixes "getModuleFileSize" for ObjectFileJIT so that module list can 
be saved. I decided to create a lldb-dap test so that this end-to-end 
functionality can be validated from our tests (instead of only command line 
lldb).

The patch also improves several other small things in the workflow:
1. It logs any minidump stream failure so that it is easier to find out what 
stream saving fails. In future, we should show process and error to end users.
2. It handles error from "getModuleFileSize" llvm::ExpectedT otherwise 
it will complain the error is not handled.

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


5 Files Affected:

- (modified) lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp 
(+17-3) 
- (modified) lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp 
(+14-4) 
- (added) lldb/test/API/tools/lldb-dap/save-core/Makefile (+5) 
- (added) lldb/test/API/tools/lldb-dap/save-core/TestDAP_save_core.py (+81) 
- (added) lldb/test/API/tools/lldb-dap/save-core/main.cpp (+8) 


``diff
diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp 
b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
index 50d1b563f469cf..cefd4cb22b6bae 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
+++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
@@ -20,6 +20,7 @@
 #include "lldb/Target/StopInfo.h"
 #include "lldb/Target/ThreadList.h"
 #include "lldb/Utility/DataExtractor.h"
+#include "lldb/Utility/LLDBLog.h"
 #include "lldb/Utility/RegisterValue.h"
 
 #include "llvm/ADT/StringRef.h"
@@ -154,8 +155,16 @@ Status WriteString(const std::string _write,
 
 llvm::Expected getModuleFileSize(Target ,
const ModuleSP ) {
-  SectionSP sect_sp = mod->GetObjectFile()->GetBaseAddress().GetSection();
+  // JIT module has the same vm and file size.
   uint64_t SizeOfImage = 0;
+  if (mod->GetObjectFile()->CalculateType() == ObjectFile::Type::eTypeJIT) {
+for (const auto  : *mod->GetObjectFile()->GetSectionList()) {
+  SizeOfImage += section->GetByteSize();
+}
+return SizeOfImage;
+  }
+
+  SectionSP sect_sp = mod->GetObjectFile()->GetBaseAddress().GetSection();
 
   if (!sect_sp) {
 return llvm::createStringError(std::errc::operation_not_supported,
@@ -226,8 +235,13 @@ Status MinidumpFileBuilder::AddModuleList(Target ) {
 std::string module_name = mod->GetSpecificationDescription();
 auto maybe_mod_size = getModuleFileSize(target, mod);
 if (!maybe_mod_size) {
-  error.SetErrorStringWithFormat("Unable to get the size of module %s.",
- module_name.c_str());
+  llvm::Error mod_size_err = maybe_mod_size.takeError();
+  llvm::handleAllErrors(std::move(mod_size_err),
+[&](const llvm::ErrorInfoBase ) {
+  error.SetErrorStringWithFormat(
+  "Unable to get the size of module %s: %s.",
+  module_name.c_str(), E.message().c_str());
+});
   return error;
 }
 
diff --git a/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp 
b/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp
index fe609c7f3d2001..1af5d99f0b1604 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp
+++ b/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp
@@ -14,6 +14,7 @@
 #include "lldb/Core/PluginManager.h"
 #include "lldb/Core/Section.h"
 #include "lldb/Target/Process.h"
+#include "lldb/Utility/LLDBLog.h"
 
 #include "llvm/Support/FileSystem.h"
 
@@ -68,26 +69,35 @@ bool ObjectFileMinidump::SaveCore(const lldb::ProcessSP 
_sp,
 
   Target  = process_sp->GetTarget();
 
+  Log *log = GetLog(LLDBLog::Object);
   error = builder.AddSystemInfo(target.GetArchitecture().GetTriple());
-  if (error.Fail())
+  if (error.Fail()) {
+LLDB_LOG(log, "AddSystemInfo failed: %s", error.AsCString());
 return false;
+  }
 
   error = builder.AddModuleList(target);
-  if (error.Fail())
+  if (error.Fail()) {
+LLDB_LOG(log, "AddModuleList failed: %s", error.AsCString());
 return false;
+  }
 
   builder.AddMiscInfo(process_sp);
 
   error = builder.AddThreadList(process_sp);
-  if (error.Fail())
+  if (error.Fail()) {
+LLDB_LOG(log, "AddThreadList failed: %s", error.AsCString());
 return false;
+  }
 
   // Add any exceptions but only if there are any in any threads.
   builder.AddExceptions(process_sp);
 
   error = builder.AddMemoryList(process_sp, 

[Lldb-commits] [lldb] Fix saving minidump from lldb-dap (PR #89113)

2024-04-17 Thread via lldb-commits

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


[Lldb-commits] [lldb] Fix saving minidump from lldb-dap (PR #89113)

2024-04-17 Thread via lldb-commits

https://github.com/jeffreytan81 created 
https://github.com/llvm/llvm-project/pull/89113

There are users reporting saving minidump from lldb-dap does not work.

Turns out our stack trace request always evaluate a function call which caused 
JIT object file like "__lldb_caller_function" to be created which can fail 
minidump builder to get its module size.

This patch fixes "getModuleFileSize" for ObjectFileJIT so that module list can 
be saved. I decided to create a lldb-dap test so that this end-to-end 
functionality can be validated from our tests (instead of only command line 
lldb).

The patch also improves several other small things in the workflow:
1. It logs any minidump stream failure so that it is easier to find out what 
stream saving fails. In future, we should show process and error to end users.
2. It handles error from "getModuleFileSize" llvm::Expected otherwise it 
will complain the error is not handled.

>From 6cccde22723157260e7c0b19bf8372aae8d1afaa Mon Sep 17 00:00:00 2001
From: jeffreytan81 
Date: Wed, 6 Mar 2024 12:07:03 -0800
Subject: [PATCH 1/2] Fix strcmp build error on buildbot

---
 lldb/test/API/functionalities/fork/concurrent_vfork/main.cpp | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lldb/test/API/functionalities/fork/concurrent_vfork/main.cpp 
b/lldb/test/API/functionalities/fork/concurrent_vfork/main.cpp
index b0a4446ba01581..40cb63755ee8a5 100644
--- a/lldb/test/API/functionalities/fork/concurrent_vfork/main.cpp
+++ b/lldb/test/API/functionalities/fork/concurrent_vfork/main.cpp
@@ -1,6 +1,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 

>From 4453a9cb876fe4ed3c5a3ea57a03a428c5d847fc Mon Sep 17 00:00:00 2001
From: jeffreytan81 
Date: Wed, 17 Apr 2024 10:59:51 -0700
Subject: [PATCH 2/2] Fix saving minidump from lldb-dap

---
 .../Minidump/MinidumpFileBuilder.cpp  | 20 -
 .../Minidump/ObjectFileMinidump.cpp   | 18 -
 .../API/tools/lldb-dap/save-core/Makefile |  5 ++
 .../lldb-dap/save-core/TestDAP_save_core.py   | 81 +++
 .../API/tools/lldb-dap/save-core/main.cpp |  8 ++
 5 files changed, 125 insertions(+), 7 deletions(-)
 create mode 100644 lldb/test/API/tools/lldb-dap/save-core/Makefile
 create mode 100644 lldb/test/API/tools/lldb-dap/save-core/TestDAP_save_core.py
 create mode 100644 lldb/test/API/tools/lldb-dap/save-core/main.cpp

diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp 
b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
index 50d1b563f469cf..cefd4cb22b6bae 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
+++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
@@ -20,6 +20,7 @@
 #include "lldb/Target/StopInfo.h"
 #include "lldb/Target/ThreadList.h"
 #include "lldb/Utility/DataExtractor.h"
+#include "lldb/Utility/LLDBLog.h"
 #include "lldb/Utility/RegisterValue.h"
 
 #include "llvm/ADT/StringRef.h"
@@ -154,8 +155,16 @@ Status WriteString(const std::string _write,
 
 llvm::Expected getModuleFileSize(Target ,
const ModuleSP ) {
-  SectionSP sect_sp = mod->GetObjectFile()->GetBaseAddress().GetSection();
+  // JIT module has the same vm and file size.
   uint64_t SizeOfImage = 0;
+  if (mod->GetObjectFile()->CalculateType() == ObjectFile::Type::eTypeJIT) {
+for (const auto  : *mod->GetObjectFile()->GetSectionList()) {
+  SizeOfImage += section->GetByteSize();
+}
+return SizeOfImage;
+  }
+
+  SectionSP sect_sp = mod->GetObjectFile()->GetBaseAddress().GetSection();
 
   if (!sect_sp) {
 return llvm::createStringError(std::errc::operation_not_supported,
@@ -226,8 +235,13 @@ Status MinidumpFileBuilder::AddModuleList(Target ) {
 std::string module_name = mod->GetSpecificationDescription();
 auto maybe_mod_size = getModuleFileSize(target, mod);
 if (!maybe_mod_size) {
-  error.SetErrorStringWithFormat("Unable to get the size of module %s.",
- module_name.c_str());
+  llvm::Error mod_size_err = maybe_mod_size.takeError();
+  llvm::handleAllErrors(std::move(mod_size_err),
+[&](const llvm::ErrorInfoBase ) {
+  error.SetErrorStringWithFormat(
+  "Unable to get the size of module %s: %s.",
+  module_name.c_str(), E.message().c_str());
+});
   return error;
 }
 
diff --git a/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp 
b/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp
index fe609c7f3d2001..1af5d99f0b1604 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp
+++ b/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp
@@ -14,6 +14,7 @@
 #include "lldb/Core/PluginManager.h"
 #include "lldb/Core/Section.h"
 #include "lldb/Target/Process.h"
+#include 

[Lldb-commits] [lldb] [lldb][DynamicLoader] Fix lldb unable to stop at _dl_debug_state if user set it before the process launched. (PR #88792)

2024-04-17 Thread Pavel Labath via lldb-commits

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


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


[Lldb-commits] [lldb] [lldb][DynamicLoader] Fix lldb unable to stop at _dl_debug_state if user set it before the process launched. (PR #88792)

2024-04-17 Thread Pavel Labath via lldb-commits


@@ -572,9 +572,14 @@ ModuleSP DynamicLoaderPOSIXDYLD::LoadInterpreterModule() {
   ModuleSpec module_spec(file, target.GetArchitecture());
 
   if (ModuleSP module_sp = target.GetOrCreateModule(module_spec,
-true /* notify */)) {
+false /* notify */)) {

labath wrote:

Might as well change to the [official 
style](https://llvm.org/docs/CodingStandards.html#comment-formatting): 
`/*notify=*/false`

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


[Lldb-commits] [lldb] [lldb] Correct documentation of LLDB_TEST_USER_ARGS (PR #89042)

2024-04-17 Thread via lldb-commits

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

LGTM, David. One of my guys is working on RISC-V tests, and wanted to use 
LLDB_TEST_USER_ARGS, but as you said it doesn't work as documented. This will 
help a lot!

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


[Lldb-commits] [lldb] adds additional information to the ProcessInfo object for elf processes (PR #88995)

2024-04-17 Thread Fred Grim via lldb-commits

feg208 wrote:

> It should never decrease, however. If you were just getting garbage values, 
> then there should be a roughly even chance the second value will be less than 
> the first. So this still gives some confidence this isn't totally bogus...

For sure. My concern is that it wouldn't increase either. The granularity is 
microseconds but with a call like getpid() for example the kernel might just 
return a cached value which, I assume after enough loops, would increase the 
timer but what is enough? And enough will change I think as build environments 
get faster machines faster kernels and so on.


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


[Lldb-commits] [lldb] Summarize std::string's when created from data. (PR #89110)

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

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


[Lldb-commits] [lldb] Summarize std::string's when created from data. (PR #89110)

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

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


[Lldb-commits] [lldb] adds additional information to the ProcessInfo object for elf processes (PR #88995)

2024-04-17 Thread Fred Grim via lldb-commits

feg208 wrote:

> That test looks good. It seems unlikely that you would buggily always produce 
> monotonically increasing times...
> 
> Not to be a pest, but since this info is only provided on Linux, we should 
> maybe add a test to make sure that asking for this information on systems 
> that don't provide it fails gracefully?

Actually looking at lldb/unittest/Host/CMakeLists.txt I see:
```
if (CMAKE_SYSTEM_NAME MATCHES "Linux|Android")
list(APPEND FILES
  linux/HostTest.cpp
  linux/SupportTest.cpp
 )
endif()
``` 

which should ensure the test is never built nor executed outside of linux

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


[Lldb-commits] [lldb] Summarize std::string's when created from data. (PR #89110)

2024-04-17 Thread via lldb-commits

llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: Jacob Lalonde (Jlalond)


Changes

std::string children are normally formatted as their summary [my_string_prop] = 
"hello world". Sbvalues/ValueObjects created from data do not follow the same 
summary formatter path and instead hit the value object printer. This change 
fixes that and adds tests for the future.

Added tests in TestDataFormatterStdString.py and a formatter for a custom 
struct.

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


4 Files Affected:

- (modified) lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp (+27-4) 
- (added) 
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/ConvertToDataFormatter.py
 (+50) 
- (modified) 
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/TestDataFormatterStdString.py
 (+28) 
- (modified) 
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/main.cpp
 (+13) 


``diff
diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp 
b/lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp
index 86bb575af5ca34..0da01e9ca07fb6 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp
@@ -254,13 +254,13 @@ bool 
lldb_private::formatters::LibStdcppStringSummaryProvider(
   } else
 addr_of_string =
 valobj.GetAddressOf(scalar_is_load_addr, _type);
-  if (addr_of_string != LLDB_INVALID_ADDRESS) {
+  if (addr_of_string != LLDB_INVALID_ADDRESS ||
+addr_type == eAddressTypeHost) {
 switch (addr_type) {
 case eAddressTypeLoad: {
   ProcessSP process_sp(valobj.GetProcessSP());
   if (!process_sp)
 return false;
-
   StringPrinter::ReadStringAndDumpToStreamOptions options(valobj);
   Status error;
   lldb::addr_t addr_of_data =
@@ -287,8 +287,31 @@ bool 
lldb_private::formatters::LibStdcppStringSummaryProvider(
   } else
 return true;
 } break;
-case eAddressTypeHost:
-  break;
+case eAddressTypeHost: {
+  // We have the host address of our std::string
+  // But we need to read the pointee data from the debugged process.
+  ProcessSP process_sp(valobj.GetProcessSP());
+  DataExtractor data;
+  Status error;
+  valobj.GetData(data, error);
+  if (error.Fail())
+return false;
+  // We want to read the address from std::string, which is the first 8 
bytes.
+  lldb::offset_t offset = 0;
+  lldb::addr_t addr = data.GetAddress();
+  if (!addr)
+  {
+stream.Printf("nullptr");
+return true;
+  }
+
+  std::string contents;
+  process_sp->ReadCStringFromMemory(addr, contents, error);
+  if (error.Fail())
+return false;
+  stream.Printf("%s", contents.c_str());
+  return true;
+} break;
 case eAddressTypeInvalid:
 case eAddressTypeFile:
   break;
diff --git 
a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/ConvertToDataFormatter.py
 
b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/ConvertToDataFormatter.py
new file mode 100644
index 00..57e42c920f8294
--- /dev/null
+++ 
b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/ConvertToDataFormatter.py
@@ -0,0 +1,50 @@
+# coding=utf8
+"""
+Helper formmater to verify Std::String by created via SBData
+"""
+
+import lldb
+
+class SyntheticFormatter:
+def __init__(self, valobj, dict):
+self.valobj = valobj
+
+def num_children(self):
+return 6
+
+def has_children(self):
+return True
+
+def get_child_at_index(self, index):
+name = None
+match index:
+case 0:
+name = "short_string"
+case 1:
+name = "long_string"
+case 2:
+name = "short_string_ptr"
+case 3:
+name = "long_string_ptr"
+case 4:
+name = "short_string_ref"
+case 5:
+name = "long_string_ref"
+case _:
+return None
+
+child = self.valobj.GetChildMemberWithName(name)
+valType = child.GetType()
+return self.valobj.CreateValueFromData(name,
+child.GetData(),
+valType)
+
+
+def __lldb_init_module(debugger, dict):
+typeName = "string_container"
+debugger.HandleCommand(
+'type synthetic add -x "'
++ typeName
++ '" --python-class '
++ f"{__name__}.SyntheticFormatter"
+)
diff --git 
a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/TestDataFormatterStdString.py
 
b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/TestDataFormatterStdString.py
index 0b78fb7dc3911c..2adfe47f9a3cfd 100644
--- 

[Lldb-commits] [lldb] Summarize std::string's when created from data. (PR #89110)

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

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


[Lldb-commits] [lldb] Summarize std::sting's when created from data. (PR #89110)

2024-04-17 Thread via lldb-commits

github-actions[bot] wrote:



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

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

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

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

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

If you have further questions, they may be answered by the [LLVM GitHub User 
Guide](https://llvm.org/docs/GitHub.html).

You can also ask questions in a comment on this PR, on the [LLVM 
Discord](https://discord.com/invite/xS7Z362) or on the 
[forums](https://discourse.llvm.org/).

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


[Lldb-commits] [lldb] Summarize std::sting's when created from data. (PR #89110)

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

https://github.com/Jlalond created 
https://github.com/llvm/llvm-project/pull/89110

std::string children are normally formatted as their summary [my_string_prop] = 
"hello world". Sbvalues/ValueObjects created from data do not follow the same 
summary formatter path and instead hit the value object printer. This change 
fixes that and adds tests for the future.

Added tests in TestDataFormatterStdString.py and a formatter for a custom 
struct.

>From e0316188d22605c670079e37855d3d8b5c944cee Mon Sep 17 00:00:00 2001
From: Jacob John Lalonde 
Date: Wed, 10 Apr 2024 14:33:40 -0700
Subject: [PATCH] Fix bug where an sbvalue containing a std::string created
 from data would not use the built in summary provider, but default to the
 valueobjectprinter

std::string children are normally formatted as their summary [my_string_prop] = 
"hello world".
Sbvalues created from data do not follow the same summary formatter path and 
instead hit the value object printer.
This change fixes that and adds tests for the future.

Added tests in TestDataFormatterStdString.py and a formatter for a custom 
struct.
---
 .../Plugins/Language/CPlusPlus/LibStdcpp.cpp  | 31 ++--
 .../string/ConvertToDataFormatter.py  | 50 +++
 .../string/TestDataFormatterStdString.py  | 28 +++
 .../libstdcpp/string/main.cpp | 13 +
 4 files changed, 118 insertions(+), 4 deletions(-)
 create mode 100644 
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/ConvertToDataFormatter.py

diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp 
b/lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp
index 86bb575af5ca34..0da01e9ca07fb6 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp
@@ -254,13 +254,13 @@ bool 
lldb_private::formatters::LibStdcppStringSummaryProvider(
   } else
 addr_of_string =
 valobj.GetAddressOf(scalar_is_load_addr, _type);
-  if (addr_of_string != LLDB_INVALID_ADDRESS) {
+  if (addr_of_string != LLDB_INVALID_ADDRESS ||
+addr_type == eAddressTypeHost) {
 switch (addr_type) {
 case eAddressTypeLoad: {
   ProcessSP process_sp(valobj.GetProcessSP());
   if (!process_sp)
 return false;
-
   StringPrinter::ReadStringAndDumpToStreamOptions options(valobj);
   Status error;
   lldb::addr_t addr_of_data =
@@ -287,8 +287,31 @@ bool 
lldb_private::formatters::LibStdcppStringSummaryProvider(
   } else
 return true;
 } break;
-case eAddressTypeHost:
-  break;
+case eAddressTypeHost: {
+  // We have the host address of our std::string
+  // But we need to read the pointee data from the debugged process.
+  ProcessSP process_sp(valobj.GetProcessSP());
+  DataExtractor data;
+  Status error;
+  valobj.GetData(data, error);
+  if (error.Fail())
+return false;
+  // We want to read the address from std::string, which is the first 8 
bytes.
+  lldb::offset_t offset = 0;
+  lldb::addr_t addr = data.GetAddress();
+  if (!addr)
+  {
+stream.Printf("nullptr");
+return true;
+  }
+
+  std::string contents;
+  process_sp->ReadCStringFromMemory(addr, contents, error);
+  if (error.Fail())
+return false;
+  stream.Printf("%s", contents.c_str());
+  return true;
+} break;
 case eAddressTypeInvalid:
 case eAddressTypeFile:
   break;
diff --git 
a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/ConvertToDataFormatter.py
 
b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/ConvertToDataFormatter.py
new file mode 100644
index 00..57e42c920f8294
--- /dev/null
+++ 
b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/ConvertToDataFormatter.py
@@ -0,0 +1,50 @@
+# coding=utf8
+"""
+Helper formmater to verify Std::String by created via SBData
+"""
+
+import lldb
+
+class SyntheticFormatter:
+def __init__(self, valobj, dict):
+self.valobj = valobj
+
+def num_children(self):
+return 6
+
+def has_children(self):
+return True
+
+def get_child_at_index(self, index):
+name = None
+match index:
+case 0:
+name = "short_string"
+case 1:
+name = "long_string"
+case 2:
+name = "short_string_ptr"
+case 3:
+name = "long_string_ptr"
+case 4:
+name = "short_string_ref"
+case 5:
+name = "long_string_ref"
+case _:
+return None
+
+child = self.valobj.GetChildMemberWithName(name)
+valType = child.GetType()
+return self.valobj.CreateValueFromData(name,
+child.GetData(),
+valType)
+
+
+def 

[Lldb-commits] [lldb] adds additional information to the ProcessInfo object for elf processes (PR #88995)

2024-04-17 Thread Fred Grim via lldb-commits

feg208 wrote:

> That test looks good. It seems unlikely that you would buggily always produce 
> monotonically increasing times...
> 
> Not to be a pest, but since this info is only provided on Linux, we should 
> maybe add a test to make sure that asking for this information on systems 
> that don't provide it fails gracefully?

Not at all. I'll add it

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


[Lldb-commits] [lldb] adds additional information to the ProcessInfo object for elf processes (PR #88995)

2024-04-17 Thread Fred Grim via lldb-commits

https://github.com/feg208 updated 
https://github.com/llvm/llvm-project/pull/88995

>From 9b8ec4d0c31ad1b228add56bc27cd79457e515c7 Mon Sep 17 00:00:00 2001
From: Fred Grim 
Date: Tue, 16 Apr 2024 14:46:37 -0700
Subject: [PATCH 1/2] adds additional information to the ProcessInfo object for
 elf processes

---
 lldb/include/lldb/Utility/ProcessInfo.h |  71 ++
 lldb/source/Host/linux/Host.cpp | 125 
 lldb/unittests/Host/linux/HostTest.cpp  |   6 ++
 3 files changed, 182 insertions(+), 20 deletions(-)

diff --git a/lldb/include/lldb/Utility/ProcessInfo.h 
b/lldb/include/lldb/Utility/ProcessInfo.h
index 7fb5b37be0f48f..e9fe71e1b851d1 100644
--- a/lldb/include/lldb/Utility/ProcessInfo.h
+++ b/lldb/include/lldb/Utility/ProcessInfo.h
@@ -139,6 +139,11 @@ class ProcessInfo {
 // to that process.
 class ProcessInstanceInfo : public ProcessInfo {
 public:
+  struct timespec {
+time_t tv_sec = 0;
+long int tv_usec = 0;
+  };
+
   ProcessInstanceInfo() = default;
 
   ProcessInstanceInfo(const char *name, const ArchSpec , lldb::pid_t pid)
@@ -172,6 +177,66 @@ class ProcessInstanceInfo : public ProcessInfo {
 return m_parent_pid != LLDB_INVALID_PROCESS_ID;
   }
 
+  lldb::pid_t GetProcessGroupID() const { return m_process_group_id; }
+
+  void SetProcessGroupID(lldb::pid_t pgrp) { m_process_group_id = pgrp; }
+
+  bool ProcessGroupIDIsValid() const {
+return m_process_group_id != LLDB_INVALID_PROCESS_ID;
+  }
+
+  lldb::pid_t GetProcessSessionID() const { return m_process_session_id; }
+
+  void SetProcessSessionID(lldb::pid_t session) {
+m_process_session_id = session;
+  }
+
+  bool ProcessSessionIDIsValid() const {
+return m_process_session_id != LLDB_INVALID_PROCESS_ID;
+  }
+
+  struct timespec GetUserTime() const { return m_user_time; }
+
+  void SetUserTime(struct timespec utime) { m_user_time = utime; }
+
+  bool UserTimeIsValid() const {
+return m_user_time.tv_sec > 0 || m_user_time.tv_usec > 0;
+  }
+
+  struct timespec GetSystemTime() const { return m_system_time; }
+
+  void SetSystemTime(struct timespec stime) { m_system_time = stime; }
+
+  bool SystemTimeIsValid() const {
+return m_system_time.tv_sec > 0 || m_system_time.tv_usec > 0;
+  }
+
+  struct timespec GetCumulativeUserTime() const {
+return m_cumulative_user_time;
+  }
+
+  void SetCumulativeUserTime(struct timespec cutime) {
+m_cumulative_user_time = cutime;
+  }
+
+  bool CumulativeUserTimeIsValid() const {
+return m_cumulative_user_time.tv_sec > 0 ||
+   m_cumulative_user_time.tv_usec > 0;
+  }
+
+  struct timespec GetCumulativeSystemTime() const {
+return m_cumulative_system_time;
+  }
+
+  void SetCumulativeSystemTime(struct timespec cstime) {
+m_cumulative_system_time = cstime;
+  }
+
+  bool CumulativeSystemTimeIsValid() const {
+return m_cumulative_system_time.tv_sec > 0 ||
+   m_cumulative_system_time.tv_sec > 0;
+  }
+
   void Dump(Stream , UserIDResolver ) const;
 
   static void DumpTableHeader(Stream , bool show_args, bool verbose);
@@ -183,6 +248,12 @@ class ProcessInstanceInfo : public ProcessInfo {
   uint32_t m_euid = UINT32_MAX;
   uint32_t m_egid = UINT32_MAX;
   lldb::pid_t m_parent_pid = LLDB_INVALID_PROCESS_ID;
+  lldb::pid_t m_process_group_id = LLDB_INVALID_PROCESS_ID;
+  lldb::pid_t m_process_session_id = LLDB_INVALID_PROCESS_ID;
+  struct timespec m_user_time {};
+  struct timespec m_system_time {};
+  struct timespec m_cumulative_user_time {};
+  struct timespec m_cumulative_system_time {};
 };
 
 typedef std::vector ProcessInstanceInfoList;
diff --git a/lldb/source/Host/linux/Host.cpp b/lldb/source/Host/linux/Host.cpp
index 6c57384aa38a13..c6490f2fc9e2f5 100644
--- a/lldb/source/Host/linux/Host.cpp
+++ b/lldb/source/Host/linux/Host.cpp
@@ -49,6 +49,29 @@ enum class ProcessState {
   TracedOrStopped,
   Zombie,
 };
+
+constexpr int task_comm_len = 16;
+
+struct StatFields {
+  ::pid_t pid = LLDB_INVALID_PROCESS_ID;
+  char comm[task_comm_len];
+  char state;
+  ::pid_t ppid = LLDB_INVALID_PROCESS_ID;
+  ::pid_t pgrp = LLDB_INVALID_PROCESS_ID;
+  ::pid_t session = LLDB_INVALID_PROCESS_ID;
+  int tty_nr;
+  int tpgid;
+  unsigned flags;
+  long unsigned minflt;
+  long unsigned cminflt;
+  long unsigned majflt;
+  long unsigned cmajflt;
+  long unsigned utime;
+  long unsigned stime;
+  long cutime;
+  long cstime;
+  //  other things. We don't need them below
+};
 }
 
 namespace lldb_private {
@@ -60,11 +83,92 @@ static bool GetStatusInfo(::pid_t Pid, ProcessInstanceInfo 
,
   ::pid_t ) {
   Log *log = GetLog(LLDBLog::Host);
 
-  auto BufferOrError = getProcFile(Pid, "status");
+  auto BufferOrError = getProcFile(Pid, "stat");
   if (!BufferOrError)
 return false;
 
   llvm::StringRef Rest = BufferOrError.get()->getBuffer();
+  if (Rest.empty())
+return false;
+  StatFields stat_fields;
+  if (sscanf(Rest.data(),
+ "%d %s %c %d %d %d %d %d %u %lu %lu %lu %lu 

[Lldb-commits] [lldb] adds additional information to the ProcessInfo object for elf processes (PR #88995)

2024-04-17 Thread via lldb-commits

jimingham wrote:

> > The user and system times should be monotonically increasing. So you could 
> > stop at a breakpoint, fetch the times, then run your program through a 
> > little spin loop to burn some CPU before hitting a second breakpoint. Then 
> > get the times again and assert that they are > the first set. You could 
> > also set a timer in the test between the first and second stop and assert 
> > that the difference in system and user time is less than or equal to the 
> > timer difference. A single threaded program can only run on one core at a 
> > time, so that should always be true.
> 
> I added a test for user time. System time seems really likely to be flaky in 
> the unittest. It'll increase with kernel time. However if I use side-effect 
> free system calls like getpid() to try and increase that counter it seems 
> like that'll move around a lot from different kernels on different machines

It should never decrease, however.  If you were just getting garbage values, 
then there should be a roughly even chance the second value will be less than 
the first.  So this still gives some confidence this isn't totally bogus...

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


[Lldb-commits] [lldb] adds additional information to the ProcessInfo object for elf processes (PR #88995)

2024-04-17 Thread via lldb-commits

jimingham wrote:

That test looks good.  It seems unlikely that you would buggily always produce 
monotonically increasing times...

Not to be a pest, but since this info is only provided on Linux, we should 
maybe add a test to make sure that asking for this information on systems that 
don't provide it fails gracefully?

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


[Lldb-commits] [lldb] [lldb][DynamicLoader] Fix lldb unable to stop at _dl_debug_state if user set it before the process launched. (PR #88792)

2024-04-17 Thread via lldb-commits

github-actions[bot] wrote:




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



You can test this locally with the following command:


``bash
git-clang-format --diff 0287a5cc4e2a5ded1ae2e4079f91052e6a6b8d9b 
7cf26285a4d75a639d4bbdbf361187c809a5569f -- 
lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
``





View the diff from clang-format here.


``diff
diff --git 
a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp 
b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
index 074cab976e..90e02847d9 100644
--- a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
+++ b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
@@ -571,8 +571,8 @@ ModuleSP DynamicLoaderPOSIXDYLD::LoadInterpreterModule() {
   FileSpec file(info.GetName().GetCString());
   ModuleSpec module_spec(file, target.GetArchitecture());
 
-  if (ModuleSP module_sp = target.GetOrCreateModule(module_spec,
-false /* notify */)) {
+  if (ModuleSP module_sp =
+  target.GetOrCreateModule(module_spec, false /* notify */)) {
 UpdateLoadedSections(module_sp, LLDB_INVALID_ADDRESS, m_interpreter_base,
  false);
 // Manually notify that dynamic linker is loaded after updating load 
section

``




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


[Lldb-commits] [lldb] adds additional information to the ProcessInfo object for elf processes (PR #88995)

2024-04-17 Thread Fred Grim via lldb-commits

feg208 wrote:

> The user and system times should be monotonically increasing. So you could 
> stop at a breakpoint, fetch the times, then run your program through a little 
> spin loop to burn some CPU before hitting a second breakpoint. Then get the 
> times again and assert that they are > the first set. You could also set a 
> timer in the test between the first and second stop and assert that the 
> difference in system and user time is less than or equal to the timer 
> difference. A single threaded program can only run on one core at a time, so 
> that should always be true.

I added a test for user time. System time seems really likely to be flaky in 
the unittest. It'll increase with kernel time. However if I use side-effect 
free system calls like getpid() to try and increase that counter it seems like 
that'll move around a lot from different kernels on different machines

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


[Lldb-commits] [lldb] adds additional information to the ProcessInfo object for elf processes (PR #88995)

2024-04-17 Thread via lldb-commits

github-actions[bot] wrote:




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



You can test this locally with the following command:


``bash
git-clang-format --diff c6e01627acf8591830ee1d211cff4d5388095f3d 
9871016e5cae2d07455cd7857d49dd424066ca41 -- 
lldb/include/lldb/Utility/ProcessInfo.h lldb/source/Host/linux/Host.cpp 
lldb/unittests/Host/linux/HostTest.cpp
``





View the diff from clang-format here.


``diff
diff --git a/lldb/unittests/Host/linux/HostTest.cpp 
b/lldb/unittests/Host/linux/HostTest.cpp
index 481686e08b..045f2ba884 100644
--- a/lldb/unittests/Host/linux/HostTest.cpp
+++ b/lldb/unittests/Host/linux/HostTest.cpp
@@ -65,7 +65,7 @@ TEST_F(HostTest, GetProcessInfo) {
   ASSERT_TRUE(Host::GetProcessInfo(getpid(), Info));
   ProcessInstanceInfo::timespec user_time = Info.GetUserTime();
   for (unsigned i = 0; i < 10'000'000; i++) {
-__asm__ __volatile__ ("" : "+g" (i) : :);
+__asm__ __volatile__("" : "+g"(i) : :);
   }
   ASSERT_TRUE(Host::GetProcessInfo(getpid(), Info));
   ProcessInstanceInfo::timespec next_user_time = Info.GetUserTime();

``




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


[Lldb-commits] [lldb] [lldb][DynamicLoader] Fix lldb unable to stop at _dl_debug_state if user set it before the process launched. (PR #88792)

2024-04-17 Thread Zequan Wu via lldb-commits

ZequanWu wrote:

> So, could the fix be as simple as passing notify=false in the first call ?

Yeah, absolutely. Updated.

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


[Lldb-commits] [lldb] [lldb][DynamicLoader] Fix lldb unable to stop at _dl_debug_state if user set it before the process launched. (PR #88792)

2024-04-17 Thread Zequan Wu via lldb-commits

https://github.com/ZequanWu updated 
https://github.com/llvm/llvm-project/pull/88792

>From 26528cd679478448edf446e0e82e5f207ffd6113 Mon Sep 17 00:00:00 2001
From: Zequan Wu 
Date: Mon, 15 Apr 2024 16:30:38 -0400
Subject: [PATCH 1/4] [lldb][DynamicLoader] Fix lldb unable to stop at
 _dl_debug_state if user set it before the process launched.

---
 lldb/include/lldb/Target/Target.h   |  3 +++
 .../POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp   |  5 +
 lldb/source/Target/Target.cpp   | 17 ++---
 .../breakpoint_command/TestBreakpointCommand.py | 17 +
 4 files changed, 35 insertions(+), 7 deletions(-)

diff --git a/lldb/include/lldb/Target/Target.h 
b/lldb/include/lldb/Target/Target.h
index 2c2e6b2831ccee..3554ef0ea5a140 100644
--- a/lldb/include/lldb/Target/Target.h
+++ b/lldb/include/lldb/Target/Target.h
@@ -878,6 +878,9 @@ class Target : public std::enable_shared_from_this,
   // the address of its previous instruction and return that address.
   lldb::addr_t GetBreakableLoadAddress(lldb::addr_t addr);
 
+  void UpdateBreakpoints(ModuleList _list, bool added,
+ bool delete_locations);
+
   void ModulesDidLoad(ModuleList _list);
 
   void ModulesDidUnload(ModuleList _list, bool delete_locations);
diff --git 
a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp 
b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
index 9baf86da4dc799..901fa53682da8e 100644
--- a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
+++ b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
@@ -576,6 +576,11 @@ ModuleSP DynamicLoaderPOSIXDYLD::LoadInterpreterModule() {
 UpdateLoadedSections(module_sp, LLDB_INVALID_ADDRESS, m_interpreter_base,
  false);
 m_interpreter_module = module_sp;
+// Manually update breakpoints after updating loaded sections, because they
+// cannot be resolve at the time when creating this module.
+ModuleList module_list;
+module_list.Append(module_sp);
+m_process->GetTarget().UpdateBreakpoints(module_list, true, false);
 return module_sp;
   }
   return nullptr;
diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp
index 09b0ac42631d1a..ec2dea5cc238d3 100644
--- a/lldb/source/Target/Target.cpp
+++ b/lldb/source/Target/Target.cpp
@@ -1687,6 +1687,13 @@ void 
Target::NotifyModulesRemoved(lldb_private::ModuleList _list) {
   ModulesDidUnload(module_list, false);
 }
 
+void Target::UpdateBreakpoints(ModuleList _list, bool added,
+   bool delete_locations) {
+  m_breakpoint_list.UpdateBreakpoints(module_list, added, delete_locations);
+  m_internal_breakpoint_list.UpdateBreakpoints(module_list, added,
+   delete_locations);
+}
+
 void Target::ModulesDidLoad(ModuleList _list) {
   const size_t num_images = module_list.GetSize();
   if (m_valid && num_images) {
@@ -1694,8 +1701,7 @@ void Target::ModulesDidLoad(ModuleList _list) {
   ModuleSP module_sp(module_list.GetModuleAtIndex(idx));
   LoadScriptingResourceForModule(module_sp, this);
 }
-m_breakpoint_list.UpdateBreakpoints(module_list, true, false);
-m_internal_breakpoint_list.UpdateBreakpoints(module_list, true, false);
+UpdateBreakpoints(module_list, true, false);
 if (m_process_sp) {
   m_process_sp->ModulesDidLoad(module_list);
 }
@@ -1713,8 +1719,7 @@ void Target::SymbolsDidLoad(ModuleList _list) {
   }
 }
 
-m_breakpoint_list.UpdateBreakpoints(module_list, true, false);
-m_internal_breakpoint_list.UpdateBreakpoints(module_list, true, false);
+UpdateBreakpoints(module_list, true, false);
 auto data_sp =
 std::make_shared(shared_from_this(), module_list);
 BroadcastEvent(eBroadcastBitSymbolsLoaded, data_sp);
@@ -1727,9 +1732,7 @@ void Target::ModulesDidUnload(ModuleList _list, 
bool delete_locations) {
 auto data_sp =
 std::make_shared(shared_from_this(), module_list);
 BroadcastEvent(eBroadcastBitModulesUnloaded, data_sp);
-m_breakpoint_list.UpdateBreakpoints(module_list, false, delete_locations);
-m_internal_breakpoint_list.UpdateBreakpoints(module_list, false,
- delete_locations);
+UpdateBreakpoints(module_list, false, delete_locations);
 
 // If a module was torn down it will have torn down the 'TypeSystemClang's
 // that we used as source 'ASTContext's for the persistent variables in
diff --git 
a/lldb/test/API/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py
 
b/lldb/test/API/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py
index ea242952e409ec..a7e23fae14a21f 100644
--- 
a/lldb/test/API/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py
+++ 

[Lldb-commits] [lldb] adds additional information to the ProcessInfo object for elf processes (PR #88995)

2024-04-17 Thread Fred Grim via lldb-commits

https://github.com/feg208 updated 
https://github.com/llvm/llvm-project/pull/88995

>From 9b8ec4d0c31ad1b228add56bc27cd79457e515c7 Mon Sep 17 00:00:00 2001
From: Fred Grim 
Date: Tue, 16 Apr 2024 14:46:37 -0700
Subject: [PATCH 1/2] adds additional information to the ProcessInfo object for
 elf processes

---
 lldb/include/lldb/Utility/ProcessInfo.h |  71 ++
 lldb/source/Host/linux/Host.cpp | 125 
 lldb/unittests/Host/linux/HostTest.cpp  |   6 ++
 3 files changed, 182 insertions(+), 20 deletions(-)

diff --git a/lldb/include/lldb/Utility/ProcessInfo.h 
b/lldb/include/lldb/Utility/ProcessInfo.h
index 7fb5b37be0f48f..e9fe71e1b851d1 100644
--- a/lldb/include/lldb/Utility/ProcessInfo.h
+++ b/lldb/include/lldb/Utility/ProcessInfo.h
@@ -139,6 +139,11 @@ class ProcessInfo {
 // to that process.
 class ProcessInstanceInfo : public ProcessInfo {
 public:
+  struct timespec {
+time_t tv_sec = 0;
+long int tv_usec = 0;
+  };
+
   ProcessInstanceInfo() = default;
 
   ProcessInstanceInfo(const char *name, const ArchSpec , lldb::pid_t pid)
@@ -172,6 +177,66 @@ class ProcessInstanceInfo : public ProcessInfo {
 return m_parent_pid != LLDB_INVALID_PROCESS_ID;
   }
 
+  lldb::pid_t GetProcessGroupID() const { return m_process_group_id; }
+
+  void SetProcessGroupID(lldb::pid_t pgrp) { m_process_group_id = pgrp; }
+
+  bool ProcessGroupIDIsValid() const {
+return m_process_group_id != LLDB_INVALID_PROCESS_ID;
+  }
+
+  lldb::pid_t GetProcessSessionID() const { return m_process_session_id; }
+
+  void SetProcessSessionID(lldb::pid_t session) {
+m_process_session_id = session;
+  }
+
+  bool ProcessSessionIDIsValid() const {
+return m_process_session_id != LLDB_INVALID_PROCESS_ID;
+  }
+
+  struct timespec GetUserTime() const { return m_user_time; }
+
+  void SetUserTime(struct timespec utime) { m_user_time = utime; }
+
+  bool UserTimeIsValid() const {
+return m_user_time.tv_sec > 0 || m_user_time.tv_usec > 0;
+  }
+
+  struct timespec GetSystemTime() const { return m_system_time; }
+
+  void SetSystemTime(struct timespec stime) { m_system_time = stime; }
+
+  bool SystemTimeIsValid() const {
+return m_system_time.tv_sec > 0 || m_system_time.tv_usec > 0;
+  }
+
+  struct timespec GetCumulativeUserTime() const {
+return m_cumulative_user_time;
+  }
+
+  void SetCumulativeUserTime(struct timespec cutime) {
+m_cumulative_user_time = cutime;
+  }
+
+  bool CumulativeUserTimeIsValid() const {
+return m_cumulative_user_time.tv_sec > 0 ||
+   m_cumulative_user_time.tv_usec > 0;
+  }
+
+  struct timespec GetCumulativeSystemTime() const {
+return m_cumulative_system_time;
+  }
+
+  void SetCumulativeSystemTime(struct timespec cstime) {
+m_cumulative_system_time = cstime;
+  }
+
+  bool CumulativeSystemTimeIsValid() const {
+return m_cumulative_system_time.tv_sec > 0 ||
+   m_cumulative_system_time.tv_sec > 0;
+  }
+
   void Dump(Stream , UserIDResolver ) const;
 
   static void DumpTableHeader(Stream , bool show_args, bool verbose);
@@ -183,6 +248,12 @@ class ProcessInstanceInfo : public ProcessInfo {
   uint32_t m_euid = UINT32_MAX;
   uint32_t m_egid = UINT32_MAX;
   lldb::pid_t m_parent_pid = LLDB_INVALID_PROCESS_ID;
+  lldb::pid_t m_process_group_id = LLDB_INVALID_PROCESS_ID;
+  lldb::pid_t m_process_session_id = LLDB_INVALID_PROCESS_ID;
+  struct timespec m_user_time {};
+  struct timespec m_system_time {};
+  struct timespec m_cumulative_user_time {};
+  struct timespec m_cumulative_system_time {};
 };
 
 typedef std::vector ProcessInstanceInfoList;
diff --git a/lldb/source/Host/linux/Host.cpp b/lldb/source/Host/linux/Host.cpp
index 6c57384aa38a13..c6490f2fc9e2f5 100644
--- a/lldb/source/Host/linux/Host.cpp
+++ b/lldb/source/Host/linux/Host.cpp
@@ -49,6 +49,29 @@ enum class ProcessState {
   TracedOrStopped,
   Zombie,
 };
+
+constexpr int task_comm_len = 16;
+
+struct StatFields {
+  ::pid_t pid = LLDB_INVALID_PROCESS_ID;
+  char comm[task_comm_len];
+  char state;
+  ::pid_t ppid = LLDB_INVALID_PROCESS_ID;
+  ::pid_t pgrp = LLDB_INVALID_PROCESS_ID;
+  ::pid_t session = LLDB_INVALID_PROCESS_ID;
+  int tty_nr;
+  int tpgid;
+  unsigned flags;
+  long unsigned minflt;
+  long unsigned cminflt;
+  long unsigned majflt;
+  long unsigned cmajflt;
+  long unsigned utime;
+  long unsigned stime;
+  long cutime;
+  long cstime;
+  //  other things. We don't need them below
+};
 }
 
 namespace lldb_private {
@@ -60,11 +83,92 @@ static bool GetStatusInfo(::pid_t Pid, ProcessInstanceInfo 
,
   ::pid_t ) {
   Log *log = GetLog(LLDBLog::Host);
 
-  auto BufferOrError = getProcFile(Pid, "status");
+  auto BufferOrError = getProcFile(Pid, "stat");
   if (!BufferOrError)
 return false;
 
   llvm::StringRef Rest = BufferOrError.get()->getBuffer();
+  if (Rest.empty())
+return false;
+  StatFields stat_fields;
+  if (sscanf(Rest.data(),
+ "%d %s %c %d %d %d %d %d %u %lu %lu %lu %lu 

[Lldb-commits] [lldb] [lldb][DynamicLoader] Fix lldb unable to stop at _dl_debug_state if user set it before the process launched. (PR #88792)

2024-04-17 Thread Pavel Labath via lldb-commits

labath wrote:

So, could the fix be as simple as passing notify=false in the first call ?

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


[Lldb-commits] [lldb] [lldb] Correct documentation of LLDB_TEST_USER_ARGS (PR #89042)

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

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


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


[Lldb-commits] [lldb] [lldb] Correct documentation of LLDB_TEST_USER_ARGS (PR #89042)

2024-04-17 Thread Mark de Wever via lldb-commits

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

Thanks! The changes LGTM, but I haven't verified the commands work as described.

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


[Lldb-commits] [lldb] adds additional information to the ProcessInfo object for elf processes (PR #88995)

2024-04-17 Thread Fred Grim via lldb-commits

feg208 wrote:

> The user and system times should be monotonically increasing. So you could 
> stop at a breakpoint, fetch the times, then run your program through a little 
> spin loop to burn some CPU before hitting a second breakpoint. Then get the 
> times again and assert that they are > the first set. You could also set a 
> timer in the test between the first and second stop and assert that the 
> difference in system and user time is less than or equal to the timer 
> difference. A single threaded program can only run on one core at a time, so 
> that should always be true. 


Ok let me add a test along these lines. I'll run the test a bunch locally and 
in this pr to make sure I don't make a mess

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


[Lldb-commits] [lldb] [lldb] Skip remote PutFile when MD5 hashes equal (PR #88812)

2024-04-17 Thread David Spickett via lldb-commits

DavidSpickett wrote:

If it were a GDB packet it would be in 
https://sourceware.org/gdb/current/onlinedocs/gdb.html/Host-I_002fO-Packets.html#Host-I_002fO-Packets
 but I see no sign of it, or in GDB's sources.

The first appearance of `vFile:MD5` is e0f8f574c7f41f5b61ec01aa290d52fc55a3f3c9 
though it is not documented as one of our extensions in 
https://github.com/llvm/llvm-project/blob/main/lldb/docs/lldb-platform-packets.txt.

So a good follow up PR would be to list it in that document. It's self 
explanatory but still, weird that it's not there.

This means we could add `vFile:betterHash` (or a more general `hash:`) 
and try sending it to the remote. If the remote responds that it doesn't 
support it, we ask it for MD5. Though we may want a way to say don't use MD5 
even if the remote would accept it, if security is the concern (a bit like ssh 
key formats work).

In the case of this specific PR, it's fixing code that should have worked all 
along but didn't. So the issue of MD5 collision is worth looking at, but I 
wouldn't let it block this.

@emaste, since you know about the shortcomings of MD5, could you open an issue 
for this? And I'll add to that the internal lldb details.

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


[Lldb-commits] [lldb] [lldb][DynamicLoader] Fix lldb unable to stop at _dl_debug_state if user set it before the process launched. (PR #88792)

2024-04-17 Thread Zequan Wu via lldb-commits

ZequanWu wrote:

> Can you check where does the second event get sent from? Is it by any chance 
> when we go through 
> [this](https://github.com/llvm/llvm-project/blob/main/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp#L460)
>  place ?

The first event is sent when creating the module for ld.so: 
https://github.com/llvm/llvm-project/blob/458328ae23d318a5055d5bac66426b8551bce01f/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp#L574.
 It gets notified at here: 
https://github.com/llvm/llvm-project/blob/458328ae23d318a5055d5bac66426b8551bce01f/lldb/source/Target/Target.cpp#L1660.
 The second event is sent because I explicitly calls `ModulesDidLoad` in this 
change (the current version) after the creating the ld.so module.

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


[Lldb-commits] [lldb] [lldb] Fix evaluation of expressions with static initializers (PR #89063)

2024-04-17 Thread Pavel Labath via lldb-commits

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


[Lldb-commits] [lldb] 915c84b - [lldb] Fix evaluation of expressions with static initializers (#89063)

2024-04-17 Thread via lldb-commits

Author: Pavel Labath
Date: 2024-04-17T14:33:18+02:00
New Revision: 915c84b1480bb3c6d2e44ca83822d2c2304b763a

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

LOG: [lldb] Fix evaluation of expressions with static initializers (#89063)

After 281d71604f418eb952e967d9dc4b26241b7f96a, llvm generates 32-bit
relocations, which overflow when we load these objects into high memory.
Interestingly, setting the code model to "large" does not help here
(perhaps it is the default?). I'm not completely sure that this is the
right thing to do, but it doesn't seem to cause any ill effects. I'll
follow up with the author of that patch about the expected behavior
here.

Added: 


Modified: 
lldb/source/Expression/IRExecutionUnit.cpp

Removed: 




diff  --git a/lldb/source/Expression/IRExecutionUnit.cpp 
b/lldb/source/Expression/IRExecutionUnit.cpp
index cb9bee8733e15d..7ad0e5ff22b2f6 100644
--- a/lldb/source/Expression/IRExecutionUnit.cpp
+++ b/lldb/source/Expression/IRExecutionUnit.cpp
@@ -13,6 +13,7 @@
 #include "llvm/IR/DiagnosticInfo.h"
 #include "llvm/IR/LLVMContext.h"
 #include "llvm/IR/Module.h"
+#include "llvm/Support/CodeGen.h"
 #include "llvm/Support/SourceMgr.h"
 #include "llvm/Support/raw_ostream.h"
 
@@ -279,10 +280,13 @@ void IRExecutionUnit::GetRunnableInfo(Status , 
lldb::addr_t _addr,
   llvm::EngineBuilder builder(std::move(m_module_up));
   llvm::Triple triple(m_module->getTargetTriple());
 
+  // PIC needed for ELF to avoid generating 32-bit relocations (which overflow
+  // if the object is loaded into high memory).
+  bool want_pic = triple.isOSBinFormatMachO() || triple.isOSBinFormatELF();
+
   builder.setEngineKind(llvm::EngineKind::JIT)
   .setErrorStr(_string)
-  .setRelocationModel(triple.isOSBinFormatMachO() ? llvm::Reloc::PIC_
-  : llvm::Reloc::Static)
+  .setRelocationModel(want_pic ? llvm::Reloc::PIC_ : llvm::Reloc::Static)
   .setMCJITMemoryManager(std::make_unique(*this))
   .setOptLevel(llvm::CodeGenOptLevel::Less);
 



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


[Lldb-commits] [lldb] [lldb][DynamicLoader] Fix lldb unable to stop at _dl_debug_state if user set it before the process launched. (PR #88792)

2024-04-17 Thread Pavel Labath via lldb-commits

labath wrote:

> > > > why not just call ModulesDidLoad and delegate this (and possibly other 
> > > > binary-just-loaded) behaviors to it?
> > > 
> > > 
> > > That's what I did first, but it breaks the test 
> > > `TestModuleLoadedNotifys.ModuleLoadedNotifysTestCase.test_launch_notifications`
> > >  because of extra broadcaster event. So, I moved out the part just 
> > > updating breakpoints.
> > 
> > 
> > The point of the test is to ensure you don't get a hundred notifications, 
> > one for each module. Having one notification for ld.so, and 99 for the rest 
> > of the modules is ok. It should be fine to just update the test to match 
> > new reality.
> 
> Updated to use `Target::ModulesDisLoad` and the test.

Oh, so the problem is that the loaded event gets sent twice (and not that it 
gets sent as a separate event, as I originally thought). While it's not the end 
of the world (it appears the mac loaded does something similar as well), this 
does seem like something that we could prevent. Can you check where does the 
second event get sent from? Is it by any chance when we go through 
[this](https://github.com/llvm/llvm-project/blob/main/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp#L460)
 place ? If so, it should be fairly easy to refactor that code to avoid putting 
the interpreter module into the event list when it is already loaded.

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


[Lldb-commits] [lldb] [lldb] Fix evaluation of expressions with static initializers (PR #89063)

2024-04-17 Thread via lldb-commits

llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: Pavel Labath (labath)


Changes

After 281d71604f418eb952e967d9dc4b26241b7f96a, llvm generates 32-bit 
relocations, which overflow when we load these objects into high memory. 
Interestingly, setting the code model to "large" does not help here (perhaps it 
is the default?). I'm not completely sure that this is the right thing to do, 
but it doesn't seem to cause any ill effects. I'll follow up with the author of 
that patch about the expected behavior here.

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


1 Files Affected:

- (modified) lldb/source/Expression/IRExecutionUnit.cpp (+6-2) 


``diff
diff --git a/lldb/source/Expression/IRExecutionUnit.cpp 
b/lldb/source/Expression/IRExecutionUnit.cpp
index cb9bee8733e15d..7ad0e5ff22b2f6 100644
--- a/lldb/source/Expression/IRExecutionUnit.cpp
+++ b/lldb/source/Expression/IRExecutionUnit.cpp
@@ -13,6 +13,7 @@
 #include "llvm/IR/DiagnosticInfo.h"
 #include "llvm/IR/LLVMContext.h"
 #include "llvm/IR/Module.h"
+#include "llvm/Support/CodeGen.h"
 #include "llvm/Support/SourceMgr.h"
 #include "llvm/Support/raw_ostream.h"
 
@@ -279,10 +280,13 @@ void IRExecutionUnit::GetRunnableInfo(Status , 
lldb::addr_t _addr,
   llvm::EngineBuilder builder(std::move(m_module_up));
   llvm::Triple triple(m_module->getTargetTriple());
 
+  // PIC needed for ELF to avoid generating 32-bit relocations (which overflow
+  // if the object is loaded into high memory).
+  bool want_pic = triple.isOSBinFormatMachO() || triple.isOSBinFormatELF();
+
   builder.setEngineKind(llvm::EngineKind::JIT)
   .setErrorStr(_string)
-  .setRelocationModel(triple.isOSBinFormatMachO() ? llvm::Reloc::PIC_
-  : llvm::Reloc::Static)
+  .setRelocationModel(want_pic ? llvm::Reloc::PIC_ : llvm::Reloc::Static)
   .setMCJITMemoryManager(std::make_unique(*this))
   .setOptLevel(llvm::CodeGenOptLevel::Less);
 

``




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


[Lldb-commits] [lldb] [lldb] Fix evaluation of expressions with static initializers (PR #89063)

2024-04-17 Thread Pavel Labath via lldb-commits

https://github.com/labath created 
https://github.com/llvm/llvm-project/pull/89063

After 281d71604f418eb952e967d9dc4b26241b7f96a, llvm generates 32-bit 
relocations, which overflow when we load these objects into high memory. 
Interestingly, setting the code model to "large" does not help here (perhaps it 
is the default?). I'm not completely sure that this is the right thing to do, 
but it doesn't seem to cause any ill effects. I'll follow up with the author of 
that patch about the expected behavior here.

>From e9b74aae92b8d8583d3549c6ef7257fd026678f5 Mon Sep 17 00:00:00 2001
From: Pavel Labath 
Date: Wed, 17 Apr 2024 11:58:44 +
Subject: [PATCH] [lldb] Fix evaluation of expressions with static initializers

After 281d71604f418eb952e967d9dc4b26241b7f96a, llvm generates 32-bit
relocations, which overflow when we load these objects into high memory.
Interestingly, setting the code model to "large" does not help here
(perhaps it is the default?). I'm not completely sure that this is the
right thing to do, but it doesn't seem to cause any ill effects.
I'll follow up with the author of that patch about the expected behavior
here.
---
 lldb/source/Expression/IRExecutionUnit.cpp | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/lldb/source/Expression/IRExecutionUnit.cpp 
b/lldb/source/Expression/IRExecutionUnit.cpp
index cb9bee8733e15d..7ad0e5ff22b2f6 100644
--- a/lldb/source/Expression/IRExecutionUnit.cpp
+++ b/lldb/source/Expression/IRExecutionUnit.cpp
@@ -13,6 +13,7 @@
 #include "llvm/IR/DiagnosticInfo.h"
 #include "llvm/IR/LLVMContext.h"
 #include "llvm/IR/Module.h"
+#include "llvm/Support/CodeGen.h"
 #include "llvm/Support/SourceMgr.h"
 #include "llvm/Support/raw_ostream.h"
 
@@ -279,10 +280,13 @@ void IRExecutionUnit::GetRunnableInfo(Status , 
lldb::addr_t _addr,
   llvm::EngineBuilder builder(std::move(m_module_up));
   llvm::Triple triple(m_module->getTargetTriple());
 
+  // PIC needed for ELF to avoid generating 32-bit relocations (which overflow
+  // if the object is loaded into high memory).
+  bool want_pic = triple.isOSBinFormatMachO() || triple.isOSBinFormatELF();
+
   builder.setEngineKind(llvm::EngineKind::JIT)
   .setErrorStr(_string)
-  .setRelocationModel(triple.isOSBinFormatMachO() ? llvm::Reloc::PIC_
-  : llvm::Reloc::Static)
+  .setRelocationModel(want_pic ? llvm::Reloc::PIC_ : llvm::Reloc::Static)
   .setMCJITMemoryManager(std::make_unique(*this))
   .setOptLevel(llvm::CodeGenOptLevel::Less);
 

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


[Lldb-commits] [lldb] [lldb/linux] Make sure the process continues running after a detach (PR #88494)

2024-04-17 Thread Pavel Labath via lldb-commits

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


[Lldb-commits] [lldb] 5f3e106 - [lldb/linux] Make sure the process continues running after a detach (#88494)

2024-04-17 Thread via lldb-commits

Author: Pavel Labath
Date: 2024-04-17T13:05:15+02:00
New Revision: 5f3e106de3cd5ce6d7ba37fb11f6ad740cb430c5

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

LOG: [lldb/linux] Make sure the process continues running after a detach 
(#88494)

Fixes #85084

Whenever an inferior thread stops, lldb-server sends a SIGSTOP to all
other threads in the process to force them to stop as well. If those
threads stop on their own before they get a signal, this SIGSTOP will
remain pending and be delivered the next time the process resumes.

Normally, this is not a problem, because lldb-server will detect this
stale SIGSTOP and resume the process. However, if we detach from the
process while it has these SIGSTOPs pending, they will get immediately
delivered, and the process will remain stopped (most likely forever).

This patch fixes that by sending a SIGCONT just before detaching from
the process. This signal cancels out any pending SIGSTOPs, and ensures
it is able to run after we detach. It does have one somewhat unfortunate
side-effect that in that the process's SIGCONT handler (if it has one)
will get executed spuriously (from the process's POV).

This could be _sometimes_ avoided by tracking which threads got send a
SIGSTOP, and whether those threads stopped due to it. From what I could
tell by observing its behavior, this is what gdb does. I have not tried
to replicate that behavior here because it adds a nontrivial amount of
complexity and the result is still uncertain -- we still need to send a
SIGCONT (and execute the handler) when any thread stops for some other
reason (and leaves our SIGSTOP hanging). Furthermore, since SIGSTOPs
don't stack, it's also possible that our SIGSTOP/SIGCONT combination
will cancel a genuine SIGSTOP being sent to the debugger application (by
someone else), and there is nothing we can do about that. For this
reason I think it's simplest and most predictible to just always send a
SIGCONT when detaching, but if it turns out this is breaking something,
we can consider implementing something more elaborate.

One alternative I did try is to use PTRACE_INTERRUPT to suspend the
threads instead of a SIGSTOP. PTRACE_INTERUPT requires using
PTRACE_SEIZE to attach to the process, which also made this solution
somewhat complicated, but the main problem with that approach is that
PTRACE_INTERRUPT is not considered to be a signal-delivery-stop, which
means it's not possible to resume it while injecting another signal to
the inferior (which some of our tests expect to be able to do). This
limitation could be worked around by forcing the thread into a signal
delivery stop whenever we need to do this, but this additional
complication is what made me think this approach is also not worthwhile.

This patch should fix (at least some of) the problems with
TestConcurrentVFork, but I've also added a dedicated test for checking
that a process keeps running after we detach. Although the problem I'm
fixing here is linux-specific, the core functinoality of not stopping
after a detach should function the same way everywhere.

Added: 
lldb/test/API/commands/process/detach-resumes/Makefile
lldb/test/API/commands/process/detach-resumes/TestDetachResumes.py
lldb/test/API/commands/process/detach-resumes/main.cpp

Modified: 
lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
lldb/test/API/functionalities/fork/concurrent_vfork/TestConcurrentVFork.py

Removed: 




diff  --git a/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp 
b/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
index 5d2b4b03fe60cb..59fc8726b76739 100644
--- a/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
+++ b/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
@@ -1089,6 +1089,10 @@ Status NativeProcessLinux::Detach() {
   if (GetID() == LLDB_INVALID_PROCESS_ID)
 return error;
 
+  // Cancel out any SIGSTOPs we may have sent while stopping the process.
+  // Otherwise, the process may stop as soon as we detach from it.
+  kill(GetID(), SIGCONT);
+
   for (const auto  : m_threads) {
 Status e = Detach(thread->GetID());
 if (e.Fail())

diff  --git a/lldb/test/API/commands/process/detach-resumes/Makefile 
b/lldb/test/API/commands/process/detach-resumes/Makefile
new file mode 100644
index 00..c46619c6623481
--- /dev/null
+++ b/lldb/test/API/commands/process/detach-resumes/Makefile
@@ -0,0 +1,4 @@
+CXX_SOURCES := main.cpp
+ENABLE_THREADS := YES
+
+include Makefile.rules

diff  --git 
a/lldb/test/API/commands/process/detach-resumes/TestDetachResumes.py 
b/lldb/test/API/commands/process/detach-resumes/TestDetachResumes.py
new file mode 100644
index 00..57727294ddc3d3
--- /dev/null
+++ 

[Lldb-commits] [lldb] [lldb] fix python extension debug suffix on Win (PR #89037)

2024-04-17 Thread David Spickett via lldb-commits

DavidSpickett wrote:

So this PR fixes debug lldb -> release Python, but if I configured against a 
debug Python then the suffix would be `_d_d`.

Debug lldb/release Python is a valid config I think, and likely what 99% of 
people want anyway. So can we add a check whether the suffix already has `_d` 
in it, so that debug lldb/debug python also works?


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


[Lldb-commits] [lldb] [lldb] Correct documentation of LLDB_TEST_USER_ARGS (PR #89042)

2024-04-17 Thread via lldb-commits

llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: David Spickett (DavidSpickett)


Changes

https://discourse.llvm.org/t/running-lldb-in-a-container/76801/4 found that the 
obvious way to use this variable doesn't work, despite what our docs and 
examples say.

Perhaps in the past it did but now you need to use ";" as a separator to make 
sure the final command line works properly.

For example "-A foo" becomes "-A foo" when python goes to run the runner 
script. The script sees this as one command line element, not two. What you 
actually want is "-A;foo" which we convert to "-A" "foo" which the script sees 
as one option and one value for that option.

The "Script:" printout from dotest is misleading here because it does `" 
".join(cmd)` so it looks like it's ok but in fact it's not.

I'm not changing that format though because printing the command as a Python 
list is not useful outside of this specific situation.

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


2 Files Affected:

- (modified) lldb/docs/resources/test.rst (+21-4) 
- (modified) lldb/test/API/CMakeLists.txt (+2-1) 


``diff
diff --git a/lldb/docs/resources/test.rst b/lldb/docs/resources/test.rst
index 2b0e9010fe280a..094fde8b1b5a0a 100644
--- a/lldb/docs/resources/test.rst
+++ b/lldb/docs/resources/test.rst
@@ -441,23 +441,40 @@ Running the Full Test Suite
 The easiest way to run the LLDB test suite is to use the ``check-lldb`` build
 target.
 
+::
+
+   $ ninja check-lldb
+
+Changing Test Suite Options
+```
+
 By default, the ``check-lldb`` target builds the test programs with the same
 compiler that was used to build LLDB. To build the tests with a different
 compiler, you can set the ``LLDB_TEST_COMPILER`` CMake variable.
 
+You can also add to the test runner options by setting the
+``LLDB_TEST_USER_ARGS`` CMake variable. This variable uses ``;`` to separate
+items which must be separate parts of the runner's command line.
+
 It is possible to customize the architecture of the test binaries and compiler
-used by appending ``-A`` and ``-C`` options respectively to the CMake variable
-``LLDB_TEST_USER_ARGS``. For example, to test LLDB against 32-bit binaries
-built with a custom version of clang, do:
+used by appending ``-A`` and ``-C`` options respectively. For example, to test
+LLDB against 32-bit binaries built with a custom version of clang, do:
 
 ::
 
-   $ cmake -DLLDB_TEST_USER_ARGS="-A i386 -C /path/to/custom/clang" -G Ninja
+   $ cmake -DLLDB_TEST_USER_ARGS="-A;i386;-C;/path/to/custom/clang" -G Ninja
$ ninja check-lldb
 
 Note that multiple ``-A`` and ``-C`` flags can be specified to
 ``LLDB_TEST_USER_ARGS``.
 
+If you want to change the LLDB settings that tests run with then you can set
+the ``--setting`` option of the test runner via this same variable. For example
+``--setting;target.disable-aslr=true``.
+
+For a full list of test runner options, see
+``/bin/lldb-dotest --help``.
+
 Running a Single Test Suite
 ```
 
diff --git a/lldb/test/API/CMakeLists.txt b/lldb/test/API/CMakeLists.txt
index 4e02112c29e4cd..9196f54ce1ae32 100644
--- a/lldb/test/API/CMakeLists.txt
+++ b/lldb/test/API/CMakeLists.txt
@@ -35,7 +35,8 @@ set(LLDB_TEST_ARCH
 # Users can override LLDB_TEST_USER_ARGS to specify arbitrary arguments to 
pass to the script
 set(LLDB_TEST_USER_ARGS
   ""
-  CACHE STRING "Specify additional arguments to pass to test runner. For 
example: '-C gcc -C clang -A i386 -A x86_64'")
+  CACHE STRING "Specify additional arguments to pass to test runner. Seperate \
+items with \";\". For example: '-C;gcc;-C;clang;-A;i386;-A;x86_64'")
 
 set(LLDB_TEST_COMMON_ARGS_VAR
   -u CXXFLAGS

``




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


[Lldb-commits] [lldb] [lldb] Correct documentation of LLDB_TEST_USER_ARGS (PR #89042)

2024-04-17 Thread David Spickett via lldb-commits

https://github.com/DavidSpickett created 
https://github.com/llvm/llvm-project/pull/89042

https://discourse.llvm.org/t/running-lldb-in-a-container/76801/4 found that the 
obvious way to use this variable doesn't work, despite what our docs and 
examples say.

Perhaps in the past it did but now you need to use ";" as a separator to make 
sure the final command line works properly.

For example "-A foo" becomes "-A foo" when python goes to run the runner 
script. The script sees this as one command line element, not two. What you 
actually want is "-A;foo" which we convert to "-A" "foo" which the script sees 
as one option and one value for that option.

The "Script:" printout from dotest is misleading here because it does `" 
".join(cmd)` so it looks like it's ok but in fact it's not.

I'm not changing that format though because printing the command as a Python 
list is not useful outside of this specific situation.

>From 424c48e1b53ebfe1119ad09b39f6730d127b254e Mon Sep 17 00:00:00 2001
From: David Spickett 
Date: Wed, 17 Apr 2024 09:49:37 +
Subject: [PATCH] [lldb] Correct documentation of LLDB_TEST_USER_ARGS

https://discourse.llvm.org/t/running-lldb-in-a-container/76801/4 found
that the obvious way to use this variable doesn't work, despite
what our docs and examples say.

Perhaps in the past it did but now you need to use ";" as a separator
to make sure the final command line works properly.

For example "-A foo" becomes "-A foo" when python goes to run the
runner script. The script sees this as one command line element,
not two. What you actually want is "-A;foo" which we convert
to "-A" "foo" which the script sees as one option and one value
for that option.

The "Script:" printout from dotest is misleading here because
it does `" ".join(cmd)` so it looks like it's ok but in fact it's not.

I'm not changing that format though because printing the command as a
Python list is not useful outside of this specific situation.
---
 lldb/docs/resources/test.rst | 25 +
 lldb/test/API/CMakeLists.txt |  3 ++-
 2 files changed, 23 insertions(+), 5 deletions(-)

diff --git a/lldb/docs/resources/test.rst b/lldb/docs/resources/test.rst
index 2b0e9010fe280a..094fde8b1b5a0a 100644
--- a/lldb/docs/resources/test.rst
+++ b/lldb/docs/resources/test.rst
@@ -441,23 +441,40 @@ Running the Full Test Suite
 The easiest way to run the LLDB test suite is to use the ``check-lldb`` build
 target.
 
+::
+
+   $ ninja check-lldb
+
+Changing Test Suite Options
+```
+
 By default, the ``check-lldb`` target builds the test programs with the same
 compiler that was used to build LLDB. To build the tests with a different
 compiler, you can set the ``LLDB_TEST_COMPILER`` CMake variable.
 
+You can also add to the test runner options by setting the
+``LLDB_TEST_USER_ARGS`` CMake variable. This variable uses ``;`` to separate
+items which must be separate parts of the runner's command line.
+
 It is possible to customize the architecture of the test binaries and compiler
-used by appending ``-A`` and ``-C`` options respectively to the CMake variable
-``LLDB_TEST_USER_ARGS``. For example, to test LLDB against 32-bit binaries
-built with a custom version of clang, do:
+used by appending ``-A`` and ``-C`` options respectively. For example, to test
+LLDB against 32-bit binaries built with a custom version of clang, do:
 
 ::
 
-   $ cmake -DLLDB_TEST_USER_ARGS="-A i386 -C /path/to/custom/clang" -G Ninja
+   $ cmake -DLLDB_TEST_USER_ARGS="-A;i386;-C;/path/to/custom/clang" -G Ninja
$ ninja check-lldb
 
 Note that multiple ``-A`` and ``-C`` flags can be specified to
 ``LLDB_TEST_USER_ARGS``.
 
+If you want to change the LLDB settings that tests run with then you can set
+the ``--setting`` option of the test runner via this same variable. For example
+``--setting;target.disable-aslr=true``.
+
+For a full list of test runner options, see
+``/bin/lldb-dotest --help``.
+
 Running a Single Test Suite
 ```
 
diff --git a/lldb/test/API/CMakeLists.txt b/lldb/test/API/CMakeLists.txt
index 4e02112c29e4cd..9196f54ce1ae32 100644
--- a/lldb/test/API/CMakeLists.txt
+++ b/lldb/test/API/CMakeLists.txt
@@ -35,7 +35,8 @@ set(LLDB_TEST_ARCH
 # Users can override LLDB_TEST_USER_ARGS to specify arbitrary arguments to 
pass to the script
 set(LLDB_TEST_USER_ARGS
   ""
-  CACHE STRING "Specify additional arguments to pass to test runner. For 
example: '-C gcc -C clang -A i386 -A x86_64'")
+  CACHE STRING "Specify additional arguments to pass to test runner. Seperate \
+items with \";\". For example: '-C;gcc;-C;clang;-A;i386;-A;x86_64'")
 
 set(LLDB_TEST_COMMON_ARGS_VAR
   -u CXXFLAGS

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


[Lldb-commits] [lldb] a16bb07 - [lldb][test] Improve invalid compiler error message

2024-04-17 Thread David Spickett via lldb-commits

Author: David Spickett
Date: 2024-04-17T09:19:26Z
New Revision: a16bb0701409376dee3a587ae351a6019d6de4e0

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

LOG: [lldb][test] Improve invalid compiler error message

I was debugging space separation issues when passing user arguments
and noticed this error is really hard to read in that scenario.

Put "" around the invalid compiler name so you can tell whether
you have spaces around it that's causing the problem.

Added: 


Modified: 
lldb/packages/Python/lldbsuite/test/dotest.py

Removed: 




diff  --git a/lldb/packages/Python/lldbsuite/test/dotest.py 
b/lldb/packages/Python/lldbsuite/test/dotest.py
index 8c29145ecc5272..2ec4a840b91675 100644
--- a/lldb/packages/Python/lldbsuite/test/dotest.py
+++ b/lldb/packages/Python/lldbsuite/test/dotest.py
@@ -248,7 +248,7 @@ def parseOptionsAndInitTestdirs():
 configuration.compiler = which(args.compiler)
 if not is_exe(configuration.compiler):
 logging.error(
-"%s is not a valid compiler executable; aborting...", 
args.compiler
+'"%s" is not a valid compiler executable; aborting...', 
args.compiler
 )
 sys.exit(-1)
 else:



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


[Lldb-commits] [lldb] [lldb] fix python extension debug suffix on Win (PR #89037)

2024-04-17 Thread Alexander M. via lldb-commits

amordo wrote:

> Looks fine but can you just confirm what
> 
> https://github.com/llvm/llvm-project/blob/d1a69e4a6ee0b04778da7728123c47eef2290564/lldb/bindings/python/get-python-config.py#L70
> 
> reports?
> I assume this does not include `_d`, otherwise there would be no need for 
> this fix?

```
C:\Users\iammorjj\llvm-project\lldb\bindings\python>python get-python-config.py 
LLDB_PYTHON_EXT_SUFFIX
.cp312-win_amd64.pyd

C:\Users\iammorjj\llvm-project\lldb\bindings\python>python_d 
get-python-config.py LLDB_PYTHON_EXT_SUFFIX
_d.cp312-win_amd64.pyd
```
Now I suppose the problem comes from 
```
foreach(var LLDB_PYTHON_RELATIVE_PATH LLDB_PYTHON_EXE_RELATIVE_PATH 
LLDB_PYTHON_EXT_SUFFIX)
if(NOT DEFINED ${var} AND NOT CMAKE_CROSSCOMPILING)
  execute_process(
COMMAND ${Python3_EXECUTABLE}
  ${CMAKE_CURRENT_SOURCE_DIR}/bindings/python/get-python-config.py
  ${var}
OUTPUT_VARIABLE value
OUTPUT_STRIP_TRAILING_WHITESPACE)
  file(TO_CMAKE_PATH "${value}" value)
  set(${var} ${value} CACHE STRING ${cachestring_${var}})
```
where `Python3_EXECUTABLE` is Release python

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


[Lldb-commits] [lldb] [lldb] fix python extension debug suffix on Win (PR #89037)

2024-04-17 Thread David Spickett via lldb-commits

DavidSpickett wrote:

The best docs I can find are https://peps.python.org/pep-0720/:
"Interpreter-specific Python extension (native module) suffix — generally 
defined as .{SOABI}.{SHLIB_SUFFIX}."

Which is less than helpful. https://peps.python.org/pep-3149/ mentions Windows 
uses a different format.

But anyway, just curious exactly what your interpreter returns.

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


[Lldb-commits] [lldb] [lldb] fix python extension debug suffix on Win (PR #89037)

2024-04-17 Thread David Spickett via lldb-commits

DavidSpickett wrote:

Looks fine but can you just confirm what 
https://github.com/llvm/llvm-project/blob/d1a69e4a6ee0b04778da7728123c47eef2290564/lldb/bindings/python/get-python-config.py#L70
 reports?

I assume this does not include `_d`, otherwise there would be no need for this 
fix?

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


[Lldb-commits] [lldb] [lldb] fix python extension debug suffix on Win (PR #89037)

2024-04-17 Thread via lldb-commits

llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: Alexander M. (amordo)


Changes

ae389b2450bd604a3f3bbe5b09b333b2d99801dd change doesn't cover "_d" suffix for 
Debug build on Windows.

Fixed #87381.

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


1 Files Affected:

- (modified) lldb/bindings/python/CMakeLists.txt (+5-1) 


``diff
diff --git a/lldb/bindings/python/CMakeLists.txt 
b/lldb/bindings/python/CMakeLists.txt
index 73b1239495e22e..183cee9ea64a06 100644
--- a/lldb/bindings/python/CMakeLists.txt
+++ b/lldb/bindings/python/CMakeLists.txt
@@ -136,7 +136,11 @@ function(finish_swig_python swig_target 
lldb_python_bindings_dir lldb_python_tar
   else()
 set(LIBLLDB_SYMLINK_DEST 
"${LLVM_SHLIB_OUTPUT_INTDIR}/liblldb${CMAKE_SHARED_LIBRARY_SUFFIX}")
   endif()
-  set(LIBLLDB_SYMLINK_OUTPUT_FILE "_lldb${LLDB_PYTHON_EXT_SUFFIX}")
+  if(WIN32 AND CMAKE_BUILD_TYPE STREQUAL Debug)
+set(LIBLLDB_SYMLINK_OUTPUT_FILE "_lldb_d${LLDB_PYTHON_EXT_SUFFIX}")
+  else()
+set(LIBLLDB_SYMLINK_OUTPUT_FILE "_lldb${LLDB_PYTHON_EXT_SUFFIX}")
+  endif()
   create_relative_symlink(${swig_target} ${LIBLLDB_SYMLINK_DEST}
   ${lldb_python_target_dir} 
${LIBLLDB_SYMLINK_OUTPUT_FILE})
 

``




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


[Lldb-commits] [lldb] [lldb] fix python extension debug suffix on Win (PR #89037)

2024-04-17 Thread Alexander M. via lldb-commits

https://github.com/amordo created 
https://github.com/llvm/llvm-project/pull/89037

ae389b2450bd604a3f3bbe5b09b333b2d99801dd change doesn't cover "_d" suffix for 
Debug build on Windows.

Fixed #87381.

>From f9aa0192bb43e15ceab107b2beb0d2d50f1f8150 Mon Sep 17 00:00:00 2001
From: Alexander Mordovskiy 
Date: Wed, 17 Apr 2024 10:35:50 +0200
Subject: [PATCH] [lldb] fix python extension debug suffix on Win

ae389b2450bd604a3f3bbe5b09b333b2d99801dd change doesn't cover "_d" suffix for 
Debug build on Windows.
---
 lldb/bindings/python/CMakeLists.txt | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/lldb/bindings/python/CMakeLists.txt 
b/lldb/bindings/python/CMakeLists.txt
index 73b1239495e22e..183cee9ea64a06 100644
--- a/lldb/bindings/python/CMakeLists.txt
+++ b/lldb/bindings/python/CMakeLists.txt
@@ -136,7 +136,11 @@ function(finish_swig_python swig_target 
lldb_python_bindings_dir lldb_python_tar
   else()
 set(LIBLLDB_SYMLINK_DEST 
"${LLVM_SHLIB_OUTPUT_INTDIR}/liblldb${CMAKE_SHARED_LIBRARY_SUFFIX}")
   endif()
-  set(LIBLLDB_SYMLINK_OUTPUT_FILE "_lldb${LLDB_PYTHON_EXT_SUFFIX}")
+  if(WIN32 AND CMAKE_BUILD_TYPE STREQUAL Debug)
+set(LIBLLDB_SYMLINK_OUTPUT_FILE "_lldb_d${LLDB_PYTHON_EXT_SUFFIX}")
+  else()
+set(LIBLLDB_SYMLINK_OUTPUT_FILE "_lldb${LLDB_PYTHON_EXT_SUFFIX}")
+  endif()
   create_relative_symlink(${swig_target} ${LIBLLDB_SYMLINK_DEST}
   ${lldb_python_target_dir} 
${LIBLLDB_SYMLINK_OUTPUT_FILE})
 

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


[Lldb-commits] [lldb] [lldb/linux] Make sure the process continues running after a detach (PR #88494)

2024-04-17 Thread David Spickett via lldb-commits

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

LGTM

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


[Lldb-commits] [lldb] [lldb/linux] Make sure the process continues running after a detach (PR #88494)

2024-04-17 Thread David Spickett via lldb-commits

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