From bc0a9a17a4a658153f4b524da3274d33a98d1c5b Mon Sep 17 00:00:00 2001 From: Jim Ingham Date: Thu, 16 Jul 2020 11:34:50 -0700 Subject: [PATCH] Add an option (-y) to "break set" and "source list" that uses the same file:line:column form that we use to print out locations. Since we print them this way it makes sense we also accept that form. Differential Revision: https://reviews.llvm.org/D83975 --- lldb/include/lldb/Interpreter/OptionValue.h | 3 + .../lldb/Interpreter/OptionValueFileColonLine.h | 64 +++++++++ lldb/include/lldb/Interpreter/OptionValues.h | 1 + lldb/include/lldb/lldb-defines.h | 2 + lldb/include/lldb/lldb-enumerations.h | 1 + lldb/packages/Python/lldbsuite/test/lldbutil.py | 36 +++++ lldb/source/Commands/CommandObjectBreakpoint.cpp | 18 ++- lldb/source/Commands/CommandObjectSource.cpp | 17 +++ lldb/source/Commands/Options.td | 17 ++- lldb/source/Interpreter/CMakeLists.txt | 1 + lldb/source/Interpreter/CommandObject.cpp | 1 + lldb/source/Interpreter/OptionValue.cpp | 2 + lldb/source/Interpreter/OptionValueArray.cpp | 1 + lldb/source/Interpreter/OptionValueDictionary.cpp | 1 + .../Interpreter/OptionValueFileColonLine.cpp | 145 +++++++++++++++++++++ lldb/source/Interpreter/OptionValueFileSpec.cpp | 7 - lldb/source/Interpreter/Property.cpp | 6 + .../breakpoint_by_file_colon_line/Makefile | 4 + .../TestBreakpointByFileColonLine.py | 42 ++++++ .../breakpoint_by_file_colon_line/main.c | 14 ++ lldb/test/API/source-manager/TestSourceManager.py | 8 ++ lldb/unittests/Interpreter/CMakeLists.txt | 1 + .../Interpreter/TestOptionValueFileColonLine.cpp | 57 ++++++++ 23 files changed, 436 insertions(+), 13 deletions(-) create mode 100644 lldb/include/lldb/Interpreter/OptionValueFileColonLine.h create mode 100644 lldb/source/Interpreter/OptionValueFileColonLine.cpp create mode 100644 lldb/test/API/functionalities/breakpoint/breakpoint_by_file_colon_line/Makefile create mode 100644 lldb/test/API/functionalities/breakpoint/breakpoint_by_file_colon_line/TestBreakpointByFileColonLine.py create mode 100644 lldb/test/API/functionalities/breakpoint/breakpoint_by_file_colon_line/main.c create mode 100644 lldb/unittests/Interpreter/TestOptionValueFileColonLine.cpp diff --git a/lldb/include/lldb/Interpreter/OptionValue.h b/lldb/include/lldb/Interpreter/OptionValue.h index 5b07427..27a5dde 100644 --- a/lldb/include/lldb/Interpreter/OptionValue.h +++ b/lldb/include/lldb/Interpreter/OptionValue.h @@ -31,6 +31,7 @@ public: eTypeChar, eTypeDictionary, eTypeEnum, + eTypeFileLineColumn, eTypeFileSpec, eTypeFileSpecList, eTypeFormat, @@ -135,6 +136,8 @@ public: return eTypeDictionary; case 1u << eTypeEnum: return eTypeEnum; + case 1u << eTypeFileLineColumn: + return eTypeFileLineColumn; case 1u << eTypeFileSpec: return eTypeFileSpec; case 1u << eTypeFileSpecList: diff --git a/lldb/include/lldb/Interpreter/OptionValueFileColonLine.h b/lldb/include/lldb/Interpreter/OptionValueFileColonLine.h new file mode 100644 index 0000000..6285c03 --- /dev/null +++ b/lldb/include/lldb/Interpreter/OptionValueFileColonLine.h @@ -0,0 +1,64 @@ +//===-- OptionValueFileColonLine.h -----------------------------------*- C++ -*-===// +// +// 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 +// +//===----------------------------------------------------------------------===// + +#ifndef LLDB_INTERPRETER_OPTIONVALUEFILECOLONLINE_H +#define LLDB_INTERPRETER_OPTIONVALUEFILECOLONLINE_H + +#include "lldb/Interpreter/OptionValue.h" + +#include "lldb/Utility/FileSpec.h" +#include "llvm/Support/Chrono.h" + +namespace lldb_private { + +class OptionValueFileColonLine : public OptionValue { +public: + OptionValueFileColonLine(); + OptionValueFileColonLine(const llvm::StringRef input); + + ~OptionValueFileColonLine() override {} + + OptionValue::Type GetType() const override { return eTypeFileLineColumn; } + + void DumpValue(const ExecutionContext *exe_ctx, Stream &strm, + uint32_t dump_mask) override; + + Status + SetValueFromString(llvm::StringRef value, + VarSetOperationType op = eVarSetOperationAssign) override; + Status + SetValueFromString(const char *, + VarSetOperationType = eVarSetOperationAssign) = delete; + + bool Clear() override { + m_file_spec.Clear(); + m_line_number = LLDB_INVALID_LINE_NUMBER; + m_column_number = 0; + } + + lldb::OptionValueSP DeepCopy() const override; + + void AutoComplete(CommandInterpreter &interpreter, + CompletionRequest &request) override; + + FileSpec &GetFileSpec() { return m_file_spec; } + uint32_t GetLineNumber() { return m_line_number; } + uint32_t GetColumnNumber() { return m_column_number; } + + void SetCompletionMask(uint32_t mask) { m_completion_mask = mask; } + +protected: + FileSpec m_file_spec; + uint32_t m_line_number; + uint32_t m_column_number; + uint32_t m_completion_mask; +}; + +} // namespace lldb_private + +#endif // LLDB_INTERPRETER_OPTIONVALUEFILECOLONLINE_H diff --git a/lldb/include/lldb/Interpreter/OptionValues.h b/lldb/include/lldb/Interpreter/OptionValues.h index 36e7c19..6efc9e1 100644 --- a/lldb/include/lldb/Interpreter/OptionValues.h +++ b/lldb/include/lldb/Interpreter/OptionValues.h @@ -17,6 +17,7 @@ #include "lldb/Interpreter/OptionValueChar.h" #include "lldb/Interpreter/OptionValueDictionary.h" #include "lldb/Interpreter/OptionValueEnumeration.h" +#include "lldb/Interpreter/OptionValueFileColonLine.h" #include "lldb/Interpreter/OptionValueFileSpec.h" #include "lldb/Interpreter/OptionValueFileSpecList.h" #include "lldb/Interpreter/OptionValueFormat.h" diff --git a/lldb/include/lldb/lldb-defines.h b/lldb/include/lldb/lldb-defines.h index fea8079..487cd0b 100644 --- a/lldb/include/lldb/lldb-defines.h +++ b/lldb/include/lldb/lldb-defines.h @@ -95,6 +95,7 @@ #define LLDB_INVALID_SIGNAL_NUMBER INT32_MAX #define LLDB_INVALID_OFFSET UINT64_MAX // Must match max of lldb::offset_t #define LLDB_INVALID_LINE_NUMBER UINT32_MAX +#define LLDB_INVALID_COLUMN_NUMBER 0 #define LLDB_INVALID_QUEUE_ID 0 /// CPU Type definitions @@ -119,6 +120,7 @@ #define LLDB_OPT_SET_9 (1U << 8) #define LLDB_OPT_SET_10 (1U << 9) #define LLDB_OPT_SET_11 (1U << 10) +#define LLDB_OPT_SET_12 (1U << 11) #define LLDB_OPT_SET_FROM_TO(A, B) \ (((1U << (B)) - 1) ^ (((1U << (A)) - 1) >> 1)) diff --git a/lldb/include/lldb/lldb-enumerations.h b/lldb/include/lldb/lldb-enumerations.h index b3e8d60..051289d 100644 --- a/lldb/include/lldb/lldb-enumerations.h +++ b/lldb/include/lldb/lldb-enumerations.h @@ -526,6 +526,7 @@ enum CommandArgumentType { eArgTypeExpression, eArgTypeExpressionPath, eArgTypeExprFormat, + eArgTypeFileLineColumn, eArgTypeFilename, eArgTypeFormat, eArgTypeFrameIndex, diff --git a/lldb/packages/Python/lldbsuite/test/lldbutil.py b/lldb/packages/Python/lldbsuite/test/lldbutil.py index 25fcd5f..1ce6844 100644 --- a/lldb/packages/Python/lldbsuite/test/lldbutil.py +++ b/lldb/packages/Python/lldbsuite/test/lldbutil.py @@ -502,6 +502,29 @@ def run_break_set_by_source_regexp( return get_bpno_from_match(break_results) +def run_break_set_by_file_colon_line( + test, + specifier, + path, + line_number, + column_number = 0, + extra_options=None, + num_expected_locations=-1): + command = 'breakpoint set -y "%s"'%(specifier) + if extra_options: + command += " " + extra_options + + print("About to run: '%s'", command) + break_results = run_break_set_command(test, command) + check_breakpoint_result( + test, + break_results, + num_locations = num_expected_locations, + file_name = path, + line_number = line_number, + column_number = column_number) + + return get_bpno_from_match(break_results) def run_break_set_command(test, command): """Run the command passed in - it must be some break set variant - and analyze the result. @@ -515,6 +538,7 @@ def run_break_set_command(test, command): If there is only one location, the dictionary MAY contain: file - source file name line_no - source line number + column - source column number symbol - symbol name inline_symbol - inlined symbol name offset - offset from the original symbol @@ -566,6 +590,7 @@ def check_breakpoint_result( break_results, file_name=None, line_number=-1, + column_number=0, symbol_name=None, symbol_match_exact=True, module_name=None, @@ -605,6 +630,17 @@ def check_breakpoint_result( (line_number, out_line_number)) + if column_number != 0: + out_column_number = 0 + if 'column' in break_results: + out_column_number = break_results['column'] + + test.assertTrue( + column_number == out_column_number, + "Breakpoint column number %s doesn't match resultant column %s." % + (column_number, + out_column_number)) + if symbol_name: out_symbol_name = "" # Look first for the inlined symbol name, otherwise use the symbol diff --git a/lldb/source/Commands/CommandObjectBreakpoint.cpp b/lldb/source/Commands/CommandObjectBreakpoint.cpp index be7ef8a..b62fe6c 100644 --- a/lldb/source/Commands/CommandObjectBreakpoint.cpp +++ b/lldb/source/Commands/CommandObjectBreakpoint.cpp @@ -17,6 +17,7 @@ #include "lldb/Interpreter/OptionArgParser.h" #include "lldb/Interpreter/OptionGroupPythonClassWithDict.h" #include "lldb/Interpreter/OptionValueBoolean.h" +#include "lldb/Interpreter/OptionValueFileColonLine.h" #include "lldb/Interpreter/OptionValueString.h" #include "lldb/Interpreter/OptionValueUInt64.h" #include "lldb/Interpreter/Options.h" @@ -443,7 +444,22 @@ public: case 'X': m_source_regex_func_names.insert(std::string(option_arg)); break; - + + case 'y': + { + OptionValueFileColonLine value; + Status fcl_err = value.SetValueFromString(option_arg); + if (!fcl_err.Success()) { + error.SetErrorStringWithFormat( + "Invalid value for file:line specifier: %s", + fcl_err.AsCString()); + } else { + m_filenames.AppendIfUnique(value.GetFileSpec()); + m_line_num = value.GetLineNumber(); + m_column = value.GetColumnNumber(); + } + } break; + default: llvm_unreachable("Unimplemented option"); } diff --git a/lldb/source/Commands/CommandObjectSource.cpp b/lldb/source/Commands/CommandObjectSource.cpp index 1ccfd3a..8fff22a 100644 --- a/lldb/source/Commands/CommandObjectSource.cpp +++ b/lldb/source/Commands/CommandObjectSource.cpp @@ -16,6 +16,7 @@ #include "lldb/Host/OptionParser.h" #include "lldb/Interpreter/CommandReturnObject.h" #include "lldb/Interpreter/OptionArgParser.h" +#include "lldb/Interpreter/OptionValueFileColonLine.h" #include "lldb/Interpreter/Options.h" #include "lldb/Symbol/CompileUnit.h" #include "lldb/Symbol/Function.h" @@ -667,6 +668,22 @@ class CommandObjectSourceList : public CommandObjectParsed { case 'r': reverse = true; break; + case 'y': + { + OptionValueFileColonLine value; + Status fcl_err = value.SetValueFromString(option_arg); + if (!fcl_err.Success()) { + error.SetErrorStringWithFormat( + "Invalid value for file:line specifier: %s", + fcl_err.AsCString()); + } else { + file_name = value.GetFileSpec().GetPath(); + start_line = value.GetLineNumber(); + // I don't see anything useful to do with a column number, but I don't + // want to complain since someone may well have cut and pasted a + // listing from somewhere that included a column. + } + } break; default: llvm_unreachable("Unimplemented option"); } diff --git a/lldb/source/Commands/Options.td b/lldb/source/Commands/Options.td index d6f1e0a..6384179 100644 --- a/lldb/source/Commands/Options.td +++ b/lldb/source/Commands/Options.td @@ -105,7 +105,7 @@ let Command = "breakpoint dummy" in { let Command = "breakpoint set" in { def breakpoint_set_shlib : Option<"shlib", "s">, Arg<"ShlibName">, - Completion<"Module">, Groups<[1,2,3,4,5,6,7,8,9,11]>, // *not* in group 10 + Completion<"Module">, Groups<[1,2,3,4,5,6,7,8,9,11,12]>, // *not* in group 10 Desc<"Set the breakpoint only in this shared library. Can repeat this " "option multiple times to specify multiple shared libraries.">; def breakpoint_set_hardware : Option<"hardware", "H">, @@ -186,21 +186,24 @@ let Command = "breakpoint set" in { "expression (note: currently only implemented for setting breakpoints on " "identifiers). If not set the target.language setting is used.">; def breakpoint_set_skip_prologue : Option<"skip-prologue", "K">, - Arg<"Boolean">, Groups<[1,3,4,5,6,7,8]>, + Arg<"Boolean">, Groups<[1,3,4,5,6,7,8,12]>, Desc<"sKip the prologue if the breakpoint is at the beginning of a " "function. If not set the target.skip-prologue setting is used.">; def breakpoint_set_breakpoint_name : Option<"breakpoint-name", "N">, Arg<"BreakpointName">, Desc<"Adds this to the list of names for this breakpoint.">; def breakpoint_set_address_slide : Option<"address-slide", "R">, - Arg<"Address">, Groups<[1,3,4,5,6,7,8]>, + Arg<"Address">, Groups<[1,3,4,5,6,7,8,12]>, Desc<"Add the specified offset to whatever address(es) the breakpoint " "resolves to. At present this applies the offset directly as given, and " "doesn't try to align it to instruction boundaries.">; def breakpoint_set_move_to_nearest_code : Option<"move-to-nearest-code", "m">, - Groups<[1, 9]>, Arg<"Boolean">, + Groups<[1,9,12]>, Arg<"Boolean">, Desc<"Move breakpoints to nearest code. If not set the " "target.move-to-nearest-codesetting is used.">; + def breakpoint_set_file_colon_line : Option<"joint-specifier", "y">, Group<12>, Arg<"FileLineColumn">, + Required, Completion<"SourceFile">, + Desc<"A specifier in the form filename:line[:column] for setting file & line breakpoints.">; /* Don't add this option till it actually does something useful... def breakpoint_set_exception_typename : Option<"exception-typename", "O">, Arg<"TypeName">, Desc<"The breakpoint will only stop if an " @@ -729,7 +732,7 @@ let Command = "source info" in { let Command = "source list" in { def source_list_count : Option<"count", "c">, Arg<"Count">, Desc<"The number of source lines to display.">; - def source_list_shlib : Option<"shlib", "s">, Groups<[1,2]>, Arg<"ShlibName">, + def source_list_shlib : Option<"shlib", "s">, Groups<[1,2,5]>, Arg<"ShlibName">, Completion<"Module">, Desc<"Look up the source file in the given shared library.">; def source_list_show_breakpoints : Option<"show-breakpoints", "b">, @@ -747,6 +750,10 @@ let Command = "source list" in { " information for the corresponding file and line.">; def source_list_reverse : Option<"reverse", "r">, Group<4>, Desc<"Reverse the" " listing to look backwards from the last displayed block of source.">; + def source_list_file_colon_line : Option<"joint-specifier", "y">, Group<5>, + Arg<"FileLineColumn">, Completion<"SourceFile">, + Desc<"A specifier in the form filename:line[:column] from which to display" + " source.">; } let Command = "target dependents" in { diff --git a/lldb/source/Interpreter/CMakeLists.txt b/lldb/source/Interpreter/CMakeLists.txt index 0ed3986..7a8c826 100644 --- a/lldb/source/Interpreter/CMakeLists.txt +++ b/lldb/source/Interpreter/CMakeLists.txt @@ -35,6 +35,7 @@ add_lldb_library(lldbInterpreter OptionValueChar.cpp OptionValueDictionary.cpp OptionValueEnumeration.cpp + OptionValueFileColonLine.cpp OptionValueFileSpec.cpp OptionValueFileSpecList.cpp OptionValueFormat.cpp diff --git a/lldb/source/Interpreter/CommandObject.cpp b/lldb/source/Interpreter/CommandObject.cpp index 538f7a1..6f58b8b 100644 --- a/lldb/source/Interpreter/CommandObject.cpp +++ b/lldb/source/Interpreter/CommandObject.cpp @@ -1064,6 +1064,7 @@ CommandObject::ArgumentTableEntry CommandObject::g_arguments_data[] = { { eArgTypeIndex, "index", CommandCompletions::eNoCompletion, { nullptr, false }, "An index into a list." }, { eArgTypeLanguage, "source-language", CommandCompletions::eNoCompletion, { LanguageTypeHelpTextCallback, true }, nullptr }, { eArgTypeLineNum, "linenum", CommandCompletions::eNoCompletion, { nullptr, false }, "Line number in a source file." }, + { eArgTypeFileLineColumn, "linespec", CommandCompletions::eNoCompletion, { nullptr, false }, "A source specifier in the form file:line[:column]" }, { eArgTypeLogCategory, "log-category", CommandCompletions::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." }, { eArgTypeLogChannel, "log-channel", CommandCompletions::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)." }, { eArgTypeMethod, "method", CommandCompletions::eNoCompletion, { nullptr, false }, "A C++ method name." }, diff --git a/lldb/source/Interpreter/OptionValue.cpp b/lldb/source/Interpreter/OptionValue.cpp index 198be85..0f10309 100644 --- a/lldb/source/Interpreter/OptionValue.cpp +++ b/lldb/source/Interpreter/OptionValue.cpp @@ -471,6 +471,8 @@ const char *OptionValue::GetBuiltinTypeAsCString(Type t) { return "dictionary"; case eTypeEnum: return "enum"; + case eTypeFileLineColumn: + return "file:line:column specifier"; case eTypeFileSpec: return "file"; case eTypeFileSpecList: diff --git a/lldb/source/Interpreter/OptionValueArray.cpp b/lldb/source/Interpreter/OptionValueArray.cpp index 9be11e3..0b293cc 100644 --- a/lldb/source/Interpreter/OptionValueArray.cpp +++ b/lldb/source/Interpreter/OptionValueArray.cpp @@ -52,6 +52,7 @@ void OptionValueArray::DumpValue(const ExecutionContext *exe_ctx, Stream &strm, case eTypeChar: case eTypeEnum: case eTypeFileSpec: + case eTypeFileLineColumn: case eTypeFormat: case eTypeSInt64: case eTypeString: diff --git a/lldb/source/Interpreter/OptionValueDictionary.cpp b/lldb/source/Interpreter/OptionValueDictionary.cpp index caadccd..79323f5 100644 --- a/lldb/source/Interpreter/OptionValueDictionary.cpp +++ b/lldb/source/Interpreter/OptionValueDictionary.cpp @@ -62,6 +62,7 @@ void OptionValueDictionary::DumpValue(const ExecutionContext *exe_ctx, case eTypeBoolean: case eTypeChar: case eTypeEnum: + case eTypeFileLineColumn: case eTypeFileSpec: case eTypeFormat: case eTypeSInt64: diff --git a/lldb/source/Interpreter/OptionValueFileColonLine.cpp b/lldb/source/Interpreter/OptionValueFileColonLine.cpp new file mode 100644 index 0000000..dac557c --- /dev/null +++ b/lldb/source/Interpreter/OptionValueFileColonLine.cpp @@ -0,0 +1,145 @@ +//===-- OptionValueFileColonLine.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 "lldb/Interpreter/OptionValueFileColonLine.h" + +#include "lldb/DataFormatters/FormatManager.h" +#include "lldb/Interpreter/CommandCompletions.h" +#include "lldb/Interpreter/CommandInterpreter.h" +#include "lldb/Utility/Args.h" +#include "lldb/Utility/State.h" + +using namespace lldb; +using namespace lldb_private; + +// This is an OptionValue for parsing file:line:column specifications. +// I set the completer to "source file" which isn't quite right, but we can +// only usefully complete in the file name part of it so it should be good +// enough. +OptionValueFileColonLine::OptionValueFileColonLine() + : OptionValue(), m_file_spec(), m_line_number(LLDB_INVALID_LINE_NUMBER), + m_column_number(LLDB_INVALID_COLUMN_NUMBER), + m_completion_mask(CommandCompletions::eSourceFileCompletion) {} + +OptionValueFileColonLine::OptionValueFileColonLine(llvm::StringRef input) + : OptionValue(), m_file_spec(), m_line_number(LLDB_INVALID_LINE_NUMBER), + m_column_number(LLDB_INVALID_COLUMN_NUMBER), + m_completion_mask(CommandCompletions::eSourceFileCompletion) { + SetValueFromString(input, eVarSetOperationAssign); +} + +void OptionValueFileColonLine::DumpValue(const ExecutionContext *exe_ctx, + Stream &strm, uint32_t dump_mask) { + if (dump_mask & eDumpOptionType) + strm.Printf("(%s)", GetTypeAsCString()); + if (dump_mask & eDumpOptionValue) { + if (dump_mask & eDumpOptionType) + strm.PutCString(" = "); + + if (m_file_spec) + strm << '"' << m_file_spec.GetPath().c_str() << '"'; + if (m_line_number != LLDB_INVALID_LINE_NUMBER) + strm.Printf(":%d", m_line_number); + if (m_column_number != LLDB_INVALID_COLUMN_NUMBER) + strm.Printf(":%d", m_column_number); + } +} + +Status OptionValueFileColonLine::SetValueFromString(llvm::StringRef value, + VarSetOperationType op) { + Status error; + switch (op) { + case eVarSetOperationClear: + Clear(); + NotifyValueChanged(); + break; + + case eVarSetOperationReplace: + case eVarSetOperationAssign: + if (value.size() > 0) { + // This is in the form filename:linenumber:column. + // I wish we could use filename:linenumber.column, that would make the + // parsing unambiguous and so much easier... + // But clang & gcc both print the output with two : so we're stuck with + // the two colons. Practically, the only actual ambiguity this introduces + // is with files like "foo:10", which doesn't seem terribly likely. + + // Providing the column is optional, so the input value might have one or + // two colons. First pick off the last colon separated piece. + // It has to be there, since the line number is required: + llvm::StringRef last_piece; + llvm::StringRef left_of_last_piece; + + std::tie(left_of_last_piece, last_piece) = value.rsplit(':'); + if (last_piece.empty()) { + error.SetErrorStringWithFormat("Line specifier must include file and " + "line: '%s'", + value.str().c_str()); + return error; + } + + // Now see if there's another colon and if so pull out the middle piece: + // Then check whether the middle piece is an integer. If it is, then it + // was the line number, and if it isn't we're going to assume that there + // was a colon in the filename (see note at the beginning of the function) + // and ignore it. + llvm::StringRef file_name; + llvm::StringRef middle_piece; + + std::tie(file_name, middle_piece) = left_of_last_piece.rsplit(':'); + if (middle_piece.empty() || !llvm::to_integer(middle_piece, + m_line_number)) { + // The middle piece was empty or not an integer, so there were only two + // legit pieces; our original division was right. Reassign the file + // name and pull out the line number: + file_name = left_of_last_piece; + if (!llvm::to_integer(last_piece, m_line_number)) { + error.SetErrorStringWithFormat("Bad line number value '%s' in: '%s'", + last_piece.str().c_str(), + value.str().c_str()); + return error; + } + } else { + // There were three pieces, and we've got the line number. So now + // we just need to check the column number which was the last peice. + if (!llvm::to_integer(last_piece, m_column_number)) { + error.SetErrorStringWithFormat("Bad column value '%s' in: '%s'", + last_piece.str().c_str(), + value.str().c_str()); + return error; + } + } + + m_value_was_set = true; + m_file_spec.SetFile(file_name, FileSpec::Style::native); + NotifyValueChanged(); + } else { + error.SetErrorString("invalid value string"); + } + break; + + case eVarSetOperationInsertBefore: + case eVarSetOperationInsertAfter: + case eVarSetOperationRemove: + case eVarSetOperationAppend: + case eVarSetOperationInvalid: + error = OptionValue::SetValueFromString(value, op); + break; + } + return error; +} + +lldb::OptionValueSP OptionValueFileColonLine::DeepCopy() const { + return OptionValueSP(new OptionValueFileColonLine(*this)); +} + +void OptionValueFileColonLine::AutoComplete(CommandInterpreter &interpreter, + CompletionRequest &request) { + CommandCompletions::InvokeCommonCompletionCallbacks( + interpreter, m_completion_mask, request, nullptr); +} diff --git a/lldb/source/Interpreter/OptionValueFileSpec.cpp b/lldb/source/Interpreter/OptionValueFileSpec.cpp index 15acb7e..a03fd55 100644 --- a/lldb/source/Interpreter/OptionValueFileSpec.cpp +++ b/lldb/source/Interpreter/OptionValueFileSpec.cpp @@ -64,13 +64,6 @@ Status OptionValueFileSpec::SetValueFromString(llvm::StringRef value, case eVarSetOperationReplace: case eVarSetOperationAssign: if (value.size() > 0) { - // The setting value may have whitespace, double-quotes, or single-quotes - // around the file path to indicate that internal spaces are not word - // breaks. Strip off any ws & quotes from the start and end of the file - // path - we aren't doing any word // breaking here so the quoting is - // unnecessary. NB this will cause a problem if someone tries to specify - // a file path that legitimately begins or ends with a " or ' character, - // or whitespace. value = value.trim("\"' \t"); m_value_was_set = true; m_current_value.SetFile(value.str(), FileSpec::Style::native); diff --git a/lldb/source/Interpreter/Property.cpp b/lldb/source/Interpreter/Property.cpp index 9238112..a024976 100644 --- a/lldb/source/Interpreter/Property.cpp +++ b/lldb/source/Interpreter/Property.cpp @@ -99,6 +99,12 @@ Property::Property(const PropertyDefinition &definition) } break; + case OptionValue::eTypeFileLineColumn: + // "definition.default_uint_value" is not used for a + // OptionValue::eTypeFileSpecList + m_value_sp = std::make_shared(); + break; + case OptionValue::eTypeFileSpec: { // "definition.default_uint_value" represents if the // "definition.default_cstr_value" should be resolved or not diff --git a/lldb/test/API/functionalities/breakpoint/breakpoint_by_file_colon_line/Makefile b/lldb/test/API/functionalities/breakpoint/breakpoint_by_file_colon_line/Makefile new file mode 100644 index 0000000..ad42b20 --- /dev/null +++ b/lldb/test/API/functionalities/breakpoint/breakpoint_by_file_colon_line/Makefile @@ -0,0 +1,4 @@ +C_SOURCES := main.c +CFLAGS_EXTRAS := -std=c99 -gcolumn-info + +include Makefile.rules diff --git a/lldb/test/API/functionalities/breakpoint/breakpoint_by_file_colon_line/TestBreakpointByFileColonLine.py b/lldb/test/API/functionalities/breakpoint/breakpoint_by_file_colon_line/TestBreakpointByFileColonLine.py new file mode 100644 index 0000000..011fcdc --- /dev/null +++ b/lldb/test/API/functionalities/breakpoint/breakpoint_by_file_colon_line/TestBreakpointByFileColonLine.py @@ -0,0 +1,42 @@ +""" +Test setting a breakpoint by line and column. +""" + + + +import lldb +from lldbsuite.test.decorators import * +from lldbsuite.test.lldbtest import * +from lldbsuite.test import lldbutil + + +class BreakpointByLineAndColumnTestCase(TestBase): + + mydir = TestBase.compute_mydir(__file__) + + def testBreakpointSpecWithLine(self): + self.build() + exe = self.getBuildArtifact("a.out") + + target = self.dbg.CreateTarget(exe) + self.assertTrue(target, VALID_TARGET) + + # This one should work: + lldbutil.run_break_set_by_file_colon_line(self, "main.c:11", "main.c", 11, num_expected_locations = 1) + # Let's try an illegal specifier to make sure the command fails. I'm not being exhaustive + # since the UnitTest has more bad patterns. I'm just testing that if the SetFromString + # fails, we propagate the error. + self.expect("break set -y 'foo.c'", error=True) + + ## Skip gcc version less 7.1 since it doesn't support -gcolumn-info + @skipIf(compiler="gcc", compiler_version=['<', '7.1']) + def testBreakpointByLine(self): + self.build() + exe = self.getBuildArtifact("a.out") + + target = self.dbg.CreateTarget(exe) + self.assertTrue(target, VALID_TARGET) + + main_c = lldb.SBFileSpec("main.c") + lldbutil.run_break_set_by_file_colon_line(self, "main.c:11:50", "main.c", 11, num_expected_locations = 1) + diff --git a/lldb/test/API/functionalities/breakpoint/breakpoint_by_file_colon_line/main.c b/lldb/test/API/functionalities/breakpoint/breakpoint_by_file_colon_line/main.c new file mode 100644 index 0000000..6e3f7e2 --- /dev/null +++ b/lldb/test/API/functionalities/breakpoint/breakpoint_by_file_colon_line/main.c @@ -0,0 +1,14 @@ +int square(int x) +{ + return x * x; +} + +int main (int argc, char const *argv[]) +{ + int did_call = 0; + + // Line 11. v Column 50. + if(square(argc+1) != 0) { did_call = 1; return square(argc); } + // ^ + return square(0); +} diff --git a/lldb/test/API/source-manager/TestSourceManager.py b/lldb/test/API/source-manager/TestSourceManager.py index 714b736..3ce056f 100644 --- a/lldb/test/API/source-manager/TestSourceManager.py +++ b/lldb/test/API/source-manager/TestSourceManager.py @@ -197,6 +197,14 @@ class SourceManagerTestCase(TestBase): SOURCE_DISPLAYED_CORRECTLY, substrs=['Hello world']) + # Do the same thing with a file & line spec: + self.expect( + "source list -y main-copy.c:%d" % + self.line, + SOURCE_DISPLAYED_CORRECTLY, + substrs=['Hello world']) + + # The '-b' option shows the line table locations from the debug information # that indicates valid places to set source level breakpoints. diff --git a/lldb/unittests/Interpreter/CMakeLists.txt b/lldb/unittests/Interpreter/CMakeLists.txt index 0de5b0b..28663ec 100644 --- a/lldb/unittests/Interpreter/CMakeLists.txt +++ b/lldb/unittests/Interpreter/CMakeLists.txt @@ -1,6 +1,7 @@ add_lldb_unittest(InterpreterTests TestCompletion.cpp TestOptionArgParser.cpp + TestOptionValueFileColonLine.cpp LINK_LIBS lldbInterpreter diff --git a/lldb/unittests/Interpreter/TestOptionValueFileColonLine.cpp b/lldb/unittests/Interpreter/TestOptionValueFileColonLine.cpp new file mode 100644 index 0000000..e5b2c77 --- /dev/null +++ b/lldb/unittests/Interpreter/TestOptionValueFileColonLine.cpp @@ -0,0 +1,57 @@ +//===-- ArgsTest.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 "lldb/Interpreter/OptionValueFileColonLine.h" +#include "lldb/Utility/FileSpec.h" +#include "lldb/Utility/Status.h" +#include "gtest/gtest.h" + +using namespace lldb_private; + +void CheckSetting(const char *input, bool success, const char *path = nullptr, + uint32_t line_number = LLDB_INVALID_ADDRESS, + uint32_t column_number = 0) { + + OptionValueFileColonLine value; + Status error; + llvm::StringRef s_ref(input); + error = value.SetValueFromString(s_ref); + ASSERT_EQ(error.Success(), success); + + // If we were meant to fail, we don't need to do more checks: + if (!success) + return; + + ASSERT_EQ(value.GetLineNumber(), line_number); + ASSERT_EQ(value.GetColumnNumber(), column_number); + std::string value_path = value.GetFileSpec().GetPath(); + ASSERT_STREQ(value_path.c_str(), path); +} + +TEST(OptionValueFileColonLine, setFromString) { + OptionValueFileColonLine value; + Status error; + + // Make sure a default constructed value is invalid: + ASSERT_EQ(value.GetLineNumber(), LLDB_INVALID_LINE_NUMBER); + ASSERT_EQ(value.GetColumnNumber(), 0); + ASSERT_FALSE(value.GetFileSpec()); + + // Make sure it is an error to pass a specifier with no line number: + CheckSetting("foo.c", false); + + // Now try with just a file & line: + CheckSetting("foo.c:12", true, "foo.c", 12); + CheckSetting("foo.c:12:20", true, "foo.c", 12, 20); + // Make sure a colon doesn't mess us up: + CheckSetting("foo:bar.c:12", true, "foo:bar.c", 12); + CheckSetting("foo:bar.c:12:20", true, "foo:bar.c", 12, 20); + // Try errors in the line number: + CheckSetting("foo.c:12c", false); + CheckSetting("foo.c:12:20c", false); +} -- 2.7.4