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