serhiy.redko created this revision.
serhiy.redko requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.
launch.json's fields have ambiguity with how final target is created:
1. There are dedicated fields to specify target like "program", "pid", "waitFor"
2. There are "launchCommands" and "attachCommands" fields that might or might
not create a target.
Current implementation for handling of "launchCommands" and "attachCommands"
implies
they are rather mutually exclusive with other target defining parameters like
"program", "pid" etc
in case they create a new target.
So user can provide in the launch.json fields "program", "pid", "core_file" and
these parameters
will be silently ignored by implementation in case "launchCommands" and
"attachCommands" are provided
as well and create a new target. For attach "core_file" is ignored in case
"attachCommands"
field is just provided, no matter whether commands create or not target.
At the same time, implementation always creates target by means of
CreateTargetFromArguments()
API in expectance of target dedicated fields in launch.json like "program",
"pid", even in case
"launchCommands" and "attachCommands" (that do create a target) are provided.
Creation of this first, "auxilary" target, introduces issue with target
settings specified for second
target and created by "launchCommands" or "attachCommands": target settings for
the second target are 'hijacked'
by target created by CreateTargetFromArguments() call and not applied to target
created
by "launchCommands"/"attachCommands".
For example, for "launchCommands":
settings set target.exec-search-paths /some/path
target create /some/file
target.exec-search-paths is not applied to the final target created in
"launchCommands".
The settings is applied to previously created "auxilary" target which doesn't
become selected.
This breaks debugging of the target created by
"launchCommands"/"attachCommands".
A possible workaround for the issue would be a strict requirement to set target
settings
after target creation command, however it might not be feasible for some
settings and
inconsistent with cli lldb usage results where this issue is not reproduced.
This change avoids creation of "auxilary" target in case
"launchCommands"/"attachCommands"
are provided. It also uses proper SetTarget() setter to set final target and
subscribe it
for notifications.
This change potentially can break the case when user provides
"launchCommands"/"attachCommands"
that don't create a target, but rely on the "auxilary" target that they can
tune further by commands
in "launchCommands", "attachCommands". This doesn't look like a correct usage
intent to me
because it requires from users quite thorough inspection of 'launch'/'attach'
requests implementation and
understanding which launch.json fields are ignored or not by implementation.
In case user needs to tune created target I would recommend to create new
"postTargetCommands" launch.json
field with clear instructions to not create target and assume that target
already exists.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D94997
Files:
lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
lldb/test/API/tools/lldb-vscode/launch/TestVSCode_launch.py
lldb/tools/lldb-vscode/lldb-vscode.cpp
lldb/tools/lldb-vscode/package.json
Index: lldb/tools/lldb-vscode/package.json
===================================================================
--- lldb/tools/lldb-vscode/package.json
+++ lldb/tools/lldb-vscode/package.json
@@ -85,7 +85,7 @@
"properties": {
"program": {
"type": "string",
- "description": "Path to the program to debug."
+ "description": "Path to the program to debug. Ignored if \"launchCommands\" is provided."
},
"args": {
"type": [
@@ -163,7 +163,7 @@
},
"launchCommands": {
"type": "array",
- "description": "Custom commands that are executed instead of launching a process. A target will be created with the launch arguments prior to executing these commands. The commands may optionally create a new target and must perform a launch. A valid process must exist after these commands complete or the \"launch\" will fail.",
+ "description": "Custom commands that are executed instead of launching a process. The commands must create a target and must perform a launch. A valid process must exist after these commands complete or the \"launch\" will fail. The following settings will be ignored if provided: \"program\", \"cwd\", \"args\", \"env\", \"disableASLR\", \"disableSTDIO\", \"shellExpandArguments\", \"detachOnError\", \"runInTerminal\", \"targetTriple\", \"platformName\"",
"default": []
},
"stopCommands": {
@@ -187,18 +187,18 @@
"properties": {
"program": {
"type": "string",
- "description": "Path to the program to attach to."
+ "description": "Path to the program to attach to. Ignored if \"attachCommands\" is provided."
},
"pid": {
"type": [
"number",
"string"
],
- "description": "System process ID to attach to."
+ "description": "System process ID to attach to. Ignored if \"attachCommands\" is provided."
},
"waitFor": {
"type": "boolean",
- "description": "If set to true, then wait for the process to launch by looking for a process with a basename that matches `program`. No process ID needs to be specified when using this flag.",
+ "description": "If set to true, then wait for the process to launch by looking for a process with a basename that matches `program`. No process ID needs to be specified when using this flag. Ignored if \"attachCommands\" is provided.",
"default": true
},
"sourcePath": {
@@ -224,7 +224,7 @@
},
"attachCommands": {
"type": "array",
- "description": "Custom commands that are executed instead of attaching to a process ID or to a process by name. These commands may optionally create a new target and must perform an attach. A valid process must exist after these commands complete or the \"attach\" will fail.",
+ "description": "Custom commands that are executed instead of attaching to a process ID or to a process by name. These commands must create a target and must perform an attach. A valid process must exist after these commands complete or the \"attach\" will fail. The following settings will be ignored if provided: \"program\", \"pid\", \"waitFor\", \"coreFile\", \"targetTriple\", \"platformName\"",
"default": []
},
"initCommands": {
Index: lldb/tools/lldb-vscode/lldb-vscode.cpp
===================================================================
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -578,28 +578,43 @@
// Run any initialize LLDB commands the user specified in the launch.json
g_vsc.RunInitCommands();
- lldb::SBError status;
- g_vsc.SetTarget(g_vsc.CreateTargetFromArguments(*arguments, status));
- if (status.Fail()) {
- response["success"] = llvm::json::Value(false);
- EmplaceSafeString(response, "message", status.GetCString());
- g_vsc.SendJSON(llvm::json::Value(std::move(response)));
- return;
- }
-
- // Run any pre run LLDB commands the user specified in the launch.json
- g_vsc.RunPreRunCommands();
+ if (!attachCommands.empty()) {
+ // We have "attachCommands" that are a set of commands that are expected
+ // to execute the commands after which a process should be created. If there
+ // is no valid process after running these commands, we have failed.
- if (pid == LLDB_INVALID_PROCESS_ID && wait_for) {
- char attach_msg[256];
- auto attach_msg_len = snprintf(attach_msg, sizeof(attach_msg),
- "Waiting to attach to \"%s\"...",
- g_vsc.target.GetExecutable().GetFilename());
- g_vsc.SendOutput(OutputType::Console,
- llvm::StringRef(attach_msg, attach_msg_len));
- }
- if (attachCommands.empty()) {
+ // Run any pre run LLDB commands the user specified in the launch.json
+ g_vsc.RunPreRunCommands();
+ g_vsc.RunLLDBCommands("Running attachCommands:", attachCommands);
+ // The custom commands are expected to create a new target
+ if (g_vsc.debugger.GetNumTargets() > 0) {
+ g_vsc.SetTarget(g_vsc.debugger.GetSelectedTarget());
+ } else {
+ error.SetErrorString("attachCommands failed to create target");
+ }
+ } else {
// No "attachCommands", just attach normally.
+ lldb::SBError status;
+ g_vsc.SetTarget(g_vsc.CreateTargetFromArguments(*arguments, status));
+ if (status.Fail()) {
+ response["success"] = llvm::json::Value(false);
+ EmplaceSafeString(response, "message", status.GetCString());
+ g_vsc.SendJSON(llvm::json::Value(std::move(response)));
+ return;
+ }
+
+ // Run any pre run LLDB commands the user specified in the launch.json
+ g_vsc.RunPreRunCommands();
+
+ if (pid == LLDB_INVALID_PROCESS_ID && wait_for) {
+ char attach_msg[256];
+ auto attach_msg_len = snprintf(
+ attach_msg, sizeof(attach_msg), "Waiting to attach to \"%s\"...",
+ g_vsc.target.GetExecutable().GetFilename());
+ g_vsc.SendOutput(OutputType::Console,
+ llvm::StringRef(attach_msg, attach_msg_len));
+ }
+
// Disable async events so the attach will be successful when we return from
// the launch call and the launch will happen synchronously
g_vsc.debugger.SetAsync(false);
@@ -609,14 +624,6 @@
g_vsc.target.LoadCore(core_file.data(), error);
// Reenable async events
g_vsc.debugger.SetAsync(true);
- } else {
- // We have "attachCommands" that are a set of commands that are expected
- // to execute the commands after which a process should be created. If there
- // is no valid process after running these commands, we have failed.
- g_vsc.RunLLDBCommands("Running attachCommands:", attachCommands);
- // The custom commands might have created a new target so we should use the
- // selected target after these commands are run.
- g_vsc.target = g_vsc.debugger.GetSelectedTarget();
}
SetSourceMapFromArguments(*arguments);
@@ -1547,67 +1554,77 @@
SetSourceMapFromArguments(*arguments);
- lldb::SBError status;
- g_vsc.SetTarget(g_vsc.CreateTargetFromArguments(*arguments, status));
- if (status.Fail()) {
- response["success"] = llvm::json::Value(false);
- EmplaceSafeString(response, "message", status.GetCString());
- g_vsc.SendJSON(llvm::json::Value(std::move(response)));
- return;
- }
+ if (!launchCommands.empty()) {
+ // if "launchCommands" are provided, then they are expected to make the
+ // launch happen for launch requests and they replace the normal logic that
+ // would implement the launch. Run any pre run LLDB commands the user
+ // specified in the launch.json
+ g_vsc.RunPreRunCommands();
+ g_vsc.RunLLDBCommands("Running launchCommands:", launchCommands);
+ // The custom commands are expected to create a new target
+ if (g_vsc.debugger.GetNumTargets() > 0) {
+ g_vsc.SetTarget(g_vsc.debugger.GetSelectedTarget());
+ } else {
+ error.SetErrorString("launchCommands failed to create target");
+ }
+ } else {
+ // the normal logic that would implement the launch
+ lldb::SBError status;
+ g_vsc.SetTarget(g_vsc.CreateTargetFromArguments(*arguments, status));
+ if (status.Fail()) {
+ response["success"] = llvm::json::Value(false);
+ EmplaceSafeString(response, "message", status.GetCString());
+ g_vsc.SendJSON(llvm::json::Value(std::move(response)));
+ return;
+ }
- if (GetBoolean(arguments, "runInTerminal", false)) {
- request_runInTerminal(request, response);
- g_vsc.SendJSON(llvm::json::Value(std::move(response)));
- return;
- }
+ if (GetBoolean(arguments, "runInTerminal", false)) {
+ request_runInTerminal(request, response);
+ g_vsc.SendJSON(llvm::json::Value(std::move(response)));
+ return;
+ }
+
+ // Instantiate a launch info instance for the target.
+ auto launch_info = g_vsc.target.GetLaunchInfo();
+
+ // Grab the current working directory if there is one and set it in the
+ // launch info.
+ const auto cwd = GetString(arguments, "cwd");
+ if (!cwd.empty())
+ launch_info.SetWorkingDirectory(cwd.data());
+
+ // Extract any extra arguments and append them to our program arguments for
+ // when we launch
+ auto args = GetStrings(arguments, "args");
+ if (!args.empty())
+ launch_info.SetArguments(MakeArgv(args).data(), true);
+
+ // Pass any environment variables along that the user specified.
+ auto envs = GetStrings(arguments, "env");
+ if (!envs.empty())
+ launch_info.SetEnvironmentEntries(MakeArgv(envs).data(), true);
+
+ auto flags = launch_info.GetLaunchFlags();
+
+ if (GetBoolean(arguments, "disableASLR", true))
+ flags |= lldb::eLaunchFlagDisableASLR;
+ if (GetBoolean(arguments, "disableSTDIO", false))
+ flags |= lldb::eLaunchFlagDisableSTDIO;
+ if (GetBoolean(arguments, "shellExpandArguments", false))
+ flags |= lldb::eLaunchFlagShellExpandArguments;
+ const bool detachOnError = GetBoolean(arguments, "detachOnError", false);
+ launch_info.SetDetachOnError(detachOnError);
+ launch_info.SetLaunchFlags(flags | lldb::eLaunchFlagDebug |
+ lldb::eLaunchFlagStopAtEntry);
+
+ // Run any pre run LLDB commands the user specified in the launch.json
+ g_vsc.RunPreRunCommands();
- // Instantiate a launch info instance for the target.
- auto launch_info = g_vsc.target.GetLaunchInfo();
-
- // Grab the current working directory if there is one and set it in the
- // launch info.
- const auto cwd = GetString(arguments, "cwd");
- if (!cwd.empty())
- launch_info.SetWorkingDirectory(cwd.data());
-
- // Extract any extra arguments and append them to our program arguments for
- // when we launch
- auto args = GetStrings(arguments, "args");
- if (!args.empty())
- launch_info.SetArguments(MakeArgv(args).data(), true);
-
- // Pass any environment variables along that the user specified.
- auto envs = GetStrings(arguments, "env");
- if (!envs.empty())
- launch_info.SetEnvironmentEntries(MakeArgv(envs).data(), true);
-
- auto flags = launch_info.GetLaunchFlags();
-
- if (GetBoolean(arguments, "disableASLR", true))
- flags |= lldb::eLaunchFlagDisableASLR;
- if (GetBoolean(arguments, "disableSTDIO", false))
- flags |= lldb::eLaunchFlagDisableSTDIO;
- if (GetBoolean(arguments, "shellExpandArguments", false))
- flags |= lldb::eLaunchFlagShellExpandArguments;
- const bool detatchOnError = GetBoolean(arguments, "detachOnError", false);
- launch_info.SetDetachOnError(detatchOnError);
- launch_info.SetLaunchFlags(flags | lldb::eLaunchFlagDebug |
- lldb::eLaunchFlagStopAtEntry);
-
- // Run any pre run LLDB commands the user specified in the launch.json
- g_vsc.RunPreRunCommands();
- if (launchCommands.empty()) {
// Disable async events so the launch will be successful when we return from
// the launch call and the launch will happen synchronously
g_vsc.debugger.SetAsync(false);
g_vsc.target.Launch(launch_info, error);
g_vsc.debugger.SetAsync(true);
- } else {
- g_vsc.RunLLDBCommands("Running launchCommands:", launchCommands);
- // The custom commands might have created a new target so we should use the
- // selected target after these commands are run.
- g_vsc.target = g_vsc.debugger.GetSelectedTarget();
}
if (error.Fail()) {
Index: lldb/test/API/tools/lldb-vscode/launch/TestVSCode_launch.py
===================================================================
--- lldb/test/API/tools/lldb-vscode/launch/TestVSCode_launch.py
+++ lldb/test/API/tools/lldb-vscode/launch/TestVSCode_launch.py
@@ -373,6 +373,7 @@
'''
self.build_and_create_debug_adaptor()
program = self.getBuildArtifact("a.out")
+ execSearchPaths = '/unique/path/to/symbols/launch/test'
source = 'main.c'
first_line = line_number(source, '// breakpoint 1')
@@ -382,6 +383,7 @@
# also we can verify that "stopCommands" get run as the
# breakpoints get hit
launchCommands = [
+ 'settings set target.exec-search-paths "%s"' % (execSearchPaths),
'target create "%s"' % (program),
'br s -f main.c -l %d' % first_line,
'br s -f main.c -l %d' % second_line,
@@ -391,7 +393,7 @@
initCommands = ['target list', 'platform list']
preRunCommands = ['image list a.out', 'image dump sections a.out']
stopCommands = ['frame variable', 'bt']
- exitCommands = ['expr 2+3', 'expr 3+4']
+ exitCommands = ['expr 2+3', 'expr 3+4', 'settings show target.exec-search-paths']
self.launch(program,
initCommands=initCommands,
preRunCommands=preRunCommands,
@@ -428,6 +430,8 @@
# "exitCommands" that were run after the second breakpoint was hit
output = self.get_console(timeout=1.0)
self.verify_commands('exitCommands', output, exitCommands)
+ # confirm that output contains correct target.exec-search-paths value
+ self.verify_contains_text('exitCommands', output, execSearchPaths)
@skipIfWindows
@skipIfNetBSD # Hangs on NetBSD as well
@@ -440,7 +444,7 @@
'''
self.build_and_create_debug_adaptor()
program = self.getBuildArtifact("a.out")
-
+
terminateCommands = ['expr 4+2']
self.launch(program=program,
terminateCommands=terminateCommands)
Index: lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
===================================================================
--- lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
+++ lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
@@ -126,6 +126,13 @@
"verify '%s' found in console output for '%s'" % (
cmd, flavor))
+ def verify_contains_text(self, flavor, output, text):
+ self.assertTrue(output and len(output) > 0, "expect console output")
+ found = text in output
+ self.assertTrue(found,
+ "verify '%s' found in console output for '%s'" % (
+ text, flavor))
+
def get_dict_value(self, d, key_path):
'''Verify each key in the key_path array is in contained in each
dictionary within "d". Assert if any key isn't in the
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
