Add support for DWARF embedded source to llvm-symbolizer

[aorlov]

Summary

This patch adds DWARF embedded source printout to llvm-symbolizer.

[dblaikie]

llvm/lib/DebugInfo/Symbolize/DIPrinter.cpp

121–122

Could you pull this (& any other refactoring you think makes the semantic-changing patch more isolated/focussed) into a preliminary commit?

305–322

I take it this is trimming the trailing lines that won't be needed, while leaving the leading lines so the algorithm in printContext can still retrieve the lines in a source-agnostic way (so it can retrieve them in the same way from embedded source as non-embedded source)

It seems unfortunate to have split the pruning logic in two places like this - perhaps a generic function that takes the line iterator & just the desired context (pruning both leading and trailing lines) - then use that function here, and have the "Source" attribute contain the fully pruned text.

Then in printContext if the Source value is provided, use that literally - otherwise open the file and prune its leading/trailing using the same function?

(also that refactoring - pulling out such a reusable pruning function - should be done as a separate commit)

[aorlov]

https://reviews.llvm.org/D102539

[dblaikie]

llvm/lib/DebugInfo/Symbolize/DIPrinter.cpp

309

It's not valid to dereference an end iterator like this.

What is this code intended to address?

316–325

This looks rather similar to PlainPrinterBase::printContext - perhaps it could be refactored out into a common function?

[aorlov]

llvm/lib/DebugInfo/Symbolize/DIPrinter.cpp

309

Note llvm::line_iterator is badly designed.
It is checks *Pos == '\0' in many places instead of comparing Pos with Buffer.end().
It requires '\0' at the end of buffer which is not necessary actually.
Look at an assert on llvm/lib/Support/LineIterator.cpp, line 47.

316–325

Correct. I copied the Source context to JSON as is in the first implementation. Then I decided to format it too. I have moved the code to the common function format().

[aorlov]

llvm/lib/DebugInfo/Symbolize/DIPrinter.cpp

309

Note the current implementation of line_iterator can be used with the full source, but not with the pruned source if it didn't coincide with the end of the source file.

[dblaikie]

llvm/lib/DebugInfo/Symbolize/DIPrinter.cpp

92

This still isn't valid - there's no guarantee that end is dereferenceable. (using [] doesn't make the code any safer/different - it's still a dereference)

309

Either the code will need to unconditionally copy the StringRef into a std::string (in which case, maybe it'd be simpler if the SourceCode structure stored the data in a std::string in the first place? then it wouldn't need the MemoryBuffer member either, since it wouldn't be holding live pointers to the memory mapped file) - or the line_iterator would need to be fixed or a different tool used - which, maybe that's the thing to do? If the original code that extracted the relevant lines already used the line iterator (so features like blank line skipping, etc, have already been handled) - maybe this code could do something simpler, like splitting on newlines, rather than using the full line iterator?

[aorlov]

llvm/lib/DebugInfo/Symbolize/DIPrinter.cpp

92

I just used the same checking as llvm/lib/Support/LineIterator.cpp, line 47:

assert(Buffer.getBufferEnd()[0] == '\0');

Any suggestions?

309

Note currently line_iterator does not skip blank lines.
I will prepare an own implementation without llvm::line_iterator.

[jhenderson]

I've got no specific comments on the source code, just one small nit in the test. I'll leave @dblaikie to continue the rest of the review.

llvm/test/tools/llvm-symbolizer/source.ll

9

It would probably be a good idea to run the exact same check for GNU output (or llvm-addr2line), just to cover our bases. That way, if the behaviour ever diverges in the future, there's a clear test to update.

[jhenderson]

llvm/test/tools/llvm-symbolizer/source.ll

2

I think "output" is a more common term to use and easier to understand.

8

No need to repeat "source context" in this text - it's implied by the top-level comment.

9

This is simpler and more traditional for the input arguments. Also CODE is the default and not needed.

17

18

20–24

There's only one small difference between this and the LLVM style, if I'm not mistaken. You could avoid duplciation by doing something like:

; RUN: ... | FileCheck ... --check-prefixes=COMMON,LLVM;
; RUN: ... | FileCheck ... --check-prefixes=COMMON,GNU

;      COMMON:foo
;   LLVM-NEXT:/source.c:3:13
;    GNU-NEXT:/source.c:3
; COMMON-NEXT:2  : // Line 2
; COMMON-NEXT:3 >: void foo() {}
; COMMON-NEXT:4  : // Line 4

26

27

[jhenderson]

Test looks good to me. I'm leaving the source for @dblaikie.

[dblaikie]

Sure, seems OK. Thanks!

[jhenderson]

Formally: LGTM!

llvm/lib/DebugInfo/Symbolize/DIPrinter.cpp

public:
 
   SourceCode(
       StringRef FileName, int64_t Line, int Lines,
       const Optional<StringRef> &EmbeddedSource = Optional<StringRef>(None))
       : Line(Line), Lines(Lines),
         FirstLine(std::max(static_cast<int64_t>(1), Line - Lines / 2)),
         LastLine(FirstLine + Lines - 1),
         PrunedSource(pruneSource(load(FileName, EmbeddedSource))) {}
-};
 
-void PlainPrinterBase::printHeader(uint64_t Address) {
-  if (Config.PrintAddress) {
-    OS << "0x";
-    OS.write_hex(Address);
-    StringRef Delimiter = Config.Pretty ? ": " : "\n";
-    OS << Delimiter;
-  }
-}
-
-// Prints source code around in the FileName the Line.
-void PlainPrinterBase::printContext(SourceCode SourceCode) {
-  if (!SourceCode.PrunedSource)
+  void format(raw_ostream &OS) {
+    if (!PrunedSource)
       return;
 
-  StringRef Source = *SourceCode.PrunedSource;
+    StringRef Source = *PrunedSource;
     std::string SourceCopy;
-  if (*Source.end() != '\0') {
+    if (Source.end()[0] != '\0') {
       SourceCopy = Source.str();
       Source = SourceCopy;
     }
 
-  size_t MaxLineNumberWidth = std::ceil(std::log10(SourceCode.LastLine));
+    size_t MaxLineNumberWidth = std::ceil(std::log10(LastLine));
     for (line_iterator I = line_iterator(MemoryBufferRef(Source, ""), false);
          !I.is_at_eof(); ++I) {
-    int64_t L = SourceCode.FirstLine + I.line_number() - 1;
+      int64_t L = FirstLine + I.line_number() - 1;
       OS << format_decimal(L, MaxLineNumberWidth);
-    if (L == SourceCode.Line)
+      if (L == Line)
         OS << " >: ";
       else
         OS << "  : ";
       OS << *I << '\n';
     }
   }
+};
+
+void PlainPrinterBase::printHeader(uint64_t Address) {
+  if (Config.PrintAddress) {
+    OS << "0x";
+    OS.write_hex(Address);
+    StringRef Delimiter = Config.Pretty ? ": " : "\n";
+    OS << Delimiter;
+  }
+}
+
+// Prints source code around in the FileName the Line.
+void PlainPrinterBase::printContext(SourceCode SourceCode) {
+  SourceCode.format(OS);
+}
 
 void PlainPrinterBase::printFunctionName(StringRef FunctionName, bool Inlined) {
   if (Config.PrintFunctions) {
     if (FunctionName == DILineInfo::BadString)
       FunctionName = DILineInfo::Addr2LineBadString;
     StringRef Delimiter = Config.Pretty ? " at " : "\n";
     StringRef Prefix = (Config.Pretty && Inlined) ? " (inlined by) " : "";
     OS << Prefix << FunctionName << Delimiter;
   }
 }
 
 void LLVMPrinter::printSimpleLocation(StringRef Filename,
                                       const DILineInfo &Info) {
   OS << Filename << ':' << Info.Line << ':' << Info.Column << '\n';
-  printContext(SourceCode(Filename, Info.Line, Config.SourceContextLines));
+  printContext(
+      SourceCode(Filename, Info.Line, Config.SourceContextLines, Info.Source));
 }
 
 void GNUPrinter::printSimpleLocation(StringRef Filename,
                                      const DILineInfo &Info) {
   OS << Filename << ':' << Info.Line;
   if (Info.Discriminator)
     OS << " (discriminator " << Info.Discriminator << ')';
   OS << '\n';
-  printContext(SourceCode(Filename, Info.Line, Config.SourceContextLines));
+  printContext(
+      SourceCode(Filename, Info.Line, Config.SourceContextLines, Info.Source));
 }
 
 void PlainPrinterBase::printVerbose(StringRef Filename,
                                     const DILineInfo &Info) {
   OS << "  Filename: " << Filename << '\n';
   if (Info.StartLine) {
     OS << "  Function start filename: " << Info.StartFileName << '\n';
     OS << "  Function start line: " << Info.StartLine << '\n';
void JSONPrinter::print(const Request &Request, const DILineInfo &Info) {
   InliningInfo.addFrame(Info);
   print(Request, InliningInfo);
 }
 
 void JSONPrinter::print(const Request &Request, const DIInliningInfo &Info) {
   json::Array Array;
   for (uint32_t I = 0, N = Info.getNumberOfFrames(); I < N; ++I) {
     const DILineInfo &LineInfo = Info.getFrame(I);
-    Array.push_back(json::Object(
+    json::Object Object(
         {{"FunctionName", LineInfo.FunctionName != DILineInfo::BadString
                               ? LineInfo.FunctionName
                               : ""},
          {"StartFileName", LineInfo.StartFileName != DILineInfo::BadString
                                ? LineInfo.StartFileName
                                : ""},
          {"StartLine", LineInfo.StartLine},
          {"FileName",
           LineInfo.FileName != DILineInfo::BadString ? LineInfo.FileName : ""},
          {"Line", LineInfo.Line},
          {"Column", LineInfo.Column},
-         {"Discriminator", LineInfo.Discriminator}}));
+         {"Discriminator", LineInfo.Discriminator}});
+    SourceCode SourceCode(LineInfo.FileName, LineInfo.Line,
+                          Config.SourceContextLines, LineInfo.Source);
+    std::string FormattedSource;
+    raw_string_ostream Stream(FormattedSource);
+    SourceCode.format(Stream);
+    if (!FormattedSource.empty())
+      Object["Source"] = std::move(FormattedSource);
+    Array.push_back(std::move(Object));
   }
   json::Object Json = toJSON(Request);
   Json["Symbol"] = std::move(Array);
   if (ObjectList)
     ObjectList->push_back(std::move(Json));
   else
     printJSON(std::move(Json));
 }
 
 void JSONPrinter::print(const Request &Request, const DIGlobal &Global) {
   json::Object Data(
       {{"Name", Global.Name != DILineInfo::BadString ? Global.Name : ""},
        {"Start", toHex(Global.Start)},
        {"Size", toHex(Global.Size)}});
   json::Object Json = toJSON(Request);
   Json["Data"] = std::move(Data);
   if (ObjectList)
     ObjectList->push_back(std::move(Json));
   else
     printJSON(std::move(Json));
 }
 

llvm/test/tools/llvm-symbolizer/source.ll

+;; This test checks printout of the DWARF embedded source.
+
+; REQUIRES: x86-registered-target
+
+; RUN: llc -filetype=obj -o %t.o %s 
+
+;; Check the source context plain printout.
+; RUN: llvm-symbolizer --print-source-context-lines=3 "CODE %t.o 0" | \
+; RUN:   FileCheck %s --strict-whitespace --match-full-lines --implicit-check-not={{.}}
+;      CHECK:foo
+; CHECK-NEXT:/source.c:3:13
+; CHECK-NEXT:2  : // Line 2
+; CHECK-NEXT:3 >: void foo() {}
+; CHECK-NEXT:4  : // Line 4
+
+;; Check the source context printout to JSON.
+; RUN: llvm-symbolizer --print-source-context-lines=3 "CODE %t.o 0" --output-style=JSON | \
+; RUN:   FileCheck %s --check-prefix=JSON --strict-whitespace --match-full-lines --implicit-check-not={{.}}
+; JSON:[{"Address":"0x0","ModuleName":"{{.*}}.o","Symbol":[{"Column":13,"Discriminator":0,"FileName":"/source.c","FunctionName":"foo","Line":3,"Source":"2  : // Line 2\n3 >: void foo() {}\n4  : // Line 4\n","StartFileName":"/source.c","StartLine":3}]}]
+
+;; Generated from the following source:
+;; // Line 1
+;; // Line 2
+;; void foo() {}
+;; // Line 4
+;; // Line 5
+;; clang --target=x86_64-pc-linux -gdwarf-5 -gembed-source -g -emit-llvm -S source.c -o source.ll
+
+source_filename = "source.c"
+target triple = "x86_64-pc-linux"
+
+define dso_local void @foo() #0 !dbg !7 {
+entry:
+  ret void, !dbg !10
+}
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!3, !4, !5}
+!llvm.ident = !{!6}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "clang version 12.0.0", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, splitDebugInlining: false, nameTableKind: None)
+!1 = !DIFile(filename: "source.c", directory: "/", source: "// Line 1\0A// Line 2\0Avoid foo() {}\0A// Line 4\0A// Line 5\0A")
+!2 = !{}
+!3 = !{i32 7, !"Dwarf Version", i32 5}
+!4 = !{i32 2, !"Debug Info Version", i32 3}
+!5 = !{i32 1, !"wchar_size", i32 4}
+!6 = !{!"clang version 12.0.0"}
+!7 = distinct !DISubprogram(name: "foo", scope: !1, file: !1, line: 3, type: !8, scopeLine: 3, spFlags: DISPFlagDefinition, unit: !0, retainedNodes: !2)
+!8 = !DISubroutineType(types: !9)
+!9 = !{null}
+!10 = !DILocation(line: 3, column: 13, scope: !7)