llvm-symbolizer --markup-filter does not adjust addresses without debug info

[mysterymath]

If a lookup for debug info fails when symbolizing a log containing markup, a raw representation of the element is printed instead of a best effort version of the value. This makes sense for syntactic or semantic errors, but in cases where debug info simply cannot be found, there is still a good deal of information that the filter can print simply through the contextual markup elements. In particular, it can print addresses adjusted by the runtime memory layout of the process, which makes it easier to e.g. objdump the addresses later.

Fixing this should just be a matter of printing something like [[[PC <addr>]]] instead of the current printRawElement. In these cases the correct address has already been computed, so it should be a relatively straightforward change.

Reference:
llvm/lib/DebugInfo/Symbolize/MarkupFilter.cpp
Symbolizer Markup Format

[llvmbot]

@llvm/issue-subscribers-tools-llvm-symbolizer

[llvmbot]

Hi!

This issue may be a good introductory issue for people new to working on LLVM. If you would like to work on this issue, your first steps are:

1. Assign the issue to you.
2. Fix the issue locally.
3. Run the test suite locally.
   3.1) Remember that the subdirectories under test/ create fine-grained testing targets, so you can
   e.g. use make check-clang-ast to only run Clang's AST tests.
4. Create a git commit
5. Run git clang-format HEAD~1 to format your changes.
6. Submit the patch to Phabricator.
   6.1) Detailed instructions can be found here

For more instructions on how to submit a patch to LLVM, see our documentation.

If you have any further questions about this issue, don't hesitate to ask via a comment on this Github issue.

@llvm/issue-subscribers-good-first-issue

[cchung100m]

Hi @mysterymath

I am new to LLVM and interested in beginner issue to start with. Could you assign this issue to me for practicing the contribution process?

[thetazero]

Doesn't look like this is still active, could I take this on @cchung100m?

[maximeplante]

I'm working on a fix: maximeplante@b70221b. Could I be the the assignee for this bug since it hasn't been touched in a while?

Before I make a pull request with my work I have a question: I am worried [[[PC 0x...]]] will be confused with [[[pc:0x...]]]. Should we name it something like [[[No symbol 0x...]]]?

[mysterymath]

I'm working on a fix: maximeplante@b70221b. Could I be the the assignee for this bug since it hasn't been touched in a while?

Before I make a pull request with my work I have a question: I am worried [[[PC 0x...]]] will be confused with [[[pc:0x...]]]. Should we name it something like [[[No symbol 0x...]]]?

No symbol definitely doesn't preserve the fact that this was a program counter, but I'd agree that it shouldn't be confusable for the unadjusted raw markup. Perhaps switching from square brackets to angle brackets would suffice: <<<PC ...>>>. We'd also probably want to come up with as human-readable of a syntax as possible for each of the existing cases of printRawElement. PC seems reasonable here, since that's one of two common abbreviations for the program counter.

[maximeplante]

I can change it to <<<PC ...>>>.

We'd also probably want to come up with as human-readable of a syntax as possible for each of the existing cases of printRawElement.

Do you have in mind to use the square bracket approach for printRawElement too?

[mysterymath]

I can change it to <<<PC ...>>>.

We'd also probably want to come up with as human-readable of a syntax as possible for each of the existing cases of printRawElement.

Do you have in mind to use the square bracket approach for printRawElement too?

For consistency we should probably change all of the output for to use angle brackets, even printRawElement. The original intent was for the output to be symbolizable again at a later time when more information was available. When we looked into it, the semantics got very squirrely though, and there doesn't seem to be much of a demand for it. So we should probably transform all elements into something that doesn't look like a markup element.

[maximeplante]

Should I just PR the change I have right now and then follow-up with a separate bug covering the change from square brackets to angled brackets everywhere else?

[mysterymath]

Should I just PR the change I have right now and then follow-up with a separate bug covering the change from square brackets to angled brackets everywhere else?

Right now the behavior is consistent, if not ideal. Since it's avoidable, we should avoid introducing an inconsistent state in the meantime. Fixing up the various brackets shouldn't produce too big of a PR, I'd think.

If you like to get a head start on review though, you can through up a draft for me to look through.