This is an archive of the discontinued LLVM Phabricator instance.

[dfsan] Replace dfs$ prefix with .dfsan suffix
ClosedPublic

Authored by gbalats on Jun 17 2021, 4:13 PM.

Details

Summary

The current naming scheme adds the dfs$ prefix to all DFSan-instrumented functions.
This breaks mangling and prevents stack trace printers and other tools from automatically
demangling function names.

This new naming scheme is mangling-compatible, with the .dfsan suffix being a vendor-specific suffix:
https://itanium-cxx-abi.github.io/cxx-abi/abi.html#mangling-structure

With this fix, demangling utils would work out-of-the-box.

Diff Detail

Event Timeline

gbalats created this revision.Jun 17 2021, 4:13 PM
gbalats requested review of this revision.Jun 17 2021, 4:13 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 17 2021, 4:13 PM
Herald added subscribers: llvm-commits, Restricted Project, cfe-commits. · View Herald Transcript
gbalats edited the summary of this revision. (Show Details)Jun 17 2021, 4:13 PM
kcc added a subscriber: kcc.Jun 17 2021, 4:56 PM

Yey, great idea! :)
(I am not reviewing the code; but the change looks straightforward)

llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp
1134

Based on http://web.mit.edu/rhel-doc/3/rhel-as-en-3/symver.html, there must be a @ in the .symver line after the first match.
Please change Pos != std::string::npos to be like

Pos = Asm.find("@", Pos);
assert(Pos != std::string::npos);
gbalats updated this revision to Diff 352883.Jun 17 2021, 5:36 PM
gbalats edited the summary of this revision. (Show Details)

Report fatal error when .symver doesn't include '@'.

This revision is now accepted and ready to land.Jun 17 2021, 5:38 PM
gbalats marked an inline comment as done.Jun 17 2021, 5:40 PM
gbalats added inline comments.
llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp
1134

Done. Used report_fatal_error instead of assert since this can be triggered by user input. Ideally, I think we should be using some recoverable error mechanism, but since we're not doing it elsewhere, I'm not going to introduce this with this change.

This revision was landed with ongoing or failed builds.Jun 17 2021, 10:45 PM
This revision was automatically updated to reflect the committed changes.
gbalats marked an inline comment as done.

Looking into this. I missed (due to the "\\" in the expression) updating this:
https://github.com/llvm/llvm-project/blob/main/compiler-rt/lib/fuzzer/dataflow/DataFlow.cpp#L93-L94

I will try to get ninja check-fuzzer working again and either do a small fix forward or revert (if it turns out to be more complicated than just updating this expression).

Looking into this. I missed (due to the "\\" in the expression) updating this:
https://github.com/llvm/llvm-project/blob/main/compiler-rt/lib/fuzzer/dataflow/DataFlow.cpp#L93-L94

I will try to get ninja check-fuzzer working again and either do a small fix forward or revert (if it turns out to be more complicated than just updating this expression).

https://reviews.llvm.org/D104568