From 7b31a73ffe87e6302ab433da8128058fce8f54bd Mon Sep 17 00:00:00 2001 From: Daniel Thornburgh Date: Fri, 23 Jun 2023 15:24:48 -0700 Subject: [PATCH] [Symbolizer] Ignore unknown additional symbolizer markup fields The symbolizer markup syntax is structured such that fields require only previous fields for their interpretation; this was originally intended to make adding new fields a natural extension mechanism for existing elements. This codifies this into the spec and makes the behavior of the llvm-symbolizer match. Extra fields are now warned about, but ignored, rather than ignoring the whole element. Reviewed By: mcgrathr Differential Revision: https://reviews.llvm.org/D153821 --- llvm/docs/SymbolizerMarkupFormat.rst | 5 +++ .../llvm/DebugInfo/Symbolize/MarkupFilter.h | 2 +- llvm/lib/DebugInfo/Symbolize/MarkupFilter.cpp | 37 ++++++++++------------ .../test/DebugInfo/symbolize-filter-markup-bt.test | 11 ++++--- .../test/DebugInfo/symbolize-filter-markup-pc.test | 11 ++++--- .../DebugInfo/symbolize-filter-markup-reset.test | 9 ++++-- 6 files changed, 44 insertions(+), 31 deletions(-) diff --git a/llvm/docs/SymbolizerMarkupFormat.rst b/llvm/docs/SymbolizerMarkupFormat.rst index 1cf1d64..d6b22a4 100644 --- a/llvm/docs/SymbolizerMarkupFormat.rst +++ b/llvm/docs/SymbolizerMarkupFormat.rst @@ -143,6 +143,11 @@ what they contain is specified for each element type. No markup elements or ANSI SGR control sequences are interpreted inside the contents of a field. +Implementations must ignore markup fields after those expected; this allows +adding new fields to backwards-compatibly extend elements. Implementations need +not ignore them silently, but the element should behave otherwise as if the +fields were removed. + In the descriptions of each element type, ``printf``-style placeholders indicate field contents: diff --git a/llvm/include/llvm/DebugInfo/Symbolize/MarkupFilter.h b/llvm/include/llvm/DebugInfo/Symbolize/MarkupFilter.h index 429f9cb..a1514d9 100644 --- a/llvm/include/llvm/DebugInfo/Symbolize/MarkupFilter.h +++ b/llvm/include/llvm/DebugInfo/Symbolize/MarkupFilter.h @@ -123,7 +123,7 @@ private: bool checkTag(const MarkupNode &Node) const; bool checkNumFields(const MarkupNode &Element, size_t Size) const; bool checkNumFieldsAtLeast(const MarkupNode &Element, size_t Size) const; - bool checkNumFieldsAtMost(const MarkupNode &Element, size_t Size) const; + void warnNumFieldsAtMost(const MarkupNode &Element, size_t Size) const; void reportTypeError(StringRef Str, StringRef TypeName) const; void reportLocation(StringRef::iterator Loc) const; diff --git a/llvm/lib/DebugInfo/Symbolize/MarkupFilter.cpp b/llvm/lib/DebugInfo/Symbolize/MarkupFilter.cpp index 48833a7..a2bc257 100644 --- a/llvm/lib/DebugInfo/Symbolize/MarkupFilter.cpp +++ b/llvm/lib/DebugInfo/Symbolize/MarkupFilter.cpp @@ -133,9 +133,8 @@ bool MarkupFilter::tryReset(const MarkupNode &Node, endAnyModuleInfoLine(); for (const MarkupNode &Node : DeferredNodes) filterNode(Node); - highlight(); - OS << "[[[reset]]]" << lineEnding(); - restoreColor(); + printRawElement(Node); + OS << lineEnding(); Modules.clear(); MMaps.clear(); @@ -239,8 +238,7 @@ bool MarkupFilter::tryPC(const MarkupNode &Node) { return false; if (!checkNumFieldsAtLeast(Node, 1)) return true; - if (!checkNumFieldsAtMost(Node, 2)) - return true; + warnNumFieldsAtMost(Node, 2); std::optional Addr = parseAddr(Node.Fields[0]); if (!Addr) @@ -293,8 +291,7 @@ bool MarkupFilter::tryBackTrace(const MarkupNode &Node) { return false; if (!checkNumFieldsAtLeast(Node, 2)) return true; - if (!checkNumFieldsAtMost(Node, 3)) - return true; + warnNumFieldsAtMost(Node, 3); std::optional FrameNumber = parseFrameNumber(Node.Fields[0]); if (!FrameNumber) @@ -655,10 +652,12 @@ bool MarkupFilter::checkTag(const MarkupNode &Node) const { bool MarkupFilter::checkNumFields(const MarkupNode &Element, size_t Size) const { if (Element.Fields.size() != Size) { - WithColor::error(errs()) << "expected " << Size << " field(s); found " - << Element.Fields.size() << "\n"; + bool Warn = Element.Fields.size() > Size; + WithColor(errs(), Warn ? HighlightColor::Warning : HighlightColor::Error) + << (Warn ? "warning: " : "error: ") << "expected " << Size + << " field(s); found " << Element.Fields.size() << "\n"; reportLocation(Element.Tag.end()); - return false; + return Warn; } return true; } @@ -675,16 +674,14 @@ bool MarkupFilter::checkNumFieldsAtLeast(const MarkupNode &Element, return true; } -bool MarkupFilter::checkNumFieldsAtMost(const MarkupNode &Element, - size_t Size) const { - if (Element.Fields.size() > Size) { - WithColor::error(errs()) - << "expected at most " << Size << " field(s); found " - << Element.Fields.size() << "\n"; - reportLocation(Element.Tag.end()); - return false; - } - return true; +void MarkupFilter::warnNumFieldsAtMost(const MarkupNode &Element, + size_t Size) const { + if (Element.Fields.size() <= Size) + return; + WithColor::warning(errs()) + << "expected at most " << Size << " field(s); found " + << Element.Fields.size() << "\n"; + reportLocation(Element.Tag.end()); } void MarkupFilter::reportTypeError(StringRef Str, StringRef TypeName) const { diff --git a/llvm/test/DebugInfo/symbolize-filter-markup-bt.test b/llvm/test/DebugInfo/symbolize-filter-markup-bt.test index a170113..b8d8e18 100644 --- a/llvm/test/DebugInfo/symbolize-filter-markup-bt.test +++ b/llvm/test/DebugInfo/symbolize-filter-markup-bt.test @@ -18,12 +18,14 @@ CHECK: #0.1 0x0000000000000018 second /tmp[[SEP]]tmp.c:8:3 (a.o+0x8) CHECK: #0 0x0000000000000018 first /tmp[[SEP]]tmp.c:4:3 (a.o+0x8) CHECK: #0 0x0000000000000019 first /tmp[[SEP]]tmp.c:5:1 (a.o+0x9) CHECK: #0 0x00000000000000fe (a.o+0xee) + +CHECK: #0 0x00000000000000fe (a.o+0xee) +ERR: warning: expected at most 3 field(s); found 4 CHECK: [[BEGIN]]bt:0:0x111[[END]] +ERR: error: no mmap covers address ERR: error: expected at least 2 field(s); found 0 -ERR: error: no mmap covers address ERR: error: expected PC type; found '' -ERR: error: expected at most 3 field(s); found 4 ;--- input {{{module:0:a.o:elf:abcdef}}} @@ -34,10 +36,11 @@ ERR: error: expected at most 3 field(s); found 4 {{{bt:0:0x19:pc}}} {{{bt:0:0xff}}} -{{{bt}}} +{{{bt:0:0xff:pc:ext}}} {{{bt:0:0x111}}} + +{{{bt}}} {{{bt:0:0:}}} -{{{bt:0:0:pc:}}} ;--- asm.s # Generated by running "clang -finline -g -S tmp.c" in the following tmp.c on # Linux x86_64: diff --git a/llvm/test/DebugInfo/symbolize-filter-markup-pc.test b/llvm/test/DebugInfo/symbolize-filter-markup-pc.test index 19b5e5b..1aed432 100644 --- a/llvm/test/DebugInfo/symbolize-filter-markup-pc.test +++ b/llvm/test/DebugInfo/symbolize-filter-markup-pc.test @@ -14,13 +14,15 @@ CHECK: first[/dir[[SEP:[/\\]]]tmp.c:3] CHECK: first[/dir[[SEP]]tmp.c:5] CHECK: first[/dir[[SEP]]tmp.c:4] CHECK: first[/dir[[SEP]]tmp.c:5] + CHECK: [[BEGIN]]pc:0xff[[END]] CHECK: [[BEGIN]]pc:0x100[[END]] +CHECK: first[/dir[[SEP]]tmp.c:5] +ERR: error: no mmap covers address +ERR: warning: expected at most 2 field(s); found 3 ERR: error: expected at least 1 field(s); found 0 -ERR: error: no mmap covers address ERR: error: expected PC type; found '' -ERR: error: expected at most 2 field(s); found 3 ;--- input {{{module:0:a.o:elf:abcdef}}} @@ -29,12 +31,13 @@ ERR: error: expected at most 2 field(s); found 3 {{{pc:0x9}}} {{{pc:0x9:ra}}} {{{pc:0x9:pc}}} + {{{pc:0xff}}} +{{{pc:0x100}}} +{{{pc:0x9:pc:ext}}} {{{pc}}} -{{{pc:0x100}}} {{{pc:0x9:}}} -{{{pc:0x9:pc:}}} ;--- asm.s .text .file "tmp.c" diff --git a/llvm/test/DebugInfo/symbolize-filter-markup-reset.test b/llvm/test/DebugInfo/symbolize-filter-markup-reset.test index 75ffd5d..bbb05bc 100644 --- a/llvm/test/DebugInfo/symbolize-filter-markup-reset.test +++ b/llvm/test/DebugInfo/symbolize-filter-markup-reset.test @@ -8,7 +8,10 @@ CHECK: [[BEGIN:\[{3}]]ELF module #0x0 "a.o"; BuildID=ab [0x0-0x0](r)[[END:\]{3}] CHECK: {{ }}[[BEGIN]]reset[[END]] CHECK: [[BEGIN:\[{3}]]ELF module #0x0 "b.o"; BuildID=cd [0x1-0x1](r)[[END:\]{3}]] -ERR: error: expected 0 field(s); found 1 +CHECK: [[BEGIN]]reset:ext[[END]] +ERR: warning: expected 0 field(s); found 1 + +CHECK: [[BEGIN:\[{3}]]ELF module #0x0 "a.o"; BuildID=ab [0x0-0x0](r)[[END:\]{3}]] ;--- log {{{reset}}} @@ -18,4 +21,6 @@ ERR: error: expected 0 field(s); found 1 {{{module:0:b.o:elf:cd}}} {{{mmap:0x1:1:load:0:r:0}}} -{{{reset:}}} +{{{reset:ext}}} +{{{module:0:a.o:elf:ab}}} +{{{mmap:0:1:load:0:r:0}}} -- 2.7.4