This allows making a couple llvm-symbolizer tests run in all environments.
Keeping the existing exact prefix checks to make sure this is a strict NFC patch.
Differential D68133
[Symbolize] Use the local MSVC C++ demangler instead of relying on dbghelp. NFC. mstorsjo on Sep 27 2019, 5:51 AM. Authored by
Details
This allows making a couple llvm-symbolizer tests run in all environments. Keeping the existing exact prefix checks to make sure this is a strict NFC patch.
Diff Detail Event TimelineComment Actions Had to revert this one, as it broke sanitizer tests: http://lab.llvm.org:8011/builders/sanitizer-windows/builds/52441 So apparently it seems (again) like it would be useful if the llvm microsoft demangler could be configured to leave out parts of the output. In D68134 (in LLDB) we can consider changing the testcases to include the new demangler output, but there was a concern that there might be cases within LLDB that try to parse the demangled function names. And here it significantly changes the libfuzzer deduplication token output, which I think also isn't ideal. @thakis - you brought it up earlier that adding options to it could be possible. Do you, who I think is familiar with that demangler, have time to make a PoC of it? Otherwise I might have a stab at it sometimes later... Comment Actions That links is dead. Which customization is needed here? microsoftDemangle() already gets a Flags argument; we can just add more flags to it. Comment Actions Ah crap, I should have quoted the relevant bits... IIRC, it at least had extra public: at the start of the symbol, extra return type and calling convention specifiers. D68134 has a more concrete case of what needs to change in LLDB if the llvm demangler is used as-is there.
Oh, that's nice! Then this shoudln't probably be too big a deal to extend. I think for this particular case, for the fuzzer sanitizer's backtrace disambiguation log, there might be a risk that something is expecting to process the log, which might not be ready to handle extra unexpected keywords. I might be overly cautious though. In the LLDB case, a concern was voiced that some parts of LLDB might try to parse the demangled symbol names, and not expect those extra keywords there. Comment Actions I guess ideally we would have individual flags for all the UnDecorateSymbolName flags used here: UNDNAME_NO_ACCESS_SPECIFIERS, UNDNAME_NO_ALLOCATION_LANGUAGE, UNDNAME_NO_THROW_SIGNATURES, UNDNAME_NO_MEMBER_TYPE, UNDNAME_NO_MS_KEYWORDS, UNDNAME_NO_FUNCTION_RETURNS. Comment Actions
I think LLDB/win and fuzzers/win might have few enough clients that we can just trying to change it and see what breaks. Comment Actions Using specific flags to microsoftDemangle, to avoid spurious changes in libfuzzer tests when llvm-symbolizer is used there. (This means that it can't use the nice unified demangle function frontend that handles both itanium and ms style, and which does the char*->std::string wrapping.) Comment Actions It seems like this still broke a buildbot. @rnk, as it's your bot, can you look into it before we revert this? The logs don't really show enough detail... http://lab.llvm.org:8011/builders/sanitizer-windows/builds/53114/steps/stage%201%20check/logs/stdio Comment Actions Yep, done in r375147. The demangler difference is: MS: f(char,char *,char *) LLVM: f(char, char *, char *) Comment Actions Awesome, thanks! I guess that should have been deducible from the buildbot log, but I overlooked the spacing. |