The unobufscation support for BCSymbolMaps was the last piece of code that hasn't been upstreamed yet. This patch contains a reworked version of the existing code and relevant tests.
Details
Diff Detail
- Repository
- rL LLVM
Event Timeline
llvm/test/tools/dsymutil/ARM/obfuscated.test | ||
---|---|---|
5 ↗ | (On Diff #180351) | nit: how about splitting those lines at 80 chars? RUN: ... \ RUN: -... |
llvm/tools/dsymutil/DwarfStreamer.cpp | ||
581 ↗ | (On Diff #180351) | comment should be on the declaration not the implementation, I believe. |
597 ↗ | (On Diff #180351) | Any plans to implement this soon? |
llvm/tools/dsymutil/SymbolMap.cpp | ||
50 ↗ | (On Diff #180351) | s/that weird/cross reference to the MC code that explains why we do this/ ? |
54 ↗ | (On Diff #180351) | this is in namespace llvm, does this need to be qualified? |
68 ↗ | (On Diff #180351) | It's unfortunate that dsymutil on non-macOS will behave differently... but this is not a regression |
69 ↗ | (On Diff #180351) | . |
74 ↗ | (On Diff #180351) | why parent_path first and then ..? |
93 ↗ | (On Diff #180351) | perhaps introduce a local var to make this more readable with 80 chars? |
133 ↗ | (On Diff #180351) | perhaps use StringRef::consume() for premature optimization? |
llvm/tools/dsymutil/SymbolMap.h | ||
22 ↗ | (On Diff #180351) | Doxygen comment? |
26 ↗ | (On Diff #180351) | ArrayRef or && or const& perhaps? |
Feedback Adrian
llvm/tools/dsymutil/DwarfStreamer.cpp | ||
---|---|---|
597 ↗ | (On Diff #180351) | Soon, hopefully :-) |
llvm/tools/dsymutil/SymbolMap.cpp | ||
133 ↗ | (On Diff #180351) | I tried but this makes the code more complicated because we need to account for either the version number missing or the whole thing missing. |
llvm/tools/dsymutil/SymbolMap.h | ||
26 ↗ | (On Diff #180351) | Neither would improve performance as we're moving it into a member. I believe it's customary to take the argument by value in this case (although an r-value reference would be equivalent). |
llvm/tools/dsymutil/dsymutil.cpp | ||
---|---|---|
515 ↗ | (On Diff #180564) | That comment doesn't carry a lot of information :-) |