[Lldb-commits] [lldb] [lldb][X86] Fix setting target features in ClangExpressionParser (PR #82364)
https://github.com/kovdan01 created https://github.com/llvm/llvm-project/pull/82364 Currently, for x86 and x86_64 triples, "+sse" and "+sse2" are appended to `Features` vector of `TargetOptions` unconditionally. This vector is later reset in `TargetInfo::CreateTargetInfo` and filled using info from `FeaturesAsWritten` vector, so previous modifications of the `Features` vector have no effect. For x86_64 triple, we append "sse2" unconditionally in `X86TargetInfo::initFeatureMap`, so despite the `Features` vector reset, we still have the desired sse features enabled. The corresponding code in `X86TargetInfo::initFeatureMap` is marked as FIXME, so we should not probably rely on it and should set desired features properly in `ClangExpressionParser`. This patch changes the vector the features are appended to from `Features` to `FeaturesAsWritten`. It's not reset later and is used to compute resulting `Features` vector. >From 0b7e2f905687995a710dad6fc8da5bea602e39f1 Mon Sep 17 00:00:00 2001 From: Daniil Kovalev Date: Mon, 19 Feb 2024 17:19:36 +0300 Subject: [PATCH] [lldb][X86] Fix settings target features in ClangExpressionParser Currently, for x86 and x86_64 triples, "+sse" and "+sse2" are appended to `Features` vector of `TargetOptions` unconditionally. This vector is later reset in `TargetInfo::CreateTargetInfo` and filled using info from `FeaturesAsWritten` vector, so previous modifications of the `Features` vector have no effect. For x86_64 triple, we append "sse2" unconditionally in `X86TargetInfo::initFeatureMap`, so despite the `Features` vector reset, we still have the desired sse features enabled. The corresponding code in `X86TargetInfo::initFeatureMap` is marked as FIXME, so we should not probably rely on it and should set desired features properly in `ClangExpressionParser`. This patch changes the vector the features are appended to from `Features` to `FeaturesAsWritten`. It's not reset later and is used to compute resulting `Features` vector. --- .../Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp index 574d661e2a215e..822d286cd6c3c4 100644 --- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp +++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp @@ -445,8 +445,8 @@ ClangExpressionParser::ClangExpressionParser( // Supported subsets of x86 if (target_machine == llvm::Triple::x86 || target_machine == llvm::Triple::x86_64) { -m_compiler->getTargetOpts().Features.push_back("+sse"); -m_compiler->getTargetOpts().Features.push_back("+sse2"); +m_compiler->getTargetOpts().FeaturesAsWritten.push_back("+sse"); +m_compiler->getTargetOpts().FeaturesAsWritten.push_back("+sse2"); } // Set the target CPU to generate code for. This will be empty for any CPU ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][X86] Fix setting target features in ClangExpressionParser (PR #82364)
https://github.com/kovdan01 ready_for_review https://github.com/llvm/llvm-project/pull/82364 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][X86] Fix setting target features in ClangExpressionParser (PR #82364)
llvmbot wrote: @llvm/pr-subscribers-lldb Author: Daniil Kovalev (kovdan01) Changes Currently, for x86 and x86_64 triples, "+sse" and "+sse2" are appended to `Features` vector of `TargetOptions` unconditionally. This vector is later reset in `TargetInfo::CreateTargetInfo` and filled using info from `FeaturesAsWritten` vector, so previous modifications of the `Features` vector have no effect. For x86_64 triple, we append "sse2" unconditionally in `X86TargetInfo::initFeatureMap`, so despite the `Features` vector reset, we still have the desired sse features enabled. The corresponding code in `X86TargetInfo::initFeatureMap` is marked as FIXME, so we should not probably rely on it and should set desired features properly in `ClangExpressionParser`. This patch changes the vector the features are appended to from `Features` to `FeaturesAsWritten`. It's not reset later and is used to compute resulting `Features` vector. --- Full diff: https://github.com/llvm/llvm-project/pull/82364.diff 1 Files Affected: - (modified) lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp (+2-2) ``diff diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp index 574d661e2a215e..822d286cd6c3c4 100644 --- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp +++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp @@ -445,8 +445,8 @@ ClangExpressionParser::ClangExpressionParser( // Supported subsets of x86 if (target_machine == llvm::Triple::x86 || target_machine == llvm::Triple::x86_64) { -m_compiler->getTargetOpts().Features.push_back("+sse"); -m_compiler->getTargetOpts().Features.push_back("+sse2"); +m_compiler->getTargetOpts().FeaturesAsWritten.push_back("+sse"); +m_compiler->getTargetOpts().FeaturesAsWritten.push_back("+sse2"); } // Set the target CPU to generate code for. This will be empty for any CPU `` https://github.com/llvm/llvm-project/pull/82364 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] ae8facc - [lldb-dap] Do not write over the existing error if launchCommands fail during debugger launch. (#82051)
Author: John Harrison Date: 2024-02-20T09:35:15-08:00 New Revision: ae8facc1f8e8a2d04b5a3519fc1901db110fe452 URL: https://github.com/llvm/llvm-project/commit/ae8facc1f8e8a2d04b5a3519fc1901db110fe452 DIFF: https://github.com/llvm/llvm-project/commit/ae8facc1f8e8a2d04b5a3519fc1901db110fe452.diff LOG: [lldb-dap] Do not write over the existing error if launchCommands fail during debugger launch. (#82051) This fixes an issue where the error is lost if a command while executing `launchCommands` when launching the debugger. This should fix #82048 Added: Modified: lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py lldb/tools/lldb-dap/lldb-dap.cpp Removed: diff --git a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py index 73bd037fd328cb..288cc8cf9a48c8 100644 --- a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py +++ b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py @@ -122,6 +122,8 @@ def verify_commands(self, flavor, output, commands): for cmd in commands: found = False for line in lines: +if len(cmd) > 0 and (cmd[0] == "!" or cmd[0] == "?"): +cmd = cmd[1:] if line.startswith(prefix) and cmd in line: found = True break diff --git a/lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py b/lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py index 829fb3b7ba6a4c..04d741c1d47201 100644 --- a/lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py +++ b/lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py @@ -2,7 +2,6 @@ Test lldb-dap setBreakpoints request """ - import dap_server from lldbsuite.test.decorators import * from lldbsuite.test.lldbtest import * @@ -398,7 +397,7 @@ def test_extra_launch_commands(self): # Verify all "preRunCommands" were found in console output self.verify_commands("preRunCommands", output, preRunCommands) -# Verify all "launchCommands" were founc in console output +# Verify all "launchCommands" were found in console output # After execution, program should launch self.verify_commands("launchCommands", output, launchCommands) # Verify the "stopCommands" here @@ -420,6 +419,47 @@ def test_extra_launch_commands(self): output = self.get_console(timeout=1.0) self.verify_commands("exitCommands", output, exitCommands) +@skipIfWindows +@skipIfRemote +def test_failing_launch_commands(self): +""" +Tests "launchCommands" failures prevents a launch. +""" +self.build_and_create_debug_adaptor() +program = self.getBuildArtifact("a.out") + +# Run an invalid launch command, in this case a bad path. +launchCommands = ['!target create "/bad/path%s"' % (program)] + +initCommands = ["target list", "platform list"] +preRunCommands = ["image list a.out", "image dump sections a.out"] +response = self.launch( +program, +initCommands=initCommands, +preRunCommands=preRunCommands, +launchCommands=launchCommands, +expectFailure=True, +) + +self.assertFalse(response["success"]) +self.assertRegex( +response["message"], +r"Failed to run launch commands\. See the Debug Console for more details", +) + +# Get output from the console. This should contain both the +# "initCommands" and the "preRunCommands". +output = self.get_console() +# Verify all "initCommands" were found in console output +self.verify_commands("initCommands", output, initCommands) +# Verify all "preRunCommands" were found in console output +self.verify_commands("preRunCommands", output, preRunCommands) + +# Verify all "launchCommands" were founc in console output +# The launch should fail due to the invalid command. +self.verify_commands("launchCommands", output, launchCommands) +self.assertRegex(output, r"unable to find executable for '/bad/path/") + @skipIfWindows @skipIfNetBSD # Hangs on NetBSD as well @skipIf(archs=["arm", "aarch64"], oslist=["linux"]) diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp index 67022347e6d624..78b0b4078706aa 100644 --- a/lldb/tools/lldb-dap/lldb-dap.cpp +++ b/lldb/tools/lldb-dap/lldb-dap.cpp @@ -1779,8 +1779,10 @@ lldb::SBError LaunchProcess(const llvm::json::Object &request) { // Set the launch info so that run commands can access the configured // launch details. g_dap.target.SetLaunchInfo(launch_info); -if (ll
[Lldb-commits] [lldb] [lldb-dap] Do not write over the existing error if launchCommands fail during debugger launch. (PR #82051)
https://github.com/ashgti closed https://github.com/llvm/llvm-project/pull/82051 ___ 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,23 @@ 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; +// Include child process PID/TID for forks. +// Client expects " " format for parsing. +if (stop_info.reason == eStopReasonFork || +stop_info.reason == eStopReasonVFork) { + m_stop_description = std::to_string(stop_info.details.fork.child_pid); + m_stop_description += " "; + m_stop_description += std::to_string(stop_info.details.fork.child_tid); +} clayborg wrote: We should be doing this work in: ``` void NativeThreadLinux::SetStoppedByFork(bool is_vfork, lldb::pid_t 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)
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)
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)
@@ -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. clayborg wrote: ``` // Number of fork() or vfork() operations being handled. ``` 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] Support DW_OP_WASM_location in DWARFExpression (PR #78977)
paolosevMSFT wrote: Hi @JDevlieghere, could you take a look at the latest changes and unit tests? https://github.com/llvm/llvm-project/pull/78977 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Implement WebAssembly debugging (PR #77949)
https://github.com/paolosevMSFT updated https://github.com/llvm/llvm-project/pull/77949 >From 30d932bb0988e1c78c3e023be2270259df5e6664 Mon Sep 17 00:00:00 2001 From: Paolo Severini Date: Fri, 12 Jan 2024 09:10:59 -0800 Subject: [PATCH 1/5] Add support for source-level debugging of WebAssembly code --- lldb/include/lldb/Target/Process.h| 40 +++ lldb/source/Core/Value.cpp| 2 +- lldb/source/Expression/DWARFExpression.cpp| 41 +++ .../source/Interpreter/CommandInterpreter.cpp | 18 ++ lldb/source/Plugins/Process/CMakeLists.txt| 1 + .../Process/gdb-remote/ProcessGDBRemote.cpp | 7 +- .../Process/gdb-remote/ProcessGDBRemote.h | 2 + .../Plugins/Process/wasm/CMakeLists.txt | 12 + .../Plugins/Process/wasm/ProcessWasm.cpp | 291 ++ .../source/Plugins/Process/wasm/ProcessWasm.h | 129 .../Plugins/Process/wasm/ThreadWasm.cpp | 55 lldb/source/Plugins/Process/wasm/ThreadWasm.h | 44 +++ .../Plugins/Process/wasm/UnwindWasm.cpp | 76 + lldb/source/Plugins/Process/wasm/UnwindWasm.h | 55 .../Process/wasm/wasmRegisterContext.cpp | 108 +++ .../Process/wasm/wasmRegisterContext.h| 75 + 16 files changed, 954 insertions(+), 2 deletions(-) create mode 100644 lldb/source/Plugins/Process/wasm/CMakeLists.txt create mode 100644 lldb/source/Plugins/Process/wasm/ProcessWasm.cpp create mode 100644 lldb/source/Plugins/Process/wasm/ProcessWasm.h create mode 100644 lldb/source/Plugins/Process/wasm/ThreadWasm.cpp create mode 100644 lldb/source/Plugins/Process/wasm/ThreadWasm.h create mode 100644 lldb/source/Plugins/Process/wasm/UnwindWasm.cpp create mode 100644 lldb/source/Plugins/Process/wasm/UnwindWasm.h create mode 100644 lldb/source/Plugins/Process/wasm/wasmRegisterContext.cpp create mode 100644 lldb/source/Plugins/Process/wasm/wasmRegisterContext.h diff --git a/lldb/include/lldb/Target/Process.h b/lldb/include/lldb/Target/Process.h index 24c599e044c78f..587ae085b479b7 100644 --- a/lldb/include/lldb/Target/Process.h +++ b/lldb/include/lldb/Target/Process.h @@ -1548,6 +1548,46 @@ class Process : public std::enable_shared_from_this, virtual size_t ReadMemory(lldb::addr_t vm_addr, void *buf, size_t size, Status &error); + /// Read of memory from a process. + /// + /// This function will read memory from the current process's address space + /// and remove any traps that may have been inserted into the memory. + /// + /// This overloads accepts an ExecutionContext as additional argument. By + /// default, it calls the previous overload without the ExecutionContext + /// argument, but it can be overridden by Process subclasses. + /// + /// \param[in] vm_addr + /// A virtual load address that indicates where to start reading + /// memory from. + /// + /// \param[out] buf + /// A byte buffer that is at least \a size bytes long that + /// will receive the memory bytes. + /// + /// \param[in] size + /// The number of bytes to read. + /// + /// \param[in] exe_ctx + ///The current execution context, if available. + /// + /// \param[out] error + /// An error that indicates the success or failure of this + /// operation. If error indicates success (error.Success()), + /// then the value returned can be trusted, otherwise zero + /// will be returned. + /// + /// \return + /// The number of bytes that were actually read into \a buf. If + /// the returned number is greater than zero, yet less than \a + /// size, then this function will get called again with \a + /// vm_addr, \a buf, and \a size updated appropriately. Zero is + /// returned in the case of an error. + virtual size_t ReadMemory(lldb::addr_t vm_addr, void *buf, size_t size, +ExecutionContext *exe_ctx, Status &error) { +return ReadMemory(vm_addr, buf, size, error); + } + /// Read of memory from a process. /// /// This function has the same semantics of ReadMemory except that it diff --git a/lldb/source/Core/Value.cpp b/lldb/source/Core/Value.cpp index 995cc934c82044..47a5fdee773886 100644 --- a/lldb/source/Core/Value.cpp +++ b/lldb/source/Core/Value.cpp @@ -552,7 +552,7 @@ Status Value::GetValueAsData(ExecutionContext *exe_ctx, DataExtractor &data, if (process) { const size_t bytes_read = - process->ReadMemory(address, dst, byte_size, error); + process->ReadMemory(address, dst, byte_size, exe_ctx, error); if (bytes_read != byte_size) error.SetErrorStringWithFormat( "read memory from 0x%" PRIx64 " failed (%u of %u bytes read)", diff --git a/lldb/source/Expression/DWARFExpression.cpp b/lldb/source/Expression/DWARFExpression.cpp index fe4928d4f43a43..ca24611724dc7c 100644 --- a/lldb/source/Expression/DWARFExpression.cpp +++ b/lldb/source/Expression/DWARFExpression.c
[Lldb-commits] [lldb] [lldb] Standardize command option parsing error messages (PR #82273)
bulbazord wrote: > This LGTM! > > I don't think I can see far enough ahead on what you are planning here, but I > was just wondering if the ultimate goal is to have the `option_arg.getAsT` > return an `Expected`. In this case, wouldn't all these arguments (short > option, long option, additional context) have to be part of the interface of > `getAsT`? I suspect this is not your goal, but I can't see a way around that The ultimate goal is to have option parsing be much more streamlined and automated. The majority of option parsing is just taking some primitive value and trying to grab its value from a string. There are places where we need additional context (e.g. in this PR, there's a place where we want to select a thread ID so there's more validation needed). I want to be able to write those pretty naturally too. The first step in my plan is to centralize how we create error messages. The majority of parsing errors will be "I couldn't turn this into a boolean/integer/address/other", so I'm going to create something that is better than what we have today. The step after that will be to revamp the parsing step. Ideally instead of writing `OptionArgParser::ToBoolean` the way we do today, there will be something like `llvm::Expected value_or_error = ParseAsBool(option);` where `option` is the actual OptionDefinition. I'm not tied to these abstractions, but I am committed to the path of making this easier to write and maintain. https://github.com/llvm/llvm-project/pull/82273 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Add the RegisterCompleter to eArgTypeRegisterName in g_argument_table (PR #82428)
https://github.com/jimingham created https://github.com/llvm/llvm-project/pull/82428 This is a follow-on to: https://github.com/llvm/llvm-project/pull/82085 The completer for register names was missing from the argument table. I somehow missed that the only register completer test was x86_64, so that test broke. I added the completer in to the right slot in the argument table, and added a small completions test that just uses the alias register names. If we end up having a platform that doesn't define register names, we'll have to skip this test there, but it should add a sniff test for register completion that will run most everywhere. >From bec7ab45ffbad48b8794cf425a07148a0490b008 Mon Sep 17 00:00:00 2001 From: Jim Ingham Date: Tue, 20 Feb 2024 09:59:43 -0800 Subject: [PATCH 1/2] Revert "Revert "Centralize the handling of completion for simple argument lists. (#82085)"" This reverts commit 9ed8b272c30a01c450616f0ed8b2373d5d618ebb. --- lldb/include/lldb/Interpreter/CommandObject.h | 9 ++- .../Interpreter/CommandOptionArgumentTable.h | 21 +++--- lldb/include/lldb/lldb-enumerations.h | 3 + .../source/Commands/CommandObjectCommands.cpp | 14 .../Commands/CommandObjectDWIMPrint.cpp | 6 -- lldb/source/Commands/CommandObjectDWIMPrint.h | 4 - lldb/source/Commands/CommandObjectFrame.cpp | 19 - .../source/Commands/CommandObjectPlatform.cpp | 75 +-- lldb/source/Commands/CommandObjectPlugin.cpp | 7 -- lldb/source/Commands/CommandObjectProcess.cpp | 19 + .../source/Commands/CommandObjectRegister.cpp | 7 +- lldb/source/Commands/CommandObjectSession.cpp | 7 -- .../source/Commands/CommandObjectSettings.cpp | 8 -- lldb/source/Commands/CommandObjectTarget.cpp | 29 +-- lldb/source/Commands/CommandObjectThread.cpp | 13 +--- lldb/source/Commands/CommandObjectType.cpp| 32 .../Commands/CommandObjectWatchpoint.cpp | 10 --- lldb/source/Interpreter/CommandObject.cpp | 37 + lldb/test/API/commands/help/TestHelp.py | 2 +- 19 files changed, 86 insertions(+), 236 deletions(-) diff --git a/lldb/include/lldb/Interpreter/CommandObject.h b/lldb/include/lldb/Interpreter/CommandObject.h index b99de56f534469..a326c6dc38a37a 100644 --- a/lldb/include/lldb/Interpreter/CommandObject.h +++ b/lldb/include/lldb/Interpreter/CommandObject.h @@ -242,6 +242,13 @@ class CommandObject : public std::enable_shared_from_this { ///The completion request that needs to be answered. virtual void HandleCompletion(CompletionRequest &request); + /// The default version handles argument definitions that have only one + /// argument type, and use one of the argument types that have an entry in + /// the CommonCompletions. Override this if you have a more complex + /// argument setup. + /// FIXME: we should be able to extend this to more complex argument + /// definitions provided we have completers for all the argument types. + /// /// The input array contains a parsed version of the line. /// /// We've constructed the map of options and their arguments as well if that @@ -251,7 +258,7 @@ class CommandObject : public std::enable_shared_from_this { ///The completion request that needs to be answered. virtual void HandleArgumentCompletion(CompletionRequest &request, - OptionElementVector &opt_element_vector) {} + OptionElementVector &opt_element_vector); bool HelpTextContainsWord(llvm::StringRef search_word, bool search_short_help = true, diff --git a/lldb/include/lldb/Interpreter/CommandOptionArgumentTable.h b/lldb/include/lldb/Interpreter/CommandOptionArgumentTable.h index d0cf54c31ca73f..ab5eff699b4cf0 100644 --- a/lldb/include/lldb/Interpreter/CommandOptionArgumentTable.h +++ b/lldb/include/lldb/Interpreter/CommandOptionArgumentTable.h @@ -243,7 +243,7 @@ static constexpr CommandObject::ArgumentTableEntry g_argument_table[] = { { lldb::eArgTypeLogCategory, "log-category", lldb::CompletionType::eNoCompletion, {}, { nullptr, false }, "The name of a category within a log channel, e.g. all (try \"log list\" to see a list of all channels and their categories." }, { lldb::eArgTypeLogChannel, "log-channel", lldb::CompletionType::eNoCompletion, {}, { nullptr, false }, "The name of a log channel, e.g. process.gdb-remote (try \"log list\" to see a list of all channels and their categories)." }, { lldb::eArgTypeMethod, "method", lldb::CompletionType::eNoCompletion, {}, { nullptr, false }, "A C++ method name." }, -{ lldb::eArgTypeName, "name", lldb::eTypeCategoryNameCompletion, {}, { nullptr, false }, "Help text goes here." }, +{ lldb::eArgTypeName, "name", lldb::eTypeCategoryNameCompletion, {}, { nullptr, false }, "The name of a type category." }, { lldb::eArgTypeNewPathPrefix, "new-path-prefix", lldb::CompletionType::eNoCompletion, {}, { nullptr, false }, "Help text goes
[Lldb-commits] [lldb] Add the RegisterCompleter to eArgTypeRegisterName in g_argument_table (PR #82428)
llvmbot wrote: @llvm/pr-subscribers-lldb Author: None (jimingham) Changes This is a follow-on to: https://github.com/llvm/llvm-project/pull/82085 The completer for register names was missing from the argument table. I somehow missed that the only register completer test was x86_64, so that test broke. I added the completer in to the right slot in the argument table, and added a small completions test that just uses the alias register names. If we end up having a platform that doesn't define register names, we'll have to skip this test there, but it should add a sniff test for register completion that will run most everywhere. --- Patch is 41.58 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/82428.diff 20 Files Affected: - (modified) lldb/include/lldb/Interpreter/CommandObject.h (+8-1) - (modified) lldb/include/lldb/Interpreter/CommandOptionArgumentTable.h (+13-10) - (modified) lldb/include/lldb/lldb-enumerations.h (+3) - (modified) lldb/source/Commands/CommandObjectCommands.cpp (-14) - (modified) lldb/source/Commands/CommandObjectDWIMPrint.cpp (-6) - (modified) lldb/source/Commands/CommandObjectDWIMPrint.h (-4) - (modified) lldb/source/Commands/CommandObjectFrame.cpp (-19) - (modified) lldb/source/Commands/CommandObjectPlatform.cpp (+18-57) - (modified) lldb/source/Commands/CommandObjectPlugin.cpp (-7) - (modified) lldb/source/Commands/CommandObjectProcess.cpp (+1-18) - (modified) lldb/source/Commands/CommandObjectRegister.cpp (+2-5) - (modified) lldb/source/Commands/CommandObjectSession.cpp (-7) - (modified) lldb/source/Commands/CommandObjectSettings.cpp (-8) - (modified) lldb/source/Commands/CommandObjectTarget.cpp (+3-26) - (modified) lldb/source/Commands/CommandObjectThread.cpp (+1-12) - (modified) lldb/source/Commands/CommandObjectType.cpp (-32) - (modified) lldb/source/Commands/CommandObjectWatchpoint.cpp (-10) - (modified) lldb/source/Interpreter/CommandObject.cpp (+37) - (modified) lldb/test/API/commands/help/TestHelp.py (+1-1) - (modified) lldb/test/API/functionalities/completion/TestCompletion.py (+21) ``diff diff --git a/lldb/include/lldb/Interpreter/CommandObject.h b/lldb/include/lldb/Interpreter/CommandObject.h index b99de56f534469..a326c6dc38a37a 100644 --- a/lldb/include/lldb/Interpreter/CommandObject.h +++ b/lldb/include/lldb/Interpreter/CommandObject.h @@ -242,6 +242,13 @@ class CommandObject : public std::enable_shared_from_this { ///The completion request that needs to be answered. virtual void HandleCompletion(CompletionRequest &request); + /// The default version handles argument definitions that have only one + /// argument type, and use one of the argument types that have an entry in + /// the CommonCompletions. Override this if you have a more complex + /// argument setup. + /// FIXME: we should be able to extend this to more complex argument + /// definitions provided we have completers for all the argument types. + /// /// The input array contains a parsed version of the line. /// /// We've constructed the map of options and their arguments as well if that @@ -251,7 +258,7 @@ class CommandObject : public std::enable_shared_from_this { ///The completion request that needs to be answered. virtual void HandleArgumentCompletion(CompletionRequest &request, - OptionElementVector &opt_element_vector) {} + OptionElementVector &opt_element_vector); bool HelpTextContainsWord(llvm::StringRef search_word, bool search_short_help = true, diff --git a/lldb/include/lldb/Interpreter/CommandOptionArgumentTable.h b/lldb/include/lldb/Interpreter/CommandOptionArgumentTable.h index d0cf54c31ca73f..9248e2ac814461 100644 --- a/lldb/include/lldb/Interpreter/CommandOptionArgumentTable.h +++ b/lldb/include/lldb/Interpreter/CommandOptionArgumentTable.h @@ -243,7 +243,7 @@ static constexpr CommandObject::ArgumentTableEntry g_argument_table[] = { { lldb::eArgTypeLogCategory, "log-category", lldb::CompletionType::eNoCompletion, {}, { nullptr, false }, "The name of a category within a log channel, e.g. all (try \"log list\" to see a list of all channels and their categories." }, { lldb::eArgTypeLogChannel, "log-channel", lldb::CompletionType::eNoCompletion, {}, { nullptr, false }, "The name of a log channel, e.g. process.gdb-remote (try \"log list\" to see a list of all channels and their categories)." }, { lldb::eArgTypeMethod, "method", lldb::CompletionType::eNoCompletion, {}, { nullptr, false }, "A C++ method name." }, -{ lldb::eArgTypeName, "name", lldb::eTypeCategoryNameCompletion, {}, { nullptr, false }, "Help text goes here." }, +{ lldb::eArgTypeName, "name", lldb::eTypeCategoryNameCompletion, {}, { nullptr, false }, "The name of a type category." }, { lldb::eArgTypeNewPathPrefix, "new-path-prefix", lldb::CompletionType::eNoCompletion, {}
[Lldb-commits] [lldb] [lldb] Standardize command option parsing error messages (PR #82273)
felipepiovezan wrote: > > This LGTM! > > I don't think I can see far enough ahead on what you are planning here, but > > I was just wondering if the ultimate goal is to have the > > `option_arg.getAsT` return an `Expected`. In this case, wouldn't all > > these arguments (short option, long option, additional context) have to be > > part of the interface of `getAsT`? I suspect this is not your goal, but I > > can't see a way around that > > The ultimate goal is to have option parsing be much more streamlined and > automated. The majority of option parsing is just taking some primitive value > and trying to grab its value from a string. There are places where we need > additional context (e.g. in this PR, there's a place where we want to select > a thread ID so there's more validation needed). I want to be able to write > those pretty naturally too. > > The first step in my plan is to centralize how we create error messages. The > majority of parsing errors will be "I couldn't turn this into a > boolean/integer/address/other", so I'm going to create something that is > better than what we have today. The step after that will be to revamp the > parsing step. Ideally instead of writing `OptionArgParser::ToBoolean` the way > we do today, there will be something like `llvm::Expected > value_or_error = ParseAsBool(option);` where `option` is the actual > OptionDefinition. I'm not tied to these abstractions, but I am committed to > the path of making this easier to write and maintain. Ohhh I think I see it now! > where `option` is the actual OptionDefinition And the `option` itself would know how to report / construct the error strings? https://github.com/llvm/llvm-project/pull/82273 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Add the RegisterCompleter to eArgTypeRegisterName in g_argument_table (PR #82428)
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 5a20a208037d32d52f0c626ea3b199278ff0df0a 9a430ce7c8065f21abef474d233a6b41937563ac -- lldb/include/lldb/Interpreter/CommandObject.h lldb/include/lldb/Interpreter/CommandOptionArgumentTable.h lldb/include/lldb/lldb-enumerations.h lldb/source/Commands/CommandObjectCommands.cpp lldb/source/Commands/CommandObjectDWIMPrint.cpp lldb/source/Commands/CommandObjectDWIMPrint.h lldb/source/Commands/CommandObjectFrame.cpp lldb/source/Commands/CommandObjectPlatform.cpp lldb/source/Commands/CommandObjectPlugin.cpp lldb/source/Commands/CommandObjectProcess.cpp lldb/source/Commands/CommandObjectRegister.cpp lldb/source/Commands/CommandObjectSession.cpp lldb/source/Commands/CommandObjectSettings.cpp lldb/source/Commands/CommandObjectTarget.cpp lldb/source/Commands/CommandObjectThread.cpp lldb/source/Commands/CommandObjectType.cpp lldb/source/Commands/CommandObjectWatchpoint.cpp lldb/source/Interpreter/CommandObject.cpp `` View the diff from clang-format here. ``diff diff --git a/lldb/source/Interpreter/CommandObject.cpp b/lldb/source/Interpreter/CommandObject.cpp index 93c53e89f7..50b59a57f8 100644 --- a/lldb/source/Interpreter/CommandObject.cpp +++ b/lldb/source/Interpreter/CommandObject.cpp @@ -316,7 +316,7 @@ void CommandObject::HandleArgumentCompletion( assert(entry_ptr && "We said there was one entry, but there wasn't."); return; // Not worth crashing if asserts are off... } - + CommandArgumentEntry &entry = *entry_ptr; // For now, we only handle the simple case of one homogenous argument type. if (entry.size() != 1) @@ -338,10 +338,8 @@ void CommandObject::HandleArgumentCompletion( lldb_private::CommandCompletions::InvokeCommonCompletionCallbacks( GetCommandInterpreter(), arg_entry->completion_type, request, nullptr); - } - bool CommandObject::HelpTextContainsWord(llvm::StringRef search_word, bool search_short_help, bool search_long_help, `` https://github.com/llvm/llvm-project/pull/82428 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 563ef30 - Add the RegisterCompleter to eArgTypeRegisterName in g_argument_table (#82428)
Author: jimingham Date: 2024-02-20T14:18:03-08:00 New Revision: 563ef306017a47d387f1c36dd562b172c1ad0626 URL: https://github.com/llvm/llvm-project/commit/563ef306017a47d387f1c36dd562b172c1ad0626 DIFF: https://github.com/llvm/llvm-project/commit/563ef306017a47d387f1c36dd562b172c1ad0626.diff LOG: Add the RegisterCompleter to eArgTypeRegisterName in g_argument_table (#82428) This is a follow-on to: https://github.com/llvm/llvm-project/pull/82085 The completer for register names was missing from the argument table. I somehow missed that the only register completer test was x86_64, so that test broke. I added the completer in to the right slot in the argument table, and added a small completions test that just uses the alias register names. If we end up having a platform that doesn't define register names, we'll have to skip this test there, but it should add a sniff test for register completion that will run most everywhere. Added: Modified: lldb/include/lldb/Interpreter/CommandObject.h lldb/include/lldb/Interpreter/CommandOptionArgumentTable.h lldb/include/lldb/lldb-enumerations.h lldb/source/Commands/CommandObjectCommands.cpp lldb/source/Commands/CommandObjectDWIMPrint.cpp lldb/source/Commands/CommandObjectDWIMPrint.h lldb/source/Commands/CommandObjectFrame.cpp lldb/source/Commands/CommandObjectPlatform.cpp lldb/source/Commands/CommandObjectPlugin.cpp lldb/source/Commands/CommandObjectProcess.cpp lldb/source/Commands/CommandObjectRegister.cpp lldb/source/Commands/CommandObjectSession.cpp lldb/source/Commands/CommandObjectSettings.cpp lldb/source/Commands/CommandObjectTarget.cpp lldb/source/Commands/CommandObjectThread.cpp lldb/source/Commands/CommandObjectType.cpp lldb/source/Commands/CommandObjectWatchpoint.cpp lldb/source/Interpreter/CommandObject.cpp lldb/test/API/commands/help/TestHelp.py lldb/test/API/functionalities/completion/TestCompletion.py Removed: diff --git a/lldb/include/lldb/Interpreter/CommandObject.h b/lldb/include/lldb/Interpreter/CommandObject.h index b99de56f534469..a326c6dc38a37a 100644 --- a/lldb/include/lldb/Interpreter/CommandObject.h +++ b/lldb/include/lldb/Interpreter/CommandObject.h @@ -242,6 +242,13 @@ class CommandObject : public std::enable_shared_from_this { ///The completion request that needs to be answered. virtual void HandleCompletion(CompletionRequest &request); + /// The default version handles argument definitions that have only one + /// argument type, and use one of the argument types that have an entry in + /// the CommonCompletions. Override this if you have a more complex + /// argument setup. + /// FIXME: we should be able to extend this to more complex argument + /// definitions provided we have completers for all the argument types. + /// /// The input array contains a parsed version of the line. /// /// We've constructed the map of options and their arguments as well if that @@ -251,7 +258,7 @@ class CommandObject : public std::enable_shared_from_this { ///The completion request that needs to be answered. virtual void HandleArgumentCompletion(CompletionRequest &request, - OptionElementVector &opt_element_vector) {} + OptionElementVector &opt_element_vector); bool HelpTextContainsWord(llvm::StringRef search_word, bool search_short_help = true, diff --git a/lldb/include/lldb/Interpreter/CommandOptionArgumentTable.h b/lldb/include/lldb/Interpreter/CommandOptionArgumentTable.h index d0cf54c31ca73f..9248e2ac814461 100644 --- a/lldb/include/lldb/Interpreter/CommandOptionArgumentTable.h +++ b/lldb/include/lldb/Interpreter/CommandOptionArgumentTable.h @@ -243,7 +243,7 @@ static constexpr CommandObject::ArgumentTableEntry g_argument_table[] = { { lldb::eArgTypeLogCategory, "log-category", lldb::CompletionType::eNoCompletion, {}, { nullptr, false }, "The name of a category within a log channel, e.g. all (try \"log list\" to see a list of all channels and their categories." }, { lldb::eArgTypeLogChannel, "log-channel", lldb::CompletionType::eNoCompletion, {}, { nullptr, false }, "The name of a log channel, e.g. process.gdb-remote (try \"log list\" to see a list of all channels and their categories)." }, { lldb::eArgTypeMethod, "method", lldb::CompletionType::eNoCompletion, {}, { nullptr, false }, "A C++ method name." }, -{ lldb::eArgTypeName, "name", lldb::eTypeCategoryNameCompletion, {}, { nullptr, false }, "Help text goes here." }, +{ lldb::eArgTypeName, "name", lldb::eTypeCategoryNameCompletion, {}, { nullptr, false }, "The name of a type category." }, { lldb::eArgTypeNewPathPrefix, "new-path-prefix", lldb::CompletionType::eNoCompletion, {}, { nullptr, false }, "Help text goes here." }, { lldb::eArgTypeNumL
[Lldb-commits] [lldb] Add the RegisterCompleter to eArgTypeRegisterName in g_argument_table (PR #82428)
https://github.com/jimingham closed https://github.com/llvm/llvm-project/pull/82428 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Add more ways to find the .dwp file. (PR #81067)
jeffreytan81 wrote: I believe the concern regarding standardizing the methods for locating debug information files is valid. Based on my experience, the only reliable approach to achieving this is either documenting it in a well-agreed spec, document, or website, or enforcing all symbolication tools to utilize a shared implementation. Microsoft employs the latter approach by mandating that all tools (such as Windbg, Microsoft Visual Studio debugger, Sysinternals tools, UMDH, etc.) share/use msdia.dll for symbolication. Additionally, as I mentioned in previous inline comments, the most useful feature for these symbolication tools is detailed symbol search paths logging/callbacks, which enable users to self-discover the implementation rules. In my opinion, improving in this direction is much more useful than enforcing consistent implementations across all tools, and it is also cheaper/easier to achieve. If a user can figure out through logging where their dwp/debuginfo/dSYM files are being searched by the debugger/tools, they are happy to self-fix the issue without concern for the implementation/consistency. Since there is currently no standard/official documentation yet, I will accept the patch to unblock the customers. Feel free to continue the discussion and improve the experience for users. https://github.com/llvm/llvm-project/pull/81067 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Add more ways to find the .dwp file. (PR #81067)
https://github.com/jeffreytan81 approved this pull request. https://github.com/llvm/llvm-project/pull/81067 ___ 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. jeffreytan81 wrote: This is only used by vfork() not fork(). 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 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 updated https://github.com/llvm/llvm-project/pull/81564 >From d65900f5e6169062fc0988b57fb5be2474789025 Mon Sep 17 00:00:00 2001 From: jeffreytan81 Date: Mon, 12 Feb 2024 18:08:23 -0800 Subject: [PATCH 1/5] Fix lldb crash while handling concurrent vfork() --- .../Process/Linux/NativeThreadLinux.cpp | 12 - .../Process/gdb-remote/ProcessGDBRemote.cpp | 21 ++--- .../Process/gdb-remote/ProcessGDBRemote.h | 3 +- .../fork/concurrent_vfork/Makefile| 4 ++ .../concurrent_vfork/TestConcurrentVFork.py | 31 + .../fork/concurrent_vfork/main.cpp| 46 +++ 6 files changed, 108 insertions(+), 9 deletions(-) create mode 100644 lldb/test/API/functionalities/fork/concurrent_vfork/Makefile create mode 100644 lldb/test/API/functionalities/fork/concurrent_vfork/TestConcurrentVFork.py create mode 100644 lldb/test/API/functionalities/fork/concurrent_vfork/main.cpp 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..8ab2257e0a79b9 100644 --- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp @@ -266,7 +266,7 @@ ProcessGDBRemote::ProcessGDBRemote(lldb::TargetSP target_sp, 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 +5615,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 +5674,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 +5685,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); + } Process::DidExec(); } diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h index c1ea1cc7905587..29ed770c1275ea 100644 --- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h +++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h @@ -301,7 +301,8 @@ class ProcessGDBRemote : public Process, using FlashRange = FlashRangeVector::Entry;
[Lldb-commits] [lldb] [lldb] Standardize command option parsing error messages (PR #82273)
bulbazord wrote: > > > This LGTM! > > > I don't think I can see far enough ahead on what you are planning here, > > > but I was just wondering if the ultimate goal is to have the > > > `option_arg.getAsT` return an `Expected`. In this case, wouldn't all > > > these arguments (short option, long option, additional context) have to > > > be part of the interface of `getAsT`? I suspect this is not your goal, > > > but I can't see a way around that > > > > > > The ultimate goal is to have option parsing be much more streamlined and > > automated. The majority of option parsing is just taking some primitive > > value and trying to grab its value from a string. There are places where we > > need additional context (e.g. in this PR, there's a place where we want to > > select a thread ID so there's more validation needed). I want to be able to > > write those pretty naturally too. > > The first step in my plan is to centralize how we create error messages. > > The majority of parsing errors will be "I couldn't turn this into a > > boolean/integer/address/other", so I'm going to create something that is > > better than what we have today. The step after that will be to revamp the > > parsing step. Ideally instead of writing `OptionArgParser::ToBoolean` the > > way we do today, there will be something like `llvm::Expected > > value_or_error = ParseAsBool(option);` where `option` is the actual > > OptionDefinition. I'm not tied to these abstractions, but I am committed to > > the path of making this easier to write and maintain. > > Ohhh I think I see it now! > > > where `option` is the actual OptionDefinition > > And the `option` itself would know how to report / construct the error > strings? I would like it to be that way. Let's see if it works out! 😄 https://github.com/llvm/llvm-project/pull/82273 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Remove the "generic" register completion test. (PR #82445)
https://github.com/jimingham created https://github.com/llvm/llvm-project/pull/82445 For reasons that are not clear to me, on arm64, the alias registers are listed in list of register info's we do completion against, but for x86_64 they are not. Maybe this is a difference in how the dynamic register builders work for the two systems. Anyway, it doesn't look possible to make a generic one. >From 35ea76b02b4cbd41412b05ea39c9a443eb011526 Mon Sep 17 00:00:00 2001 From: Jim Ingham Date: Tue, 20 Feb 2024 16:25:00 -0800 Subject: [PATCH] Remove the "generic" register completion test. For reasons that are not clear to me, on arm64, the alias registers are listed in list of register info's we do completion against, but for x86_64 they are not. Maybe this is a difference in how the dynamic register builders work for the two systems. Anyway, it doesn't look possible to make a generic one. --- .../completion/TestCompletion.py | 21 --- 1 file changed, 21 deletions(-) diff --git a/lldb/test/API/functionalities/completion/TestCompletion.py b/lldb/test/API/functionalities/completion/TestCompletion.py index b4681062a7d4e8..f71bc73928f0f4 100644 --- a/lldb/test/API/functionalities/completion/TestCompletion.py +++ b/lldb/test/API/functionalities/completion/TestCompletion.py @@ -787,27 +787,6 @@ def test_register_read_and_write_on_x86(self): # register write can only take exact one register name as argument self.complete_from_to("register write rbx ", []) -def test_register_read_and_write_generic(self): -"""Test the completion of the commands register read and write on x86""" - -self.build() -self.main_source_spec = lldb.SBFileSpec("main.cpp") -lldbutil.run_to_source_breakpoint(self, "// Break here", self.main_source_spec) - -# test cases for register read -self.complete_from_to("register read f", ["fp"]) -# register read can take multiple register names as arguments -self.complete_from_to("register read sp ", ["sp", "fp"]) -# complete with prefix '$' -self.complete_from_to("register read sp $", ["$sp", "$fp"]) -self.complete_from_to("register read $x0 ", ["sp", "fp"]) - -# test cases for register write -self.complete_from_to("register write ", ["fp", "sp"]) -self.complete_from_to("register write f", ["fp"]) -# register write can only take exact one register name as argument -self.complete_from_to("register write fp ", []) - def test_common_completion_target_stophook_ids(self): subcommands = ["delete", "enable", "disable"] ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] dc672d2 - Remove the "generic" register completion test. (#82445)
Author: jimingham Date: 2024-02-20T16:28:48-08:00 New Revision: dc672d2f6a48fb3d502c260eb353f389723ec417 URL: https://github.com/llvm/llvm-project/commit/dc672d2f6a48fb3d502c260eb353f389723ec417 DIFF: https://github.com/llvm/llvm-project/commit/dc672d2f6a48fb3d502c260eb353f389723ec417.diff LOG: Remove the "generic" register completion test. (#82445) For reasons that are not clear to me, on arm64, the alias registers are listed in list of register info's we do completion against, but for x86_64 they are not. Maybe this is a difference in how the dynamic register builders work for the two systems. Anyway, it doesn't look possible to make a generic one. Added: Modified: lldb/test/API/functionalities/completion/TestCompletion.py Removed: diff --git a/lldb/test/API/functionalities/completion/TestCompletion.py b/lldb/test/API/functionalities/completion/TestCompletion.py index b4681062a7d4e8..f71bc73928f0f4 100644 --- a/lldb/test/API/functionalities/completion/TestCompletion.py +++ b/lldb/test/API/functionalities/completion/TestCompletion.py @@ -787,27 +787,6 @@ def test_register_read_and_write_on_x86(self): # register write can only take exact one register name as argument self.complete_from_to("register write rbx ", []) -def test_register_read_and_write_generic(self): -"""Test the completion of the commands register read and write on x86""" - -self.build() -self.main_source_spec = lldb.SBFileSpec("main.cpp") -lldbutil.run_to_source_breakpoint(self, "// Break here", self.main_source_spec) - -# test cases for register read -self.complete_from_to("register read f", ["fp"]) -# register read can take multiple register names as arguments -self.complete_from_to("register read sp ", ["sp", "fp"]) -# complete with prefix '$' -self.complete_from_to("register read sp $", ["$sp", "$fp"]) -self.complete_from_to("register read $x0 ", ["sp", "fp"]) - -# test cases for register write -self.complete_from_to("register write ", ["fp", "sp"]) -self.complete_from_to("register write f", ["fp"]) -# register write can only take exact one register name as argument -self.complete_from_to("register write fp ", []) - def test_common_completion_target_stophook_ids(self): subcommands = ["delete", "enable", "disable"] ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Remove the "generic" register completion test. (PR #82445)
https://github.com/jimingham closed https://github.com/llvm/llvm-project/pull/82445 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Remove the "generic" register completion test. (PR #82445)
llvmbot wrote: @llvm/pr-subscribers-lldb Author: None (jimingham) Changes For reasons that are not clear to me, on arm64, the alias registers are listed in list of register info's we do completion against, but for x86_64 they are not. Maybe this is a difference in how the dynamic register builders work for the two systems. Anyway, it doesn't look possible to make a generic one. --- Full diff: https://github.com/llvm/llvm-project/pull/82445.diff 1 Files Affected: - (modified) lldb/test/API/functionalities/completion/TestCompletion.py (-21) ``diff diff --git a/lldb/test/API/functionalities/completion/TestCompletion.py b/lldb/test/API/functionalities/completion/TestCompletion.py index b4681062a7d4e8..f71bc73928f0f4 100644 --- a/lldb/test/API/functionalities/completion/TestCompletion.py +++ b/lldb/test/API/functionalities/completion/TestCompletion.py @@ -787,27 +787,6 @@ def test_register_read_and_write_on_x86(self): # register write can only take exact one register name as argument self.complete_from_to("register write rbx ", []) -def test_register_read_and_write_generic(self): -"""Test the completion of the commands register read and write on x86""" - -self.build() -self.main_source_spec = lldb.SBFileSpec("main.cpp") -lldbutil.run_to_source_breakpoint(self, "// Break here", self.main_source_spec) - -# test cases for register read -self.complete_from_to("register read f", ["fp"]) -# register read can take multiple register names as arguments -self.complete_from_to("register read sp ", ["sp", "fp"]) -# complete with prefix '$' -self.complete_from_to("register read sp $", ["$sp", "$fp"]) -self.complete_from_to("register read $x0 ", ["sp", "fp"]) - -# test cases for register write -self.complete_from_to("register write ", ["fp", "sp"]) -self.complete_from_to("register write f", ["fp"]) -# register write can only take exact one register name as argument -self.complete_from_to("register write fp ", []) - def test_common_completion_target_stophook_ids(self): subcommands = ["delete", "enable", "disable"] `` https://github.com/llvm/llvm-project/pull/82445 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Support DW_OP_WASM_location in DWARFExpression (PR #78977)
xujuntwt95329 wrote: Hi @JDevlieghere Could you please give us some suggestions about the next steps for this PR and https://github.com/llvm/llvm-project/pull/77949? Thanks a lot! https://github.com/llvm/llvm-project/pull/78977 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 5a45d32 - [lldb] Add more ways to find the .dwp file. (#81067)
Author: Greg Clayton Date: 2024-02-20T18:17:01-08:00 New Revision: 5a45d32b5b42dc4ed4852b0045391a1c2be41b48 URL: https://github.com/llvm/llvm-project/commit/5a45d32b5b42dc4ed4852b0045391a1c2be41b48 DIFF: https://github.com/llvm/llvm-project/commit/5a45d32b5b42dc4ed4852b0045391a1c2be41b48.diff LOG: [lldb] Add more ways to find the .dwp file. (#81067) When using split DWARF we can run into many different ways to store debug info: - lldb loads `` which contains skeleton DWARF and needs to find `.dwp` - lldb loads `` which is stripped but has .gnu_debuglink pointing to `.debug` with skeleton DWARF and needs to find `.dwp` - lldb loads `` which is stripped but has .gnu_debuglink pointing to `.debug` with skeleton DWARF and needs to find `.debug.dwp` - lldb loads `.debug` and needs to find `.dwp` Previously we only handled the first two cases. This patch adds support for the latter two. Added: Modified: lldb/include/lldb/Utility/FileSpecList.h lldb/source/Plugins/SymbolFile/DWARF/LogChannelDWARF.cpp lldb/source/Plugins/SymbolFile/DWARF/LogChannelDWARF.h lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp lldb/test/Shell/SymbolFile/DWARF/x86/dwp-separate-debug-file.cpp Removed: diff --git a/lldb/include/lldb/Utility/FileSpecList.h b/lldb/include/lldb/Utility/FileSpecList.h index 49edc667ddd5b6..6eb3bb9971f13a 100644 --- a/lldb/include/lldb/Utility/FileSpecList.h +++ b/lldb/include/lldb/Utility/FileSpecList.h @@ -238,6 +238,10 @@ class FileSpecList { const_iterator begin() const { return m_files.begin(); } const_iterator end() const { return m_files.end(); } + llvm::iterator_range files() const { +return llvm::make_range(begin(), end()); + } + protected: collection m_files; ///< A collection of FileSpec objects. }; diff --git a/lldb/source/Plugins/SymbolFile/DWARF/LogChannelDWARF.cpp b/lldb/source/Plugins/SymbolFile/DWARF/LogChannelDWARF.cpp index 6b063f3bd88d85..795355b57a06db 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/LogChannelDWARF.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/LogChannelDWARF.cpp @@ -22,6 +22,7 @@ static constexpr Log::Category g_categories[] = { {{"map"}, {"log insertions of object files into DWARF debug maps"}, DWARFLog::DebugMap}, +{{"split"}, {"log split DWARF related activities"}, DWARFLog::SplitDwarf}, }; static Log::Channel g_channel(g_categories, DWARFLog::DebugInfo); diff --git a/lldb/source/Plugins/SymbolFile/DWARF/LogChannelDWARF.h b/lldb/source/Plugins/SymbolFile/DWARF/LogChannelDWARF.h index 662aa6757e2ffc..7f254a1162bd10 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/LogChannelDWARF.h +++ b/lldb/source/Plugins/SymbolFile/DWARF/LogChannelDWARF.h @@ -20,6 +20,7 @@ enum class DWARFLog : Log::MaskType { DebugMap = Log::ChannelFlag<2>, Lookups = Log::ChannelFlag<3>, TypeCompletion = Log::ChannelFlag<4>, + SplitDwarf = Log::ChannelFlag<5>, LLVM_MARK_AS_BITMASK_ENUM(TypeCompletion) }; LLVM_ENABLE_BITMASK_ENUMS_IN_NAMESPACE(); diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp index 42211b9a21b0e3..84ff4c2565a050 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp @@ -4349,26 +4349,60 @@ SymbolFileDWARFDebugMap *SymbolFileDWARF::GetDebugMapSymfile() { const std::shared_ptr &SymbolFileDWARF::GetDwpSymbolFile() { llvm::call_once(m_dwp_symfile_once_flag, [this]() { +// Create a list of files to try and append .dwp to. +FileSpecList symfiles; +// Append the module's object file path. +const FileSpec module_fspec = m_objfile_sp->GetModule()->GetFileSpec(); +symfiles.Append(module_fspec); +// Append the object file for this SymbolFile only if it is diff erent from +// the module's file path. Our main module could be "a.out", our symbol file +// could be "a.debug" and our ".dwp" file might be "a.debug.dwp" instead of +// "a.out.dwp". +const FileSpec symfile_fspec(m_objfile_sp->GetFileSpec()); +if (symfile_fspec != module_fspec) { + symfiles.Append(symfile_fspec); +} else { + // If we don't have a separate debug info file, then try stripping the + // extension. The main module could be "a.debug" and the .dwp file could + // be "a.dwp" instead of "a.debug.dwp". + ConstString filename_no_ext = + module_fspec.GetFileNameStrippingExtension(); + if (filename_no_ext != module_fspec.GetFilename()) { +FileSpec module_spec_no_ext(module_fspec); +module_spec_no_ext.SetFilename(filename_no_ext); +symfiles.Append(module_spec_no_ext); + } +} +Log *log = GetLog(DWARFLog::SplitDwarf); +FileSpecList search_paths = Target::GetDefaultDebugFileSearchPaths(); ModuleSpec module_spec; module_spec.GetFileSpec
[Lldb-commits] [lldb] [lldb] Add more ways to find the .dwp file. (PR #81067)
https://github.com/clayborg closed https://github.com/llvm/llvm-project/pull/81067 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Implement WebAssembly debugging (PR #77949)
@@ -24,17 +24,33 @@ def format_register_value(val): return result +def make_code_address(module_id, offset): +return 0x4000 | (module_id << 32) | offset + xujuntwt95329 wrote: @paolosevMSFT Seems the rule require two empty lines before class definition ```suggestion ``` https://github.com/llvm/llvm-project/pull/77949 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Replace assertEquals with assertEqual (NFC) (PR #82073)
rupprecht wrote: Thanks! Nice cleanup. https://github.com/llvm/llvm-project/pull/82073 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Implement WebAssembly debugging (PR #77949)
xwang98 wrote: There is a strong need for this capability of LLDB from the WebAssembly Micro Runtime community. It will be awesome to have it in the LLDB. https://github.com/llvm/llvm-project/pull/77949 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Implement WebAssembly debugging (PR #77949)
@@ -2595,6 +2604,37 @@ bool DWARFExpression::Evaluate( break; } +case DW_OP_WASM_location: { + uint8_t wasm_op = opcodes.GetU8(&offset); + uint32_t index; + + /* LLDB doesn't have an address space to represents WebAssembly locals, + * globals and operand stacks. + * We encode these elements into virtual registers: + * | tag: 2 bits | index: 30 bits | + * where tag is: + *0: Not a WebAssembly location + *1: Local + *2: Global + *3: Operand stack value + */ + if (wasm_op == 3) { +index = opcodes.GetU32(&offset); +wasm_op = 2; // Global + } else { +index = opcodes.GetULEB128(&offset); + } + + reg_num = (((wasm_op + 1) & 0x03) << 30) | (index & 0x3fff); clayborg wrote: Why are we adding 1 to the wasm_op? Can't we just set it correctly above? Also it would be nice to have an enum or constant for `0x03` like `kWasmOpMask`, and for `30` like `kWasmOpShift' https://github.com/llvm/llvm-project/pull/77949 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Implement WebAssembly debugging (PR #77949)
@@ -2595,6 +2604,37 @@ bool DWARFExpression::Evaluate( break; } +case DW_OP_WASM_location: { + uint8_t wasm_op = opcodes.GetU8(&offset); + uint32_t index; + + /* LLDB doesn't have an address space to represents WebAssembly locals, + * globals and operand stacks. + * We encode these elements into virtual registers: + * | tag: 2 bits | index: 30 bits | + * where tag is: + *0: Not a WebAssembly location + *1: Local + *2: Global + *3: Operand stack value clayborg wrote: I see `WasmVirtualRegisterKinds` below, but we seem to be adjusting the values all around the code for some reason by adding 1 or subtracting 1. I would suggest to define this enum correct and never adjust the values all throughout the code. Make `WasmVirtualRegisterKinds` have the correct values in the enum definition and just use these enums everywhere without adjusting? https://github.com/llvm/llvm-project/pull/77949 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Implement WebAssembly debugging (PR #77949)
@@ -1450,6 +1450,14 @@ class Process : public std::enable_shared_from_this, /// platforms where there is a difference (only Arm Thumb at this time). lldb::addr_t FixAnyAddress(lldb::addr_t pc); + /// Some targets might use bits in a code address to represent additional + /// information; for example WebAssembly targets have a different memory space + /// per module and have a different address space per memory and code. + virtual lldb::addr_t FixMemoryAddress(lldb::addr_t address, clayborg wrote: Above we have FixCodeAddres() and FixDataAddress(). Will this function fix any kind of address, or just a code address? If it is just code, then maybe we can just modify the above `FixCodeAddress(...)` and add the StackFrame to that API which can default to nullptr. https://github.com/llvm/llvm-project/pull/77949 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Implement WebAssembly debugging (PR #77949)
@@ -346,6 +346,15 @@ static offset_t GetOpcodeDataSize(const DataExtractor &data, return (offset - data_offset) + subexpr_len; } + case DW_OP_WASM_location: { +uint8_t wasm_op = data.GetU8(&offset); +if (wasm_op == 3) clayborg wrote: It would be great to not use magic numbers. What does `3` really mean? Is there an enum we can use? https://github.com/llvm/llvm-project/pull/77949 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Implement WebAssembly debugging (PR #77949)
@@ -2595,6 +2604,37 @@ bool DWARFExpression::Evaluate( break; } +case DW_OP_WASM_location: { + uint8_t wasm_op = opcodes.GetU8(&offset); + uint32_t index; + + /* LLDB doesn't have an address space to represents WebAssembly locals, + * globals and operand stacks. + * We encode these elements into virtual registers: + * | tag: 2 bits | index: 30 bits | + * where tag is: + *0: Not a WebAssembly location + *1: Local + *2: Global + *3: Operand stack value + */ + if (wasm_op == 3) { +index = opcodes.GetU32(&offset); +wasm_op = 2; // Global clayborg wrote: Use `WasmLocationTag` enum suggested above ``` wasm_op = eWasmGlobal; ``` https://github.com/llvm/llvm-project/pull/77949 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Implement WebAssembly debugging (PR #77949)
@@ -0,0 +1,108 @@ +//=== wasmRegisterContext.cpp -===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===--===// + +#include "wasmRegisterContext.h" +#include "Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h" +#include "ProcessWasm.h" +#include "ThreadWasm.h" +#include "lldb/Utility/RegisterValue.h" +#include + +using namespace lldb; +using namespace lldb_private; +using namespace lldb_private::process_gdb_remote; +using namespace lldb_private::wasm; + +WasmRegisterContext::WasmRegisterContext( +wasm::ThreadWasm &thread, uint32_t concrete_frame_idx, +GDBRemoteDynamicRegisterInfoSP reg_info_sp) +: GDBRemoteRegisterContext(thread, concrete_frame_idx, reg_info_sp, false, + false) {} + +WasmRegisterContext::~WasmRegisterContext() = default; + +uint32_t WasmRegisterContext::ConvertRegisterKindToRegisterNumber( +lldb::RegisterKind kind, uint32_t num) { + return num; +} + +size_t WasmRegisterContext::GetRegisterCount() { return 0; } + +const RegisterInfo *WasmRegisterContext::GetRegisterInfoAtIndex(size_t reg) { + uint32_t tag = (reg >> 30) & 0x03; clayborg wrote: Also it would be nice to have an enum or constant for `0x03` like `kWasmOpMask`, and for `30` like `kWasmOpShift' https://github.com/llvm/llvm-project/pull/77949 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Implement WebAssembly debugging (PR #77949)
@@ -0,0 +1,108 @@ +//=== wasmRegisterContext.cpp -===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===--===// + +#include "wasmRegisterContext.h" +#include "Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h" +#include "ProcessWasm.h" +#include "ThreadWasm.h" +#include "lldb/Utility/RegisterValue.h" +#include + +using namespace lldb; +using namespace lldb_private; +using namespace lldb_private::process_gdb_remote; +using namespace lldb_private::wasm; + +WasmRegisterContext::WasmRegisterContext( +wasm::ThreadWasm &thread, uint32_t concrete_frame_idx, +GDBRemoteDynamicRegisterInfoSP reg_info_sp) +: GDBRemoteRegisterContext(thread, concrete_frame_idx, reg_info_sp, false, + false) {} + +WasmRegisterContext::~WasmRegisterContext() = default; + +uint32_t WasmRegisterContext::ConvertRegisterKindToRegisterNumber( +lldb::RegisterKind kind, uint32_t num) { + return num; +} + +size_t WasmRegisterContext::GetRegisterCount() { return 0; } + +const RegisterInfo *WasmRegisterContext::GetRegisterInfoAtIndex(size_t reg) { + uint32_t tag = (reg >> 30) & 0x03; + if (tag == 0) { +return m_reg_info_sp->GetRegisterInfoAtIndex(reg); + } clayborg wrote: no braces for single line if statments per llvm coding guidelines. https://github.com/llvm/llvm-project/pull/77949 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Implement WebAssembly debugging (PR #77949)
@@ -2595,6 +2604,37 @@ bool DWARFExpression::Evaluate( break; } +case DW_OP_WASM_location: { + uint8_t wasm_op = opcodes.GetU8(&offset); + uint32_t index; + + /* LLDB doesn't have an address space to represents WebAssembly locals, + * globals and operand stacks. + * We encode these elements into virtual registers: + * | tag: 2 bits | index: 30 bits | + * where tag is: + *0: Not a WebAssembly location + *1: Local + *2: Global + *3: Operand stack value + */ + if (wasm_op == 3) { clayborg wrote: What is `3` make an enum for this instead of using magic numbers. Same enum as the previous magic `3` we saw in previous `DW_OP_WASM_location` case. https://github.com/llvm/llvm-project/pull/77949 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Implement WebAssembly debugging (PR #77949)
@@ -0,0 +1,108 @@ +//=== wasmRegisterContext.cpp -===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===--===// + +#include "wasmRegisterContext.h" +#include "Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h" +#include "ProcessWasm.h" +#include "ThreadWasm.h" +#include "lldb/Utility/RegisterValue.h" +#include + +using namespace lldb; +using namespace lldb_private; +using namespace lldb_private::process_gdb_remote; +using namespace lldb_private::wasm; + +WasmRegisterContext::WasmRegisterContext( +wasm::ThreadWasm &thread, uint32_t concrete_frame_idx, +GDBRemoteDynamicRegisterInfoSP reg_info_sp) +: GDBRemoteRegisterContext(thread, concrete_frame_idx, reg_info_sp, false, + false) {} + +WasmRegisterContext::~WasmRegisterContext() = default; + +uint32_t WasmRegisterContext::ConvertRegisterKindToRegisterNumber( +lldb::RegisterKind kind, uint32_t num) { + return num; +} + +size_t WasmRegisterContext::GetRegisterCount() { return 0; } + +const RegisterInfo *WasmRegisterContext::GetRegisterInfoAtIndex(size_t reg) { + uint32_t tag = (reg >> 30) & 0x03; + if (tag == 0) { +return m_reg_info_sp->GetRegisterInfoAtIndex(reg); + } + + auto it = m_register_map.find(reg); + if (it == m_register_map.end()) { +WasmVirtualRegisterKinds kind = +static_cast(tag - 1); clayborg wrote: Why are we adjusting the value here? Just make the WasmVirtualRegisterKinds definition be correct and avoid subtracting 1? https://github.com/llvm/llvm-project/pull/77949 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Implement WebAssembly debugging (PR #77949)
@@ -0,0 +1,76 @@ +//===-- UnwindWasm.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 "UnwindWasm.h" +#include "Plugins/Process/gdb-remote/ThreadGDBRemote.h" +#include "Plugins/Process/wasm/ProcessWasm.h" +#include "Plugins/Process/wasm/ThreadWasm.h" +#include "lldb/lldb-forward.h" +#include "wasmRegisterContext.h" + +using namespace lldb; +using namespace lldb_private; +using namespace process_gdb_remote; +using namespace wasm; + +class WasmGDBRemoteRegisterContext : public GDBRemoteRegisterContext { +public: + WasmGDBRemoteRegisterContext(ThreadGDBRemote &thread, + uint32_t concrete_frame_idx, + GDBRemoteDynamicRegisterInfoSP ®_info_sp, + uint64_t pc) + : GDBRemoteRegisterContext(thread, concrete_frame_idx, reg_info_sp, false, + false) { +PrivateSetRegisterValue(0, pc); + } +}; + +lldb::RegisterContextSP +UnwindWasm::DoCreateRegisterContextForFrame(lldb_private::StackFrame *frame) { + if (m_frames.size() <= frame->GetFrameIndex()) { +return lldb::RegisterContextSP(); + } + + ThreadSP thread = frame->GetThread(); + ProcessSP process_sp = thread->GetProcess(); + ThreadWasm *wasm_thread = static_cast(thread.get()); + ProcessWasm *wasm_process = static_cast(process_sp.get()); + std::shared_ptr reg_ctx_sp = + std::make_shared(*wasm_thread, +frame->GetConcreteFrameIndex(), +wasm_process->GetRegisterInfo()); + return reg_ctx_sp; +} + +uint32_t UnwindWasm::DoGetFrameCount() { + if (!m_unwind_complete) { +m_unwind_complete = true; +m_frames.clear(); + +ThreadWasm &wasm_thread = static_cast(GetThread()); +if (!wasm_thread.GetWasmCallStack(m_frames)) + m_frames.clear(); + } + return m_frames.size(); +} + +bool UnwindWasm::DoGetFrameInfoAtIndex(uint32_t frame_idx, lldb::addr_t &cfa, + lldb::addr_t &pc, + bool &behaves_like_zeroth_frame) { + if (m_frames.size() == 0) { +DoGetFrameCount(); + } clayborg wrote: no braces for single line if statments per llvm coding guidelines. https://github.com/llvm/llvm-project/pull/77949 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Implement WebAssembly debugging (PR #77949)
@@ -0,0 +1,108 @@ +//=== wasmRegisterContext.cpp -===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===--===// + +#include "wasmRegisterContext.h" +#include "Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h" +#include "ProcessWasm.h" +#include "ThreadWasm.h" +#include "lldb/Utility/RegisterValue.h" +#include + +using namespace lldb; +using namespace lldb_private; +using namespace lldb_private::process_gdb_remote; +using namespace lldb_private::wasm; + +WasmRegisterContext::WasmRegisterContext( +wasm::ThreadWasm &thread, uint32_t concrete_frame_idx, +GDBRemoteDynamicRegisterInfoSP reg_info_sp) +: GDBRemoteRegisterContext(thread, concrete_frame_idx, reg_info_sp, false, + false) {} + +WasmRegisterContext::~WasmRegisterContext() = default; + +uint32_t WasmRegisterContext::ConvertRegisterKindToRegisterNumber( +lldb::RegisterKind kind, uint32_t num) { + return num; +} + +size_t WasmRegisterContext::GetRegisterCount() { return 0; } + +const RegisterInfo *WasmRegisterContext::GetRegisterInfoAtIndex(size_t reg) { + uint32_t tag = (reg >> 30) & 0x03; + if (tag == 0) { +return m_reg_info_sp->GetRegisterInfoAtIndex(reg); + } + + auto it = m_register_map.find(reg); + if (it == m_register_map.end()) { +WasmVirtualRegisterKinds kind = +static_cast(tag - 1); +std::tie(it, std::ignore) = m_register_map.insert( +{reg, + std::make_unique(kind, reg & 0x3fff)}); + } + return it->second.get(); +} + +size_t WasmRegisterContext::GetRegisterSetCount() { return 0; } + +const RegisterSet *WasmRegisterContext::GetRegisterSet(size_t reg_set) { + return nullptr; +} + +bool WasmRegisterContext::ReadRegister(const RegisterInfo *reg_info, + RegisterValue &value) { + if (reg_info->name) { +return GDBRemoteRegisterContext::ReadRegister(reg_info, value); + } clayborg wrote: no braces for single line if statments per llvm coding guidelines. https://github.com/llvm/llvm-project/pull/77949 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits