date:20240605

[Lldb-commits] [lldb] [lldb-dap] Added "port" property to vscode "attach" command. (PR #91570)

2024-06-05 Thread Pavel Labath via lldb-commits


labath wrote:

> > FWIW, I think this feature would be more useful and easier to implement if 
> > there was just a single setting called "connection url" or something, which 
> > accepted all of the usual lldb connection urls (unix-abstract-connect://, 
> > listen://, ...).
> > Since a simple tcp port connection is definitely going to be the most 
> > commonly used connection, you can, if you want, treat a simple number (or 
> > `:number`) as an alias to `connect://localhost:number`. lldb-server does 
> > something similar.
> > PS: I'm clicking request changes because I don't believe the test for the 
> > feature can go in in this form. I don't consider myself an owner for the 
> > rest, so you can consider my comments as suggestions.
> 
> I apologize for missing your earlier comment. To clarify. Using the URL 
> "connect://localhost:12345" or "connect://:12345" with the "connect" scheme 
> helps invoke the appropriate function to establish a connection. Pls ref: 
> https://github.com/llvm/llvm-project/blob/main/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp#L148
>  $gdb-remote number - would work from the command-line, because lldb is 
> taking care of adding "connect://".
> 
> Please let me know if my understating is incorrect.

Your understanding of the code is correct. What I am suggesting is basically to 
emulate this command line behavior in vscode. 

Basically, instead of two host+port settings, you could have one setting with a 
sufficiently vague name ("connection url", "remote address", ...) to cover the 
expanded scope. Then, you implementation could take the user-provided string 
and fill in the blanks:
- if the user types "1234", you'd convert it to `connect://localhost:1234`
- if the user types "my-host:1234", you'd convert it to 
`connect://localhost:1234`
- if the user types "unix-abstract-connect://..." (or any other scheme 
supported by lldb), you'd pass the value unmodified

The advantage of this is that you'd automatically support every connection 
protocol that lldb understands with almost no effort. If we stick to the 
current implementation, then if someone later needs to connect through unix 
sockets (or whatever), he would have to go back and add *another* setting to 
support connecting through that.

Now, I do see the appeal of having an explicit host/port fields (they're much 
easier to understand for the average user), which is why I'm not insisting on 
this approach. Nonetheless, I think this is an option worth considering, and I 
think we could avoid most of the confusion by carefully crafting the 
description of this field to guide the user to the most common scenarios.

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

[Lldb-commits] [lldb] [lldb-dap] Added "port" property to vscode "attach" command. (PR #91570)

2024-06-05 Thread Pavel Labath via lldb-commits



@@ -0,0 +1,202 @@
+"""
+Test lldb-dap "port" configuration to "attach" request
+"""
+
+
+import dap_server
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+from lldbsuite.test import lldbplatformutil
+import lldbdap_testcase
+import os
+import shutil
+import subprocess
+import tempfile
+import threading
+import sys
+import socket
+import select
+
+
+# A class representing a pipe for communicating with debug server.
+# This class includes menthods to open the pipe and read the port number from 
it.
+class Pipe(object):
+def __init__(self, prefix):
+self.name = os.path.join(prefix, "stub_port_number")
+os.mkfifo(self.name)
+self._fd = os.open(self.name, os.O_RDONLY | os.O_NONBLOCK)
+
+def finish_connection(self, timeout):
+pass
+
+def read(self, size, timeout):
+(readers, _, _) = select.select([self._fd], [], [], timeout)
+if self._fd not in readers:
+raise TimeoutError
+return os.read(self._fd, size)
+
+def close(self):
+os.close(self._fd)
+
+
+class TestDAP_attachByPortNum(lldbdap_testcase.DAPTestCaseBase):
+default_timeout = 20
+
+def set_and_hit_breakpoint(self, continueToExit=True):
+source = "main.c"
+main_source_path = os.path.join(os.getcwd(), source)
+breakpoint1_line = line_number(main_source_path, "// breakpoint 1")
+lines = [breakpoint1_line]
+# Set breakpoint in the thread function so we can step the threads
+breakpoint_ids = self.set_source_breakpoints(main_source_path, lines)
+self.assertEqual(
+len(breakpoint_ids), len(lines), "expect correct number of 
breakpoints"
+)
+self.continue_to_breakpoints(breakpoint_ids)
+if continueToExit:
+self.continue_to_exit()
+
+def get_debug_server_command_line_args(self):
+args = []
+if lldbplatformutil.getPlatform() == "linux":
+args = ["gdbserver"]
+elif lldbplatformutil.getPlatform() == "macosx":
+args = ["--listen"]
+if lldb.remote_platform:
+args += ["*:0"]
+else:
+args += ["localhost:0"]
+return args
+
+def get_debug_server_pipe(self):
+pipe = Pipe(self.getBuildDir())
+self.addTearDownHook(lambda: pipe.close())
+pipe.finish_connection(self.default_timeout)
+return pipe
+
+@skipIfWindows
+@skipIfNetBSD
+def test_by_port(self):
+"""
+Tests attaching to a process by port.
+"""
+self.build_and_create_debug_adaptor()
+program = self.getBuildArtifact("a.out")
+
+debug_server_tool = self.getBuiltinDebugServerTool()
+
+pipe = self.get_debug_server_pipe()
+args = self.get_debug_server_command_line_args()
+args += [program]
+args += ["--named-pipe", pipe.name]
+
+self.process = self.spawnSubprocess(
+debug_server_tool, args, install_remote=False
+)
+
+# Read the port number from the debug server pipe.
+port = pipe.read(10, self.default_timeout)
+# Trim null byte, convert to int
+port = int(port[:-1])
+self.assertIsNotNone(
+port, " Failed to read the port number from debug server pipe"
+)
+
+self.attach(program=program, gdbRemotePort=port, sourceInitFile=True)
+self.set_and_hit_breakpoint(continueToExit=True)
+self.process.terminate()
+
+@skipIfWindows
+@skipIfNetBSD
+def test_by_port_and_pid(self):
+"""
+Tests attaching to a process by process ID and port number.
+"""
+self.build_and_create_debug_adaptor()
+program = self.getBuildArtifact("a.out")
+
+debug_server_tool = self.getBuiltinDebugServerTool()
+pipe = self.get_debug_server_pipe()
+args = self.get_debug_server_command_line_args()
+args += [program]
+args += ["--named-pipe", pipe.name]
+
+self.process = self.spawnSubprocess(
+debug_server_tool, args, install_remote=False
+)
+
+# Read the port number from the debug server pipe.
+port = pipe.read(10, self.default_timeout)
+# Trim null byte, convert to int
+port = int(port[:-1])

labath wrote:

You don't actually need to launch lldb server just to check that this returns 
an error, right? Since noone will be actually connecting to the port (or 
attaching to the pid), you could just pass `1`, or something like that..

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

[Lldb-commits] [lldb] [lldb-dap] Added "port" property to vscode "attach" command. (PR #91570)

2024-06-05 Thread Pavel Labath via lldb-commits



@@ -0,0 +1,202 @@
+"""
+Test lldb-dap "port" configuration to "attach" request
+"""
+
+
+import dap_server
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+from lldbsuite.test import lldbplatformutil
+import lldbdap_testcase
+import os
+import shutil
+import subprocess
+import tempfile
+import threading
+import sys
+import socket
+import select
+
+
+# A class representing a pipe for communicating with debug server.
+# This class includes menthods to open the pipe and read the port number from 
it.
+class Pipe(object):
+def __init__(self, prefix):
+self.name = os.path.join(prefix, "stub_port_number")
+os.mkfifo(self.name)
+self._fd = os.open(self.name, os.O_RDONLY | os.O_NONBLOCK)
+
+def finish_connection(self, timeout):
+pass
+
+def read(self, size, timeout):
+(readers, _, _) = select.select([self._fd], [], [], timeout)
+if self._fd not in readers:
+raise TimeoutError
+return os.read(self._fd, size)
+
+def close(self):
+os.close(self._fd)

labath wrote:

Instead of copying the implementation, please move it somewhere where it can be 
accessed from both tests (`lldbgdbserverutils` is probabable a fine place for 
it).

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

[Lldb-commits] [lldb] [lldb-dap] Added "port" property to vscode "attach" command. (PR #91570)

2024-06-05 Thread Pavel Labath via lldb-commits



@@ -0,0 +1,202 @@
+"""
+Test lldb-dap "port" configuration to "attach" request
+"""
+
+
+import dap_server
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+from lldbsuite.test import lldbplatformutil
+import lldbdap_testcase
+import os
+import shutil
+import subprocess
+import tempfile
+import threading
+import sys
+import socket
+import select
+
+
+# A class representing a pipe for communicating with debug server.
+# This class includes menthods to open the pipe and read the port number from 
it.
+class Pipe(object):
+def __init__(self, prefix):
+self.name = os.path.join(prefix, "stub_port_number")
+os.mkfifo(self.name)
+self._fd = os.open(self.name, os.O_RDONLY | os.O_NONBLOCK)
+
+def finish_connection(self, timeout):
+pass
+
+def read(self, size, timeout):
+(readers, _, _) = select.select([self._fd], [], [], timeout)
+if self._fd not in readers:
+raise TimeoutError
+return os.read(self._fd, size)
+
+def close(self):
+os.close(self._fd)
+
+
+class TestDAP_attachByPortNum(lldbdap_testcase.DAPTestCaseBase):
+default_timeout = 20
+
+def set_and_hit_breakpoint(self, continueToExit=True):
+source = "main.c"
+main_source_path = os.path.join(os.getcwd(), source)
+breakpoint1_line = line_number(main_source_path, "// breakpoint 1")
+lines = [breakpoint1_line]
+# Set breakpoint in the thread function so we can step the threads
+breakpoint_ids = self.set_source_breakpoints(main_source_path, lines)
+self.assertEqual(
+len(breakpoint_ids), len(lines), "expect correct number of 
breakpoints"
+)
+self.continue_to_breakpoints(breakpoint_ids)
+if continueToExit:
+self.continue_to_exit()
+
+def get_debug_server_command_line_args(self):
+args = []
+if lldbplatformutil.getPlatform() == "linux":
+args = ["gdbserver"]
+elif lldbplatformutil.getPlatform() == "macosx":
+args = ["--listen"]
+if lldb.remote_platform:
+args += ["*:0"]
+else:
+args += ["localhost:0"]
+return args
+
+def get_debug_server_pipe(self):
+pipe = Pipe(self.getBuildDir())
+self.addTearDownHook(lambda: pipe.close())
+pipe.finish_connection(self.default_timeout)
+return pipe
+
+@skipIfWindows
+@skipIfNetBSD
+def test_by_port(self):
+"""
+Tests attaching to a process by port.
+"""
+self.build_and_create_debug_adaptor()
+program = self.getBuildArtifact("a.out")
+
+debug_server_tool = self.getBuiltinDebugServerTool()
+
+pipe = self.get_debug_server_pipe()
+args = self.get_debug_server_command_line_args()
+args += [program]
+args += ["--named-pipe", pipe.name]
+
+self.process = self.spawnSubprocess(
+debug_server_tool, args, install_remote=False
+)
+
+# Read the port number from the debug server pipe.
+port = pipe.read(10, self.default_timeout)
+# Trim null byte, convert to int
+port = int(port[:-1])
+self.assertIsNotNone(
+port, " Failed to read the port number from debug server pipe"
+)
+
+self.attach(program=program, gdbRemotePort=port, sourceInitFile=True)
+self.set_and_hit_breakpoint(continueToExit=True)
+self.process.terminate()
+
+@skipIfWindows
+@skipIfNetBSD
+def test_by_port_and_pid(self):
+"""
+Tests attaching to a process by process ID and port number.
+"""
+self.build_and_create_debug_adaptor()
+program = self.getBuildArtifact("a.out")
+
+debug_server_tool = self.getBuiltinDebugServerTool()
+pipe = self.get_debug_server_pipe()
+args = self.get_debug_server_command_line_args()
+args += [program]
+args += ["--named-pipe", pipe.name]
+
+self.process = self.spawnSubprocess(
+debug_server_tool, args, install_remote=False
+)
+
+# Read the port number from the debug server pipe.
+port = pipe.read(10, self.default_timeout)
+# Trim null byte, convert to int
+port = int(port[:-1])
+self.assertIsNotNone(
+port, " Failed to read the port number from debug server pipe"
+)
+
+pid = self.process.pid
+
+response = self.attach(
+program=program,
+pid=pid,
+gdbRemotePort=port,
+sourceInitFile=True,
+expectFailure=True,
+)
+if not (response and response["success"]):
+self.assertFalse(
+response["success"], "The user can't specify both pid and port"
+)
+self.process.terminate()
+
+@skipIfWindows
+@skipIfNetBSD
+def test_by_i

[Lldb-commits] [lldb] [lldb][RISCV] Add RegisterContextPOSIXCore for RISC-V 64 (PR #93297)

2024-06-05 Thread Alexey Merzlyakov via lldb-commits


https://github.com/AlexeyMerzlyakov updated 
https://github.com/llvm/llvm-project/pull/93297

>From d30c3b7017bd9f4b9f442ee728d7e3d7847c60cf Mon Sep 17 00:00:00 2001
From: Alexey Merzlyakov 
Date: Fri, 24 May 2024 11:54:16 +0300
Subject: [PATCH 1/4] Add RegisterContextPOSIXCore for RISC-V 64

Fix GetRegisterSetCount() method name misprint for RegisterContextPOSIX_riscv64
---
 .../Utility/RegisterContextPOSIX_riscv64.cpp  |  2 +-
 .../Plugins/Process/elf-core/CMakeLists.txt   |  1 +
 .../RegisterContextPOSIXCore_riscv64.cpp  | 84 +++
 .../RegisterContextPOSIXCore_riscv64.h| 60 +
 .../Process/elf-core/ThreadElfCore.cpp|  8 +-
 5 files changed, 153 insertions(+), 2 deletions(-)
 create mode 100644 
lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_riscv64.cpp
 create mode 100644 
lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_riscv64.h

diff --git 
a/lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_riscv64.cpp 
b/lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_riscv64.cpp
index 1834a94dc0260..035ce00e11626 100644
--- a/lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_riscv64.cpp
+++ b/lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_riscv64.cpp
@@ -58,7 +58,7 @@ RegisterContextPOSIX_riscv64::GetRegisterInfoAtIndex(size_t 
reg) {
 }
 
 size_t RegisterContextPOSIX_riscv64::GetRegisterSetCount() {
-  return m_register_info_up->GetRegisterCount();
+  return m_register_info_up->GetRegisterSetCount();
 }
 
 const lldb_private::RegisterSet *
diff --git a/lldb/source/Plugins/Process/elf-core/CMakeLists.txt 
b/lldb/source/Plugins/Process/elf-core/CMakeLists.txt
index 8ddc671e3ae66..72925c835b5c8 100644
--- a/lldb/source/Plugins/Process/elf-core/CMakeLists.txt
+++ b/lldb/source/Plugins/Process/elf-core/CMakeLists.txt
@@ -9,6 +9,7 @@ add_lldb_library(lldbPluginProcessElfCore PLUGIN
   RegisterContextPOSIXCore_ppc64le.cpp
   RegisterContextPOSIXCore_s390x.cpp
   RegisterContextPOSIXCore_x86_64.cpp
+  RegisterContextPOSIXCore_riscv64.cpp
   RegisterUtilities.cpp
 
   LINK_LIBS
diff --git 
a/lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_riscv64.cpp 
b/lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_riscv64.cpp
new file mode 100644
index 0..2202be4d38082
--- /dev/null
+++ b/lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_riscv64.cpp
@@ -0,0 +1,84 @@
+//===-- RegisterContextPOSIXCore_riscv64.cpp 
--===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "RegisterContextPOSIXCore_riscv64.h"
+
+#include "lldb/Utility/DataBufferHeap.h"
+
+using namespace lldb_private;
+
+std::unique_ptr
+RegisterContextCorePOSIX_riscv64::Create(
+lldb_private::Thread &thread, const lldb_private::ArchSpec &arch,
+const lldb_private::DataExtractor &gpregset,
+llvm::ArrayRef notes) {
+  Flags flags = 0;
+
+  auto register_info_up =
+  std::make_unique(arch, flags);
+  return std::unique_ptr(
+  new RegisterContextCorePOSIX_riscv64(thread, std::move(register_info_up),
+   gpregset, notes));
+}
+
+RegisterContextCorePOSIX_riscv64::RegisterContextCorePOSIX_riscv64(
+Thread &thread, std::unique_ptr register_info,
+const DataExtractor &gpregset, llvm::ArrayRef notes)
+: RegisterContextPOSIX_riscv64(thread, std::move(register_info)) {
+
+  m_gpr_buffer = std::make_shared(gpregset.GetDataStart(),
+  gpregset.GetByteSize());
+  m_gpr.SetData(m_gpr_buffer);
+  m_gpr.SetByteOrder(gpregset.GetByteOrder());
+
+  ArchSpec arch = m_register_info_up->GetTargetArchitecture();
+  DataExtractor fpregset = getRegset(notes, arch.GetTriple(), FPR_Desc);
+  m_fpr_buffer = std::make_shared(fpregset.GetDataStart(),
+  fpregset.GetByteSize());
+  m_fpr.SetData(m_fpr_buffer);
+  m_fpr.SetByteOrder(fpregset.GetByteOrder());
+}
+
+RegisterContextCorePOSIX_riscv64::~RegisterContextCorePOSIX_riscv64() = 
default;
+
+bool RegisterContextCorePOSIX_riscv64::ReadGPR() { return true; }
+
+bool RegisterContextCorePOSIX_riscv64::ReadFPR() { return true; }
+
+bool RegisterContextCorePOSIX_riscv64::WriteGPR() {
+  assert(0);
+  return false;
+}
+
+bool RegisterContextCorePOSIX_riscv64::WriteFPR() {
+  assert(0);
+  return false;
+}
+
+bool RegisterContextCorePOSIX_riscv64::ReadRegister(
+const RegisterInfo *reg_info, RegisterValue &value) {
+  const uint8_t *src = nullptr;
+  lldb::offset_t offset = reg_info->byte_offset;
+
+  if (IsGPR(reg_info->kinds[lldb::eRegisterKindLLDB])) {
+src = m_gpr.GetDataStart();
+  } else { // IsFPR
+src = m_fpr.GetDataStart();
+offs

[Lldb-commits] [lldb] [llvm] [lldb] Return an llvm::Expected from DWARFExpression::Evaluate (NFCI) (PR #94420)

2024-06-05 Thread David Spickett via lldb-commits



@@ -2244,30 +2108,26 @@ bool DWARFExpression::Evaluate(
 const uint64_t piece_bit_offset = opcodes.GetULEB128(&offset);
 switch (stack.back().GetValueType()) {
 case Value::ValueType::Invalid:
-  return false;
+  return llvm::createStringError(
+  "unable to extract bit value from invalid value.");
 case Value::ValueType::Scalar: {
   if (!stack.back().GetScalar().ExtractBitfield(piece_bit_size,
 piece_bit_offset)) {
-if (error_ptr)
-  error_ptr->SetErrorStringWithFormat(
-  "unable to extract %" PRIu64 " bit value with %" PRIu64
-  " bit offset from a %" PRIu64 " bit scalar value.",
-  piece_bit_size, piece_bit_offset,
-  (uint64_t)(stack.back().GetScalar().GetByteSize() * 8));
-return false;
+return llvm::createStringError(
+"unable to extract %" PRIu64 " bit value with %" PRIu64
+" bit offset from a %" PRIu64 " bit scalar value.",
+piece_bit_size, piece_bit_offset,
+(uint64_t)(stack.back().GetScalar().GetByteSize() * 8));
   }
 } break;
 
 case Value::ValueType::FileAddress:
 case Value::ValueType::LoadAddress:
 case Value::ValueType::HostAddress:
-  if (error_ptr) {
-error_ptr->SetErrorStringWithFormat(
-"unable to extract DW_OP_bit_piece(bit_size = %" PRIu64
-", bit_offset = %" PRIu64 ") from an address value.",
-piece_bit_size, piece_bit_offset);
-  }
-  return false;
+  return llvm::createStringError(

DavidSpickett wrote:

Which overload is this calling?

The ones in 
https://github.com/llvm/llvm-project/blob/775ebc1d710517ddb886ee8aec504e33bed38f52/llvm/include/llvm/Support/Error.h#L1256
 that take a format all require an error code.

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

[Lldb-commits] [lldb] [llvm] [lldb] Return an llvm::Expected from DWARFExpression::Evaluate (NFCI) (PR #94420)

2024-06-05 Thread David Spickett via lldb-commits



@@ -2042,7 +2043,7 @@ bool RegisterContextUnwind::ReadFrameAddress(
   return true;
 }
 UnwindLogMsg("Failed to set CFA value via DWARF expression: %s",
- error.AsCString());
+ llvm::toString(result.takeError()).c_str());

DavidSpickett wrote:

Does `takeError` disarm the result? I am wondering if this will assert that no 
one has handled the error here.

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

[Lldb-commits] [lldb] [llvm] [lldb] Return an llvm::Expected from DWARFExpression::Evaluate (NFCI) (PR #94420)

2024-06-05 Thread David Spickett via lldb-commits



@@ -572,28 +572,32 @@ TypeSP DWARFASTParserClang::ParseTypeFromDWARF(const 
SymbolContext &sc,
 static std::optional
 ExtractDataMemberLocation(DWARFDIE const &die, DWARFFormValue const 
&form_value,
   ModuleSP module_sp) {
+  Log *log = GetLog(DWARFLog::TypeCompletion | DWARFLog::Lookups);
+
   // With DWARF 3 and later, if the value is an integer constant,
   // this form value is the offset in bytes from the beginning of
   // the containing entity.
   if (!form_value.BlockData())
 return form_value.Unsigned();
 
   Value initialValue(0);
-  Value memberOffset(0);
   const DWARFDataExtractor &debug_info_data = die.GetData();
   uint32_t block_length = form_value.Unsigned();
   uint32_t block_offset =
   form_value.BlockData() - debug_info_data.GetDataStart();
-  if (!DWARFExpression::Evaluate(
-  nullptr, // ExecutionContext *
-  nullptr, // RegisterContext *
-  module_sp, DataExtractor(debug_info_data, block_offset, 
block_length),
-  die.GetCU(), eRegisterKindDWARF, &initialValue, nullptr, 
memberOffset,
-  nullptr)) {
+
+  llvm::Expected memberOffset = DWARFExpression::Evaluate(
+  nullptr, // ExecutionContext *
+  nullptr, // RegisterContext *

DavidSpickett wrote:

Maybe these comments should be the `/*foo=*/` form that (so I'm told) clang 
tooling can verify.

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

[Lldb-commits] [lldb] [llvm] [lldb] Return an llvm::Expected from DWARFExpression::Evaluate (NFCI) (PR #94420)

2024-06-05 Thread David Spickett via lldb-commits



@@ -1277,6 +1277,11 @@ inline Error createStringError(const Twine &S) {
   return createStringError(llvm::inconvertibleErrorCode(), S);
 }
 
+template 
+inline Error createStringError(char const *Fmt, const Ts &...Vals) {
+  return createStringError(llvm::inconvertibleErrorCode(), Fmt, Vals...);
+}
+

DavidSpickett wrote:

Ah here it is :)

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

[Lldb-commits] [lldb] [llvm] [lldb] Return an llvm::Expected from DWARFExpression::Evaluate (NFCI) (PR #94420)

2024-06-05 Thread David Spickett via lldb-commits



@@ -2244,30 +2108,26 @@ bool DWARFExpression::Evaluate(
 const uint64_t piece_bit_offset = opcodes.GetULEB128(&offset);
 switch (stack.back().GetValueType()) {
 case Value::ValueType::Invalid:
-  return false;
+  return llvm::createStringError(
+  "unable to extract bit value from invalid value.");
 case Value::ValueType::Scalar: {
   if (!stack.back().GetScalar().ExtractBitfield(piece_bit_size,
 piece_bit_offset)) {
-if (error_ptr)
-  error_ptr->SetErrorStringWithFormat(
-  "unable to extract %" PRIu64 " bit value with %" PRIu64
-  " bit offset from a %" PRIu64 " bit scalar value.",
-  piece_bit_size, piece_bit_offset,
-  (uint64_t)(stack.back().GetScalar().GetByteSize() * 8));
-return false;
+return llvm::createStringError(
+"unable to extract %" PRIu64 " bit value with %" PRIu64
+" bit offset from a %" PRIu64 " bit scalar value.",
+piece_bit_size, piece_bit_offset,
+(uint64_t)(stack.back().GetScalar().GetByteSize() * 8));
   }
 } break;
 
 case Value::ValueType::FileAddress:
 case Value::ValueType::LoadAddress:
 case Value::ValueType::HostAddress:
-  if (error_ptr) {
-error_ptr->SetErrorStringWithFormat(
-"unable to extract DW_OP_bit_piece(bit_size = %" PRIu64
-", bit_offset = %" PRIu64 ") from an address value.",
-piece_bit_size, piece_bit_offset);
-  }
-  return false;
+  return llvm::createStringError(

DavidSpickett wrote:

I see you added it later.

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

[Lldb-commits] [lldb] [lldb][RISCV] Add RegisterContextPOSIXCore for RISC-V 64 (PR #93297)

2024-06-05 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 
e3ed9e30b7b7102daf37f9464efc06b3d2e1a160...183f76dfb73821640cfdf19901703d0a8ff447ff
 lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
``





View the diff from darker here.


``diff
--- TestLinuxCore.py2024-06-05 08:38:06.00 +
+++ TestLinuxCore.py2024-06-05 08:55:41.407878 +
@@ -677,15 +677,44 @@
 values["t5"] = "0x0020"
 values["t6"] = "0x002ae919f1b0"
 values["zero"] = "0x0"
 values["fcsr"] = "0x"
 
-fpr_names = {"ft0", "ft1", "ft2", "ft3", "ft4", "ft5", "ft6", "ft7",
- "ft8", "ft9", "ft10", "ft11",
- "fa0", "fa1", "fa2", "fa3", "fa4", "fa5", "fa6", "fa7",
- "fs0", "fs1", "fs2", "fs3", "fs4", "fs5", "fs6", "fs7",
- "fs8", "fs9", "fs10", "fs11"}
+fpr_names = {
+"ft0",
+"ft1",
+"ft2",
+"ft3",
+"ft4",
+"ft5",
+"ft6",
+"ft7",
+"ft8",
+"ft9",
+"ft10",
+"ft11",
+"fa0",
+"fa1",
+"fa2",
+"fa3",
+"fa4",
+"fa5",
+"fa6",
+"fa7",
+"fs0",
+"fs1",
+"fs2",
+"fs3",
+"fs4",
+"fs5",
+"fs6",
+"fs7",
+"fs8",
+"fs9",
+"fs10",
+"fs11",
+}
 fpr_value = "0x"
 
 for regname, value in values.items():
 self.expect(
 "register read {}".format(regname),

``




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

[Lldb-commits] [lldb] [lldb][RISCV] Add RegisterContextPOSIXCore for RISC-V 64 (PR #93297)

2024-06-05 Thread David Spickett via lldb-commits


DavidSpickett wrote:

Not sure I agree with the formatter here but we must bow to it regardless :)

Please fix the formatting and then I'll merge this. Thanks for the 
contribution! 

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

[Lldb-commits] [lldb] [lldb][RISCV] Add RegisterContextPOSIXCore for RISC-V 64 (PR #93297)

2024-06-05 Thread Alexey Merzlyakov via lldb-commits


https://github.com/AlexeyMerzlyakov updated 
https://github.com/llvm/llvm-project/pull/93297

>From d30c3b7017bd9f4b9f442ee728d7e3d7847c60cf Mon Sep 17 00:00:00 2001
From: Alexey Merzlyakov 
Date: Fri, 24 May 2024 11:54:16 +0300
Subject: [PATCH 1/5] Add RegisterContextPOSIXCore for RISC-V 64

Fix GetRegisterSetCount() method name misprint for RegisterContextPOSIX_riscv64
---
 .../Utility/RegisterContextPOSIX_riscv64.cpp  |  2 +-
 .../Plugins/Process/elf-core/CMakeLists.txt   |  1 +
 .../RegisterContextPOSIXCore_riscv64.cpp  | 84 +++
 .../RegisterContextPOSIXCore_riscv64.h| 60 +
 .../Process/elf-core/ThreadElfCore.cpp|  8 +-
 5 files changed, 153 insertions(+), 2 deletions(-)
 create mode 100644 
lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_riscv64.cpp
 create mode 100644 
lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_riscv64.h

diff --git 
a/lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_riscv64.cpp 
b/lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_riscv64.cpp
index 1834a94dc0260..035ce00e11626 100644
--- a/lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_riscv64.cpp
+++ b/lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_riscv64.cpp
@@ -58,7 +58,7 @@ RegisterContextPOSIX_riscv64::GetRegisterInfoAtIndex(size_t 
reg) {
 }
 
 size_t RegisterContextPOSIX_riscv64::GetRegisterSetCount() {
-  return m_register_info_up->GetRegisterCount();
+  return m_register_info_up->GetRegisterSetCount();
 }
 
 const lldb_private::RegisterSet *
diff --git a/lldb/source/Plugins/Process/elf-core/CMakeLists.txt 
b/lldb/source/Plugins/Process/elf-core/CMakeLists.txt
index 8ddc671e3ae66..72925c835b5c8 100644
--- a/lldb/source/Plugins/Process/elf-core/CMakeLists.txt
+++ b/lldb/source/Plugins/Process/elf-core/CMakeLists.txt
@@ -9,6 +9,7 @@ add_lldb_library(lldbPluginProcessElfCore PLUGIN
   RegisterContextPOSIXCore_ppc64le.cpp
   RegisterContextPOSIXCore_s390x.cpp
   RegisterContextPOSIXCore_x86_64.cpp
+  RegisterContextPOSIXCore_riscv64.cpp
   RegisterUtilities.cpp
 
   LINK_LIBS
diff --git 
a/lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_riscv64.cpp 
b/lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_riscv64.cpp
new file mode 100644
index 0..2202be4d38082
--- /dev/null
+++ b/lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_riscv64.cpp
@@ -0,0 +1,84 @@
+//===-- RegisterContextPOSIXCore_riscv64.cpp 
--===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "RegisterContextPOSIXCore_riscv64.h"
+
+#include "lldb/Utility/DataBufferHeap.h"
+
+using namespace lldb_private;
+
+std::unique_ptr
+RegisterContextCorePOSIX_riscv64::Create(
+lldb_private::Thread &thread, const lldb_private::ArchSpec &arch,
+const lldb_private::DataExtractor &gpregset,
+llvm::ArrayRef notes) {
+  Flags flags = 0;
+
+  auto register_info_up =
+  std::make_unique(arch, flags);
+  return std::unique_ptr(
+  new RegisterContextCorePOSIX_riscv64(thread, std::move(register_info_up),
+   gpregset, notes));
+}
+
+RegisterContextCorePOSIX_riscv64::RegisterContextCorePOSIX_riscv64(
+Thread &thread, std::unique_ptr register_info,
+const DataExtractor &gpregset, llvm::ArrayRef notes)
+: RegisterContextPOSIX_riscv64(thread, std::move(register_info)) {
+
+  m_gpr_buffer = std::make_shared(gpregset.GetDataStart(),
+  gpregset.GetByteSize());
+  m_gpr.SetData(m_gpr_buffer);
+  m_gpr.SetByteOrder(gpregset.GetByteOrder());
+
+  ArchSpec arch = m_register_info_up->GetTargetArchitecture();
+  DataExtractor fpregset = getRegset(notes, arch.GetTriple(), FPR_Desc);
+  m_fpr_buffer = std::make_shared(fpregset.GetDataStart(),
+  fpregset.GetByteSize());
+  m_fpr.SetData(m_fpr_buffer);
+  m_fpr.SetByteOrder(fpregset.GetByteOrder());
+}
+
+RegisterContextCorePOSIX_riscv64::~RegisterContextCorePOSIX_riscv64() = 
default;
+
+bool RegisterContextCorePOSIX_riscv64::ReadGPR() { return true; }
+
+bool RegisterContextCorePOSIX_riscv64::ReadFPR() { return true; }
+
+bool RegisterContextCorePOSIX_riscv64::WriteGPR() {
+  assert(0);
+  return false;
+}
+
+bool RegisterContextCorePOSIX_riscv64::WriteFPR() {
+  assert(0);
+  return false;
+}
+
+bool RegisterContextCorePOSIX_riscv64::ReadRegister(
+const RegisterInfo *reg_info, RegisterValue &value) {
+  const uint8_t *src = nullptr;
+  lldb::offset_t offset = reg_info->byte_offset;
+
+  if (IsGPR(reg_info->kinds[lldb::eRegisterKindLLDB])) {
+src = m_gpr.GetDataStart();
+  } else { // IsFPR
+src = m_fpr.GetDataStart();
+offs

[Lldb-commits] [lldb] [lldb][RISCV] Add RegisterContextPOSIXCore for RISC-V 64 (PR #93297)

2024-06-05 Thread Alexey Merzlyakov via lldb-commits


AlexeyMerzlyakov wrote:

Oops, I've checked it with flake8, but did not have an idea about the darker 
tool. Fixed in 
[latest](https://github.com/llvm/llvm-project/pull/93297/commits/041f5729920ed5f1b22745970b488d12438af1ba)
 commit. Thank you!

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

[Lldb-commits] [lldb] [lldb][RISCV] Add RegisterContextPOSIXCore for RISC-V 64 (PR #93297)

2024-06-05 Thread David Spickett via lldb-commits


DavidSpickett wrote:

Yeah we settled on https://github.com/psf/black as the formatter, but that 
itself didn't have a way to format the contents of a commit. So "darker" is 
black but it can do that.

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

[Lldb-commits] [lldb] d3a9043 - [lldb][RISCV] Add RegisterContextPOSIXCore for RISC-V 64 (#93297)

2024-06-05 Thread via lldb-commits


Author: Alexey Merzlyakov
Date: 2024-06-05T10:14:48+01:00
New Revision: d3a9043ec2ee7ea278be4f3c86823512e44d01bf

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

LOG: [lldb][RISCV] Add RegisterContextPOSIXCore for RISC-V 64 (#93297)

The PR adds the support of CoreDump debugging for RISC-V 64. It
implements new `RegisterContextCorePOSIX_riscv64` class.

Also, the contribution fixes `GetRegisterCount()` ->
`GetRegisterSetCount()` misprint in
`RegisterContextPOSIX_riscv64::GetRegisterSetCount()` method, which
leaded to `set && "Register set should be valid."` assertion during
`register info aX` command call.

The patch was tested (on coredumps generated for simple Integer/FP
calculation code) for _cross x86_64 -> RISCV_ and _native RISCV_ LLDB
builds. There were performed basic LLDB functionality tests, such as:

 - CoreDump file load
 - Backtrace / frames
 - GP/FP registers read/info/list
 - Basic switch between threads
 - Disassembler code
 - Memory regions read / display

Added: 
lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_riscv64.cpp
lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_riscv64.h
lldb/test/API/functionalities/postmortem/elf-core/linux-riscv64.core
lldb/test/API/functionalities/postmortem/elf-core/linux-riscv64.out

Modified: 
lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_riscv64.cpp
lldb/source/Plugins/Process/elf-core/CMakeLists.txt
lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp
lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py

Removed: 




diff  --git 
a/lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_riscv64.cpp 
b/lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_riscv64.cpp
index 1834a94dc0260..035ce00e11626 100644
--- a/lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_riscv64.cpp
+++ b/lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_riscv64.cpp
@@ -58,7 +58,7 @@ RegisterContextPOSIX_riscv64::GetRegisterInfoAtIndex(size_t 
reg) {
 }
 
 size_t RegisterContextPOSIX_riscv64::GetRegisterSetCount() {
-  return m_register_info_up->GetRegisterCount();
+  return m_register_info_up->GetRegisterSetCount();
 }
 
 const lldb_private::RegisterSet *

diff  --git a/lldb/source/Plugins/Process/elf-core/CMakeLists.txt 
b/lldb/source/Plugins/Process/elf-core/CMakeLists.txt
index 8ddc671e3ae66..72925c835b5c8 100644
--- a/lldb/source/Plugins/Process/elf-core/CMakeLists.txt
+++ b/lldb/source/Plugins/Process/elf-core/CMakeLists.txt
@@ -9,6 +9,7 @@ add_lldb_library(lldbPluginProcessElfCore PLUGIN
   RegisterContextPOSIXCore_ppc64le.cpp
   RegisterContextPOSIXCore_s390x.cpp
   RegisterContextPOSIXCore_x86_64.cpp
+  RegisterContextPOSIXCore_riscv64.cpp
   RegisterUtilities.cpp
 
   LINK_LIBS

diff  --git 
a/lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_riscv64.cpp 
b/lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_riscv64.cpp
new file mode 100644
index 0..5ba18cdb9889a
--- /dev/null
+++ b/lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_riscv64.cpp
@@ -0,0 +1,82 @@
+//===-- RegisterContextPOSIXCore_riscv64.cpp 
--===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "RegisterContextPOSIXCore_riscv64.h"
+
+#include "lldb/Utility/DataBufferHeap.h"
+
+using namespace lldb_private;
+
+std::unique_ptr
+RegisterContextCorePOSIX_riscv64::Create(Thread &thread, const ArchSpec &arch,
+ const DataExtractor &gpregset,
+ llvm::ArrayRef notes) {
+  return std::unique_ptr(
+  new RegisterContextCorePOSIX_riscv64(
+  thread, std::make_unique(arch, Flags()),
+  gpregset, notes));
+}
+
+RegisterContextCorePOSIX_riscv64::RegisterContextCorePOSIX_riscv64(
+Thread &thread, std::unique_ptr register_info,
+const DataExtractor &gpregset, llvm::ArrayRef notes)
+: RegisterContextPOSIX_riscv64(thread, std::move(register_info)) {
+
+  m_gpr_buffer = std::make_shared(gpregset.GetDataStart(),
+  gpregset.GetByteSize());
+  m_gpr.SetData(m_gpr_buffer);
+  m_gpr.SetByteOrder(gpregset.GetByteOrder());
+
+  ArchSpec arch = m_register_info_up->GetTargetArchitecture();
+  DataExtractor fpregset = getRegset(notes, arch.GetTriple(), FPR_Desc);
+  m_fpr_buffer = std::make_shared(fpregset.GetDataStart(),
+  fpregset.GetByteSize());
+  m_fpr.SetData(m_fpr_buf

[Lldb-commits] [lldb] [lldb][RISCV] Add RegisterContextPOSIXCore for RISC-V 64 (PR #93297)

2024-06-05 Thread David Spickett via lldb-commits


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

[Lldb-commits] [lldb] [lldb][RISCV] Add RegisterContextPOSIXCore for RISC-V 64 (PR #93297)

2024-06-05 Thread via lldb-commits


github-actions[bot] wrote:



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

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

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

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

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

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


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

[Lldb-commits] [lldb] [lldb] Split ValueObject::CreateChildAtIndex into two functions (PR #94455)

2024-06-05 Thread Pavel Labath via lldb-commits


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

The the function is doing two fairly different things, depending on how it is 
called. While this allows for some code reuse, it also makes it hard to 
override it correctly. Possibly for this reason ValueObjectSynthetic overerides 
GetChildAtIndex instead, which forces it to reimplement some of its 
functionality, most notably caching of generated children.

Splitting this up makes it easier to move the caching to a common place (and 
hopefully makes the code easier to follow in general).

>From 130389992ae560366893637f70132d75e31aa04b Mon Sep 17 00:00:00 2001
From: Pavel Labath 
Date: Wed, 5 Jun 2024 10:21:10 +
Subject: [PATCH] [lldb] Split ValueObject::CreateChildAtIndex into two
 functions

The the function is doing two fairly different things, depending on how
it is called. While this allows for some code reuse, it also makes it
hard to override it correctly. Possibly for this reason
ValueObjectSynthetic overerides GetChildAtIndex instead, which forces it
to reimplement some of its functionality, most notably caching of
generated children.

Splitting this up makes it easier to move the caching to a common place
(and hopefully makes the code easier to follow in general).
---
 lldb/include/lldb/Core/ValueObject.h  |   9 +-
 .../lldb/Core/ValueObjectConstResult.h|  10 +-
 .../lldb/Core/ValueObjectConstResultCast.h|  10 +-
 .../lldb/Core/ValueObjectConstResultChild.h   |  10 +-
 .../lldb/Core/ValueObjectConstResultImpl.h|   4 +-
 lldb/include/lldb/Core/ValueObjectRegister.h  |   8 +-
 lldb/include/lldb/Core/ValueObjectVTable.h|   8 +-
 lldb/source/Core/ValueObject.cpp  |  85 -
 lldb/source/Core/ValueObjectConstResult.cpp   |   6 -
 .../Core/ValueObjectConstResultCast.cpp   |   6 -
 .../Core/ValueObjectConstResultChild.cpp  |   6 -
 .../Core/ValueObjectConstResultImpl.cpp   | 115 --
 lldb/source/Core/ValueObjectRegister.cpp  |  14 +--
 lldb/source/Core/ValueObjectVTable.cpp|   6 +-
 14 files changed, 175 insertions(+), 122 deletions(-)

diff --git a/lldb/include/lldb/Core/ValueObject.h 
b/lldb/include/lldb/Core/ValueObject.h
index e7e35e2b2bffc..1e8c7c9c00536 100644
--- a/lldb/include/lldb/Core/ValueObject.h
+++ b/lldb/include/lldb/Core/ValueObject.h
@@ -959,9 +959,12 @@ class ValueObject {
   /// Should only be called by ValueObject::GetChildAtIndex().
   ///
   /// \return A ValueObject managed by this ValueObject's manager.
-  virtual ValueObject *CreateChildAtIndex(size_t idx,
-  bool synthetic_array_member,
-  int32_t synthetic_index);
+  virtual ValueObject *CreateChildAtIndex(size_t idx);
+
+  /// Should only be called by ValueObject::GetSyntheticArrayMember().
+  ///
+  /// \return A ValueObject managed by this ValueObject's manager.
+  virtual ValueObject *CreateSyntheticArrayMember(size_t idx);
 
   /// Should only be called by ValueObject::GetNumChildren().
   virtual llvm::Expected
diff --git a/lldb/include/lldb/Core/ValueObjectConstResult.h 
b/lldb/include/lldb/Core/ValueObjectConstResult.h
index 37dc0867f26c9..d3b3362bd0e9e 100644
--- a/lldb/include/lldb/Core/ValueObjectConstResult.h
+++ b/lldb/include/lldb/Core/ValueObjectConstResult.h
@@ -79,9 +79,6 @@ class ValueObjectConstResult : public ValueObject {
 
   lldb::ValueObjectSP Dereference(Status &error) override;
 
-  ValueObject *CreateChildAtIndex(size_t idx, bool synthetic_array_member,
-  int32_t synthetic_index) override;
-
   lldb::ValueObjectSP GetSyntheticChildAtOffset(
   uint32_t offset, const CompilerType &type, bool can_create,
   ConstString name_const_str = ConstString()) override;
@@ -151,6 +148,13 @@ class ValueObjectConstResult : public ValueObject {
   ValueObjectConstResult(ExecutionContextScope *exe_scope,
  ValueObjectManager &manager, const Status &error);
 
+  ValueObject *CreateChildAtIndex(size_t idx) override {
+return m_impl.CreateChildAtIndex(idx);
+  }
+  ValueObject *CreateSyntheticArrayMember(size_t idx) override {
+return m_impl.CreateSyntheticArrayMember(idx);
+  }
+
   ValueObjectConstResult(const ValueObjectConstResult &) = delete;
   const ValueObjectConstResult &
   operator=(const ValueObjectConstResult &) = delete;
diff --git a/lldb/include/lldb/Core/ValueObjectConstResultCast.h 
b/lldb/include/lldb/Core/ValueObjectConstResultCast.h
index efcbe0dc6a0bd..911a08363b393 100644
--- a/lldb/include/lldb/Core/ValueObjectConstResultCast.h
+++ b/lldb/include/lldb/Core/ValueObjectConstResultCast.h
@@ -35,9 +35,6 @@ class ValueObjectConstResultCast : public ValueObjectCast {
 
   lldb::ValueObjectSP Dereference(Status &error) override;
 
-  ValueObject *CreateChildAtIndex(size_t idx, bool synthetic_array_member,
-  int32_t synthetic_index) override;
-

[Lldb-commits] [lldb] [lldb] Split ValueObject::CreateChildAtIndex into two functions (PR #94455)

2024-06-05 Thread Pavel Labath via lldb-commits


https://github.com/labath updated 
https://github.com/llvm/llvm-project/pull/94455

>From d3d666886d6397bb031a3ad88f147fb9b1e2b3ed Mon Sep 17 00:00:00 2001
From: Pavel Labath 
Date: Wed, 5 Jun 2024 10:21:10 +
Subject: [PATCH] [lldb] Split ValueObject::CreateChildAtIndex into two
 functions

The the function is doing two fairly different things, depending on how
it is called. While this allows for some code reuse, it also makes it
hard to override it correctly. Possibly for this reason
ValueObjectSynthetic overerides GetChildAtIndex instead, which forces it
to reimplement some of its functionality, most notably caching of
generated children.

Splitting this up makes it easier to move the caching to a common place
(and hopefully makes the code easier to follow in general).
---
 lldb/include/lldb/Core/ValueObject.h  |   9 +-
 .../lldb/Core/ValueObjectConstResult.h|  10 +-
 .../lldb/Core/ValueObjectConstResultCast.h|  10 +-
 .../lldb/Core/ValueObjectConstResultChild.h   |  10 +-
 .../lldb/Core/ValueObjectConstResultImpl.h|   4 +-
 lldb/include/lldb/Core/ValueObjectRegister.h  |   8 +-
 lldb/include/lldb/Core/ValueObjectVTable.h|   8 +-
 lldb/source/Core/ValueObject.cpp  |  91 --
 lldb/source/Core/ValueObjectConstResult.cpp   |   6 -
 .../Core/ValueObjectConstResultCast.cpp   |   6 -
 .../Core/ValueObjectConstResultChild.cpp  |   6 -
 .../Core/ValueObjectConstResultImpl.cpp   | 115 --
 lldb/source/Core/ValueObjectRegister.cpp  |  14 +--
 lldb/source/Core/ValueObjectVTable.cpp|   6 +-
 14 files changed, 178 insertions(+), 125 deletions(-)

diff --git a/lldb/include/lldb/Core/ValueObject.h 
b/lldb/include/lldb/Core/ValueObject.h
index e7e35e2b2bffc..1e8c7c9c00536 100644
--- a/lldb/include/lldb/Core/ValueObject.h
+++ b/lldb/include/lldb/Core/ValueObject.h
@@ -959,9 +959,12 @@ class ValueObject {
   /// Should only be called by ValueObject::GetChildAtIndex().
   ///
   /// \return A ValueObject managed by this ValueObject's manager.
-  virtual ValueObject *CreateChildAtIndex(size_t idx,
-  bool synthetic_array_member,
-  int32_t synthetic_index);
+  virtual ValueObject *CreateChildAtIndex(size_t idx);
+
+  /// Should only be called by ValueObject::GetSyntheticArrayMember().
+  ///
+  /// \return A ValueObject managed by this ValueObject's manager.
+  virtual ValueObject *CreateSyntheticArrayMember(size_t idx);
 
   /// Should only be called by ValueObject::GetNumChildren().
   virtual llvm::Expected
diff --git a/lldb/include/lldb/Core/ValueObjectConstResult.h 
b/lldb/include/lldb/Core/ValueObjectConstResult.h
index 37dc0867f26c9..d3b3362bd0e9e 100644
--- a/lldb/include/lldb/Core/ValueObjectConstResult.h
+++ b/lldb/include/lldb/Core/ValueObjectConstResult.h
@@ -79,9 +79,6 @@ class ValueObjectConstResult : public ValueObject {
 
   lldb::ValueObjectSP Dereference(Status &error) override;
 
-  ValueObject *CreateChildAtIndex(size_t idx, bool synthetic_array_member,
-  int32_t synthetic_index) override;
-
   lldb::ValueObjectSP GetSyntheticChildAtOffset(
   uint32_t offset, const CompilerType &type, bool can_create,
   ConstString name_const_str = ConstString()) override;
@@ -151,6 +148,13 @@ class ValueObjectConstResult : public ValueObject {
   ValueObjectConstResult(ExecutionContextScope *exe_scope,
  ValueObjectManager &manager, const Status &error);
 
+  ValueObject *CreateChildAtIndex(size_t idx) override {
+return m_impl.CreateChildAtIndex(idx);
+  }
+  ValueObject *CreateSyntheticArrayMember(size_t idx) override {
+return m_impl.CreateSyntheticArrayMember(idx);
+  }
+
   ValueObjectConstResult(const ValueObjectConstResult &) = delete;
   const ValueObjectConstResult &
   operator=(const ValueObjectConstResult &) = delete;
diff --git a/lldb/include/lldb/Core/ValueObjectConstResultCast.h 
b/lldb/include/lldb/Core/ValueObjectConstResultCast.h
index efcbe0dc6a0bd..911a08363b393 100644
--- a/lldb/include/lldb/Core/ValueObjectConstResultCast.h
+++ b/lldb/include/lldb/Core/ValueObjectConstResultCast.h
@@ -35,9 +35,6 @@ class ValueObjectConstResultCast : public ValueObjectCast {
 
   lldb::ValueObjectSP Dereference(Status &error) override;
 
-  ValueObject *CreateChildAtIndex(size_t idx, bool synthetic_array_member,
-  int32_t synthetic_index) override;
-
   virtual CompilerType GetCompilerType() {
 return ValueObjectCast::GetCompilerType();
   }
@@ -61,6 +58,13 @@ class ValueObjectConstResultCast : public ValueObjectCast {
   friend class ValueObjectConstResult;
   friend class ValueObjectConstResultImpl;
 
+  ValueObject *CreateChildAtIndex(size_t idx) override {
+return m_impl.CreateChildAtIndex(idx);
+  }
+  ValueObject *CreateSyntheticArrayMember(size_t idx) override {
+return m_impl.CreateSyntheticArrayMember(idx);
+  }
+

[Lldb-commits] [lldb] [lldb] Split ValueObject::CreateChildAtIndex into two functions (PR #94455)

2024-06-05 Thread via lldb-commits


llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: Pavel Labath (labath)


Changes

The the function is doing two fairly different things, depending on how it is 
called. While this allows for some code reuse, it also makes it hard to 
override it correctly. Possibly for this reason ValueObjectSynthetic overerides 
GetChildAtIndex instead, which forces it to reimplement some of its 
functionality, most notably caching of generated children.

Splitting this up makes it easier to move the caching to a common place (and 
hopefully makes the code easier to follow in general).

---

Patch is 25.08 KiB, truncated to 20.00 KiB below, full version: 
https://github.com/llvm/llvm-project/pull/94455.diff


14 Files Affected:

- (modified) lldb/include/lldb/Core/ValueObject.h (+6-3) 
- (modified) lldb/include/lldb/Core/ValueObjectConstResult.h (+7-3) 
- (modified) lldb/include/lldb/Core/ValueObjectConstResultCast.h (+7-3) 
- (modified) lldb/include/lldb/Core/ValueObjectConstResultChild.h (+7-3) 
- (modified) lldb/include/lldb/Core/ValueObjectConstResultImpl.h (+2-2) 
- (modified) lldb/include/lldb/Core/ValueObjectRegister.h (+5-3) 
- (modified) lldb/include/lldb/Core/ValueObjectVTable.h (+5-3) 
- (modified) lldb/source/Core/ValueObject.cpp (+55-36) 
- (modified) lldb/source/Core/ValueObjectConstResult.cpp (-6) 
- (modified) lldb/source/Core/ValueObjectConstResultCast.cpp (-6) 
- (modified) lldb/source/Core/ValueObjectConstResultChild.cpp (-6) 
- (modified) lldb/source/Core/ValueObjectConstResultImpl.cpp (+78-37) 
- (modified) lldb/source/Core/ValueObjectRegister.cpp (+5-9) 
- (modified) lldb/source/Core/ValueObjectVTable.cpp (+1-5) 


``diff
diff --git a/lldb/include/lldb/Core/ValueObject.h 
b/lldb/include/lldb/Core/ValueObject.h
index e7e35e2b2bffc..1e8c7c9c00536 100644
--- a/lldb/include/lldb/Core/ValueObject.h
+++ b/lldb/include/lldb/Core/ValueObject.h
@@ -959,9 +959,12 @@ class ValueObject {
   /// Should only be called by ValueObject::GetChildAtIndex().
   ///
   /// \return A ValueObject managed by this ValueObject's manager.
-  virtual ValueObject *CreateChildAtIndex(size_t idx,
-  bool synthetic_array_member,
-  int32_t synthetic_index);
+  virtual ValueObject *CreateChildAtIndex(size_t idx);
+
+  /// Should only be called by ValueObject::GetSyntheticArrayMember().
+  ///
+  /// \return A ValueObject managed by this ValueObject's manager.
+  virtual ValueObject *CreateSyntheticArrayMember(size_t idx);
 
   /// Should only be called by ValueObject::GetNumChildren().
   virtual llvm::Expected
diff --git a/lldb/include/lldb/Core/ValueObjectConstResult.h 
b/lldb/include/lldb/Core/ValueObjectConstResult.h
index 37dc0867f26c9..d3b3362bd0e9e 100644
--- a/lldb/include/lldb/Core/ValueObjectConstResult.h
+++ b/lldb/include/lldb/Core/ValueObjectConstResult.h
@@ -79,9 +79,6 @@ class ValueObjectConstResult : public ValueObject {
 
   lldb::ValueObjectSP Dereference(Status &error) override;
 
-  ValueObject *CreateChildAtIndex(size_t idx, bool synthetic_array_member,
-  int32_t synthetic_index) override;
-
   lldb::ValueObjectSP GetSyntheticChildAtOffset(
   uint32_t offset, const CompilerType &type, bool can_create,
   ConstString name_const_str = ConstString()) override;
@@ -151,6 +148,13 @@ class ValueObjectConstResult : public ValueObject {
   ValueObjectConstResult(ExecutionContextScope *exe_scope,
  ValueObjectManager &manager, const Status &error);
 
+  ValueObject *CreateChildAtIndex(size_t idx) override {
+return m_impl.CreateChildAtIndex(idx);
+  }
+  ValueObject *CreateSyntheticArrayMember(size_t idx) override {
+return m_impl.CreateSyntheticArrayMember(idx);
+  }
+
   ValueObjectConstResult(const ValueObjectConstResult &) = delete;
   const ValueObjectConstResult &
   operator=(const ValueObjectConstResult &) = delete;
diff --git a/lldb/include/lldb/Core/ValueObjectConstResultCast.h 
b/lldb/include/lldb/Core/ValueObjectConstResultCast.h
index efcbe0dc6a0bd..911a08363b393 100644
--- a/lldb/include/lldb/Core/ValueObjectConstResultCast.h
+++ b/lldb/include/lldb/Core/ValueObjectConstResultCast.h
@@ -35,9 +35,6 @@ class ValueObjectConstResultCast : public ValueObjectCast {
 
   lldb::ValueObjectSP Dereference(Status &error) override;
 
-  ValueObject *CreateChildAtIndex(size_t idx, bool synthetic_array_member,
-  int32_t synthetic_index) override;
-
   virtual CompilerType GetCompilerType() {
 return ValueObjectCast::GetCompilerType();
   }
@@ -61,6 +58,13 @@ class ValueObjectConstResultCast : public ValueObjectCast {
   friend class ValueObjectConstResult;
   friend class ValueObjectConstResultImpl;
 
+  ValueObject *CreateChildAtIndex(size_t idx) override {
+return m_impl.CreateChildAtIndex(idx);
+  }
+  ValueObject *CreateSyntheticArrayMember(size_t idx) override {
+return m_impl.CreateSyntheticArrayMember(idx

[Lldb-commits] [lldb] [lldb] Split ValueObject::CreateChildAtIndex into two functions (PR #94455)

2024-06-05 Thread Pavel Labath via lldb-commits



@@ -488,66 +488,85 @@ void ValueObject::SetNumChildren(uint32_t num_children) {
   m_children.SetChildrenCount(num_children);
 }
 
-ValueObject *ValueObject::CreateChildAtIndex(size_t idx,
- bool synthetic_array_member,
- int32_t synthetic_index) {
-  ValueObject *valobj = nullptr;
-
+ValueObject *ValueObject::CreateChildAtIndex(size_t idx) {
   bool omit_empty_base_classes = true;
-  bool ignore_array_bounds = synthetic_array_member;
-  std::string child_name_str;
+  bool ignore_array_bounds = false;
+  std::string child_name;
   uint32_t child_byte_size = 0;
   int32_t child_byte_offset = 0;
   uint32_t child_bitfield_bit_size = 0;
   uint32_t child_bitfield_bit_offset = 0;
   bool child_is_base_class = false;
   bool child_is_deref_of_parent = false;
   uint64_t language_flags = 0;
-
-  const bool transparent_pointers = !synthetic_array_member;
+  const bool transparent_pointers = true;
 
   ExecutionContext exe_ctx(GetExecutionContextRef());
 
   auto child_compiler_type_or_err =
   GetCompilerType().GetChildCompilerTypeAtIndex(
   &exe_ctx, idx, transparent_pointers, omit_empty_base_classes,
-  ignore_array_bounds, child_name_str, child_byte_size,
-  child_byte_offset, child_bitfield_bit_size, 
child_bitfield_bit_offset,
+  ignore_array_bounds, child_name, child_byte_size, child_byte_offset,
+  child_bitfield_bit_size, child_bitfield_bit_offset,
   child_is_base_class, child_is_deref_of_parent, this, language_flags);
-  CompilerType child_compiler_type;
-  if (!child_compiler_type_or_err)
+  if (!child_compiler_type_or_err || !child_compiler_type_or_err->IsValid()) {
 LLDB_LOG_ERROR(GetLog(LLDBLog::Types),
child_compiler_type_or_err.takeError(),
"could not find child: {0}");
-  else
-child_compiler_type = *child_compiler_type_or_err;
+return nullptr;
+  }
+
+  return new ValueObjectChild(
+  *this, *child_compiler_type_or_err, ConstString(child_name),
+  child_byte_size, child_byte_offset, child_bitfield_bit_size,
+  child_bitfield_bit_offset, child_is_base_class, child_is_deref_of_parent,
+  eAddressTypeInvalid, language_flags);
+}
+
+ValueObject *ValueObject::CreateSyntheticArrayMember(size_t idx) {
+  bool omit_empty_base_classes = true;
+  bool ignore_array_bounds = true;
+  std::string child_name;
+  uint32_t child_byte_size = 0;
+  int32_t child_byte_offset = 0;
+  uint32_t child_bitfield_bit_size = 0;
+  uint32_t child_bitfield_bit_offset = 0;
+  bool child_is_base_class = false;
+  bool child_is_deref_of_parent = false;
+  uint64_t language_flags = 0;
+  const bool transparent_pointers = false;
 
-  if (child_compiler_type) {
-if (synthetic_index)
-  child_byte_offset += child_byte_size * synthetic_index;
+  ExecutionContext exe_ctx(GetExecutionContextRef());
 
-ConstString child_name;
-if (!child_name_str.empty())
-  child_name.SetCString(child_name_str.c_str());

labath wrote:

It seems that one side effect of "simplifying" this is that the name of 
anonymous struct members changes from nullptr/None to "". None of lldb tests 
broke but this broke some (rather dodgy) downstream code of ours. I would be 
inclined to keep this change, as I don't think we can make a promise (and keep 
it) to preserve details like this.

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

[Lldb-commits] [lldb] [LLDB] Add more helper functions to ValueObject class. (PR #87197)

2024-06-05 Thread via lldb-commits


https://github.com/cmtice updated 
https://github.com/llvm/llvm-project/pull/87197

>From 68cb68d3f93aed6b3479fb305131b99ec599c9d8 Mon Sep 17 00:00:00 2001
From: Caroline Tice 
Date: Sun, 31 Mar 2024 10:59:38 -0700
Subject: [PATCH 1/5] [LLDB] Add more helper functions to ValueObject class.

Create additional helper functions for the ValueObject class, for:
  - returning the value as an APSInt or APFloat
  - additional type casting options
  - additional ways to create ValueObjects from various types of data
  - dereferencing a ValueObject

These helper functions are needed for implementing the Data Inspection
Language, described in
https://discourse.llvm.org/t/rfc-data-inspection-language/69893
---
 lldb/include/lldb/Core/ValueObject.h |  61 
 lldb/source/Core/ValueObject.cpp | 405 +++
 2 files changed, 466 insertions(+)

diff --git a/lldb/include/lldb/Core/ValueObject.h 
b/lldb/include/lldb/Core/ValueObject.h
index e7e35e2b2bffc..0c8dbf384a326 100644
--- a/lldb/include/lldb/Core/ValueObject.h
+++ b/lldb/include/lldb/Core/ValueObject.h
@@ -441,6 +441,19 @@ class ValueObject {
 
   virtual int64_t GetValueAsSigned(int64_t fail_value, bool *success = 
nullptr);
 
+  llvm::APSInt GetValueAsAPSInt();
+
+  llvm::APFloat GetValueAsFloat();
+
+  bool GetValueAsBool();
+
+  /// Update the value of the current object to be the integer in the 'value'
+  /// parameter.
+  void UpdateIntegerValue(const llvm::APInt &value);
+
+  /// Assign the integer value 'new_val_sp' to the current object.
+  void UpdateIntegerValue(lldb::ValueObjectSP new_val_sp);
+
   virtual bool SetValueFromCString(const char *value_str, Status &error);
 
   /// Return the module associated with this value object in case the value is
@@ -618,6 +631,24 @@ class ValueObject {
   virtual lldb::ValueObjectSP CastPointerType(const char *name,
   lldb::TypeSP &type_sp);
 
+  /// Return the target load address assocaited with this value object.
+  lldb::addr_t GetLoadAddress();
+
+  lldb::ValueObjectSP CastDerivedToBaseType(CompilerType type,
+const std::vector &idx);
+
+  lldb::ValueObjectSP CastBaseToDerivedType(CompilerType type, uint64_t 
offset);
+
+  lldb::ValueObjectSP CastScalarToBasicType(CompilerType type, Status &error);
+
+  lldb::ValueObjectSP CastEnumToBasicType(CompilerType type);
+
+  lldb::ValueObjectSP CastPointerToBasicType(CompilerType type);
+
+  lldb::ValueObjectSP CastIntegerOrEnumToEnumType(CompilerType type);
+
+  lldb::ValueObjectSP CastFloatToEnumType(CompilerType type, Status &error);
+
   /// If this object represents a C++ class with a vtable, return an object
   /// that represents the virtual function table. If the object isn't a class
   /// with a vtable, return a valid ValueObject with the error set correctly.
@@ -668,6 +699,32 @@ class ValueObject {
   CreateValueObjectFromData(llvm::StringRef name, const DataExtractor &data,
 const ExecutionContext &exe_ctx, CompilerType 
type);
 
+  static lldb::ValueObjectSP
+  CreateValueObjectFromBytes(lldb::TargetSP target_sp, const void *bytes,
+ CompilerType type);
+
+  static lldb::ValueObjectSP CreateValueObjectFromBytes(lldb::TargetSP target,
+const void *bytes,
+lldb::BasicType type);
+
+  static lldb::ValueObjectSP CreateValueObjectFromAPInt(lldb::TargetSP target,
+const llvm::APInt &v,
+CompilerType type);
+
+  static lldb::ValueObjectSP
+  CreateValueObjectFromAPFloat(lldb::TargetSP target, const llvm::APFloat &v,
+   CompilerType type);
+
+  static lldb::ValueObjectSP CreateValueObjectFromPointer(lldb::TargetSP 
target,
+  uintptr_t addr,
+  CompilerType type);
+
+  static lldb::ValueObjectSP CreateValueObjectFromBool(lldb::TargetSP target,
+   bool value);
+
+  static lldb::ValueObjectSP CreateValueObjectFromNullptr(lldb::TargetSP 
target,
+  CompilerType type);
+
   lldb::ValueObjectSP Persist();
 
   /// Returns true if this is a char* or a char[] if it is a char* and
@@ -719,6 +776,10 @@ class ValueObject {
 ClearUserVisibleData(eClearUserVisibleDataItemsSummary);
   }
 
+  void SetDerefValobj(ValueObject *deref) { m_deref_valobj = deref; }
+
+  ValueObject *GetDerefValobj() { return m_deref_valobj; }
+
   void SetValueFormat(lldb::TypeFormatImplSP format) {
 m_type_format_sp = std::move(format);
 ClearUserVisibleData(eClearUserVisibleDataItemsValue);
diff --git a/lldb/source/Core/ValueObject.cpp b/lldb/source/Core/

[Lldb-commits] [lldb] [LLDB] Add more helper functions to ValueObject class. (PR #87197)

2024-06-05 Thread via lldb-commits


cmtice wrote:

I've updated with a few minor fixes. I've addressed all the review comments (I 
think).  I've verified/tested that all the casting, etc. works properly. Please 
review/approve this. Thanks!


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

[Lldb-commits] [lldb] [llvm] [lldb] Return an llvm::Expected from DWARFExpression::Evaluate (NFCI) (PR #94420)

2024-06-05 Thread Jonas Devlieghere via lldb-commits



@@ -2042,7 +2043,7 @@ bool RegisterContextUnwind::ReadFrameAddress(
   return true;
 }
 UnwindLogMsg("Failed to set CFA value via DWARF expression: %s",
- error.AsCString());
+ llvm::toString(result.takeError()).c_str());

JDevlieghere wrote:

Yup

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

[Lldb-commits] [lldb] Fix flaky TestDAP_console test. (PR #94494)

2024-06-05 Thread Miro Bucko via lldb-commits


https://github.com/mbucko created 
https://github.com/llvm/llvm-project/pull/94494

Test Plan:
llvm-lit llvm-project/lldb/test/API/tools/lldb-dap/console/TestDAP_console.py

>From 0d1eb164ab12773cb574de6066b717fd4c4c9e31 Mon Sep 17 00:00:00 2001
From: Miro Bucko 
Date: Wed, 5 Jun 2024 09:03:38 -0700
Subject: [PATCH] Fix flaky TestDAP_console test.

Test Plan:
llvm-lit llvm-project/lldb/test/API/tools/lldb-dap/console/TestDAP_console.py
---
 lldb/test/API/tools/lldb-dap/console/TestDAP_console.py | 4 
 1 file changed, 4 insertions(+)

diff --git a/lldb/test/API/tools/lldb-dap/console/TestDAP_console.py 
b/lldb/test/API/tools/lldb-dap/console/TestDAP_console.py
index e6345818bf087..fad4c31bb7658 100644
--- a/lldb/test/API/tools/lldb-dap/console/TestDAP_console.py
+++ b/lldb/test/API/tools/lldb-dap/console/TestDAP_console.py
@@ -141,6 +141,10 @@ def test_exit_status_message_sigterm(self):
 
 # Get the console output
 console_output = self.collect_console(1.0)
+n_reads = 0
+while n_reads < 10 and "exited with status" not in console_output:
+console_output += self.collect_console(1.0)
+n_reads += 1
 
 # Verify the exit status message is printed.
 self.assertIn(

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

[Lldb-commits] [lldb] Fix flaky TestDAP_console test. (PR #94494)

2024-06-05 Thread via lldb-commits


llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: Miro Bucko (mbucko)


Changes

Test Plan:
llvm-lit llvm-project/lldb/test/API/tools/lldb-dap/console/TestDAP_console.py

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


1 Files Affected:

- (modified) lldb/test/API/tools/lldb-dap/console/TestDAP_console.py (+4) 


``diff
diff --git a/lldb/test/API/tools/lldb-dap/console/TestDAP_console.py 
b/lldb/test/API/tools/lldb-dap/console/TestDAP_console.py
index e6345818bf087..fad4c31bb7658 100644
--- a/lldb/test/API/tools/lldb-dap/console/TestDAP_console.py
+++ b/lldb/test/API/tools/lldb-dap/console/TestDAP_console.py
@@ -141,6 +141,10 @@ def test_exit_status_message_sigterm(self):
 
 # Get the console output
 console_output = self.collect_console(1.0)
+n_reads = 0
+while n_reads < 10 and "exited with status" not in console_output:
+console_output += self.collect_console(1.0)
+n_reads += 1
 
 # Verify the exit status message is printed.
 self.assertIn(

``




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

[Lldb-commits] [lldb] Fix flaky TestDAP_console test. (PR #94494)

2024-06-05 Thread Walter Erquinigo via lldb-commits


https://github.com/walter-erquinigo approved this pull request.


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

[Lldb-commits] [lldb] [llvm] [lldb] Return an llvm::Expected from DWARFExpression::Evaluate (NFCI) (PR #94420)

2024-06-05 Thread Jonas Devlieghere via lldb-commits


https://github.com/JDevlieghere updated 
https://github.com/llvm/llvm-project/pull/94420

>From 22bb28a5e3fc84c75e1013c3b0c15654e7b786da Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere 
Date: Tue, 4 Jun 2024 17:04:15 -0700
Subject: [PATCH 1/5] [Support] Add variadic createStringError overload (NFC)

---
 llvm/include/llvm/Support/Error.h | 5 +
 1 file changed, 5 insertions(+)

diff --git a/llvm/include/llvm/Support/Error.h 
b/llvm/include/llvm/Support/Error.h
index 662c3ea46e3c1..1fa0d8cb709cc 100644
--- a/llvm/include/llvm/Support/Error.h
+++ b/llvm/include/llvm/Support/Error.h
@@ -1277,6 +1277,11 @@ inline Error createStringError(const Twine &S) {
   return createStringError(llvm::inconvertibleErrorCode(), S);
 }
 
+template 
+inline Error createStringError(char const *Fmt, const Ts &...Vals) {
+  return createStringError(llvm::inconvertibleErrorCode(), Fmt, Vals...);
+}
+
 template 
 inline Error createStringError(std::errc EC, char const *Fmt,
const Ts &... Vals) {

>From d5c288ddb9473a52a8635a32d3a5cc057c9cb986 Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere 
Date: Tue, 4 Jun 2024 19:04:07 -0700
Subject: [PATCH 2/5] [lldb] Return an llvm::Expected from
 DWARFExpression::Evaluate (NFCI)

Change the interface of DWARFExpression::Evaluate and
DWARFExpressionList::Evaluate to return an llvm::Expected instead of a
boolean. This eliminates the Status output parameter and improves error
handling.
---
 .../include/lldb/Expression/DWARFExpression.h |  13 +-
 .../lldb/Expression/DWARFExpressionList.h |  10 +-
 lldb/source/Core/ValueObjectVariable.cpp  |   8 +-
 lldb/source/Expression/DWARFExpression.cpp| 747 +++---
 .../source/Expression/DWARFExpressionList.cpp |  28 +-
 .../SymbolFile/DWARF/DWARFASTParserClang.cpp  |  20 +-
 .../SymbolFile/DWARF/SymbolFileDWARF.cpp  |  16 +-
 lldb/source/Symbol/Function.cpp   |  17 +-
 lldb/source/Target/RegisterContextUnwind.cpp  |  23 +-
 lldb/source/Target/StackFrame.cpp |  19 +-
 .../Expression/DWARFExpressionTest.cpp|  78 +-
 11 files changed, 401 insertions(+), 578 deletions(-)

diff --git a/lldb/include/lldb/Expression/DWARFExpression.h 
b/lldb/include/lldb/Expression/DWARFExpression.h
index 1d85308d1caa7..e85ba464dea6b 100644
--- a/lldb/include/lldb/Expression/DWARFExpression.h
+++ b/lldb/include/lldb/Expression/DWARFExpression.h
@@ -132,13 +132,12 @@ class DWARFExpression {
   /// \return
   /// True on success; false otherwise.  If error_ptr is non-NULL,
   /// details of the failure are provided through it.
-  static bool Evaluate(ExecutionContext *exe_ctx, RegisterContext *reg_ctx,
-   lldb::ModuleSP module_sp, const DataExtractor &opcodes,
-   const plugin::dwarf::DWARFUnit *dwarf_cu,
-   const lldb::RegisterKind reg_set,
-   const Value *initial_value_ptr,
-   const Value *object_address_ptr, Value &result,
-   Status *error_ptr);
+  static llvm::Expected
+  Evaluate(ExecutionContext *exe_ctx, RegisterContext *reg_ctx,
+   lldb::ModuleSP module_sp, const DataExtractor &opcodes,
+   const plugin::dwarf::DWARFUnit *dwarf_cu,
+   const lldb::RegisterKind reg_set, const Value *initial_value_ptr,
+   const Value *object_address_ptr);
 
   static bool ParseDWARFLocationList(const plugin::dwarf::DWARFUnit *dwarf_cu,
  const DataExtractor &data,
diff --git a/lldb/include/lldb/Expression/DWARFExpressionList.h 
b/lldb/include/lldb/Expression/DWARFExpressionList.h
index c2218ad4af0a7..f711a1cbe9bbd 100644
--- a/lldb/include/lldb/Expression/DWARFExpressionList.h
+++ b/lldb/include/lldb/Expression/DWARFExpressionList.h
@@ -9,6 +9,7 @@
 #ifndef LLDB_EXPRESSION_DWARFEXPRESSIONLIST_H
 #define LLDB_EXPRESSION_DWARFEXPRESSIONLIST_H
 
+#include "lldb/Core/Value.h"
 #include "lldb/Expression/DWARFExpression.h"
 #include "lldb/Utility/RangeMap.h"
 #include "lldb/lldb-private.h"
@@ -113,10 +114,11 @@ class DWARFExpressionList {
 
   void SetModule(const lldb::ModuleSP &module) { m_module_wp = module; }
 
-  bool Evaluate(ExecutionContext *exe_ctx, RegisterContext *reg_ctx,
-lldb::addr_t func_load_addr, const Value *initial_value_ptr,
-const Value *object_address_ptr, Value &result,
-Status *error_ptr) const;
+  llvm::Expected Evaluate(ExecutionContext *exe_ctx,
+ RegisterContext *reg_ctx,
+ lldb::addr_t func_load_addr,
+ const Value *initial_value_ptr,
+ const Value *object_address_ptr) const;
 
 private:
   // RangeDataVector requires a comparator for DWARFExpression, but it doesn't
diff --git a/lldb/source/Core/ValueObjectVariable.cpp 
b/lldb/source/Core/ValueObjectVariable.cpp
index 67d71c90a959d..51eb11d3a189e 100644
-

[Lldb-commits] [lldb] Fix flaky TestDAP_console test. (PR #94494)

2024-06-05 Thread Jonas Devlieghere via lldb-commits


JDevlieghere wrote:

Would this affect other calls to `collect_console` too? Do we usually look for 
a particular string? If so, would it make sense to change the function (rather 
than this particular call site) or have a little helper? 

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

[Lldb-commits] [lldb] [llvm] [lldb] Return an llvm::Expected from DWARFExpression::Evaluate (NFCI) (PR #94420)

2024-06-05 Thread Adrian Prantl via lldb-commits


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

[Lldb-commits] [lldb] [llvm] [lldb] Return an llvm::Expected from DWARFExpression::Evaluate (NFCI) (PR #94420)

2024-06-05 Thread Adrian Prantl via lldb-commits



@@ -52,9 +52,10 @@ __attribute__((noinline)) void func4_amb(int &sink, int x) {
   //% expect_cmd_failure=True)
   //% self.filecheck("expr sink", "main.cpp","-check-prefix=FUNC4-EXPR",
   //% expect_cmd_failure=True)
-  // FUNC4-EXPR-FAIL: couldn't get the value of variable x: Could not evaluate
-  // DW_OP_entry_value. FUNC4-EXPR: couldn't get the value of variable sink:
-  // Could not evaluate DW_OP_entry_value.
+  // FUNC4-EXPR-FAIL: couldn't get the value of variable x: could not evaluate
+  // DW_OP_entry_value: no matching call site param found FUNC4-EXPR: couldn't
+  // get the value of variable sink: could not evaluate DW_OP_entry_value: no

adrian-prantl wrote:

There's a missing newline that disables part of the check.

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

[Lldb-commits] [lldb] [llvm] [lldb] Return an llvm::Expected from DWARFExpression::Evaluate (NFCI) (PR #94420)

2024-06-05 Thread Adrian Prantl via lldb-commits



@@ -1350,10 +1300,8 @@ bool DWARFExpression::Evaluate(
   if (pick_idx < stack.size())
 stack.push_back(stack[stack.size() - 1 - pick_idx]);
   else {
-if (error_ptr)
-  error_ptr->SetErrorStringWithFormat(
-  "Index %u out of range for DW_OP_pick.\n", pick_idx);
-return false;
+return llvm::createStringError(
+"Index %u out of range for DW_OP_pick.\n", pick_idx);

adrian-prantl wrote:

drop the extra newline

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

[Lldb-commits] [lldb] [llvm] [lldb] Return an llvm::Expected from DWARFExpression::Evaluate (NFCI) (PR #94420)

2024-06-05 Thread Adrian Prantl via lldb-commits



@@ -2341,9 +2198,7 @@ bool DWARFExpression::Evaluate(
 // the stack by the called expression may be used as return values by prior
 // agreement between the calling and called expressions.
 case DW_OP_call2:
-  if (error_ptr)
-error_ptr->SetErrorString("Unimplemented opcode DW_OP_call2.");
-  return false;
+  return llvm::createStringError("Unimplemented opcode DW_OP_call2.");

adrian-prantl wrote:

drop all the `.` at the end of the error messages

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

[Lldb-commits] [lldb] [llvm] [lldb] Return an llvm::Expected from DWARFExpression::Evaluate (NFCI) (PR #94420)

2024-06-05 Thread Adrian Prantl via lldb-commits


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

This is going to be great! Thanks.
Minor nitpicks inside.

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

[Lldb-commits] [lldb] [llvm] [lldb] Return an llvm::Expected from DWARFExpression::Evaluate (NFCI) (PR #94420)

2024-06-05 Thread Jonas Devlieghere via lldb-commits



@@ -1350,10 +1300,8 @@ bool DWARFExpression::Evaluate(
   if (pick_idx < stack.size())
 stack.push_back(stack[stack.size() - 1 - pick_idx]);
   else {
-if (error_ptr)
-  error_ptr->SetErrorStringWithFormat(
-  "Index %u out of range for DW_OP_pick.\n", pick_idx);
-return false;
+return llvm::createStringError(
+"Index %u out of range for DW_OP_pick.\n", pick_idx);

JDevlieghere wrote:

The capitalization, periods and newlines were the small follows up mentioned 
offline. There's a lot that can be improved, but I think it deserves its own 
PR. 

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

[Lldb-commits] [lldb] [llvm] [lldb] Return an llvm::Expected from DWARFExpression::Evaluate (NFCI) (PR #94420)

2024-06-05 Thread Jonas Devlieghere via lldb-commits



@@ -52,9 +52,10 @@ __attribute__((noinline)) void func4_amb(int &sink, int x) {
   //% expect_cmd_failure=True)
   //% self.filecheck("expr sink", "main.cpp","-check-prefix=FUNC4-EXPR",
   //% expect_cmd_failure=True)
-  // FUNC4-EXPR-FAIL: couldn't get the value of variable x: Could not evaluate
-  // DW_OP_entry_value. FUNC4-EXPR: couldn't get the value of variable sink:
-  // Could not evaluate DW_OP_entry_value.
+  // FUNC4-EXPR-FAIL: couldn't get the value of variable x: could not evaluate
+  // DW_OP_entry_value: no matching call site param found FUNC4-EXPR: couldn't
+  // get the value of variable sink: could not evaluate DW_OP_entry_value: no

JDevlieghere wrote:

Ugh, this is clang-format wrapping the line. I'll need to wrap that between 
`clang-format off`. 

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

[Lldb-commits] [lldb] [lldb] UpdateFormatsIfNeeded should respect the dynamic value type (PR #93262)

2024-06-05 Thread via lldb-commits


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

I wish I knew why the original code was ignoring `GetDynamicValueType` here, 
when it does use it on the very next line...  The old code looks wrong to me.  
However, the places where dynamic values matter for the Format choice are 
uncommon - you generally set formats on scalars, and scalars don't have dynamic 
types...  So maybe this was just an expression of "how could this matter"?

In any case, this does seem more correct on the face of it.

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

[Lldb-commits] [lldb] [llvm] [lldb] Return an llvm::Expected from DWARFExpression::Evaluate (NFCI) (PR #94420)

2024-06-05 Thread Jonas Devlieghere via lldb-commits


https://github.com/JDevlieghere updated 
https://github.com/llvm/llvm-project/pull/94420

>From 22bb28a5e3fc84c75e1013c3b0c15654e7b786da Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere 
Date: Tue, 4 Jun 2024 17:04:15 -0700
Subject: [PATCH 1/6] [Support] Add variadic createStringError overload (NFC)

---
 llvm/include/llvm/Support/Error.h | 5 +
 1 file changed, 5 insertions(+)

diff --git a/llvm/include/llvm/Support/Error.h 
b/llvm/include/llvm/Support/Error.h
index 662c3ea46e3c1..1fa0d8cb709cc 100644
--- a/llvm/include/llvm/Support/Error.h
+++ b/llvm/include/llvm/Support/Error.h
@@ -1277,6 +1277,11 @@ inline Error createStringError(const Twine &S) {
   return createStringError(llvm::inconvertibleErrorCode(), S);
 }
 
+template 
+inline Error createStringError(char const *Fmt, const Ts &...Vals) {
+  return createStringError(llvm::inconvertibleErrorCode(), Fmt, Vals...);
+}
+
 template 
 inline Error createStringError(std::errc EC, char const *Fmt,
const Ts &... Vals) {

>From d5c288ddb9473a52a8635a32d3a5cc057c9cb986 Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere 
Date: Tue, 4 Jun 2024 19:04:07 -0700
Subject: [PATCH 2/6] [lldb] Return an llvm::Expected from
 DWARFExpression::Evaluate (NFCI)

Change the interface of DWARFExpression::Evaluate and
DWARFExpressionList::Evaluate to return an llvm::Expected instead of a
boolean. This eliminates the Status output parameter and improves error
handling.
---
 .../include/lldb/Expression/DWARFExpression.h |  13 +-
 .../lldb/Expression/DWARFExpressionList.h |  10 +-
 lldb/source/Core/ValueObjectVariable.cpp  |   8 +-
 lldb/source/Expression/DWARFExpression.cpp| 747 +++---
 .../source/Expression/DWARFExpressionList.cpp |  28 +-
 .../SymbolFile/DWARF/DWARFASTParserClang.cpp  |  20 +-
 .../SymbolFile/DWARF/SymbolFileDWARF.cpp  |  16 +-
 lldb/source/Symbol/Function.cpp   |  17 +-
 lldb/source/Target/RegisterContextUnwind.cpp  |  23 +-
 lldb/source/Target/StackFrame.cpp |  19 +-
 .../Expression/DWARFExpressionTest.cpp|  78 +-
 11 files changed, 401 insertions(+), 578 deletions(-)

diff --git a/lldb/include/lldb/Expression/DWARFExpression.h 
b/lldb/include/lldb/Expression/DWARFExpression.h
index 1d85308d1caa7..e85ba464dea6b 100644
--- a/lldb/include/lldb/Expression/DWARFExpression.h
+++ b/lldb/include/lldb/Expression/DWARFExpression.h
@@ -132,13 +132,12 @@ class DWARFExpression {
   /// \return
   /// True on success; false otherwise.  If error_ptr is non-NULL,
   /// details of the failure are provided through it.
-  static bool Evaluate(ExecutionContext *exe_ctx, RegisterContext *reg_ctx,
-   lldb::ModuleSP module_sp, const DataExtractor &opcodes,
-   const plugin::dwarf::DWARFUnit *dwarf_cu,
-   const lldb::RegisterKind reg_set,
-   const Value *initial_value_ptr,
-   const Value *object_address_ptr, Value &result,
-   Status *error_ptr);
+  static llvm::Expected
+  Evaluate(ExecutionContext *exe_ctx, RegisterContext *reg_ctx,
+   lldb::ModuleSP module_sp, const DataExtractor &opcodes,
+   const plugin::dwarf::DWARFUnit *dwarf_cu,
+   const lldb::RegisterKind reg_set, const Value *initial_value_ptr,
+   const Value *object_address_ptr);
 
   static bool ParseDWARFLocationList(const plugin::dwarf::DWARFUnit *dwarf_cu,
  const DataExtractor &data,
diff --git a/lldb/include/lldb/Expression/DWARFExpressionList.h 
b/lldb/include/lldb/Expression/DWARFExpressionList.h
index c2218ad4af0a7..f711a1cbe9bbd 100644
--- a/lldb/include/lldb/Expression/DWARFExpressionList.h
+++ b/lldb/include/lldb/Expression/DWARFExpressionList.h
@@ -9,6 +9,7 @@
 #ifndef LLDB_EXPRESSION_DWARFEXPRESSIONLIST_H
 #define LLDB_EXPRESSION_DWARFEXPRESSIONLIST_H
 
+#include "lldb/Core/Value.h"
 #include "lldb/Expression/DWARFExpression.h"
 #include "lldb/Utility/RangeMap.h"
 #include "lldb/lldb-private.h"
@@ -113,10 +114,11 @@ class DWARFExpressionList {
 
   void SetModule(const lldb::ModuleSP &module) { m_module_wp = module; }
 
-  bool Evaluate(ExecutionContext *exe_ctx, RegisterContext *reg_ctx,
-lldb::addr_t func_load_addr, const Value *initial_value_ptr,
-const Value *object_address_ptr, Value &result,
-Status *error_ptr) const;
+  llvm::Expected Evaluate(ExecutionContext *exe_ctx,
+ RegisterContext *reg_ctx,
+ lldb::addr_t func_load_addr,
+ const Value *initial_value_ptr,
+ const Value *object_address_ptr) const;
 
 private:
   // RangeDataVector requires a comparator for DWARFExpression, but it doesn't
diff --git a/lldb/source/Core/ValueObjectVariable.cpp 
b/lldb/source/Core/ValueObjectVariable.cpp
index 67d71c90a959d..51eb11d3a189e 100644
-

[Lldb-commits] [lldb] Fix flaky TestDAP_console test. (PR #94494)

2024-06-05 Thread Miro Bucko via lldb-commits

mbucko wrote:

> Would this affect other calls to `collect_console` too? Do we usually look 
> for a particular string? If so, would it make sense to change the function 
> (rather than this particular call site) or have a little helper?

The problem is that the console hasn't had enough time to collect the logs 
after the process was terminated. This fix will keep collecting the logs until 
the "exited with status..." log has been collected so that we can check it. The 
caller could specify 10.0 seconds timeout instead of 1.0 second, but this way 
the caller can exit earlier rather than forcefully waiting on the timeout. 

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

[Lldb-commits] [lldb] [llvm] [lldb] Return an llvm::Expected from DWARFExpression::Evaluate (NFCI) (PR #94420)

2024-06-05 Thread Jonas Devlieghere via lldb-commits


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

[Lldb-commits] [lldb] 539b72f - [lldb] Return an llvm::Expected from DWARFExpression::Evaluate (NFCI) (#94420)

2024-06-05 Thread via lldb-commits


Author: Jonas Devlieghere
Date: 2024-06-05T10:57:46-07:00
New Revision: 539b72f2e15f0d8a74a6c05c7085035040a3a831

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

LOG: [lldb] Return an llvm::Expected from DWARFExpression::Evaluate (NFCI) 
(#94420)

Change the signature of `DWARFExpression::Evaluate` and
`DWARFExpressionList::Evaluate` to return an `llvm::Expected` instead of a
boolean. This eliminates the `Status` output parameter and generally improves
error handling.

Added: 


Modified: 
lldb/include/lldb/Expression/DWARFExpression.h
lldb/include/lldb/Expression/DWARFExpressionList.h
lldb/source/Core/ValueObjectVariable.cpp
lldb/source/Expression/DWARFExpression.cpp
lldb/source/Expression/DWARFExpressionList.cpp
lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
lldb/source/Symbol/Function.cpp
lldb/source/Target/RegisterContextUnwind.cpp
lldb/source/Target/StackFrame.cpp
lldb/test/API/functionalities/param_entry_vals/basic_entry_values/main.cpp
lldb/unittests/Expression/DWARFExpressionTest.cpp
llvm/include/llvm/Support/Error.h

Removed: 




diff  --git a/lldb/include/lldb/Expression/DWARFExpression.h 
b/lldb/include/lldb/Expression/DWARFExpression.h
index 1d85308d1caa7..e85ba464dea6b 100644
--- a/lldb/include/lldb/Expression/DWARFExpression.h
+++ b/lldb/include/lldb/Expression/DWARFExpression.h
@@ -132,13 +132,12 @@ class DWARFExpression {
   /// \return
   /// True on success; false otherwise.  If error_ptr is non-NULL,
   /// details of the failure are provided through it.
-  static bool Evaluate(ExecutionContext *exe_ctx, RegisterContext *reg_ctx,
-   lldb::ModuleSP module_sp, const DataExtractor &opcodes,
-   const plugin::dwarf::DWARFUnit *dwarf_cu,
-   const lldb::RegisterKind reg_set,
-   const Value *initial_value_ptr,
-   const Value *object_address_ptr, Value &result,
-   Status *error_ptr);
+  static llvm::Expected
+  Evaluate(ExecutionContext *exe_ctx, RegisterContext *reg_ctx,
+   lldb::ModuleSP module_sp, const DataExtractor &opcodes,
+   const plugin::dwarf::DWARFUnit *dwarf_cu,
+   const lldb::RegisterKind reg_set, const Value *initial_value_ptr,
+   const Value *object_address_ptr);
 
   static bool ParseDWARFLocationList(const plugin::dwarf::DWARFUnit *dwarf_cu,
  const DataExtractor &data,

diff  --git a/lldb/include/lldb/Expression/DWARFExpressionList.h 
b/lldb/include/lldb/Expression/DWARFExpressionList.h
index c2218ad4af0a7..f711a1cbe9bbd 100644
--- a/lldb/include/lldb/Expression/DWARFExpressionList.h
+++ b/lldb/include/lldb/Expression/DWARFExpressionList.h
@@ -9,6 +9,7 @@
 #ifndef LLDB_EXPRESSION_DWARFEXPRESSIONLIST_H
 #define LLDB_EXPRESSION_DWARFEXPRESSIONLIST_H
 
+#include "lldb/Core/Value.h"
 #include "lldb/Expression/DWARFExpression.h"
 #include "lldb/Utility/RangeMap.h"
 #include "lldb/lldb-private.h"
@@ -113,10 +114,11 @@ class DWARFExpressionList {
 
   void SetModule(const lldb::ModuleSP &module) { m_module_wp = module; }
 
-  bool Evaluate(ExecutionContext *exe_ctx, RegisterContext *reg_ctx,
-lldb::addr_t func_load_addr, const Value *initial_value_ptr,
-const Value *object_address_ptr, Value &result,
-Status *error_ptr) const;
+  llvm::Expected Evaluate(ExecutionContext *exe_ctx,
+ RegisterContext *reg_ctx,
+ lldb::addr_t func_load_addr,
+ const Value *initial_value_ptr,
+ const Value *object_address_ptr) const;
 
 private:
   // RangeDataVector requires a comparator for DWARFExpression, but it doesn't

diff  --git a/lldb/source/Core/ValueObjectVariable.cpp 
b/lldb/source/Core/ValueObjectVariable.cpp
index 67d71c90a959d..51eb11d3a189e 100644
--- a/lldb/source/Core/ValueObjectVariable.cpp
+++ b/lldb/source/Core/ValueObjectVariable.cpp
@@ -164,8 +164,11 @@ bool ValueObjectVariable::UpdateValue() {
 target);
 }
 Value old_value(m_value);
-if (expr_list.Evaluate(&exe_ctx, nullptr, loclist_base_load_addr, nullptr,
-   nullptr, m_value, &m_error)) {
+llvm::Expected maybe_value = expr_list.Evaluate(
+&exe_ctx, nullptr, loclist_base_load_addr, nullptr, nullptr);
+
+if (maybe_value) {
+  m_value = *maybe_value;
   m_resolved_value = m_value;
   m_value.SetContext(Value::ContextType::Variable, variable);
 
@@ -246,6 +249,7 @@ bool ValueObjectVariable::UpdateValue() {
 
   SetValueIsValid(m_e

[Lldb-commits] [lldb] 59e9160 - [lldb] UpdateFormatsIfNeeded should respect the dynamic value type (#93262)

2024-06-05 Thread via lldb-commits


Author: Augusto Noronha
Date: 2024-06-05T11:08:36-07:00
New Revision: 59e9160ac8c906c5448c2094cd28cb5bc7678a3f

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

LOG: [lldb] UpdateFormatsIfNeeded should respect the dynamic value type (#93262)

UpdateFormatsIfNeeded has hardcoded the call to GetFormat with no
dynamic values. GetFormat will try to find the synthetic children of the
ValueObject, and passing the wrong one can fail, which can be bad for
performance but should not be user visible. Fix the performace bug by
passing the dynamic value type of the ValueObject.

rdar://122506593

Added: 


Modified: 
lldb/source/Core/ValueObject.cpp

Removed: 




diff  --git a/lldb/source/Core/ValueObject.cpp 
b/lldb/source/Core/ValueObject.cpp
index 1443d9dfc3280..c5c434a941b34 100644
--- a/lldb/source/Core/ValueObject.cpp
+++ b/lldb/source/Core/ValueObject.cpp
@@ -216,7 +216,7 @@ bool ValueObject::UpdateFormatsIfNeeded() {
 m_last_format_mgr_revision = DataVisualization::GetCurrentRevision();
 any_change = true;
 
-SetValueFormat(DataVisualization::GetFormat(*this, eNoDynamicValues));
+SetValueFormat(DataVisualization::GetFormat(*this, GetDynamicValueType()));
 SetSummaryFormat(
 DataVisualization::GetSummaryFormat(*this, GetDynamicValueType()));
 SetSyntheticChildren(



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

[Lldb-commits] [lldb] [lldb] UpdateFormatsIfNeeded should respect the dynamic value type (PR #93262)

2024-06-05 Thread Augusto Noronha via lldb-commits


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

[Lldb-commits] [lldb] [lldb/crashlog] Add `--no-parallel-image-loading` hidden flag (PR #94513)

2024-06-05 Thread Med Ismail Bennani via lldb-commits


https://github.com/medismailben created 
https://github.com/llvm/llvm-project/pull/94513

This patch adds the `--no-parallel-image-loading` to the crashlog command. By 
default, image loading will happen in parallel in the crashlog script however, 
sometimes, when running tests or debugging the crashlog script itself, it's 
better to load the images sequentially.

As its name suggests, this flag will disable the default image loading 
behaviour to load all the images sequencially in the main thread.

>From 4f3d0326e133fd0d1edc746d91000c27df56e22b Mon Sep 17 00:00:00 2001
From: Med Ismail Bennani 
Date: Tue, 4 Jun 2024 23:54:56 -0700
Subject: [PATCH] [lldb/crashlog] Add `--no-parallel-image-loading` hidden flag

This patch adds the `--no-parallel-image-loading` to the crashlog
command. By default, image loading will happen in parallel in the
crashlog script however, sometimes, when running tests or debugging the
crashlog script itself, it's better to load the images sequentially.

As its name suggests, this flag will disable the default image loading
behaviour to load all the images sequencially in the main thread.

Signed-off-by: Med Ismail Bennani 
---
 lldb/examples/python/crashlog.py  | 54 +--
 .../python/crashlog_scripted_process.py   |  7 +++
 2 files changed, 45 insertions(+), 16 deletions(-)

diff --git a/lldb/examples/python/crashlog.py b/lldb/examples/python/crashlog.py
index 641b2e64d53b1..485e60ab95bba 100755
--- a/lldb/examples/python/crashlog.py
+++ b/lldb/examples/python/crashlog.py
@@ -555,30 +555,44 @@ def load_images(self, options, loaded_images=None):
 
 futures = []
 with tempfile.TemporaryDirectory() as obj_dir:
-with concurrent.futures.ThreadPoolExecutor() as executor:
 
-def add_module(image, target, obj_dir):
-return image, image.add_module(target, obj_dir)
+def add_module(image, target, obj_dir):
+return image, image.add_module(target, obj_dir)
+
+if options.parallel_image_loading:
+with concurrent.futures.ThreadPoolExecutor() as executor:
+for image in images_to_load:
+if image not in loaded_images:
+if image.uuid == uuid.UUID(int=0):
+continue
+futures.append(
+executor.submit(
+add_module,
+image=image,
+target=self.target,
+obj_dir=obj_dir,
+)
+)
 
+for future in concurrent.futures.as_completed(futures):
+image, err = future.result()
+if err:
+print(err)
+else:
+loaded_images.append(image)
+else:
 for image in images_to_load:
 if image not in loaded_images:
 if image.uuid == uuid.UUID(int=0):
 continue
-futures.append(
-executor.submit(
-add_module,
-image=image,
-target=self.target,
-obj_dir=obj_dir,
-)
+image, err = add_module(
+image=image, target=self.target, obj_dir=obj_dir
 )
 
-for future in concurrent.futures.as_completed(futures):
-image, err = future.result()
-if err:
-print(err)
-else:
-loaded_images.append(image)
+if err:
+print(err)
+else:
+loaded_images.append(image)
 
 
 class CrashLogFormatException(Exception):
@@ -1528,6 +1542,7 @@ def load_crashlog_in_scripted_process(debugger, 
crashlog_path, options, result):
 "file_path": crashlog_path,
 "load_all_images": options.load_all_images,
 "crashed_only": options.crashed_only,
+"parallel_image_loading": options.parallel_image_loading,
 }
 )
 )
@@ -1720,6 +1735,13 @@ def CreateSymbolicateCrashLogOptions(
 help="show source for all threads, not just the crashed thread",
 default=False,
 )
+arg_parser.add_argument(
+"--no-parallel-image-loading",
+dest="parallel_image_loading",
+action="store_false",
+help=argparse.SUPPRESS,
+default=True,
+)
 if add_interactive_options:
 arg_parser.add_argument(

[Lldb-commits] [lldb] [lldb/crashlog] Add `--no-parallel-image-loading` hidden flag (PR #94513)

2024-06-05 Thread via lldb-commits


llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: Med Ismail Bennani (medismailben)


Changes

This patch adds the `--no-parallel-image-loading` to the crashlog command. By 
default, image loading will happen in parallel in the crashlog script however, 
sometimes, when running tests or debugging the crashlog script itself, it's 
better to load the images sequentially.

As its name suggests, this flag will disable the default image loading 
behaviour to load all the images sequencially in the main thread.

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


2 Files Affected:

- (modified) lldb/examples/python/crashlog.py (+38-16) 
- (modified) lldb/examples/python/crashlog_scripted_process.py (+7) 


``diff
diff --git a/lldb/examples/python/crashlog.py b/lldb/examples/python/crashlog.py
index 641b2e64d53b1..485e60ab95bba 100755
--- a/lldb/examples/python/crashlog.py
+++ b/lldb/examples/python/crashlog.py
@@ -555,30 +555,44 @@ def load_images(self, options, loaded_images=None):
 
 futures = []
 with tempfile.TemporaryDirectory() as obj_dir:
-with concurrent.futures.ThreadPoolExecutor() as executor:
 
-def add_module(image, target, obj_dir):
-return image, image.add_module(target, obj_dir)
+def add_module(image, target, obj_dir):
+return image, image.add_module(target, obj_dir)
+
+if options.parallel_image_loading:
+with concurrent.futures.ThreadPoolExecutor() as executor:
+for image in images_to_load:
+if image not in loaded_images:
+if image.uuid == uuid.UUID(int=0):
+continue
+futures.append(
+executor.submit(
+add_module,
+image=image,
+target=self.target,
+obj_dir=obj_dir,
+)
+)
 
+for future in concurrent.futures.as_completed(futures):
+image, err = future.result()
+if err:
+print(err)
+else:
+loaded_images.append(image)
+else:
 for image in images_to_load:
 if image not in loaded_images:
 if image.uuid == uuid.UUID(int=0):
 continue
-futures.append(
-executor.submit(
-add_module,
-image=image,
-target=self.target,
-obj_dir=obj_dir,
-)
+image, err = add_module(
+image=image, target=self.target, obj_dir=obj_dir
 )
 
-for future in concurrent.futures.as_completed(futures):
-image, err = future.result()
-if err:
-print(err)
-else:
-loaded_images.append(image)
+if err:
+print(err)
+else:
+loaded_images.append(image)
 
 
 class CrashLogFormatException(Exception):
@@ -1528,6 +1542,7 @@ def load_crashlog_in_scripted_process(debugger, 
crashlog_path, options, result):
 "file_path": crashlog_path,
 "load_all_images": options.load_all_images,
 "crashed_only": options.crashed_only,
+"parallel_image_loading": options.parallel_image_loading,
 }
 )
 )
@@ -1720,6 +1735,13 @@ def CreateSymbolicateCrashLogOptions(
 help="show source for all threads, not just the crashed thread",
 default=False,
 )
+arg_parser.add_argument(
+"--no-parallel-image-loading",
+dest="parallel_image_loading",
+action="store_false",
+help=argparse.SUPPRESS,
+default=True,
+)
 if add_interactive_options:
 arg_parser.add_argument(
 "-i",
diff --git a/lldb/examples/python/crashlog_scripted_process.py 
b/lldb/examples/python/crashlog_scripted_process.py
index 26c5c37b7371d..ebce4834ef198 100644
--- a/lldb/examples/python/crashlog_scripted_process.py
+++ b/lldb/examples/python/crashlog_scripted_process.py
@@ -84,6 +84,13 @@ def __init__(self, exe_ctx: lldb.SBExecutionContext, args: 
lldb.SBStructuredData
 if crashed_only.GetType() == lldb.eStructuredDataTypeBoolean:
 self.options.crashed_only = crashed_only.GetBooleanValue()
 
+parallel_image_loading = args.GetValueForKey("parallel_i

[Lldb-commits] [lldb] [lldb/crashlog] Add `--no-parallel-image-loading` hidden flag (PR #94513)

2024-06-05 Thread Jonas Devlieghere via lldb-commits



@@ -1720,6 +1735,13 @@ def CreateSymbolicateCrashLogOptions(
 help="show source for all threads, not just the crashed thread",
 default=False,
 )
+arg_parser.add_argument(
+"--no-parallel-image-loading",
+dest="parallel_image_loading",

JDevlieghere wrote:

The name of the flag and the variable seem to be opposites. Does argparse 
automatically change the value because the flag contains `-no`. 

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

[Lldb-commits] [lldb] [lldb/crashlog] Add `--no-parallel-image-loading` hidden flag (PR #94513)

2024-06-05 Thread Jonas Devlieghere via lldb-commits



@@ -555,30 +555,44 @@ def load_images(self, options, loaded_images=None):
 
 futures = []
 with tempfile.TemporaryDirectory() as obj_dir:
-with concurrent.futures.ThreadPoolExecutor() as executor:
 
-def add_module(image, target, obj_dir):
-return image, image.add_module(target, obj_dir)
+def add_module(image, target, obj_dir):
+return image, image.add_module(target, obj_dir)
+
+if options.parallel_image_loading:
+with concurrent.futures.ThreadPoolExecutor() as executor:

JDevlieghere wrote:

Instead of duplicating the code, I would limit the number of workers in the 
ThreadPool to one.  

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

[Lldb-commits] [lldb] [lldb/crashlog] Add `--no-parallel-image-loading` hidden flag (PR #94513)

2024-06-05 Thread Jonas Devlieghere via lldb-commits



@@ -1720,6 +1735,13 @@ def CreateSymbolicateCrashLogOptions(
 help="show source for all threads, not just the crashed thread",
 default=False,
 )
+arg_parser.add_argument(
+"--no-parallel-image-loading",
+dest="parallel_image_loading",
+action="store_false",
+help=argparse.SUPPRESS,
+default=True,

JDevlieghere wrote:

Does this mean parallel loading is on or off by default? The negation confuses 
me. 

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

[Lldb-commits] [lldb] 39e12e0 - [lldb] Update error message in TestDAP_optimized after #94420

2024-06-05 Thread Jonas Devlieghere via lldb-commits


Author: Jonas Devlieghere
Date: 2024-06-05T11:25:17-07:00
New Revision: 39e12e0ab2dd3b0ed9741c9796de15a020741727

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

LOG: [lldb] Update error message in TestDAP_optimized after #94420

Fixes:
  https://lab.llvm.org/buildbot/#/builders/68/builds/75571

Added: 


Modified: 
lldb/test/API/tools/lldb-dap/optimized/TestDAP_optimized.py

Removed: 




diff  --git a/lldb/test/API/tools/lldb-dap/optimized/TestDAP_optimized.py 
b/lldb/test/API/tools/lldb-dap/optimized/TestDAP_optimized.py
index dc7f4f98875fd..42a95de309a08 100644
--- a/lldb/test/API/tools/lldb-dap/optimized/TestDAP_optimized.py
+++ b/lldb/test/API/tools/lldb-dap/optimized/TestDAP_optimized.py
@@ -47,6 +47,6 @@ def test_optimized_variable(self):
 self.assertTrue(optimized_variable["value"].startswith("https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb/crashlog] Use environment variable to manually set dsymForUUIDBinary (PR #94517)

2024-06-05 Thread Med Ismail Bennani via lldb-commits


https://github.com/medismailben created 
https://github.com/llvm/llvm-project/pull/94517

In lldb, users can change the `dsymForUUID` binary using the 
`LLDB_APPLE_DSYMFORUUID_EXECUTABLE` environment variable.

This patch changes the crashlog to support the same behaviour as lldb and uses 
this environment variable to disable `dsymForUUID` lookups in crashlog test by 
having it be empty. Since CI bots shoudn't have access to images on build 
records, it doesn't make sense to make use of `dsymForUUID` in tests.

rdar://128953725

>From f9f743a48170d8c10cc954454f956028c896e252 Mon Sep 17 00:00:00 2001
From: Med Ismail Bennani 
Date: Wed, 5 Jun 2024 11:24:36 -0700
Subject: [PATCH] [lldb/crashlog] Use environment variable to manually set
 dsymForUUIDBinary

In lldb, users can change the `dsymForUUID` binary using the
`LLDB_APPLE_DSYMFORUUID_EXECUTABLE` environment variable.

This patch changes the crashlog to support the same behaviour as lldb
and uses this environment variable to disable `dsymForUUID` lookups in
crashlog test by having it be empty. Since CI bots shoudn't have
access to images on build records, it doesn't make sense to make use of
`dsymForUUID` in tests.

rdar://128953725

Signed-off-by: Med Ismail Bennani 
---
 lldb/examples/python/crashlog.py  | 4 +++-
 .../Shell/ScriptInterpreter/Python/Crashlog/lit.local.cfg | 2 ++
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/lldb/examples/python/crashlog.py b/lldb/examples/python/crashlog.py
index 641b2e64d53b1..c874cb4d32e66 100755
--- a/lldb/examples/python/crashlog.py
+++ b/lldb/examples/python/crashlog.py
@@ -284,7 +284,9 @@ class DarwinImage(symbolication.Image):
 """Class that represents a binary images in a darwin crash log"""
 
 dsymForUUIDBinary = "/usr/local/bin/dsymForUUID"
-if not os.path.exists(dsymForUUIDBinary):
+if "LLDB_APPLE_DSYMFORUUID_EXECUTABLE" in os.environ:
+dsymForUUIDBinary = os.environ["LLDB_APPLE_DSYMFORUUID_EXECUTABLE"]
+elif not os.path.exists(dsymForUUIDBinary):
 try:
 dsymForUUIDBinary = (
 subprocess.check_output("which dsymForUUID", shell=True)
diff --git a/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/lit.local.cfg 
b/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/lit.local.cfg
index 417069653d68e..3da9265b3553d 100644
--- a/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/lit.local.cfg
+++ b/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/lit.local.cfg
@@ -3,3 +3,5 @@ if 'system-darwin' not in config.available_features:
 
 if 'lldb-repro' in config.available_features:
   config.unsupported = True
+
+config.environment["LLDB_APPLE_DSYMFORUUID_EXECUTABLE"] = ""

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

[Lldb-commits] [lldb] [lldb/crashlog] Use environment variable to manually set dsymForUUIDBinary (PR #94517)

2024-06-05 Thread via lldb-commits


llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: Med Ismail Bennani (medismailben)


Changes

In lldb, users can change the `dsymForUUID` binary using the 
`LLDB_APPLE_DSYMFORUUID_EXECUTABLE` environment variable.

This patch changes the crashlog to support the same behaviour as lldb and uses 
this environment variable to disable `dsymForUUID` lookups in crashlog test by 
having it be empty. Since CI bots shoudn't have access to images on build 
records, it doesn't make sense to make use of `dsymForUUID` in tests.

rdar://128953725

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


2 Files Affected:

- (modified) lldb/examples/python/crashlog.py (+3-1) 
- (modified) lldb/test/Shell/ScriptInterpreter/Python/Crashlog/lit.local.cfg 
(+2) 


``diff
diff --git a/lldb/examples/python/crashlog.py b/lldb/examples/python/crashlog.py
index 641b2e64d53b1..c874cb4d32e66 100755
--- a/lldb/examples/python/crashlog.py
+++ b/lldb/examples/python/crashlog.py
@@ -284,7 +284,9 @@ class DarwinImage(symbolication.Image):
 """Class that represents a binary images in a darwin crash log"""
 
 dsymForUUIDBinary = "/usr/local/bin/dsymForUUID"
-if not os.path.exists(dsymForUUIDBinary):
+if "LLDB_APPLE_DSYMFORUUID_EXECUTABLE" in os.environ:
+dsymForUUIDBinary = os.environ["LLDB_APPLE_DSYMFORUUID_EXECUTABLE"]
+elif not os.path.exists(dsymForUUIDBinary):
 try:
 dsymForUUIDBinary = (
 subprocess.check_output("which dsymForUUID", shell=True)
diff --git a/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/lit.local.cfg 
b/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/lit.local.cfg
index 417069653d68e..3da9265b3553d 100644
--- a/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/lit.local.cfg
+++ b/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/lit.local.cfg
@@ -3,3 +3,5 @@ if 'system-darwin' not in config.available_features:
 
 if 'lldb-repro' in config.available_features:
   config.unsupported = True
+
+config.environment["LLDB_APPLE_DSYMFORUUID_EXECUTABLE"] = ""

``




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

[Lldb-commits] [lldb] [lldb/crashlog] Use environment variable to manually set dsymForUUIDBinary (PR #94517)

2024-06-05 Thread Jonas Devlieghere via lldb-commits


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


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

[Lldb-commits] [lldb] [lldb/crashlog] Add `--no-parallel-image-loading` hidden flag (PR #94513)

2024-06-05 Thread Med Ismail Bennani via lldb-commits



@@ -555,30 +555,44 @@ def load_images(self, options, loaded_images=None):
 
 futures = []
 with tempfile.TemporaryDirectory() as obj_dir:
-with concurrent.futures.ThreadPoolExecutor() as executor:
 
-def add_module(image, target, obj_dir):
-return image, image.add_module(target, obj_dir)
+def add_module(image, target, obj_dir):
+return image, image.add_module(target, obj_dir)
+
+if options.parallel_image_loading:
+with concurrent.futures.ThreadPoolExecutor() as executor:

medismailben wrote:

Good point!

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

[Lldb-commits] [lldb] [lldb][api-test] Add API test for SBCommandInterpreter::CommandOverrideCallback (PR #94518)

2024-06-05 Thread Chelsea Cassanova via lldb-commits


https://github.com/chelcassanova created 
https://github.com/llvm/llvm-project/pull/94518

`SBCommandInterpreter::CommandOverrideCallback` was not being exposed to the 
Python API has no coverage in the
API test suite, so this commits exposes and adds a test for it. Doing this 
involves also adding a typemap for the callback used for this function so that 
it matches the functionality of other callback functions that are exposed to 
Python.

>From 90602ed1b2f27fecb9e1fc28419587cf6bcb04bf Mon Sep 17 00:00:00 2001
From: Chelsea Cassanova 
Date: Wed, 5 Jun 2024 11:24:01 -0700
Subject: [PATCH] [lldb][api-test] Add API test for
 SBCommandInterpreter::CommandOverrideCallback

`SBCommandInterpreter::CommandOverrideCallback` was not being exposed to
the Python API has no coverage in the
API test suite, so this commits exposes and adds a test for it. Doing
this involves also adding a typemap for the callback used for this
function so that it matches the functionality of other callback
functions that are exposed to Python.
---
 lldb/bindings/python/python-typemaps.swig | 27 -
 lldb/bindings/python/python-wrapper.swig  | 13 
 lldb/include/lldb/API/SBCommandInterpreter.h  |  2 --
 .../TestCommandOverrideCallback.py| 30 +++
 4 files changed, 69 insertions(+), 3 deletions(-)
 create mode 100644 
lldb/test/API/python_api/interpreter/TestCommandOverrideCallback.py

diff --git a/lldb/bindings/python/python-typemaps.swig 
b/lldb/bindings/python/python-typemaps.swig
index 8d4b740e5f35c..1bf93c59fbc7b 100644
--- a/lldb/bindings/python/python-typemaps.swig
+++ b/lldb/bindings/python/python-typemaps.swig
@@ -427,7 +427,6 @@ template <> bool SetNumberFromPyObject(double 
&number, PyObject *obj) {
   free($1);
 }
 
-
 // For Log::LogOutputCallback
 %typemap(in) (lldb::LogOutputCallback log_callback, void *baton) {
   if (!($input == Py_None ||
@@ -476,6 +475,32 @@ template <> bool SetNumberFromPyObject(double 
&number, PyObject *obj) {
   $1 = $1 || PyCallable_Check(reinterpret_cast($input));
 }
 
+%typemap(in) (lldb::CommandOverrideCallback callback, void *baton) {
+  if (!($input == Py_None ||
+PyCallable_Check(reinterpret_cast($input {
+PyErr_SetString(PyExc_TypeError, "Need a callable object or None!");
+SWIG_fail;
+  }
+
+  // FIXME (filcab): We can't currently check if our callback is already
+  // LLDBSwigPythonCallPythonSBDebuggerTerminateCallback (to DECREF the 
previous
+  // baton) nor can we just remove all traces of a callback, if we want to
+  // revert to a file logging mechanism.
+
+  // Don't lose the callback reference
+  Py_INCREF($input);
+  $1 = LLDBSwigPythonCallPythonSBCommandInterpreterSetCommandOverrideCallback;
+  $2 = $input;
+}
+
+%typemap(typecheck) (lldb::CommandOverrideCallback callback,
+
+  void *baton) {
+
+  $1 = $input == Py_None;
+  $1 = $1 || PyCallable_Check(reinterpret_cast($input));
+}
+
 %typemap(in) lldb::FileSP {
   PythonFile py_file(PyRefType::Borrowed, $input);
   if (!py_file) {
diff --git a/lldb/bindings/python/python-wrapper.swig 
b/lldb/bindings/python/python-wrapper.swig
index 1370afc885d43..0a332cb0a2ff4 100644
--- a/lldb/bindings/python/python-wrapper.swig
+++ b/lldb/bindings/python/python-wrapper.swig
@@ -1099,6 +1099,19 @@ static void 
LLDBSwigPythonCallPythonSBDebuggerTerminateCallback(lldb::user_id_t
   }
 }
 
+static bool 
LLDBSwigPythonCallPythonSBCommandInterpreterSetCommandOverrideCallback(void 
*baton, const char **argv) {
+  bool b = false;
+  if (baton != Py_None) {
+SWIG_PYTHON_THREAD_BEGIN_BLOCK;
+PyObject *result = PyObject_CallFunction(
+  reinterpret_cast(baton), const_cast("s"), argv); // 
WRONG!
+b = result ? PyObject_IsTrue(result) : false;
+Py_XDECREF(result);
+SWIG_PYTHON_THREAD_END_BLOCK;
+  }
+  return b;
+}
+
 static SBError LLDBSwigPythonCallLocateModuleCallback(
 void *callback_baton, const SBModuleSpec &module_spec_sb,
 SBFileSpec &module_file_spec_sb, SBFileSpec &symbol_file_spec_sb) {
diff --git a/lldb/include/lldb/API/SBCommandInterpreter.h 
b/lldb/include/lldb/API/SBCommandInterpreter.h
index 8ac36344b3a79..084b6d9adb703 100644
--- a/lldb/include/lldb/API/SBCommandInterpreter.h
+++ b/lldb/include/lldb/API/SBCommandInterpreter.h
@@ -265,11 +265,9 @@ class SBCommandInterpreter {
   // Catch commands before they execute by registering a callback that will get
   // called when the command gets executed. This allows GUI or command line
   // interfaces to intercept a command and stop it from happening
-#ifndef SWIG
   bool SetCommandOverrideCallback(const char *command_name,
   lldb::CommandOverrideCallback callback,
   void *baton);
-#endif
 
   /// Return true if the command interpreter is the active IO handler.
   ///
diff --git 
a/lldb/test/API/python_api/interpreter/TestCommandOverrideCallback.py 
b/lldb/test/API/python_api/interpreter/

[Lldb-commits] [lldb] [lldb][api-test] Add API test for SBCommandInterpreter::CommandOverrideCallback (PR #94518)

2024-06-05 Thread via lldb-commits


llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: Chelsea Cassanova (chelcassanova)


Changes

`SBCommandInterpreter::CommandOverrideCallback` was not being exposed to the 
Python API has no coverage in the
API test suite, so this commits exposes and adds a test for it. Doing this 
involves also adding a typemap for the callback used for this function so that 
it matches the functionality of other callback functions that are exposed to 
Python.

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


4 Files Affected:

- (modified) lldb/bindings/python/python-typemaps.swig (+26-1) 
- (modified) lldb/bindings/python/python-wrapper.swig (+13) 
- (modified) lldb/include/lldb/API/SBCommandInterpreter.h (-2) 
- (added) lldb/test/API/python_api/interpreter/TestCommandOverrideCallback.py 
(+30) 


``diff
diff --git a/lldb/bindings/python/python-typemaps.swig 
b/lldb/bindings/python/python-typemaps.swig
index 8d4b740e5f35c..1bf93c59fbc7b 100644
--- a/lldb/bindings/python/python-typemaps.swig
+++ b/lldb/bindings/python/python-typemaps.swig
@@ -427,7 +427,6 @@ template <> bool SetNumberFromPyObject(double 
&number, PyObject *obj) {
   free($1);
 }
 
-
 // For Log::LogOutputCallback
 %typemap(in) (lldb::LogOutputCallback log_callback, void *baton) {
   if (!($input == Py_None ||
@@ -476,6 +475,32 @@ template <> bool SetNumberFromPyObject(double 
&number, PyObject *obj) {
   $1 = $1 || PyCallable_Check(reinterpret_cast($input));
 }
 
+%typemap(in) (lldb::CommandOverrideCallback callback, void *baton) {
+  if (!($input == Py_None ||
+PyCallable_Check(reinterpret_cast($input {
+PyErr_SetString(PyExc_TypeError, "Need a callable object or None!");
+SWIG_fail;
+  }
+
+  // FIXME (filcab): We can't currently check if our callback is already
+  // LLDBSwigPythonCallPythonSBDebuggerTerminateCallback (to DECREF the 
previous
+  // baton) nor can we just remove all traces of a callback, if we want to
+  // revert to a file logging mechanism.
+
+  // Don't lose the callback reference
+  Py_INCREF($input);
+  $1 = LLDBSwigPythonCallPythonSBCommandInterpreterSetCommandOverrideCallback;
+  $2 = $input;
+}
+
+%typemap(typecheck) (lldb::CommandOverrideCallback callback,
+
+  void *baton) {
+
+  $1 = $input == Py_None;
+  $1 = $1 || PyCallable_Check(reinterpret_cast($input));
+}
+
 %typemap(in) lldb::FileSP {
   PythonFile py_file(PyRefType::Borrowed, $input);
   if (!py_file) {
diff --git a/lldb/bindings/python/python-wrapper.swig 
b/lldb/bindings/python/python-wrapper.swig
index 1370afc885d43..0a332cb0a2ff4 100644
--- a/lldb/bindings/python/python-wrapper.swig
+++ b/lldb/bindings/python/python-wrapper.swig
@@ -1099,6 +1099,19 @@ static void 
LLDBSwigPythonCallPythonSBDebuggerTerminateCallback(lldb::user_id_t
   }
 }
 
+static bool 
LLDBSwigPythonCallPythonSBCommandInterpreterSetCommandOverrideCallback(void 
*baton, const char **argv) {
+  bool b = false;
+  if (baton != Py_None) {
+SWIG_PYTHON_THREAD_BEGIN_BLOCK;
+PyObject *result = PyObject_CallFunction(
+  reinterpret_cast(baton), const_cast("s"), argv); // 
WRONG!
+b = result ? PyObject_IsTrue(result) : false;
+Py_XDECREF(result);
+SWIG_PYTHON_THREAD_END_BLOCK;
+  }
+  return b;
+}
+
 static SBError LLDBSwigPythonCallLocateModuleCallback(
 void *callback_baton, const SBModuleSpec &module_spec_sb,
 SBFileSpec &module_file_spec_sb, SBFileSpec &symbol_file_spec_sb) {
diff --git a/lldb/include/lldb/API/SBCommandInterpreter.h 
b/lldb/include/lldb/API/SBCommandInterpreter.h
index 8ac36344b3a79..084b6d9adb703 100644
--- a/lldb/include/lldb/API/SBCommandInterpreter.h
+++ b/lldb/include/lldb/API/SBCommandInterpreter.h
@@ -265,11 +265,9 @@ class SBCommandInterpreter {
   // Catch commands before they execute by registering a callback that will get
   // called when the command gets executed. This allows GUI or command line
   // interfaces to intercept a command and stop it from happening
-#ifndef SWIG
   bool SetCommandOverrideCallback(const char *command_name,
   lldb::CommandOverrideCallback callback,
   void *baton);
-#endif
 
   /// Return true if the command interpreter is the active IO handler.
   ///
diff --git 
a/lldb/test/API/python_api/interpreter/TestCommandOverrideCallback.py 
b/lldb/test/API/python_api/interpreter/TestCommandOverrideCallback.py
new file mode 100644
index 0..067f37f88096d
--- /dev/null
+++ b/lldb/test/API/python_api/interpreter/TestCommandOverrideCallback.py
@@ -0,0 +1,30 @@
+from typing_extensions import override
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class CommandOverrideCallback(TestBase):
+def setUp(self):
+TestBase.setUp(self)
+self.line = line_number("main.c", "Hello world.")
+
+def test_command_override_callback(self):
+self.build()
+

[Lldb-commits] [lldb] [lldb][api-test] Add API test for SBCommandInterpreter::CommandOverrideCallback (PR #94518)

2024-06-05 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 
c6c08eee37bada190bd1aa4593c88a5e2c8cdaac...90602ed1b2f27fecb9e1fc28419587cf6bcb04bf
 lldb/test/API/python_api/interpreter/TestCommandOverrideCallback.py
``





View the diff from darker here.


``diff
--- TestCommandOverrideCallback.py  2024-06-05 18:31:49.00 +
+++ TestCommandOverrideCallback.py  2024-06-05 18:35:47.810046 +
@@ -1,10 +1,11 @@
 from typing_extensions import override
 import lldb
 from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
+
 
 class CommandOverrideCallback(TestBase):
 def setUp(self):
 TestBase.setUp(self)
 self.line = line_number("main.c", "Hello world.")
@@ -20,10 +21,11 @@
 # Retrieve the associated command interpreter from our debugger.
 ci = self.dbg.GetCommandInterpreter()
 self.assertTrue(ci, VALID_COMMAND_INTERPRETER)
 
 override_string = "what"
+
 def foo(string):
 nonlocal override_string
 override_string = string
 return False
 

``




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

[Lldb-commits] [lldb] [lldb/crashlog] Add `--no-parallel-image-loading` hidden flag (PR #94513)

2024-06-05 Thread Med Ismail Bennani via lldb-commits


https://github.com/medismailben updated 
https://github.com/llvm/llvm-project/pull/94513

>From eb62de94ceaf3ee440046a2884210fb4f8774edf Mon Sep 17 00:00:00 2001
From: Med Ismail Bennani 
Date: Wed, 5 Jun 2024 12:28:59 -0700
Subject: [PATCH] [lldb/crashlog] Add `--no-parallel-image-loading` hidden flag

This patch adds the `--no-parallel-image-loading` to the crashlog
command. By default, image loading will happen in parallel in the
crashlog script however, sometimes, when running tests or debugging the
crashlog script itself, it's better to load the images sequentially.

As its name suggests, this flag will disable the default image loading
behaviour to load all the images sequencially in the main thread.

Signed-off-by: Med Ismail Bennani 
---
 lldb/examples/python/crashlog.py   | 18 +++---
 .../python/crashlog_scripted_process.py|  7 +++
 2 files changed, 22 insertions(+), 3 deletions(-)

diff --git a/lldb/examples/python/crashlog.py b/lldb/examples/python/crashlog.py
index 641b2e64d53b1..e191bb4165259 100755
--- a/lldb/examples/python/crashlog.py
+++ b/lldb/examples/python/crashlog.py
@@ -555,11 +555,15 @@ def load_images(self, options, loaded_images=None):
 
 futures = []
 with tempfile.TemporaryDirectory() as obj_dir:
-with concurrent.futures.ThreadPoolExecutor() as executor:
 
-def add_module(image, target, obj_dir):
-return image, image.add_module(target, obj_dir)
+def add_module(image, target, obj_dir):
+return image, image.add_module(target, obj_dir)
 
+max_worker = None
+if options.no_parallel_image_loading:
+max_worker = 1
+
+with concurrent.futures.ThreadPoolExecutor(max_worker) as executor:
 for image in images_to_load:
 if image not in loaded_images:
 if image.uuid == uuid.UUID(int=0):
@@ -1528,6 +1532,7 @@ def load_crashlog_in_scripted_process(debugger, 
crashlog_path, options, result):
 "file_path": crashlog_path,
 "load_all_images": options.load_all_images,
 "crashed_only": options.crashed_only,
+"no_parallel_image_loading": options.no_parallel_image_loading,
 }
 )
 )
@@ -1720,6 +1725,13 @@ def CreateSymbolicateCrashLogOptions(
 help="show source for all threads, not just the crashed thread",
 default=False,
 )
+arg_parser.add_argument(
+"--no-parallel-image-loading",
+dest="no_parallel_image_loading",
+action="store_true",
+help=argparse.SUPPRESS,
+default=False,
+)
 if add_interactive_options:
 arg_parser.add_argument(
 "-i",
diff --git a/lldb/examples/python/crashlog_scripted_process.py 
b/lldb/examples/python/crashlog_scripted_process.py
index 26c5c37b7371d..ebce4834ef198 100644
--- a/lldb/examples/python/crashlog_scripted_process.py
+++ b/lldb/examples/python/crashlog_scripted_process.py
@@ -84,6 +84,13 @@ def __init__(self, exe_ctx: lldb.SBExecutionContext, args: 
lldb.SBStructuredData
 if crashed_only.GetType() == lldb.eStructuredDataTypeBoolean:
 self.options.crashed_only = crashed_only.GetBooleanValue()
 
+parallel_image_loading = args.GetValueForKey("parallel_image_loading")
+if parallel_image_loading and parallel_image_loading.IsValid():
+if parallel_image_loading.GetType() == 
lldb.eStructuredDataTypeBoolean:
+self.options.parallel_image_loading = (
+parallel_image_loading.GetBooleanValue()
+)
+
 self.pid = super().get_process_id()
 self.crashed_thread_idx = 0
 self.exception = None

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

[Lldb-commits] [lldb] [lldb][api-test] Add API test for SBCommandInterpreter::CommandOverrideCallback (PR #94518)

2024-06-05 Thread Chelsea Cassanova via lldb-commits


https://github.com/chelcassanova updated 
https://github.com/llvm/llvm-project/pull/94518

>From e4ba69ac4983e4e2882200e7efe0e850eeefae21 Mon Sep 17 00:00:00 2001
From: Chelsea Cassanova 
Date: Wed, 5 Jun 2024 11:24:01 -0700
Subject: [PATCH] [lldb][api-test] Add API test for
 SBCommandInterpreter::CommandOverrideCallback

`SBCommandInterpreter::CommandOverrideCallback` was not being exposed to
the Python API has no coverage in the
API test suite, so this commits exposes and adds a test for it. Doing
this involves also adding a typemap for the callback used for this
function so that it matches the functionality of other callback
functions that are exposed to Python.
---
 lldb/bindings/python/python-typemaps.swig | 27 -
 lldb/bindings/python/python-wrapper.swig  | 13 
 lldb/include/lldb/API/SBCommandInterpreter.h  |  2 --
 .../TestCommandOverrideCallback.py| 30 +++
 4 files changed, 69 insertions(+), 3 deletions(-)
 create mode 100644 
lldb/test/API/python_api/interpreter/TestCommandOverrideCallback.py

diff --git a/lldb/bindings/python/python-typemaps.swig 
b/lldb/bindings/python/python-typemaps.swig
index 8d4b740e5f35c..1bf93c59fbc7b 100644
--- a/lldb/bindings/python/python-typemaps.swig
+++ b/lldb/bindings/python/python-typemaps.swig
@@ -427,7 +427,6 @@ template <> bool SetNumberFromPyObject(double 
&number, PyObject *obj) {
   free($1);
 }
 
-
 // For Log::LogOutputCallback
 %typemap(in) (lldb::LogOutputCallback log_callback, void *baton) {
   if (!($input == Py_None ||
@@ -476,6 +475,32 @@ template <> bool SetNumberFromPyObject(double 
&number, PyObject *obj) {
   $1 = $1 || PyCallable_Check(reinterpret_cast($input));
 }
 
+%typemap(in) (lldb::CommandOverrideCallback callback, void *baton) {
+  if (!($input == Py_None ||
+PyCallable_Check(reinterpret_cast($input {
+PyErr_SetString(PyExc_TypeError, "Need a callable object or None!");
+SWIG_fail;
+  }
+
+  // FIXME (filcab): We can't currently check if our callback is already
+  // LLDBSwigPythonCallPythonSBDebuggerTerminateCallback (to DECREF the 
previous
+  // baton) nor can we just remove all traces of a callback, if we want to
+  // revert to a file logging mechanism.
+
+  // Don't lose the callback reference
+  Py_INCREF($input);
+  $1 = LLDBSwigPythonCallPythonSBCommandInterpreterSetCommandOverrideCallback;
+  $2 = $input;
+}
+
+%typemap(typecheck) (lldb::CommandOverrideCallback callback,
+
+  void *baton) {
+
+  $1 = $input == Py_None;
+  $1 = $1 || PyCallable_Check(reinterpret_cast($input));
+}
+
 %typemap(in) lldb::FileSP {
   PythonFile py_file(PyRefType::Borrowed, $input);
   if (!py_file) {
diff --git a/lldb/bindings/python/python-wrapper.swig 
b/lldb/bindings/python/python-wrapper.swig
index 1370afc885d43..0a332cb0a2ff4 100644
--- a/lldb/bindings/python/python-wrapper.swig
+++ b/lldb/bindings/python/python-wrapper.swig
@@ -1099,6 +1099,19 @@ static void 
LLDBSwigPythonCallPythonSBDebuggerTerminateCallback(lldb::user_id_t
   }
 }
 
+static bool 
LLDBSwigPythonCallPythonSBCommandInterpreterSetCommandOverrideCallback(void 
*baton, const char **argv) {
+  bool b = false;
+  if (baton != Py_None) {
+SWIG_PYTHON_THREAD_BEGIN_BLOCK;
+PyObject *result = PyObject_CallFunction(
+  reinterpret_cast(baton), const_cast("s"), argv); // 
WRONG!
+b = result ? PyObject_IsTrue(result) : false;
+Py_XDECREF(result);
+SWIG_PYTHON_THREAD_END_BLOCK;
+  }
+  return b;
+}
+
 static SBError LLDBSwigPythonCallLocateModuleCallback(
 void *callback_baton, const SBModuleSpec &module_spec_sb,
 SBFileSpec &module_file_spec_sb, SBFileSpec &symbol_file_spec_sb) {
diff --git a/lldb/include/lldb/API/SBCommandInterpreter.h 
b/lldb/include/lldb/API/SBCommandInterpreter.h
index 8ac36344b3a79..084b6d9adb703 100644
--- a/lldb/include/lldb/API/SBCommandInterpreter.h
+++ b/lldb/include/lldb/API/SBCommandInterpreter.h
@@ -265,11 +265,9 @@ class SBCommandInterpreter {
   // Catch commands before they execute by registering a callback that will get
   // called when the command gets executed. This allows GUI or command line
   // interfaces to intercept a command and stop it from happening
-#ifndef SWIG
   bool SetCommandOverrideCallback(const char *command_name,
   lldb::CommandOverrideCallback callback,
   void *baton);
-#endif
 
   /// Return true if the command interpreter is the active IO handler.
   ///
diff --git 
a/lldb/test/API/python_api/interpreter/TestCommandOverrideCallback.py 
b/lldb/test/API/python_api/interpreter/TestCommandOverrideCallback.py
new file mode 100644
index 0..067f37f88096d
--- /dev/null
+++ b/lldb/test/API/python_api/interpreter/TestCommandOverrideCallback.py
@@ -0,0 +1,30 @@
+from typing_extensions import override
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import ll

[Lldb-commits] [lldb] [lldb][api-test] Add API test for SBCommandInterpreter::CommandOverrideCallback (PR #94518)

2024-06-05 Thread Med Ismail Bennani via lldb-commits


https://github.com/medismailben requested changes to this pull request.


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

[Lldb-commits] [lldb] [lldb][api-test] Add API test for SBCommandInterpreter::CommandOverrideCallback (PR #94518)

2024-06-05 Thread Med Ismail Bennani via lldb-commits


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

[Lldb-commits] [lldb] [lldb][api-test] Add API test for SBCommandInterpreter::CommandOverrideCallback (PR #94518)

2024-06-05 Thread Med Ismail Bennani via lldb-commits



@@ -1099,6 +1099,19 @@ static void 
LLDBSwigPythonCallPythonSBDebuggerTerminateCallback(lldb::user_id_t
   }
 }
 
+static bool 
LLDBSwigPythonCallPythonSBCommandInterpreterSetCommandOverrideCallback(void 
*baton, const char **argv) {
+  bool b = false;
+  if (baton != Py_None) {
+SWIG_PYTHON_THREAD_BEGIN_BLOCK;
+PyObject *result = PyObject_CallFunction(
+  reinterpret_cast(baton), const_cast("s"), argv); // 
WRONG!

medismailben wrote:

You need to convert your `char**` into a `PyList` of `PyString` before passing 
it here I think

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

[Lldb-commits] [lldb] [lldb][api-test] Add API test for SBCommandInterpreter::CommandOverrideCallback (PR #94518)

2024-06-05 Thread Med Ismail Bennani via lldb-commits



@@ -476,6 +475,32 @@ template <> bool SetNumberFromPyObject(double 
&number, PyObject *obj) {
   $1 = $1 || PyCallable_Check(reinterpret_cast($input));
 }
 
+%typemap(in) (lldb::CommandOverrideCallback callback, void *baton) {
+  if (!($input == Py_None ||
+PyCallable_Check(reinterpret_cast($input {
+PyErr_SetString(PyExc_TypeError, "Need a callable object or None!");
+SWIG_fail;
+  }
+
+  // FIXME (filcab): We can't currently check if our callback is already

medismailben wrote:

filcab ?

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

[Lldb-commits] [lldb] [lldb] Use packaging module instead of pkg_resources (PR #93712)

2024-06-05 Thread Daniel Thornburgh via lldb-commits


mysterymath wrote:

@JDevlieghere , we just landed a patch that adds packaging for Fuchsia. Should 
be good to go ahead.

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

[Lldb-commits] [lldb] [lldb/crashlog] Add `--no-parallel-image-loading` hidden flag (PR #94513)

2024-06-05 Thread Med Ismail Bennani via lldb-commits


https://github.com/medismailben updated 
https://github.com/llvm/llvm-project/pull/94513

>From cc405078245b248b2102f2b24272bee17e5ec752 Mon Sep 17 00:00:00 2001
From: Med Ismail Bennani 
Date: Wed, 5 Jun 2024 13:12:01 -0700
Subject: [PATCH] [lldb/crashlog] Add `--no-parallel-image-loading` hidden flag

This patch adds the `--no-parallel-image-loading` to the crashlog
command. By default, image loading will happen in parallel in the
crashlog script however, sometimes, when running tests or debugging the
crashlog script itself, it's better to load the images sequentially.

As its name suggests, this flag will disable the default image loading
behaviour to load all the images sequencially in the main thread.

Signed-off-by: Med Ismail Bennani 
---
 lldb/examples/python/crashlog.py   | 18 +++---
 .../python/crashlog_scripted_process.py|  8 
 2 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/lldb/examples/python/crashlog.py b/lldb/examples/python/crashlog.py
index 641b2e64d53b1..e191bb4165259 100755
--- a/lldb/examples/python/crashlog.py
+++ b/lldb/examples/python/crashlog.py
@@ -555,11 +555,15 @@ def load_images(self, options, loaded_images=None):
 
 futures = []
 with tempfile.TemporaryDirectory() as obj_dir:
-with concurrent.futures.ThreadPoolExecutor() as executor:
 
-def add_module(image, target, obj_dir):
-return image, image.add_module(target, obj_dir)
+def add_module(image, target, obj_dir):
+return image, image.add_module(target, obj_dir)
 
+max_worker = None
+if options.no_parallel_image_loading:
+max_worker = 1
+
+with concurrent.futures.ThreadPoolExecutor(max_worker) as executor:
 for image in images_to_load:
 if image not in loaded_images:
 if image.uuid == uuid.UUID(int=0):
@@ -1528,6 +1532,7 @@ def load_crashlog_in_scripted_process(debugger, 
crashlog_path, options, result):
 "file_path": crashlog_path,
 "load_all_images": options.load_all_images,
 "crashed_only": options.crashed_only,
+"no_parallel_image_loading": options.no_parallel_image_loading,
 }
 )
 )
@@ -1720,6 +1725,13 @@ def CreateSymbolicateCrashLogOptions(
 help="show source for all threads, not just the crashed thread",
 default=False,
 )
+arg_parser.add_argument(
+"--no-parallel-image-loading",
+dest="no_parallel_image_loading",
+action="store_true",
+help=argparse.SUPPRESS,
+default=False,
+)
 if add_interactive_options:
 arg_parser.add_argument(
 "-i",
diff --git a/lldb/examples/python/crashlog_scripted_process.py 
b/lldb/examples/python/crashlog_scripted_process.py
index 26c5c37b7371d..2ee030239ee37 100644
--- a/lldb/examples/python/crashlog_scripted_process.py
+++ b/lldb/examples/python/crashlog_scripted_process.py
@@ -53,6 +53,7 @@ def set_crashlog(self, crashlog):
 class CrashLogOptions:
 load_all_images = False
 crashed_only = True
+no_parallel_image_loading = False
 
 def __init__(self, exe_ctx: lldb.SBExecutionContext, args: 
lldb.SBStructuredData):
 super().__init__(exe_ctx, args)
@@ -84,6 +85,13 @@ def __init__(self, exe_ctx: lldb.SBExecutionContext, args: 
lldb.SBStructuredData
 if crashed_only.GetType() == lldb.eStructuredDataTypeBoolean:
 self.options.crashed_only = crashed_only.GetBooleanValue()
 
+no_parallel_image_loading = 
args.GetValueForKey("no_parallel_image_loading")
+if no_parallel_image_loading and no_parallel_image_loading.IsValid():
+if no_parallel_image_loading.GetType() == 
lldb.eStructuredDataTypeBoolean:
+self.options.no_parallel_image_loading = (
+no_parallel_image_loading.GetBooleanValue()
+)
+
 self.pid = super().get_process_id()
 self.crashed_thread_idx = 0
 self.exception = None

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

[Lldb-commits] [lldb] f8afa76 - [lldb/crashlog] Use environment variable to manually set dsymForUUIDBinary (#94517)

2024-06-05 Thread via lldb-commits


Author: Med Ismail Bennani
Date: 2024-06-05T13:21:27-07:00
New Revision: f8afa763c6194f5bf485480e1fb94b953942f876

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

LOG: [lldb/crashlog] Use environment variable to manually set dsymForUUIDBinary 
(#94517)

In lldb, users can change the `dsymForUUID` binary using the
`LLDB_APPLE_DSYMFORUUID_EXECUTABLE` environment variable.

This patch changes the crashlog to support the same behaviour as lldb
and uses this environment variable to disable `dsymForUUID` lookups in
crashlog test by having it be empty. Since CI bots shoudn't have access
to images on build records, it doesn't make sense to make use of
`dsymForUUID` in tests.

rdar://128953725

Signed-off-by: Med Ismail Bennani 

Added: 


Modified: 
lldb/examples/python/crashlog.py
lldb/test/Shell/ScriptInterpreter/Python/Crashlog/lit.local.cfg

Removed: 




diff  --git a/lldb/examples/python/crashlog.py 
b/lldb/examples/python/crashlog.py
index 641b2e64d53b1..c874cb4d32e66 100755
--- a/lldb/examples/python/crashlog.py
+++ b/lldb/examples/python/crashlog.py
@@ -284,7 +284,9 @@ class DarwinImage(symbolication.Image):
 """Class that represents a binary images in a darwin crash log"""
 
 dsymForUUIDBinary = "/usr/local/bin/dsymForUUID"
-if not os.path.exists(dsymForUUIDBinary):
+if "LLDB_APPLE_DSYMFORUUID_EXECUTABLE" in os.environ:
+dsymForUUIDBinary = os.environ["LLDB_APPLE_DSYMFORUUID_EXECUTABLE"]
+elif not os.path.exists(dsymForUUIDBinary):
 try:
 dsymForUUIDBinary = (
 subprocess.check_output("which dsymForUUID", shell=True)

diff  --git a/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/lit.local.cfg 
b/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/lit.local.cfg
index 417069653d68e..3da9265b3553d 100644
--- a/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/lit.local.cfg
+++ b/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/lit.local.cfg
@@ -3,3 +3,5 @@ if 'system-darwin' not in config.available_features:
 
 if 'lldb-repro' in config.available_features:
   config.unsupported = True
+
+config.environment["LLDB_APPLE_DSYMFORUUID_EXECUTABLE"] = ""



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

[Lldb-commits] [lldb] [lldb/crashlog] Use environment variable to manually set dsymForUUIDBinary (PR #94517)

2024-06-05 Thread Med Ismail Bennani via lldb-commits


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

[Lldb-commits] [lldb] [lldb][api-test] Add API test for SBCommandInterpreter::CommandOverrideCallback (PR #94518)

2024-06-05 Thread Chelsea Cassanova via lldb-commits



@@ -1099,6 +1099,19 @@ static void 
LLDBSwigPythonCallPythonSBDebuggerTerminateCallback(lldb::user_id_t
   }
 }
 
+static bool 
LLDBSwigPythonCallPythonSBCommandInterpreterSetCommandOverrideCallback(void 
*baton, const char **argv) {
+  bool b = false;
+  if (baton != Py_None) {
+SWIG_PYTHON_THREAD_BEGIN_BLOCK;
+PyObject *result = PyObject_CallFunction(
+  reinterpret_cast(baton), const_cast("s"), argv); // 
WRONG!

chelcassanova wrote:

Yes, forgot to do this here when I cleaned this up and you can see the comment 
from that 😅 

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

[Lldb-commits] [lldb] [LLDB] Add more helper functions to ValueObject class. (PR #87197)

2024-06-05 Thread via lldb-commits



@@ -3015,11 +3017,12 @@ llvm::Expected 
ValueObject::CastDerivedToBaseType(
 
   lldb::TargetSP target = GetTargetSP();
   // The `value` can be a pointer, but GetChildAtIndex works for pointers too.
-  lldb::ValueObjectSP inner_value;
+  lldb::ValueObjectSP inner_value = GetSP();
 
   for (const uint32_t i : base_type_indices)
 // Force static value, otherwise we can end up with the "real" type.
-inner_value = GetChildAtIndex(i, /*can_create_synthetic*/ false);

jimingham wrote:

I don't understand this change.  How is getting 
`this->GetSP()->GetChildAtIndex` different from `this->GetChildAtIndex`?

The other change here seems to be to change can_create_synthetic from false to 
true.  IIUC, the comment directly above this change says why it chose `false` 
here, though it is a somewhat confusing comment.  Since you seem to disagree 
with that comment, you should probably change the comment to say why you think 
`true` is the right value here.

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

[Lldb-commits] [lldb] [LLDB] Add more helper functions to ValueObject class. (PR #87197)

2024-06-05 Thread via lldb-commits



@@ -3015,11 +3017,12 @@ llvm::Expected 
ValueObject::CastDerivedToBaseType(
 
   lldb::TargetSP target = GetTargetSP();
   // The `value` can be a pointer, but GetChildAtIndex works for pointers too.
-  lldb::ValueObjectSP inner_value;
+  lldb::ValueObjectSP inner_value = GetSP();
 
   for (const uint32_t i : base_type_indices)
 // Force static value, otherwise we can end up with the "real" type.
-inner_value = GetChildAtIndex(i, /*can_create_synthetic*/ false);

cmtice wrote:

The difference is that now in each loop iteration the rhs is 
'inner_loop->GetChildAtIndex...', which will be different each iteration 
through the loop; before it was just 'GetChildAtIndex', which did not change on 
each iteration.

re false->true: The original lldb-eval code on which that was based used 
'false', but it was also making the call through the SB API interface 
(SBValues, SBTypes, etc). I found that the cast test cases passed with 
lldb-eval when this value was false, but failed in my translated, migrated 
version. Setting this to true allows those failing test cases to pass.

I don't have any easy way to add those test cases to this PR because they all 
use 'frame variable' (using my DIL implementation) for doing the casts...But I 
do have them, and I am running (and passing) them.

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

[Lldb-commits] [lldb] [LLDB] Add more helper functions to ValueObject class. (PR #87197)

2024-06-05 Thread via lldb-commits



@@ -3138,13 +3141,13 @@ lldb::ValueObjectSP 
ValueObject::CastToBasicType(CompilerType type) {
 val_byte_size = temp.value();
 
   if (is_pointer) {
-if (type.IsBoolean() && type_byte_size < val_byte_size) {
+if (!type.IsBoolean() && type_byte_size < val_byte_size) {

jimingham wrote:

The `!` here seems right, but still this logic is odd.  

With this code, if I pass 32 bit float (on a 64 bit pointers system), the error 
I'll get is that it's too small.  OTOH, if I pass a 64 bit float, I'll get 
"must be integer".  That's confusing.  If you aren't planning to take anything 
but integers, it seems like you should check that first, then check the size.

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

[Lldb-commits] [lldb] [LLDB] Add more helper functions to ValueObject class. (PR #87197)

2024-06-05 Thread via lldb-commits


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

[Lldb-commits] [lldb] [lldb] [NFC] Rewrite TestRedefinitionsInInlines.py as an API test (PR #94539)

2024-06-05 Thread Jason Molenda via lldb-commits


https://github.com/jasonmolenda created 
https://github.com/llvm/llvm-project/pull/94539

Rewrite an inline test as an API test, to be a little easier to debug, and add 
some additional checks that we're in the inlined test1, then step and we are 
now in the inlined test2 functions.

>From c02e38e5ef017a966ef8b94a56e394309462 Mon Sep 17 00:00:00 2001
From: Jason Molenda 
Date: Wed, 5 Jun 2024 15:27:23 -0700
Subject: [PATCH] [lldb] [NFC] Rewrite TestRedefinitionsInInlines.py as an API
 test

Rewrite an inline test as an API test, to be a little easier to
debug, and add some additional checks that we're in the inlined
test1, then step and we are now in the inlined test2 functions.
---
 lldb/test/API/lang/c/inlines/Makefile |  3 +
 .../c/inlines/TestRedefinitionsInInlines.py   | 73 +++
 lldb/test/API/lang/c/inlines/main.c   | 23 +++---
 3 files changed, 74 insertions(+), 25 deletions(-)
 create mode 100644 lldb/test/API/lang/c/inlines/Makefile

diff --git a/lldb/test/API/lang/c/inlines/Makefile 
b/lldb/test/API/lang/c/inlines/Makefile
new file mode 100644
index 0..f9555f92be8cd
--- /dev/null
+++ b/lldb/test/API/lang/c/inlines/Makefile
@@ -0,0 +1,3 @@
+C_SOURCES := main.c 
+
+include Makefile.rules
diff --git a/lldb/test/API/lang/c/inlines/TestRedefinitionsInInlines.py 
b/lldb/test/API/lang/c/inlines/TestRedefinitionsInInlines.py
index 024b9dad9b0fb..d48f88ad7d563 100644
--- a/lldb/test/API/lang/c/inlines/TestRedefinitionsInInlines.py
+++ b/lldb/test/API/lang/c/inlines/TestRedefinitionsInInlines.py
@@ -1,14 +1,61 @@
-from lldbsuite.test import lldbinline
-from lldbsuite.test import decorators
-
-lldbinline.MakeInlineTest(
-__file__,
-globals(),
-[
-decorators.expectedFailureAll(
-compiler="clang",
-compiler_version=["<", "3.5"],
-bugnumber="llvm.org/pr27845",
+"""Test that inlined argument variables have their correct location in 
debuginfo"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TestRedefinitionsInInlines(TestBase):
+
+# https://github.com/llvm/llvm-project/issues/28219
+@skipIf(compiler="clang", compiler_version=["<", "3.5"])
+def test(self):
+self.source = "main.c"
+self.build()
+(target, process, thread, bp1) = lldbutil.run_to_source_breakpoint(
+self, "first breakpoint", lldb.SBFileSpec(self.source, False)
 )
-],
-)
+
+bp2 = target.BreakpointCreateBySourceRegex(
+"second breakpoint", lldb.SBFileSpec(self.source, False)
+)
+bp3 = target.BreakpointCreateBySourceRegex(
+"third breakpoint", lldb.SBFileSpec(self.source, False)
+)
+
+# When called from main(), test2 is passed in the value of 42 in 'b'
+self.expect("expression b", DATA_TYPES_DISPLAYED_CORRECTLY, 
substrs=["42"])
+
+process.Continue()
+
+self.assertState(process.GetState(), lldb.eStateStopped)
+thread = lldbutil.get_stopped_thread(process, 
lldb.eStopReasonBreakpoint)
+self.assertIsNotNone(thread)
+bp_id = thread.GetStopReasonDataAtIndex(0)
+self.assertEqual(bp_id, bp2.GetID())
+
+self.expect("expression b", DATA_TYPES_DISPLAYED_CORRECTLY, 
substrs=["42"])
+self.expect("expression c", DATA_TYPES_DISPLAYED_CORRECTLY, 
substrs=["84"])
+
+process.Continue()
+
+# Now we're in test1(), and the first thing it does is call test2(24). 
 "Step in"
+# and check that we have the value 24 as the argument.
+self.assertState(process.GetState(), lldb.eStateStopped)
+thread = lldbutil.get_stopped_thread(process, 
lldb.eStopReasonBreakpoint)
+self.assertIsNotNone(thread)
+bp_id = thread.GetStopReasonDataAtIndex(0)
+self.assertEqual(bp_id, bp3.GetID())
+
+frame = thread.GetFrameAtIndex(0)
+self.assertTrue(frame.IsInlined())
+self.assertEqual(frame.GetFunctionName(), "test1")
+
+thread.StepInto()
+
+frame = thread.GetFrameAtIndex(0)
+self.assertTrue(frame.IsInlined())
+self.assertEqual(frame.GetFunctionName(), "test2")
+
+self.expect("expression b", DATA_TYPES_DISPLAYED_CORRECTLY, 
substrs=["24"])
diff --git a/lldb/test/API/lang/c/inlines/main.c 
b/lldb/test/API/lang/c/inlines/main.c
index 8fe49180800bd..6ecc04dd508fc 100644
--- a/lldb/test/API/lang/c/inlines/main.c
+++ b/lldb/test/API/lang/c/inlines/main.c
@@ -3,23 +3,22 @@
 inline void test1(int) __attribute__ ((always_inline));
 inline void test2(int) __attribute__ ((always_inline));
 
+// Called once from main with b==42 then called from test1 with b==24.
 void test2(int b) {
-printf("test2(%d)\n", b); //% self.expect("expression b", 
DATA_TYPES_DISPLAYED_CORRECTLY, substrs = ["42"])
-{
-  int c = b * 2;
-  printf("c=%d\n", c); //% self.expect("expression b", 
DATA_TYPES

[Lldb-commits] [lldb] [lldb] [NFC] Rewrite TestRedefinitionsInInlines.py as an API test (PR #94539)

2024-06-05 Thread via lldb-commits


llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: Jason Molenda (jasonmolenda)


Changes

Rewrite an inline test as an API test, to be a little easier to debug, and add 
some additional checks that we're in the inlined test1, then step and we are 
now in the inlined test2 functions.

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


3 Files Affected:

- (added) lldb/test/API/lang/c/inlines/Makefile (+3) 
- (modified) lldb/test/API/lang/c/inlines/TestRedefinitionsInInlines.py 
(+60-13) 
- (modified) lldb/test/API/lang/c/inlines/main.c (+11-12) 


``diff
diff --git a/lldb/test/API/lang/c/inlines/Makefile 
b/lldb/test/API/lang/c/inlines/Makefile
new file mode 100644
index 0..f9555f92be8cd
--- /dev/null
+++ b/lldb/test/API/lang/c/inlines/Makefile
@@ -0,0 +1,3 @@
+C_SOURCES := main.c 
+
+include Makefile.rules
diff --git a/lldb/test/API/lang/c/inlines/TestRedefinitionsInInlines.py 
b/lldb/test/API/lang/c/inlines/TestRedefinitionsInInlines.py
index 024b9dad9b0fb..d48f88ad7d563 100644
--- a/lldb/test/API/lang/c/inlines/TestRedefinitionsInInlines.py
+++ b/lldb/test/API/lang/c/inlines/TestRedefinitionsInInlines.py
@@ -1,14 +1,61 @@
-from lldbsuite.test import lldbinline
-from lldbsuite.test import decorators
-
-lldbinline.MakeInlineTest(
-__file__,
-globals(),
-[
-decorators.expectedFailureAll(
-compiler="clang",
-compiler_version=["<", "3.5"],
-bugnumber="llvm.org/pr27845",
+"""Test that inlined argument variables have their correct location in 
debuginfo"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TestRedefinitionsInInlines(TestBase):
+
+# https://github.com/llvm/llvm-project/issues/28219
+@skipIf(compiler="clang", compiler_version=["<", "3.5"])
+def test(self):
+self.source = "main.c"
+self.build()
+(target, process, thread, bp1) = lldbutil.run_to_source_breakpoint(
+self, "first breakpoint", lldb.SBFileSpec(self.source, False)
 )
-],
-)
+
+bp2 = target.BreakpointCreateBySourceRegex(
+"second breakpoint", lldb.SBFileSpec(self.source, False)
+)
+bp3 = target.BreakpointCreateBySourceRegex(
+"third breakpoint", lldb.SBFileSpec(self.source, False)
+)
+
+# When called from main(), test2 is passed in the value of 42 in 'b'
+self.expect("expression b", DATA_TYPES_DISPLAYED_CORRECTLY, 
substrs=["42"])
+
+process.Continue()
+
+self.assertState(process.GetState(), lldb.eStateStopped)
+thread = lldbutil.get_stopped_thread(process, 
lldb.eStopReasonBreakpoint)
+self.assertIsNotNone(thread)
+bp_id = thread.GetStopReasonDataAtIndex(0)
+self.assertEqual(bp_id, bp2.GetID())
+
+self.expect("expression b", DATA_TYPES_DISPLAYED_CORRECTLY, 
substrs=["42"])
+self.expect("expression c", DATA_TYPES_DISPLAYED_CORRECTLY, 
substrs=["84"])
+
+process.Continue()
+
+# Now we're in test1(), and the first thing it does is call test2(24). 
 "Step in"
+# and check that we have the value 24 as the argument.
+self.assertState(process.GetState(), lldb.eStateStopped)
+thread = lldbutil.get_stopped_thread(process, 
lldb.eStopReasonBreakpoint)
+self.assertIsNotNone(thread)
+bp_id = thread.GetStopReasonDataAtIndex(0)
+self.assertEqual(bp_id, bp3.GetID())
+
+frame = thread.GetFrameAtIndex(0)
+self.assertTrue(frame.IsInlined())
+self.assertEqual(frame.GetFunctionName(), "test1")
+
+thread.StepInto()
+
+frame = thread.GetFrameAtIndex(0)
+self.assertTrue(frame.IsInlined())
+self.assertEqual(frame.GetFunctionName(), "test2")
+
+self.expect("expression b", DATA_TYPES_DISPLAYED_CORRECTLY, 
substrs=["24"])
diff --git a/lldb/test/API/lang/c/inlines/main.c 
b/lldb/test/API/lang/c/inlines/main.c
index 8fe49180800bd..6ecc04dd508fc 100644
--- a/lldb/test/API/lang/c/inlines/main.c
+++ b/lldb/test/API/lang/c/inlines/main.c
@@ -3,23 +3,22 @@
 inline void test1(int) __attribute__ ((always_inline));
 inline void test2(int) __attribute__ ((always_inline));
 
+// Called once from main with b==42 then called from test1 with b==24.
 void test2(int b) {
-printf("test2(%d)\n", b); //% self.expect("expression b", 
DATA_TYPES_DISPLAYED_CORRECTLY, substrs = ["42"])
-{
-  int c = b * 2;
-  printf("c=%d\n", c); //% self.expect("expression b", 
DATA_TYPES_DISPLAYED_CORRECTLY, substrs = ["42"])
-   //% self.expect("expression c", 
DATA_TYPES_DISPLAYED_CORRECTLY, substrs = ["84"])
-}
+  printf("test2(%d)\n", b); // first breakpoint
+  {
+int c = b * 2;
+printf("c=%d\n", c); // second breakpoint
+  }
 }
 
 void test1(int a) {
 printf("test1(%d)\n",  a);
-test2(a+1);//% self.runCmd("step")
-   //%

[Lldb-commits] [lldb] [lldb/crashlog] Add `--no-parallel-image-loading` hidden flag (PR #94513)

2024-06-05 Thread Jonas Devlieghere via lldb-commits


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


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

[Lldb-commits] [lldb] [lldb] [NFC] Rewrite TestRedefinitionsInInlines.py as an API test (PR #94539)

2024-06-05 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 
55d2fffdae5531759569e4ea8985c3de2e96bcc1...c02e38e5ef017a966ef8b94a56e394309462
 lldb/test/API/lang/c/inlines/TestRedefinitionsInInlines.py
``





View the diff from darker here.


``diff
--- TestRedefinitionsInInlines.py   2024-06-05 22:28:35.00 +
+++ TestRedefinitionsInInlines.py   2024-06-05 22:31:59.388390 +
@@ -5,11 +5,10 @@
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
 
 
 class TestRedefinitionsInInlines(TestBase):
-
 # https://github.com/llvm/llvm-project/issues/28219
 @skipIf(compiler="clang", compiler_version=["<", "3.5"])
 def test(self):
 self.source = "main.c"
 self.build()

``




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

[Lldb-commits] [lldb] [lldb] [NFC] Rewrite TestRedefinitionsInInlines.py as an API test (PR #94539)

2024-06-05 Thread Jason Molenda via lldb-commits


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

>From c02e38e5ef017a966ef8b94a56e394309462 Mon Sep 17 00:00:00 2001
From: Jason Molenda 
Date: Wed, 5 Jun 2024 15:27:23 -0700
Subject: [PATCH 1/2] [lldb] [NFC] Rewrite TestRedefinitionsInInlines.py as an
 API test

Rewrite an inline test as an API test, to be a little easier to
debug, and add some additional checks that we're in the inlined
test1, then step and we are now in the inlined test2 functions.
---
 lldb/test/API/lang/c/inlines/Makefile |  3 +
 .../c/inlines/TestRedefinitionsInInlines.py   | 73 +++
 lldb/test/API/lang/c/inlines/main.c   | 23 +++---
 3 files changed, 74 insertions(+), 25 deletions(-)
 create mode 100644 lldb/test/API/lang/c/inlines/Makefile

diff --git a/lldb/test/API/lang/c/inlines/Makefile 
b/lldb/test/API/lang/c/inlines/Makefile
new file mode 100644
index 0..f9555f92be8cd
--- /dev/null
+++ b/lldb/test/API/lang/c/inlines/Makefile
@@ -0,0 +1,3 @@
+C_SOURCES := main.c 
+
+include Makefile.rules
diff --git a/lldb/test/API/lang/c/inlines/TestRedefinitionsInInlines.py 
b/lldb/test/API/lang/c/inlines/TestRedefinitionsInInlines.py
index 024b9dad9b0fb..d48f88ad7d563 100644
--- a/lldb/test/API/lang/c/inlines/TestRedefinitionsInInlines.py
+++ b/lldb/test/API/lang/c/inlines/TestRedefinitionsInInlines.py
@@ -1,14 +1,61 @@
-from lldbsuite.test import lldbinline
-from lldbsuite.test import decorators
-
-lldbinline.MakeInlineTest(
-__file__,
-globals(),
-[
-decorators.expectedFailureAll(
-compiler="clang",
-compiler_version=["<", "3.5"],
-bugnumber="llvm.org/pr27845",
+"""Test that inlined argument variables have their correct location in 
debuginfo"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TestRedefinitionsInInlines(TestBase):
+
+# https://github.com/llvm/llvm-project/issues/28219
+@skipIf(compiler="clang", compiler_version=["<", "3.5"])
+def test(self):
+self.source = "main.c"
+self.build()
+(target, process, thread, bp1) = lldbutil.run_to_source_breakpoint(
+self, "first breakpoint", lldb.SBFileSpec(self.source, False)
 )
-],
-)
+
+bp2 = target.BreakpointCreateBySourceRegex(
+"second breakpoint", lldb.SBFileSpec(self.source, False)
+)
+bp3 = target.BreakpointCreateBySourceRegex(
+"third breakpoint", lldb.SBFileSpec(self.source, False)
+)
+
+# When called from main(), test2 is passed in the value of 42 in 'b'
+self.expect("expression b", DATA_TYPES_DISPLAYED_CORRECTLY, 
substrs=["42"])
+
+process.Continue()
+
+self.assertState(process.GetState(), lldb.eStateStopped)
+thread = lldbutil.get_stopped_thread(process, 
lldb.eStopReasonBreakpoint)
+self.assertIsNotNone(thread)
+bp_id = thread.GetStopReasonDataAtIndex(0)
+self.assertEqual(bp_id, bp2.GetID())
+
+self.expect("expression b", DATA_TYPES_DISPLAYED_CORRECTLY, 
substrs=["42"])
+self.expect("expression c", DATA_TYPES_DISPLAYED_CORRECTLY, 
substrs=["84"])
+
+process.Continue()
+
+# Now we're in test1(), and the first thing it does is call test2(24). 
 "Step in"
+# and check that we have the value 24 as the argument.
+self.assertState(process.GetState(), lldb.eStateStopped)
+thread = lldbutil.get_stopped_thread(process, 
lldb.eStopReasonBreakpoint)
+self.assertIsNotNone(thread)
+bp_id = thread.GetStopReasonDataAtIndex(0)
+self.assertEqual(bp_id, bp3.GetID())
+
+frame = thread.GetFrameAtIndex(0)
+self.assertTrue(frame.IsInlined())
+self.assertEqual(frame.GetFunctionName(), "test1")
+
+thread.StepInto()
+
+frame = thread.GetFrameAtIndex(0)
+self.assertTrue(frame.IsInlined())
+self.assertEqual(frame.GetFunctionName(), "test2")
+
+self.expect("expression b", DATA_TYPES_DISPLAYED_CORRECTLY, 
substrs=["24"])
diff --git a/lldb/test/API/lang/c/inlines/main.c 
b/lldb/test/API/lang/c/inlines/main.c
index 8fe49180800bd..6ecc04dd508fc 100644
--- a/lldb/test/API/lang/c/inlines/main.c
+++ b/lldb/test/API/lang/c/inlines/main.c
@@ -3,23 +3,22 @@
 inline void test1(int) __attribute__ ((always_inline));
 inline void test2(int) __attribute__ ((always_inline));
 
+// Called once from main with b==42 then called from test1 with b==24.
 void test2(int b) {
-printf("test2(%d)\n", b); //% self.expect("expression b", 
DATA_TYPES_DISPLAYED_CORRECTLY, substrs = ["42"])
-{
-  int c = b * 2;
-  printf("c=%d\n", c); //% self.expect("expression b", 
DATA_TYPES_DISPLAYED_CORRECTLY, substrs = ["42"])
-   //% self.expect("expression c", 
DATA_TYPES_DISPLAYED_CORRECTLY, substrs = ["84"])
-}
+  printf("test2(%d)\n", b); // f

[Lldb-commits] [lldb] [lldb] [NFC] Rewrite TestRedefinitionsInInlines.py as an API test (PR #94539)

2024-06-05 Thread Jason Molenda via lldb-commits


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

[Lldb-commits] [lldb] 19bce17 - [lldb] [NFC] Rewrite TestRedefinitionsInInlines.py as an API test (#94539)

2024-06-05 Thread via lldb-commits


Author: Jason Molenda
Date: 2024-06-05T15:39:40-07:00
New Revision: 19bce1702bd1e399bea76d0de2a649a14551b000

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

LOG: [lldb] [NFC] Rewrite TestRedefinitionsInInlines.py as an API test (#94539)

Rewrite an inline test as an API test, to be a little easier to debug,
and add some additional checks that we're in the inlined test1, then
step and we are now in the inlined test2 functions.

Added: 
lldb/test/API/lang/c/inlines/Makefile

Modified: 
lldb/test/API/lang/c/inlines/TestRedefinitionsInInlines.py
lldb/test/API/lang/c/inlines/main.c

Removed: 




diff  --git a/lldb/test/API/lang/c/inlines/Makefile 
b/lldb/test/API/lang/c/inlines/Makefile
new file mode 100644
index 0..f9555f92be8cd
--- /dev/null
+++ b/lldb/test/API/lang/c/inlines/Makefile
@@ -0,0 +1,3 @@
+C_SOURCES := main.c 
+
+include Makefile.rules

diff  --git a/lldb/test/API/lang/c/inlines/TestRedefinitionsInInlines.py 
b/lldb/test/API/lang/c/inlines/TestRedefinitionsInInlines.py
index 024b9dad9b0fb..062fd88f7d272 100644
--- a/lldb/test/API/lang/c/inlines/TestRedefinitionsInInlines.py
+++ b/lldb/test/API/lang/c/inlines/TestRedefinitionsInInlines.py
@@ -1,14 +1,60 @@
-from lldbsuite.test import lldbinline
-from lldbsuite.test import decorators
-
-lldbinline.MakeInlineTest(
-__file__,
-globals(),
-[
-decorators.expectedFailureAll(
-compiler="clang",
-compiler_version=["<", "3.5"],
-bugnumber="llvm.org/pr27845",
+"""Test that inlined argument variables have their correct location in 
debuginfo"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TestRedefinitionsInInlines(TestBase):
+# https://github.com/llvm/llvm-project/issues/28219
+@skipIf(compiler="clang", compiler_version=["<", "3.5"])
+def test(self):
+self.source = "main.c"
+self.build()
+(target, process, thread, bp1) = lldbutil.run_to_source_breakpoint(
+self, "first breakpoint", lldb.SBFileSpec(self.source, False)
+)
+
+bp2 = target.BreakpointCreateBySourceRegex(
+"second breakpoint", lldb.SBFileSpec(self.source, False)
+)
+bp3 = target.BreakpointCreateBySourceRegex(
+"third breakpoint", lldb.SBFileSpec(self.source, False)
 )
-],
-)
+
+# When called from main(), test2 is passed in the value of 42 in 'b'
+self.expect("expression b", DATA_TYPES_DISPLAYED_CORRECTLY, 
substrs=["42"])
+
+process.Continue()
+
+self.assertState(process.GetState(), lldb.eStateStopped)
+thread = lldbutil.get_stopped_thread(process, 
lldb.eStopReasonBreakpoint)
+self.assertIsNotNone(thread)
+bp_id = thread.GetStopReasonDataAtIndex(0)
+self.assertEqual(bp_id, bp2.GetID())
+
+self.expect("expression b", DATA_TYPES_DISPLAYED_CORRECTLY, 
substrs=["42"])
+self.expect("expression c", DATA_TYPES_DISPLAYED_CORRECTLY, 
substrs=["84"])
+
+process.Continue()
+
+# Now we're in test1(), and the first thing it does is call test2(24). 
 "Step in"
+# and check that we have the value 24 as the argument.
+self.assertState(process.GetState(), lldb.eStateStopped)
+thread = lldbutil.get_stopped_thread(process, 
lldb.eStopReasonBreakpoint)
+self.assertIsNotNone(thread)
+bp_id = thread.GetStopReasonDataAtIndex(0)
+self.assertEqual(bp_id, bp3.GetID())
+
+frame = thread.GetFrameAtIndex(0)
+self.assertTrue(frame.IsInlined())
+self.assertEqual(frame.GetFunctionName(), "test1")
+
+thread.StepInto()
+
+frame = thread.GetFrameAtIndex(0)
+self.assertTrue(frame.IsInlined())
+self.assertEqual(frame.GetFunctionName(), "test2")
+
+self.expect("expression b", DATA_TYPES_DISPLAYED_CORRECTLY, 
substrs=["24"])

diff  --git a/lldb/test/API/lang/c/inlines/main.c 
b/lldb/test/API/lang/c/inlines/main.c
index 8fe49180800bd..6ecc04dd508fc 100644
--- a/lldb/test/API/lang/c/inlines/main.c
+++ b/lldb/test/API/lang/c/inlines/main.c
@@ -3,23 +3,22 @@
 inline void test1(int) __attribute__ ((always_inline));
 inline void test2(int) __attribute__ ((always_inline));
 
+// Called once from main with b==42 then called from test1 with b==24.
 void test2(int b) {
-printf("test2(%d)\n", b); //% self.expect("expression b", 
DATA_TYPES_DISPLAYED_CORRECTLY, substrs = ["42"])
-{
-  int c = b * 2;
-  printf("c=%d\n", c); //% self.expect("expression b", 
DATA_TYPES_DISPLAYED_CORRECTLY, substrs = ["42"])
-   //% self.expect("expression c", 
DATA_TYPES_DISPLAYED_CORRECTLY, substrs = ["84"])
-

[Lldb-commits] [lldb] 68a9cb7 - [lldb/crashlog] Add `--no-parallel-image-loading` hidden flag (#94513)

2024-06-05 Thread via lldb-commits


Author: Med Ismail Bennani
Date: 2024-06-05T15:45:42-07:00
New Revision: 68a9cb799511506045ca26c04e7933f0e0ed46ec

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

LOG: [lldb/crashlog] Add `--no-parallel-image-loading` hidden flag (#94513)

This patch adds the `--no-parallel-image-loading` to the crashlog
command. By default, image loading will happen in parallel in the
crashlog script however, sometimes, when running tests or debugging the
crashlog script itself, it's better to load the images sequentially.

As its name suggests, this flag will disable the default image loading
behaviour to load all the images sequencially in the main thread.

Signed-off-by: Med Ismail Bennani 

Added: 


Modified: 
lldb/examples/python/crashlog.py
lldb/examples/python/crashlog_scripted_process.py

Removed: 




diff  --git a/lldb/examples/python/crashlog.py 
b/lldb/examples/python/crashlog.py
index c874cb4d32e66..7c6c60e518d75 100755
--- a/lldb/examples/python/crashlog.py
+++ b/lldb/examples/python/crashlog.py
@@ -557,11 +557,15 @@ def load_images(self, options, loaded_images=None):
 
 futures = []
 with tempfile.TemporaryDirectory() as obj_dir:
-with concurrent.futures.ThreadPoolExecutor() as executor:
 
-def add_module(image, target, obj_dir):
-return image, image.add_module(target, obj_dir)
+def add_module(image, target, obj_dir):
+return image, image.add_module(target, obj_dir)
 
+max_worker = None
+if options.no_parallel_image_loading:
+max_worker = 1
+
+with concurrent.futures.ThreadPoolExecutor(max_worker) as executor:
 for image in images_to_load:
 if image not in loaded_images:
 if image.uuid == uuid.UUID(int=0):
@@ -1530,6 +1534,7 @@ def load_crashlog_in_scripted_process(debugger, 
crashlog_path, options, result):
 "file_path": crashlog_path,
 "load_all_images": options.load_all_images,
 "crashed_only": options.crashed_only,
+"no_parallel_image_loading": options.no_parallel_image_loading,
 }
 )
 )
@@ -1722,6 +1727,13 @@ def CreateSymbolicateCrashLogOptions(
 help="show source for all threads, not just the crashed thread",
 default=False,
 )
+arg_parser.add_argument(
+"--no-parallel-image-loading",
+dest="no_parallel_image_loading",
+action="store_true",
+help=argparse.SUPPRESS,
+default=False,
+)
 if add_interactive_options:
 arg_parser.add_argument(
 "-i",

diff  --git a/lldb/examples/python/crashlog_scripted_process.py 
b/lldb/examples/python/crashlog_scripted_process.py
index 26c5c37b7371d..2ee030239ee37 100644
--- a/lldb/examples/python/crashlog_scripted_process.py
+++ b/lldb/examples/python/crashlog_scripted_process.py
@@ -53,6 +53,7 @@ def set_crashlog(self, crashlog):
 class CrashLogOptions:
 load_all_images = False
 crashed_only = True
+no_parallel_image_loading = False
 
 def __init__(self, exe_ctx: lldb.SBExecutionContext, args: 
lldb.SBStructuredData):
 super().__init__(exe_ctx, args)
@@ -84,6 +85,13 @@ def __init__(self, exe_ctx: lldb.SBExecutionContext, args: 
lldb.SBStructuredData
 if crashed_only.GetType() == lldb.eStructuredDataTypeBoolean:
 self.options.crashed_only = crashed_only.GetBooleanValue()
 
+no_parallel_image_loading = 
args.GetValueForKey("no_parallel_image_loading")
+if no_parallel_image_loading and no_parallel_image_loading.IsValid():
+if no_parallel_image_loading.GetType() == 
lldb.eStructuredDataTypeBoolean:
+self.options.no_parallel_image_loading = (
+no_parallel_image_loading.GetBooleanValue()
+)
+
 self.pid = super().get_process_id()
 self.crashed_thread_idx = 0
 self.exception = None



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

[Lldb-commits] [lldb] [lldb/crashlog] Add `--no-parallel-image-loading` hidden flag (PR #94513)

2024-06-05 Thread Med Ismail Bennani via lldb-commits


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

[Lldb-commits] [lldb] [LLDB] Add more helper functions to ValueObject class. (PR #87197)

2024-06-05 Thread via lldb-commits



@@ -3015,11 +3017,12 @@ llvm::Expected 
ValueObject::CastDerivedToBaseType(
 
   lldb::TargetSP target = GetTargetSP();
   // The `value` can be a pointer, but GetChildAtIndex works for pointers too.
-  lldb::ValueObjectSP inner_value;
+  lldb::ValueObjectSP inner_value = GetSP();
 
   for (const uint32_t i : base_type_indices)
 // Force static value, otherwise we can end up with the "real" type.
-inner_value = GetChildAtIndex(i, /*can_create_synthetic*/ false);

jimingham wrote:

Ah, I missed that I was looking at a diff against your original diff which 
didn't make sense.  In context the code seems fine.   

But then that also showed that you are the author of the "somewhat confusing 
comment".  Can you make that comment more helpful?  Suppressing the synthetic 
child production would force this child to get its type directly from the 
CompilerType, which I would call the "real" child.  So that comment seems to 
make sense when we passed `true` but doesn't make sense when passing true.

You don't need to introduce tests for this at this stage if that's dependent on 
other changes, but this seems like a tricky point, so it should have a comment. 
 But this one doesn't make sense to me.

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

[Lldb-commits] [lldb] [LLDB] Add more helper functions to ValueObject class. (PR #87197)

2024-06-05 Thread via lldb-commits


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

[Lldb-commits] [lldb] [lldb] Split ValueObject::CreateChildAtIndex into two functions (PR #94455)

2024-06-05 Thread via lldb-commits



@@ -46,75 +46,116 @@ lldb::ValueObjectSP 
ValueObjectConstResultImpl::Dereference(Status &error) {
   return m_impl_backend->ValueObject::Dereference(error);
 }
 
-ValueObject *ValueObjectConstResultImpl::CreateChildAtIndex(
-size_t idx, bool synthetic_array_member, int32_t synthetic_index) {
+ValueObject *ValueObjectConstResultImpl::CreateChildAtIndex(size_t idx) {
   if (m_impl_backend == nullptr)
 return nullptr;
 
   m_impl_backend->UpdateValueIfNeeded(false);
 
-  ValueObjectConstResultChild *valobj = nullptr;
-
   bool omit_empty_base_classes = true;
-  bool ignore_array_bounds = synthetic_array_member;
-  std::string child_name_str;
+  bool ignore_array_bounds = false;
+  std::string child_name;
   uint32_t child_byte_size = 0;
   int32_t child_byte_offset = 0;
   uint32_t child_bitfield_bit_size = 0;
   uint32_t child_bitfield_bit_offset = 0;
   bool child_is_base_class = false;
   bool child_is_deref_of_parent = false;
   uint64_t language_flags;
-
-  const bool transparent_pointers = !synthetic_array_member;
+  const bool transparent_pointers = true;
   CompilerType compiler_type = m_impl_backend->GetCompilerType();
 
   ExecutionContext exe_ctx(m_impl_backend->GetExecutionContextRef());
 
   auto child_compiler_type_or_err = compiler_type.GetChildCompilerTypeAtIndex(
   &exe_ctx, idx, transparent_pointers, omit_empty_base_classes,
-  ignore_array_bounds, child_name_str, child_byte_size, child_byte_offset,
+  ignore_array_bounds, child_name, child_byte_size, child_byte_offset,
   child_bitfield_bit_size, child_bitfield_bit_offset, child_is_base_class,
   child_is_deref_of_parent, m_impl_backend, language_flags);
-  CompilerType child_compiler_type;
-  if (!child_compiler_type_or_err)
+
+  // One might think we should check that the size of the children
+  // is always strictly positive, hence we could avoid creating a
+  // ValueObject if that's not the case, but it turns out there
+  // are languages out there which allow zero-size types with
+  // children (e.g. Swift).
+  if (!child_compiler_type_or_err || !child_compiler_type_or_err->IsValid()) {
 LLDB_LOG_ERROR(GetLog(LLDBLog::Types),
child_compiler_type_or_err.takeError(),
"could not find child: {0}");
-  else
-child_compiler_type = *child_compiler_type_or_err;
+return nullptr;
+  }
 
+  lldb::addr_t child_live_addr = LLDB_INVALID_ADDRESS;
+  // Transfer the live address (with offset) to the child.  But if
+  // the parent is a pointer, the live address is where that pointer
+  // value lives in memory, so the children live addresses aren't
+  // offsets from that value, they are just other load addresses that
+  // are recorded in the Value of the child ValueObjects.
+  if (m_live_address != LLDB_INVALID_ADDRESS && !compiler_type.IsPointerType())
+child_live_addr = m_live_address + child_byte_offset;
+
+  return new ValueObjectConstResultChild(
+  *m_impl_backend, *child_compiler_type_or_err, ConstString(child_name),
+  child_byte_size, child_byte_offset, child_bitfield_bit_size,
+  child_bitfield_bit_offset, child_is_base_class, child_is_deref_of_parent,
+  child_live_addr, language_flags);
+}
+
+ValueObject *
+ValueObjectConstResultImpl::CreateSyntheticArrayMember(size_t idx) {
+  if (m_impl_backend == nullptr)
+return nullptr;
+
+  m_impl_backend->UpdateValueIfNeeded(false);
+
+  bool omit_empty_base_classes = true;
+  bool ignore_array_bounds = true;
+  std::string child_name;
+  uint32_t child_byte_size = 0;
+  int32_t child_byte_offset = 0;
+  uint32_t child_bitfield_bit_size = 0;
+  uint32_t child_bitfield_bit_offset = 0;
+  bool child_is_base_class = false;
+  bool child_is_deref_of_parent = false;
+  uint64_t language_flags;
+
+  const bool transparent_pointers = false;
+  CompilerType compiler_type = m_impl_backend->GetCompilerType();
+
+  ExecutionContext exe_ctx(m_impl_backend->GetExecutionContextRef());
+
+  auto child_compiler_type_or_err = compiler_type.GetChildCompilerTypeAtIndex(
+  &exe_ctx, 0, transparent_pointers, omit_empty_base_classes,
+  ignore_array_bounds, child_name, child_byte_size, child_byte_offset,
+  child_bitfield_bit_size, child_bitfield_bit_offset, child_is_base_class,
+  child_is_deref_of_parent, m_impl_backend, language_flags);
   // One might think we should check that the size of the children

jimingham wrote:

Shouldn't this version of this comment be removed?  It appears at line 76 in 
your changed version.

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

[Lldb-commits] [lldb] [lldb] Split ValueObject::CreateChildAtIndex into two functions (PR #94455)

2024-06-05 Thread via lldb-commits



@@ -46,75 +46,116 @@ lldb::ValueObjectSP 
ValueObjectConstResultImpl::Dereference(Status &error) {
   return m_impl_backend->ValueObject::Dereference(error);
 }
 
-ValueObject *ValueObjectConstResultImpl::CreateChildAtIndex(
-size_t idx, bool synthetic_array_member, int32_t synthetic_index) {
+ValueObject *ValueObjectConstResultImpl::CreateChildAtIndex(size_t idx) {
   if (m_impl_backend == nullptr)
 return nullptr;
 
   m_impl_backend->UpdateValueIfNeeded(false);
 
-  ValueObjectConstResultChild *valobj = nullptr;
-
   bool omit_empty_base_classes = true;
-  bool ignore_array_bounds = synthetic_array_member;
-  std::string child_name_str;
+  bool ignore_array_bounds = false;
+  std::string child_name;
   uint32_t child_byte_size = 0;
   int32_t child_byte_offset = 0;
   uint32_t child_bitfield_bit_size = 0;
   uint32_t child_bitfield_bit_offset = 0;
   bool child_is_base_class = false;
   bool child_is_deref_of_parent = false;
   uint64_t language_flags;
-
-  const bool transparent_pointers = !synthetic_array_member;
+  const bool transparent_pointers = true;
   CompilerType compiler_type = m_impl_backend->GetCompilerType();
 
   ExecutionContext exe_ctx(m_impl_backend->GetExecutionContextRef());
 
   auto child_compiler_type_or_err = compiler_type.GetChildCompilerTypeAtIndex(
   &exe_ctx, idx, transparent_pointers, omit_empty_base_classes,
-  ignore_array_bounds, child_name_str, child_byte_size, child_byte_offset,
+  ignore_array_bounds, child_name, child_byte_size, child_byte_offset,
   child_bitfield_bit_size, child_bitfield_bit_offset, child_is_base_class,
   child_is_deref_of_parent, m_impl_backend, language_flags);
-  CompilerType child_compiler_type;
-  if (!child_compiler_type_or_err)
+
+  // One might think we should check that the size of the children
+  // is always strictly positive, hence we could avoid creating a
+  // ValueObject if that's not the case, but it turns out there
+  // are languages out there which allow zero-size types with
+  // children (e.g. Swift).
+  if (!child_compiler_type_or_err || !child_compiler_type_or_err->IsValid()) {
 LLDB_LOG_ERROR(GetLog(LLDBLog::Types),
child_compiler_type_or_err.takeError(),
"could not find child: {0}");
-  else
-child_compiler_type = *child_compiler_type_or_err;
+return nullptr;
+  }
 
+  lldb::addr_t child_live_addr = LLDB_INVALID_ADDRESS;
+  // Transfer the live address (with offset) to the child.  But if
+  // the parent is a pointer, the live address is where that pointer
+  // value lives in memory, so the children live addresses aren't
+  // offsets from that value, they are just other load addresses that
+  // are recorded in the Value of the child ValueObjects.
+  if (m_live_address != LLDB_INVALID_ADDRESS && !compiler_type.IsPointerType())
+child_live_addr = m_live_address + child_byte_offset;
+
+  return new ValueObjectConstResultChild(
+  *m_impl_backend, *child_compiler_type_or_err, ConstString(child_name),
+  child_byte_size, child_byte_offset, child_bitfield_bit_size,
+  child_bitfield_bit_offset, child_is_base_class, child_is_deref_of_parent,
+  child_live_addr, language_flags);
+}
+
+ValueObject *
+ValueObjectConstResultImpl::CreateSyntheticArrayMember(size_t idx) {
+  if (m_impl_backend == nullptr)
+return nullptr;
+
+  m_impl_backend->UpdateValueIfNeeded(false);
+
+  bool omit_empty_base_classes = true;
+  bool ignore_array_bounds = true;
+  std::string child_name;
+  uint32_t child_byte_size = 0;
+  int32_t child_byte_offset = 0;
+  uint32_t child_bitfield_bit_size = 0;
+  uint32_t child_bitfield_bit_offset = 0;
+  bool child_is_base_class = false;
+  bool child_is_deref_of_parent = false;
+  uint64_t language_flags;
+
+  const bool transparent_pointers = false;
+  CompilerType compiler_type = m_impl_backend->GetCompilerType();
+
+  ExecutionContext exe_ctx(m_impl_backend->GetExecutionContextRef());
+
+  auto child_compiler_type_or_err = compiler_type.GetChildCompilerTypeAtIndex(
+  &exe_ctx, 0, transparent_pointers, omit_empty_base_classes,
+  ignore_array_bounds, child_name, child_byte_size, child_byte_offset,
+  child_bitfield_bit_size, child_bitfield_bit_offset, child_is_base_class,
+  child_is_deref_of_parent, m_impl_backend, language_flags);
   // One might think we should check that the size of the children

jimingham wrote:

Ah, no, didn't read far enough.  You do need to repeat the comment.

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

[Lldb-commits] [lldb] [lldb] Split ValueObject::CreateChildAtIndex into two functions (PR #94455)

2024-06-05 Thread via lldb-commits


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

This is much nicer.  I would actually prefer we returned names that are "" 
rather than nullptr, but in any case I agree there's no particular reason to 
make that a contract.

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

[Lldb-commits] [lldb] [lldb/crashlog] Remove aarch64 requirement on crashlog tests (PR #94553)

2024-06-05 Thread Med Ismail Bennani via lldb-commits


https://github.com/medismailben created 
https://github.com/llvm/llvm-project/pull/94553

This PR removes the `target-aarch64` requirement on the crashlog tests to 
exercice them on Intel bots and make image loading single-threaded temporarily 
while implementing a fix for a deadlock issue when loading the images in 
parallel.

Signed-off-by: Med Ismail Bennani 


>From 460f99862754f96c04ceebee43023bf579879122 Mon Sep 17 00:00:00 2001
From: Med Ismail Bennani 
Date: Tue, 4 Jun 2024 20:36:53 -0700
Subject: [PATCH 1/2] [lldb/crashlog] Remove aarch64 requirement on crashlog
 tests

This patch removes the `target-aarch64` requirement on the crashlog
tests.

Signed-off-by: Med Ismail Bennani 
---
 .../Python/Crashlog/app_specific_backtrace_crashlog.test| 2 +-
 .../Python/Crashlog/interactive_crashlog_invalid_target.test| 2 +-
 .../Python/Crashlog/interactive_crashlog_json.test  | 2 +-
 .../Python/Crashlog/interactive_crashlog_legacy.test| 2 +-
 .../Python/Crashlog/last_exception_backtrace_crashlog.test  | 2 +-
 .../Python/Crashlog/skipped_status_interactive_crashlog.test| 2 +-
 6 files changed, 6 insertions(+), 6 deletions(-)

diff --git 
a/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/app_specific_backtrace_crashlog.test
 
b/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/app_specific_backtrace_crashlog.test
index c57cefdaf32d2..eb26ab90bb6ce 100644
--- 
a/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/app_specific_backtrace_crashlog.test
+++ 
b/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/app_specific_backtrace_crashlog.test
@@ -1,4 +1,4 @@
-# REQUIRES: python, native && target-aarch64 && system-darwin
+# REQUIRES: python, native && system-darwin
 
 # RUN: mkdir -p %t.dir
 # RUN: yaml2obj %S/Inputs/application_specific_info/asi.yaml > %t.dir/asi
diff --git 
a/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/interactive_crashlog_invalid_target.test
 
b/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/interactive_crashlog_invalid_target.test
index abd1e7c3da53d..eb1f5f456a2dc 100644
--- 
a/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/interactive_crashlog_invalid_target.test
+++ 
b/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/interactive_crashlog_invalid_target.test
@@ -1,4 +1,4 @@
-# REQUIRES: python, native && target-aarch64 && system-darwin
+# REQUIRES: python, native && system-darwin
 
 # RUN: %lldb -o 'command script import lldb.macosx.crashlog' \
 # RUN: -o 'crashlog -V' \
diff --git 
a/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/interactive_crashlog_json.test
 
b/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/interactive_crashlog_json.test
index fccd71ce31f73..684be2846f78d 100644
--- 
a/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/interactive_crashlog_json.test
+++ 
b/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/interactive_crashlog_json.test
@@ -1,4 +1,4 @@
-# REQUIRES: python, native && target-aarch64 && system-darwin
+# REQUIRES: python, native && system-darwin
 
 # RUN: mkdir -p %t.dir
 # RUN: yaml2obj %S/Inputs/interactive_crashlog/multithread-test.yaml > 
%t.dir/multithread-test
diff --git 
a/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/interactive_crashlog_legacy.test
 
b/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/interactive_crashlog_legacy.test
index 6e2826e88aedf..271a4c2aa90f4 100644
--- 
a/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/interactive_crashlog_legacy.test
+++ 
b/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/interactive_crashlog_legacy.test
@@ -1,4 +1,4 @@
-# REQUIRES: python, native && target-aarch64 && system-darwin
+# REQUIRES: python, native && system-darwin
 
 # RUN: mkdir -p %t.dir
 # RUN: yaml2obj %S/Inputs/interactive_crashlog/multithread-test.yaml > 
%t.dir/multithread-test
diff --git 
a/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/last_exception_backtrace_crashlog.test
 
b/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/last_exception_backtrace_crashlog.test
index c2f61963ed0cf..2084fc18c1272 100644
--- 
a/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/last_exception_backtrace_crashlog.test
+++ 
b/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/last_exception_backtrace_crashlog.test
@@ -1,4 +1,4 @@
-# REQUIRES: python, native && target-aarch64 && system-darwin
+# REQUIRES: python, native && system-darwin
 
 # RUN: mkdir -p %t.dir
 # RUN: yaml2obj %S/Inputs/application_specific_info/asi.yaml > %t.dir/asi
diff --git 
a/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/skipped_status_interactive_crashlog.test
 
b/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/skipped_status_interactive_crashlog.test
index 81e06868eaee7..64cd0904371aa 100644
--- 
a/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/skipped_status_interactive_crashlog.test
+++ 
b/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/skipped_status_interactive_crashlog.test
@@ -1,4 +1,4 @@
-# REQUIRES: python, native && target-aarch64 && system-darwin
+# REQUIRES: python, native && system-darw

[Lldb-commits] [lldb] [lldb/crashlog] Remove aarch64 requirement on crashlog tests (PR #94553)

2024-06-05 Thread via lldb-commits


llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: Med Ismail Bennani (medismailben)


Changes

This PR removes the `target-aarch64` requirement on the crashlog tests to 
exercice them on Intel bots and make image loading single-threaded temporarily 
while implementing a fix for a deadlock issue when loading the images in 
parallel.

Signed-off-by: Med Ismail Bennani 


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


10 Files Affected:

- (modified) 
lldb/test/Shell/ScriptInterpreter/Python/Crashlog/altered_threadState.test 
(+1-1) 
- (modified) 
lldb/test/Shell/ScriptInterpreter/Python/Crashlog/app_specific_backtrace_crashlog.test
 (+2-2) 
- (modified) 
lldb/test/Shell/ScriptInterpreter/Python/Crashlog/interactive_crashlog_invalid_target.test
 (+2-2) 
- (modified) 
lldb/test/Shell/ScriptInterpreter/Python/Crashlog/interactive_crashlog_json.test
 (+2-2) 
- (modified) 
lldb/test/Shell/ScriptInterpreter/Python/Crashlog/interactive_crashlog_legacy.test
 (+2-2) 
- (modified) lldb/test/Shell/ScriptInterpreter/Python/Crashlog/json.test (+3-3) 
- (modified) 
lldb/test/Shell/ScriptInterpreter/Python/Crashlog/last_exception_backtrace_crashlog.test
 (+2-2) 
- (modified) 
lldb/test/Shell/ScriptInterpreter/Python/Crashlog/no_threadState.test (+1-1) 
- (modified) 
lldb/test/Shell/ScriptInterpreter/Python/Crashlog/skipped_status_interactive_crashlog.test
 (+2-2) 
- (modified) lldb/test/Shell/ScriptInterpreter/Python/Crashlog/text.test (+1-1) 


``diff
diff --git 
a/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/altered_threadState.test 
b/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/altered_threadState.test
index 5a946a38b1952..2d95d37c151c6 100644
--- a/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/altered_threadState.test
+++ b/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/altered_threadState.test
@@ -1,7 +1,7 @@
 # RUN: %clang_host -g %S/Inputs/test.c -o %t.out
 # RUN: cp %S/Inputs/altered_threadState.crash %t.crash
 # RUN: %python %S/patch-crashlog.py --binary %t.out --crashlog %t.crash 
--offsets '{"main":20, "bar":9, "foo":16}'
-# RUN: %lldb %t.out -o 'command script import lldb.macosx.crashlog' -o 
'crashlog %t.crash' 2>&1 | FileCheck %s
+# RUN: %lldb %t.out -o 'command script import lldb.macosx.crashlog' -o 
'crashlog --no-parallel-image-loading %t.crash' 2>&1 | FileCheck %s
 
 # CHECK: "crashlog" {{.*}} commands have been installed, use the "--help" 
options on these commands
 
diff --git 
a/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/app_specific_backtrace_crashlog.test
 
b/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/app_specific_backtrace_crashlog.test
index c57cefdaf32d2..36e37a3db7fc3 100644
--- 
a/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/app_specific_backtrace_crashlog.test
+++ 
b/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/app_specific_backtrace_crashlog.test
@@ -1,9 +1,9 @@
-# REQUIRES: python, native && target-aarch64 && system-darwin
+# REQUIRES: python, native && system-darwin
 
 # RUN: mkdir -p %t.dir
 # RUN: yaml2obj %S/Inputs/application_specific_info/asi.yaml > %t.dir/asi
 # RUN: %lldb -o 'command script import lldb.macosx.crashlog' \
-# RUN: -o 'crashlog -a -i -t %t.dir/asi 
%S/Inputs/application_specific_info/asi.txt' \
+# RUN: -o 'crashlog -a -i --no-parallel-image-loading -t %t.dir/asi 
%S/Inputs/application_specific_info/asi.txt' \
 # RUN: -o "thread list" -o "bt all" 2>&1 | FileCheck %s
 
 # CHECK: "crashlog" {{.*}} commands have been installed, use the "--help" 
options on these commands
diff --git 
a/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/interactive_crashlog_invalid_target.test
 
b/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/interactive_crashlog_invalid_target.test
index abd1e7c3da53d..7643f88bf075e 100644
--- 
a/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/interactive_crashlog_invalid_target.test
+++ 
b/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/interactive_crashlog_invalid_target.test
@@ -1,8 +1,8 @@
-# REQUIRES: python, native && target-aarch64 && system-darwin
+# REQUIRES: python, native && system-darwin
 
 # RUN: %lldb -o 'command script import lldb.macosx.crashlog' \
 # RUN: -o 'crashlog -V' \
-# RUN: -o 'crashlog -a -i -t /this_file_does_not_exist 
%S/Inputs/interactive_crashlog/multithread-test.ips' 2>&1 | FileCheck %s
+# RUN: -o 'crashlog -a -i --no-parallel-image-loading -t 
/this_file_does_not_exist %S/Inputs/interactive_crashlog/multithread-test.ips' 
2>&1 | FileCheck %s
 
 # CHECK: "crashlog" {{.*}} commands have been installed, use the "--help" 
options on these commands
 
diff --git 
a/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/interactive_crashlog_json.test
 
b/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/interactive_crashlog_json.test
index fccd71ce31f73..e523f3ccab7be 100644
--- 
a/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/interactive_crashlog_json.test
+++ 
b/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/interactive_crashlo

[Lldb-commits] [lldb] [lldb] Fix inconsistencies in DWARFExpression errors (PR #94554)

2024-06-05 Thread Jonas Devlieghere via lldb-commits


https://github.com/JDevlieghere created 
https://github.com/llvm/llvm-project/pull/94554

This patch make all errors start with a lowercase letter and removes trailing 
periods and newlines. This fixes inconsistencies between error messages and 
facilitate concatenating them.

>From aa32cb4434a0e87fac423cfb5e0f2ed946de80c5 Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere 
Date: Wed, 5 Jun 2024 19:00:35 -0700
Subject: [PATCH] [lldb] Fix inconsistencies in DWARFExpression errors

This patch make all errors start with a lowercase letter and removes
trailing periods and newlines. This fixes inconsistencies between error
messages and facilitate concatenating them.
---
 lldb/source/Expression/DWARFExpression.cpp | 169 ++---
 1 file changed, 80 insertions(+), 89 deletions(-)

diff --git a/lldb/source/Expression/DWARFExpression.cpp 
b/lldb/source/Expression/DWARFExpression.cpp
index 80d03c84fadbd..4681dbafb6f9c 100644
--- a/lldb/source/Expression/DWARFExpression.cpp
+++ b/lldb/source/Expression/DWARFExpression.cpp
@@ -973,7 +973,7 @@ llvm::Expected DWARFExpression::Evaluate(
 case DW_OP_deref: {
   if (stack.empty())
 return llvm::createStringError(
-"Expression stack empty for DW_OP_deref.");
+"expression stack empty for DW_OP_deref");
   Value::ValueType value_type = stack.back().GetValueType();
   switch (value_type) {
   case Value::ValueType::HostAddress: {
@@ -1023,16 +1023,16 @@ llvm::Expected DWARFExpression::Evaluate(
   pointer_addr, error.AsCString());
 }
   } else {
-return llvm::createStringError("NULL process for DW_OP_deref.\n");
+return llvm::createStringError("NULL process for DW_OP_deref");
   }
 } else {
   return llvm::createStringError(
-  "NULL execution context for DW_OP_deref.\n");
+  "NULL execution context for DW_OP_deref");
 }
 break;
 
   case Value::ValueType::Invalid:
-return llvm::createStringError("Invalid value type for 
DW_OP_deref.\n");
+return llvm::createStringError("invalid value type for DW_OP_deref");
   }
 
 } break;
@@ -1052,7 +1052,7 @@ llvm::Expected DWARFExpression::Evaluate(
 case DW_OP_deref_size: {
   if (stack.empty()) {
 return llvm::createStringError(
-"Expression stack empty for DW_OP_deref_size.");
+"expression stack empty for DW_OP_deref_size");
   }
   uint8_t size = opcodes.GetU8(&offset);
   if (size > 8) {
@@ -1164,18 +1164,17 @@ llvm::Expected DWARFExpression::Evaluate(
 }
   } else {
 
-return llvm::createStringError(
-"NULL process for DW_OP_deref_size.\n");
+return llvm::createStringError("NULL process for 
DW_OP_deref_size");
   }
 } else {
   return llvm::createStringError(
-  "NULL execution context for DW_OP_deref_size.\n");
+  "NULL execution context for DW_OP_deref_size");
 }
 break;
 
   case Value::ValueType::Invalid:
 
-return llvm::createStringError("Invalid value for 
DW_OP_deref_size.\n");
+return llvm::createStringError("invalid value for DW_OP_deref_size");
   }
 
 } break;
@@ -1196,9 +1195,7 @@ llvm::Expected DWARFExpression::Evaluate(
 // extended to the size of an address on the target machine before being
 // pushed on the expression stack.
 case DW_OP_xderef_size:
-
-  return llvm::createStringError(
-  "Unimplemented opcode: DW_OP_xderef_size.");
+  return llvm::createStringError("unimplemented opcode: 
DW_OP_xderef_size");
 // OPCODE: DW_OP_xderef
 // OPERANDS: none
 // DESCRIPTION: Provides an extended dereference mechanism. The entry at
@@ -1210,7 +1207,7 @@ llvm::Expected DWARFExpression::Evaluate(
 // retrieved from the dereferenced address is the size of an address on the
 // target machine.
 case DW_OP_xderef:
-  return llvm::createStringError("Unimplemented opcode: DW_OP_xderef.");
+  return llvm::createStringError("unimplemented opcode: DW_OP_xderef");
 
 // All DW_OP_constXXX opcodes have a single operand as noted below:
 //
@@ -1263,7 +1260,7 @@ llvm::Expected DWARFExpression::Evaluate(
 // DESCRIPTION: duplicates the value at the top of the stack
 case DW_OP_dup:
   if (stack.empty()) {
-return llvm::createStringError("Expression stack empty for 
DW_OP_dup.");
+return llvm::createStringError("expression stack empty for DW_OP_dup");
   } else
 stack.push_back(stack.back());
   break;
@@ -1273,8 +1270,7 @@ llvm::Expected DWARFExpression::Evaluate(
 // DESCRIPTION: pops the value at the top of the stack
 case DW_OP_drop:
   if (stack.empty()) {
-return llvm::createStringError(
-"Expression stack empty for DW_OP_drop.");
+return llvm::createStringError("expres

[Lldb-commits] [lldb] [llvm] [lldb] Return an llvm::Expected from DWARFExpression::Evaluate (NFCI) (PR #94420)

2024-06-05 Thread Jonas Devlieghere via lldb-commits



@@ -1350,10 +1300,8 @@ bool DWARFExpression::Evaluate(
   if (pick_idx < stack.size())
 stack.push_back(stack[stack.size() - 1 - pick_idx]);
   else {
-if (error_ptr)
-  error_ptr->SetErrorStringWithFormat(
-  "Index %u out of range for DW_OP_pick.\n", pick_idx);
-return false;
+return llvm::createStringError(
+"Index %u out of range for DW_OP_pick.\n", pick_idx);

JDevlieghere wrote:

https://github.com/llvm/llvm-project/pull/94554

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

[Lldb-commits] [lldb] [lldb] Fix inconsistencies in DWARFExpression errors (PR #94554)

2024-06-05 Thread via lldb-commits


llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: Jonas Devlieghere (JDevlieghere)


Changes

This patch make all errors start with a lowercase letter and removes trailing 
periods and newlines. This fixes inconsistencies between error messages and 
facilitate concatenating them.

---

Patch is 23.78 KiB, truncated to 20.00 KiB below, full version: 
https://github.com/llvm/llvm-project/pull/94554.diff


1 Files Affected:

- (modified) lldb/source/Expression/DWARFExpression.cpp (+80-89) 


``diff
diff --git a/lldb/source/Expression/DWARFExpression.cpp 
b/lldb/source/Expression/DWARFExpression.cpp
index 80d03c84fadbd..4681dbafb6f9c 100644
--- a/lldb/source/Expression/DWARFExpression.cpp
+++ b/lldb/source/Expression/DWARFExpression.cpp
@@ -973,7 +973,7 @@ llvm::Expected DWARFExpression::Evaluate(
 case DW_OP_deref: {
   if (stack.empty())
 return llvm::createStringError(
-"Expression stack empty for DW_OP_deref.");
+"expression stack empty for DW_OP_deref");
   Value::ValueType value_type = stack.back().GetValueType();
   switch (value_type) {
   case Value::ValueType::HostAddress: {
@@ -1023,16 +1023,16 @@ llvm::Expected DWARFExpression::Evaluate(
   pointer_addr, error.AsCString());
 }
   } else {
-return llvm::createStringError("NULL process for DW_OP_deref.\n");
+return llvm::createStringError("NULL process for DW_OP_deref");
   }
 } else {
   return llvm::createStringError(
-  "NULL execution context for DW_OP_deref.\n");
+  "NULL execution context for DW_OP_deref");
 }
 break;
 
   case Value::ValueType::Invalid:
-return llvm::createStringError("Invalid value type for 
DW_OP_deref.\n");
+return llvm::createStringError("invalid value type for DW_OP_deref");
   }
 
 } break;
@@ -1052,7 +1052,7 @@ llvm::Expected DWARFExpression::Evaluate(
 case DW_OP_deref_size: {
   if (stack.empty()) {
 return llvm::createStringError(
-"Expression stack empty for DW_OP_deref_size.");
+"expression stack empty for DW_OP_deref_size");
   }
   uint8_t size = opcodes.GetU8(&offset);
   if (size > 8) {
@@ -1164,18 +1164,17 @@ llvm::Expected DWARFExpression::Evaluate(
 }
   } else {
 
-return llvm::createStringError(
-"NULL process for DW_OP_deref_size.\n");
+return llvm::createStringError("NULL process for 
DW_OP_deref_size");
   }
 } else {
   return llvm::createStringError(
-  "NULL execution context for DW_OP_deref_size.\n");
+  "NULL execution context for DW_OP_deref_size");
 }
 break;
 
   case Value::ValueType::Invalid:
 
-return llvm::createStringError("Invalid value for 
DW_OP_deref_size.\n");
+return llvm::createStringError("invalid value for DW_OP_deref_size");
   }
 
 } break;
@@ -1196,9 +1195,7 @@ llvm::Expected DWARFExpression::Evaluate(
 // extended to the size of an address on the target machine before being
 // pushed on the expression stack.
 case DW_OP_xderef_size:
-
-  return llvm::createStringError(
-  "Unimplemented opcode: DW_OP_xderef_size.");
+  return llvm::createStringError("unimplemented opcode: 
DW_OP_xderef_size");
 // OPCODE: DW_OP_xderef
 // OPERANDS: none
 // DESCRIPTION: Provides an extended dereference mechanism. The entry at
@@ -1210,7 +1207,7 @@ llvm::Expected DWARFExpression::Evaluate(
 // retrieved from the dereferenced address is the size of an address on the
 // target machine.
 case DW_OP_xderef:
-  return llvm::createStringError("Unimplemented opcode: DW_OP_xderef.");
+  return llvm::createStringError("unimplemented opcode: DW_OP_xderef");
 
 // All DW_OP_constXXX opcodes have a single operand as noted below:
 //
@@ -1263,7 +1260,7 @@ llvm::Expected DWARFExpression::Evaluate(
 // DESCRIPTION: duplicates the value at the top of the stack
 case DW_OP_dup:
   if (stack.empty()) {
-return llvm::createStringError("Expression stack empty for 
DW_OP_dup.");
+return llvm::createStringError("expression stack empty for DW_OP_dup");
   } else
 stack.push_back(stack.back());
   break;
@@ -1273,8 +1270,7 @@ llvm::Expected DWARFExpression::Evaluate(
 // DESCRIPTION: pops the value at the top of the stack
 case DW_OP_drop:
   if (stack.empty()) {
-return llvm::createStringError(
-"Expression stack empty for DW_OP_drop.");
+return llvm::createStringError("expression stack empty for 
DW_OP_drop");
   } else
 stack.pop_back();
   break;
@@ -1286,7 +1282,7 @@ llvm::Expected DWARFExpression::Evaluate(
 case DW_OP_over:
   if (stack.size() < 2) {
 return llvm::createStringError(
-"Expression stack nee

[Lldb-commits] [lldb] [lldb] Refactor ReadRegisterValueAsScalar to return an llvm::Error (NFC) (PR #94556)

2024-06-05 Thread Jonas Devlieghere via lldb-commits


https://github.com/JDevlieghere created 
https://github.com/llvm/llvm-project/pull/94556

None

>From b20884bc0db2a0cb0bb928beb629670d8c86369f Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere 
Date: Wed, 5 Jun 2024 19:36:39 -0700
Subject: [PATCH] [lldb] Refactor ReadRegisterValueAsScalar to return an
 llvm::Error (NFC)

---
 lldb/source/Expression/DWARFExpression.cpp | 130 +
 1 file changed, 56 insertions(+), 74 deletions(-)

diff --git a/lldb/source/Expression/DWARFExpression.cpp 
b/lldb/source/Expression/DWARFExpression.cpp
index 80d03c84fadbd..50426ab5003dc 100644
--- a/lldb/source/Expression/DWARFExpression.cpp
+++ b/lldb/source/Expression/DWARFExpression.cpp
@@ -94,51 +94,38 @@ void DWARFExpression::SetRegisterKind(RegisterKind 
reg_kind) {
   m_reg_kind = reg_kind;
 }
 
-
-static bool ReadRegisterValueAsScalar(RegisterContext *reg_ctx,
-  lldb::RegisterKind reg_kind,
-  uint32_t reg_num, Status *error_ptr,
-  Value &value) {
-  if (reg_ctx == nullptr) {
-if (error_ptr)
-  error_ptr->SetErrorString("No register context in frame.\n");
-  } else {
-uint32_t native_reg =
-reg_ctx->ConvertRegisterKindToRegisterNumber(reg_kind, reg_num);
-if (native_reg == LLDB_INVALID_REGNUM) {
-  if (error_ptr)
-error_ptr->SetErrorStringWithFormat("Unable to convert register "
-"kind=%u reg_num=%u to a native "
-"register number.\n",
-reg_kind, reg_num);
-} else {
-  const RegisterInfo *reg_info =
-  reg_ctx->GetRegisterInfoAtIndex(native_reg);
-  RegisterValue reg_value;
-  if (reg_ctx->ReadRegister(reg_info, reg_value)) {
-if (reg_value.GetScalarValue(value.GetScalar())) {
-  value.SetValueType(Value::ValueType::Scalar);
-  value.SetContext(Value::ContextType::RegisterInfo,
-   const_cast(reg_info));
-  if (error_ptr)
-error_ptr->Clear();
-  return true;
-} else {
-  // If we get this error, then we need to implement a value buffer in
-  // the dwarf expression evaluation function...
-  if (error_ptr)
-error_ptr->SetErrorStringWithFormat(
-"register %s can't be converted to a scalar value",
-reg_info->name);
-}
-  } else {
-if (error_ptr)
-  error_ptr->SetErrorStringWithFormat("register %s is not available",
-  reg_info->name);
-  }
+static llvm::Error ReadRegisterValueAsScalar(RegisterContext *reg_ctx,
+ lldb::RegisterKind reg_kind,
+ uint32_t reg_num, Value &value) {
+  if (reg_ctx == nullptr)
+return llvm::createStringError("no register context in frame");
+
+  const uint32_t native_reg =
+  reg_ctx->ConvertRegisterKindToRegisterNumber(reg_kind, reg_num);
+  if (native_reg == LLDB_INVALID_REGNUM)
+return llvm::createStringError(
+"unable to convert register kind=%u reg_num=%u to a native "
+"register number",
+reg_kind, reg_num);
+
+  const RegisterInfo *reg_info = reg_ctx->GetRegisterInfoAtIndex(native_reg);
+  RegisterValue reg_value;
+  if (reg_ctx->ReadRegister(reg_info, reg_value)) {
+if (reg_value.GetScalarValue(value.GetScalar())) {
+  value.SetValueType(Value::ValueType::Scalar);
+  value.SetContext(Value::ContextType::RegisterInfo,
+   const_cast(reg_info));
+  return llvm::Error::success();
 }
+
+// If we get this error, then we need to implement a value buffer in
+// the dwarf expression evaluation function...
+return llvm::createStringError(
+"register %s can't be converted to a scalar value", reg_info->name);
   }
-  return false;
+
+  return llvm::createStringError("register %s is not available",
+ reg_info->name);
 }
 
 /// Return the length in bytes of the set of operands for \p op. No guarantees
@@ -1839,11 +1826,10 @@ llvm::Expected DWARFExpression::Evaluate(
   dwarf4_location_description_kind = Register;
   reg_num = op - DW_OP_reg0;
 
-  Status read_err;
-  if (ReadRegisterValueAsScalar(reg_ctx, reg_kind, reg_num, &read_err, 
tmp))
-stack.push_back(tmp);
-  else
-return read_err.ToError();
+  if (llvm::Error err =
+  ReadRegisterValueAsScalar(reg_ctx, reg_kind, reg_num, tmp))
+return err;
+  stack.push_back(tmp);
 } break;
 // OPCODE: DW_OP_regx
 // OPERANDS:
@@ -1853,10 +1839,10 @@ llvm::Expected DWARFExpression::Evaluate(
   dwarf4_location_description_kind = Register;
   reg_num = opcodes.GetULEB128(&offset);
   Status read_err;
-  if (ReadRegisterValu

[Lldb-commits] [lldb] [lldb] Refactor ReadRegisterValueAsScalar to return an llvm::Error (NFC) (PR #94556)

2024-06-05 Thread via lldb-commits


llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: Jonas Devlieghere (JDevlieghere)


Changes



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


1 Files Affected:

- (modified) lldb/source/Expression/DWARFExpression.cpp (+56-74) 


``diff
diff --git a/lldb/source/Expression/DWARFExpression.cpp 
b/lldb/source/Expression/DWARFExpression.cpp
index 80d03c84fadbd..50426ab5003dc 100644
--- a/lldb/source/Expression/DWARFExpression.cpp
+++ b/lldb/source/Expression/DWARFExpression.cpp
@@ -94,51 +94,38 @@ void DWARFExpression::SetRegisterKind(RegisterKind 
reg_kind) {
   m_reg_kind = reg_kind;
 }
 
-
-static bool ReadRegisterValueAsScalar(RegisterContext *reg_ctx,
-  lldb::RegisterKind reg_kind,
-  uint32_t reg_num, Status *error_ptr,
-  Value &value) {
-  if (reg_ctx == nullptr) {
-if (error_ptr)
-  error_ptr->SetErrorString("No register context in frame.\n");
-  } else {
-uint32_t native_reg =
-reg_ctx->ConvertRegisterKindToRegisterNumber(reg_kind, reg_num);
-if (native_reg == LLDB_INVALID_REGNUM) {
-  if (error_ptr)
-error_ptr->SetErrorStringWithFormat("Unable to convert register "
-"kind=%u reg_num=%u to a native "
-"register number.\n",
-reg_kind, reg_num);
-} else {
-  const RegisterInfo *reg_info =
-  reg_ctx->GetRegisterInfoAtIndex(native_reg);
-  RegisterValue reg_value;
-  if (reg_ctx->ReadRegister(reg_info, reg_value)) {
-if (reg_value.GetScalarValue(value.GetScalar())) {
-  value.SetValueType(Value::ValueType::Scalar);
-  value.SetContext(Value::ContextType::RegisterInfo,
-   const_cast(reg_info));
-  if (error_ptr)
-error_ptr->Clear();
-  return true;
-} else {
-  // If we get this error, then we need to implement a value buffer in
-  // the dwarf expression evaluation function...
-  if (error_ptr)
-error_ptr->SetErrorStringWithFormat(
-"register %s can't be converted to a scalar value",
-reg_info->name);
-}
-  } else {
-if (error_ptr)
-  error_ptr->SetErrorStringWithFormat("register %s is not available",
-  reg_info->name);
-  }
+static llvm::Error ReadRegisterValueAsScalar(RegisterContext *reg_ctx,
+ lldb::RegisterKind reg_kind,
+ uint32_t reg_num, Value &value) {
+  if (reg_ctx == nullptr)
+return llvm::createStringError("no register context in frame");
+
+  const uint32_t native_reg =
+  reg_ctx->ConvertRegisterKindToRegisterNumber(reg_kind, reg_num);
+  if (native_reg == LLDB_INVALID_REGNUM)
+return llvm::createStringError(
+"unable to convert register kind=%u reg_num=%u to a native "
+"register number",
+reg_kind, reg_num);
+
+  const RegisterInfo *reg_info = reg_ctx->GetRegisterInfoAtIndex(native_reg);
+  RegisterValue reg_value;
+  if (reg_ctx->ReadRegister(reg_info, reg_value)) {
+if (reg_value.GetScalarValue(value.GetScalar())) {
+  value.SetValueType(Value::ValueType::Scalar);
+  value.SetContext(Value::ContextType::RegisterInfo,
+   const_cast(reg_info));
+  return llvm::Error::success();
 }
+
+// If we get this error, then we need to implement a value buffer in
+// the dwarf expression evaluation function...
+return llvm::createStringError(
+"register %s can't be converted to a scalar value", reg_info->name);
   }
-  return false;
+
+  return llvm::createStringError("register %s is not available",
+ reg_info->name);
 }
 
 /// Return the length in bytes of the set of operands for \p op. No guarantees
@@ -1839,11 +1826,10 @@ llvm::Expected DWARFExpression::Evaluate(
   dwarf4_location_description_kind = Register;
   reg_num = op - DW_OP_reg0;
 
-  Status read_err;
-  if (ReadRegisterValueAsScalar(reg_ctx, reg_kind, reg_num, &read_err, 
tmp))
-stack.push_back(tmp);
-  else
-return read_err.ToError();
+  if (llvm::Error err =
+  ReadRegisterValueAsScalar(reg_ctx, reg_kind, reg_num, tmp))
+return err;
+  stack.push_back(tmp);
 } break;
 // OPCODE: DW_OP_regx
 // OPERANDS:
@@ -1853,10 +1839,10 @@ llvm::Expected DWARFExpression::Evaluate(
   dwarf4_location_description_kind = Register;
   reg_num = opcodes.GetULEB128(&offset);
   Status read_err;
-  if (ReadRegisterValueAsScalar(reg_ctx, reg_kind, reg_num, &read_err, 
tmp))
-stack.push_back(tmp);
-  else
-return read_err.ToError();
+  if (llvm::Error err =
+

[Lldb-commits] [lldb] [lldb] Refactor ResolveLoadAddress to return an llvm::Expected (NFC) (PR #94558)

2024-06-05 Thread Jonas Devlieghere via lldb-commits


https://github.com/JDevlieghere created 
https://github.com/llvm/llvm-project/pull/94558

None

>From 6bb911ab3ee8a9f9d10662ed41442be85da9df14 Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere 
Date: Wed, 5 Jun 2024 19:42:50 -0700
Subject: [PATCH] [lldb] Refactor ResolveLoadAddress to return an
 llvm::Expected (NFC)

---
 lldb/source/Expression/DWARFExpression.cpp | 48 --
 1 file changed, 17 insertions(+), 31 deletions(-)

diff --git a/lldb/source/Expression/DWARFExpression.cpp 
b/lldb/source/Expression/DWARFExpression.cpp
index 80d03c84fadbd..f0abaa07d7197 100644
--- a/lldb/source/Expression/DWARFExpression.cpp
+++ b/lldb/source/Expression/DWARFExpression.cpp
@@ -782,7 +782,6 @@ void UpdateValueTypeFromLocationDescription(Log *log, const 
DWARFUnit *dwarf_cu,
 ///
 /// \param exe_ctx Pointer to the execution context
 /// \param module_sp shared_ptr contains the module if we have one
-/// \param error_ptr pointer to Status object if we have one
 /// \param dw_op_type C-style string used to vary the error output
 /// \param file_addr the file address we are trying to resolve and turn into a
 ///  load address
@@ -793,32 +792,22 @@ void UpdateValueTypeFromLocationDescription(Log *log, 
const DWARFUnit *dwarf_cu,
 ///  the load address succeed or an empty Optinal otherwise. If
 ///  check_sectionoffset is true we consider LLDB_INVALID_ADDRESS a
 ///  success if so_addr.IsSectionOffset() is true.
-static std::optional
+static llvm::Expected
 ResolveLoadAddress(ExecutionContext *exe_ctx, lldb::ModuleSP &module_sp,
-   Status *error_ptr, const char *dw_op_type,
-   lldb::addr_t file_addr, Address &so_addr,
-   bool check_sectionoffset = false) {
-  if (!module_sp) {
-if (error_ptr)
-  error_ptr->SetErrorStringWithFormat(
-  "need module to resolve file address for %s", dw_op_type);
-return {};
-  }
+   const char *dw_op_type, lldb::addr_t file_addr,
+   Address &so_addr, bool check_sectionoffset = false) {
+  if (!module_sp)
+return llvm::createStringError("need module to resolve file address for 
%s",
+   dw_op_type);
 
-  if (!module_sp->ResolveFileAddress(file_addr, so_addr)) {
-if (error_ptr)
-  error_ptr->SetErrorString("failed to resolve file address in module");
-return {};
-  }
+  if (!module_sp->ResolveFileAddress(file_addr, so_addr))
+return llvm::createStringError("failed to resolve file address in module");
 
-  addr_t load_addr = so_addr.GetLoadAddress(exe_ctx->GetTargetPtr());
+  const addr_t load_addr = so_addr.GetLoadAddress(exe_ctx->GetTargetPtr());
 
   if (load_addr == LLDB_INVALID_ADDRESS &&
-  (check_sectionoffset && !so_addr.IsSectionOffset())) {
-if (error_ptr)
-  error_ptr->SetErrorString("failed to resolve load address");
-return {};
-  }
+  (check_sectionoffset && !so_addr.IsSectionOffset()))
+return llvm::createStringError("failed to resolve load address");
 
   return load_addr;
 }
@@ -988,12 +977,11 @@ llvm::Expected DWARFExpression::Evaluate(
 LLDB_INVALID_ADDRESS);
 
 Address so_addr;
-Status load_err;
 auto maybe_load_addr = ResolveLoadAddress(
-exe_ctx, module_sp, &load_err, "DW_OP_deref", file_addr, so_addr);
+exe_ctx, module_sp, "DW_OP_deref", file_addr, so_addr);
 
 if (!maybe_load_addr)
-  return load_err.ToError();
+  return maybe_load_addr.takeError();
 
 stack.back().GetScalar() = *maybe_load_addr;
 // Fall through to load address promotion code below.
@@ -1105,14 +1093,12 @@ llvm::Expected DWARFExpression::Evaluate(
 auto file_addr =
 stack.back().GetScalar().ULongLong(LLDB_INVALID_ADDRESS);
 Address so_addr;
-Status resolve_err;
-auto maybe_load_addr =
-ResolveLoadAddress(exe_ctx, module_sp, &resolve_err,
-   "DW_OP_deref_size", file_addr, so_addr,
-   /*check_sectionoffset=*/true);
+auto maybe_load_addr = ResolveLoadAddress(
+exe_ctx, module_sp, "DW_OP_deref_size", file_addr, so_addr,
+/*check_sectionoffset=*/true);
 
 if (!maybe_load_addr)
-  return resolve_err.ToError();
+  return maybe_load_addr.takeError();
 
 addr_t load_addr = *maybe_load_addr;
 

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

[Lldb-commits] [lldb] [lldb] Refactor ResolveLoadAddress to return an llvm::Expected (NFC) (PR #94558)

2024-06-05 Thread via lldb-commits


llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: Jonas Devlieghere (JDevlieghere)


Changes



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


1 Files Affected:

- (modified) lldb/source/Expression/DWARFExpression.cpp (+17-31) 


``diff
diff --git a/lldb/source/Expression/DWARFExpression.cpp 
b/lldb/source/Expression/DWARFExpression.cpp
index 80d03c84fadbd..f0abaa07d7197 100644
--- a/lldb/source/Expression/DWARFExpression.cpp
+++ b/lldb/source/Expression/DWARFExpression.cpp
@@ -782,7 +782,6 @@ void UpdateValueTypeFromLocationDescription(Log *log, const 
DWARFUnit *dwarf_cu,
 ///
 /// \param exe_ctx Pointer to the execution context
 /// \param module_sp shared_ptr contains the module if we have one
-/// \param error_ptr pointer to Status object if we have one
 /// \param dw_op_type C-style string used to vary the error output
 /// \param file_addr the file address we are trying to resolve and turn into a
 ///  load address
@@ -793,32 +792,22 @@ void UpdateValueTypeFromLocationDescription(Log *log, 
const DWARFUnit *dwarf_cu,
 ///  the load address succeed or an empty Optinal otherwise. If
 ///  check_sectionoffset is true we consider LLDB_INVALID_ADDRESS a
 ///  success if so_addr.IsSectionOffset() is true.
-static std::optional
+static llvm::Expected
 ResolveLoadAddress(ExecutionContext *exe_ctx, lldb::ModuleSP &module_sp,
-   Status *error_ptr, const char *dw_op_type,
-   lldb::addr_t file_addr, Address &so_addr,
-   bool check_sectionoffset = false) {
-  if (!module_sp) {
-if (error_ptr)
-  error_ptr->SetErrorStringWithFormat(
-  "need module to resolve file address for %s", dw_op_type);
-return {};
-  }
+   const char *dw_op_type, lldb::addr_t file_addr,
+   Address &so_addr, bool check_sectionoffset = false) {
+  if (!module_sp)
+return llvm::createStringError("need module to resolve file address for 
%s",
+   dw_op_type);
 
-  if (!module_sp->ResolveFileAddress(file_addr, so_addr)) {
-if (error_ptr)
-  error_ptr->SetErrorString("failed to resolve file address in module");
-return {};
-  }
+  if (!module_sp->ResolveFileAddress(file_addr, so_addr))
+return llvm::createStringError("failed to resolve file address in module");
 
-  addr_t load_addr = so_addr.GetLoadAddress(exe_ctx->GetTargetPtr());
+  const addr_t load_addr = so_addr.GetLoadAddress(exe_ctx->GetTargetPtr());
 
   if (load_addr == LLDB_INVALID_ADDRESS &&
-  (check_sectionoffset && !so_addr.IsSectionOffset())) {
-if (error_ptr)
-  error_ptr->SetErrorString("failed to resolve load address");
-return {};
-  }
+  (check_sectionoffset && !so_addr.IsSectionOffset()))
+return llvm::createStringError("failed to resolve load address");
 
   return load_addr;
 }
@@ -988,12 +977,11 @@ llvm::Expected DWARFExpression::Evaluate(
 LLDB_INVALID_ADDRESS);
 
 Address so_addr;
-Status load_err;
 auto maybe_load_addr = ResolveLoadAddress(
-exe_ctx, module_sp, &load_err, "DW_OP_deref", file_addr, so_addr);
+exe_ctx, module_sp, "DW_OP_deref", file_addr, so_addr);
 
 if (!maybe_load_addr)
-  return load_err.ToError();
+  return maybe_load_addr.takeError();
 
 stack.back().GetScalar() = *maybe_load_addr;
 // Fall through to load address promotion code below.
@@ -1105,14 +1093,12 @@ llvm::Expected DWARFExpression::Evaluate(
 auto file_addr =
 stack.back().GetScalar().ULongLong(LLDB_INVALID_ADDRESS);
 Address so_addr;
-Status resolve_err;
-auto maybe_load_addr =
-ResolveLoadAddress(exe_ctx, module_sp, &resolve_err,
-   "DW_OP_deref_size", file_addr, so_addr,
-   /*check_sectionoffset=*/true);
+auto maybe_load_addr = ResolveLoadAddress(
+exe_ctx, module_sp, "DW_OP_deref_size", file_addr, so_addr,
+/*check_sectionoffset=*/true);
 
 if (!maybe_load_addr)
-  return resolve_err.ToError();
+  return maybe_load_addr.takeError();
 
 addr_t load_addr = *maybe_load_addr;
 

``




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

[Lldb-commits] [lldb] [lldb] Fix inconsistencies in DWARFExpression errors (PR #94554)

2024-06-05 Thread Adrian Prantl via lldb-commits



@@ -1657,7 +1650,7 @@ llvm::Expected DWARFExpression::Evaluate(
 case DW_OP_eq:
   if (stack.size() < 2) {
 return llvm::createStringError(
-"Expression stack needs at least 2 items for DW_OP_eq.");
+"expression stack needs at least 2 items for DW_OP_eq");

adrian-prantl wrote:

The obvious next step to refactor will be to hoist the arity check of the 
operators into a single helper that uses a table and a %s in the error message 
:-)

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

[Lldb-commits] [lldb] [lldb] Fix inconsistencies in DWARFExpression errors (PR #94554)

2024-06-05 Thread Adrian Prantl via lldb-commits


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


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

[Lldb-commits] [lldb] [lldb] Fix inconsistencies in DWARFExpression errors (PR #94554)

2024-06-05 Thread Jonas Devlieghere via lldb-commits



@@ -1657,7 +1650,7 @@ llvm::Expected DWARFExpression::Evaluate(
 case DW_OP_eq:
   if (stack.size() < 2) {
 return llvm::createStringError(
-"Expression stack needs at least 2 items for DW_OP_eq.");
+"expression stack needs at least 2 items for DW_OP_eq");

JDevlieghere wrote:

Sounds like a job for Dwarf.def!

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

[Lldb-commits] [lldb] 6ca0f44 - [lldb] Fix inconsistencies in DWARFExpression errors (#94554)

2024-06-05 Thread via lldb-commits


Author: Jonas Devlieghere
Date: 2024-06-05T19:47:49-07:00
New Revision: 6ca0f44cd89aa802c306c303764eabf83a7f5029

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

LOG: [lldb] Fix inconsistencies in DWARFExpression errors (#94554)

This patch make all errors start with a lowercase letter and removes
trailing periods and newlines. This fixes inconsistencies between error
messages and facilitate concatenating them.

Added: 


Modified: 
lldb/source/Expression/DWARFExpression.cpp

Removed: 




diff  --git a/lldb/source/Expression/DWARFExpression.cpp 
b/lldb/source/Expression/DWARFExpression.cpp
index 80d03c84fadbd..4681dbafb6f9c 100644
--- a/lldb/source/Expression/DWARFExpression.cpp
+++ b/lldb/source/Expression/DWARFExpression.cpp
@@ -973,7 +973,7 @@ llvm::Expected DWARFExpression::Evaluate(
 case DW_OP_deref: {
   if (stack.empty())
 return llvm::createStringError(
-"Expression stack empty for DW_OP_deref.");
+"expression stack empty for DW_OP_deref");
   Value::ValueType value_type = stack.back().GetValueType();
   switch (value_type) {
   case Value::ValueType::HostAddress: {
@@ -1023,16 +1023,16 @@ llvm::Expected DWARFExpression::Evaluate(
   pointer_addr, error.AsCString());
 }
   } else {
-return llvm::createStringError("NULL process for DW_OP_deref.\n");
+return llvm::createStringError("NULL process for DW_OP_deref");
   }
 } else {
   return llvm::createStringError(
-  "NULL execution context for DW_OP_deref.\n");
+  "NULL execution context for DW_OP_deref");
 }
 break;
 
   case Value::ValueType::Invalid:
-return llvm::createStringError("Invalid value type for 
DW_OP_deref.\n");
+return llvm::createStringError("invalid value type for DW_OP_deref");
   }
 
 } break;
@@ -1052,7 +1052,7 @@ llvm::Expected DWARFExpression::Evaluate(
 case DW_OP_deref_size: {
   if (stack.empty()) {
 return llvm::createStringError(
-"Expression stack empty for DW_OP_deref_size.");
+"expression stack empty for DW_OP_deref_size");
   }
   uint8_t size = opcodes.GetU8(&offset);
   if (size > 8) {
@@ -1164,18 +1164,17 @@ llvm::Expected DWARFExpression::Evaluate(
 }
   } else {
 
-return llvm::createStringError(
-"NULL process for DW_OP_deref_size.\n");
+return llvm::createStringError("NULL process for 
DW_OP_deref_size");
   }
 } else {
   return llvm::createStringError(
-  "NULL execution context for DW_OP_deref_size.\n");
+  "NULL execution context for DW_OP_deref_size");
 }
 break;
 
   case Value::ValueType::Invalid:
 
-return llvm::createStringError("Invalid value for 
DW_OP_deref_size.\n");
+return llvm::createStringError("invalid value for DW_OP_deref_size");
   }
 
 } break;
@@ -1196,9 +1195,7 @@ llvm::Expected DWARFExpression::Evaluate(
 // extended to the size of an address on the target machine before being
 // pushed on the expression stack.
 case DW_OP_xderef_size:
-
-  return llvm::createStringError(
-  "Unimplemented opcode: DW_OP_xderef_size.");
+  return llvm::createStringError("unimplemented opcode: 
DW_OP_xderef_size");
 // OPCODE: DW_OP_xderef
 // OPERANDS: none
 // DESCRIPTION: Provides an extended dereference mechanism. The entry at
@@ -1210,7 +1207,7 @@ llvm::Expected DWARFExpression::Evaluate(
 // retrieved from the dereferenced address is the size of an address on the
 // target machine.
 case DW_OP_xderef:
-  return llvm::createStringError("Unimplemented opcode: DW_OP_xderef.");
+  return llvm::createStringError("unimplemented opcode: DW_OP_xderef");
 
 // All DW_OP_constXXX opcodes have a single operand as noted below:
 //
@@ -1263,7 +1260,7 @@ llvm::Expected DWARFExpression::Evaluate(
 // DESCRIPTION: duplicates the value at the top of the stack
 case DW_OP_dup:
   if (stack.empty()) {
-return llvm::createStringError("Expression stack empty for 
DW_OP_dup.");
+return llvm::createStringError("expression stack empty for DW_OP_dup");
   } else
 stack.push_back(stack.back());
   break;
@@ -1273,8 +1270,7 @@ llvm::Expected DWARFExpression::Evaluate(
 // DESCRIPTION: pops the value at the top of the stack
 case DW_OP_drop:
   if (stack.empty()) {
-return llvm::createStringError(
-"Expression stack empty for DW_OP_drop.");
+return llvm::createStringError("expression stack empty for 
DW_OP_drop");
   } else
 stack.po

[Lldb-commits] [lldb] [lldb] Fix inconsistencies in DWARFExpression errors (PR #94554)

2024-06-05 Thread Jonas Devlieghere via lldb-commits


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

1 2 >

1 - 100 of 106 matches

Mail list logo