[Lldb-commits] [lldb] bc0a9a1 - Add an option (-y) to "break set" and "source list" that uses the same

Jim Ingham via lldb-commits lldb-commits at lists.llvm.org
Mon Jul 20 17:40:59 PDT 2020


Author: Jim Ingham
Date: 2020-07-20T17:40:36-07:00
New Revision: bc0a9a17a4a658153f4b524da3274d33a98d1c5b

URL: https://github.com/llvm/llvm-project/commit/bc0a9a17a4a658153f4b524da3274d33a98d1c5b
DIFF: https://github.com/llvm/llvm-project/commit/bc0a9a17a4a658153f4b524da3274d33a98d1c5b.diff

LOG: 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

Added: 
    lldb/include/lldb/Interpreter/OptionValueFileColonLine.h
    lldb/source/Interpreter/OptionValueFileColonLine.cpp
    lldb/test/API/functionalities/breakpoint/breakpoint_by_file_colon_line/Makefile
    lldb/test/API/functionalities/breakpoint/breakpoint_by_file_colon_line/TestBreakpointByFileColonLine.py
    lldb/test/API/functionalities/breakpoint/breakpoint_by_file_colon_line/main.c
    lldb/unittests/Interpreter/TestOptionValueFileColonLine.cpp

Modified: 
    lldb/include/lldb/Interpreter/OptionValue.h
    lldb/include/lldb/Interpreter/OptionValues.h
    lldb/include/lldb/lldb-defines.h
    lldb/include/lldb/lldb-enumerations.h
    lldb/packages/Python/lldbsuite/test/lldbutil.py
    lldb/source/Commands/CommandObjectBreakpoint.cpp
    lldb/source/Commands/CommandObjectSource.cpp
    lldb/source/Commands/Options.td
    lldb/source/Interpreter/CMakeLists.txt
    lldb/source/Interpreter/CommandObject.cpp
    lldb/source/Interpreter/OptionValue.cpp
    lldb/source/Interpreter/OptionValueArray.cpp
    lldb/source/Interpreter/OptionValueDictionary.cpp
    lldb/source/Interpreter/OptionValueFileSpec.cpp
    lldb/source/Interpreter/Property.cpp
    lldb/test/API/source-manager/TestSourceManager.py
    lldb/unittests/Interpreter/CMakeLists.txt

Removed: 
    


################################################################################
diff  --git a/lldb/include/lldb/Interpreter/OptionValue.h b/lldb/include/lldb/Interpreter/OptionValue.h
index 5b07427094bf..27a5ddea116b 100644
--- a/lldb/include/lldb/Interpreter/OptionValue.h
+++ b/lldb/include/lldb/Interpreter/OptionValue.h
@@ -31,6 +31,7 @@ class OptionValue {
     eTypeChar,
     eTypeDictionary,
     eTypeEnum,
+    eTypeFileLineColumn,
     eTypeFileSpec,
     eTypeFileSpecList,
     eTypeFormat,
@@ -135,6 +136,8 @@ class OptionValue {
       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 000000000000..6285c03afa14
--- /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 36e7c192d60a..6efc9e1ad064 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 fea8079779a1..487cd0b01d5c 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 b3e8d604913f..051289de439d 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 25fcd5f29ee2..1ce6844d973c 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 be7ef8a1b60b..b62fe6c93cd8 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 @@ class CommandObjectBreakpointSet : public CommandObjectParsed {
       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 1ccfd3a5166f..8fff22a06366 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 d6f1e0a3c96d..6384179f0ef1 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 0ed39869467e..7a8c826d040c 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 538f7a1ba693..6f58b8ba0ea7 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 198be85a7b47..0f1030927808 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 9be11e32e2db..0b293ccfc248 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 caadccd04232..79323f502d17 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 000000000000..dac557c4248a
--- /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 15acb7e5e5b0..a03fd55d0385 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 923811249853..a02497662ff5 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<OptionValueFileColonLine>();
+    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 000000000000..ad42b20df4ed
--- /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 000000000000..011fcdce8da4
--- /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 000000000000..6e3f7e2ddbf9
--- /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 714b736da02e..3ce056f2d029 100644
--- a/lldb/test/API/source-manager/TestSourceManager.py
+++ b/lldb/test/API/source-manager/TestSourceManager.py
@@ -197,6 +197,14 @@ def test_modify_source_file_while_debugging(self):
             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 0de5b0b72488..28663ec02a2a 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 000000000000..e5b2c77a1616
--- /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);
+}


        


More information about the lldb-commits mailing list