Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[DebugInfo] Pass string ownership to MarkupFilter #75403

Conversation

vitalybuka
Copy link
Collaborator

@vitalybuka vitalybuka commented Dec 13, 2023

Last getline call destroys InputString, and finish accesses dead StringRef.

Detected with #72677.

Fixes https://lab.llvm.org/buildbot/#/builders/sanitizer-x86_64-linux-fast

Created using spr 1.3.4
@llvmbot
Copy link
Collaborator

llvmbot commented Dec 13, 2023

@llvm/pr-subscribers-debuginfo

@llvm/pr-subscribers-llvm-binary-utilities

Author: Vitaly Buka (vitalybuka)

Changes

Last getline call destroys InputString, and finish fails
accesses dead StringRef.

Detected with #72677.


Full diff: https://github.com/llvm/llvm-project/pull/75403.diff

3 Files Affected:

  • (modified) llvm/include/llvm/DebugInfo/Symbolize/MarkupFilter.h (+2-2)
  • (modified) llvm/lib/DebugInfo/Symbolize/MarkupFilter.cpp (+6-4)
  • (modified) llvm/tools/llvm-symbolizer/llvm-symbolizer.cpp (+1-1)
diff --git a/llvm/include/llvm/DebugInfo/Symbolize/MarkupFilter.h b/llvm/include/llvm/DebugInfo/Symbolize/MarkupFilter.h
index a1514d91702b9f..10613ec57586df 100644
--- a/llvm/include/llvm/DebugInfo/Symbolize/MarkupFilter.h
+++ b/llvm/include/llvm/DebugInfo/Symbolize/MarkupFilter.h
@@ -39,7 +39,7 @@ class MarkupFilter {
   ///
   /// Invalid or unimplemented markup elements are removed. Some output may be
   /// deferred until future filter() or finish() call.
-  void filter(StringRef Line);
+  void filter(std::string &&InputLine);
 
   /// Records that the input stream has ended and writes any deferred output.
   void finish();
@@ -142,7 +142,7 @@ class MarkupFilter {
   MarkupParser Parser;
 
   // Current line being filtered.
-  StringRef Line;
+  std::string Line;
 
   // A module info line currently being built. This incorporates as much mmap
   // information as possible before being emitted.
diff --git a/llvm/lib/DebugInfo/Symbolize/MarkupFilter.cpp b/llvm/lib/DebugInfo/Symbolize/MarkupFilter.cpp
index f7503ef49693d8..01d49709f9b29c 100644
--- a/llvm/lib/DebugInfo/Symbolize/MarkupFilter.cpp
+++ b/llvm/lib/DebugInfo/Symbolize/MarkupFilter.cpp
@@ -41,8 +41,8 @@ MarkupFilter::MarkupFilter(raw_ostream &OS, LLVMSymbolizer &Symbolizer,
       ColorsEnabled(
           ColorsEnabled.value_or(WithColor::defaultAutoDetectFunction()(OS))) {}
 
-void MarkupFilter::filter(StringRef Line) {
-  this->Line = Line;
+void MarkupFilter::filter(std::string &&InputLine) {
+  Line = std::move(InputLine);
   resetColor();
 
   Parser.parseLine(Line);
@@ -695,7 +695,9 @@ void MarkupFilter::reportTypeError(StringRef Str, StringRef TypeName) const {
 // passed to beginLine().
 void MarkupFilter::reportLocation(StringRef::iterator Loc) const {
   errs() << Line;
-  WithColor(errs().indent(Loc - Line.begin()), HighlightColor::String) << '^';
+  WithColor(errs().indent(Loc - StringRef(Line).begin()),
+            HighlightColor::String)
+      << '^';
   errs() << '\n';
 }
 
@@ -741,7 +743,7 @@ uint64_t MarkupFilter::adjustAddr(uint64_t Addr, PCType Type) const {
 }
 
 StringRef MarkupFilter::lineEnding() const {
-  return Line.ends_with("\r\n") ? "\r\n" : "\n";
+  return StringRef(Line).ends_with("\r\n") ? "\r\n" : "\n";
 }
 
 bool MarkupFilter::MMap::contains(uint64_t Addr) const {
diff --git a/llvm/tools/llvm-symbolizer/llvm-symbolizer.cpp b/llvm/tools/llvm-symbolizer/llvm-symbolizer.cpp
index 447c18abadc174..3d33534d95b7e0 100644
--- a/llvm/tools/llvm-symbolizer/llvm-symbolizer.cpp
+++ b/llvm/tools/llvm-symbolizer/llvm-symbolizer.cpp
@@ -430,7 +430,7 @@ static void filterMarkup(const opt::InputArgList &Args, LLVMSymbolizer &Symboliz
   std::string InputString;
   while (std::getline(std::cin, InputString)) {
     InputString += '\n';
-    Filter.filter(InputString);
+    Filter.filter(std::move(InputString));
   }
   Filter.finish();
 }

Copy link
Collaborator

@jh7370 jh7370 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@vitalybuka vitalybuka merged commit b3e1114 into main Dec 14, 2023
6 checks passed
@vitalybuka vitalybuka deleted the users/vitalybuka/spr/debuginfo-pass-string-ownership-to-markupfilter branch December 14, 2023 08:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants