[Lldb-commits] [lldb] [llvm] [WIP][lldb-dap] Add support for data breakpoint. (PR #81541)

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

ZequanWu wrote:

> This looks great overall! Could you split this PR into two? One with the 
> refactoring and another one with the new features? That would make reviewing 
> easier

Yes, I already split it out: https://github.com/llvm/llvm-project/pull/80753 is 
the refactor pr. I'll rebase this one once that is merged.

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


[Lldb-commits] [lldb] [llvm] [lldb-dap][NFC] Add Breakpoint struct to share common logic. (PR #80753)

2024-02-13 Thread Walter Erquinigo via lldb-commits

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

thanks! 

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


[Lldb-commits] [lldb] [llvm] [WIP][lldb-dap] Add support for data breakpoint. (PR #81541)

2024-02-13 Thread Walter Erquinigo via lldb-commits

walter-erquinigo wrote:

just ping me when this PR is rebased

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


[Lldb-commits] [lldb] d58c128 - [lldb-dap][NFC] Add Breakpoint struct to share common logic. (#80753)

2024-02-13 Thread via lldb-commits

Author: Zequan Wu
Date: 2024-02-13T11:38:02-05:00
New Revision: d58c128bc42b8a9cc45516ba9fe9e6a3c322d7b3

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

LOG: [lldb-dap][NFC] Add Breakpoint struct to share common logic. (#80753)

This adds a layer between `SounceBreakpoint`/`FunctionBreakpoint` and
`BreakpointBase` to have better separation and encapsulation so we are
not directly operating on `SBBreakpoint`.

I basically moved the `SBBreakpoint` and the methods that requires it
from `BreakpointBase` to `Breakpoint`. This allows adding support for
data watchpoint easier by sharing the logic inside `BreakpointBase`.

Added: 
lldb/tools/lldb-dap/Breakpoint.cpp
lldb/tools/lldb-dap/Breakpoint.h

Modified: 
lldb/tools/lldb-dap/BreakpointBase.cpp
lldb/tools/lldb-dap/BreakpointBase.h
lldb/tools/lldb-dap/CMakeLists.txt
lldb/tools/lldb-dap/FunctionBreakpoint.cpp
lldb/tools/lldb-dap/FunctionBreakpoint.h
lldb/tools/lldb-dap/JSONUtils.cpp
lldb/tools/lldb-dap/JSONUtils.h
lldb/tools/lldb-dap/SourceBreakpoint.cpp
lldb/tools/lldb-dap/SourceBreakpoint.h
lldb/tools/lldb-dap/lldb-dap.cpp
llvm/utils/gn/secondary/lldb/tools/lldb-dap/BUILD.gn

Removed: 




diff  --git a/lldb/tools/lldb-dap/Breakpoint.cpp 
b/lldb/tools/lldb-dap/Breakpoint.cpp
new file mode 100644
index 00..0c33d4b114d760
--- /dev/null
+++ b/lldb/tools/lldb-dap/Breakpoint.cpp
@@ -0,0 +1,76 @@
+//===-- Breakpoint.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 "Breakpoint.h"
+#include "DAP.h"
+#include "JSONUtils.h"
+#include "llvm/ADT/StringExtras.h"
+
+using namespace lldb_dap;
+
+void Breakpoint::SetCondition() { bp.SetCondition(condition.c_str()); }
+
+void Breakpoint::SetHitCondition() {
+  uint64_t hitCount = 0;
+  if (llvm::to_integer(hitCondition, hitCount))
+bp.SetIgnoreCount(hitCount - 1);
+}
+
+void Breakpoint::CreateJsonObject(llvm::json::Object &object) {
+  // Each breakpoint location is treated as a separate breakpoint for VS code.
+  // They don't have the notion of a single breakpoint with multiple locations.
+  if (!bp.IsValid())
+return;
+  object.try_emplace("verified", bp.GetNumResolvedLocations() > 0);
+  object.try_emplace("id", bp.GetID());
+  // VS Code DAP doesn't currently allow one breakpoint to have multiple
+  // locations so we just report the first one. If we report all locations
+  // then the IDE starts showing the wrong line numbers and locations for
+  // other source file and line breakpoints in the same file.
+
+  // Below we search for the first resolved location in a breakpoint and report
+  // this as the breakpoint location since it will have a complete location
+  // that is at least loaded in the current process.
+  lldb::SBBreakpointLocation bp_loc;
+  const auto num_locs = bp.GetNumLocations();
+  for (size_t i = 0; i < num_locs; ++i) {
+bp_loc = bp.GetLocationAtIndex(i);
+if (bp_loc.IsResolved())
+  break;
+  }
+  // If not locations are resolved, use the first location.
+  if (!bp_loc.IsResolved())
+bp_loc = bp.GetLocationAtIndex(0);
+  auto bp_addr = bp_loc.GetAddress();
+
+  if (bp_addr.IsValid()) {
+std::string formatted_addr =
+"0x" + llvm::utohexstr(bp_addr.GetLoadAddress(g_dap.target));
+object.try_emplace("instructionReference", formatted_addr);
+auto line_entry = bp_addr.GetLineEntry();
+const auto line = line_entry.GetLine();
+if (line != UINT32_MAX)
+  object.try_emplace("line", line);
+const auto column = line_entry.GetColumn();
+if (column != 0)
+  object.try_emplace("column", column);
+object.try_emplace("source", CreateSource(line_entry));
+  }
+}
+
+bool Breakpoint::MatchesName(const char *name) { return bp.MatchesName(name); }
+
+void Breakpoint::SetBreakpoint() {
+  // See comments in BreakpointBase::GetBreakpointLabel() for details of why
+  // we add a label to our breakpoints.
+  bp.AddName(GetBreakpointLabel());
+  if (!condition.empty())
+SetCondition();
+  if (!hitCondition.empty())
+SetHitCondition();
+}

diff  --git a/lldb/tools/lldb-dap/Breakpoint.h 
b/lldb/tools/lldb-dap/Breakpoint.h
new file mode 100644
index 00..47a9d9c59ae2b7
--- /dev/null
+++ b/lldb/tools/lldb-dap/Breakpoint.h
@@ -0,0 +1,33 @@
+//===-- Breakpoint.h *- C++ 
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.tx

[Lldb-commits] [lldb] [llvm] [lldb-dap][NFC] Add Breakpoint struct to share common logic. (PR #80753)

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

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


[Lldb-commits] [lldb] [llvm] [WIP][lldb-dap] Add support for data breakpoint. (PR #81541)

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

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

>From a2d28693da09a569b49bc39a4743e302b2479d87 Mon Sep 17 00:00:00 2001
From: Zequan Wu 
Date: Tue, 13 Feb 2024 11:55:33 -0500
Subject: [PATCH] [lldb-dap] Add support for data breakpoint.

This implements functionality to handle DataBreakpointInfo request and 
SetDataBreakpoints request.

It doesn't handle the case when name is an expression, see Todo comment for 
details.
---
 .../test/tools/lldb-dap/dap_server.py |  38 +++
 .../tools/lldb-dap/databreakpoint/Makefile|   3 +
 .../TestDAP_setDataBreakpoints.py |  88 +++
 .../tools/lldb-dap/databreakpoint/main.cpp|  17 ++
 lldb/tools/lldb-dap/CMakeLists.txt|   1 +
 lldb/tools/lldb-dap/DAPForward.h  |   2 +
 lldb/tools/lldb-dap/Watchpoint.cpp|  48 
 lldb/tools/lldb-dap/Watchpoint.h  |  34 +++
 lldb/tools/lldb-dap/lldb-dap.cpp  | 249 ++
 .../gn/secondary/lldb/tools/lldb-dap/BUILD.gn |   1 +
 10 files changed, 481 insertions(+)
 create mode 100644 lldb/test/API/tools/lldb-dap/databreakpoint/Makefile
 create mode 100644 
lldb/test/API/tools/lldb-dap/databreakpoint/TestDAP_setDataBreakpoints.py
 create mode 100644 lldb/test/API/tools/lldb-dap/databreakpoint/main.cpp
 create mode 100644 lldb/tools/lldb-dap/Watchpoint.cpp
 create mode 100644 lldb/tools/lldb-dap/Watchpoint.h

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..8c192d21461cc9 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,32 @@ def request_setFunctionBreakpoints(self, names, 
condition=None, hitCondition=Non
 }
 return self.send_recv(command_dict)
 
+def request_dataBreakpointInfo(self, variablesReference, name):
+args_dict = {"variablesReference": variablesReference, "name": name}
+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..0de2d5fb3075ac
--- /dev/null
+++ b/lldb/test/API/tools/lldb-dap/databreakpoint/TestDAP_setDataBreakpoints.py
@@ -0,0 +1,88 @@
+"""
+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"]
+
+@skipIfWindows
+@skipIfRemote
+def test_functionality(self):
+"""Tests setting data breakpoints."""
+program = self.getBuildArtifact("a.out")
+self.build_and_launch(program)
+sour

[Lldb-commits] [lldb] [llvm] [WIP][lldb-dap] Add support for data breakpoint. (PR #81541)

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

ZequanWu wrote:

> just ping me when this PR is rebased

Rebased.

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


[Lldb-commits] [lldb] [llvm] [WIP][lldb-dap] Add support for data breakpoint. (PR #81541)

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


@@ -895,6 +906,32 @@ def request_setFunctionBreakpoints(self, names, 
condition=None, hitCondition=Non
 }
 return self.send_recv(command_dict)
 
+def request_dataBreakpointInfo(self, variablesReference, name):
+args_dict = {"variablesReference": variablesReference, "name": name}
+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:

ZequanWu wrote:

`dataBreakpoints` is a list of the dataBreakpoint which has following format:
```
{
dataId: (address in hex)/(size in bytes)
accessType: read/write/readWrite
[condition]: string
[hitCondition]: string
}
```

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


[Lldb-commits] [lldb] [llvm] [lldb-dap] Add support for data breakpoint. (PR #81541)

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

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


[Lldb-commits] [lldb] [llvm] [lldb-dap] Add support for data breakpoint. (PR #81541)

2024-02-13 Thread Walter Erquinigo via lldb-commits

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


[Lldb-commits] [lldb] [llvm] [lldb-dap] Add support for data breakpoint. (PR #81541)

2024-02-13 Thread Walter Erquinigo via lldb-commits

https://github.com/walter-erquinigo commented:

Amazing stuff! I've been wanting this for a while. I left some minor comments.

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


[Lldb-commits] [lldb] [llvm] [lldb-dap] Add support for data breakpoint. (PR #81541)

2024-02-13 Thread Walter Erquinigo via lldb-commits


@@ -2591,6 +2594,248 @@ void request_setFunctionBreakpoints(const 
llvm::json::Object &request) {
   g_dap.SendJSON(llvm::json::Value(std::move(response)));
 }
 
+// "DataBreakpointInfoRequest": {
+//   "allOf": [ { "$ref": "#/definitions/Request" }, {
+// "type": "object",
+// "description": "Obtains information on a possible data breakpoint that
+// could be set on an expression or variable.\nClients should only call 
this
+// request if the corresponding capability `supportsDataBreakpoints` is
+// true.", "properties": {
+//   "command": {
+// "type": "string",
+// "enum": [ "dataBreakpointInfo" ]
+//   },
+//   "arguments": {
+// "$ref": "#/definitions/DataBreakpointInfoArguments"
+//   }
+// },
+// "required": [ "command", "arguments"  ]
+//   }]
+// },
+// "DataBreakpointInfoArguments": {
+//   "type": "object",
+//   "description": "Arguments for `dataBreakpointInfo` request.",
+//   "properties": {
+// "variablesReference": {
+//   "type": "integer",
+//   "description": "Reference to the variable container if the data
+//   breakpoint is requested for a child of the container. The
+//   `variablesReference` must have been obtained in the current suspended
+//   state. See 'Lifetime of Object References' in the Overview section for
+//   details."
+// },
+// "name": {
+//   "type": "string",
+//   "description": "The name of the variable's child to obtain data
+//   breakpoint information for.\nIf `variablesReference` isn't specified,
+//   this can be an expression."
+// },
+// "frameId": {
+//   "type": "integer",
+//   "description": "When `name` is an expression, evaluate it in the scope
+//   of this stack frame. If not specified, the expression is evaluated in
+//   the global scope. When `variablesReference` is specified, this 
property
+//   has no effect."
+// }
+//   },
+//   "required": [ "name" ]
+// },
+// "DataBreakpointInfoResponse": {
+//   "allOf": [ { "$ref": "#/definitions/Response" }, {
+// "type": "object",
+// "description": "Response to `dataBreakpointInfo` request.",
+// "properties": {
+//   "body": {
+// "type": "object",
+// "properties": {
+//   "dataId": {
+// "type": [ "string", "null" ],
+// "description": "An identifier for the data on which a data
+// breakpoint can be registered with the `setDataBreakpoints`
+// request or null if no data breakpoint is available. If a
+// `variablesReference` or `frameId` is passed, the `dataId` is
+// valid in the current suspended state, otherwise it's valid
+// indefinitely. See 'Lifetime of Object References' in the 
Overview
+// section for details. Breakpoints set using the `dataId` in the
+// `setDataBreakpoints` request may outlive the lifetime of the
+// associated `dataId`."
+//   },
+//   "description": {
+// "type": "string",
+// "description": "UI string that describes on what data the
+// breakpoint is set on or why a data breakpoint is not available."
+//   },
+//   "accessTypes": {
+// "type": "array",
+// "items": {
+//   "$ref": "#/definitions/DataBreakpointAccessType"
+// },
+// "description": "Attribute lists the available access types for a
+// potential data breakpoint. A UI client could surface this
+// information."
+//   },
+//   "canPersist": {
+// "type": "boolean",
+// "description": "Attribute indicates that a potential data
+// breakpoint could be persisted across sessions."
+//   }
+// },
+// "required": [ "dataId", "description" ]
+//   }
+// },
+// "required": [ "body" ]
+//   }]
+// }
+void request_dataBreakpointInfo(const llvm::json::Object &request) {
+  llvm::json::Object response;
+  FillResponse(request, response);
+  llvm::json::Object body;
+  lldb::SBError error;
+  llvm::json::Array accessTypes{"read", "write", "readWrite"};
+  const auto *arguments = request.getObject("arguments");
+  const auto variablesReference =
+  GetUnsigned(arguments, "variablesReference", 0);

walter-erquinigo wrote:

it'd be better to use a non-zero default value, which might collide with an 
actual variable ref id

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


[Lldb-commits] [lldb] [llvm] [lldb-dap] Add support for data breakpoint. (PR #81541)

2024-02-13 Thread Walter Erquinigo via lldb-commits


@@ -2591,6 +2594,248 @@ void request_setFunctionBreakpoints(const 
llvm::json::Object &request) {
   g_dap.SendJSON(llvm::json::Value(std::move(response)));
 }
 
+// "DataBreakpointInfoRequest": {
+//   "allOf": [ { "$ref": "#/definitions/Request" }, {
+// "type": "object",
+// "description": "Obtains information on a possible data breakpoint that
+// could be set on an expression or variable.\nClients should only call 
this
+// request if the corresponding capability `supportsDataBreakpoints` is
+// true.", "properties": {
+//   "command": {
+// "type": "string",
+// "enum": [ "dataBreakpointInfo" ]
+//   },
+//   "arguments": {
+// "$ref": "#/definitions/DataBreakpointInfoArguments"
+//   }
+// },
+// "required": [ "command", "arguments"  ]
+//   }]
+// },
+// "DataBreakpointInfoArguments": {
+//   "type": "object",
+//   "description": "Arguments for `dataBreakpointInfo` request.",
+//   "properties": {
+// "variablesReference": {
+//   "type": "integer",
+//   "description": "Reference to the variable container if the data
+//   breakpoint is requested for a child of the container. The
+//   `variablesReference` must have been obtained in the current suspended
+//   state. See 'Lifetime of Object References' in the Overview section for
+//   details."
+// },
+// "name": {
+//   "type": "string",
+//   "description": "The name of the variable's child to obtain data
+//   breakpoint information for.\nIf `variablesReference` isn't specified,
+//   this can be an expression."
+// },
+// "frameId": {
+//   "type": "integer",
+//   "description": "When `name` is an expression, evaluate it in the scope
+//   of this stack frame. If not specified, the expression is evaluated in
+//   the global scope. When `variablesReference` is specified, this 
property
+//   has no effect."
+// }
+//   },
+//   "required": [ "name" ]
+// },
+// "DataBreakpointInfoResponse": {
+//   "allOf": [ { "$ref": "#/definitions/Response" }, {
+// "type": "object",
+// "description": "Response to `dataBreakpointInfo` request.",
+// "properties": {
+//   "body": {
+// "type": "object",
+// "properties": {
+//   "dataId": {
+// "type": [ "string", "null" ],
+// "description": "An identifier for the data on which a data
+// breakpoint can be registered with the `setDataBreakpoints`
+// request or null if no data breakpoint is available. If a
+// `variablesReference` or `frameId` is passed, the `dataId` is
+// valid in the current suspended state, otherwise it's valid
+// indefinitely. See 'Lifetime of Object References' in the 
Overview
+// section for details. Breakpoints set using the `dataId` in the
+// `setDataBreakpoints` request may outlive the lifetime of the
+// associated `dataId`."
+//   },
+//   "description": {
+// "type": "string",
+// "description": "UI string that describes on what data the
+// breakpoint is set on or why a data breakpoint is not available."
+//   },
+//   "accessTypes": {
+// "type": "array",
+// "items": {
+//   "$ref": "#/definitions/DataBreakpointAccessType"
+// },
+// "description": "Attribute lists the available access types for a
+// potential data breakpoint. A UI client could surface this
+// information."
+//   },
+//   "canPersist": {
+// "type": "boolean",
+// "description": "Attribute indicates that a potential data
+// breakpoint could be persisted across sessions."
+//   }
+// },
+// "required": [ "dataId", "description" ]
+//   }
+// },
+// "required": [ "body" ]
+//   }]
+// }
+void request_dataBreakpointInfo(const llvm::json::Object &request) {
+  llvm::json::Object response;
+  FillResponse(request, response);
+  llvm::json::Object body;
+  lldb::SBError error;
+  llvm::json::Array accessTypes{"read", "write", "readWrite"};
+  const auto *arguments = request.getObject("arguments");
+  const auto variablesReference =
+  GetUnsigned(arguments, "variablesReference", 0);
+  llvm::StringRef name = GetString(arguments, "name");
+  lldb::SBFrame frame = g_dap.GetLLDBFrame(*arguments);
+  bool is_duplicated_variable_name = name.contains(" @");
+
+  lldb::SBValue variable;
+  if (lldb::SBValueList *top_scope = GetTopLevelScope(variablesReference)) {
+// variablesReference is one of our scopes, not an actual variable it is
+// asking for a variable in locals or globals or registers
+int64_t end_idx = top_scope->GetSize();
+// Searching backward so that we choose the variable in closest scope
+// among variables of t

[Lldb-commits] [clang] [clang-tools-extra] [lldb] [c++20] P1907R1: Support for generalized non-type template arguments of scalar type. (PR #78041)

2024-02-13 Thread David Blaikie via lldb-commits


@@ -5401,6 +5409,8 @@ std::string CGDebugInfo::GetName(const Decl *D, bool 
Qualified) const {
 // feasible some day.
 return TA.getAsIntegral().getBitWidth() <= 64 &&
IsReconstitutableType(TA.getIntegralType());
+  case TemplateArgument::StructuralValue:
+return false;

dwblaikie wrote:

I did finally take a look at this - I think the Clang side of things is fine - 
if anything improvements could be made to LLVM to be able to lower constants to 
DWARF for a wider variety of values.
eg: For the `float` example, the IR is:
```
!8 = !DITemplateValueParameter(name: "F", type: !9, value: float 1.00e+00)
```
But the DWARF backend can't handle the float constant - the handling is in 
`DwarfUnit::constructTemplateValueParameterDIE` (in 
llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp) and currently only handles 
ConstantInt (for basic stuff - bools, ints, etc) and GlobalValue (for pointer 
non-type template parameters).

So for these new floats and such, the DWARF doesn't include the value, only the 
type.

GCC uses this encoding, for instance:
```
0x002b: DW_TAG_template_value_parameter [3]   (0x001e)
  DW_AT_name [DW_FORM_string]   ("F")
  DW_AT_type [DW_FORM_ref4] (cu + 0x004d => {0x004d} 
"float")
  DW_AT_const_value [DW_FORM_block1](<0x04> 00 00 80 3f )
```

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


[Lldb-commits] [clang] [clang-tools-extra] [lldb] [c++20] P1907R1: Support for generalized non-type template arguments of scalar type. (PR #78041)

2024-02-13 Thread Andrey Ali Khan Bolshakov via lldb-commits


@@ -5401,6 +5409,8 @@ std::string CGDebugInfo::GetName(const Decl *D, bool 
Qualified) const {
 // feasible some day.
 return TA.getAsIntegral().getBitWidth() <= 64 &&
IsReconstitutableType(TA.getIntegralType());
+  case TemplateArgument::StructuralValue:
+return false;

bolshakov-a wrote:

Thanks for clarifying! Probably, I'll come back to this some day to extend the 
backend, but I'm not sure.

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


[Lldb-commits] [lldb] 502a88b - [lldb][NFCI] Add header guard to PlatformRemoteAppleXR.h (#81565)

2024-02-13 Thread via lldb-commits

Author: Alex Langford
Date: 2024-02-13T10:13:35-08:00
New Revision: 502a88bae799694d0ed90e1839cd7a0aacb6bc9d

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

LOG: [lldb][NFCI] Add header guard to PlatformRemoteAppleXR.h (#81565)

Added: 


Modified: 
lldb/source/Plugins/Platform/MacOSX/PlatformRemoteAppleXR.h

Removed: 




diff  --git a/lldb/source/Plugins/Platform/MacOSX/PlatformRemoteAppleXR.h 
b/lldb/source/Plugins/Platform/MacOSX/PlatformRemoteAppleXR.h
index 4fed6e15eda31c..2fbb6caad8110f 100644
--- a/lldb/source/Plugins/Platform/MacOSX/PlatformRemoteAppleXR.h
+++ b/lldb/source/Plugins/Platform/MacOSX/PlatformRemoteAppleXR.h
@@ -6,6 +6,9 @@
 //
 
//===--===//
 
+#ifndef LLDB_SOURCE_PLUGINS_PLATFORM_MACOSX_PLATFORMREMOTEAPPLEXR_H
+#define LLDB_SOURCE_PLUGINS_PLATFORM_MACOSX_PLATFORMREMOTEAPPLEXR_H
+
 #include "PlatformRemoteDarwinDevice.h"
 
 namespace lldb_private {
@@ -36,3 +39,5 @@ class PlatformRemoteAppleXR : public 
PlatformRemoteDarwinDevice {
   llvm::StringRef GetPlatformName() override;
 };
 } // namespace lldb_private
+
+#endif // LLDB_SOURCE_PLUGINS_PLATFORM_MACOSX_PLATFORMREMOTEAPPLEXR_H



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


[Lldb-commits] [lldb] [lldb][NFCI] Add header guard to PlatformRemoteAppleXR.h (PR #81565)

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

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


[Lldb-commits] [lldb] [lldb] Detect a Darwin kernel issue and work around it (PR #81573)

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


@@ -825,6 +806,56 @@ StopInfoSP 
StopInfoMachException::CreateStopReasonWithMachException(
 break;
   }
 
-  return StopInfoSP(new StopInfoMachException(thread, exc_type, exc_data_count,
-  exc_code, exc_sub_code));
+  return StopInfoSP(new StopInfoMachException(

bulbazord wrote:

Since you're already here: `make_shared`? 😄 

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


[Lldb-commits] [lldb] [lldb] Detect a Darwin kernel issue and work around it (PR #81573)

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

https://github.com/bulbazord commented:

Makes sense to me!

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


[Lldb-commits] [lldb] [lldb] Detect a Darwin kernel issue and work around it (PR #81573)

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


@@ -79,6 +79,11 @@ class StopInfo : public 
std::enable_shared_from_this {
 
   virtual bool IsValidForOperatingSystemThread(Thread &thread) { return true; }
 
+  /// A Continue operation can result in a false stop event
+  /// before any execution has happened in certain cases on Darwin.
+  /// We need to silently continue again time.

bulbazord wrote:

"continue again time." -> "continue again one more time."

Is this what you meant?

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


[Lldb-commits] [lldb] [lldb] Detect a Darwin kernel issue and work around it (PR #81573)

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

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


[Lldb-commits] [lldb] [lldb] Detect a Darwin kernel issue and work around it (PR #81573)

2024-02-13 Thread via lldb-commits

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


[Lldb-commits] [lldb] [lldb] Detect a Darwin kernel issue and work around it (PR #81573)

2024-02-13 Thread via lldb-commits


@@ -79,6 +79,11 @@ class StopInfo : public 
std::enable_shared_from_this {
 
   virtual bool IsValidForOperatingSystemThread(Thread &thread) { return true; }
 
+  /// A Continue operation can result in a false stop event
+  /// before any execution has happened in certain cases on Darwin.
+  /// We need to silently continue again time.
+  virtual bool IsContinueInterrupted(Thread &thread) { return false; }

jimingham wrote:

This may be a little picky, but the continue that was interrupted is in the 
past by the time you call this.  WasContinueInterrupted reads more naturally to 
me because of that. 

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


[Lldb-commits] [lldb] [lldb] Detect a Darwin kernel issue and work around it (PR #81573)

2024-02-13 Thread via lldb-commits


@@ -79,6 +79,11 @@ class StopInfo : public 
std::enable_shared_from_this {
 
   virtual bool IsValidForOperatingSystemThread(Thread &thread) { return true; }
 
+  /// A Continue operation can result in a false stop event
+  /// before any execution has happened in certain cases on Darwin.
+  /// We need to silently continue again time.

jimingham wrote:

"again time" -> "again one more time"?

Also, this isn't a Darwin specific feature, on any system where the Continue 
might stall, you could use this.  I'd describe what it does rather than 
primarily refer to Darwin. 

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


[Lldb-commits] [lldb] [lldb] Detect a Darwin kernel issue and work around it (PR #81573)

2024-02-13 Thread via lldb-commits


@@ -1226,6 +1227,7 @@ class Thread : public 
std::enable_shared_from_this,
   friend class StackFrameList;
   friend class StackFrame;
   friend class OperatingSystem;
+  friend class StopInfoMachException;

jimingham wrote:

Why is this necessary?  GetPreviousFrameZeroPC is public, right?

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


[Lldb-commits] [lldb] [lldb] Detect a Darwin kernel issue and work around it (PR #81573)

2024-02-13 Thread via lldb-commits


@@ -825,6 +806,56 @@ StopInfoSP 
StopInfoMachException::CreateStopReasonWithMachException(
 break;
   }
 
-  return StopInfoSP(new StopInfoMachException(thread, exc_type, exc_data_count,
-  exc_code, exc_sub_code));
+  return StopInfoSP(new StopInfoMachException(
+  thread, exc_type, exc_data_count, exc_code, exc_sub_code,
+  not_stepping_but_got_singlestep_exception));
+}
+
+// Detect an unusual situation on Darwin where:
+//
+//   0. We did an instruction-step before this.
+//   1. We have a hardware breakpoint or watchpoint set.
+//   2. We are resuming the process.

jimingham wrote:

Technically we also "resume" the process when we single step.  You mean we're 
not single stepping here, I'd say the explicitly.

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


[Lldb-commits] [lldb] [lldb] Detect a Darwin kernel issue and work around it (PR #81573)

2024-02-13 Thread via lldb-commits

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

This looks good.  You're being careful to do as narrow as possible a check for 
the buggy behavior.  Handling this when the private StopInfo is great, that 
renders it a non-event in the higher level ShouldStop negotiations, which is 
exactly what we want.

I had a couple trivial comments, and I don't think you should need the `friend 
class` statement.  If you do need that for reasons I missed, we should probably 
make some API to get around that, StopInfoMachException really shouldn't need 
private access to the Thread class.

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


[Lldb-commits] [lldb] [lldb] Detect a Darwin kernel issue and work around it (PR #81573)

2024-02-13 Thread via lldb-commits

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


[Lldb-commits] [lldb] a04c636 - Don't count all the frames just to skip the current inlined ones. (#80918)

2024-02-13 Thread via lldb-commits

Author: jimingham
Date: 2024-02-13T11:06:32-08:00
New Revision: a04c6366b156f508cdf84a32ef4484b53a6dabee

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

LOG: Don't count all the frames just to skip the current inlined ones. (#80918)

The algorithm to find the DW_OP_entry_value requires you to find the
nearest non-inlined frame. It did that by counting the number of stack
frames so that it could use that as a loop stopper.

That is unnecessary and inefficient. Unnecessary because GetFrameAtIndex
will return a null frame when you step past the oldest frame, so you
already have the "got to the end" signal without counting all the stack
frames.
And counting all the stack frames can be expensive.

Added: 


Modified: 
lldb/include/lldb/Target/Thread.h
lldb/source/Expression/DWARFExpression.cpp

Removed: 




diff  --git a/lldb/include/lldb/Target/Thread.h 
b/lldb/include/lldb/Target/Thread.h
index e423dd4a6d2baa..30863ad4c90299 100644
--- a/lldb/include/lldb/Target/Thread.h
+++ b/lldb/include/lldb/Target/Thread.h
@@ -390,6 +390,13 @@ class Thread : public std::enable_shared_from_this,
   /// and having the thread call the SystemRuntime again.
   virtual bool ThreadHasQueueInformation() const { return false; }
 
+  /// GetStackFrameCount can be expensive.  Stacks can get very deep, and they
+  /// require memory reads for each frame.  So only use GetStackFrameCount 
when 
+  /// you need to know the depth of the stack.  When iterating over frames, its
+  /// better to generate the frames one by one with GetFrameAtIndex, and when
+  /// that returns NULL, you are at the end of the stack.  That way your loop
+  /// will only do the work it needs to, without forcing lldb to realize
+  /// StackFrames you weren't going to look at.
   virtual uint32_t GetStackFrameCount() {
 return GetStackFrameList()->GetNumFrames();
   }

diff  --git a/lldb/source/Expression/DWARFExpression.cpp 
b/lldb/source/Expression/DWARFExpression.cpp
index fe4928d4f43a43..c061fd1140fff7 100644
--- a/lldb/source/Expression/DWARFExpression.cpp
+++ b/lldb/source/Expression/DWARFExpression.cpp
@@ -608,11 +608,10 @@ static bool Evaluate_DW_OP_entry_value(std::vector 
&stack,
   StackFrameSP parent_frame = nullptr;
   addr_t return_pc = LLDB_INVALID_ADDRESS;
   uint32_t current_frame_idx = current_frame->GetFrameIndex();
-  uint32_t num_frames = thread->GetStackFrameCount();
-  for (uint32_t parent_frame_idx = current_frame_idx + 1;
-   parent_frame_idx < num_frames; ++parent_frame_idx) {
+
+  for (uint32_t parent_frame_idx = current_frame_idx + 1;;parent_frame_idx++) {
 parent_frame = thread->GetStackFrameAtIndex(parent_frame_idx);
-// Require a valid sequence of frames.
+// If this is null, we're at the end of the stack.
 if (!parent_frame)
   break;
 



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


[Lldb-commits] [lldb] Don't count all the frames just to skip the current inlined ones. (PR #80918)

2024-02-13 Thread via lldb-commits

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


[Lldb-commits] [lldb] [lldb] Store proper integer bitwidth in Scalar Type (PR #81451)

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

bulbazord wrote:

> > This uses `DataExtractor::GetMaxU64` which already does this under the 
> > hood. What does this do that isn't already being done? It may help if you 
> > add a test case to show what you are trying to fix.
> 
> @clayborg @bulbazord The problem with GetMaxU64 is that it always returns 
> uint64_t even though actual type was uint32_t, so when byteswap is performed 
> it becomes invalid integer, to fixed this we need to store correct bitwidth 
> not higher. i.e. Suppose there is actual 32 bit integer i.e. 0x 
> `GetMaxU64` will return 0x (prmoted to uint64_t from 
> uint32_t) and when performing byteswap on this it becomes 0x 
> which is invalid.

That makes sense to me, but how are you encountering this behavior? What 
specifically is this fixing? If you can write a test case that fails without 
your change but works with your change, that would help us understand what 
you're trying to fix. Correctness is important, and tests help us verify that 
we're not inadvertently introducing regressions.

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


[Lldb-commits] [lldb] Don't cause a premature UpdateIfNeeded when checking in SBValue::GetSP (PR #80222)

2024-02-13 Thread via lldb-commits

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


[Lldb-commits] [lldb] Don't cause a premature UpdateIfNeeded when checking in SBValue::GetSP (PR #80222)

2024-02-13 Thread via lldb-commits

jimingham wrote:

I thought I had some clever idea that made me think this would be sufficient.  
But on further thought, I think I was just delusional.  I'll have to think 
about this some more.

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


[Lldb-commits] [lldb] a69ecb2 - Add the ability to define a Python based command that uses CommandObjectParsed (#70734)

2024-02-13 Thread via lldb-commits

Author: jimingham
Date: 2024-02-13T11:09:47-08:00
New Revision: a69ecb2420f644e31f18fcc61a07b3ca627e8939

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

LOG: Add the ability to define a Python based command that uses 
CommandObjectParsed (#70734)

This allows you to specify options and arguments and their definitions
and then have lldb handle the completions, help, etc. in the same way
that lldb does for its parsed commands internally.

This feature has some design considerations as well as the code, so I've
also set up an RFC, but I did this one first and will put the RFC
address in here once I've pushed it...

Note, the lldb "ParsedCommand interface" doesn't actually do all the
work that it should. For instance, saying the type of an option that has
a completer doesn't automatically hook up the completer, and ditto for
argument values. We also do almost no work to verify that the arguments
match their definition, or do auto-completion for them. This patch
allows you to make a command that's bug-for-bug compatible with built-in
ones, but I didn't want to stall it on getting the auto-command checking
to work all the way correctly.

As an overall design note, my primary goal here was to make an interface
that worked well in the script language. For that I needed, for
instance, to have a property-based way to get all the option values that
were specified. It was much more convenient to do that by making a
fairly bare-bones C interface to define the options and arguments of a
command, and set their values, and then wrap that in a Python class
(installed along with the other bits of the lldb python module) which
you can then derive from to make your new command. This approach will
also make it easier to experiment.

See the file test_commands.py in the test case for examples of how this
works.

Added: 
lldb/examples/python/templates/parsed_cmd.py
lldb/test/API/commands/command/script/add/TestAddParsedCommand.py
lldb/test/API/commands/command/script/add/test_commands.py

Modified: 
lldb/bindings/python/CMakeLists.txt
lldb/bindings/python/python-wrapper.swig
lldb/examples/python/cmdtemplate.py
lldb/include/lldb/Interpreter/CommandObject.h
lldb/include/lldb/Interpreter/ScriptInterpreter.h
lldb/source/Commands/CommandObjectCommands.cpp
lldb/source/Commands/Options.td
lldb/source/Interpreter/CommandObject.cpp
lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
lldb/source/Plugins/ScriptInterpreter/Python/SWIGPythonBridge.h
lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp

Removed: 




diff  --git a/lldb/bindings/python/CMakeLists.txt 
b/lldb/bindings/python/CMakeLists.txt
index c941f764dfc92a..73b1239495e22e 100644
--- a/lldb/bindings/python/CMakeLists.txt
+++ b/lldb/bindings/python/CMakeLists.txt
@@ -96,13 +96,15 @@ function(finish_swig_python swig_target 
lldb_python_bindings_dir lldb_python_tar
 ${lldb_python_target_dir}
 "utils"
 FILES "${LLDB_SOURCE_DIR}/examples/python/in_call_stack.py"
-  "${LLDB_SOURCE_DIR}/examples/python/symbolication.py")
+  "${LLDB_SOURCE_DIR}/examples/python/symbolication.py"
+  )
 
   create_python_package(
 ${swig_target}
 ${lldb_python_target_dir}
 "plugins"
 FILES
+"${LLDB_SOURCE_DIR}/examples/python/templates/parsed_cmd.py"
 "${LLDB_SOURCE_DIR}/examples/python/templates/scripted_process.py"
 "${LLDB_SOURCE_DIR}/examples/python/templates/scripted_platform.py"
 "${LLDB_SOURCE_DIR}/examples/python/templates/operating_system.py")

diff  --git a/lldb/bindings/python/python-wrapper.swig 
b/lldb/bindings/python/python-wrapper.swig
index 17bc7b1f219870..1370afc885d43f 100644
--- a/lldb/bindings/python/python-wrapper.swig
+++ b/lldb/bindings/python/python-wrapper.swig
@@ -287,12 +287,12 @@ PythonObject 
lldb_private::python::SWIGBridge::LLDBSwigPythonCreateScriptedThrea
 }
 
 bool lldb_private::python::SWIGBridge::LLDBSWIGPythonCallThreadPlan(
-void *implementor, const char *method_name, lldb_private::Event *event,
+void *implementer, const char *method_name, lldb_private::Event *event,
 bool &got_error) {
   got_error = false;
 
   PyErr_Cleaner py_err_cleaner(false);
-  PythonObject self(PyRefType::Borrowed, static_cast(implementor));
+  PythonObject self(PyRefType::Borrowed, static_cast(implementer));
   auto pfunc = self.ResolveName(method_name);
 
   if (!pfunc.IsAllocated())
@@ -325,12 +325,12 @@ bool 
lldb_private::python::SWIGBridge::LLDBSWIGPythonCallThreadPlan(
 }
 
 bool lldb_private::python::SWIGBridge::LLDBSWIGPythonCallThreadPlan(
-void *implemen

[Lldb-commits] [lldb] Add the ability to define a Python based command that uses CommandObjectParsed (PR #70734)

2024-02-13 Thread via lldb-commits

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


[Lldb-commits] [lldb] [lldb][NFCI] Remove CommandObjectProcessHandle::VerifyCommandOptionValue (PR #79901)

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

bulbazord wrote:

ping @clayborg How does this look now?

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


[Lldb-commits] [lldb] Fix lldb crash while handling concurrent vfork() (PR #81564)

2024-02-13 Thread via lldb-commits

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


[Lldb-commits] [lldb] Fix lldb crash while handling concurrent vfork() (PR #81564)

2024-02-13 Thread via lldb-commits

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


[Lldb-commits] [lldb] Fix lldb crash while handling concurrent vfork() (PR #81564)

2024-02-13 Thread via lldb-commits

llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: None (jeffreytan81)


Changes

We got user reporting lldb crash while the debuggee is calling vfork() 
concurrently from multiple threads. 
The crash happens because the current implementation can only handle single 
vfork, vforkdone protocol transaction.

This diff fixes the crash by lldb-server storing forked debuggee's  pair in jstopinfo which will be decoded by lldb client to create 
StopInfoVFork for follow parent/child policy. Each StopInfoVFork will later 
have a corresponding vforkdone packet. So the patch also changes the 
`m_vfork_in_progress` to be reference counting based. 

Two new test cases are added which crash/assert without the changes in this 
patch.

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


6 Files Affected:

- (modified) lldb/source/Plugins/Process/Linux/NativeThreadLinux.cpp (+11-1) 
- (modified) lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp 
(+15-9) 
- (modified) lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h (+2-1) 
- (added) lldb/test/API/functionalities/fork/concurrent_vfork/Makefile (+4) 
- (added) 
lldb/test/API/functionalities/fork/concurrent_vfork/TestConcurrentVFork.py 
(+31) 
- (added) lldb/test/API/functionalities/fork/concurrent_vfork/main.cpp (+45) 


``diff
diff --git a/lldb/source/Plugins/Process/Linux/NativeThreadLinux.cpp 
b/lldb/source/Plugins/Process/Linux/NativeThreadLinux.cpp
index b62e9f643fa792..cf8a1e7d34392a 100644
--- a/lldb/source/Plugins/Process/Linux/NativeThreadLinux.cpp
+++ b/lldb/source/Plugins/Process/Linux/NativeThreadLinux.cpp
@@ -120,7 +120,7 @@ bool NativeThreadLinux::GetStopReason(ThreadStopInfo 
&stop_info,
   case eStateCrashed:
   case eStateExited:
   case eStateSuspended:
-  case eStateUnloaded:
+  case eStateUnloaded: {
 if (log)
   LogThreadStopInfo(*log, m_stop_info, "m_stop_info in thread:");
 stop_info = m_stop_info;
@@ -128,7 +128,17 @@ bool NativeThreadLinux::GetStopReason(ThreadStopInfo 
&stop_info,
 if (log)
   LogThreadStopInfo(*log, stop_info, "returned stop_info:");
 
+// Include child process PID/TID for forks.
+// Client expects " " format.
+if (stop_info.reason == eStopReasonFork ||
+stop_info.reason == eStopReasonVFork) {
+  description = std::to_string(stop_info.details.fork.child_pid);
+  description += " ";
+  description += std::to_string(stop_info.details.fork.child_tid);
+}
+
 return true;
+  }
 
   case eStateInvalid:
   case eStateConnected:
diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp 
b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
index 629b191f3117aa..6fdb062e712c78 100644
--- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -263,10 +263,9 @@ ProcessGDBRemote::ProcessGDBRemote(lldb::TargetSP 
target_sp,
   m_continue_C_tids(), m_continue_s_tids(), m_continue_S_tids(),
   m_max_memory_size(0), m_remote_stub_max_memory_size(0),
   m_addr_to_mmap_size(), m_thread_create_bp_sp(),
-  m_waiting_for_attach(false),
-  m_command_sp(), m_breakpoint_pc_offset(0),
+  m_waiting_for_attach(false), m_command_sp(), m_breakpoint_pc_offset(0),
   m_initial_tid(LLDB_INVALID_THREAD_ID), m_allow_flash_writes(false),
-  m_erased_flash_ranges(), m_vfork_in_progress(false) {
+  m_erased_flash_ranges(), m_vfork_in_progress(0) {
   m_async_broadcaster.SetEventName(eBroadcastBitAsyncThreadShouldExit,
"async thread should exit");
   m_async_broadcaster.SetEventName(eBroadcastBitAsyncContinue,
@@ -5615,8 +5614,12 @@ void ProcessGDBRemote::DidFork(lldb::pid_t child_pid, 
lldb::tid_t child_tid) {
 void ProcessGDBRemote::DidVFork(lldb::pid_t child_pid, lldb::tid_t child_tid) {
   Log *log = GetLog(GDBRLog::Process);
 
-  assert(!m_vfork_in_progress);
-  m_vfork_in_progress = true;
+  LLDB_LOG(
+  log,
+  "ProcessGDBRemote::DidFork() called for child_pid: {0}, child_tid {1}",
+  child_pid, child_tid);
+  assert(m_vfork_in_progress >= 0);
+  ++m_vfork_in_progress;
 
   // Disable all software breakpoints for the duration of vfork.
   if (m_gdb_comm.SupportsGDBStoppointPacket(eBreakpointSoftware))
@@ -5670,8 +5673,8 @@ void ProcessGDBRemote::DidVFork(lldb::pid_t child_pid, 
lldb::tid_t child_tid) {
 }
 
 void ProcessGDBRemote::DidVForkDone() {
-  assert(m_vfork_in_progress);
-  m_vfork_in_progress = false;
+  --m_vfork_in_progress;
+  assert(m_vfork_in_progress >= 0);
 
   // Reenable all software breakpoints that were enabled before vfork.
   if (m_gdb_comm.SupportsGDBStoppointPacket(eBreakpointSoftware))
@@ -5681,7 +5684,10 @@ void ProcessGDBRemote::DidVForkDone() {
 void ProcessGDBRemote::DidExec() {
   // If we are following children, vfork is finished by exec (rather than
   // vforkdone that is submitted for parent).
-  if (GetFollowForkMode() == eFollowC

[Lldb-commits] [lldb] Fix lldb crash while handling concurrent vfork() (PR #81564)

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

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


[Lldb-commits] [lldb] Fix lldb crash while handling concurrent vfork() (PR #81564)

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


@@ -0,0 +1,31 @@
+"""
+Make sure that the concurrent vfork() from multiple threads works correctly.
+"""
+
+
+import lldb
+import lldbsuite.test.lldbutil as lldbutil
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.decorators import *
+
+
+class TestConcurrentVFork(TestBase):
+NO_DEBUG_INFO_TESTCASE = True
+
+@skipIfWindows
+def test_vfork_follow_parent(self):

clayborg wrote:

Can you document what you are testing for here a bit more? I know what you are 
doing, but others might not.

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


[Lldb-commits] [lldb] Fix lldb crash while handling concurrent vfork() (PR #81564)

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

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


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


[Lldb-commits] [lldb] Fix lldb crash while handling concurrent vfork() (PR #81564)

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


@@ -0,0 +1,31 @@
+"""
+Make sure that the concurrent vfork() from multiple threads works correctly.
+"""
+
+
+import lldb
+import lldbsuite.test.lldbutil as lldbutil
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.decorators import *
+
+
+class TestConcurrentVFork(TestBase):
+NO_DEBUG_INFO_TESTCASE = True
+
+@skipIfWindows
+def test_vfork_follow_parent(self):
+self.build()
+lldbutil.run_to_source_breakpoint(
+self, "// break here", lldb.SBFileSpec("main.cpp")
+)
+self.runCmd("settings set target.process.follow-fork-mode parent")
+self.expect("continue", substrs=["exited with status = 0"])

clayborg wrote:

Can we check for a output from our process that matches "Parent process" to 
verify it worked as expected?

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


[Lldb-commits] [lldb] Fix lldb crash while handling concurrent vfork() (PR #81564)

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


@@ -0,0 +1,31 @@
+"""
+Make sure that the concurrent vfork() from multiple threads works correctly.
+"""
+
+
+import lldb
+import lldbsuite.test.lldbutil as lldbutil
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.decorators import *
+
+
+class TestConcurrentVFork(TestBase):
+NO_DEBUG_INFO_TESTCASE = True
+
+@skipIfWindows
+def test_vfork_follow_parent(self):
+self.build()
+lldbutil.run_to_source_breakpoint(
+self, "// break here", lldb.SBFileSpec("main.cpp")
+)
+self.runCmd("settings set target.process.follow-fork-mode parent")
+self.expect("continue", substrs=["exited with status = 0"])
+
+@skipIfWindows
+def test_vfork_follow_child(self):
+self.build()
+lldbutil.run_to_source_breakpoint(
+self, "// break here", lldb.SBFileSpec("main.cpp")
+)
+self.runCmd("settings set target.process.follow-fork-mode child")
+self.expect("continue", substrs=["exited with status = 0"])

clayborg wrote:

Can we check for a output from our process that matches "Child process" to 
verify it worked as expected?

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


[Lldb-commits] [lldb] Fix lldb crash while handling concurrent vfork() (PR #81564)

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


@@ -0,0 +1,45 @@
+#include 
+#include 
+#include 
+#include 
+

clayborg wrote:

Might be a good idea to create a global `std::vector` and a mutex to 
protect it. And any child processes that get forked get added to this vector. 
So maybe add:
```
std::mutex g_child_pids_mutex;
std::vector g_child_pids;
```
I will comment below where to use it.


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


[Lldb-commits] [lldb] Fix lldb crash while handling concurrent vfork() (PR #81564)

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


@@ -120,15 +120,25 @@ bool NativeThreadLinux::GetStopReason(ThreadStopInfo 
&stop_info,
   case eStateCrashed:
   case eStateExited:
   case eStateSuspended:
-  case eStateUnloaded:
+  case eStateUnloaded: {
 if (log)
   LogThreadStopInfo(*log, m_stop_info, "m_stop_info in thread:");
 stop_info = m_stop_info;
 description = m_stop_description;
 if (log)
   LogThreadStopInfo(*log, stop_info, "returned stop_info:");
 
+// Include child process PID/TID for forks.
+// Client expects " " format.
+if (stop_info.reason == eStopReasonFork ||
+stop_info.reason == eStopReasonVFork) {
+  description = std::to_string(stop_info.details.fork.child_pid);
+  description += " ";
+  description += std::to_string(stop_info.details.fork.child_tid);

clayborg wrote:

Why are we adding it here and not changing the original "m_stop_description" to 
contain it? We want everyone to get all of the information.

If this is a human readable string, we should probably print out something like:
```
child-pid = 12345, child-tid = 345645
```
instead of two magic numbers separate by a space. 

Or is this output used in the GDB remote packets somehow? Don't we need to add 
the child pid and tid to the JSON stop info for the other threads in a 
key/value pair encoding?


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


[Lldb-commits] [lldb] Fix lldb crash while handling concurrent vfork() (PR #81564)

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


@@ -5681,7 +5684,10 @@ void ProcessGDBRemote::DidVForkDone() {
 void ProcessGDBRemote::DidExec() {
   // If we are following children, vfork is finished by exec (rather than
   // vforkdone that is submitted for parent).
-  if (GetFollowForkMode() == eFollowChild)
-m_vfork_in_progress = false;
+  if (GetFollowForkMode() == eFollowChild) {
+if (m_vfork_in_progress > 0)
+  --m_vfork_in_progress;
+assert(m_vfork_in_progress >= 0);

clayborg wrote:

This assert does nothing as you won't decrement it if it is not `> 0`. Remove

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


[Lldb-commits] [lldb] Fix lldb crash while handling concurrent vfork() (PR #81564)

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


@@ -0,0 +1,45 @@
+#include 
+#include 
+#include 
+#include 
+
+int call_vfork() {
+  printf("Before vfork\n");
+
+  pid_t child_pid = vfork();
+
+  if (child_pid == -1) {
+// Error handling
+perror("vfork");
+return 1;
+  } else if (child_pid == 0) {
+// This code is executed by the child process
+printf("Child process\n");
+_exit(0); // Exit the child process
+  } else {
+// This code is executed by the parent process
+printf("Parent process\n");

clayborg wrote:

Add the pid to the global vector:
```
printf("Parent process\n");
std::lock_guard Lock(g_child_pids_mutex);
g_child_pids.append(child_pid);
```

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


[Lldb-commits] [lldb] Fix lldb crash while handling concurrent vfork() (PR #81564)

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


@@ -5670,8 +5673,8 @@ void ProcessGDBRemote::DidVFork(lldb::pid_t child_pid, 
lldb::tid_t child_tid) {
 }
 
 void ProcessGDBRemote::DidVForkDone() {
-  assert(m_vfork_in_progress);
-  m_vfork_in_progress = false;
+  --m_vfork_in_progress;
+  assert(m_vfork_in_progress >= 0);

clayborg wrote:

```
assert(m_vfork_in_progress > 0);
 --m_vfork_in_progress;
```

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


[Lldb-commits] [lldb] Fix lldb crash while handling concurrent vfork() (PR #81564)

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


@@ -301,7 +301,8 @@ class ProcessGDBRemote : public Process,
   using FlashRange = FlashRangeVector::Entry;
   FlashRangeVector m_erased_flash_ranges;
 
-  bool m_vfork_in_progress;
+  // Number of vfork in process.
+  int m_vfork_in_progress;

clayborg wrote:

Do we really want a number that can go below zero? Doesn't make sense. I would 
store this as a `uint32_t` and never let it get decremented if it is zero.

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


[Lldb-commits] [lldb] Fix lldb crash while handling concurrent vfork() (PR #81564)

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


@@ -0,0 +1,45 @@
+#include 
+#include 
+#include 
+#include 
+
+int call_vfork() {
+  printf("Before vfork\n");
+
+  pid_t child_pid = vfork();
+
+  if (child_pid == -1) {
+// Error handling
+perror("vfork");
+return 1;
+  } else if (child_pid == 0) {
+// This code is executed by the child process
+printf("Child process\n");
+_exit(0); // Exit the child process
+  } else {
+// This code is executed by the parent process
+printf("Parent process\n");
+  }
+
+  printf("After vfork\n");
+  return 0;
+}
+
+void worker_thread() { call_vfork(); }
+
+void create_threads(int num_threads) {
+  std::vector threads;
+  for (int i = 0; i < num_threads; ++i) {
+threads.emplace_back(std::thread(worker_thread));
+  }
+  printf("Created %d threads, joining...\n",
+ num_threads); // end_of_create_threads
+  for (auto &thread : threads) {
+thread.join();
+  }
+}
+
+int main() {
+  int num_threads = 5; // break here
+  create_threads(num_threads);
+}

clayborg wrote:

Now reap the children you spawned:
```
std::lock_guard Lock(g_child_pids_mutex);
for (pid_t child_pid: g_child_pids) {
  int child_status = 0;
  pid_t pid = waitpid(child_pid, &child_status, 0);
  if (child_status != 0)
_exit(1); // This will let our program know that some child processes 
didn't exist with a zero exit status.
  if (pid !=  child_pid)
_exit(2); // This will let our program know it didn't succeed
}
```
This will ensure that if we stayed with the parent, then we successfully 
resumed all of the child processes and that they didn't crash and that they 
exited. If we don't resume the child processes correctly, they can get caught 
in limbo if they weren't resumed and this test will deadlock waiting for the 
child process to exit. There are some options you can do that allows you to not 
hang where instead of zero passed as the last parameter of `waitpid()`, you can 
specify `WNOHANG`, but that will get racy. We would poll for a few seconds. We 
will want to make sure that the test doesn't just deadlock and cause the test 
suite to never complete.


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


[Lldb-commits] [lldb] [lldb][test] Switch LLDB API tests from vendored unittest2 to unittest (PR #79945)

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

https://github.com/rupprecht updated 
https://github.com/llvm/llvm-project/pull/79945

>From ff9ef2f1dba9b2227916c16ebf7c11810e67a1dd Mon Sep 17 00:00:00 2001
From: Jordan Rupprecht 
Date: Mon, 29 Jan 2024 19:07:36 -0800
Subject: [PATCH 1/6] Blanket unittest2 -> unittest replacement (excluding
 docs)

---
 .../Python/lldbsuite/test/configuration.py|  4 +--
 .../Python/lldbsuite/test/decorators.py   | 34 +--
 lldb/packages/Python/lldbsuite/test/dotest.py | 12 +++
 .../Python/lldbsuite/test/lldbtest.py |  8 ++---
 .../Python/lldbsuite/test/test_result.py  |  4 +--
 .../API/commands/expression/test/TestExprs.py |  4 +--
 .../TestThreadPlanUserBreakpoint.py   |  2 +-
 .../jitloader_gdb/TestJITLoaderGDB.py |  4 +--
 .../thread/state/TestThreadStates.py  |  6 ++--
 .../API/functionalities/tty/TestTerminal.py   |  6 ++--
 .../API/lang/c/shared_lib/TestSharedLib.py|  4 +--
 .../TestSharedLibStrippedSymbols.py   |  4 +--
 .../lang/cpp/namespace/TestNamespaceLookup.py | 10 +++---
 .../TestCppReferenceToOuterClass.py   |  4 +--
 .../lang/objc/hidden-ivars/TestHiddenIvars.py |  4 +--
 .../API/macosx/universal/TestUniversal.py |  8 ++---
 .../test/test_lldbgdbserverutils.py   |  4 +--
 17 files changed, 61 insertions(+), 61 deletions(-)

diff --git a/lldb/packages/Python/lldbsuite/test/configuration.py 
b/lldb/packages/Python/lldbsuite/test/configuration.py
index 2a4b9b3c070c7f..685f491c85fe19 100644
--- a/lldb/packages/Python/lldbsuite/test/configuration.py
+++ b/lldb/packages/Python/lldbsuite/test/configuration.py
@@ -12,14 +12,14 @@
 
 
 # Third-party modules
-import unittest2
+import unittest
 
 # LLDB Modules
 import lldbsuite
 
 
 # The test suite.
-suite = unittest2.TestSuite()
+suite = unittest.TestSuite()
 
 # The list of categories we said we care about
 categories_list = None
diff --git a/lldb/packages/Python/lldbsuite/test/decorators.py 
b/lldb/packages/Python/lldbsuite/test/decorators.py
index 0fb146913388e0..a5e1fa51cf6e63 100644
--- a/lldb/packages/Python/lldbsuite/test/decorators.py
+++ b/lldb/packages/Python/lldbsuite/test/decorators.py
@@ -11,7 +11,7 @@
 import subprocess
 
 # Third-party modules
-import unittest2
+import unittest
 
 # LLDB modules
 import lldb
@@ -115,11 +115,11 @@ def _compiler_supports(
 
 def expectedFailureIf(condition, bugnumber=None):
 def expectedFailure_impl(func):
-if isinstance(func, type) and issubclass(func, unittest2.TestCase):
+if isinstance(func, type) and issubclass(func, unittest.TestCase):
 raise Exception("Decorator can only be used to decorate a test 
method")
 
 if condition:
-return unittest2.expectedFailure(func)
+return unittest.expectedFailure(func)
 return func
 
 if callable(bugnumber):
@@ -130,14 +130,14 @@ def expectedFailure_impl(func):
 
 def expectedFailureIfFn(expected_fn, bugnumber=None):
 def expectedFailure_impl(func):
-if isinstance(func, type) and issubclass(func, unittest2.TestCase):
+if isinstance(func, type) and issubclass(func, unittest.TestCase):
 raise Exception("Decorator can only be used to decorate a test 
method")
 
 @wraps(func)
 def wrapper(*args, **kwargs):
 xfail_reason = expected_fn(*args, **kwargs)
 if xfail_reason is not None:
-xfail_func = unittest2.expectedFailure(func)
+xfail_func = unittest.expectedFailure(func)
 xfail_func(*args, **kwargs)
 else:
 func(*args, **kwargs)
@@ -157,11 +157,11 @@ def wrapper(*args, **kwargs):
 
 def skipTestIfFn(expected_fn, bugnumber=None):
 def skipTestIfFn_impl(func):
-if isinstance(func, type) and issubclass(func, unittest2.TestCase):
+if isinstance(func, type) and issubclass(func, unittest.TestCase):
 reason = expected_fn()
 # The return value is the reason (or None if we don't skip), so
 # reason is used for both args.
-return unittest2.skipIf(condition=reason, reason=reason)(func)
+return unittest.skipIf(condition=reason, reason=reason)(func)
 
 @wraps(func)
 def wrapper(*args, **kwargs):
@@ -191,7 +191,7 @@ def wrapper(*args, **kwargs):
 
 def _xfailForDebugInfo(expected_fn, bugnumber=None):
 def expectedFailure_impl(func):
-if isinstance(func, type) and issubclass(func, unittest2.TestCase):
+if isinstance(func, type) and issubclass(func, unittest.TestCase):
 raise Exception("Decorator can only be used to decorate a test 
method")
 
 func.__xfail_for_debug_info_cat_fn__ = expected_fn
@@ -205,7 +205,7 @@ def expectedFailure_impl(func):
 
 def _skipForDebugInfo(expected_fn, bugnumber=None):
 def skipImpl(func):
-if isinstance(func, type) and issubclass(func, unittest2.TestCase):
+if isinstance(func, type) and issubclass(fun

[Lldb-commits] [lldb] [llvm] [lldb-dap] Add support for data breakpoint. (PR #81541)

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

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

>From a2d28693da09a569b49bc39a4743e302b2479d87 Mon Sep 17 00:00:00 2001
From: Zequan Wu 
Date: Tue, 13 Feb 2024 11:55:33 -0500
Subject: [PATCH 1/2] [lldb-dap] Add support for data breakpoint.

This implements functionality to handle DataBreakpointInfo request and 
SetDataBreakpoints request.

It doesn't handle the case when name is an expression, see Todo comment for 
details.
---
 .../test/tools/lldb-dap/dap_server.py |  38 +++
 .../tools/lldb-dap/databreakpoint/Makefile|   3 +
 .../TestDAP_setDataBreakpoints.py |  88 +++
 .../tools/lldb-dap/databreakpoint/main.cpp|  17 ++
 lldb/tools/lldb-dap/CMakeLists.txt|   1 +
 lldb/tools/lldb-dap/DAPForward.h  |   2 +
 lldb/tools/lldb-dap/Watchpoint.cpp|  48 
 lldb/tools/lldb-dap/Watchpoint.h  |  34 +++
 lldb/tools/lldb-dap/lldb-dap.cpp  | 249 ++
 .../gn/secondary/lldb/tools/lldb-dap/BUILD.gn |   1 +
 10 files changed, 481 insertions(+)
 create mode 100644 lldb/test/API/tools/lldb-dap/databreakpoint/Makefile
 create mode 100644 
lldb/test/API/tools/lldb-dap/databreakpoint/TestDAP_setDataBreakpoints.py
 create mode 100644 lldb/test/API/tools/lldb-dap/databreakpoint/main.cpp
 create mode 100644 lldb/tools/lldb-dap/Watchpoint.cpp
 create mode 100644 lldb/tools/lldb-dap/Watchpoint.h

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..8c192d21461cc9 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,32 @@ def request_setFunctionBreakpoints(self, names, 
condition=None, hitCondition=Non
 }
 return self.send_recv(command_dict)
 
+def request_dataBreakpointInfo(self, variablesReference, name):
+args_dict = {"variablesReference": variablesReference, "name": name}
+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..0de2d5fb3075ac
--- /dev/null
+++ b/lldb/test/API/tools/lldb-dap/databreakpoint/TestDAP_setDataBreakpoints.py
@@ -0,0 +1,88 @@
+"""
+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"]
+
+@skipIfWindows
+@skipIfRemote
+def test_functionality(self):
+"""Tests setting data breakpoints."""
+program = self.getBuildArtifact("a.out")
+self.build_and_launch(program)
+

[Lldb-commits] [lldb] [llvm] [lldb-dap] Add support for data breakpoint. (PR #81541)

2024-02-13 Thread Walter Erquinigo via lldb-commits

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


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


[Lldb-commits] [lldb] [llvm] [lldb-dap] Add support for data breakpoint. (PR #81541)

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


@@ -2591,6 +2594,248 @@ void request_setFunctionBreakpoints(const 
llvm::json::Object &request) {
   g_dap.SendJSON(llvm::json::Value(std::move(response)));
 }
 
+// "DataBreakpointInfoRequest": {
+//   "allOf": [ { "$ref": "#/definitions/Request" }, {
+// "type": "object",
+// "description": "Obtains information on a possible data breakpoint that
+// could be set on an expression or variable.\nClients should only call 
this
+// request if the corresponding capability `supportsDataBreakpoints` is
+// true.", "properties": {
+//   "command": {
+// "type": "string",
+// "enum": [ "dataBreakpointInfo" ]
+//   },
+//   "arguments": {
+// "$ref": "#/definitions/DataBreakpointInfoArguments"
+//   }
+// },
+// "required": [ "command", "arguments"  ]
+//   }]
+// },
+// "DataBreakpointInfoArguments": {
+//   "type": "object",
+//   "description": "Arguments for `dataBreakpointInfo` request.",
+//   "properties": {
+// "variablesReference": {
+//   "type": "integer",
+//   "description": "Reference to the variable container if the data
+//   breakpoint is requested for a child of the container. The
+//   `variablesReference` must have been obtained in the current suspended
+//   state. See 'Lifetime of Object References' in the Overview section for
+//   details."
+// },
+// "name": {
+//   "type": "string",
+//   "description": "The name of the variable's child to obtain data
+//   breakpoint information for.\nIf `variablesReference` isn't specified,
+//   this can be an expression."
+// },
+// "frameId": {
+//   "type": "integer",
+//   "description": "When `name` is an expression, evaluate it in the scope
+//   of this stack frame. If not specified, the expression is evaluated in
+//   the global scope. When `variablesReference` is specified, this 
property
+//   has no effect."
+// }
+//   },
+//   "required": [ "name" ]
+// },
+// "DataBreakpointInfoResponse": {
+//   "allOf": [ { "$ref": "#/definitions/Response" }, {
+// "type": "object",
+// "description": "Response to `dataBreakpointInfo` request.",
+// "properties": {
+//   "body": {
+// "type": "object",
+// "properties": {
+//   "dataId": {
+// "type": [ "string", "null" ],
+// "description": "An identifier for the data on which a data
+// breakpoint can be registered with the `setDataBreakpoints`
+// request or null if no data breakpoint is available. If a
+// `variablesReference` or `frameId` is passed, the `dataId` is
+// valid in the current suspended state, otherwise it's valid
+// indefinitely. See 'Lifetime of Object References' in the 
Overview
+// section for details. Breakpoints set using the `dataId` in the
+// `setDataBreakpoints` request may outlive the lifetime of the
+// associated `dataId`."
+//   },
+//   "description": {
+// "type": "string",
+// "description": "UI string that describes on what data the
+// breakpoint is set on or why a data breakpoint is not available."
+//   },
+//   "accessTypes": {
+// "type": "array",
+// "items": {
+//   "$ref": "#/definitions/DataBreakpointAccessType"
+// },
+// "description": "Attribute lists the available access types for a
+// potential data breakpoint. A UI client could surface this
+// information."
+//   },
+//   "canPersist": {
+// "type": "boolean",
+// "description": "Attribute indicates that a potential data
+// breakpoint could be persisted across sessions."
+//   }
+// },
+// "required": [ "dataId", "description" ]
+//   }
+// },
+// "required": [ "body" ]
+//   }]
+// }
+void request_dataBreakpointInfo(const llvm::json::Object &request) {
+  llvm::json::Object response;
+  FillResponse(request, response);
+  llvm::json::Object body;
+  lldb::SBError error;
+  llvm::json::Array accessTypes{"read", "write", "readWrite"};
+  const auto *arguments = request.getObject("arguments");
+  const auto variablesReference =
+  GetUnsigned(arguments, "variablesReference", 0);

ZequanWu wrote:

If `variablesReference` is 0, it's treated as not provided as it's only 
meaningful if its value > 0. This is also how `request_variables` and 
`request_setVariable` handle it.

The spec 
https://microsoft.github.io/debug-adapter-protocol/specification#Types_Variable 
says: "If `variablesReference` is > 0, the variable is structured ...".

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

[Lldb-commits] [lldb] [llvm] [lldb-dap] Add support for data breakpoint. (PR #81541)

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


@@ -2591,6 +2594,248 @@ void request_setFunctionBreakpoints(const 
llvm::json::Object &request) {
   g_dap.SendJSON(llvm::json::Value(std::move(response)));
 }
 
+// "DataBreakpointInfoRequest": {
+//   "allOf": [ { "$ref": "#/definitions/Request" }, {
+// "type": "object",
+// "description": "Obtains information on a possible data breakpoint that
+// could be set on an expression or variable.\nClients should only call 
this
+// request if the corresponding capability `supportsDataBreakpoints` is
+// true.", "properties": {
+//   "command": {
+// "type": "string",
+// "enum": [ "dataBreakpointInfo" ]
+//   },
+//   "arguments": {
+// "$ref": "#/definitions/DataBreakpointInfoArguments"
+//   }
+// },
+// "required": [ "command", "arguments"  ]
+//   }]
+// },
+// "DataBreakpointInfoArguments": {
+//   "type": "object",
+//   "description": "Arguments for `dataBreakpointInfo` request.",
+//   "properties": {
+// "variablesReference": {
+//   "type": "integer",
+//   "description": "Reference to the variable container if the data
+//   breakpoint is requested for a child of the container. The
+//   `variablesReference` must have been obtained in the current suspended
+//   state. See 'Lifetime of Object References' in the Overview section for
+//   details."
+// },
+// "name": {
+//   "type": "string",
+//   "description": "The name of the variable's child to obtain data
+//   breakpoint information for.\nIf `variablesReference` isn't specified,
+//   this can be an expression."
+// },
+// "frameId": {
+//   "type": "integer",
+//   "description": "When `name` is an expression, evaluate it in the scope
+//   of this stack frame. If not specified, the expression is evaluated in
+//   the global scope. When `variablesReference` is specified, this 
property
+//   has no effect."
+// }
+//   },
+//   "required": [ "name" ]
+// },
+// "DataBreakpointInfoResponse": {
+//   "allOf": [ { "$ref": "#/definitions/Response" }, {
+// "type": "object",
+// "description": "Response to `dataBreakpointInfo` request.",
+// "properties": {
+//   "body": {
+// "type": "object",
+// "properties": {
+//   "dataId": {
+// "type": [ "string", "null" ],
+// "description": "An identifier for the data on which a data
+// breakpoint can be registered with the `setDataBreakpoints`
+// request or null if no data breakpoint is available. If a
+// `variablesReference` or `frameId` is passed, the `dataId` is
+// valid in the current suspended state, otherwise it's valid
+// indefinitely. See 'Lifetime of Object References' in the 
Overview
+// section for details. Breakpoints set using the `dataId` in the
+// `setDataBreakpoints` request may outlive the lifetime of the
+// associated `dataId`."
+//   },
+//   "description": {
+// "type": "string",
+// "description": "UI string that describes on what data the
+// breakpoint is set on or why a data breakpoint is not available."
+//   },
+//   "accessTypes": {
+// "type": "array",
+// "items": {
+//   "$ref": "#/definitions/DataBreakpointAccessType"
+// },
+// "description": "Attribute lists the available access types for a
+// potential data breakpoint. A UI client could surface this
+// information."
+//   },
+//   "canPersist": {
+// "type": "boolean",
+// "description": "Attribute indicates that a potential data
+// breakpoint could be persisted across sessions."
+//   }
+// },
+// "required": [ "dataId", "description" ]
+//   }
+// },
+// "required": [ "body" ]
+//   }]
+// }
+void request_dataBreakpointInfo(const llvm::json::Object &request) {
+  llvm::json::Object response;
+  FillResponse(request, response);
+  llvm::json::Object body;
+  lldb::SBError error;
+  llvm::json::Array accessTypes{"read", "write", "readWrite"};
+  const auto *arguments = request.getObject("arguments");
+  const auto variablesReference =
+  GetUnsigned(arguments, "variablesReference", 0);
+  llvm::StringRef name = GetString(arguments, "name");
+  lldb::SBFrame frame = g_dap.GetLLDBFrame(*arguments);
+  bool is_duplicated_variable_name = name.contains(" @");
+
+  lldb::SBValue variable;
+  if (lldb::SBValueList *top_scope = GetTopLevelScope(variablesReference)) {
+// variablesReference is one of our scopes, not an actual variable it is
+// asking for a variable in locals or globals or registers
+int64_t end_idx = top_scope->GetSize();
+// Searching backward so that we choose the variable in closest scope
+// among variables of t

[Lldb-commits] [lldb] [llvm] [lldb-dap] Add support for data breakpoint. (PR #81541)

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

ZequanWu wrote:

I updated to add support for setting data breakpoint with expression. If 
`variablesReference` is 0 or not provided, interpret `name` as `${number of 
bytes}@${expression}` to set data breakpoint at the given expression because 
the spec 
https://microsoft.github.io/debug-adapter-protocol/specification#Requests_DataBreakpointInfo
 doesn't say how the client could specify the number of bytes to watch. But I 
have concern that using `@` as a separator here, maybe some other languages 
uses `@` as part of their syntax.

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


[Lldb-commits] [lldb] [llvm] [lldb-dap] Add support for data breakpoint. (PR #81541)

2024-02-13 Thread Walter Erquinigo via lldb-commits

walter-erquinigo wrote:

I think using @ is fine, but we can revisit it later if we see any issues.

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


[Lldb-commits] [lldb] [lldb][NFCI] Remove CommandObjectProcessHandle::VerifyCommandOptionValue (PR #79901)

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


@@ -1666,33 +1646,52 @@ class CommandObjectProcessHandle : public 
CommandObjectParsed {
 // the user's options.
 ProcessSP process_sp = target.GetProcessSP();
 
-int stop_action = -1;   // -1 means leave the current setting alone
-int pass_action = -1;   // -1 means leave the current setting alone
-int notify_action = -1; // -1 means leave the current setting alone
+std::optional stop_action = {};
+std::optional pass_action = {};
+std::optional notify_action = {};
 
-if (!m_options.stop.empty() &&
-!VerifyCommandOptionValue(m_options.stop, stop_action)) {
-  result.AppendError("Invalid argument for command option --stop; must be "
- "true or false.\n");
-  return;
+if (!m_options.stop.empty()) {
+  bool success = false;
+  bool value = OptionArgParser::ToBoolean(m_options.stop, false, &success);

clayborg wrote:

Why don't we do this error checking in 
`CommandObjectProcessHandle::CommandOptions::SetOptionValue(...)`? We can 
return an error from there instead of making it successfully though option 
parsing only to look closer at the option values and return an error later in 
this function?

Also, I see many places use this version of `OptionArgParser::ToBoolean(...)`, 
and there is a ton of duplicated code like:
- call `OptionArgParser::ToBoolean(...)`
- check value of success
- make error string and everyone picks their own "hey this isn't a valid 
boolean value (like Jim already mentioned below).

Since there are already so many uses of the current and only version of 
`OptionArgParser::ToBoolean(...)`, maybe we make an overload of this:

```
llvm::Expected OptionArgParser::ToBoolean(llvm::StringRef ref);
```
Then in the `CommandObjectProcessHandle::CommandOptions::SetOptionValue(...) 
function we can do:
```
case 's':
  if (auto ExpectedBool = OptionArgParser::ToBoolean(option_arg)) {
stop = *ExpectedBool;
  else
error = Status(ExpectedBool.takeError());
```
And the new `OptionArgParser::ToBoolean()` overload would return an error as 
Jim wants below:
```
"Invalid  value "%s"
```
Where %s gets substitued with "option_arg.str().c_str()". And then the help 
would help people figure this out.

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


[Lldb-commits] [lldb] f0b271e - Temporarily disable the TestAddParsedCommand.py while I figure out

2024-02-13 Thread Jim Ingham via lldb-commits

Author: Jim Ingham
Date: 2024-02-13T13:16:30-08:00
New Revision: f0b271e4ebd2180f497694838ec9db0b2fd8ab4b

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

LOG: Temporarily disable the TestAddParsedCommand.py while I figure out
why it's crashing on the x86_64 Debian Linux worker.

Added: 


Modified: 
lldb/test/API/commands/command/script/add/TestAddParsedCommand.py

Removed: 




diff  --git a/lldb/test/API/commands/command/script/add/TestAddParsedCommand.py 
b/lldb/test/API/commands/command/script/add/TestAddParsedCommand.py
index 7dba9c6937f211..c044e2bf8c8d28 100644
--- a/lldb/test/API/commands/command/script/add/TestAddParsedCommand.py
+++ b/lldb/test/API/commands/command/script/add/TestAddParsedCommand.py
@@ -13,6 +13,9 @@
 class ParsedCommandTestCase(TestBase):
 NO_DEBUG_INFO_TESTCASE = True
 
+# This crashes on the x86_64 Debian bot, but the failure is not helpful.
+# Disable the test while I try to find a way to reproduce.
+@skipIfLinux 
 def test(self):
 self.pycmd_tests()
 



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


[Lldb-commits] [lldb] 91f4a84 - [lldb][DWARFIndex] Use IDX_parent to implement GetFullyQualifiedType query (#79932)

2024-02-13 Thread via lldb-commits

Author: Felipe de Azevedo Piovezan
Date: 2024-02-13T13:20:49-08:00
New Revision: 91f4a84a1504e718e4f4d4eef5db7713dc30a030

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

LOG: [lldb][DWARFIndex] Use IDX_parent to implement GetFullyQualifiedType query 
(#79932)

This commit changes DebugNamesDWARFIndex so that it now overrides
`GetFullyQualifiedType` and attempts to use DW_IDX_parent, when
available, to speed up such queries. When this type of information is
not available, the base-class implementation is used.

With this commit, we now achieve the 4x speedups reported in [1].

[1]:
https://discourse.llvm.org/t/rfc-improve-dwarf-5-debug-names-type-lookup-parsing-speed/74151/38

Added: 
lldb/unittests/SymbolFile/DWARF/DWARFDebugNamesIndexTest.cpp

Modified: 
lldb/source/Plugins/SymbolFile/DWARF/DWARFDeclContext.h
lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
lldb/unittests/SymbolFile/DWARF/CMakeLists.txt

Removed: 




diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDeclContext.h 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDeclContext.h
index a20a862d340296..7e6c5f51f4beb5 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDeclContext.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDeclContext.h
@@ -47,6 +47,10 @@ class DWARFDeclContext {
 
   DWARFDeclContext() : m_entries() {}
 
+  DWARFDeclContext(llvm::ArrayRef entries) {
+llvm::append_range(m_entries, entries);
+  }
+
   void AppendDeclContext(dw_tag_t tag, const char *name) {
 m_entries.push_back(Entry(tag, name));
   }

diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
index b718f98340a70b..4da0d56fdcacb4 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
@@ -13,6 +13,7 @@
 #include "lldb/Core/Module.h"
 #include "lldb/Utility/RegularExpression.h"
 #include "lldb/Utility/Stream.h"
+#include "llvm/ADT/Sequence.h"
 #include 
 
 using namespace lldb_private;
@@ -218,6 +219,108 @@ void DebugNamesDWARFIndex::GetCompleteObjCClass(
   m_fallback.GetCompleteObjCClass(class_name, must_be_implementation, 
callback);
 }
 
+namespace {
+using Entry = llvm::DWARFDebugNames::Entry;
+
+/// If `entry` and all of its parents have an `IDX_parent`, use that 
information
+/// to build and return a list of at most `max_parents` parent Entries.
+/// `entry` itself is not included in the list.
+/// If any parent does not have an `IDX_parent`, or the Entry data is 
corrupted,
+/// nullopt is returned.
+std::optional>
+getParentChain(Entry entry, uint32_t max_parents) {
+  llvm::SmallVector parent_entries;
+
+  do {
+if (!entry.hasParentInformation())
+  return std::nullopt;
+
+llvm::Expected> parent = entry.getParentDIEEntry();
+if (!parent) {
+  // Bad data.
+  LLDB_LOG_ERROR(
+  GetLog(DWARFLog::Lookups), parent.takeError(),
+  "Failed to extract parent entry from a non-empty IDX_parent");
+  return std::nullopt;
+}
+
+// Last parent in the chain.
+if (!parent->has_value())
+  break;
+
+parent_entries.push_back(**parent);
+entry = **parent;
+  } while (parent_entries.size() < max_parents);
+
+  return parent_entries;
+}
+} // namespace
+
+void DebugNamesDWARFIndex::GetFullyQualifiedType(
+const DWARFDeclContext &context,
+llvm::function_ref callback) {
+  if (context.GetSize() == 0)
+return;
+
+  llvm::StringRef leaf_name = context[0].name;
+  llvm::SmallVector parent_names;
+  for (auto idx : llvm::seq(1, context.GetSize()))
+parent_names.emplace_back(context[idx].name);
+
+  // For each entry, grab its parent chain and check if we have a match.
+  for (const DebugNames::Entry &entry :
+   m_debug_names_up->equal_range(leaf_name)) {
+if (!isType(entry.tag()))
+  continue;
+
+// Grab at most one extra parent, subsequent parents are not necessary to
+// test equality.
+std::optional> parent_chain =
+getParentChain(entry, parent_names.size() + 1);
+
+if (!parent_chain) {
+  // Fallback: use the base class implementation.
+  if (!ProcessEntry(entry, [&](DWARFDIE die) {
+return GetFullyQualifiedTypeImpl(context, die, callback);
+  }))
+return;
+  continue;
+}
+
+if (SameParentChain(parent_names, *parent_chain) &&
+!ProcessEntry(entry, callback))
+  return;
+  }
+}
+
+bool DebugNamesDWARFIndex::SameParentChain(
+llvm::ArrayRef parent_names,
+llvm::ArrayRef parent_entries) const {
+
+  if (parent_entries.size() != pare

[Lldb-commits] [lldb] [lldb][DWARFIndex] Use IDX_parent to implement GetFullyQualifiedType query (PR #79932)

2024-02-13 Thread Felipe de Azevedo Piovezan via lldb-commits

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


[Lldb-commits] [lldb] [lldb] Store proper integer bitwidth in Scalar Type (PR #81451)

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

clayborg wrote:

> > This uses `DataExtractor::GetMaxU64` which already does this under the 
> > hood. What does this do that isn't already being done? It may help if you 
> > add a test case to show what you are trying to fix.
> 
> @clayborg @bulbazord The problem with GetMaxU64 is that it always returns 
> uint64_t even though actual type was uint32_t, so when byteswap is performed 
> it becomes invalid integer, to fixed this we need to store correct bitwidth 
> not higher. i.e. Suppose there is actual 32 bit integer i.e. 0x 
> `GetMaxU64` will return 0x (prmoted to uint64_t from 
> uint32_t) and when performing byteswap on this it becomes 0x 
> which is invalid.

So you are saying someone is taking the resulting Scalar and trying to byteswap 
it? Errors can still happen then in this class for int8_t/uint8_t and 
int16_t/uint16_t as there are no overloads for these in the `Scalar` class. We 
only currently have:
```
  Scalar(int v);
  Scalar(unsigned int v);
  Scalar(long v);
  Scalar(unsigned long v);
  Scalar(long long v);
  Scalar(unsigned long long v);
```
It would suggest if we are going to make a requirement that Scalar will hold 
the correct bit widths, then we need to stop using "int" and "long" and switch 
to using the `int*_t` and `uint*_t` typedefs to make things clear:
```
  Scalar(int8_t v);
  Scalar(int16_t v);
  Scalar(int32_t v);
  Scalar(int64_t v);
  Scalar(uint8_t v);
  Scalar(uint16_t v);
  Scalar(uint32_t v);
  Scalar(uint64_t v);
```
Then we have all of the bit widths handled correctly.

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


[Lldb-commits] [lldb] [lldb] Store proper integer bitwidth in Scalar Type (PR #81451)

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

clayborg wrote:

What code is taking a scalar and then trying to byte swap it after it has been 
created?

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


[Lldb-commits] [lldb] [lldb] Fix `FindDirectNestedType` not working with class templates (PR #81666)

2024-02-13 Thread Vlad Serebrennikov via lldb-commits

https://github.com/Endilll created 
https://github.com/llvm/llvm-project/pull/81666

This patch attempts to fix lookup in class template specialization that have an 
integral non-type template parameter of non-int type. unsigned 3 might be 
printed as `3` or `3U` depending on PrintingPolicy. This patch bring it in line 
with what Clang does (which addes the suffix).

>From bf92dc89858668518a5d842eac34bdf1b3eaade2 Mon Sep 17 00:00:00 2001
From: Vlad Serebrennikov 
Date: Wed, 14 Feb 2024 00:26:09 +0300
Subject: [PATCH] [lldb] Fix `FindDirectNestedType` not working with class
 templates

This patch attempts to fix lookup in class template specialization that have an 
integral non-type template parameter of non-int type. unsigned 3 might be 
printed as `3` or `3U` depending on PrintingPolicy. This patch bring it in line 
with what Clang does (which addes the suffix).
---
 .../TypeSystem/Clang/TypeSystemClang.cpp|  9 +++--
 lldb/test/API/python_api/type/TestTypeList.py   | 17 +
 lldb/test/API/python_api/type/main.cpp  | 13 +
 3 files changed, 37 insertions(+), 2 deletions(-)

diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp 
b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
index a41e2c690853d2..e6a9d3f4f02836 100644
--- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -2165,6 +2165,7 @@ PrintingPolicy TypeSystemClang::GetTypePrintingPolicy() {
   // (and we then would have suppressed them from the type name) and also 
setups
   // where LLDB wasn't able to reconstruct the default arguments.
   printing_policy.SuppressDefaultTemplateArgs = false;
+  printing_policy.AlwaysIncludeTypeForTemplateArgument = true;
   return printing_policy;
 }
 
@@ -9265,8 +9266,12 @@ ConstString TypeSystemClang::DeclContextGetName(void 
*opaque_decl_ctx) {
   if (opaque_decl_ctx) {
 clang::NamedDecl *named_decl =
 llvm::dyn_cast((clang::DeclContext 
*)opaque_decl_ctx);
-if (named_decl)
-  return ConstString(named_decl->getName());
+if (named_decl) {
+  std::string name;
+  llvm::raw_string_ostream stream{name};
+  named_decl->getNameForDiagnostic(stream, GetTypePrintingPolicy(), 
/*qualified=*/ false);
+  return ConstString(name);
+}
   }
   return ConstString();
 }
diff --git a/lldb/test/API/python_api/type/TestTypeList.py 
b/lldb/test/API/python_api/type/TestTypeList.py
index c267defb58edf9..f874e87771c624 100644
--- a/lldb/test/API/python_api/type/TestTypeList.py
+++ b/lldb/test/API/python_api/type/TestTypeList.py
@@ -150,6 +150,23 @@ def test(self):
 invalid_type = task_type.FindDirectNestedType(None)
 self.assertFalse(invalid_type)
 
+# Check that FindDirectNestedType works with types from AST
+pointer = frame0.FindVariable("pointer")
+pointer_type = pointer.GetType()
+self.assertTrue(pointer_type)
+self.DebugSBType(pointer_type)
+pointer_info_type = pointer_type.template_args[0]
+self.assertTrue(pointer_info_type)
+self.DebugSBType(pointer_info_type)
+
+pointer_masks1_type = pointer_info_type.FindDirectNestedType("Masks1")
+self.assertTrue(pointer_masks1_type)
+self.DebugSBType(pointer_masks1_type)
+
+pointer_masks2_type = pointer_info_type.FindDirectNestedType("Masks2")
+self.assertTrue(pointer_masks2_type)
+self.DebugSBType(pointer_masks2_type)
+
 # We'll now get the child member 'id' from 'task_head'.
 id = task_head.GetChildMemberWithName("id")
 self.DebugSBValue(id)
diff --git a/lldb/test/API/python_api/type/main.cpp 
b/lldb/test/API/python_api/type/main.cpp
index 98de9707d88654..b587acdd96c590 100644
--- a/lldb/test/API/python_api/type/main.cpp
+++ b/lldb/test/API/python_api/type/main.cpp
@@ -34,6 +34,15 @@ class Task {
 {}
 };
 
+template 
+struct PointerInfo {
+  enum Masks1 { pointer_mask };
+  enum class Masks2 { pointer_mask };
+};
+
+template >
+struct Pointer {};
+
 enum EnumType {};
 enum class ScopedEnumType {};
 enum class EnumUChar : unsigned char {};
@@ -71,5 +80,9 @@ int main (int argc, char const *argv[])
 ScopedEnumType scoped_enum_type;
 EnumUChar scoped_enum_type_uchar;
 
+Pointer<3> pointer;
+PointerInfo<3>::Masks1 mask1 = PointerInfo<3>::Masks1::pointer_mask;
+PointerInfo<3>::Masks2 mask2 = PointerInfo<3>::Masks2::pointer_mask;
+
 return 0; // Break at this line
 }

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


[Lldb-commits] [lldb] [lldb] Fix `FindDirectNestedType` not working with class templates (PR #81666)

2024-02-13 Thread via lldb-commits

llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: Vlad Serebrennikov (Endilll)


Changes

This patch attempts to fix lookup in class template specialization that have an 
integral non-type template parameter of non-int type. unsigned 3 might be 
printed as `3` or `3U` depending on PrintingPolicy. This patch bring it in line 
with what Clang does (which addes the suffix).

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


3 Files Affected:

- (modified) lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp (+7-2) 
- (modified) lldb/test/API/python_api/type/TestTypeList.py (+17) 
- (modified) lldb/test/API/python_api/type/main.cpp (+13) 


``diff
diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp 
b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
index a41e2c690853d2..e6a9d3f4f02836 100644
--- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -2165,6 +2165,7 @@ PrintingPolicy TypeSystemClang::GetTypePrintingPolicy() {
   // (and we then would have suppressed them from the type name) and also 
setups
   // where LLDB wasn't able to reconstruct the default arguments.
   printing_policy.SuppressDefaultTemplateArgs = false;
+  printing_policy.AlwaysIncludeTypeForTemplateArgument = true;
   return printing_policy;
 }
 
@@ -9265,8 +9266,12 @@ ConstString TypeSystemClang::DeclContextGetName(void 
*opaque_decl_ctx) {
   if (opaque_decl_ctx) {
 clang::NamedDecl *named_decl =
 llvm::dyn_cast((clang::DeclContext 
*)opaque_decl_ctx);
-if (named_decl)
-  return ConstString(named_decl->getName());
+if (named_decl) {
+  std::string name;
+  llvm::raw_string_ostream stream{name};
+  named_decl->getNameForDiagnostic(stream, GetTypePrintingPolicy(), 
/*qualified=*/ false);
+  return ConstString(name);
+}
   }
   return ConstString();
 }
diff --git a/lldb/test/API/python_api/type/TestTypeList.py 
b/lldb/test/API/python_api/type/TestTypeList.py
index c267defb58edf9..f874e87771c624 100644
--- a/lldb/test/API/python_api/type/TestTypeList.py
+++ b/lldb/test/API/python_api/type/TestTypeList.py
@@ -150,6 +150,23 @@ def test(self):
 invalid_type = task_type.FindDirectNestedType(None)
 self.assertFalse(invalid_type)
 
+# Check that FindDirectNestedType works with types from AST
+pointer = frame0.FindVariable("pointer")
+pointer_type = pointer.GetType()
+self.assertTrue(pointer_type)
+self.DebugSBType(pointer_type)
+pointer_info_type = pointer_type.template_args[0]
+self.assertTrue(pointer_info_type)
+self.DebugSBType(pointer_info_type)
+
+pointer_masks1_type = pointer_info_type.FindDirectNestedType("Masks1")
+self.assertTrue(pointer_masks1_type)
+self.DebugSBType(pointer_masks1_type)
+
+pointer_masks2_type = pointer_info_type.FindDirectNestedType("Masks2")
+self.assertTrue(pointer_masks2_type)
+self.DebugSBType(pointer_masks2_type)
+
 # We'll now get the child member 'id' from 'task_head'.
 id = task_head.GetChildMemberWithName("id")
 self.DebugSBValue(id)
diff --git a/lldb/test/API/python_api/type/main.cpp 
b/lldb/test/API/python_api/type/main.cpp
index 98de9707d88654..b587acdd96c590 100644
--- a/lldb/test/API/python_api/type/main.cpp
+++ b/lldb/test/API/python_api/type/main.cpp
@@ -34,6 +34,15 @@ class Task {
 {}
 };
 
+template 
+struct PointerInfo {
+  enum Masks1 { pointer_mask };
+  enum class Masks2 { pointer_mask };
+};
+
+template >
+struct Pointer {};
+
 enum EnumType {};
 enum class ScopedEnumType {};
 enum class EnumUChar : unsigned char {};
@@ -71,5 +80,9 @@ int main (int argc, char const *argv[])
 ScopedEnumType scoped_enum_type;
 EnumUChar scoped_enum_type_uchar;
 
+Pointer<3> pointer;
+PointerInfo<3>::Masks1 mask1 = PointerInfo<3>::Masks1::pointer_mask;
+PointerInfo<3>::Masks2 mask2 = PointerInfo<3>::Masks2::pointer_mask;
+
 return 0; // Break at this line
 }

``




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


[Lldb-commits] [lldb] [lldb] Fix `FindDirectNestedType` not working with class templates (PR #81666)

2024-02-13 Thread via lldb-commits

github-actions[bot] wrote:




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



You can test this locally with the following command:


``bash
git-clang-format --diff fb48fd18c240574841378defacadff34238089bb 
bf92dc89858668518a5d842eac34bdf1b3eaade2 -- 
lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp 
lldb/test/API/python_api/type/main.cpp
``





View the diff from clang-format here.


``diff
diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp 
b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
index e6a9d3f4f0..0652ac0e13 100644
--- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -9269,7 +9269,8 @@ ConstString TypeSystemClang::DeclContextGetName(void 
*opaque_decl_ctx) {
 if (named_decl) {
   std::string name;
   llvm::raw_string_ostream stream{name};
-  named_decl->getNameForDiagnostic(stream, GetTypePrintingPolicy(), 
/*qualified=*/ false);
+  named_decl->getNameForDiagnostic(stream, GetTypePrintingPolicy(),
+   /*qualified=*/false);
   return ConstString(name);
 }
   }
diff --git a/lldb/test/API/python_api/type/main.cpp 
b/lldb/test/API/python_api/type/main.cpp
index b587acdd96..391f58e3e5 100644
--- a/lldb/test/API/python_api/type/main.cpp
+++ b/lldb/test/API/python_api/type/main.cpp
@@ -34,8 +34,7 @@ public:
 {}
 };
 
-template 
-struct PointerInfo {
+template  struct PointerInfo {
   enum Masks1 { pointer_mask };
   enum class Masks2 { pointer_mask };
 };

``




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


[Lldb-commits] [lldb] [lldb] Fix `FindDirectNestedType` not working with class templates (PR #81666)

2024-02-13 Thread Vlad Serebrennikov via lldb-commits

https://github.com/Endilll updated 
https://github.com/llvm/llvm-project/pull/81666

>From bf92dc89858668518a5d842eac34bdf1b3eaade2 Mon Sep 17 00:00:00 2001
From: Vlad Serebrennikov 
Date: Wed, 14 Feb 2024 00:26:09 +0300
Subject: [PATCH 1/2] [lldb] Fix `FindDirectNestedType` not working with class
 templates

This patch attempts to fix lookup in class template specialization that have an 
integral non-type template parameter of non-int type. unsigned 3 might be 
printed as `3` or `3U` depending on PrintingPolicy. This patch bring it in line 
with what Clang does (which addes the suffix).
---
 .../TypeSystem/Clang/TypeSystemClang.cpp|  9 +++--
 lldb/test/API/python_api/type/TestTypeList.py   | 17 +
 lldb/test/API/python_api/type/main.cpp  | 13 +
 3 files changed, 37 insertions(+), 2 deletions(-)

diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp 
b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
index a41e2c690853d2..e6a9d3f4f02836 100644
--- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -2165,6 +2165,7 @@ PrintingPolicy TypeSystemClang::GetTypePrintingPolicy() {
   // (and we then would have suppressed them from the type name) and also 
setups
   // where LLDB wasn't able to reconstruct the default arguments.
   printing_policy.SuppressDefaultTemplateArgs = false;
+  printing_policy.AlwaysIncludeTypeForTemplateArgument = true;
   return printing_policy;
 }
 
@@ -9265,8 +9266,12 @@ ConstString TypeSystemClang::DeclContextGetName(void 
*opaque_decl_ctx) {
   if (opaque_decl_ctx) {
 clang::NamedDecl *named_decl =
 llvm::dyn_cast((clang::DeclContext 
*)opaque_decl_ctx);
-if (named_decl)
-  return ConstString(named_decl->getName());
+if (named_decl) {
+  std::string name;
+  llvm::raw_string_ostream stream{name};
+  named_decl->getNameForDiagnostic(stream, GetTypePrintingPolicy(), 
/*qualified=*/ false);
+  return ConstString(name);
+}
   }
   return ConstString();
 }
diff --git a/lldb/test/API/python_api/type/TestTypeList.py 
b/lldb/test/API/python_api/type/TestTypeList.py
index c267defb58edf9..f874e87771c624 100644
--- a/lldb/test/API/python_api/type/TestTypeList.py
+++ b/lldb/test/API/python_api/type/TestTypeList.py
@@ -150,6 +150,23 @@ def test(self):
 invalid_type = task_type.FindDirectNestedType(None)
 self.assertFalse(invalid_type)
 
+# Check that FindDirectNestedType works with types from AST
+pointer = frame0.FindVariable("pointer")
+pointer_type = pointer.GetType()
+self.assertTrue(pointer_type)
+self.DebugSBType(pointer_type)
+pointer_info_type = pointer_type.template_args[0]
+self.assertTrue(pointer_info_type)
+self.DebugSBType(pointer_info_type)
+
+pointer_masks1_type = pointer_info_type.FindDirectNestedType("Masks1")
+self.assertTrue(pointer_masks1_type)
+self.DebugSBType(pointer_masks1_type)
+
+pointer_masks2_type = pointer_info_type.FindDirectNestedType("Masks2")
+self.assertTrue(pointer_masks2_type)
+self.DebugSBType(pointer_masks2_type)
+
 # We'll now get the child member 'id' from 'task_head'.
 id = task_head.GetChildMemberWithName("id")
 self.DebugSBValue(id)
diff --git a/lldb/test/API/python_api/type/main.cpp 
b/lldb/test/API/python_api/type/main.cpp
index 98de9707d88654..b587acdd96c590 100644
--- a/lldb/test/API/python_api/type/main.cpp
+++ b/lldb/test/API/python_api/type/main.cpp
@@ -34,6 +34,15 @@ class Task {
 {}
 };
 
+template 
+struct PointerInfo {
+  enum Masks1 { pointer_mask };
+  enum class Masks2 { pointer_mask };
+};
+
+template >
+struct Pointer {};
+
 enum EnumType {};
 enum class ScopedEnumType {};
 enum class EnumUChar : unsigned char {};
@@ -71,5 +80,9 @@ int main (int argc, char const *argv[])
 ScopedEnumType scoped_enum_type;
 EnumUChar scoped_enum_type_uchar;
 
+Pointer<3> pointer;
+PointerInfo<3>::Masks1 mask1 = PointerInfo<3>::Masks1::pointer_mask;
+PointerInfo<3>::Masks2 mask2 = PointerInfo<3>::Masks2::pointer_mask;
+
 return 0; // Break at this line
 }

>From 153c8ec6abda4315c5bce6d088513a6ff63f26f9 Mon Sep 17 00:00:00 2001
From: Vlad Serebrennikov 
Date: Wed, 14 Feb 2024 00:33:17 +0300
Subject: [PATCH 2/2] Run clang-format

---
 lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp | 3 ++-
 lldb/test/API/python_api/type/main.cpp   | 3 +--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp 
b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
index e6a9d3f4f02836..0652ac0e134f53 100644
--- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -9269,7 +9269,8 @@ ConstString TypeSystemClang::DeclContextGetName(void 
*opaque_decl_ct

[Lldb-commits] [lldb] [lldb] Support custom printf formatting for variables (PR #81196)

2024-02-13 Thread Dave Lee via lldb-commits

https://github.com/kastiglione updated 
https://github.com/llvm/llvm-project/pull/81196

>From 81a2034ff2b41e30a1f5b82c86b4d5d4c429ed52 Mon Sep 17 00:00:00 2001
From: Dave Lee 
Date: Thu, 8 Feb 2024 13:59:12 -0800
Subject: [PATCH 1/2] [lldb] Support custom printf formatting for variables

---
 lldb/source/Core/FormatEntity.cpp | 25 +++--
 1 file changed, 23 insertions(+), 2 deletions(-)

diff --git a/lldb/source/Core/FormatEntity.cpp 
b/lldb/source/Core/FormatEntity.cpp
index fa5eadc6ff4e9a..0e929203935304 100644
--- a/lldb/source/Core/FormatEntity.cpp
+++ b/lldb/source/Core/FormatEntity.cpp
@@ -883,8 +883,29 @@ static bool DumpValue(Stream &s, const SymbolContext *sc,
   }
 
   if (!is_array_range) {
-LLDB_LOGF(log,
-  "[Debugger::FormatPrompt] dumping ordinary printable output");
+if (!entry.printf_format.empty()) {
+  auto type_info = target->GetTypeInfo();
+  if (type_info & eTypeIsInteger) {
+if (type_info & eTypeIsSigned) {
+  bool success = false;
+  auto integer = target->GetValueAsSigned(0, &success);
+  if (success) {
+LLDB_LOGF(log, "dumping using printf format");
+s.Printf(entry.printf_format.c_str(), integer);
+return true;
+  }
+} else {
+  bool success = false;
+  auto integer = target->GetValueAsUnsigned(0, &success);
+  if (success) {
+LLDB_LOGF(log, "dumping using printf format");
+s.Printf(entry.printf_format.c_str(), integer);
+return true;
+  }
+}
+  }
+}
+LLDB_LOGF(log, "dumping ordinary printable output");
 return target->DumpPrintableRepresentation(s, val_obj_display,
custom_format);
   } else {

>From 335ab1de4b39257e3bbb3bd969a0dd6991747558 Mon Sep 17 00:00:00 2001
From: Dave Lee 
Date: Tue, 13 Feb 2024 13:26:35 -0800
Subject: [PATCH 2/2] Factor out DumpValueWithPrintf; Add test

---
 lldb/source/Core/FormatEntity.cpp | 45 +++
 .../custom-printf-summary/Makefile|  2 +
 .../TestCustomPrintfSummary.py| 11 +
 .../custom-printf-summary/main.c  | 13 ++
 4 files changed, 52 insertions(+), 19 deletions(-)
 create mode 100644 
lldb/test/API/functionalities/data-formatter/custom-printf-summary/Makefile
 create mode 100644 
lldb/test/API/functionalities/data-formatter/custom-printf-summary/TestCustomPrintfSummary.py
 create mode 100644 
lldb/test/API/functionalities/data-formatter/custom-printf-summary/main.c

diff --git a/lldb/source/Core/FormatEntity.cpp 
b/lldb/source/Core/FormatEntity.cpp
index 0e929203935304..57a05507d844cf 100644
--- a/lldb/source/Core/FormatEntity.cpp
+++ b/lldb/source/Core/FormatEntity.cpp
@@ -658,6 +658,25 @@ static char ConvertValueObjectStyleToChar(
   return '\0';
 }
 
+static bool DumpValueWithPrintf(Stream &s, llvm::StringRef format,
+ValueObject &target) {
+  auto type_info = target.GetTypeInfo();
+  if (type_info & eTypeIsInteger) {
+if (type_info & eTypeIsSigned) {
+  if (auto integer = target.GetValueAsSigned()) {
+s.Printf(format.data(), *integer);
+return true;
+  }
+} else {
+  if (auto integer = target.GetValueAsUnsigned()) {
+s.Printf(format.data(), *integer);
+return true;
+  }
+}
+  }
+  return false;
+}
+
 static bool DumpValue(Stream &s, const SymbolContext *sc,
   const ExecutionContext *exe_ctx,
   const FormatEntity::Entry &entry, ValueObject *valobj) {
@@ -884,25 +903,13 @@ static bool DumpValue(Stream &s, const SymbolContext *sc,
 
   if (!is_array_range) {
 if (!entry.printf_format.empty()) {
-  auto type_info = target->GetTypeInfo();
-  if (type_info & eTypeIsInteger) {
-if (type_info & eTypeIsSigned) {
-  bool success = false;
-  auto integer = target->GetValueAsSigned(0, &success);
-  if (success) {
-LLDB_LOGF(log, "dumping using printf format");
-s.Printf(entry.printf_format.c_str(), integer);
-return true;
-  }
-} else {
-  bool success = false;
-  auto integer = target->GetValueAsUnsigned(0, &success);
-  if (success) {
-LLDB_LOGF(log, "dumping using printf format");
-s.Printf(entry.printf_format.c_str(), integer);
-return true;
-  }
-}
+  if (DumpValueWithPrintf(s, entry.printf_format, *target)) {
+LLDB_LOGF(log, "dumping using printf format");
+return true;
+  } else {
+LLDB_LOG(log,
+ "unsupported printf format '{0}' - for type info flags {1}",
+ entry.printf_format, target->GetTypeInfo());
   }
 }
 LLDB_LOGF(log, "dumping ordinary printable output");
diff --git 
a/lldb/test/API/functionalities/data-formatter/custom-p

[Lldb-commits] [lldb] [lldb] Fix `FindDirectNestedType` not working with class templates (PR #81666)

2024-02-13 Thread Vlad Serebrennikov via lldb-commits

Endilll wrote:

This PR is a draft because new test, which is simplified `llvm::PointerIntPair` 
and `llvm::PointerIntPairInfo`, doesn't pass even with the proposed fix.

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


[Lldb-commits] [lldb] [lldb] Support custom printf formatting for variables (PR #81196)

2024-02-13 Thread Dave Lee via lldb-commits

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


[Lldb-commits] [lldb] [llvm] [lldb-dap] Add support for data breakpoint. (PR #81541)

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

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


[Lldb-commits] [lldb] 8c56e78 - [lldb-dap] Add support for data breakpoint. (#81541)

2024-02-13 Thread via lldb-commits

Author: Zequan Wu
Date: 2024-02-13T16:38:56-05:00
New Revision: 8c56e78ec531f0e2460213c20fff869b6b7add99

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

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

This implements functionality to handle `DataBreakpointInfo` request and
`SetDataBreakpoints` request.

If variablesReference is 0 or not provided, interpret name as ${number
of bytes}@${expression} to set data breakpoint at the given expression
because the spec
https://microsoft.github.io/debug-adapter-protocol/specification#Requests_DataBreakpointInfo
doesn't say how the client could specify the number of bytes to watch.

This is based on top of https://github.com/llvm/llvm-project/pull/80753.

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
llvm/utils/gn/secondary/lldb/tools/lldb-dap/BUILD.gn

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..40ca6473649ea9
--- /dev/null
+++ b/lldb/test/API/tools/lldb-dap/databreakpoint/TestDAP_setDataBreakpoints.py
@@ -0,0 +1,123 @@
+"""
+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_tes

[Lldb-commits] [lldb] [llvm] [lldb-dap] Add support for data breakpoint. (PR #81541)

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

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


[Lldb-commits] [lldb] [llvm] [lldb-dap] Add support for data breakpoint. (PR #81541)

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


@@ -2697,58 +2737,41 @@ void request_dataBreakpointInfo(const 
llvm::json::Object &request) {
   GetUnsigned(arguments, "variablesReference", 0);
   llvm::StringRef name = GetString(arguments, "name");
   lldb::SBFrame frame = g_dap.GetLLDBFrame(*arguments);
-  bool is_duplicated_variable_name = name.contains(" @");
+  lldb::SBValue variable = FindVariable(variablesReference, name);
+  std::string addr, size;
 
-  lldb::SBValue variable;
-  if (lldb::SBValueList *top_scope = GetTopLevelScope(variablesReference)) {
-// variablesReference is one of our scopes, not an actual variable it is
-// asking for a variable in locals or globals or registers
-int64_t end_idx = top_scope->GetSize();
-// Searching backward so that we choose the variable in closest scope
-// among variables of the same name.
-for (int64_t i = end_idx - 1; i >= 0; --i) {
-  lldb::SBValue curr_variable = top_scope->GetValueAtIndex(i);
-  std::string variable_name = CreateUniqueVariableNameForDisplay(
-  curr_variable, is_duplicated_variable_name);
-  if (variable_name == name) {
-variable = curr_variable;
-break;
-  }
-}
+  if (variable.IsValid()) {
+addr = llvm::utohexstr(variable.GetLoadAddress());

clayborg wrote:

Need some error checking here. `variable.GetLoadAddress()` will only return a 
valid address if the variable is actually in memory. So if 
`variable.GetLoadAddress()` returns LLDB_INVALID_ADDRESS we need to return an 
appropriate error like " does not exist in memory, its location is 
" where  is the result of `const char 
*SBValue::GetLocation()`.

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


[Lldb-commits] [lldb] [llvm] [lldb-dap] Add support for data breakpoint. (PR #81541)

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


@@ -2697,58 +2737,41 @@ void request_dataBreakpointInfo(const 
llvm::json::Object &request) {
   GetUnsigned(arguments, "variablesReference", 0);
   llvm::StringRef name = GetString(arguments, "name");
   lldb::SBFrame frame = g_dap.GetLLDBFrame(*arguments);
-  bool is_duplicated_variable_name = name.contains(" @");
+  lldb::SBValue variable = FindVariable(variablesReference, name);
+  std::string addr, size;
 
-  lldb::SBValue variable;
-  if (lldb::SBValueList *top_scope = GetTopLevelScope(variablesReference)) {
-// variablesReference is one of our scopes, not an actual variable it is
-// asking for a variable in locals or globals or registers
-int64_t end_idx = top_scope->GetSize();
-// Searching backward so that we choose the variable in closest scope
-// among variables of the same name.
-for (int64_t i = end_idx - 1; i >= 0; --i) {
-  lldb::SBValue curr_variable = top_scope->GetValueAtIndex(i);
-  std::string variable_name = CreateUniqueVariableNameForDisplay(
-  curr_variable, is_duplicated_variable_name);
-  if (variable_name == name) {
-variable = curr_variable;
-break;
-  }
-}
+  if (variable.IsValid()) {
+addr = llvm::utohexstr(variable.GetLoadAddress());
+size = llvm::utostr(variable.GetByteSize());
+  } else if (variablesReference == 0 && frame.IsValid()) {
+// Name might be an expression. In this case we assume that name is 
composed
+// of the number of bytes to watch and expression, separated by '@':
+// "${size}@${expression}"
+llvm::StringRef expr;
+std::tie(size, expr) = name.split('@');
+lldb::SBValue value = frame.EvaluateExpression(expr.data());
+if (value.GetError().Fail()) {
+  lldb::SBError error = value.GetError();
+  const char *error_cstr = error.GetCString();
+  body.try_emplace("dataId", nullptr);
+  body.try_emplace("description", error_cstr && error_cstr[0]
+  ? std::string(error_cstr)
+  : "evaluation failed");
+} else
+  addr = llvm::utohexstr(value.GetValueAsUnsigned());

clayborg wrote:

We probably want to check this address by making sure it is valid. What if this 
return zero?

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


[Lldb-commits] [lldb] [llvm] [lldb-dap] Add support for data breakpoint. (PR #81541)

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


@@ -561,6 +561,46 @@ void EventThreadFunction() {
   }
 }
 
+lldb::SBValue FindVariable(uint64_t variablesReference, llvm::StringRef name) {
+  lldb::SBValue variable;
+  if (lldb::SBValueList *top_scope = GetTopLevelScope(variablesReference)) {
+bool is_duplicated_variable_name = name.contains(" @");
+// variablesReference is one of our scopes, not an actual variable it is
+// asking for a variable in locals or globals or registers
+int64_t end_idx = top_scope->GetSize();
+// Searching backward so that we choose the variable in closest scope
+// among variables of the same name.
+for (int64_t i = end_idx - 1; i >= 0; --i) {
+  lldb::SBValue curr_variable = top_scope->GetValueAtIndex(i);
+  std::string variable_name = CreateUniqueVariableNameForDisplay(
+  curr_variable, is_duplicated_variable_name);

clayborg wrote:

Do we want to check the name that might include the "foo @ main.cpp:12" or just 
look for "foo"? Also, some members of a struct be anonymous unions, we probably 
want to make sure we can do the right thing with those

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


[Lldb-commits] [lldb] [llvm] [lldb-dap] Add support for data breakpoint. (PR #81541)

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


@@ -2697,58 +2737,41 @@ void request_dataBreakpointInfo(const 
llvm::json::Object &request) {
   GetUnsigned(arguments, "variablesReference", 0);
   llvm::StringRef name = GetString(arguments, "name");
   lldb::SBFrame frame = g_dap.GetLLDBFrame(*arguments);
-  bool is_duplicated_variable_name = name.contains(" @");
+  lldb::SBValue variable = FindVariable(variablesReference, name);
+  std::string addr, size;
 
-  lldb::SBValue variable;
-  if (lldb::SBValueList *top_scope = GetTopLevelScope(variablesReference)) {
-// variablesReference is one of our scopes, not an actual variable it is
-// asking for a variable in locals or globals or registers
-int64_t end_idx = top_scope->GetSize();
-// Searching backward so that we choose the variable in closest scope
-// among variables of the same name.
-for (int64_t i = end_idx - 1; i >= 0; --i) {
-  lldb::SBValue curr_variable = top_scope->GetValueAtIndex(i);
-  std::string variable_name = CreateUniqueVariableNameForDisplay(
-  curr_variable, is_duplicated_variable_name);
-  if (variable_name == name) {
-variable = curr_variable;
-break;
-  }
-}
+  if (variable.IsValid()) {
+addr = llvm::utohexstr(variable.GetLoadAddress());
+size = llvm::utostr(variable.GetByteSize());

clayborg wrote:

Need to check if the byte size is not zero and return an error if it is.

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


[Lldb-commits] [lldb] [llvm] [lldb-dap] Add support for data breakpoint. (PR #81541)

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


@@ -2697,58 +2737,41 @@ void request_dataBreakpointInfo(const 
llvm::json::Object &request) {
   GetUnsigned(arguments, "variablesReference", 0);
   llvm::StringRef name = GetString(arguments, "name");
   lldb::SBFrame frame = g_dap.GetLLDBFrame(*arguments);
-  bool is_duplicated_variable_name = name.contains(" @");
+  lldb::SBValue variable = FindVariable(variablesReference, name);
+  std::string addr, size;
 
-  lldb::SBValue variable;
-  if (lldb::SBValueList *top_scope = GetTopLevelScope(variablesReference)) {
-// variablesReference is one of our scopes, not an actual variable it is
-// asking for a variable in locals or globals or registers
-int64_t end_idx = top_scope->GetSize();
-// Searching backward so that we choose the variable in closest scope
-// among variables of the same name.
-for (int64_t i = end_idx - 1; i >= 0; --i) {
-  lldb::SBValue curr_variable = top_scope->GetValueAtIndex(i);
-  std::string variable_name = CreateUniqueVariableNameForDisplay(
-  curr_variable, is_duplicated_variable_name);
-  if (variable_name == name) {
-variable = curr_variable;
-break;
-  }
-}
+  if (variable.IsValid()) {
+addr = llvm::utohexstr(variable.GetLoadAddress());
+size = llvm::utostr(variable.GetByteSize());
+  } else if (variablesReference == 0 && frame.IsValid()) {
+// Name might be an expression. In this case we assume that name is 
composed
+// of the number of bytes to watch and expression, separated by '@':
+// "${size}@${expression}"

clayborg wrote:

Is this format VS Code supported, or just something we are making up?

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


[Lldb-commits] [lldb] 5b38615 - [lldb][test] Switch LLDB API tests from vendored unittest2 to unittest (#79945)

2024-02-13 Thread via lldb-commits

Author: Jordan Rupprecht
Date: 2024-02-13T16:19:41-06:00
New Revision: 5b386158aacac4b41126983a5379d36ed413d0ea

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

LOG: [lldb][test] Switch LLDB API tests from vendored unittest2 to unittest 
(#79945)

This removes the dependency LLDB API tests have on
lldb/third_party/Python/module/unittest2, and instead uses the standard
one provided by Python.

This does not actually remove the vendored dep yet, nor update the docs.
I'll do both those once this sticks.

Non-trivial changes to call out:
- expected failures (i.e. "bugnumber") don't have a reason anymore, so
those params were removed
- `assertItemsEqual` is now called `assertCountEqual`
- When a test is marked xfail, our copy of unittest2 considers failures
during teardown to be OK, but modern unittest does not. See
TestThreadLocal.py. (Very likely could be a real bug/leak).
- Our copy of unittest2 was patched to print all test results, even ones
that don't happen, e.g. `(5 passes, 0 failures, 1 errors, 0 skipped,
...)`, but standard unittest prints a terser message that omits test
result types that didn't happen, e.g. `OK (skipped=1)`. Our lit
integration parses this stderr and needs to be updated w/ that
expectation.

I tested this w/ `ninja check-lldb-api` on Linux. There's a good chance
non-Linux tests have similar quirks, but I'm not able to uncover those.

Added: 


Modified: 
lldb/packages/Python/lldbsuite/test/configuration.py
lldb/packages/Python/lldbsuite/test/decorators.py
lldb/packages/Python/lldbsuite/test/dotest.py
lldb/packages/Python/lldbsuite/test/lldbtest.py
lldb/packages/Python/lldbsuite/test/test_result.py
lldb/test/API/commands/expression/test/TestExprs.py

lldb/test/API/functionalities/breakpoint/thread_plan_user_breakpoint/TestThreadPlanUserBreakpoint.py
lldb/test/API/functionalities/jitloader_gdb/TestJITLoaderGDB.py
lldb/test/API/functionalities/thread/state/TestThreadStates.py
lldb/test/API/functionalities/tty/TestTerminal.py
lldb/test/API/lang/c/shared_lib/TestSharedLib.py

lldb/test/API/lang/c/shared_lib_stripped_symbols/TestSharedLibStrippedSymbols.py
lldb/test/API/lang/cpp/namespace/TestNamespaceLookup.py

lldb/test/API/lang/cpp/reference-to-outer-type/TestCppReferenceToOuterClass.py
lldb/test/API/lang/cpp/thread_local/TestThreadLocal.py
lldb/test/API/lang/objc/hidden-ivars/TestHiddenIvars.py
lldb/test/API/lldbtest.py
lldb/test/API/macosx/universal/TestUniversal.py

lldb/test/API/tools/lldb-server/libraries-svr4/TestGdbRemoteLibrariesSvr4Support.py
lldb/test/API/tools/lldb-server/test/test_lldbgdbserverutils.py

Removed: 




diff  --git a/lldb/packages/Python/lldbsuite/test/configuration.py 
b/lldb/packages/Python/lldbsuite/test/configuration.py
index 2a4b9b3c070c7f..685f491c85fe19 100644
--- a/lldb/packages/Python/lldbsuite/test/configuration.py
+++ b/lldb/packages/Python/lldbsuite/test/configuration.py
@@ -12,14 +12,14 @@
 
 
 # Third-party modules
-import unittest2
+import unittest
 
 # LLDB Modules
 import lldbsuite
 
 
 # The test suite.
-suite = unittest2.TestSuite()
+suite = unittest.TestSuite()
 
 # The list of categories we said we care about
 categories_list = None

diff  --git a/lldb/packages/Python/lldbsuite/test/decorators.py 
b/lldb/packages/Python/lldbsuite/test/decorators.py
index 0fb146913388e0..a5e1fa51cf6e63 100644
--- a/lldb/packages/Python/lldbsuite/test/decorators.py
+++ b/lldb/packages/Python/lldbsuite/test/decorators.py
@@ -11,7 +11,7 @@
 import subprocess
 
 # Third-party modules
-import unittest2
+import unittest
 
 # LLDB modules
 import lldb
@@ -115,11 +115,11 @@ def _compiler_supports(
 
 def expectedFailureIf(condition, bugnumber=None):
 def expectedFailure_impl(func):
-if isinstance(func, type) and issubclass(func, unittest2.TestCase):
+if isinstance(func, type) and issubclass(func, unittest.TestCase):
 raise Exception("Decorator can only be used to decorate a test 
method")
 
 if condition:
-return unittest2.expectedFailure(func)
+return unittest.expectedFailure(func)
 return func
 
 if callable(bugnumber):
@@ -130,14 +130,14 @@ def expectedFailure_impl(func):
 
 def expectedFailureIfFn(expected_fn, bugnumber=None):
 def expectedFailure_impl(func):
-if isinstance(func, type) and issubclass(func, unittest2.TestCase):
+if isinstance(func, type) and issubclass(func, unittest.TestCase):
 raise Exception("Decorator can only be used to decorate a test 
method")
 
 @wraps(func)
 def wrapper(*args, **kwargs):
 xfail_reason = expected_fn(*args, **kwargs)
 if xfail_reason is not None:
-xfail_fun

[Lldb-commits] [lldb] [lldb][test] Switch LLDB API tests from vendored unittest2 to unittest (PR #79945)

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

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


[Lldb-commits] [lldb] [lldb] Fix `FindDirectNestedType` not working with class templates (PR #81666)

2024-02-13 Thread Michael Buch via lldb-commits


@@ -150,6 +150,23 @@ def test(self):
 invalid_type = task_type.FindDirectNestedType(None)
 self.assertFalse(invalid_type)
 
+# Check that FindDirectNestedType works with types from AST
+pointer = frame0.FindVariable("pointer")
+pointer_type = pointer.GetType()
+self.assertTrue(pointer_type)
+self.DebugSBType(pointer_type)
+pointer_info_type = pointer_type.template_args[0]

Michael137 wrote:

```suggestion
pointer_info_type = pointer_type.template_args[1]
```

then the tests works :)

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


[Lldb-commits] [lldb] [lldb] Support custom printf formatting for variables (PR #81196)

2024-02-13 Thread Dave Lee via lldb-commits

https://github.com/kastiglione updated 
https://github.com/llvm/llvm-project/pull/81196

>From 81a2034ff2b41e30a1f5b82c86b4d5d4c429ed52 Mon Sep 17 00:00:00 2001
From: Dave Lee 
Date: Thu, 8 Feb 2024 13:59:12 -0800
Subject: [PATCH 1/3] [lldb] Support custom printf formatting for variables

---
 lldb/source/Core/FormatEntity.cpp | 25 +++--
 1 file changed, 23 insertions(+), 2 deletions(-)

diff --git a/lldb/source/Core/FormatEntity.cpp 
b/lldb/source/Core/FormatEntity.cpp
index fa5eadc6ff4e9a..0e929203935304 100644
--- a/lldb/source/Core/FormatEntity.cpp
+++ b/lldb/source/Core/FormatEntity.cpp
@@ -883,8 +883,29 @@ static bool DumpValue(Stream &s, const SymbolContext *sc,
   }
 
   if (!is_array_range) {
-LLDB_LOGF(log,
-  "[Debugger::FormatPrompt] dumping ordinary printable output");
+if (!entry.printf_format.empty()) {
+  auto type_info = target->GetTypeInfo();
+  if (type_info & eTypeIsInteger) {
+if (type_info & eTypeIsSigned) {
+  bool success = false;
+  auto integer = target->GetValueAsSigned(0, &success);
+  if (success) {
+LLDB_LOGF(log, "dumping using printf format");
+s.Printf(entry.printf_format.c_str(), integer);
+return true;
+  }
+} else {
+  bool success = false;
+  auto integer = target->GetValueAsUnsigned(0, &success);
+  if (success) {
+LLDB_LOGF(log, "dumping using printf format");
+s.Printf(entry.printf_format.c_str(), integer);
+return true;
+  }
+}
+  }
+}
+LLDB_LOGF(log, "dumping ordinary printable output");
 return target->DumpPrintableRepresentation(s, val_obj_display,
custom_format);
   } else {

>From 335ab1de4b39257e3bbb3bd969a0dd6991747558 Mon Sep 17 00:00:00 2001
From: Dave Lee 
Date: Tue, 13 Feb 2024 13:26:35 -0800
Subject: [PATCH 2/3] Factor out DumpValueWithPrintf; Add test

---
 lldb/source/Core/FormatEntity.cpp | 45 +++
 .../custom-printf-summary/Makefile|  2 +
 .../TestCustomPrintfSummary.py| 11 +
 .../custom-printf-summary/main.c  | 13 ++
 4 files changed, 52 insertions(+), 19 deletions(-)
 create mode 100644 
lldb/test/API/functionalities/data-formatter/custom-printf-summary/Makefile
 create mode 100644 
lldb/test/API/functionalities/data-formatter/custom-printf-summary/TestCustomPrintfSummary.py
 create mode 100644 
lldb/test/API/functionalities/data-formatter/custom-printf-summary/main.c

diff --git a/lldb/source/Core/FormatEntity.cpp 
b/lldb/source/Core/FormatEntity.cpp
index 0e929203935304..57a05507d844cf 100644
--- a/lldb/source/Core/FormatEntity.cpp
+++ b/lldb/source/Core/FormatEntity.cpp
@@ -658,6 +658,25 @@ static char ConvertValueObjectStyleToChar(
   return '\0';
 }
 
+static bool DumpValueWithPrintf(Stream &s, llvm::StringRef format,
+ValueObject &target) {
+  auto type_info = target.GetTypeInfo();
+  if (type_info & eTypeIsInteger) {
+if (type_info & eTypeIsSigned) {
+  if (auto integer = target.GetValueAsSigned()) {
+s.Printf(format.data(), *integer);
+return true;
+  }
+} else {
+  if (auto integer = target.GetValueAsUnsigned()) {
+s.Printf(format.data(), *integer);
+return true;
+  }
+}
+  }
+  return false;
+}
+
 static bool DumpValue(Stream &s, const SymbolContext *sc,
   const ExecutionContext *exe_ctx,
   const FormatEntity::Entry &entry, ValueObject *valobj) {
@@ -884,25 +903,13 @@ static bool DumpValue(Stream &s, const SymbolContext *sc,
 
   if (!is_array_range) {
 if (!entry.printf_format.empty()) {
-  auto type_info = target->GetTypeInfo();
-  if (type_info & eTypeIsInteger) {
-if (type_info & eTypeIsSigned) {
-  bool success = false;
-  auto integer = target->GetValueAsSigned(0, &success);
-  if (success) {
-LLDB_LOGF(log, "dumping using printf format");
-s.Printf(entry.printf_format.c_str(), integer);
-return true;
-  }
-} else {
-  bool success = false;
-  auto integer = target->GetValueAsUnsigned(0, &success);
-  if (success) {
-LLDB_LOGF(log, "dumping using printf format");
-s.Printf(entry.printf_format.c_str(), integer);
-return true;
-  }
-}
+  if (DumpValueWithPrintf(s, entry.printf_format, *target)) {
+LLDB_LOGF(log, "dumping using printf format");
+return true;
+  } else {
+LLDB_LOG(log,
+ "unsupported printf format '{0}' - for type info flags {1}",
+ entry.printf_format, target->GetTypeInfo());
   }
 }
 LLDB_LOGF(log, "dumping ordinary printable output");
diff --git 
a/lldb/test/API/functionalities/data-formatter/custom-p

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

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

https://github.com/ZequanWu created 
https://github.com/llvm/llvm-project/pull/81680

Followup fixes to resolve comments in 
https://github.com/llvm/llvm-project/pull/81541

>From f9c509c519c9937d2e2de5098c4241e936187527 Mon Sep 17 00:00:00 2001
From: Zequan Wu 
Date: Tue, 13 Feb 2024 17:41:26 -0500
Subject: [PATCH] [lldb-dap] Followup fixs for data breakpoints

---
 lldb/tools/lldb-dap/lldb-dap.cpp | 31 ---
 1 file changed, 24 insertions(+), 7 deletions(-)

diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp
index 6bf2ec28432cd3..a3c6109370e685 100644
--- a/lldb/tools/lldb-dap/lldb-dap.cpp
+++ b/lldb/tools/lldb-dap/lldb-dap.cpp
@@ -2741,8 +2741,20 @@ void request_dataBreakpointInfo(const llvm::json::Object 
&request) {
   std::string addr, size;
 
   if (variable.IsValid()) {
-addr = llvm::utohexstr(variable.GetLoadAddress());
-size = llvm::utostr(variable.GetByteSize());
+lldb::addr_t load_addr = variable.GetLoadAddress();
+size_t byte_size = variable.GetByteSize();
+if (load_addr == LLDB_INVALID_ADDRESS) {
+  body.try_emplace("dataId", nullptr);
+  body.try_emplace("description",
+   "does not exist in memory, its location is " +
+   std::string(variable.GetLocation()));
+} else if (byte_size == 0) {
+  body.try_emplace("dataId", nullptr);
+  body.try_emplace("description", "variable size is 0");
+} else {
+  addr = llvm::utohexstr(load_addr);
+  size = llvm::utostr(byte_size);
+}
   } else if (variablesReference == 0 && frame.IsValid()) {
 // Name might be an expression. In this case we assume that name is 
composed
 // of the number of bytes to watch and expression, separated by '@':
@@ -2757,13 +2769,18 @@ void request_dataBreakpointInfo(const 
llvm::json::Object &request) {
   body.try_emplace("description", error_cstr && error_cstr[0]
   ? std::string(error_cstr)
   : "evaluation failed");
-} else
-  addr = llvm::utohexstr(value.GetValueAsUnsigned());
+} else {
+  uint64_t value_as_unsigned = value.GetValueAsUnsigned();
+  if (value_as_unsigned == 0) {
+body.try_emplace("dataId", nullptr);
+body.try_emplace("description",
+ "unable to evaluate expression to an address.");
+  }
+  addr = llvm::utohexstr(value_as_unsigned);
+}
   } else {
-auto state = g_dap.target.GetProcess().GetState();
 body.try_emplace("dataId", nullptr);
-body.try_emplace("description",
- "variable not found: " + llvm::utostr(state));
+body.try_emplace("description", "variable not found");
   }
 
   if (!body.getObject("dataId")) {

___
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-13 Thread via lldb-commits

llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: Zequan Wu (ZequanWu)


Changes

Followup fixes to resolve comments in 
https://github.com/llvm/llvm-project/pull/81541

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


1 Files Affected:

- (modified) lldb/tools/lldb-dap/lldb-dap.cpp (+24-7) 


``diff
diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp
index 6bf2ec28432cd3..a3c6109370e685 100644
--- a/lldb/tools/lldb-dap/lldb-dap.cpp
+++ b/lldb/tools/lldb-dap/lldb-dap.cpp
@@ -2741,8 +2741,20 @@ void request_dataBreakpointInfo(const llvm::json::Object 
&request) {
   std::string addr, size;
 
   if (variable.IsValid()) {
-addr = llvm::utohexstr(variable.GetLoadAddress());
-size = llvm::utostr(variable.GetByteSize());
+lldb::addr_t load_addr = variable.GetLoadAddress();
+size_t byte_size = variable.GetByteSize();
+if (load_addr == LLDB_INVALID_ADDRESS) {
+  body.try_emplace("dataId", nullptr);
+  body.try_emplace("description",
+   "does not exist in memory, its location is " +
+   std::string(variable.GetLocation()));
+} else if (byte_size == 0) {
+  body.try_emplace("dataId", nullptr);
+  body.try_emplace("description", "variable size is 0");
+} else {
+  addr = llvm::utohexstr(load_addr);
+  size = llvm::utostr(byte_size);
+}
   } else if (variablesReference == 0 && frame.IsValid()) {
 // Name might be an expression. In this case we assume that name is 
composed
 // of the number of bytes to watch and expression, separated by '@':
@@ -2757,13 +2769,18 @@ void request_dataBreakpointInfo(const 
llvm::json::Object &request) {
   body.try_emplace("description", error_cstr && error_cstr[0]
   ? std::string(error_cstr)
   : "evaluation failed");
-} else
-  addr = llvm::utohexstr(value.GetValueAsUnsigned());
+} else {
+  uint64_t value_as_unsigned = value.GetValueAsUnsigned();
+  if (value_as_unsigned == 0) {
+body.try_emplace("dataId", nullptr);
+body.try_emplace("description",
+ "unable to evaluate expression to an address.");
+  }
+  addr = llvm::utohexstr(value_as_unsigned);
+}
   } else {
-auto state = g_dap.target.GetProcess().GetState();
 body.try_emplace("dataId", nullptr);
-body.try_emplace("description",
- "variable not found: " + llvm::utostr(state));
+body.try_emplace("description", "variable not found");
   }
 
   if (!body.getObject("dataId")) {

``




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] [llvm] [lldb-dap] Add support for data breakpoint. (PR #81541)

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


@@ -2697,58 +2737,41 @@ void request_dataBreakpointInfo(const 
llvm::json::Object &request) {
   GetUnsigned(arguments, "variablesReference", 0);
   llvm::StringRef name = GetString(arguments, "name");
   lldb::SBFrame frame = g_dap.GetLLDBFrame(*arguments);
-  bool is_duplicated_variable_name = name.contains(" @");
+  lldb::SBValue variable = FindVariable(variablesReference, name);
+  std::string addr, size;
 
-  lldb::SBValue variable;
-  if (lldb::SBValueList *top_scope = GetTopLevelScope(variablesReference)) {
-// variablesReference is one of our scopes, not an actual variable it is
-// asking for a variable in locals or globals or registers
-int64_t end_idx = top_scope->GetSize();
-// Searching backward so that we choose the variable in closest scope
-// among variables of the same name.
-for (int64_t i = end_idx - 1; i >= 0; --i) {
-  lldb::SBValue curr_variable = top_scope->GetValueAtIndex(i);
-  std::string variable_name = CreateUniqueVariableNameForDisplay(
-  curr_variable, is_duplicated_variable_name);
-  if (variable_name == name) {
-variable = curr_variable;
-break;
-  }
-}
+  if (variable.IsValid()) {
+addr = llvm::utohexstr(variable.GetLoadAddress());
+size = llvm::utostr(variable.GetByteSize());
+  } else if (variablesReference == 0 && frame.IsValid()) {
+// Name might be an expression. In this case we assume that name is 
composed
+// of the number of bytes to watch and expression, separated by '@':
+// "${size}@${expression}"
+llvm::StringRef expr;
+std::tie(size, expr) = name.split('@');
+lldb::SBValue value = frame.EvaluateExpression(expr.data());
+if (value.GetError().Fail()) {
+  lldb::SBError error = value.GetError();
+  const char *error_cstr = error.GetCString();
+  body.try_emplace("dataId", nullptr);
+  body.try_emplace("description", error_cstr && error_cstr[0]
+  ? std::string(error_cstr)
+  : "evaluation failed");
+} else
+  addr = llvm::utohexstr(value.GetValueAsUnsigned());

ZequanWu wrote:

Sent a fix at https://github.com/llvm/llvm-project/pull/81680

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


[Lldb-commits] [lldb] [llvm] [lldb-dap] Add support for data breakpoint. (PR #81541)

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


@@ -561,6 +561,46 @@ void EventThreadFunction() {
   }
 }
 
+lldb::SBValue FindVariable(uint64_t variablesReference, llvm::StringRef name) {
+  lldb::SBValue variable;
+  if (lldb::SBValueList *top_scope = GetTopLevelScope(variablesReference)) {
+bool is_duplicated_variable_name = name.contains(" @");
+// variablesReference is one of our scopes, not an actual variable it is
+// asking for a variable in locals or globals or registers
+int64_t end_idx = top_scope->GetSize();
+// Searching backward so that we choose the variable in closest scope
+// among variables of the same name.
+for (int64_t i = end_idx - 1; i >= 0; --i) {
+  lldb::SBValue curr_variable = top_scope->GetValueAtIndex(i);
+  std::string variable_name = CreateUniqueVariableNameForDisplay(
+  curr_variable, is_duplicated_variable_name);

ZequanWu wrote:

This functions is shared between `request_setVariable` and 
`request_dataBreakpointInfo`, so it checks name that contains `foo @ 
main.cpp:12`. I tested this manually.

For anonymous unions inside struct, I also tested manually. If we watch either 
`c` or `d`, changes on either value stops the program as expected.
```
struct A {
  int a;
  int b;
  union {
char c;
int d;
  };
};
```

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


[Lldb-commits] [lldb] [llvm] [lldb-dap] Add support for data breakpoint. (PR #81541)

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


@@ -2697,58 +2737,41 @@ void request_dataBreakpointInfo(const 
llvm::json::Object &request) {
   GetUnsigned(arguments, "variablesReference", 0);
   llvm::StringRef name = GetString(arguments, "name");
   lldb::SBFrame frame = g_dap.GetLLDBFrame(*arguments);
-  bool is_duplicated_variable_name = name.contains(" @");
+  lldb::SBValue variable = FindVariable(variablesReference, name);
+  std::string addr, size;
 
-  lldb::SBValue variable;
-  if (lldb::SBValueList *top_scope = GetTopLevelScope(variablesReference)) {
-// variablesReference is one of our scopes, not an actual variable it is
-// asking for a variable in locals or globals or registers
-int64_t end_idx = top_scope->GetSize();
-// Searching backward so that we choose the variable in closest scope
-// among variables of the same name.
-for (int64_t i = end_idx - 1; i >= 0; --i) {
-  lldb::SBValue curr_variable = top_scope->GetValueAtIndex(i);
-  std::string variable_name = CreateUniqueVariableNameForDisplay(
-  curr_variable, is_duplicated_variable_name);
-  if (variable_name == name) {
-variable = curr_variable;
-break;
-  }
-}
+  if (variable.IsValid()) {
+addr = llvm::utohexstr(variable.GetLoadAddress());
+size = llvm::utostr(variable.GetByteSize());

ZequanWu wrote:

Sent a fix at https://github.com/llvm/llvm-project/pull/81680

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


[Lldb-commits] [lldb] [llvm] [lldb-dap] Add support for data breakpoint. (PR #81541)

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


@@ -2697,58 +2737,41 @@ void request_dataBreakpointInfo(const 
llvm::json::Object &request) {
   GetUnsigned(arguments, "variablesReference", 0);
   llvm::StringRef name = GetString(arguments, "name");
   lldb::SBFrame frame = g_dap.GetLLDBFrame(*arguments);
-  bool is_duplicated_variable_name = name.contains(" @");
+  lldb::SBValue variable = FindVariable(variablesReference, name);
+  std::string addr, size;
 
-  lldb::SBValue variable;
-  if (lldb::SBValueList *top_scope = GetTopLevelScope(variablesReference)) {
-// variablesReference is one of our scopes, not an actual variable it is
-// asking for a variable in locals or globals or registers
-int64_t end_idx = top_scope->GetSize();
-// Searching backward so that we choose the variable in closest scope
-// among variables of the same name.
-for (int64_t i = end_idx - 1; i >= 0; --i) {
-  lldb::SBValue curr_variable = top_scope->GetValueAtIndex(i);
-  std::string variable_name = CreateUniqueVariableNameForDisplay(
-  curr_variable, is_duplicated_variable_name);
-  if (variable_name == name) {
-variable = curr_variable;
-break;
-  }
-}
+  if (variable.IsValid()) {
+addr = llvm::utohexstr(variable.GetLoadAddress());
+size = llvm::utostr(variable.GetByteSize());
+  } else if (variablesReference == 0 && frame.IsValid()) {
+// Name might be an expression. In this case we assume that name is 
composed
+// of the number of bytes to watch and expression, separated by '@':
+// "${size}@${expression}"

ZequanWu wrote:

I'm making this up, since the spec doesn't say how to specify the number of 
bytes to watch. Do you have better idea?

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


[Lldb-commits] [lldb] [llvm] [lldb-dap] Add support for data breakpoint. (PR #81541)

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


@@ -2697,58 +2737,41 @@ void request_dataBreakpointInfo(const 
llvm::json::Object &request) {
   GetUnsigned(arguments, "variablesReference", 0);
   llvm::StringRef name = GetString(arguments, "name");
   lldb::SBFrame frame = g_dap.GetLLDBFrame(*arguments);
-  bool is_duplicated_variable_name = name.contains(" @");
+  lldb::SBValue variable = FindVariable(variablesReference, name);
+  std::string addr, size;
 
-  lldb::SBValue variable;
-  if (lldb::SBValueList *top_scope = GetTopLevelScope(variablesReference)) {
-// variablesReference is one of our scopes, not an actual variable it is
-// asking for a variable in locals or globals or registers
-int64_t end_idx = top_scope->GetSize();
-// Searching backward so that we choose the variable in closest scope
-// among variables of the same name.
-for (int64_t i = end_idx - 1; i >= 0; --i) {
-  lldb::SBValue curr_variable = top_scope->GetValueAtIndex(i);
-  std::string variable_name = CreateUniqueVariableNameForDisplay(
-  curr_variable, is_duplicated_variable_name);
-  if (variable_name == name) {
-variable = curr_variable;
-break;
-  }
-}
+  if (variable.IsValid()) {
+addr = llvm::utohexstr(variable.GetLoadAddress());

ZequanWu wrote:

Sent a fix at https://github.com/llvm/llvm-project/pull/81680

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


[Lldb-commits] [lldb] [lldb] Add comment on cross printing of summary/value (PR #81681)

2024-02-13 Thread Dave Lee via lldb-commits

https://github.com/kastiglione created 
https://github.com/llvm/llvm-project/pull/81681

None

>From 35a48898e20ba45db829dbf137d88a8417dd4d7c Mon Sep 17 00:00:00 2001
From: Dave Lee 
Date: Tue, 13 Feb 2024 14:43:33 -0800
Subject: [PATCH] [lldb] Add comment on cross printing of summary/value

---
 lldb/source/Core/ValueObject.cpp | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lldb/source/Core/ValueObject.cpp b/lldb/source/Core/ValueObject.cpp
index e80042826f7d64..2a2bb128d33c57 100644
--- a/lldb/source/Core/ValueObject.cpp
+++ b/lldb/source/Core/ValueObject.cpp
@@ -1312,6 +1312,7 @@ bool ValueObject::DumpPrintableRepresentation(
   break;
 }
 
+// Try summary if no value, and value if no summary.
 if (str.empty()) {
   if (val_obj_display == eValueObjectRepresentationStyleValue)
 str = GetSummaryAsCString();

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


[Lldb-commits] [lldb] [lldb] Add comment on cross printing of summary/value (PR #81681)

2024-02-13 Thread via lldb-commits

llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: Dave Lee (kastiglione)


Changes



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


1 Files Affected:

- (modified) lldb/source/Core/ValueObject.cpp (+1) 


``diff
diff --git a/lldb/source/Core/ValueObject.cpp b/lldb/source/Core/ValueObject.cpp
index e80042826f7d64..2a2bb128d33c57 100644
--- a/lldb/source/Core/ValueObject.cpp
+++ b/lldb/source/Core/ValueObject.cpp
@@ -1312,6 +1312,7 @@ bool ValueObject::DumpPrintableRepresentation(
   break;
 }
 
+// Try summary if no value, and value if no summary.
 if (str.empty()) {
   if (val_obj_display == eValueObjectRepresentationStyleValue)
 str = GetSummaryAsCString();

``




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


[Lldb-commits] [lldb] [lldb] Add comment on cross printing of summary/value (PR #81681)

2024-02-13 Thread Dave Lee via lldb-commits

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


[Lldb-commits] [lldb] [lldb] Add comment on cross printing of summary/value (PR #81681)

2024-02-13 Thread Dave Lee via lldb-commits

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


[Lldb-commits] [lldb] [lldb] Add comment on cross printing of summary/value (PR #81681)

2024-02-13 Thread Dave Lee via lldb-commits

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


[Lldb-commits] [lldb] [lldb] Add comment on cross printing of summary/value (PR #81681)

2024-02-13 Thread Dave Lee via lldb-commits

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


[Lldb-commits] [lldb] [lldb] Add comment on cross printing of summary/value (PR #81681)

2024-02-13 Thread via lldb-commits

jimingham wrote:

That did deserve a comment.  To be pedantic, in the second branch you try 
`value/location` if no summary, but what you have is fine.

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


[Lldb-commits] [lldb] [lldb] Add comment on cross printing of summary/value (PR #81681)

2024-02-13 Thread via lldb-commits

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


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


[Lldb-commits] [lldb] [lldb] Detect a Darwin kernel issue and work around it (PR #81573)

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

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

>From 177a242271c99ed3fec4c5f211cd6d07c6d88488 Mon Sep 17 00:00:00 2001
From: Jason Molenda 
Date: Mon, 12 Feb 2024 21:34:13 -0800
Subject: [PATCH 1/2] [lldb] Detect a Darwin kernel issue and work around it

On arm64 machines, when there is a hardware breakpoint or watchpoint
set, and lldb has instruction-stepped a thread, and then done a
Process::Resume, we will sometimes receive an extra "instruction
step completed" mach exception and the pc has not advanced.  From
a user's perspective, they hit Continue and lldb stops again at the
same spot.  From the testsuite's perspective, this has been a
constant source of testsuite failures for any test using hardware
watchpoints and breakpoints, the arm64 CI bots seem especially good
at hitting this issue.

Jim and I have been slowly looking at this for a few months now,
and finally I decided to try to detect this situation in lldb and
silently resume the process again when it happens.

We were already detecting this "got an insn-step finished mach
exception but this thread was not instruction stepping" combination
in StopInfoMachException where we take the mach exception and create
a StopInfo object for it.  We had a lot of logging we used to
understand the failure as it was hit on the bots in assert builds.

This patch adds a new case to `Thread::GetPrivateStopInfo()` to
call the StopInfo's (new) `IsContinueInterrupted()` method.  In
StopInfoMachException, where we previously had logging for assert
builds, I now note it in an ivar, and when `Thread::GetPrivateStopInfo()`
asks if this has happened, we check all of the combination of events
that this comes up:  We have a hardware breakpoint or watchpoint,
we were not instruction stepping this thread but got an insn-step
mach exception, the pc is the same as the previous stop's pc.  And
in that case, `Thread::GetPrivateStopInfo()` returns no StopInfo
-- indicating that this thread would like to resume execution.

The `Thread` object has two StackFrameLists, `m_curr_frames_sp` and
`m_prev_frames_sp`.  When a thread resumes execution, we move
`m_curr_frames_sp` in to `m_prev_frames_sp` and when it stops
executing, w euse `m_prev_frames_sp` to seed the new `m_curr_frames_sp`
if most of the stack is the same as before.

In this same location, I now save the Thread's RegisterContext::GetPC
into an ivar, `m_prev_framezero_pc`.  StopInfoMachException needs
this information to check all of the conditions I outlined above
for `IsContinueInterrupted`.

This has passed exhaustive testing and we do not have any testsuite
failures for hardware watchpoints and breakpoints due to this kernel
bug with the patch in place.  In focusing on these tests for thousands
of runs, I have found two other uncommon race conditions for the
TestConcurrent* tests on arm64.  TestConcurrentManyBreakpoints.py
(which uses no hardware watchpoint/breakpoints) will sometimes only
have 99 breakpoints when it expects 100, and any of the concurrent
tests using the shared harness (I've seen it in
TestConcurrentWatchBreakDelay.py, TestConcurrentTwoBreakpointsOneSignal.py,
TestConcurrentSignalDelayWatch.py) can fail when the test harness
checks that there is only one thread still running at the end, and
it finds two -- one of them under pthread_exit / pthread_terminate.
Both of these failures happen on github main without my changes,
and with my changes - they are unrelated race conditions in these
tests, and I'm sure I'll be looking into them at some point if they
hit the CI bots with frequency.  On my computer, these are in the
0.3-0.5% of the time class.  But the CI bots do have different
timing.
---
 lldb/include/lldb/Target/StopInfo.h   |  5 ++
 lldb/include/lldb/Target/Thread.h |  7 ++
 .../Process/Utility/StopInfoMachException.cpp | 83 +--
 .../Process/Utility/StopInfoMachException.h   | 11 ++-
 lldb/source/Target/Thread.cpp | 19 -
 5 files changed, 94 insertions(+), 31 deletions(-)

diff --git a/lldb/include/lldb/Target/StopInfo.h 
b/lldb/include/lldb/Target/StopInfo.h
index 305fc5d0e0fbe5..1eb409fdefe392 100644
--- a/lldb/include/lldb/Target/StopInfo.h
+++ b/lldb/include/lldb/Target/StopInfo.h
@@ -79,6 +79,11 @@ class StopInfo : public 
std::enable_shared_from_this {
 
   virtual bool IsValidForOperatingSystemThread(Thread &thread) { return true; }
 
+  /// A Continue operation can result in a false stop event
+  /// before any execution has happened in certain cases on Darwin.
+  /// We need to silently continue again time.
+  virtual bool IsContinueInterrupted(Thread &thread) { return false; }
+
   // Sometimes the thread plan logic will know that it wants a given stop to
   // stop or not, regardless of what the ordinary logic for that StopInfo would
   // dictate.  The main example of this is the ThreadPlanCallFunction, which
diff --git a/lldb/include/lldb/Target/Thread.h 
b/lldb/include/lldb/Target/Th

[Lldb-commits] [lldb] [llvm] [lldb-dap] Add support for data breakpoint. (PR #81541)

2024-02-13 Thread Nico Weber via lldb-commits


@@ -52,5 +52,6 @@ executable("lldb-dap") {
 "RunInTerminal.cpp",
 "SourceBreakpoint.cpp",
 "lldb-dap.cpp",
+"Watchpoint.cpp"

nico wrote:

Please don't update gn build files if you don't use the gn build. There's a bot 
that is able to do it most of the time, and it doesn't make mistakes like 
forgetting the trailing comma here :)

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


  1   2   >