Add an option to "break set" and "source list" that takes a line spec in the form file:line:column

[jingham]

Summary

I think it is important to have separate the file & line specifiers in "break set" because that way no matter what you've named your file, it will always be easy to pass it to lldb. OTOH, for most filenames the "file:line:column" format is unambiguous, and we do print line spec's as "foo.c:10:20" so by rights there should be a way to use that specification.

This patch adds another option to break set (and also source list) that takes a specification of this form. I called it "-y" partly because there aren't that many letters left to use in "break set". Also, y is pretty easy to type, and it's the Spanish equivalent of "and", and since this is a "file AND line" specifier, that provides not too bad of a mnemonic.

For now I reused the source file completer for this, since if you are typing the source file part, that will do the right thing, and we don't do much useful completion on the line & column at present. It would be a nice polish to have another completer that put a ":" after the filename it completes, but I'm not going to do that in this patch. Even nicer would be to have a completer that would complete the line & column entries to only the file/line/column combinations that have line table entries. But that's definitely another patch.

[JDevlieghere]

lldb/include/lldb/Interpreter/OptionValueFileColonLine.h

2

This actually looks like a legit issue.

27

I think these comments add very little, if anything. The override keyword already conveys the same information. Same for the comment below.

44

This should be LLDB_INVALID_COLUMN_NUMBER and probably also be UINT32_MAX unless we rely on it being 0 anywhere?

lldb/include/lldb/lldb-enumerations.h

541

FileLineColumn maybe to match the other enum value?

lldb/source/Interpreter/OptionValue.cpp

474

Can we rename this to eTypeFileLineColumn to make it explicit that it can include all three?

lldb/source/Interpreter/OptionValueFileColonLine.cpp

2

Can you please clang format this file? I know you don't (always) agree but otherwise there are going to be a bunch of unrelated changes when someone else touches (part of) this file.

46

spurious newline?

68

period

75

I think parsing this would be a lot easier with a regex: ([^:]+):(\d+)(:(\d+))?. What do you think?

[jingham]

Address review comments.

[jingham]

lldb/include/lldb/Interpreter/OptionValueFileColonLine.h

27

I removed them here, but note they were just copied from OptionValueFileSpec.h. We started with some header file template that had these way back in the day, and they are all over. If we don't want them around then it would be better to just do that as a patch and not piecemeal.

lldb/source/Interpreter/OptionValueFileColonLine.cpp

68

Ah, nuts. I forgot the period. I'll add it before checking this in.

75

A regex seems overpowered for what I'm doing here, plus that might tempt people to add more stuff to the specifier and I don't want to back into -y being the gdb breakpoint specifier mini-language...

[jingham]

lldb/include/lldb/Interpreter/OptionValueFileColonLine.h

44

The code before this change was using 0 as the invalid value, and there was no define. I added the define and set it to 0, but I haven't propagated that everywhere it is used. That's an orthogonal cleanup which I'd rather do separately.

[labath]

lldb/source/Interpreter/OptionValueFileColonLine.cpp

75

I am not a fan of regex parsing, but I still can't escape the feeling that this should be easier. Maybe a utility function would help:

bool chop_number(StringRef &str, uint32_t &num) {
  auto parts = str.rsplit(':');
  if (to_integer(parts.second, num)) {
    str = parts.first;
    return true;
  }
  return false;
}
...
if (!input.contains(':')
  one_colon_expected();
if (!chop_number(input, col))
  bad_line_number(); // This complains about line numbers because col will be promoted to a line number if the second chop_number fails.
if (!chop_number(input, line)) {
  line = col;
  col = 0;
}
file = input;

121–128

This seems excessive. The command interpreter should already handle the word splitting and quotes, which means that things like br set -y "f i l e.cpp":12:23 will work without this. And it can handle escaping properly, so br set -y "\"file.cpp:12:23" would work, if this code wouldn't get in the way.

130

SetFile already accepts a StringRef.

[jingham]

Address review comments

[jingham]

lldb/source/Interpreter/OptionValueFileColonLine.cpp

75

The only tricky bit was making sure that you got the right error, and were able to show the string that was wrong. I tried your approach but also passing around the found string bit, but it really didn't make anything clearer.

But I reworked it a little to make the logic clearer and added some comments. This version seems pretty straightforward to me.

121–128

I wondered about that too. This is what OptionValueFileSpec did - I started this one from there. So I removed it from both places, and the testsuite is still okay with it.

[labath]

Looks fine to me, modulo the clang-format issues.

lldb/source/Interpreter/OptionValueFileColonLine.cpp

75

Yeah, this looks better. I see now the reason for some of that complexity -- you wanted foo:2b:47 to be considered invalid instead of as "line 47 in file foo:2b -- which seems reasonable.

97

This is one of the reasons I'd recommend llvm::to_integer over StringRef::getAsInteger.

lldb/unittests/Interpreter/TestOptionValueFileColonLine.cpp

38

unused variable

[JDevlieghere]

What Pavel said :-)

lldb/include/lldb/Interpreter/OptionValue.h

enum Type {
     eTypeInvalid = 0,
     eTypeArch,
     eTypeArgs,
     eTypeArray,
     eTypeBoolean,
     eTypeChar,
     eTypeDictionary,
     eTypeEnum,
+    eTypeFileLineColumn,
     eTypeFileSpec,
     eTypeFileSpecList,
     eTypeFormat,
     eTypeLanguage,
     eTypePathMap,
     eTypeProperties,
     eTypeRegex,
     eTypeSInt64,
static OptionValue::Type ConvertTypeMaskToType(uint32_t type_mask) {
     case 1u << eTypeBoolean:
       return eTypeBoolean;
     case 1u << eTypeChar:
       return eTypeChar;
     case 1u << eTypeDictionary:
       return eTypeDictionary;
     case 1u << eTypeEnum:
       return eTypeEnum;
+    case 1u << eTypeFileLineColumn:
+      return eTypeFileLineColumn;
     case 1u << eTypeFileSpec:
       return eTypeFileSpec;
     case 1u << eTypeFileSpecList:
       return eTypeFileSpecList;
     case 1u << eTypeFormat:
       return eTypeFormat;
     case 1u << eTypeLanguage:
       return eTypeLanguage;

lldb/include/lldb/Interpreter/OptionValueFileColonLine.h

+//===-- 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

lldb/include/lldb/Interpreter/OptionValues.h

 #include "lldb/Interpreter/OptionValue.h"
 #include "lldb/Interpreter/OptionValueArch.h"
 #include "lldb/Interpreter/OptionValueArgs.h"
 #include "lldb/Interpreter/OptionValueArray.h"
 #include "lldb/Interpreter/OptionValueBoolean.h"
 #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"
 #include "lldb/Interpreter/OptionValueFormatEntity.h"
 #include "lldb/Interpreter/OptionValueLanguage.h"
 #include "lldb/Interpreter/OptionValuePathMappings.h"
 #include "lldb/Interpreter/OptionValueProperties.h"
 #include "lldb/Interpreter/OptionValueRegex.h"
 #include "lldb/Interpreter/OptionValueSInt64.h"
 #include "lldb/Interpreter/OptionValueString.h"
 #include "lldb/Interpreter/OptionValueUInt64.h"
 #include "lldb/Interpreter/OptionValueUUID.h"
 
 #endif // LLDB_INTERPRETER_OPTIONVALUES_H

lldb/include/lldb/lldb-defines.h

 #define LLDB_INVALID_REGNUM UINT32_MAX
 #define LLDB_INVALID_UID UINT64_MAX
 #define LLDB_INVALID_PROCESS_ID 0
 #define LLDB_INVALID_THREAD_ID 0
 #define LLDB_INVALID_FRAME_ID UINT32_MAX
 #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
 #define LLDB_ARCH_DEFAULT "systemArch"
 #define LLDB_ARCH_DEFAULT_32BIT "systemArch32"
 #define LLDB_ARCH_DEFAULT_64BIT "systemArch64"
 #define LLDB_INVALID_CPUTYPE (0xFFFFFFFEu)
 
 /// Option Set definitions
 // FIXME: I'm sure there's some #define magic that can create all 32 sets on the
 // fly.  That would have the added benefit of making this unreadable.
 #define LLDB_MAX_NUM_OPTION_SETS 32
 #define LLDB_OPT_SET_ALL 0xFFFFFFFFU
 #define LLDB_OPT_SET_1 (1U << 0)
 #define LLDB_OPT_SET_2 (1U << 1)
 #define LLDB_OPT_SET_3 (1U << 2)
 #define LLDB_OPT_SET_4 (1U << 3)
 #define LLDB_OPT_SET_5 (1U << 4)
 #define LLDB_OPT_SET_6 (1U << 5)
 #define LLDB_OPT_SET_7 (1U << 6)
 #define LLDB_OPT_SET_8 (1U << 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))
 
 #if defined(_WIN32) && !defined(MAX_PATH)
 #define MAX_PATH 260
 #endif
 
 // ignore GCC function attributes
 #if defined(_MSC_VER) && !defined(__clang__)
 #define __attribute__(X)
 #endif
 
 #define UNUSED_IF_ASSERT_DISABLED(x) ((void)(x))
 
 #endif // LLDB_LLDB_DEFINES_H

lldb/include/lldb/lldb-enumerations.h

enum CommandArgumentType {
   eArgTypeCount,
   eArgTypeDescriptionVerbosity,
   eArgTypeDirectoryName,
   eArgTypeDisassemblyFlavor,
   eArgTypeEndAddress,
   eArgTypeExpression,
   eArgTypeExpressionPath,
   eArgTypeExprFormat,
+  eArgTypeFileLineColumn,
   eArgTypeFilename,
   eArgTypeFormat,
   eArgTypeFrameIndex,
   eArgTypeFullName,
   eArgTypeFunctionName,
   eArgTypeFunctionOrSymbol,
   eArgTypeGDBFormat,
   eArgTypeHelpText,
   eArgTypeIndex,
   eArgTypeLanguage,
   eArgTypeLineNum,
   eArgTypeLogCategory,
   eArgTypeLogChannel,
   eArgTypeMethod,
   eArgTypeName,
   eArgTypeNewPathPrefix,
   eArgTypeNumLines,
   eArgTypeNumberPerLine,
   eArgTypeOffset,
   eArgTypeOldPathPrefix,

lldb/packages/Python/lldbsuite/test/lldbutil.py

def run_break_set_by_source_regexp(
 
     check_breakpoint_result(
         test,
         break_results,
         num_locations=num_expected_locations)
 
     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.
     Returns a dictionary of information gleaned from the command-line results.
     Will assert if the breakpoint setting fails altogether.
 
     Dictionary will contain:
         bpno          - breakpoint of the newly created breakpoint, -1 on error.
         num_locations - number of locations set for the breakpoint.
 
     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
         module        - module
         address       - address at which the breakpoint was set."""
 
     patterns = [
         r"^Breakpoint (?P<bpno>[0-9]+): (?P<num_locations>[0-9]+) locations\.$",
def get_bpno_from_match(break_results):
     return int(break_results['bpno'])
 
 
 def check_breakpoint_result(
         test,
         break_results,
         file_name=None,
         line_number=-1,
+        column_number=0,
         symbol_name=None,
         symbol_match_exact=True,
         module_name=None,
         offset=-1,
         num_locations=-1):
 
     out_num_locations = break_results['num_locations']
 
if line_number != -1:
             out_line_number = break_results['line_no']
 
         test.assertTrue(
             line_number == out_line_number,
             "Breakpoint line number %s doesn't match resultant line %s." %
             (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
         # name:
         if 'inline_symbol' in break_results and break_results['inline_symbol']:
             out_symbol_name = break_results['inline_symbol']
         elif 'symbol' in break_results:
             out_symbol_name = break_results['symbol']

lldb/source/Commands/CommandObjectBreakpoint.cpp

 #include "lldb/Breakpoint/BreakpointIDList.h"
 #include "lldb/Breakpoint/BreakpointLocation.h"
 #include "lldb/Host/OptionParser.h"
 #include "lldb/Interpreter/CommandInterpreter.h"
 #include "lldb/Interpreter/CommandReturnObject.h"
 #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"
 #include "lldb/Target/Language.h"
 #include "lldb/Target/StackFrame.h"
 #include "lldb/Target/Target.h"
 #include "lldb/Target/ThreadSpec.h"
 #include "lldb/Utility/RegularExpression.h"
Status SetOptionValue(uint32_t option_idx, llvm::StringRef option_arg,
           error.SetErrorStringWithFormat(
               "Invalid boolean value for on-throw option: '%s'",
               option_arg.str().c_str());
       } break;
 
       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");
       }
 
       return error;
     }
 
     void OptionParsingStarting(ExecutionContext *execution_context) override {

lldb/source/Commands/CommandObjectSource.cpp

 #include "lldb/Core/Debugger.h"
 #include "lldb/Core/FileLineResolver.h"
 #include "lldb/Core/Module.h"
 #include "lldb/Core/ModuleSpec.h"
 #include "lldb/Core/SourceManager.h"
 #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"
 #include "lldb/Symbol/Symbol.h"
 #include "lldb/Target/SectionLoadList.h"
 #include "lldb/Target/StackFrame.h"
 #include "lldb/Utility/FileSpec.h"
 
Status SetOptionValue(uint32_t option_idx, llvm::StringRef option_arg,
         break;
 
       case 'b':
         show_bp_locs = true;
         break;
       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");
       }
 
       return error;
     }
 
     void OptionParsingStarting(ExecutionContext *execution_context) override {

lldb/source/Commands/Options.td

let Command = "breakpoint dummy" in {
   def breakpoint_dummy_options_dummy_breakpoints :
     Option<"dummy-breakpoints", "D">, Group<1>,
     Desc<"Act on Dummy breakpoints - i.e. breakpoints set before a file is "
     "provided, which prime new targets.">;
 }
 
 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">,
     Desc<"Require the breakpoint to use hardware breakpoints.">;
   def breakpoint_set_file : Option<"file", "f">, Arg<"Filename">,
     Completion<"SourceFile">, Groups<[1,3,4,5,6,7,8,9,11]>,
     Desc<"Specifies the source file in which to set this breakpoint.  Note, by "
     "default lldb only looks for files that are #included if they use the "
let Command = "breakpoint set" in {
   def breakpoint_set_on_catch : Option<"on-catch", "h">, Group<10>,
     Arg<"Boolean">, Desc<"Set the breakpoint on exception catcH.">;
   def breakpoint_set_language : Option<"language", "L">, GroupRange<3, 8>,
     Arg<"Language">,
     Desc<"Specifies the Language to use when interpreting the breakpoint's "
     "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 "
     "exception Object of this type is thrown.  Can be repeated multiple times "
     "to stop for multiple object types">;
    */
 }
 
let Command = "source info" in {
   def source_info_address : Option<"address", "a">, Group<3>,
     Arg<"AddressOrExpression">, Desc<"Lookup the address and display the source"
     " information for the corresponding file and line.">;
 }
 
 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">,
     Desc<"Show the line table locations from the debug information that "
     "indicate valid places to set source level breakpoints.">;
   def source_list_file : Option<"file", "f">, Group<1>, Arg<"Filename">,
     Completion<"SourceFile">, Desc<"The file from which to display source.">;
   def source_list_line : Option<"line", "l">, Group<1>, Arg<"LineNum">,
     Desc<"The line number at which to start the display source.">;
   def source_list_name : Option<"name", "n">, Group<2>, Arg<"Symbol">,
     Completion<"Symbol">,
     Desc<"The name of a function whose source to display.">;
   def source_list_address : Option<"address", "a">, Group<3>,
     Arg<"AddressOrExpression">, Desc<"Lookup the address and display the source"
     " 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 {
   def dependents_no_dependents : Option<"no-dependents", "d">, Group<1>,
     OptionalEnumArg<"Value", "OptionEnumValues(g_dependents_enumaration)">,
     Desc<"Whether or not to load dependents when creating a target. If the "
          "option is not specified, the value is implicitly 'default'. If the "
          "option is specified but without a value, the value is implicitly "

lldb/source/Interpreter/CMakeLists.txt

add_lldb_library(lldbInterpreter
   OptionValue.cpp
   OptionValueArch.cpp
   OptionValueArgs.cpp
   OptionValueArray.cpp
   OptionValueBoolean.cpp
   OptionValueChar.cpp
   OptionValueDictionary.cpp
   OptionValueEnumeration.cpp
+  OptionValueFileColonLine.cpp
   OptionValueFileSpec.cpp
   OptionValueFileSpecList.cpp
   OptionValueFormat.cpp
   OptionValueFormatEntity.cpp
   OptionValueLanguage.cpp
   OptionValuePathMappings.cpp
   OptionValueProperties.cpp
   OptionValueRegex.cpp

lldb/source/Interpreter/CommandObject.cpp

CommandObject::ArgumentTableEntry CommandObject::g_arguments_data[] = {
     { eArgTypeFullName, "fullname", CommandCompletions::eNoCompletion, { nullptr, false }, "Help text goes here." },
     { eArgTypeFunctionName, "function-name", CommandCompletions::eNoCompletion, { nullptr, false }, "The name of a function." },
     { eArgTypeFunctionOrSymbol, "function-or-symbol", CommandCompletions::eNoCompletion, { nullptr, false }, "The name of a function or symbol." },
     { eArgTypeGDBFormat, "gdb-format", CommandCompletions::eNoCompletion, { GDBFormatHelpTextCallback, true }, nullptr },
     { eArgTypeHelpText, "help-text", CommandCompletions::eNoCompletion, { nullptr, false }, "Text to be used as help for some other entity in LLDB" },
     { 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." },
     { eArgTypeName, "name", CommandCompletions::eNoCompletion, { nullptr, false }, "Help text goes here." },
     { eArgTypeNewPathPrefix, "new-path-prefix", CommandCompletions::eNoCompletion, { nullptr, false }, "Help text goes here." },
     { eArgTypeNumLines, "num-lines", CommandCompletions::eNoCompletion, { nullptr, false }, "The number of lines to use." },
     { eArgTypeNumberPerLine, "number-per-line", CommandCompletions::eNoCompletion, { nullptr, false }, "The number of items per line to display." },
     { eArgTypeOffset, "offset", CommandCompletions::eNoCompletion, { nullptr, false }, "Help text goes here." },

lldb/source/Interpreter/OptionValue.cpp

const char *OptionValue::GetBuiltinTypeAsCString(Type t) {
   case eTypeBoolean:
     return "boolean";
   case eTypeChar:
     return "char";
   case eTypeDictionary:
     return "dictionary";
   case eTypeEnum:
     return "enum";
+  case eTypeFileLineColumn:
+    return "file:line:column specifier";
   case eTypeFileSpec:
     return "file";
   case eTypeFileSpecList:
     return "file-list";
   case eTypeFormat:
     return "format";
   case eTypeFormatEntity:
     return "format-string";

lldb/source/Interpreter/OptionValueArray.cpp

for (uint32_t i = 0; i < size; ++i) {
       case eTypePathMap:
         m_values[i]->DumpValue(exe_ctx, strm, dump_mask | extra_dump_options);
         break;
 
       case eTypeBoolean:
       case eTypeChar:
       case eTypeEnum:
       case eTypeFileSpec:
+      case eTypeFileLineColumn:
       case eTypeFormat:
       case eTypeSInt64:
       case eTypeString:
       case eTypeUInt64:
       case eTypeUUID:
         // No need to show the type for dictionaries of simple items
         m_values[i]->DumpValue(exe_ctx, strm, (dump_mask & (~eDumpOptionType)) |
                                                   extra_dump_options);

lldb/source/Interpreter/OptionValueDictionary.cpp

for (pos = m_values.begin(); pos != end; ++pos) {
       case eTypePathMap:
         strm.PutChar(' ');
         option_value->DumpValue(exe_ctx, strm, dump_mask | extra_dump_options);
         break;
 
       case eTypeBoolean:
       case eTypeChar:
       case eTypeEnum:
+      case eTypeFileLineColumn:
       case eTypeFileSpec:
       case eTypeFormat:
       case eTypeSInt64:
       case eTypeString:
       case eTypeUInt64:
       case eTypeUUID:
         // No need to show the type for dictionaries of simple items
         strm.PutCString("=");

lldb/source/Interpreter/OptionValueFileColonLine.cpp

+//===-- 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);
+}

lldb/source/Interpreter/OptionValueFileSpec.cpp

Status OptionValueFileSpec::SetValueFromString(llvm::StringRef value,
   case eVarSetOperationClear:
     Clear();
     NotifyValueChanged();
     break;
 
   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);
       if (m_resolve)
         FileSystem::Instance().Resolve(m_current_value);
       m_data_sp.reset();
       m_data_mod_time = llvm::sys::TimePoint<>();
       NotifyValueChanged();

lldb/source/Interpreter/Property.cpp

// default value.
           // set the current value to the default and clear the boolean that
           // says that the value has been set.
           enum_value->Clear();
         }
       }
     }
     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
     const bool resolve = definition.default_uint_value != 0;
     FileSpec file_spec = FileSpec(definition.default_cstr_value);
     if (resolve)
       FileSystem::Instance().Resolve(file_spec);
     m_value_sp = std::make_shared<OptionValueFileSpec>(file_spec, resolve);

lldb/test/API/functionalities/breakpoint/breakpoint_by_file_colon_line/Makefile

+C_SOURCES := main.c
+CFLAGS_EXTRAS := -std=c99 -gcolumn-info
+
+include Makefile.rules

lldb/test/API/functionalities/breakpoint/breakpoint_by_file_colon_line/TestBreakpointByFileColonLine.py

+"""
+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)
+        

lldb/test/API/functionalities/breakpoint/breakpoint_by_file_colon_line/main.c

+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);
+}

lldb/test/API/source-manager/TestSourceManager.py

def test_modify_source_file_while_debugging(self):
 
         # Display some source code.
         self.expect(
             "source list -f main-copy.c -l %d" %
             self.line,
             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.
 
         # The file to display is implicit in this case.
         self.runCmd("source list -l %d -c 3 -b" % self.line)
         output = self.res.GetOutput().splitlines()[0]
 
         # If the breakpoint set command succeeded, we should expect a positive number

lldb/unittests/Interpreter/CMakeLists.txt

 add_lldb_unittest(InterpreterTests
   TestCompletion.cpp
   TestOptionArgParser.cpp
+  TestOptionValueFileColonLine.cpp
 
   LINK_LIBS
     lldbInterpreter
     lldbUtilityHelpers
   )

lldb/unittests/Interpreter/TestOptionValueFileColonLine.cpp

+//===-- 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);
+}