[NativePDB] Fix setting breakpoint by file and line

[zturner]

Summary

There were several problems preventing this from working.  The
first is that when the PDB had an absolute path to the main
source file, we would construct an invalid path by prepending the
compilation directory to it anyway.  So we needed to check if the
path is already absolute first.

Second, LLDB assumes that the zero'th item in the support file list
is the main compilation unit.  We were respecting this requirement,
but LLDB *also* requires that file to appear somewhere in the list
starting from index 1 as well.  So the main compilation file should
appear in the support file list twice.  And when parsing a line
table, it expects the LineEntry records to be constructed using
the 1-based index.  With these two fixes we can now set breakpoints
by file and line using the native PDB reader.

[clayborg]

LGTM

[aleksandr.urakov]

LGTM too!

[labath]

lldb/lit/SymbolFile/NativePDB/break-by-line.cpp

9–10

You could consider using break-by-source-regex (-p) to avoid this. The regex should be transformed into a line number long before it reaches the pdb plugin.

[amccarth]

It looks like the code changes landed (probably as part of another patch) but not the tests. I'll look and see if the tests should to be revived.

[jingham]

In D56010#1623762, @amccarth wrote:

It looks like the code changes landed (probably as part of another patch) but not the tests. I'll look and see if the tests should to be revived.

I can't see that any of these tests test breakpoint setting features that aren't tested in a bunch of places in the Python testsuite, and except for the building part, none of them look PDB specific. Do we not know how to build & run PDB executables in the dotest testsuite?

[amccarth]

It turns out these tests all seem to exist, so I guess Zachary submitted them as part of other CLs.

lldb/lit/SymbolFile/NativePDB/Inputs/break-by-line.lldbinit

+break set -f break-by-line.cpp -l 14
+break list
+quit

lldb/lit/SymbolFile/NativePDB/break-by-function.cpp

 // clang-format off
 // REQUIRES: lld
 
 // Test that we can set simple breakpoints using PDB on any platform.
 // RUN: %build --compiler=clang-cl --nodefaultlib -o %t.exe -- %s 
 // RUN: env LLDB_USE_NATIVE_PDB_READER=1 %lldb -f %t.exe -s \
-// RUN:     %p/Inputs/breakpoints.lldbinit | FileCheck %s
+// RUN:     %p/Inputs/break-by-function.lldbinit | FileCheck %s
 
 // Use different indentation style for each overload so that the starting
 // line is in a different place.
+
 int OvlGlobalFn(int X) {
   return X + 42;
 }
 
 int OvlGlobalFn(int X, int Y) { return X + Y + 42; }
 
 int OvlGlobalFn(int X, int Y, int Z)
 {
int main(int argc, char **argv) {
   // Note the comments here, we want to make sure the line number reported
   // for the breakpoint is the first actual line of code.
   int Result = OvlGlobalFn(argc) + OvlGlobalFn(argc, argc)
     + OvlGlobalFn(argc, argc, argc) + StaticFn(argc);
   return Result;
 }
 
 
-// CHECK:      (lldb) target create "{{.*}}simple-breakpoints.cpp.tmp.exe"
-// CHECK:      Current executable set to '{{.*}}simple-breakpoints.cpp.tmp.exe'
+// CHECK:      (lldb) target create "{{.*}}break-by-function.cpp.tmp.exe"
+// CHECK:      Current executable set to '{{.*}}break-by-function.cpp.tmp.exe'
 // CHECK:      (lldb) break set -n main
-// CHECK:      Breakpoint 1: where = simple-breakpoints.cpp.tmp.exe`main + {{[0-9]+}}
-// CHECK-SAME:               at simple-breakpoints.cpp:30
+// CHECK:      Breakpoint 1: where = break-by-function.cpp.tmp.exe`main + {{[0-9]+}}
 // CHECK:      (lldb) break set -n OvlGlobalFn
 // CHECK:      Breakpoint 2: 3 locations.
 // CHECK:      (lldb) break set -n StaticFn
-// CHECK:      Breakpoint 3: where = simple-breakpoints.cpp.tmp.exe`StaticFn + {{[0-9]+}}
-// CHECK-SAME:               at simple-breakpoints.cpp:23
+// CHECK:      Breakpoint 3: where = break-by-function.cpp.tmp.exe`StaticFn + {{[0-9]+}}
 // CHECK:      (lldb) break set -n DoesntExist
 // CHECK:      Breakpoint 4: no locations (pending).
 // CHECK:      (lldb) break list
 // CHECK:      Current breakpoints:
 // CHECK:      1: name = 'main', locations = 1
-// CHECK:        1.1: where = simple-breakpoints.cpp.tmp.exe`main + {{[0-9]+}}
-// CHECK-SAME:                at simple-breakpoints.cpp:30
+// CHECK:        1.1: where = break-by-function.cpp.tmp.exe`main + {{[0-9]+}}
 // CHECK:      2: name = 'OvlGlobalFn', locations = 3
-// CHECK:        2.1: where = simple-breakpoints.cpp.tmp.exe`OvlGlobalFn + {{[0-9]+}}
-// CHECK-SAME:                at simple-breakpoints.cpp:12
-// CHECK:        2.2: where = simple-breakpoints.cpp.tmp.exe`OvlGlobalFn
-// CHECK-SAME:                at simple-breakpoints.cpp:15
-// CHECK:        2.3: where = simple-breakpoints.cpp.tmp.exe`OvlGlobalFn + {{[0-9]+}}
-// CHECK-SAME:                at simple-breakpoints.cpp:19
+// CHECK:        2.1: where = break-by-function.cpp.tmp.exe`OvlGlobalFn + {{[0-9]+}}
+// CHECK:        2.2: where = break-by-function.cpp.tmp.exe`OvlGlobalFn
+// CHECK:        2.3: where = break-by-function.cpp.tmp.exe`OvlGlobalFn + {{[0-9]+}}
 // CHECK:      3: name = 'StaticFn', locations = 1
-// CHECK:        3.1: where = simple-breakpoints.cpp.tmp.exe`StaticFn + {{[0-9]+}}
-// CHECK-SAME:                at simple-breakpoints.cpp:23
+// CHECK:        3.1: where = break-by-function.cpp.tmp.exe`StaticFn + {{[0-9]+}}
 // CHECK:      4: name = 'DoesntExist', locations = 0 (pending)

lldb/lit/SymbolFile/NativePDB/break-by-line.cpp

+// clang-format off
+// REQUIRES: lld
+
+// Test that we can set simple breakpoints using PDB on any platform.
+// RUN: %build --compiler=clang-cl --nodefaultlib -o %t.exe -- %s
+// RUN: env LLDB_USE_NATIVE_PDB_READER=1 %lldb -f %t.exe -s \
+// RUN:     %p/Inputs/break-by-line.lldbinit | FileCheck %s
+
+// This is a separate test from break-by-function.cpp because this test is
+// sensitive to edits in the source file.
+
+namespace NS {
+  int NamespaceFn(int X) {
+    return X + 42;
+  }
+}
+
+int main(int argc, char **argv) {
+  return NS::NamespaceFn(argc);
+}
+
+
+// CHECK:      (lldb) target create "{{.*}}break-by-line.cpp.tmp.exe"
+// CHECK:      Current executable set to '{{.*}}break-by-line.cpp.tmp.exe'
+// CHECK:      (lldb) break set -f break-by-line.cpp -l 14
+// CHECK:      Breakpoint 1: where = break-by-line.cpp.tmp.exe`NS::NamespaceFn + {{[0-9]+}} at break-by-line.cpp:14

lldb/source/Plugins/SymbolFile/NativePDB/CompileUnitIndex.cpp

CompileUnitIndex::GetMainSourceFile(const CompilandIndexItem &item) const {
   StringIdRecord file_name;
   CVType dir_cvt = types.getType(item.m_build_info[0]);
   CVType file_cvt = types.getType(item.m_build_info[2]);
   llvm::cantFail(
       TypeDeserializer::deserializeAs<StringIdRecord>(dir_cvt, working_dir));
   llvm::cantFail(
       TypeDeserializer::deserializeAs<StringIdRecord>(file_cvt, file_name));
 
+  llvm::sys::path::Style style = working_dir.String.startswith("/")
+                                     ? llvm::sys::path::Style::posix
+                                     : llvm::sys::path::Style::windows;
+  if (llvm::sys::path::is_absolute(file_name.String, style))
+    return file_name.String;
+
   llvm::SmallString<64> absolute_path = working_dir.String;
   llvm::sys::path::append(absolute_path, file_name.String);
   return absolute_path;
 }

lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h

public:
   uint32_t FindGlobalVariables(const ConstString &name,
                                const CompilerDeclContext *parent_decl_ctx,
                                uint32_t max_matches,
                                VariableList &variables) override;
 
   size_t ParseTypes(const SymbolContext &sc) override;
   size_t ParseVariablesForContext(const SymbolContext &sc) override;
 
+  void AddSymbols(Symtab &symtab) override;
+
   CompilerDecl GetDeclForUID(lldb::user_id_t uid) override;
   CompilerDeclContext GetDeclContextForUID(lldb::user_id_t uid) override;
   CompilerDeclContext GetDeclContextContainingUID(lldb::user_id_t uid) override;
   Type *ResolveTypeUID(lldb::user_id_t type_uid) override;
   llvm::Optional<ArrayInfo> GetDynamicArrayInfoForUID(
       lldb::user_id_t type_uid,
       const lldb_private::ExecutionContext *exe_ctx) override;
 
   bool CompleteType(CompilerType &compiler_type) override;
   uint32_t ResolveSymbolContext(const Address &so_addr,
                                 lldb::SymbolContextItem resolve_scope,
                                 SymbolContext &sc) override;
+  uint32_t ResolveSymbolContext(const FileSpec &file_spec, uint32_t line,
+                                bool check_inlines,
+                                lldb::SymbolContextItem resolve_scope,
+                                SymbolContextList &sc_list) override;
 
   size_t GetTypes(SymbolContextScope *sc_scope, lldb::TypeClass type_mask,
                   TypeList &type_list) override;
 
   uint32_t FindFunctions(const ConstString &name,
                          const CompilerDeclContext *parent_decl_ctx,
                          lldb::FunctionNameType name_type_mask,
                          bool include_inlines, bool append,

lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp

CompilandIndexItem *item =
       m_index->compilands().GetCompiland(uid.asCompiland().modi);
   lldbassert(item);
   if (!item->m_compile_opts)
     return lldb::eLanguageTypeUnknown;
 
   return TranslateLanguage(item->m_compile_opts->getLanguage());
 }
 
+void SymbolFileNativePDB::AddSymbols(Symtab &symtab) { return; }
+
 size_t SymbolFileNativePDB::ParseCompileUnitFunctions(const SymbolContext &sc) {
   lldbassert(sc.comp_unit);
   return false;
 }
 
 static bool NeedsResolvedCompileUnit(uint32_t resolve_scope) {
   // If any of these flags are set, we need to resolve the compile unit.
   uint32_t flags = eSymbolContextCompUnit;
if (auto *line_table = sc.comp_unit->GetLineTable()) {
       if (line_table->FindLineEntryByAddress(addr, sc.line_entry))
         resolved_flags |= eSymbolContextLineEntry;
     }
   }
 
   return resolved_flags;
 }
 
+uint32_t SymbolFileNativePDB::ResolveSymbolContext(
+    const FileSpec &file_spec, uint32_t line, bool check_inlines,
+    lldb::SymbolContextItem resolve_scope, SymbolContextList &sc_list) {
+  return 0;
+}
+
 static void AppendLineEntryToSequence(LineTable &table, LineSequence &sequence,
                                       const CompilandIndexItem &cci,
                                       lldb::addr_t base_addr,
                                       uint32_t file_number,
                                       const LineFragmentHeader &block,
                                       const LineNumberEntry &cur) {
   LineInfo cur_info(cur.Flags);
 
for (const LineColumnEntry &group : lines) {
       if (!efn) {
         llvm::consumeError(efn.takeError());
         continue;
       }
 
       // LLDB wants the index of the file in the list of support files.
       auto fn_iter = llvm::find(cci->m_file_list, *efn);
       lldbassert(fn_iter != cci->m_file_list.end());
-      uint32_t file_index = std::distance(cci->m_file_list.begin(), fn_iter);
+      // LLDB support file indices are 1-based.
+      uint32_t file_index =
+          1 + std::distance(cci->m_file_list.begin(), fn_iter);
 
       std::unique_ptr<LineSequence> sequence(
           line_table->CreateLineSequenceContainer());
       lldbassert(!group.LineNumbers.empty());
 
       for (const LineNumberEntry &entry : group.LineNumbers) {
         AppendLineEntryToSequence(*line_table, *sequence, *cci, virtual_addr,
                                   file_index, *lfh, entry);
bool SymbolFileNativePDB::ParseCompileUnitSupportFiles(
 
   for (llvm::StringRef f : cci->m_file_list) {
     FileSpec::Style style =
         f.startswith("/") ? FileSpec::Style::posix : FileSpec::Style::windows;
     FileSpec spec(f, style);
     support_files.Append(spec);
   }
 
+  llvm::SmallString<64> main_source_file =
+      m_index->compilands().GetMainSourceFile(*cci);
+  FileSpec::Style style = main_source_file.startswith("/")
+                              ? FileSpec::Style::posix
+                              : FileSpec::Style::windows;
+  FileSpec spec(main_source_file, style);
+  support_files.Insert(0, spec);
   return true;
 }
 
 bool SymbolFileNativePDB::ParseImportedModules(
     const SymbolContext &sc, std::vector<ConstString> &imported_modules) {
   // PDB does not yet support module debug info
   return false;
 }