[PATCH] D32064: [asan] Disable ASan global-GC depending on the target and compiler flags

Reid Kleckner via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Apr 19 14:58:23 PDT 2017


rnk added a subscriber: chandlerc.
rnk added a comment.

In https://reviews.llvm.org/D32064#728683, @pcc wrote:

> In https://reviews.llvm.org/D32064#728629, @rnk wrote:
>
> > In https://reviews.llvm.org/D32064#726861, @pcc wrote:
> >
> > > I think it would be better to move this logic to the driver and have it pass in an `-mllvm` flag. The sanitizer passes should really be taking no arguments in the constructor like the other passes, so I don't want us to add another argument here.
> >
> >
> > That seems like the opposite of the direction we've been moving, though. cl::opt flags can't appear twice, and this means one process can't do two asan compilations in two LLVMContexts in parallel with different settings.
>
>
> Yes, but adding an argument is also the wrong direction. This information should really be passed either via the module (e.g. module flags or attributes) or the TargetMachine. If we aren't going to do that, we might as well pass it via `-mllvm`, as it is simpler.


I'm really just echoing what I thought was conventional wisdom. I think avoiding new `cl::opt`s is something that @chandlerc cares more about. It's been said on llvm-dev that `cl::opt` should mostly be for debugging. We should be able to ship a production compiler that effectively compiles every cl::opt to its default value.

I don't see why constructor parameters are the wrong direction. It's already how ASan instrumentation takes every other global pass option. Only tsan uses -mllvm flags created by clang. If we aren't going to make ASan consistent, we should standardize on what we already have.


Repository:
  rL LLVM

https://reviews.llvm.org/D32064





More information about the cfe-commits mailing list