This is an archive of the discontinued LLVM Phabricator instance.

[lldb] Add support for escaping fish arguments
ClosedPublic

Authored by teemperor on Jun 21 2021, 5:14 AM.

Details

Summary

LLDB supports having globbing regexes in the process launch arguments that will be resolved
using the user's shell. This requires that we pass the launch args to the shell and then read
back the expanded arguments using LLDB's argdumper utility.

As the shell will not just expand the globbing regexes but all special characters, we need to
escape all non-globbing charcters such as $, &, <, >, etc. as those otherwise are interpreted and
removed in the step where we expand the globbing characters. Also because the special characters
are shell-specific, LLDB needs to maintain a list of all the characters that need to be escaped for
each specific shell.

This patch adds the list of special characters that need to be escaped for fish. Without this patch
on systems where fish is the user's shell having any of these special characters in your arguments or path to
the binary will cause the process launch to fail. E.g., lldb -- ./calc 1<2 is failing without this patch.
The same happens if the absolute path to calc is in a directory that contains for example parentheses
or other special characters.

Diff Detail

Event Timeline

teemperor requested review of this revision.Jun 21 2021, 5:14 AM
teemperor created this revision.
teemperor updated this revision to Diff 353344.Jun 21 2021, 5:19 AM
  • Add pipe and backslash also as special characters.
JDevlieghere added inline comments.Jun 21 2021, 7:38 AM
lldb/source/Utility/Args.cpp
387

I guess this should be fish?

Whoops, wrong diff...

teemperor updated this revision to Diff 353374.Jun 21 2021, 7:53 AM
  • Upload the fish diff
This revision is now accepted and ready to land.Jun 21 2021, 8:00 AM
This revision was landed with ongoing or failed builds.Jul 11 2022, 4:45 PM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptJul 11 2022, 4:45 PM