From 6c089b2af5d8d98f66b27b67f70958f520820a76 Mon Sep 17 00:00:00 2001 From: Jim Ingham Date: Wed, 31 Aug 2022 10:13:05 -0700 Subject: [PATCH] Be more careful to maintain quoting information when parsing commands. This is particularly a problem for alias construction, where you might want to have a backtick surrounded option in the alias. Before this patch: command alias expression -Z \`argc\` -- argv for instance would be rendered as: expression -Z argc -- argv and would fail to work. Differential Revision: https://reviews.llvm.org/D133045 --- lldb/include/lldb/Interpreter/CommandInterpreter.h | 8 +++ lldb/packages/Python/lldbsuite/test/lldbtest.py | 7 +++ lldb/source/Interpreter/CommandAlias.cpp | 14 +++-- lldb/source/Interpreter/CommandInterpreter.cpp | 62 +++++++++++++++++----- lldb/source/Interpreter/CommandObject.cpp | 2 +- lldb/source/Interpreter/Options.cpp | 38 ++++++++----- lldb/source/Utility/Args.cpp | 5 ++ lldb/test/API/commands/command/backticks/Makefile | 3 ++ .../command/backticks/TestBackticksInAlias.py | 32 +++++++++++ lldb/test/API/commands/command/backticks/main.c | 5 ++ 10 files changed, 142 insertions(+), 34 deletions(-) create mode 100644 lldb/test/API/commands/command/backticks/Makefile create mode 100644 lldb/test/API/commands/command/backticks/TestBackticksInAlias.py create mode 100644 lldb/test/API/commands/command/backticks/main.c diff --git a/lldb/include/lldb/Interpreter/CommandInterpreter.h b/lldb/include/lldb/Interpreter/CommandInterpreter.h index 0f137a7..71587ee 100644 --- a/lldb/include/lldb/Interpreter/CommandInterpreter.h +++ b/lldb/include/lldb/Interpreter/CommandInterpreter.h @@ -239,6 +239,14 @@ public: eCommandTypesAllThem = 0xFFFF //< all commands }; + // The CommandAlias and CommandInterpreter both have a hand in + // substituting for alias commands. They work by writing special tokens + // in the template form of the Alias command, and then detecting them when the + // command is executed. These are the special tokens: + static const char * const g_no_argument; + static const char * const g_need_argument; + static const char * const g_argument; + CommandInterpreter(Debugger &debugger, bool synchronous_execution); ~CommandInterpreter() override = default; diff --git a/lldb/packages/Python/lldbsuite/test/lldbtest.py b/lldb/packages/Python/lldbsuite/test/lldbtest.py index 101921f..5c05876 100644 --- a/lldb/packages/Python/lldbsuite/test/lldbtest.py +++ b/lldb/packages/Python/lldbsuite/test/lldbtest.py @@ -2472,6 +2472,13 @@ FileCheck output: self.fail(self._formatMessage(msg, "'{}' is not success".format(error))) + """Assert that a command return object is successful""" + def assertCommandReturn(self, obj, msg=None): + if not obj.Succeeded(): + error = obj.GetError() + self.fail(self._formatMessage(msg, + "'{}' is not success".format(error))) + """Assert two states are equal""" def assertState(self, first, second, msg=None): if first != second: diff --git a/lldb/source/Interpreter/CommandAlias.cpp b/lldb/source/Interpreter/CommandAlias.cpp index d55b3fd..e36edca 100644 --- a/lldb/source/Interpreter/CommandAlias.cpp +++ b/lldb/source/Interpreter/CommandAlias.cpp @@ -60,11 +60,12 @@ static bool ProcessAliasOptionsArgs(lldb::CommandObjectSP &cmd_obj_sp, if (!options_string.empty()) { if (cmd_obj_sp->WantsRawCommandString()) - option_arg_vector->emplace_back("", -1, options_string); + option_arg_vector->emplace_back(CommandInterpreter::g_argument, + -1, options_string); else { for (auto &entry : args.entries()) { if (!entry.ref().empty()) - option_arg_vector->emplace_back(std::string(""), -1, + option_arg_vector->emplace_back(std::string(CommandInterpreter::g_argument), -1, std::string(entry.ref())); } } @@ -153,11 +154,12 @@ void CommandAlias::GetAliasExpansion(StreamString &help_string) const { for (const auto &opt_entry : *options) { std::tie(opt, std::ignore, value) = opt_entry; - if (opt == "") { + if (opt == CommandInterpreter::g_argument) { help_string.Printf(" %s", value.c_str()); } else { help_string.Printf(" %s", opt.c_str()); - if ((value != "") && (value != "" && !value.empty() && + if (opt == CommandInterpreter::g_argument && !value.empty() && llvm::StringRef(value).endswith("--")) { m_is_dashdash_alias = eLazyBoolYes; break; @@ -206,6 +208,8 @@ std::pair CommandAlias::Desugar() { return {nullptr, nullptr}; if (underlying->IsAlias()) { + // FIXME: This doesn't work if the original alias fills a slot in the + // underlying alias, since this just appends the two lists. auto desugared = ((CommandAlias *)underlying.get())->Desugar(); auto options = GetOptionArguments(); options->insert(options->begin(), desugared.second->begin(), diff --git a/lldb/source/Interpreter/CommandInterpreter.cpp b/lldb/source/Interpreter/CommandInterpreter.cpp index 0596d85..2218d54 100644 --- a/lldb/source/Interpreter/CommandInterpreter.cpp +++ b/lldb/source/Interpreter/CommandInterpreter.cpp @@ -105,6 +105,11 @@ static constexpr const char *InitFileWarning = "and\n" "accept the security risk."; +constexpr const char *CommandInterpreter::g_no_argument = ""; +constexpr const char *CommandInterpreter::g_need_argument = ""; +constexpr const char *CommandInterpreter::g_argument = ""; + + #define LLDB_PROPERTIES_interpreter #include "InterpreterProperties.inc" @@ -1634,7 +1639,7 @@ CommandObject *CommandInterpreter::BuildAliasResult( std::string value; for (const auto &entry : *option_arg_vector) { std::tie(option, value_type, value) = entry; - if (option == "") { + if (option == g_argument) { result_str.Printf(" %s", value.c_str()); continue; } @@ -1656,11 +1661,33 @@ CommandObject *CommandInterpreter::BuildAliasResult( index); return nullptr; } else { - size_t strpos = raw_input_string.find(cmd_args.GetArgumentAtIndex(index)); - if (strpos != std::string::npos) + const Args::ArgEntry &entry = cmd_args[index]; + size_t strpos = raw_input_string.find(entry.c_str()); + const char quote_char = entry.GetQuoteChar(); + if (strpos != std::string::npos) { + const size_t start_fudge = quote_char == '\0' ? 0 : 1; + const size_t len_fudge = quote_char == '\0' ? 0 : 2; + + // Make sure we aren't going outside the bounds of the cmd string: + if (strpos < start_fudge) { + result.AppendError("Unmatched quote at command beginning."); + return nullptr; + } + llvm::StringRef arg_text = entry.ref(); + if (strpos - start_fudge + arg_text.size() + len_fudge + > raw_input_string.size()) { + result.AppendError("Unmatched quote at command end."); + return nullptr; + } raw_input_string = raw_input_string.erase( - strpos, strlen(cmd_args.GetArgumentAtIndex(index))); - result_str.Printf("%s", cmd_args.GetArgumentAtIndex(index)); + strpos - start_fudge, + strlen(cmd_args.GetArgumentAtIndex(index)) + len_fudge); + } + if (quote_char == '\0') + result_str.Printf("%s", cmd_args.GetArgumentAtIndex(index)); + else + result_str.Printf("%c%s%c", quote_char, + entry.c_str(), quote_char); } } @@ -1911,13 +1938,6 @@ bool CommandInterpreter::HandleCommand(const char *command_line, return true; } - Status error(PreprocessCommand(command_string)); - - if (error.Fail()) { - result.AppendError(error.AsCString()); - return false; - } - // Phase 1. // Before we do ANY kind of argument processing, we need to figure out what @@ -1935,6 +1955,20 @@ bool CommandInterpreter::HandleCommand(const char *command_line, CommandObject *cmd_obj = ResolveCommandImpl(command_string, result); + // We have to preprocess the whole command string for Raw commands, since we + // don't know the structure of the command. For parsed commands, we only + // treat backticks as quote characters specially. + // FIXME: We probably want to have raw commands do their own preprocessing. + // For instance, I don't think people expect substitution in expr expressions. + if (cmd_obj && cmd_obj->WantsRawCommandString()) { + Status error(PreprocessCommand(command_string)); + + if (error.Fail()) { + result.AppendError(error.AsCString()); + return false; + } + } + // Although the user may have abbreviated the command, the command_string now // has the command expanded to the full name. For example, if the input was // "br s -n main", command_string is now "breakpoint set -n main". @@ -2163,7 +2197,7 @@ void CommandInterpreter::BuildAliasCommandArgs(CommandObject *alias_cmd_obj, std::string value; for (const auto &option_entry : *option_arg_vector) { std::tie(option, value_type, value) = option_entry; - if (option == "") { + if (option == g_argument) { if (!wants_raw_input || (value != "--")) { // Since we inserted this above, make sure we don't insert it twice new_args.AppendArgument(value); @@ -2174,7 +2208,7 @@ void CommandInterpreter::BuildAliasCommandArgs(CommandObject *alias_cmd_obj, if (value_type != OptionParser::eOptionalArgument) new_args.AppendArgument(option); - if (value == "") + if (value == g_no_argument) continue; int index = GetOptionArgumentPosition(value.c_str()); diff --git a/lldb/source/Interpreter/CommandObject.cpp b/lldb/source/Interpreter/CommandObject.cpp index 5ab3392..4500378 100644 --- a/lldb/source/Interpreter/CommandObject.cpp +++ b/lldb/source/Interpreter/CommandObject.cpp @@ -727,7 +727,7 @@ bool CommandObjectParsed::Execute(const char *args_string, } if (!handled) { for (auto entry : llvm::enumerate(cmd_args.entries())) { - if (!entry.value().ref().empty() && entry.value().ref().front() == '`') { + if (!entry.value().ref().empty() && entry.value().GetQuoteChar() == '`') { cmd_args.ReplaceArgumentAtIndex( entry.index(), m_interpreter.ProcessEmbeddedScriptCommands(entry.value().c_str())); diff --git a/lldb/source/Interpreter/Options.cpp b/lldb/source/Interpreter/Options.cpp index 09af51a..ae36250 100644 --- a/lldb/source/Interpreter/Options.cpp +++ b/lldb/source/Interpreter/Options.cpp @@ -1002,37 +1002,47 @@ llvm::Expected Options::ParseAlias(const Args &args, .str(), llvm::inconvertibleErrorCode()); } - if (!option_arg) - option_arg = ""; - option_arg_vector->emplace_back(std::string(option_str.GetString()), - has_arg, std::string(option_arg)); - // Find option in the argument list; also see if it was supposed to take an // argument and if one was supplied. Remove option (and argument, if // given) from the argument list. Also remove them from the // raw_input_string, if one was passed in. + // Note: We also need to preserve any option argument values that were + // surrounded by backticks, as we lose track of them in the + // option_args_vector. size_t idx = FindArgumentIndexForOption(args_copy, long_options[long_options_index]); + std::string option_to_insert; + if (option_arg) { + if (idx != size_t(-1) && has_arg) { + bool arg_has_backtick = args_copy[idx + 1].GetQuoteChar() == '`'; + if (arg_has_backtick) + option_to_insert = "`"; + option_to_insert += option_arg; + if (arg_has_backtick) + option_to_insert += "`"; + } else + option_to_insert = option_arg; + } else + option_to_insert = CommandInterpreter::g_no_argument; + + option_arg_vector->emplace_back(std::string(option_str.GetString()), + has_arg, option_to_insert); + if (idx == size_t(-1)) continue; if (!input_line.empty()) { - auto tmp_arg = args_copy[idx].ref(); + llvm::StringRef tmp_arg = args_copy[idx].ref(); size_t pos = input_line.find(std::string(tmp_arg)); if (pos != std::string::npos) input_line.erase(pos, tmp_arg.size()); } args_copy.DeleteArgumentAtIndex(idx); - if ((long_options[long_options_index].definition->option_has_arg != - OptionParser::eNoArgument) && - (OptionParser::GetOptionArgument() != nullptr) && - (idx < args_copy.GetArgumentCount()) && - (args_copy[idx].ref() == OptionParser::GetOptionArgument())) { + if (option_to_insert != CommandInterpreter::g_no_argument) { if (input_line.size() > 0) { - auto tmp_arg = args_copy[idx].ref(); - size_t pos = input_line.find(std::string(tmp_arg)); + size_t pos = input_line.find(option_to_insert); if (pos != std::string::npos) - input_line.erase(pos, tmp_arg.size()); + input_line.erase(pos, option_to_insert.size()); } args_copy.DeleteArgumentAtIndex(idx); } diff --git a/lldb/source/Utility/Args.cpp b/lldb/source/Utility/Args.cpp index daccb91..42c84d7 100644 --- a/lldb/source/Utility/Args.cpp +++ b/lldb/source/Utility/Args.cpp @@ -215,7 +215,12 @@ bool Args::GetCommandString(std::string &command) const { for (size_t i = 0; i < m_entries.size(); ++i) { if (i > 0) command += ' '; + char quote = m_entries[i].quote; + if (quote != '\0') + command += quote; command += m_entries[i].ref(); + if (quote != '\0') + command += quote; } return !m_entries.empty(); diff --git a/lldb/test/API/commands/command/backticks/Makefile b/lldb/test/API/commands/command/backticks/Makefile new file mode 100644 index 0000000..1049594 --- /dev/null +++ b/lldb/test/API/commands/command/backticks/Makefile @@ -0,0 +1,3 @@ +C_SOURCES := main.c + +include Makefile.rules diff --git a/lldb/test/API/commands/command/backticks/TestBackticksInAlias.py b/lldb/test/API/commands/command/backticks/TestBackticksInAlias.py new file mode 100644 index 0000000..495e52d --- /dev/null +++ b/lldb/test/API/commands/command/backticks/TestBackticksInAlias.py @@ -0,0 +1,32 @@ +""" +Test that an alias can contain active backticks +""" + + + +import lldb +from lldbsuite.test.lldbtest import * +import lldbsuite.test.lldbutil as lldbutil + + +class TestBackticksInAlias(TestBase): + NO_DEBUG_INFO_TESTCASE = True + + def test_backticks_in_alias(self): + """Test that an alias can contain active backticks.""" + self.build() + (target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint(self, "break here", lldb.SBFileSpec("main.c")) + interp = self.dbg.GetCommandInterpreter(); + result = lldb.SBCommandReturnObject() + interp.HandleCommand('command alias _test-argv-cmd expression -Z \`argc\` -- argv', result) + self.assertCommandReturn(result, "Made the alias") + interp.HandleCommand("_test-argv-cmd", result) + self.assertCommandReturn(result, "The alias worked") + + # Now try a harder case where we create this using an alias: + interp.HandleCommand('command alias _test-argv-parray-cmd parray \`argc\` argv', result) + self.assertCommandReturn(result, "Made the alias") + interp.HandleCommand("_test-argv-parray-cmd", result) + self.assertFalse(result.Succeeded(), "CommandAlias::Desugar currently fails if a alias substitutes %N arguments in another alias") + + diff --git a/lldb/test/API/commands/command/backticks/main.c b/lldb/test/API/commands/command/backticks/main.c new file mode 100644 index 0000000..a3abaaa --- /dev/null +++ b/lldb/test/API/commands/command/backticks/main.c @@ -0,0 +1,5 @@ +int main (int argc, char const *argv[]) +{ + return argc; // break here +} + -- 2.7.4