[Lldb-commits] [lldb] [lldb] [debugserver] fix qLaunchSuccess error, add QErrorStringInPacketSupported (PR #82593)

2024-02-22 Thread David Spickett via lldb-commits

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


[Lldb-commits] [lldb] [lldb] [debugserver] fix qLaunchSuccess error, add QErrorStringInPacketSupported (PR #82593)

2024-02-22 Thread David Spickett via lldb-commits

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


[Lldb-commits] [lldb] [lldb] Fix nullptr dereference on running x86 binary with x86-disabled llvm (PR #82603)

2024-02-22 Thread Daniil Kovalev via lldb-commits

https://github.com/kovdan01 created 
https://github.com/llvm/llvm-project/pull/82603

If `LLVM_TARGETS_TO_BUILD` does not contain `X86` and we try to run an x86 
binary in lldb, we get a `nullptr` dereference in `LLVMDisasmInstruction(...)`. 
We try to call `getDisAsm()` method on a `LLVMDisasmContext *DC` which is null. 
The pointer is passed from 
`x86AssemblyInspectionEngine::instruction_length(...)` and is originally 
`m_disasm_context` member of `x86AssemblyInspectionEngine`. This should be 
filled by `LLVMCreateDisasm(...)` in the class constructor, but not having X86 
target enabled in llvm makes `TargetRegistry::lookupTarget(...)` call return 
`nullptr`, which results in `m_disasm_context` initialized with `nullptr` as 
well.

This patch adds if statements against `m_disasm_context` in 
`x86AssemblyInspectionEngine::GetNonCallSiteUnwindPlanFromAssembly(...)` and 
`x86AssemblyInspectionEngine::FindFirstNonPrologueInstruction(...)` so 
subsequent calls to `x86AssemblyInspectionEngine::instruction_length(...)` do 
not cause a null pointer dereference.

>From 5c9ac5382958d88cbe2b89128957b3a0908c9d88 Mon Sep 17 00:00:00 2001
From: Daniil Kovalev 
Date: Thu, 22 Feb 2024 11:42:44 +0300
Subject: [PATCH] [lldb] Fix nullptr dereference on running x86 binary with
 x86-disabled llvm

If `LLVM_TARGETS_TO_BUILD` does not contain `X86` and we try to run an
x86 binary in lldb, we get a `nullptr` dereference in
`LLVMDisasmInstruction(...)`. We try to call `getDisAsm()` method on a
`LLVMDisasmContext *DC` which is null. The pointer is passed from
`x86AssemblyInspectionEngine::instruction_length(...)` and is originally
`m_disasm_context` member of `x86AssemblyInspectionEngine`. This should
be filled by `LLVMCreateDisasm(...)` in the class constructor, but not having
X86 target enabled in llvm makes `TargetRegistry::lookupTarget(...)`
call return `nullptr`, which results in `m_disasm_context` initialized
with `nullptr` as well.

This patch adds if statements against `m_disasm_context` in
`x86AssemblyInspectionEngine::GetNonCallSiteUnwindPlanFromAssembly(...)` and
`x86AssemblyInspectionEngine::FindFirstNonPrologueInstruction(...)` so
subsequent calls to `x86AssemblyInspectionEngine::instruction_length(...)` do
not cause a null pointer dereference.
---
 .../UnwindAssembly/x86/x86AssemblyInspectionEngine.cpp  | 6 ++
 1 file changed, 6 insertions(+)

diff --git 
a/lldb/source/Plugins/UnwindAssembly/x86/x86AssemblyInspectionEngine.cpp 
b/lldb/source/Plugins/UnwindAssembly/x86/x86AssemblyInspectionEngine.cpp
index 2032c5a68d054c..6bfaa54135a959 100644
--- a/lldb/source/Plugins/UnwindAssembly/x86/x86AssemblyInspectionEngine.cpp
+++ b/lldb/source/Plugins/UnwindAssembly/x86/x86AssemblyInspectionEngine.cpp
@@ -909,6 +909,9 @@ bool 
x86AssemblyInspectionEngine::GetNonCallSiteUnwindPlanFromAssembly(
   if (!m_register_map_initialized)
 return false;
 
+  if (m_disasm_context == nullptr)
+return false;
+
   addr_t current_func_text_offset = 0;
   int current_sp_bytes_offset_from_fa = 0;
   bool is_aligned = false;
@@ -1570,6 +1573,9 @@ bool 
x86AssemblyInspectionEngine::FindFirstNonPrologueInstruction(
   if (!m_register_map_initialized)
 return false;
 
+  if (m_disasm_context == nullptr)
+return false;
+
   while (offset < size) {
 int regno;
 int insn_len;

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


[Lldb-commits] [lldb] [lldb] Don't rely on ScriptInterpreterPythonImpl::Initialize in the unit tests (PR #82096)

2024-02-22 Thread David Spickett via lldb-commits

DavidSpickett wrote:

Thanks! Fixed the x86 bot and I confirmed it locally.

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


[Lldb-commits] [lldb] [lldb] Fix nullptr dereference on running x86 binary with x86-disabled llvm (PR #82603)

2024-02-22 Thread Daniil Kovalev via lldb-commits

kovdan01 wrote:

Please suggest ideas how this could be tested. It looks like that 
UnwindAssembly unittests is the right place for it - but UnwindAssemblyx86Tests 
cmake target is not built without X86 in `LLVM_TARGETS_TO_BUILD`, which is the 
pre-condition of the issue this PR is fixing.

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


[Lldb-commits] [lldb] [lldb] Fix nullptr dereference on running x86 binary with x86-disabled llvm (PR #82603)

2024-02-22 Thread Daniil Kovalev via lldb-commits

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


[Lldb-commits] [lldb] [lldb] Fix nullptr dereference on running x86 binary with x86-disabled llvm (PR #82603)

2024-02-22 Thread via lldb-commits

llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: Daniil Kovalev (kovdan01)


Changes

If `LLVM_TARGETS_TO_BUILD` does not contain `X86` and we try to run an x86 
binary in lldb, we get a `nullptr` dereference in `LLVMDisasmInstruction(...)`. 
We try to call `getDisAsm()` method on a `LLVMDisasmContext *DC` which is null. 
The pointer is passed from 
`x86AssemblyInspectionEngine::instruction_length(...)` and is originally 
`m_disasm_context` member of `x86AssemblyInspectionEngine`. This should be 
filled by `LLVMCreateDisasm(...)` in the class constructor, but not having X86 
target enabled in llvm makes `TargetRegistry::lookupTarget(...)` call return 
`nullptr`, which results in `m_disasm_context` initialized with `nullptr` as 
well.

This patch adds if statements against `m_disasm_context` in 
`x86AssemblyInspectionEngine::GetNonCallSiteUnwindPlanFromAssembly(...)` and 
`x86AssemblyInspectionEngine::FindFirstNonPrologueInstruction(...)` so 
subsequent calls to `x86AssemblyInspectionEngine::instruction_length(...)` do 
not cause a null pointer dereference.

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


1 Files Affected:

- (modified) 
lldb/source/Plugins/UnwindAssembly/x86/x86AssemblyInspectionEngine.cpp (+6) 


``diff
diff --git 
a/lldb/source/Plugins/UnwindAssembly/x86/x86AssemblyInspectionEngine.cpp 
b/lldb/source/Plugins/UnwindAssembly/x86/x86AssemblyInspectionEngine.cpp
index 2032c5a68d054c..6bfaa54135a959 100644
--- a/lldb/source/Plugins/UnwindAssembly/x86/x86AssemblyInspectionEngine.cpp
+++ b/lldb/source/Plugins/UnwindAssembly/x86/x86AssemblyInspectionEngine.cpp
@@ -909,6 +909,9 @@ bool 
x86AssemblyInspectionEngine::GetNonCallSiteUnwindPlanFromAssembly(
   if (!m_register_map_initialized)
 return false;
 
+  if (m_disasm_context == nullptr)
+return false;
+
   addr_t current_func_text_offset = 0;
   int current_sp_bytes_offset_from_fa = 0;
   bool is_aligned = false;
@@ -1570,6 +1573,9 @@ bool 
x86AssemblyInspectionEngine::FindFirstNonPrologueInstruction(
   if (!m_register_map_initialized)
 return false;
 
+  if (m_disasm_context == nullptr)
+return false;
+
   while (offset < size) {
 int regno;
 int insn_len;

``




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


[Lldb-commits] [lldb] [lldb] Fix nullptr dereference on running x86 binary with x86-disabled llvm (PR #82603)

2024-02-22 Thread David Spickett via lldb-commits

DavidSpickett wrote:

Can this code be hit when using an x86 core file? Then you could write a shell 
test thatis `UNSUPPORTED: x86-registered-target` (whatever the proper syntax 
is) and assert that it does not crash.

It won't get run on the build bots, they enable all targets, but it's nice to 
have anyway. Someone downstream might appreciate it.

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


[Lldb-commits] [lldb] [lldb] Implement WebAssembly debugging (PR #77949)

2024-02-22 Thread Paolo Severini via lldb-commits


@@ -1450,6 +1450,14 @@ class Process : public 
std::enable_shared_from_this,
   /// platforms where there is a difference (only Arm Thumb at this time).
   lldb::addr_t FixAnyAddress(lldb::addr_t pc);
 
+  /// Some targets might use bits in a code address to represent additional
+  /// information; for example WebAssembly targets have a different memory 
space
+  /// per module and have a different address space per memory and code.
+  virtual lldb::addr_t FixMemoryAddress(lldb::addr_t address,

paolosevMSFT wrote:

I thought to add a new function because `Process::FixCodeAddress`, 
`Process::FixDataAddress`, `Process::FixAnyAddress` by default forward the call 
to `ABI::FixCodeAddress`, `ABI::FixDataAddress`, `ABI::FixAnyAddress`, which 
are implemented by plugin-specific ABIs like `ABISysV_arm`, `ABIMacOSX_arm`. 
But here we are not really dealing with a different ABI, it is more a custom 
representation of Wasm memory addresses used between the debugger and the Wasm 
engine.

If you prefer, we could reuse `Process::FixAnyAddress`, making it virtual and 
overriding it in `ProcessWasm`.

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


[Lldb-commits] [lldb] [lldb] Implement WebAssembly debugging (PR #77949)

2024-02-22 Thread Paolo Severini via lldb-commits


@@ -346,6 +346,15 @@ static offset_t GetOpcodeDataSize(const DataExtractor 
&data,
 return (offset - data_offset) + subexpr_len;
   }
 
+  case DW_OP_WASM_location: {
+uint8_t wasm_op = data.GetU8(&offset);
+if (wasm_op == 3)

paolosevMSFT wrote:

I will add an enum in a new header file 
lldb/source/Plugins/SymbolFile/DWARF/DWARFWasm.h.

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


[Lldb-commits] [lldb] Re-land [lldb-dap] Add support for data breakpoint. (PR #81909)

2024-02-22 Thread Zequan Wu via lldb-commits

ZequanWu wrote:

Ping.

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


[Lldb-commits] [lldb] [lldb] Don't rely on ScriptInterpreterPythonImpl::Initialize in the unit tests (PR #82096)

2024-02-22 Thread Jonas Devlieghere via lldb-commits

JDevlieghere wrote:

Thank you!

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


[Lldb-commits] [lldb] [lldb] Implement WebAssembly debugging (PR #77949)

2024-02-22 Thread Xu Jun via lldb-commits


@@ -0,0 +1,108 @@
+//=== wasmRegisterContext.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 "wasmRegisterContext.h"
+#include "Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h"
+#include "ProcessWasm.h"
+#include "ThreadWasm.h"
+#include "lldb/Utility/RegisterValue.h"
+#include 
+
+using namespace lldb;
+using namespace lldb_private;
+using namespace lldb_private::process_gdb_remote;
+using namespace lldb_private::wasm;
+
+WasmRegisterContext::WasmRegisterContext(
+wasm::ThreadWasm &thread, uint32_t concrete_frame_idx,
+GDBRemoteDynamicRegisterInfoSP reg_info_sp)
+: GDBRemoteRegisterContext(thread, concrete_frame_idx, reg_info_sp, false,
+   false) {}
+
+WasmRegisterContext::~WasmRegisterContext() = default;
+
+uint32_t WasmRegisterContext::ConvertRegisterKindToRegisterNumber(
+lldb::RegisterKind kind, uint32_t num) {
+  return num;
+}
+
+size_t WasmRegisterContext::GetRegisterCount() { return 0; }
+
+const RegisterInfo *WasmRegisterContext::GetRegisterInfoAtIndex(size_t reg) {
+  uint32_t tag = (reg >> 30) & 0x03;
+  if (tag == 0) {
+return m_reg_info_sp->GetRegisterInfoAtIndex(reg);
+  }
+
+  auto it = m_register_map.find(reg);
+  if (it == m_register_map.end()) {
+WasmVirtualRegisterKinds kind =
+static_cast(tag - 1);

xujuntwt95329 wrote:

Thanks for the suggestion, that would be better.

@paolosevMSFT Maybe we can modify the `eLocal` to `1` in 
`wasmRegisterContext.h:WasmVirtualRegisterKinds` so we don't need to do 
addition and subtracting on the tag
https://github.com/paolosevMSFT/llvm-project/blob/89c4ec1b1dec51df187e84c83ad3cb25eb7e847a/lldb/source/Plugins/Process/wasm/wasmRegisterContext.h#L25

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


[Lldb-commits] [lldb] [lldb] Implement WebAssembly debugging (PR #77949)

2024-02-22 Thread Paolo Severini via lldb-commits


@@ -2595,6 +2604,37 @@ bool DWARFExpression::Evaluate(
   break;
 }
 
+case DW_OP_WASM_location: {
+  uint8_t wasm_op = opcodes.GetU8(&offset);
+  uint32_t index;
+
+  /* LLDB doesn't have an address space to represents WebAssembly locals,
+   * globals and operand stacks.
+   * We encode these elements into virtual registers:
+   *   | tag: 2 bits | index: 30 bits |
+   *   where tag is:
+   *0: Not a WebAssembly location
+   *1: Local
+   *2: Global
+   *3: Operand stack value

paolosevMSFT wrote:

It is better to keep the tag '0' to represent normal (non-Wasm-location) 
registers like PC, so I would keep the values like this. But for readability we 
can add a static function `WasmVirtualRegisterKinds 
WasmVirtualRegisterInfo::VirtualRegisterKindFromDWARFLocation(DWARFWasmLocation 
dwarf_location)` that does this conversion from the DWARF representation into 
the virtual register tag.

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


[Lldb-commits] [lldb] [lldb] Implement WebAssembly debugging (PR #77949)

2024-02-22 Thread Paolo Severini via lldb-commits

https://github.com/paolosevMSFT updated 
https://github.com/llvm/llvm-project/pull/77949

>From 30d932bb0988e1c78c3e023be2270259df5e6664 Mon Sep 17 00:00:00 2001
From: Paolo Severini 
Date: Fri, 12 Jan 2024 09:10:59 -0800
Subject: [PATCH 1/6] Add support for source-level debugging of WebAssembly
 code

---
 lldb/include/lldb/Target/Process.h|  40 +++
 lldb/source/Core/Value.cpp|   2 +-
 lldb/source/Expression/DWARFExpression.cpp|  41 +++
 .../source/Interpreter/CommandInterpreter.cpp |  18 ++
 lldb/source/Plugins/Process/CMakeLists.txt|   1 +
 .../Process/gdb-remote/ProcessGDBRemote.cpp   |   7 +-
 .../Process/gdb-remote/ProcessGDBRemote.h |   2 +
 .../Plugins/Process/wasm/CMakeLists.txt   |  12 +
 .../Plugins/Process/wasm/ProcessWasm.cpp  | 291 ++
 .../source/Plugins/Process/wasm/ProcessWasm.h | 129 
 .../Plugins/Process/wasm/ThreadWasm.cpp   |  55 
 lldb/source/Plugins/Process/wasm/ThreadWasm.h |  44 +++
 .../Plugins/Process/wasm/UnwindWasm.cpp   |  76 +
 lldb/source/Plugins/Process/wasm/UnwindWasm.h |  55 
 .../Process/wasm/wasmRegisterContext.cpp  | 108 +++
 .../Process/wasm/wasmRegisterContext.h|  75 +
 16 files changed, 954 insertions(+), 2 deletions(-)
 create mode 100644 lldb/source/Plugins/Process/wasm/CMakeLists.txt
 create mode 100644 lldb/source/Plugins/Process/wasm/ProcessWasm.cpp
 create mode 100644 lldb/source/Plugins/Process/wasm/ProcessWasm.h
 create mode 100644 lldb/source/Plugins/Process/wasm/ThreadWasm.cpp
 create mode 100644 lldb/source/Plugins/Process/wasm/ThreadWasm.h
 create mode 100644 lldb/source/Plugins/Process/wasm/UnwindWasm.cpp
 create mode 100644 lldb/source/Plugins/Process/wasm/UnwindWasm.h
 create mode 100644 lldb/source/Plugins/Process/wasm/wasmRegisterContext.cpp
 create mode 100644 lldb/source/Plugins/Process/wasm/wasmRegisterContext.h

diff --git a/lldb/include/lldb/Target/Process.h 
b/lldb/include/lldb/Target/Process.h
index 24c599e044c78f..587ae085b479b7 100644
--- a/lldb/include/lldb/Target/Process.h
+++ b/lldb/include/lldb/Target/Process.h
@@ -1548,6 +1548,46 @@ class Process : public 
std::enable_shared_from_this,
   virtual size_t ReadMemory(lldb::addr_t vm_addr, void *buf, size_t size,
 Status &error);
 
+  /// Read of memory from a process.
+  ///
+  /// This function will read memory from the current process's address space
+  /// and remove any traps that may have been inserted into the memory.
+  ///
+  /// This overloads accepts an ExecutionContext as additional argument. By
+  /// default, it calls the previous overload without the ExecutionContext
+  /// argument, but it can be overridden by Process subclasses.
+  ///
+  /// \param[in] vm_addr
+  /// A virtual load address that indicates where to start reading
+  /// memory from.
+  ///
+  /// \param[out] buf
+  /// A byte buffer that is at least \a size bytes long that
+  /// will receive the memory bytes.
+  ///
+  /// \param[in] size
+  /// The number of bytes to read.
+  ///
+  /// \param[in] exe_ctx
+  ///The current execution context, if available.
+  ///
+  /// \param[out] error
+  /// An error that indicates the success or failure of this
+  /// operation. If error indicates success (error.Success()),
+  /// then the value returned can be trusted, otherwise zero
+  /// will be returned.
+  ///
+  /// \return
+  /// The number of bytes that were actually read into \a buf. If
+  /// the returned number is greater than zero, yet less than \a
+  /// size, then this function will get called again with \a
+  /// vm_addr, \a buf, and \a size updated appropriately. Zero is
+  /// returned in the case of an error.
+  virtual size_t ReadMemory(lldb::addr_t vm_addr, void *buf, size_t size,
+ExecutionContext *exe_ctx, Status &error) {
+return ReadMemory(vm_addr, buf, size, error);
+  }
+
   /// Read of memory from a process.
   ///
   /// This function has the same semantics of ReadMemory except that it
diff --git a/lldb/source/Core/Value.cpp b/lldb/source/Core/Value.cpp
index 995cc934c82044..47a5fdee773886 100644
--- a/lldb/source/Core/Value.cpp
+++ b/lldb/source/Core/Value.cpp
@@ -552,7 +552,7 @@ Status Value::GetValueAsData(ExecutionContext *exe_ctx, 
DataExtractor &data,
 
 if (process) {
   const size_t bytes_read =
-  process->ReadMemory(address, dst, byte_size, error);
+  process->ReadMemory(address, dst, byte_size, exe_ctx, error);
   if (bytes_read != byte_size)
 error.SetErrorStringWithFormat(
 "read memory from 0x%" PRIx64 " failed (%u of %u bytes read)",
diff --git a/lldb/source/Expression/DWARFExpression.cpp 
b/lldb/source/Expression/DWARFExpression.cpp
index fe4928d4f43a43..ca24611724dc7c 100644
--- a/lldb/source/Expression/DWARFExpression.cpp
+++ b/lldb/source/Expression/DWARFExpression.c

[Lldb-commits] [lldb] [llvm] Adapt DebugTypeODRUniquingTest to new SpecificationOf attribute. (PR #82666)

2024-02-22 Thread Volodymyr Sapsai via lldb-commits

https://github.com/vsapsai created 
https://github.com/llvm/llvm-project/pull/82666

None

error: too big or took too long to generate
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [llvm] Adapt DebugTypeODRUniquingTest to new SpecificationOf attribute. (PR #82666)

2024-02-22 Thread Volodymyr Sapsai via lldb-commits

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


[Lldb-commits] [lldb] [lldb][test] Remove vendored packages `unittest2` and `progress` (PR #82670)

2024-02-22 Thread via lldb-commits

llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: Jordan Rupprecht (rupprecht)


Changes

The `unittest2` package is unused since 
5b386158aacac4b41126983a5379d36ed413d0ea.

The `progress` package was only used internally by `unittest2`, so it can be 
deleted as well.

---

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


30 Files Affected:

- (removed) lldb/third_party/Python/module/progress/progress.py (-181) 
- (removed) lldb/third_party/Python/module/unittest2/unittest2/__init__.py 
(-78) 
- (removed) lldb/third_party/Python/module/unittest2/unittest2/__main__.py 
(-10) 
- (removed) lldb/third_party/Python/module/unittest2/unittest2/case.py (-1169) 
- (removed) lldb/third_party/Python/module/unittest2/unittest2/collector.py 
(-10) 
- (removed) lldb/third_party/Python/module/unittest2/unittest2/compatibility.py 
(-67) 
- (removed) lldb/third_party/Python/module/unittest2/unittest2/loader.py (-339) 
- (removed) lldb/third_party/Python/module/unittest2/unittest2/main.py (-257) 
- (removed) lldb/third_party/Python/module/unittest2/unittest2/result.py (-197) 
- (removed) lldb/third_party/Python/module/unittest2/unittest2/runner.py (-206) 
- (removed) lldb/third_party/Python/module/unittest2/unittest2/signals.py (-63) 
- (removed) lldb/third_party/Python/module/unittest2/unittest2/suite.py (-286) 
- (removed) lldb/third_party/Python/module/unittest2/unittest2/test/__init__.py 
(-1) 
- (removed) lldb/third_party/Python/module/unittest2/unittest2/test/dummy.py () 
- (removed) lldb/third_party/Python/module/unittest2/unittest2/test/support.py 
(-189) 
- (removed) 
lldb/third_party/Python/module/unittest2/unittest2/test/test_assertions.py 
(-269) 
- (removed) 
lldb/third_party/Python/module/unittest2/unittest2/test/test_break.py (-258) 
- (removed) 
lldb/third_party/Python/module/unittest2/unittest2/test/test_case.py (-1244) 
- (removed) 
lldb/third_party/Python/module/unittest2/unittest2/test/test_discovery.py 
(-392) 
- (removed) 
lldb/third_party/Python/module/unittest2/unittest2/test/test_functiontestcase.py
 (-148) 
- (removed) 
lldb/third_party/Python/module/unittest2/unittest2/test/test_loader.py (-1380) 
- (removed) 
lldb/third_party/Python/module/unittest2/unittest2/test/test_new_tests.py (-52) 
- (removed) 
lldb/third_party/Python/module/unittest2/unittest2/test/test_program.py (-251) 
- (removed) 
lldb/third_party/Python/module/unittest2/unittest2/test/test_result.py (-426) 
- (removed) 
lldb/third_party/Python/module/unittest2/unittest2/test/test_runner.py (-136) 
- (removed) 
lldb/third_party/Python/module/unittest2/unittest2/test/test_setups.py (-596) 
- (removed) 
lldb/third_party/Python/module/unittest2/unittest2/test/test_skipping.py (-154) 
- (removed) 
lldb/third_party/Python/module/unittest2/unittest2/test/test_suite.py (-363) 
- (removed) 
lldb/third_party/Python/module/unittest2/unittest2/test/test_unittest2_with.py 
(-148) 
- (removed) lldb/third_party/Python/module/unittest2/unittest2/util.py (-105) 


``diff
diff --git a/lldb/third_party/Python/module/progress/progress.py 
b/lldb/third_party/Python/module/progress/progress.py
deleted file mode 100644
index f844b9800c0192..00
--- a/lldb/third_party/Python/module/progress/progress.py
+++ /dev/null
@@ -1,181 +0,0 @@
-#!/usr/bin/env python
-
-import use_lldb_suite
-
-import sys
-import time
-
-
-class ProgressBar(object):
-"""ProgressBar class holds the options of the progress bar.
-The options are:
-start   State from which start the progress. For example, if start is
-5 and the end is 10, the progress of this state is 50%
-end State in which the progress has terminated.
-width   --
-fillString to use for "filled" used to represent the progress
-blank   String to use for "filled" used to represent remaining space.
-format  Format
-incremental
-"""
-light_block = chr(0x2591).encode("utf-8")
-solid_block = chr(0x2588).encode("utf-8")
-solid_right_arrow = chr(0x25BA).encode("utf-8")
-
-def __init__(self,
- start=0,
- end=10,
- width=12,
- fill=chr(0x25C9).encode("utf-8"),
- blank=chr(0x25CC).encode("utf-8"),
- marker=chr(0x25CE).encode("utf-8"),
- format='[%(fill)s%(marker)s%(blank)s] %(progress)s%%',
- incremental=True):
-super(ProgressBar, self).__init__()
-
-self.start = start
-self.end = end
-self.width = width
-self.fill = fill
-self.blank = blank
-self.marker = marker
-self.format = format
-self.incremental = incremental
-self.step = 100 / float(width)  # fix
-self.reset()
-
-def __add__(self, increment):
-increment = self._get_progress(increment)
-if 100 > self.progress + increment:
-self.pro

[Lldb-commits] [lldb] [lldb] [debugserver] fix qLaunchSuccess error, add QErrorStringInPacketSupported (PR #82593)

2024-02-22 Thread Alex Langford via lldb-commits


@@ -3944,15 +3955,22 @@ rnb_err_t RNBRemote::HandlePacket_v(const char *p) {
 // The order of these checks is important.  
 if (process_does_not_exist (pid_attaching_to)) {
   DNBLogError("Tried to attach to pid that doesn't exist");
-  std::string return_message = "E96;";
-  return_message += cstring_to_asciihex_string("no such process.");
+  std::string return_message = "E96";

bulbazord wrote:

I see this pattern in lots of places, is it worth abstracting out? Something 
like:

```
std::string CreateAttachError(const char *additional_context = nullptr) {
std::string message = "E96";
if (additional_context)
message += ";" + cstring_to_asciihex_string(additional_context);
return message;
}
```

What do you think?

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


[Lldb-commits] [lldb] [lldb] [debugserver] fix qLaunchSuccess error, add QErrorStringInPacketSupported (PR #82593)

2024-02-22 Thread Alex Langford via lldb-commits

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

LGTM, seems like a good fix. One question/suggestion.

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


[Lldb-commits] [lldb] [lldb] [debugserver] fix qLaunchSuccess error, add QErrorStringInPacketSupported (PR #82593)

2024-02-22 Thread Alex Langford via lldb-commits

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


[Lldb-commits] [lldb] [lldb][docs] Remove/update docs pointing to unittest2 (PR #82672)

2024-02-22 Thread Jordan Rupprecht via lldb-commits

https://github.com/rupprecht created 
https://github.com/llvm/llvm-project/pull/82672

None

>From 9b971b403448a6e7ff374558c5d963b7c5636ea1 Mon Sep 17 00:00:00 2001
From: Jordan Rupprecht 
Date: Thu, 22 Feb 2024 10:24:22 -0800
Subject: [PATCH] [lldb][docs] Remove/update docs pointing to unittest2

---
 lldb/docs/resources/test.rst   |  8 
 lldb/docs/testsuite/a-detailed-walkthrough.txt |  9 -
 .../Python/lldbsuite/test/README-TestSuite | 14 --
 3 files changed, 8 insertions(+), 23 deletions(-)

diff --git a/lldb/docs/resources/test.rst b/lldb/docs/resources/test.rst
index 52757864539ead..2b0e9010fe280a 100644
--- a/lldb/docs/resources/test.rst
+++ b/lldb/docs/resources/test.rst
@@ -17,8 +17,8 @@ The LLDB test suite consists of three different kinds of test:
   the output.
 * **API tests**: Integration tests that interact with the debugger through the
   SB API. These are written in Python and use LLDB's ``dotest.py`` testing
-  framework on top of Python's `unittest2
-  `_.
+  framework on top of Python's `unittest
+  `_.
 
 All three test suites use ``lit`` (`LLVM Integrated Tester
 `_ ) as the test driver. The test
@@ -94,7 +94,7 @@ programs from source, run them, and debug the processes.
 As mentioned before, ``dotest.py`` is LLDB's testing framework. The
 implementation is located under ``lldb/packages/Python/lldbsuite``. We have
 several extensions and custom test primitives on top of what's offered by
-`unittest2 `_. Those can be
+`unittest `_. Those can be
 found  in
 `lldbtest.py 
`_.
 
@@ -146,7 +146,7 @@ the test should be run or not.
 
 ::
 
-  @expectedFailure(checking_function_name)
+  @skipTestIfFn(checking_function_name)
 
 In addition to providing a lot more flexibility when it comes to writing the
 test, the API test also allow for much more complex scenarios when it comes to
diff --git a/lldb/docs/testsuite/a-detailed-walkthrough.txt 
b/lldb/docs/testsuite/a-detailed-walkthrough.txt
index 57c9dbce3d0ab6..8a7043786c1900 100644
--- a/lldb/docs/testsuite/a-detailed-walkthrough.txt
+++ b/lldb/docs/testsuite/a-detailed-walkthrough.txt
@@ -58,16 +58,15 @@ display their output.  For brevity, the '-t' output is not 
included here.
 Notice the 'expected failures=1' message at the end of the run.  This is 
because
 of a bug currently in lldb such that setting target.process.output-path to
 'stdout.txt' does not have any effect on the redirection of the standard output
-of the subsequent launched process.  We are using unittest2 (a backport of new
-unittest features for Python 2.4-2.6) to decorate (mark) the particular test
-method as such:
+of the subsequent launched process.  We are using unittest to decorate (mark)
+the particular test method as such:
 
-@unittest2.expectedFailure
+@unittest.expectedFailure
 # rdar://problem/8435794
 # settings set target.process.output-path does not seem to work
 def test_set_output_path(self):
 
-See http://pypi.python.org/pypi/unittest2 for more details.
+See http://docs.python.org/library/unittest.html for more details.
 
 Now let's look inside the test method:
 
diff --git a/lldb/packages/Python/lldbsuite/test/README-TestSuite 
b/lldb/packages/Python/lldbsuite/test/README-TestSuite
index f76e836ab777c0..388f94da0c409d 100644
--- a/lldb/packages/Python/lldbsuite/test/README-TestSuite
+++ b/lldb/packages/Python/lldbsuite/test/README-TestSuite
@@ -91,20 +91,6 @@ to the Python test suite under the current 'test' directory.
   Contains platform specific plugin to build binaries with dsym/dwarf debugging
   info.  Other platform specific functionalities may be added in the future.
 
-- unittest2 directory
-
-  Many new features were added to unittest in Python 2.7, including test
-  discovery. unittest2 allows you to use these features with earlier versions 
of
-  Python.
-
-  It currently has unittest2 0.5.1 from http://pypi.python.org/pypi/unittest2.
-  Version 0.5.1 of unittest2 has feature parity with unittest in Python 2.7
-  final. If you want to ensure that your tests run identically under unittest2
-  and unittest in Python 2.7 you should use unittest2 0.5.1. 
-
-  Later versions of unittest2 include changes in unittest made in Python 3.2 
and
-  onwards after the release of Python 2.7.
-
 - Profiling dotest.py runs
 
   I used the following command line thingy to do the profiling on a SnowLeopard

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


[Lldb-commits] [lldb] [lldb][docs] Remove/update docs pointing to unittest2 (PR #82672)

2024-02-22 Thread via lldb-commits

llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: Jordan Rupprecht (rupprecht)


Changes



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


3 Files Affected:

- (modified) lldb/docs/resources/test.rst (+4-4) 
- (modified) lldb/docs/testsuite/a-detailed-walkthrough.txt (+4-5) 
- (modified) lldb/packages/Python/lldbsuite/test/README-TestSuite (-14) 


``diff
diff --git a/lldb/docs/resources/test.rst b/lldb/docs/resources/test.rst
index 52757864539ead..2b0e9010fe280a 100644
--- a/lldb/docs/resources/test.rst
+++ b/lldb/docs/resources/test.rst
@@ -17,8 +17,8 @@ The LLDB test suite consists of three different kinds of test:
   the output.
 * **API tests**: Integration tests that interact with the debugger through the
   SB API. These are written in Python and use LLDB's ``dotest.py`` testing
-  framework on top of Python's `unittest2
-  `_.
+  framework on top of Python's `unittest
+  `_.
 
 All three test suites use ``lit`` (`LLVM Integrated Tester
 `_ ) as the test driver. The test
@@ -94,7 +94,7 @@ programs from source, run them, and debug the processes.
 As mentioned before, ``dotest.py`` is LLDB's testing framework. The
 implementation is located under ``lldb/packages/Python/lldbsuite``. We have
 several extensions and custom test primitives on top of what's offered by
-`unittest2 `_. Those can be
+`unittest `_. Those can be
 found  in
 `lldbtest.py 
`_.
 
@@ -146,7 +146,7 @@ the test should be run or not.
 
 ::
 
-  @expectedFailure(checking_function_name)
+  @skipTestIfFn(checking_function_name)
 
 In addition to providing a lot more flexibility when it comes to writing the
 test, the API test also allow for much more complex scenarios when it comes to
diff --git a/lldb/docs/testsuite/a-detailed-walkthrough.txt 
b/lldb/docs/testsuite/a-detailed-walkthrough.txt
index 57c9dbce3d0ab6..8a7043786c1900 100644
--- a/lldb/docs/testsuite/a-detailed-walkthrough.txt
+++ b/lldb/docs/testsuite/a-detailed-walkthrough.txt
@@ -58,16 +58,15 @@ display their output.  For brevity, the '-t' output is not 
included here.
 Notice the 'expected failures=1' message at the end of the run.  This is 
because
 of a bug currently in lldb such that setting target.process.output-path to
 'stdout.txt' does not have any effect on the redirection of the standard output
-of the subsequent launched process.  We are using unittest2 (a backport of new
-unittest features for Python 2.4-2.6) to decorate (mark) the particular test
-method as such:
+of the subsequent launched process.  We are using unittest to decorate (mark)
+the particular test method as such:
 
-@unittest2.expectedFailure
+@unittest.expectedFailure
 # rdar://problem/8435794
 # settings set target.process.output-path does not seem to work
 def test_set_output_path(self):
 
-See http://pypi.python.org/pypi/unittest2 for more details.
+See http://docs.python.org/library/unittest.html for more details.
 
 Now let's look inside the test method:
 
diff --git a/lldb/packages/Python/lldbsuite/test/README-TestSuite 
b/lldb/packages/Python/lldbsuite/test/README-TestSuite
index f76e836ab777c0..388f94da0c409d 100644
--- a/lldb/packages/Python/lldbsuite/test/README-TestSuite
+++ b/lldb/packages/Python/lldbsuite/test/README-TestSuite
@@ -91,20 +91,6 @@ to the Python test suite under the current 'test' directory.
   Contains platform specific plugin to build binaries with dsym/dwarf debugging
   info.  Other platform specific functionalities may be added in the future.
 
-- unittest2 directory
-
-  Many new features were added to unittest in Python 2.7, including test
-  discovery. unittest2 allows you to use these features with earlier versions 
of
-  Python.
-
-  It currently has unittest2 0.5.1 from http://pypi.python.org/pypi/unittest2.
-  Version 0.5.1 of unittest2 has feature parity with unittest in Python 2.7
-  final. If you want to ensure that your tests run identically under unittest2
-  and unittest in Python 2.7 you should use unittest2 0.5.1. 
-
-  Later versions of unittest2 include changes in unittest made in Python 3.2 
and
-  onwards after the release of Python 2.7.
-
 - Profiling dotest.py runs
 
   I used the following command line thingy to do the profiling on a SnowLeopard

``




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


[Lldb-commits] [lldb] [lldb][test] Remove vendored packages `unittest2` and `progress` (PR #82670)

2024-02-22 Thread Alex Langford via lldb-commits

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

🥳 

Please wait a little bit for others to look. Thanks for taking care of this!

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


[Lldb-commits] [lldb] [lldb][docs] Remove/update docs pointing to unittest2 (PR #82672)

2024-02-22 Thread Alex Langford via lldb-commits

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

LGTM!

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


[Lldb-commits] [lldb] [lldb] [debugserver] fix qLaunchSuccess error, add QErrorStringInPacketSupported (PR #82593)

2024-02-22 Thread Greg Clayton via lldb-commits


@@ -3944,15 +3955,22 @@ rnb_err_t RNBRemote::HandlePacket_v(const char *p) {
 // The order of these checks is important.  
 if (process_does_not_exist (pid_attaching_to)) {
   DNBLogError("Tried to attach to pid that doesn't exist");
-  std::string return_message = "E96;";
-  return_message += cstring_to_asciihex_string("no such process.");
+  std::string return_message = "E96";
+  if (m_enable_error_strings) {
+return_message += ";";
+return_message += cstring_to_asciihex_string("no such process.");
+  }
   return SendPacket(return_message);
 }
 if (process_is_already_being_debugged (pid_attaching_to)) {
   DNBLogError("Tried to attach to process already being debugged");
-  std::string return_message = "E96;";
-  return_message += cstring_to_asciihex_string("tried to attach to "
+  std::string return_message = "E96";
+  if (m_enable_error_strings) {
+return_message += ";";
+return_message +=
+cstring_to_asciihex_string("tried to attach to "
"process already being debugged");

clayborg wrote:

not ended with a '.' here, we pick how we want to do it and do it consistently

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


[Lldb-commits] [lldb] [lldb] [debugserver] fix qLaunchSuccess error, add QErrorStringInPacketSupported (PR #82593)

2024-02-22 Thread Greg Clayton via lldb-commits


@@ -3944,15 +3955,22 @@ rnb_err_t RNBRemote::HandlePacket_v(const char *p) {
 // The order of these checks is important.  
 if (process_does_not_exist (pid_attaching_to)) {
   DNBLogError("Tried to attach to pid that doesn't exist");
-  std::string return_message = "E96;";
-  return_message += cstring_to_asciihex_string("no such process.");
+  std::string return_message = "E96";

clayborg wrote:

It might be nice to have a member function of RNBRemote that creates an error 
packet:
```
std::string RNBRemote::CreateEXXPacket(const char *exx_str, const char 
*error_detail) {
  std::string packet(exx_str);
  if (m_enable_error_strings)
packet += ";" + cstring_to_asciihex_string(error_detail);
  return packet;
}
```
Similar to what Alex suggested above, but making a member function allows 
access to the `m_enable_error_strings` ivar.

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


[Lldb-commits] [lldb] [lldb] [debugserver] fix qLaunchSuccess error, add QErrorStringInPacketSupported (PR #82593)

2024-02-22 Thread Greg Clayton via lldb-commits


@@ -3969,30 +3987,43 @@ rnb_err_t RNBRemote::HandlePacket_v(const char *p) {
 process_username = pw->pw_name;
   }
   DNBLogError("Tried to attach to process with uid mismatch");
-  std::string return_message = "E96;";
-  std::string msg = "tried to attach to process as user '" 
-+ my_username + "' and process is running "
-"as user '" + process_username + "'";
-  return_message += cstring_to_asciihex_string(msg.c_str());
+  std::string return_message = "E96";
+  if (m_enable_error_strings) {
+return_message += ";";
+std::string msg = "tried to attach to process as user '" +
+  my_username +
+  "' and process is running "
+  "as user '" +
+  process_username + "'";
+return_message += cstring_to_asciihex_string(msg.c_str());
+  }
   return SendPacket(return_message);
 }
 if (!login_session_has_gui_access() && !developer_mode_enabled()) {
   DNBLogError("Developer mode is not enabled and this is a "
   "non-interactive session");
-  std::string return_message = "E96;";
-  return_message += cstring_to_asciihex_string("developer mode is "
+  std::string return_message = "E96";
+  if (m_enable_error_strings) {
+return_message += ";";
+return_message +=
+cstring_to_asciihex_string("developer mode is "
"not enabled on this machine "
"and this is a non-interactive "
"debug session.");

clayborg wrote:

this is ended with a '.', we pick how we want to do it and do it consistently

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


[Lldb-commits] [lldb] [lldb] [debugserver] fix qLaunchSuccess error, add QErrorStringInPacketSupported (PR #82593)

2024-02-22 Thread Greg Clayton via lldb-commits


@@ -3944,15 +3955,22 @@ rnb_err_t RNBRemote::HandlePacket_v(const char *p) {
 // The order of these checks is important.  
 if (process_does_not_exist (pid_attaching_to)) {
   DNBLogError("Tried to attach to pid that doesn't exist");
-  std::string return_message = "E96;";
-  return_message += cstring_to_asciihex_string("no such process.");
+  std::string return_message = "E96";
+  if (m_enable_error_strings) {
+return_message += ";";
+return_message += cstring_to_asciihex_string("no such process.");

clayborg wrote:

we probably want all error details to be ended with a '.' or all of them to not 
have a '.' at the end. See other error message below that don't have a '.' 
character

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


[Lldb-commits] [lldb] [lldb] [debugserver] fix qLaunchSuccess error, add QErrorStringInPacketSupported (PR #82593)

2024-02-22 Thread Greg Clayton via lldb-commits


@@ -3969,30 +3987,43 @@ rnb_err_t RNBRemote::HandlePacket_v(const char *p) {
 process_username = pw->pw_name;
   }
   DNBLogError("Tried to attach to process with uid mismatch");
-  std::string return_message = "E96;";
-  std::string msg = "tried to attach to process as user '" 
-+ my_username + "' and process is running "
-"as user '" + process_username + "'";
-  return_message += cstring_to_asciihex_string(msg.c_str());
+  std::string return_message = "E96";
+  if (m_enable_error_strings) {
+return_message += ";";
+std::string msg = "tried to attach to process as user '" +
+  my_username +
+  "' and process is running "
+  "as user '" +
+  process_username + "'";
+return_message += cstring_to_asciihex_string(msg.c_str());
+  }
   return SendPacket(return_message);
 }
 if (!login_session_has_gui_access() && !developer_mode_enabled()) {
   DNBLogError("Developer mode is not enabled and this is a "
   "non-interactive session");
-  std::string return_message = "E96;";
-  return_message += cstring_to_asciihex_string("developer mode is "
+  std::string return_message = "E96";
+  if (m_enable_error_strings) {
+return_message += ";";
+return_message +=
+cstring_to_asciihex_string("developer mode is "
"not enabled on this machine "
"and this is a non-interactive "
"debug session.");
+  }
   return SendPacket(return_message);
 }
 if (!login_session_has_gui_access()) {
   DNBLogError("This is a non-interactive session");
-  std::string return_message = "E96;";
-  return_message += cstring_to_asciihex_string("this is a "
+  std::string return_message = "E96";
+  if (m_enable_error_strings) {
+return_message += ";";
+return_message +=
+cstring_to_asciihex_string("this is a "
"non-interactive debug session, "
"cannot get permission to debug "
"processes.");

clayborg wrote:

this is ended with a '.', we pick how we want to do it and do it consistently

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


[Lldb-commits] [lldb] [lldb] [debugserver] fix qLaunchSuccess error, add QErrorStringInPacketSupported (PR #82593)

2024-02-22 Thread Greg Clayton via lldb-commits


@@ -3944,15 +3955,22 @@ rnb_err_t RNBRemote::HandlePacket_v(const char *p) {
 // The order of these checks is important.  
 if (process_does_not_exist (pid_attaching_to)) {
   DNBLogError("Tried to attach to pid that doesn't exist");
-  std::string return_message = "E96;";
-  return_message += cstring_to_asciihex_string("no such process.");
+  std::string return_message = "E96";

clayborg wrote:

Then this line becomes:
```
std::string return_message = CreateEXXPacket("E96", "no such process");
```

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


[Lldb-commits] [lldb] [lldb] [debugserver] fix qLaunchSuccess error, add QErrorStringInPacketSupported (PR #82593)

2024-02-22 Thread Greg Clayton via lldb-commits


@@ -3969,30 +3987,43 @@ rnb_err_t RNBRemote::HandlePacket_v(const char *p) {
 process_username = pw->pw_name;
   }
   DNBLogError("Tried to attach to process with uid mismatch");
-  std::string return_message = "E96;";
-  std::string msg = "tried to attach to process as user '" 
-+ my_username + "' and process is running "
-"as user '" + process_username + "'";
-  return_message += cstring_to_asciihex_string(msg.c_str());
+  std::string return_message = "E96";
+  if (m_enable_error_strings) {
+return_message += ";";
+std::string msg = "tried to attach to process as user '" +
+  my_username +
+  "' and process is running "
+  "as user '" +
+  process_username + "'";

clayborg wrote:

not ended with a '.' here, we pick how we want to do it and do it consistently

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


[Lldb-commits] [lldb] Re-land [lldb-dap] Add support for data breakpoint. (PR #81909)

2024-02-22 Thread Greg Clayton via lldb-commits

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


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


[Lldb-commits] [lldb] 2685e7e - [lldb][docs] Remove/update docs pointing to unittest2 (#82672)

2024-02-22 Thread via lldb-commits

Author: Jordan Rupprecht
Date: 2024-02-22T13:34:00-06:00
New Revision: 2685e7eadce08125672f0f6013145ae45b7a5ac3

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

LOG: [lldb][docs] Remove/update docs pointing to unittest2 (#82672)

Added: 


Modified: 
lldb/docs/resources/test.rst
lldb/docs/testsuite/a-detailed-walkthrough.txt
lldb/packages/Python/lldbsuite/test/README-TestSuite

Removed: 




diff  --git a/lldb/docs/resources/test.rst b/lldb/docs/resources/test.rst
index 52757864539ead..2b0e9010fe280a 100644
--- a/lldb/docs/resources/test.rst
+++ b/lldb/docs/resources/test.rst
@@ -17,8 +17,8 @@ The LLDB test suite consists of three 
diff erent kinds of test:
   the output.
 * **API tests**: Integration tests that interact with the debugger through the
   SB API. These are written in Python and use LLDB's ``dotest.py`` testing
-  framework on top of Python's `unittest2
-  `_.
+  framework on top of Python's `unittest
+  `_.
 
 All three test suites use ``lit`` (`LLVM Integrated Tester
 `_ ) as the test driver. The test
@@ -94,7 +94,7 @@ programs from source, run them, and debug the processes.
 As mentioned before, ``dotest.py`` is LLDB's testing framework. The
 implementation is located under ``lldb/packages/Python/lldbsuite``. We have
 several extensions and custom test primitives on top of what's offered by
-`unittest2 `_. Those can be
+`unittest `_. Those can be
 found  in
 `lldbtest.py 
`_.
 
@@ -146,7 +146,7 @@ the test should be run or not.
 
 ::
 
-  @expectedFailure(checking_function_name)
+  @skipTestIfFn(checking_function_name)
 
 In addition to providing a lot more flexibility when it comes to writing the
 test, the API test also allow for much more complex scenarios when it comes to

diff  --git a/lldb/docs/testsuite/a-detailed-walkthrough.txt 
b/lldb/docs/testsuite/a-detailed-walkthrough.txt
index 57c9dbce3d0ab6..8a7043786c1900 100644
--- a/lldb/docs/testsuite/a-detailed-walkthrough.txt
+++ b/lldb/docs/testsuite/a-detailed-walkthrough.txt
@@ -58,16 +58,15 @@ display their output.  For brevity, the '-t' output is not 
included here.
 Notice the 'expected failures=1' message at the end of the run.  This is 
because
 of a bug currently in lldb such that setting target.process.output-path to
 'stdout.txt' does not have any effect on the redirection of the standard output
-of the subsequent launched process.  We are using unittest2 (a backport of new
-unittest features for Python 2.4-2.6) to decorate (mark) the particular test
-method as such:
+of the subsequent launched process.  We are using unittest to decorate (mark)
+the particular test method as such:
 
-@unittest2.expectedFailure
+@unittest.expectedFailure
 # rdar://problem/8435794
 # settings set target.process.output-path does not seem to work
 def test_set_output_path(self):
 
-See http://pypi.python.org/pypi/unittest2 for more details.
+See http://docs.python.org/library/unittest.html for more details.
 
 Now let's look inside the test method:
 

diff  --git a/lldb/packages/Python/lldbsuite/test/README-TestSuite 
b/lldb/packages/Python/lldbsuite/test/README-TestSuite
index f76e836ab777c0..388f94da0c409d 100644
--- a/lldb/packages/Python/lldbsuite/test/README-TestSuite
+++ b/lldb/packages/Python/lldbsuite/test/README-TestSuite
@@ -91,20 +91,6 @@ to the Python test suite under the current 'test' directory.
   Contains platform specific plugin to build binaries with dsym/dwarf debugging
   info.  Other platform specific functionalities may be added in the future.
 
-- unittest2 directory
-
-  Many new features were added to unittest in Python 2.7, including test
-  discovery. unittest2 allows you to use these features with earlier versions 
of
-  Python.
-
-  It currently has unittest2 0.5.1 from http://pypi.python.org/pypi/unittest2.
-  Version 0.5.1 of unittest2 has feature parity with unittest in Python 2.7
-  final. If you want to ensure that your tests run identically under unittest2
-  and unittest in Python 2.7 you should use unittest2 0.5.1. 
-
-  Later versions of unittest2 include changes in unittest made in Python 3.2 
and
-  onwards after the release of Python 2.7.
-
 - Profiling dotest.py runs
 
   I used the following command line thingy to do the profiling on a SnowLeopard



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

[Lldb-commits] [lldb] [lldb][docs] Remove/update docs pointing to unittest2 (PR #82672)

2024-02-22 Thread Jordan Rupprecht via lldb-commits

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


[Lldb-commits] [lldb] [lldb][test] Remove vendored packages `unittest2` and `progress` (PR #82670)

2024-02-22 Thread Jordan Rupprecht via lldb-commits

rupprecht wrote:

> 🥳
> 
> Please wait a little bit for others to look. Thanks for taking care of this!

Yep, no problem! I'll give it a few days, and assume no comments == submit.

Next up: I'd like to remove `pexpect` (available on pip) and completely remove 
the lldb/third_party/ tree.

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


[Lldb-commits] [lldb] [lldb] Fix nullptr dereference on running x86 binary with x86-disabled llvm (PR #82603)

2024-02-22 Thread Jason Molenda via lldb-commits

jasonmolenda wrote:

I'm a little curious the use case that led to this failure.  We have a build of 
llvm/lldb without the X86 target, and we're using that lldb to debug an 
i386/x86_64 target (gdb connection or corefile)?  Because we can't use 
instruction emulation based unwinding, if we don't have eh_frame for every 
function, backtraces will go poorly.  Once we're off of the currently-executing 
stack frame, we can follow the frame pointer / pc spilled to the stack 
(assuming the code wasn't compiled omit-frame-pointer), but we won't be able to 
find any other spilled registers for printing variable contents in non-volatile 
/ caller spilled.   (and getting off of frame 0 without eh_frame is going to 
fail if we're in the prologue/epilogue on a thread)

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


[Lldb-commits] [lldb] [lldb] Fix nullptr dereference on running x86 binary with x86-disabled llvm (PR #82603)

2024-02-22 Thread Jason Molenda via lldb-commits

jasonmolenda wrote:

tbh most of lldb would work without the llvm target compiled in, but the 
disassembler and the Intel assembly scanning unwind plan creator both need it, 
and the latter is more important.

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


[Lldb-commits] [lldb] [lldb] Fix nullptr dereference on running x86 binary with x86-disabled llvm (PR #82603)

2024-02-22 Thread Jason Molenda via lldb-commits

jasonmolenda wrote:

Well, it would change source-level next and step too if we didn't have the 
disassembler.  When we're doing a source-level step/next, we look at the stream 
of instructions and when we have a block of non-branching instructions, we put 
a breakpoint after the block and continue, instead of instruction stepping.  We 
use the disassembler to detect instructions that can branch.  source level step 
and next would still work, but they would be a bit slower.

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


[Lldb-commits] [lldb] df6f756 - Re-land [lldb-dap] Add support for data breakpoint. (#81909)

2024-02-22 Thread via lldb-commits

Author: Zequan Wu
Date: 2024-02-22T16:11:40-05:00
New Revision: df6f756a19277d936ec83f7cebc2501327ac3add

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

LOG: Re-land [lldb-dap] Add support for data breakpoint. (#81909)

This implements functionality to handle DataBreakpointInfo request and
SetDataBreakpoints request.

Previous commit
https://github.com/llvm/llvm-project/commit/8c56e78ec531f0e2460213c20fff869b6b7add99
was reverted because setting 1 byte watchpoint failed in the new test on
ARM64. So, I changed the test to setting 4 byte watchpoint instead, and
hope this won't break it again. It also adds the fixes from
https://github.com/llvm/llvm-project/pull/81680.

Added: 
lldb/test/API/tools/lldb-dap/databreakpoint/Makefile
lldb/test/API/tools/lldb-dap/databreakpoint/TestDAP_setDataBreakpoints.py
lldb/test/API/tools/lldb-dap/databreakpoint/main.cpp
lldb/tools/lldb-dap/Watchpoint.cpp
lldb/tools/lldb-dap/Watchpoint.h

Modified: 
lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py
lldb/tools/lldb-dap/CMakeLists.txt
lldb/tools/lldb-dap/DAPForward.h
lldb/tools/lldb-dap/lldb-dap.cpp

Removed: 




diff  --git a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py 
b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py
index bb863bb8719176..27a76a652f4063 100644
--- a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py
+++ b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py
@@ -501,6 +501,18 @@ def get_local_variable_value(self, name, frameIndex=0, 
threadId=None):
 return variable["value"]
 return None
 
+def get_local_variable_child(self, name, child_name, frameIndex=0, 
threadId=None):
+local = self.get_local_variable(name, frameIndex, threadId)
+if local["variablesReference"] == 0:
+return None
+children = self.request_variables(local["variablesReference"])["body"][
+"variables"
+]
+for child in children:
+if child["name"] == child_name:
+return child
+return None
+
 def replay_packets(self, replay_file_path):
 f = open(replay_file_path, "r")
 mode = "invalid"
@@ -895,6 +907,41 @@ def request_setFunctionBreakpoints(self, names, 
condition=None, hitCondition=Non
 }
 return self.send_recv(command_dict)
 
+def request_dataBreakpointInfo(
+self, variablesReference, name, frameIndex=0, threadId=None
+):
+stackFrame = self.get_stackFrame(frameIndex=frameIndex, 
threadId=threadId)
+if stackFrame is None:
+return []
+args_dict = {
+"variablesReference": variablesReference,
+"name": name,
+"frameId": stackFrame["id"],
+}
+command_dict = {
+"command": "dataBreakpointInfo",
+"type": "request",
+"arguments": args_dict,
+}
+return self.send_recv(command_dict)
+
+def request_setDataBreakpoint(self, dataBreakpoints):
+"""dataBreakpoints is a list of dictionary with following fields:
+{
+dataId: (address in hex)/(size in bytes)
+accessType: read/write/readWrite
+[condition]: string
+[hitCondition]: string
+}
+"""
+args_dict = {"breakpoints": dataBreakpoints}
+command_dict = {
+"command": "setDataBreakpoints",
+"type": "request",
+"arguments": args_dict,
+}
+return self.send_recv(command_dict)
+
 def request_compileUnits(self, moduleId):
 args_dict = {"moduleId": moduleId}
 command_dict = {

diff  --git a/lldb/test/API/tools/lldb-dap/databreakpoint/Makefile 
b/lldb/test/API/tools/lldb-dap/databreakpoint/Makefile
new file mode 100644
index 00..8b20bcb050
--- /dev/null
+++ b/lldb/test/API/tools/lldb-dap/databreakpoint/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+
+include Makefile.rules

diff  --git 
a/lldb/test/API/tools/lldb-dap/databreakpoint/TestDAP_setDataBreakpoints.py 
b/lldb/test/API/tools/lldb-dap/databreakpoint/TestDAP_setDataBreakpoints.py
new file mode 100644
index 00..17cdad89aa6d10
--- /dev/null
+++ b/lldb/test/API/tools/lldb-dap/databreakpoint/TestDAP_setDataBreakpoints.py
@@ -0,0 +1,131 @@
+"""
+Test lldb-dap dataBreakpointInfo and setDataBreakpoints requests
+"""
+
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+import lldbdap_testcase
+
+
+class TestDAP_setDataBreakpoints(lldbdap_testcase.DAPTestCaseBase):
+def setUp(self):
+lldbdap_testcase.DAPTestCaseBase.setUp(self)
+self.accessTypes = ["read", "write", "readWrite"]
+

[Lldb-commits] [lldb] Re-land [lldb-dap] Add support for data breakpoint. (PR #81909)

2024-02-22 Thread Zequan Wu via lldb-commits

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


[Lldb-commits] [lldb] [lldb-dap] Followup fixs for data breakpoints (PR #81680)

2024-02-22 Thread Zequan Wu via lldb-commits

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


[Lldb-commits] [lldb] [lldb] [debugserver] fix qLaunchSuccess error, add QErrorStringInPacketSupported (PR #82593)

2024-02-22 Thread Jason Molenda via lldb-commits

jasonmolenda wrote:

`CreateEXXPacket("E96", "no such process");` that's a good idea, I'll rewrite 
to use a helper method like that.

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


[Lldb-commits] [lldb] [lldb] Correctly annotate threads at a bp site as hitting it (PR #82709)

2024-02-22 Thread Jason Molenda via lldb-commits

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

This is next in my series of "fix the racey tests that fail on greendragon" 
addressing the failure of TestConcurrentManyBreakpoints.py where we set a 
breakpoint in a function that 100 threads execute, and we check that we hit the 
breakpoint 100 times.  But sometimes it is only hit 99 times, and the test 
fails.

When we hit a software breakpoint, the pc value for the thread is the address 
of the breakpoint instruction - as if it had not been hit yet.  And because a 
user might ADD a breakpoint for the current pc from the commandline, when we go 
to resume execution, any thread that is sitting at a breakpoint site will be 
silently advanced past the breakpoint instruction (disable bp, instruction step 
that thread, re-enable bp) before resuming.

What this test is exposing is that there is another corner case, a thread that 
is sitting at a breakpoint site but has not yet executed the breakpoint 
instruction.  The thread will have no stop reason, no mach exception, so it 
will not be recorded as having hit the breakpoint (because it hasn't yet).  But 
when we resume execution, because it is sitting at a breakpoint site, we 
advance past it and miss the breakpoint hit.

In 2016 Abhishek Aggarwal handled a similar issue with a patch in 
`ProcessGDBRemote::SetThreadStopInfo()`, adding a breakpoint StopInfo for a 
thread sitting at a breakpoint site that has no stop reason. debugserver's 
`jThreadsInfo` would not correctly execute Abhishek's code though because it 
would respond with `"reason":"none"` for a thread with no stop reason, and 
`SetThreadStopInfo()` expected an empty reason here.  The first part of my 
patch is to clear the `reason` if it is `"none"` so we flow through the code 
correctly.

On Darwin, though, our stop reply packet (Txx...) includes the `threads`, 
`thread-pcs`, and `jstopinfo` keys, which give us the tids for all current 
threads, the pc values for those threads, and `jstopinfo` has a JSON dictionary 
with the mach exceptions for all threads that have a mach exception.  In
`ProcessGDBRemote::CalculateThreadStopInfo()` we set the StopInfo for each 
thread for a private stop and if we have `jstopinfo` it is the source of all 
the StopInfos.  I have to add the same logic here, to give the thread a 
breakpoint StopInfo even though it hasn't executed the breakpoint yet.  In this 
case we are very early in thread construction and I only have the information 
in the Txx stop reply packet -- tids, pcs, and jstopinfo, so I can't use the 
normal general mechanisms of going through the RegisterContext to get the pc, 
it's a bit different.

If I hack debugserver to not issue `jstopinfo`, `CalculateThreadStopInfo` will 
fall back to sending `qThreadStopInfo` for each thread and going through 
`ProcessGDBRemote::SetThreadStopInfo()` to set the stop infos (and with the 
`reason:none` fix, use Abhishek's code).

rdar://110549165

>From 171613d26338919e40584656a7f88899ba6cc5ca Mon Sep 17 00:00:00 2001
From: Jason Molenda 
Date: Thu, 22 Feb 2024 15:35:31 -0800
Subject: [PATCH] [lldb] Correctly annotate threads at a bp site as hitting it

This is next in my series of "fix the racey tests that fail on
greendragon" addressing the failure of TestConcurrentManyBreakpoints.py
where we set a breakpoint in a function that 100 threads execute,
and we check that we hit the breakpoint 100 times.  But sometimes
it is only hit 99 times, and the test fails.

When we hit a software breakpoint, the pc value for the thread is
the address of the breakpoint instruction - as if it had not been
hit yet.  And because a user might ADD a breakpoint for the current
pc from the commandline, when we go to resume execution, any thread
that is sitting at a breakpoint site will be silently advanced past
the breakpoint instruction (disable bp, instruction step that thread,
re-enable bp) before resuming.

What this test is exposing is that there is another corner case, a
thread that is sitting at a breakpoint site but has not yet executed
the breakpoint instruction.  The thread will have no stop reason,
no mach exception, so it will not be recorded as having hit the
breakpoint (because it hasn't yet).  But when we resume execution,
because it is sitting at a breakpoint site, we advance past it and
miss the breakpoint hit.

In 2016 Abhishek Aggarwal handled a similar issue with a patch in
`ProcessGDBRemote::SetThreadStopInfo()`, adding a breakpoint StopInfo
for a thread sitting at a breakpoint site that has no stop reason.
debugserver's `jThreadsInfo` would not correctly execute Abhishek's
code though because it would respond with `"reason":"none"` for a
thread with no stop reason, and `SetThreadStopInfo()` expected an
empty reason here.  The first part of my patch is to clear the
`reason` if it is `"none"` so we flow through the code correctly.

On Darwin, though, our stop reply packet (Txx...) includes the
`threads`, `thread-pcs`, and `j

[Lldb-commits] [lldb] [lldb] Correctly annotate threads at a bp site as hitting it (PR #82709)

2024-02-22 Thread via lldb-commits

llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: Jason Molenda (jasonmolenda)


Changes

This is next in my series of "fix the racey tests that fail on greendragon" 
addressing the failure of TestConcurrentManyBreakpoints.py where we set a 
breakpoint in a function that 100 threads execute, and we check that we hit the 
breakpoint 100 times.  But sometimes it is only hit 99 times, and the test 
fails.

When we hit a software breakpoint, the pc value for the thread is the address 
of the breakpoint instruction - as if it had not been hit yet.  And because a 
user might ADD a breakpoint for the current pc from the commandline, when we go 
to resume execution, any thread that is sitting at a breakpoint site will be 
silently advanced past the breakpoint instruction (disable bp, instruction step 
that thread, re-enable bp) before resuming.

What this test is exposing is that there is another corner case, a thread that 
is sitting at a breakpoint site but has not yet executed the breakpoint 
instruction.  The thread will have no stop reason, no mach exception, so it 
will not be recorded as having hit the breakpoint (because it hasn't yet).  But 
when we resume execution, because it is sitting at a breakpoint site, we 
advance past it and miss the breakpoint hit.

In 2016 Abhishek Aggarwal handled a similar issue with a patch in 
`ProcessGDBRemote::SetThreadStopInfo()`, adding a breakpoint StopInfo for a 
thread sitting at a breakpoint site that has no stop reason. debugserver's 
`jThreadsInfo` would not correctly execute Abhishek's code though because it 
would respond with `"reason":"none"` for a thread with no stop reason, and 
`SetThreadStopInfo()` expected an empty reason here.  The first part of my 
patch is to clear the `reason` if it is `"none"` so we flow through the code 
correctly.

On Darwin, though, our stop reply packet (Txx...) includes the `threads`, 
`thread-pcs`, and `jstopinfo` keys, which give us the tids for all current 
threads, the pc values for those threads, and `jstopinfo` has a JSON dictionary 
with the mach exceptions for all threads that have a mach exception.  In
`ProcessGDBRemote::CalculateThreadStopInfo()` we set the StopInfo for each 
thread for a private stop and if we have `jstopinfo` it is the source of all 
the StopInfos.  I have to add the same logic here, to give the thread a 
breakpoint StopInfo even though it hasn't executed the breakpoint yet.  In this 
case we are very early in thread construction and I only have the information 
in the Txx stop reply packet -- tids, pcs, and jstopinfo, so I can't use the 
normal general mechanisms of going through the RegisterContext to get the pc, 
it's a bit different.

If I hack debugserver to not issue `jstopinfo`, `CalculateThreadStopInfo` will 
fall back to sending `qThreadStopInfo` for each thread and going through 
`ProcessGDBRemote::SetThreadStopInfo()` to set the stop infos (and with the 
`reason:none` fix, use Abhishek's code).

rdar://110549165

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


2 Files Affected:

- (modified) lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp 
(+27-6) 
- (modified) lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h (+1-1) 


``diff
diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp 
b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
index 629b191f3117aa..eabbc8ad433212 100644
--- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -1600,6 +1600,26 @@ bool 
ProcessGDBRemote::CalculateThreadStopInfo(ThreadGDBRemote *thread) {
 // has no stop reason.
 thread->GetRegisterContext()->InvalidateIfNeeded(true);
 if (!GetThreadStopInfoFromJSON(thread, m_jstopinfo_sp)) {
+  // If a thread is stopped at a breakpoint site, set that as the stop
+  // reason even if it hasn't executed the breakpoint instruction yet.
+  // We will silently step over the breakpoint when we resume execution
+  // and miss the fact that this thread hit the breakpoint.
+  const size_t num_thread_ids = m_thread_ids.size();
+  for (size_t i = 0; i < num_thread_ids; i++) {
+if (m_thread_ids[i] == thread->GetID() && m_thread_pcs.size() > i) {
+  addr_t pc = m_thread_pcs[i];
+  lldb::BreakpointSiteSP bp_site_sp =
+  thread->GetProcess()->GetBreakpointSiteList().FindByAddress(pc);
+  if (bp_site_sp) {
+if (bp_site_sp->ValidForThisThread(*thread)) {
+  thread->SetStopInfo(
+  StopInfo::CreateStopReasonWithBreakpointSiteID(
+  *thread, bp_site_sp->GetID()));
+  return true;
+}
+  }
+}
+  }
   thread->SetStopInfo(StopInfoSP());
 }
 return true;
@@ -1630,7 +1650,7 @@ void ProcessGDBRemote::ParseExpeditedRegisters(
 
 ThreadSP ProcessGDBRemote::SetThreadStopInfo(
 lldb::tid_t tid,

[Lldb-commits] [lldb] [lldb] Correctly annotate threads at a bp site as hitting it (PR #82709)

2024-02-22 Thread Jonas Devlieghere via lldb-commits

https://github.com/JDevlieghere commented:

Makes sense to me, but I'll defer to @jimingham as he's the expert. 

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


[Lldb-commits] [lldb] [lldb][test] Remove vendored packages `unittest2` and `progress` (PR #82670)

2024-02-22 Thread Jonas Devlieghere via lldb-commits

JDevlieghere wrote:

Awesome, thank you making this happen! 

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


[Lldb-commits] [lldb] [lldb] Fix nullptr dereference on running x86 binary with x86-disabled llvm (PR #82603)

2024-02-22 Thread Jonas Devlieghere via lldb-commits

JDevlieghere wrote:

Also this is plugin code, it's probably more appropriate to disable the whole 
plugin if the necessary backend isn't there?

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


[Lldb-commits] [lldb] Improve and modernize logging for Process::CompleteAttach() (PR #82717)

2024-02-22 Thread Adrian Prantl via lldb-commits

https://github.com/adrian-prantl created 
https://github.com/llvm/llvm-project/pull/82717

Target::SetArchitecture() does not necessarily set the triple that is being 
passed in, and will unconditionally log the real architecture to the log 
channel. By flipping the order between the log outputs, the resulting combined 
log makes a lot more sense to read.

>From 399a3abaacc721b1a9f0a4c39ceab687ff2963e5 Mon Sep 17 00:00:00 2001
From: Adrian Prantl 
Date: Thu, 22 Feb 2024 17:03:32 -0800
Subject: [PATCH] Improve and modernize logging for Process::CompleteAttach()

Target::SetArchitecture() does not necessarily set the triple that is
being passed in, and will unconditionally log the real architecture to
the log channel. By flipping the order between the log outputs, the
resulting combined log makes a lot more sense to read.
---
 lldb/source/Target/Process.cpp | 11 ---
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp
index 23a8a66645c02d..137795cb8cec9e 100644
--- a/lldb/source/Target/Process.cpp
+++ b/lldb/source/Target/Process.cpp
@@ -2937,14 +2937,11 @@ void Process::CompleteAttach() {
   DidAttach(process_arch);
 
   if (process_arch.IsValid()) {
+LLDB_LOG(log,
+ "Process::{0} replacing process architecture with DidAttach() "
+ "architecture: \"{1}\"",
+ __FUNCTION__, process_arch.GetTriple().getTriple());
 GetTarget().SetArchitecture(process_arch);
-if (log) {
-  const char *triple_str = process_arch.GetTriple().getTriple().c_str();
-  LLDB_LOGF(log,
-"Process::%s replacing process architecture with DidAttach() "
-"architecture: %s",
-__FUNCTION__, triple_str ? triple_str : "");
-}
   }
 
   // We just attached.  If we have a platform, ask it for the process

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


[Lldb-commits] [lldb] Improve and modernize logging for Process::CompleteAttach() (PR #82717)

2024-02-22 Thread via lldb-commits

llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: Adrian Prantl (adrian-prantl)


Changes

Target::SetArchitecture() does not necessarily set the triple that is being 
passed in, and will unconditionally log the real architecture to the log 
channel. By flipping the order between the log outputs, the resulting combined 
log makes a lot more sense to read.

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


1 Files Affected:

- (modified) lldb/source/Target/Process.cpp (+4-7) 


``diff
diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp
index 23a8a66645c02d..137795cb8cec9e 100644
--- a/lldb/source/Target/Process.cpp
+++ b/lldb/source/Target/Process.cpp
@@ -2937,14 +2937,11 @@ void Process::CompleteAttach() {
   DidAttach(process_arch);
 
   if (process_arch.IsValid()) {
+LLDB_LOG(log,
+ "Process::{0} replacing process architecture with DidAttach() "
+ "architecture: \"{1}\"",
+ __FUNCTION__, process_arch.GetTriple().getTriple());
 GetTarget().SetArchitecture(process_arch);
-if (log) {
-  const char *triple_str = process_arch.GetTriple().getTriple().c_str();
-  LLDB_LOGF(log,
-"Process::%s replacing process architecture with DidAttach() "
-"architecture: %s",
-__FUNCTION__, triple_str ? triple_str : "");
-}
   }
 
   // We just attached.  If we have a platform, ask it for the process

``




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


[Lldb-commits] [lldb] [lldb] Fix nullptr dereference on running x86 binary with x86-disabled llvm (PR #82603)

2024-02-22 Thread Jason Molenda via lldb-commits

jasonmolenda wrote:

tbh I have no problems with the patch, but I think it's fixing something that I 
think should be reconsidered altogether, I'm interested to hear more about what 
the use case looks like that led to this being a problem.

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


[Lldb-commits] [lldb] [lldb] Correctly annotate threads at a bp site as hitting it (PR #82709)

2024-02-22 Thread Jason Molenda via lldb-commits

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


[Lldb-commits] [lldb] Improve and modernize logging for Process::CompleteAttach() (PR #82717)

2024-02-22 Thread Jonas Devlieghere via lldb-commits

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

LGTM

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


[Lldb-commits] [lldb] [lldb] Fix term-width setting (PR #82736)

2024-02-22 Thread Jonas Devlieghere via lldb-commits

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

I noticed that the term-width setting would always report its default value 
(80) despite the driver correctly setting the value with 
SBDebugger::SetTerminalWidth.

  (lldb) settings show term-width
  term-width (int) = 80

The issue is that the setting was defined as a SInt64 instead of a UInt64 while 
the getter returned an unsigned value. There's no reason the terminal width 
should be a signed value. My best guess it that it was using SInt64 because 
UInt64 didn't support min and max values. I fixed that and correct the type and 
now lldb reports the correct terminal width:

  (lldb) settings show term-width
  term-width (unsigned) = 189

>From 011ea5daf9f18a69cb92702aa2453492b84f5c85 Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere 
Date: Thu, 22 Feb 2024 21:01:06 -0800
Subject: [PATCH] [lldb] Fix term-width setting

I noticed that the term-width setting would always report its default
value (80) despite the driver correctly setting the value with
SBDebugger::SetTerminalWidth.

  (lldb) settings show term-width
  term-width (int) = 80

The issue is that the setting was defined as a SInt64 instead of a
UInt64 while the getter returned an unsigned value. There's no reason
the terminal width should be a signed value. My best guess it that it
was using SInt64 because UInt64 didn't support min and max values. I
fixed that and correct the type and now lldb reports the correct
terminal width:

  (lldb) settings show term-width
  term-width (unsigned) = 189
---
 .../lldb/Interpreter/OptionValueUInt64.h  | 26 +--
 lldb/source/Core/CoreProperties.td|  2 +-
 lldb/source/Core/Debugger.cpp |  4 +--
 .../API/commands/settings/TestSettings.py | 15 ---
 .../TestTrimmedProgressReporting.py   |  3 ++-
 5 files changed, 40 insertions(+), 10 deletions(-)

diff --git a/lldb/include/lldb/Interpreter/OptionValueUInt64.h 
b/lldb/include/lldb/Interpreter/OptionValueUInt64.h
index 30c27bf73d99c6..b20fc880869c65 100644
--- a/lldb/include/lldb/Interpreter/OptionValueUInt64.h
+++ b/lldb/include/lldb/Interpreter/OptionValueUInt64.h
@@ -64,13 +64,35 @@ class OptionValueUInt64 : public 
Cloneable {
 
   uint64_t GetDefaultValue() const { return m_default_value; }
 
-  void SetCurrentValue(uint64_t value) { m_current_value = value; }
+  bool SetCurrentValue(uint64_t value) {
+if (value >= m_min_value && value <= m_max_value) {
+  m_current_value = value;
+  return true;
+}
+return false;
+  }
+
+  bool SetDefaultValue(uint64_t value) {
+if (value >= m_min_value && value <= m_max_value) {
+  m_default_value = value;
+  return true;
+}
+return false;
+  }
+
+  void SetMinimumValue(int64_t v) { m_min_value = v; }
+
+  uint64_t GetMinimumValue() const { return m_min_value; }
+
+  void SetMaximumValue(int64_t v) { m_max_value = v; }
 
-  void SetDefaultValue(uint64_t value) { m_default_value = value; }
+  int64_t GetMaximumValue() const { return m_max_value; }
 
 protected:
   uint64_t m_current_value = 0;
   uint64_t m_default_value = 0;
+  uint64_t m_min_value = INT64_MIN;
+  uint64_t m_max_value = INT64_MAX;
 };
 
 } // namespace lldb_private
diff --git a/lldb/source/Core/CoreProperties.td 
b/lldb/source/Core/CoreProperties.td
index 4cfff805688c56..a6cb951187a040 100644
--- a/lldb/source/Core/CoreProperties.td
+++ b/lldb/source/Core/CoreProperties.td
@@ -132,7 +132,7 @@ let Definition = "debugger" in {
 Global,
 DefaultStringValue<"${ansi.normal}">,
 Desc<"When displaying the line marker in a color-enabled terminal, use the 
ANSI terminal code specified in this format immediately after the line to be 
marked.">;
-  def TerminalWidth: Property<"term-width", "SInt64">,
+  def TerminalWidth: Property<"term-width", "UInt64">,
 Global,
 DefaultUnsignedValue<80>,
 Desc<"The maximum number of columns to use for displaying text.">;
diff --git a/lldb/source/Core/Debugger.cpp b/lldb/source/Core/Debugger.cpp
index 97311b4716ac2f..bb81110ae35a5d 100644
--- a/lldb/source/Core/Debugger.cpp
+++ b/lldb/source/Core/Debugger.cpp
@@ -886,8 +886,8 @@ Debugger::Debugger(lldb::LogOutputCallback log_callback, 
void *baton)
   }
   assert(m_dummy_target_sp.get() && "Couldn't construct dummy target?");
 
-  OptionValueSInt64 *term_width =
-  m_collection_sp->GetPropertyAtIndexAsOptionValueSInt64(
+  OptionValueUInt64 *term_width =
+  m_collection_sp->GetPropertyAtIndexAsOptionValueUInt64(
   ePropertyTerminalWidth);
   term_width->SetMinimumValue(10);
   term_width->SetMaximumValue(1024);
diff --git a/lldb/test/API/commands/settings/TestSettings.py 
b/lldb/test/API/commands/settings/TestSettings.py
index a2d845493d1df9..104a9f09788c37 100644
--- a/lldb/test/API/commands/settings/TestSettings.py
+++ b/lldb/test/API/commands/settings/TestSettings.py
@@ -2,7 +2,6 @@
 Test lldb settings command.
 """
 
-
 import json
 import os

[Lldb-commits] [lldb] [lldb] Fix term-width setting (PR #82736)

2024-02-22 Thread via lldb-commits

llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: Jonas Devlieghere (JDevlieghere)


Changes

I noticed that the term-width setting would always report its default value 
(80) despite the driver correctly setting the value with 
SBDebugger::SetTerminalWidth.

  (lldb) settings show term-width
  term-width (int) = 80

The issue is that the setting was defined as a SInt64 instead of a UInt64 while 
the getter returned an unsigned value. There's no reason the terminal width 
should be a signed value. My best guess it that it was using SInt64 because 
UInt64 didn't support min and max values. I fixed that and correct the type and 
now lldb reports the correct terminal width:

  (lldb) settings show term-width
  term-width (unsigned) = 189

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


5 Files Affected:

- (modified) lldb/include/lldb/Interpreter/OptionValueUInt64.h (+24-2) 
- (modified) lldb/source/Core/CoreProperties.td (+1-1) 
- (modified) lldb/source/Core/Debugger.cpp (+2-2) 
- (modified) lldb/test/API/commands/settings/TestSettings.py (+11-4) 
- (modified) 
lldb/test/API/functionalities/progress_reporting/TestTrimmedProgressReporting.py
 (+2-1) 


``diff
diff --git a/lldb/include/lldb/Interpreter/OptionValueUInt64.h 
b/lldb/include/lldb/Interpreter/OptionValueUInt64.h
index 30c27bf73d99c6..b20fc880869c65 100644
--- a/lldb/include/lldb/Interpreter/OptionValueUInt64.h
+++ b/lldb/include/lldb/Interpreter/OptionValueUInt64.h
@@ -64,13 +64,35 @@ class OptionValueUInt64 : public 
Cloneable {
 
   uint64_t GetDefaultValue() const { return m_default_value; }
 
-  void SetCurrentValue(uint64_t value) { m_current_value = value; }
+  bool SetCurrentValue(uint64_t value) {
+if (value >= m_min_value && value <= m_max_value) {
+  m_current_value = value;
+  return true;
+}
+return false;
+  }
+
+  bool SetDefaultValue(uint64_t value) {
+if (value >= m_min_value && value <= m_max_value) {
+  m_default_value = value;
+  return true;
+}
+return false;
+  }
+
+  void SetMinimumValue(int64_t v) { m_min_value = v; }
+
+  uint64_t GetMinimumValue() const { return m_min_value; }
+
+  void SetMaximumValue(int64_t v) { m_max_value = v; }
 
-  void SetDefaultValue(uint64_t value) { m_default_value = value; }
+  int64_t GetMaximumValue() const { return m_max_value; }
 
 protected:
   uint64_t m_current_value = 0;
   uint64_t m_default_value = 0;
+  uint64_t m_min_value = INT64_MIN;
+  uint64_t m_max_value = INT64_MAX;
 };
 
 } // namespace lldb_private
diff --git a/lldb/source/Core/CoreProperties.td 
b/lldb/source/Core/CoreProperties.td
index 4cfff805688c56..a6cb951187a040 100644
--- a/lldb/source/Core/CoreProperties.td
+++ b/lldb/source/Core/CoreProperties.td
@@ -132,7 +132,7 @@ let Definition = "debugger" in {
 Global,
 DefaultStringValue<"${ansi.normal}">,
 Desc<"When displaying the line marker in a color-enabled terminal, use the 
ANSI terminal code specified in this format immediately after the line to be 
marked.">;
-  def TerminalWidth: Property<"term-width", "SInt64">,
+  def TerminalWidth: Property<"term-width", "UInt64">,
 Global,
 DefaultUnsignedValue<80>,
 Desc<"The maximum number of columns to use for displaying text.">;
diff --git a/lldb/source/Core/Debugger.cpp b/lldb/source/Core/Debugger.cpp
index 97311b4716ac2f..bb81110ae35a5d 100644
--- a/lldb/source/Core/Debugger.cpp
+++ b/lldb/source/Core/Debugger.cpp
@@ -886,8 +886,8 @@ Debugger::Debugger(lldb::LogOutputCallback log_callback, 
void *baton)
   }
   assert(m_dummy_target_sp.get() && "Couldn't construct dummy target?");
 
-  OptionValueSInt64 *term_width =
-  m_collection_sp->GetPropertyAtIndexAsOptionValueSInt64(
+  OptionValueUInt64 *term_width =
+  m_collection_sp->GetPropertyAtIndexAsOptionValueUInt64(
   ePropertyTerminalWidth);
   term_width->SetMinimumValue(10);
   term_width->SetMaximumValue(1024);
diff --git a/lldb/test/API/commands/settings/TestSettings.py 
b/lldb/test/API/commands/settings/TestSettings.py
index a2d845493d1df9..104a9f09788c37 100644
--- a/lldb/test/API/commands/settings/TestSettings.py
+++ b/lldb/test/API/commands/settings/TestSettings.py
@@ -2,7 +2,6 @@
 Test lldb settings command.
 """
 
-
 import json
 import os
 import re
@@ -151,14 +150,22 @@ def test_set_term_width(self):
 self.expect(
 "settings show term-width",
 SETTING_MSG("term-width"),
-startstr="term-width (int) = 70",
+startstr="term-width (unsigned) = 70",
 )
 
 # The overall display should also reflect the new setting.
 self.expect(
 "settings show",
 SETTING_MSG("term-width"),
-substrs=["term-width (int) = 70"],
+substrs=["term-width (unsigned) = 70"],
+)
+
+self.dbg.SetTerminalWidth(60)
+
+self.expect(
+"settings show",
+SETTING_MSG("term-width"),
+substrs=["term-width (

[Lldb-commits] [lldb] [lldb] Fix term-width setting (PR #82736)

2024-02-22 Thread Jonas Devlieghere via lldb-commits

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

>From c6c2edf66adbe72c959925da886043cd77bfe528 Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere 
Date: Thu, 22 Feb 2024 21:01:06 -0800
Subject: [PATCH] [lldb] Fix term-width setting

I noticed that the term-width setting would always report its default
value (80) despite the driver correctly setting the value with
SBDebugger::SetTerminalWidth.

  (lldb) settings show term-width
  term-width (int) = 80

The issue is that the setting was defined as a SInt64 instead of a
UInt64 while the getter returned an unsigned value. There's no reason
the terminal width should be a signed value. My best guess it that it
was using SInt64 because UInt64 didn't support min and max values. I
fixed that and correct the type and now lldb reports the correct
terminal width:

  (lldb) settings show term-width
  term-width (unsigned) = 189
---
 .../lldb/Interpreter/OptionValueSInt64.h  |  4 +--
 .../lldb/Interpreter/OptionValueUInt64.h  | 26 +--
 lldb/source/Core/CoreProperties.td|  2 +-
 lldb/source/Core/Debugger.cpp |  4 +--
 .../API/commands/settings/TestSettings.py | 15 ---
 .../TestTrimmedProgressReporting.py   |  3 ++-
 6 files changed, 42 insertions(+), 12 deletions(-)

diff --git a/lldb/include/lldb/Interpreter/OptionValueSInt64.h 
b/lldb/include/lldb/Interpreter/OptionValueSInt64.h
index 5efae627758aca..3cf41d38c0ef0c 100644
--- a/lldb/include/lldb/Interpreter/OptionValueSInt64.h
+++ b/lldb/include/lldb/Interpreter/OptionValueSInt64.h
@@ -86,8 +86,8 @@ class OptionValueSInt64 : public Cloneable {
 protected:
   int64_t m_current_value = 0;
   int64_t m_default_value = 0;
-  int64_t m_min_value = INT64_MIN;
-  int64_t m_max_value = INT64_MAX;
+  int64_t m_min_value = std::numeric_limits::min();
+  int64_t m_max_value = std::numeric_limits::max();
 };
 
 } // namespace lldb_private
diff --git a/lldb/include/lldb/Interpreter/OptionValueUInt64.h 
b/lldb/include/lldb/Interpreter/OptionValueUInt64.h
index 30c27bf73d99c6..07076075790c67 100644
--- a/lldb/include/lldb/Interpreter/OptionValueUInt64.h
+++ b/lldb/include/lldb/Interpreter/OptionValueUInt64.h
@@ -64,13 +64,35 @@ class OptionValueUInt64 : public 
Cloneable {
 
   uint64_t GetDefaultValue() const { return m_default_value; }
 
-  void SetCurrentValue(uint64_t value) { m_current_value = value; }
+  bool SetCurrentValue(uint64_t value) {
+if (value >= m_min_value && value <= m_max_value) {
+  m_current_value = value;
+  return true;
+}
+return false;
+  }
+
+  bool SetDefaultValue(uint64_t value) {
+if (value >= m_min_value && value <= m_max_value) {
+  m_default_value = value;
+  return true;
+}
+return false;
+  }
+
+  void SetMinimumValue(int64_t v) { m_min_value = v; }
+
+  uint64_t GetMinimumValue() const { return m_min_value; }
+
+  void SetMaximumValue(int64_t v) { m_max_value = v; }
 
-  void SetDefaultValue(uint64_t value) { m_default_value = value; }
+  uint64_t GetMaximumValue() const { return m_max_value; }
 
 protected:
   uint64_t m_current_value = 0;
   uint64_t m_default_value = 0;
+  uint64_t m_min_value = std::numeric_limits::min();
+  uint64_t m_max_value = std::numeric_limits::max();
 };
 
 } // namespace lldb_private
diff --git a/lldb/source/Core/CoreProperties.td 
b/lldb/source/Core/CoreProperties.td
index 4cfff805688c56..a6cb951187a040 100644
--- a/lldb/source/Core/CoreProperties.td
+++ b/lldb/source/Core/CoreProperties.td
@@ -132,7 +132,7 @@ let Definition = "debugger" in {
 Global,
 DefaultStringValue<"${ansi.normal}">,
 Desc<"When displaying the line marker in a color-enabled terminal, use the 
ANSI terminal code specified in this format immediately after the line to be 
marked.">;
-  def TerminalWidth: Property<"term-width", "SInt64">,
+  def TerminalWidth: Property<"term-width", "UInt64">,
 Global,
 DefaultUnsignedValue<80>,
 Desc<"The maximum number of columns to use for displaying text.">;
diff --git a/lldb/source/Core/Debugger.cpp b/lldb/source/Core/Debugger.cpp
index 97311b4716ac2f..bb81110ae35a5d 100644
--- a/lldb/source/Core/Debugger.cpp
+++ b/lldb/source/Core/Debugger.cpp
@@ -886,8 +886,8 @@ Debugger::Debugger(lldb::LogOutputCallback log_callback, 
void *baton)
   }
   assert(m_dummy_target_sp.get() && "Couldn't construct dummy target?");
 
-  OptionValueSInt64 *term_width =
-  m_collection_sp->GetPropertyAtIndexAsOptionValueSInt64(
+  OptionValueUInt64 *term_width =
+  m_collection_sp->GetPropertyAtIndexAsOptionValueUInt64(
   ePropertyTerminalWidth);
   term_width->SetMinimumValue(10);
   term_width->SetMaximumValue(1024);
diff --git a/lldb/test/API/commands/settings/TestSettings.py 
b/lldb/test/API/commands/settings/TestSettings.py
index a2d845493d1df9..104a9f09788c37 100644
--- a/lldb/test/API/commands/settings/TestSettings.py
+++ b/lldb/test/API/commands/settings/TestSettings.py
@@ -2,7 +2,6 @@

[Lldb-commits] [lldb] [lldb] Fix term-width setting (PR #82736)

2024-02-22 Thread Med Ismail Bennani via lldb-commits

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

LGTM!

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


[Lldb-commits] [lldb] [lldb] Fix term-width setting (PR #82736)

2024-02-22 Thread Jonas Devlieghere via lldb-commits

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


[Lldb-commits] [lldb] [lldb] Fix term-width setting (PR #82736)

2024-02-22 Thread Jonas Devlieghere via lldb-commits

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


[Lldb-commits] [lldb] [lldb] Fix term-width setting (PR #82736)

2024-02-22 Thread Jonas Devlieghere via lldb-commits

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

>From 8df692af718f4fafcad99ef966b230af125199e7 Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere 
Date: Thu, 22 Feb 2024 21:01:06 -0800
Subject: [PATCH] [lldb] Fix term-width setting

I noticed that the term-width setting would always report its default
value (80) despite the driver correctly setting the value with
SBDebugger::SetTerminalWidth.

  (lldb) settings show term-width
  term-width (int) = 80

The issue is that the setting was defined as a SInt64 instead of a
UInt64 while the getter returned an unsigned value. There's no reason
the terminal width should be a signed value. My best guess it that it
was using SInt64 because UInt64 didn't support min and max values. I
fixed that and correct the type and now lldb reports the correct
terminal width:

  (lldb) settings show term-width
  term-width (unsigned) = 189
---
 .../lldb/Interpreter/OptionValueSInt64.h  |  4 +--
 .../lldb/Interpreter/OptionValueUInt64.h  | 26 +--
 lldb/source/Core/CoreProperties.td|  2 +-
 lldb/source/Core/Debugger.cpp |  4 +--
 lldb/source/Interpreter/OptionValueUInt64.cpp | 13 +++---
 .../API/commands/settings/TestSettings.py | 15 ---
 .../TestTrimmedProgressReporting.py   |  3 ++-
 7 files changed, 52 insertions(+), 15 deletions(-)

diff --git a/lldb/include/lldb/Interpreter/OptionValueSInt64.h 
b/lldb/include/lldb/Interpreter/OptionValueSInt64.h
index 5efae627758aca..3cf41d38c0ef0c 100644
--- a/lldb/include/lldb/Interpreter/OptionValueSInt64.h
+++ b/lldb/include/lldb/Interpreter/OptionValueSInt64.h
@@ -86,8 +86,8 @@ class OptionValueSInt64 : public Cloneable {
 protected:
   int64_t m_current_value = 0;
   int64_t m_default_value = 0;
-  int64_t m_min_value = INT64_MIN;
-  int64_t m_max_value = INT64_MAX;
+  int64_t m_min_value = std::numeric_limits::min();
+  int64_t m_max_value = std::numeric_limits::max();
 };
 
 } // namespace lldb_private
diff --git a/lldb/include/lldb/Interpreter/OptionValueUInt64.h 
b/lldb/include/lldb/Interpreter/OptionValueUInt64.h
index 30c27bf73d99c6..07076075790c67 100644
--- a/lldb/include/lldb/Interpreter/OptionValueUInt64.h
+++ b/lldb/include/lldb/Interpreter/OptionValueUInt64.h
@@ -64,13 +64,35 @@ class OptionValueUInt64 : public 
Cloneable {
 
   uint64_t GetDefaultValue() const { return m_default_value; }
 
-  void SetCurrentValue(uint64_t value) { m_current_value = value; }
+  bool SetCurrentValue(uint64_t value) {
+if (value >= m_min_value && value <= m_max_value) {
+  m_current_value = value;
+  return true;
+}
+return false;
+  }
+
+  bool SetDefaultValue(uint64_t value) {
+if (value >= m_min_value && value <= m_max_value) {
+  m_default_value = value;
+  return true;
+}
+return false;
+  }
+
+  void SetMinimumValue(int64_t v) { m_min_value = v; }
+
+  uint64_t GetMinimumValue() const { return m_min_value; }
+
+  void SetMaximumValue(int64_t v) { m_max_value = v; }
 
-  void SetDefaultValue(uint64_t value) { m_default_value = value; }
+  uint64_t GetMaximumValue() const { return m_max_value; }
 
 protected:
   uint64_t m_current_value = 0;
   uint64_t m_default_value = 0;
+  uint64_t m_min_value = std::numeric_limits::min();
+  uint64_t m_max_value = std::numeric_limits::max();
 };
 
 } // namespace lldb_private
diff --git a/lldb/source/Core/CoreProperties.td 
b/lldb/source/Core/CoreProperties.td
index 4cfff805688c56..a6cb951187a040 100644
--- a/lldb/source/Core/CoreProperties.td
+++ b/lldb/source/Core/CoreProperties.td
@@ -132,7 +132,7 @@ let Definition = "debugger" in {
 Global,
 DefaultStringValue<"${ansi.normal}">,
 Desc<"When displaying the line marker in a color-enabled terminal, use the 
ANSI terminal code specified in this format immediately after the line to be 
marked.">;
-  def TerminalWidth: Property<"term-width", "SInt64">,
+  def TerminalWidth: Property<"term-width", "UInt64">,
 Global,
 DefaultUnsignedValue<80>,
 Desc<"The maximum number of columns to use for displaying text.">;
diff --git a/lldb/source/Core/Debugger.cpp b/lldb/source/Core/Debugger.cpp
index 97311b4716ac2f..bb81110ae35a5d 100644
--- a/lldb/source/Core/Debugger.cpp
+++ b/lldb/source/Core/Debugger.cpp
@@ -886,8 +886,8 @@ Debugger::Debugger(lldb::LogOutputCallback log_callback, 
void *baton)
   }
   assert(m_dummy_target_sp.get() && "Couldn't construct dummy target?");
 
-  OptionValueSInt64 *term_width =
-  m_collection_sp->GetPropertyAtIndexAsOptionValueSInt64(
+  OptionValueUInt64 *term_width =
+  m_collection_sp->GetPropertyAtIndexAsOptionValueUInt64(
   ePropertyTerminalWidth);
   term_width->SetMinimumValue(10);
   term_width->SetMaximumValue(1024);
diff --git a/lldb/source/Interpreter/OptionValueUInt64.cpp 
b/lldb/source/Interpreter/OptionValueUInt64.cpp
index 1999c63d11aff5..2e69c164e32ac3 100644
--- a/lldb/source/Interpreter/OptionValueUInt64.cpp
+++ b/lldb/s

[Lldb-commits] [lldb] afd4690 - [lldb] Fix term-width setting (#82736)

2024-02-22 Thread via lldb-commits

Author: Jonas Devlieghere
Date: 2024-02-22T21:48:49-08:00
New Revision: afd469023aad10786eaea3d444047a558ad8d5c1

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

LOG: [lldb] Fix term-width setting (#82736)

I noticed that the term-width setting would always report its default
value (80) despite the driver correctly setting the value with
SBDebugger::SetTerminalWidth.

```
(lldb) settings show term-width
term-width (int) = 80
```

The issue is that the setting was defined as a SInt64 instead of a
UInt64 while the getter returned an unsigned value. There's no reason
the terminal width should be a signed value. My best guess it that it
was using SInt64 because UInt64 didn't support min and max values. I
fixed that and correct the type and now lldb reports the correct
terminal width:

```
(lldb) settings show term-width
term-width (unsigned) = 189
```

rdar://123488999

Added: 


Modified: 
lldb/include/lldb/Interpreter/OptionValueSInt64.h
lldb/include/lldb/Interpreter/OptionValueUInt64.h
lldb/source/Core/CoreProperties.td
lldb/source/Core/Debugger.cpp
lldb/source/Interpreter/OptionValueUInt64.cpp
lldb/test/API/commands/settings/TestSettings.py

lldb/test/API/functionalities/progress_reporting/TestTrimmedProgressReporting.py

Removed: 




diff  --git a/lldb/include/lldb/Interpreter/OptionValueSInt64.h 
b/lldb/include/lldb/Interpreter/OptionValueSInt64.h
index 5efae627758aca..3cf41d38c0ef0c 100644
--- a/lldb/include/lldb/Interpreter/OptionValueSInt64.h
+++ b/lldb/include/lldb/Interpreter/OptionValueSInt64.h
@@ -86,8 +86,8 @@ class OptionValueSInt64 : public Cloneable {
 protected:
   int64_t m_current_value = 0;
   int64_t m_default_value = 0;
-  int64_t m_min_value = INT64_MIN;
-  int64_t m_max_value = INT64_MAX;
+  int64_t m_min_value = std::numeric_limits::min();
+  int64_t m_max_value = std::numeric_limits::max();
 };
 
 } // namespace lldb_private

diff  --git a/lldb/include/lldb/Interpreter/OptionValueUInt64.h 
b/lldb/include/lldb/Interpreter/OptionValueUInt64.h
index 30c27bf73d99c6..07076075790c67 100644
--- a/lldb/include/lldb/Interpreter/OptionValueUInt64.h
+++ b/lldb/include/lldb/Interpreter/OptionValueUInt64.h
@@ -64,13 +64,35 @@ class OptionValueUInt64 : public 
Cloneable {
 
   uint64_t GetDefaultValue() const { return m_default_value; }
 
-  void SetCurrentValue(uint64_t value) { m_current_value = value; }
+  bool SetCurrentValue(uint64_t value) {
+if (value >= m_min_value && value <= m_max_value) {
+  m_current_value = value;
+  return true;
+}
+return false;
+  }
+
+  bool SetDefaultValue(uint64_t value) {
+if (value >= m_min_value && value <= m_max_value) {
+  m_default_value = value;
+  return true;
+}
+return false;
+  }
+
+  void SetMinimumValue(int64_t v) { m_min_value = v; }
+
+  uint64_t GetMinimumValue() const { return m_min_value; }
+
+  void SetMaximumValue(int64_t v) { m_max_value = v; }
 
-  void SetDefaultValue(uint64_t value) { m_default_value = value; }
+  uint64_t GetMaximumValue() const { return m_max_value; }
 
 protected:
   uint64_t m_current_value = 0;
   uint64_t m_default_value = 0;
+  uint64_t m_min_value = std::numeric_limits::min();
+  uint64_t m_max_value = std::numeric_limits::max();
 };
 
 } // namespace lldb_private

diff  --git a/lldb/source/Core/CoreProperties.td 
b/lldb/source/Core/CoreProperties.td
index 4cfff805688c56..a6cb951187a040 100644
--- a/lldb/source/Core/CoreProperties.td
+++ b/lldb/source/Core/CoreProperties.td
@@ -132,7 +132,7 @@ let Definition = "debugger" in {
 Global,
 DefaultStringValue<"${ansi.normal}">,
 Desc<"When displaying the line marker in a color-enabled terminal, use the 
ANSI terminal code specified in this format immediately after the line to be 
marked.">;
-  def TerminalWidth: Property<"term-width", "SInt64">,
+  def TerminalWidth: Property<"term-width", "UInt64">,
 Global,
 DefaultUnsignedValue<80>,
 Desc<"The maximum number of columns to use for displaying text.">;

diff  --git a/lldb/source/Core/Debugger.cpp b/lldb/source/Core/Debugger.cpp
index 97311b4716ac2f..bb81110ae35a5d 100644
--- a/lldb/source/Core/Debugger.cpp
+++ b/lldb/source/Core/Debugger.cpp
@@ -886,8 +886,8 @@ Debugger::Debugger(lldb::LogOutputCallback log_callback, 
void *baton)
   }
   assert(m_dummy_target_sp.get() && "Couldn't construct dummy target?");
 
-  OptionValueSInt64 *term_width =
-  m_collection_sp->GetPropertyAtIndexAsOptionValueSInt64(
+  OptionValueUInt64 *term_width =
+  m_collection_sp->GetPropertyAtIndexAsOptionValueUInt64(
   ePropertyTerminalWidth);
   term_width->SetMinimumValue(10);
   term_width->SetMaximumValue(1024);

diff  --git a/lldb/source/Interpreter/OptionValueUInt64.cpp 
b/lldb/source/Interpreter/Optio

[Lldb-commits] [lldb] [lldb] Fix term-width setting (PR #82736)

2024-02-22 Thread Jonas Devlieghere via lldb-commits

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


[Lldb-commits] [lldb] [lldb] Fix term-width setting (PR #82736)

2024-02-22 Thread Greg Clayton via lldb-commits

https://github.com/clayborg commented:

LGTM

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


[Lldb-commits] [lldb] 354401f - [lldb] Fix GetTerminalWidth after afd469023aad

2024-02-22 Thread Jonas Devlieghere via lldb-commits

Author: Jonas Devlieghere
Date: 2024-02-22T23:54:13-08:00
New Revision: 354401f8d3dc08ed41895d03a12a122e9cc0482c

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

LOG: [lldb] Fix GetTerminalWidth after afd469023aad

afd469023aad fixed the type of the term-width setting but the getter
(Debugger::GetTerminalWidth) was still trying to get the terminal width
as an unsigned. This fixes TestXMLRegisterFlags.py.

Added: 


Modified: 
lldb/source/Core/Debugger.cpp

Removed: 




diff  --git a/lldb/source/Core/Debugger.cpp b/lldb/source/Core/Debugger.cpp
index bb81110ae35a5d..c3e603dbae8961 100644
--- a/lldb/source/Core/Debugger.cpp
+++ b/lldb/source/Core/Debugger.cpp
@@ -365,7 +365,7 @@ bool Debugger::SetREPLLanguage(lldb::LanguageType 
repl_lang) {
 
 uint64_t Debugger::GetTerminalWidth() const {
   const uint32_t idx = ePropertyTerminalWidth;
-  return GetPropertyAtIndexAs(
+  return GetPropertyAtIndexAs(
   idx, g_debugger_properties[idx].default_uint_value);
 }
 



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