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

[Vitaly Buka via Phabricator via cfe-commits]

vitalybuka added inline comments.


================
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),
----------------
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.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137227