[Lldb-commits] [PATCH] D94997: [lldb][lldb-vscode] Updated implementation of 'launch' and 'attach' requests to not create auxiliary target in case "launchCommands" and "attachCommands" are provided.

[Serhiy Redko via Phabricator via lldb-commits]

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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D94997.317666.patch
Type: text/x-patch
Size: 15575 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20210119/ca4a0534/attachment.bin>