[llvm] 7b31a73 - [Symbolizer] Ignore unknown additional symbolizer markup fields
Daniel Thornburgh via llvm-commits
llvm-commits at lists.llvm.org
Wed Jun 28 10:39:33 PDT 2023
Author: Daniel Thornburgh
Date: 2023-06-28T10:39:28-07:00
New Revision: 7b31a73ffe87e6302ab433da8128058fce8f54bd
URL: https://github.com/llvm/llvm-project/commit/7b31a73ffe87e6302ab433da8128058fce8f54bd
DIFF: https://github.com/llvm/llvm-project/commit/7b31a73ffe87e6302ab433da8128058fce8f54bd.diff
LOG: [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
Added:
Modified:
llvm/docs/SymbolizerMarkupFormat.rst
llvm/include/llvm/DebugInfo/Symbolize/MarkupFilter.h
llvm/lib/DebugInfo/Symbolize/MarkupFilter.cpp
llvm/test/DebugInfo/symbolize-filter-markup-bt.test
llvm/test/DebugInfo/symbolize-filter-markup-pc.test
llvm/test/DebugInfo/symbolize-filter-markup-reset.test
Removed:
################################################################################
diff --git a/llvm/docs/SymbolizerMarkupFormat.rst b/llvm/docs/SymbolizerMarkupFormat.rst
index 1cf1d64ada8d0..d6b22a409969b 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 429f9cba8a583..a1514d91702b9 100644
--- a/llvm/include/llvm/DebugInfo/Symbolize/MarkupFilter.h
+++ b/llvm/include/llvm/DebugInfo/Symbolize/MarkupFilter.h
@@ -123,7 +123,7 @@ class MarkupFilter {
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 48833a79fd456..a2bc2577b70ac 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<uint64_t> 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<uint64_t> 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 a1701135f2d9a..b8d8e1873b31f 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 19b5e5be0d376..1aed43296748a 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 75ffd5d943505..bbb05bc21835a 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}}}
More information about the llvm-commits
mailing list