[PATCH] D137227: [asan] Default to -fsanitize-address-use-odr-indicator for non-Windows

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

In D137227#3908994 <https://reviews.llvm.org/D137227#3908994>, @sbc100 wrote:

> At least for WebAssembly object files, it looks like the symbol table now contains (what look like) demangled symbols.  e.g.:
>
>   $ llvm-nm /tmp/emscripten_temp/command_0.o 
>   00003ef5 D __odr_asan_gen__numargs
>   000041a6 D __odr_asan_gen__stdcmd<1068>::init
>   000041c3 D __odr_asan_gen__stdcmd<1069>::init
>   000041e0 D __odr_asan_gen__stdcmd<1070>::init
>   000041fd D __odr_asan_gen__stdcmd<1071>::init
>   0000421a D __odr_asan_gen__stdcmd<1073>::init
>   0000423f D __odr_asan_gen__stdcmd<1074>::init
>   0000425c D __odr_asan_gen__stdcmd<1075>::init
>   00004279 D __odr_asan_gen__stdcmd<1076>::init
>   0000429e D __odr_asan_gen__stdcmd<1077>::init
>   000042bc D __odr_asan_gen__stdcmd<1078>::init
>
> Symbol names with colon in them currently cause issues with the emscripten compiler (when parsing the output of `llvm-nm`).   If this is intensional we can look into re-working the parser, but I just wanted to check if this was indented behaviour?    Are these character allows in symbol names?

The asan instrumentation just prepends `__odr_asan_gen_` to the symbol name to form a new symbol name. For ELF every byte except `\0` can be used in a symbol name, and this is totally fine.

I am unfamiliar with WebAssembly. Does the aforementioned parsing tool somehow skip printing `_stdcmd<1068>::init` symbols?



================
Comment at: llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp:777
         // Enable aliases as they should have no downside with ODR indicators.
-        UsePrivateAlias(UseOdrIndicator || ClUsePrivateAlias),
-        UseOdrIndicator(UseOdrIndicator || ClUseOdrIndicator),
+        UsePrivateAlias(UseOdrIndicator && ClUsePrivateAlias),
+        UseOdrIndicator(UseOdrIndicator && ClUseOdrIndicator),
----------------
vitalybuka wrote:
> It should be 
> ```
> UseOdrIndicator(ClUseOdrIndicator.getNumOccurrences() > 0 ? ClUseOdrIndicator : UseOdrIndicator),
> ClUsePrivateAlias(ClUsePrivateAlias.getNumOccurrences() > 0 ? ClUsePrivateAlias : UseOdrIndicator),
> ```
> 
> My rule of thumb is Cl flags, which are mostly for testing, must be direct and do exactly what was asked, even if it's not useful, and they should override fronted parameters.
> 
Good catch. I agree with this analysis.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137227/new/

https://reviews.llvm.org/D137227