[PATCH] D102355: Add support for DWARF embedded source to llvm-symbolizer

[Alex Orlov via Phabricator via llvm-commits]

aorlov marked an inline comment as done.
aorlov added inline comments.


================
Comment at: llvm/lib/DebugInfo/Symbolize/DIPrinter.cpp:305
+      std::string SourceCopy;
+      if (*Source.end() != '\0') {
+        SourceCopy = Source.str();
----------------
dblaikie wrote:
> It's not valid to dereference an end iterator like this.
> 
> What is this code intended to address?
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.


================
Comment at: llvm/lib/DebugInfo/Symbolize/DIPrinter.cpp:312-321
+      for (line_iterator I = line_iterator(MemoryBufferRef(Source, ""), false);
+           !I.is_at_eof(); ++I) {
+        int64_t L = SourceCode.FirstLine + I.line_number() - 1;
+        Stream << format_decimal(L, MaxLineNumberWidth);
+        if (L == SourceCode.Line)
+          Stream << " >: ";
+        else
----------------
dblaikie wrote:
> This looks rather similar to PlainPrinterBase::printContext - perhaps it could be refactored out into a common function?
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().


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102355/new/

https://reviews.llvm.org/D102355