delcypher added inline comments.
================
Comment at: clang/lib/Driver/ToolChains/Darwin.cpp:2298
SanitizerMask Res = ToolChain::getSupportedSanitizers();
- Res |= SanitizerKind::Address;
- Res |= SanitizerKind::Leak;
- Res |= SanitizerKind::Fuzzer;
- Res |= SanitizerKind::FuzzerNoLink;
+
+ if (sanitizerRuntimeExists("asan"))
----------------
george.karpenkov wrote:
> delcypher wrote:
> > I feel that we should assert that `Res` doesn't already contain the
> > SanitizerKind we are decided whether or not to set.
> > E.g.
> >
> > ```
> > assert(!(Res & SanitizerKind::Address));
> > if (sanitizerRuntimeExists("asan")) {
> > Res |= SanitizerKind::Address;
> > }
> > ```
> I'm not sure it would be worth crashing the driver.
Asserts are usually off in shipping compilers. If an assumption is being
violated, as a developer it's better than you know about it during debugging.
================
Comment at: clang/lib/Driver/ToolChains/Darwin.h:427
+ /// containing the sanitizer {@code SanitizerName}.
+ std::string sanitizerToRelativeLibName(StringRef SanitizerName,
+ bool Shared = true) const;
----------------
george.karpenkov wrote:
> delcypher wrote:
> > I don't like this name very much. Given that the path is relative to the
> > directory containing the library, what this function really does is given
> > the **file name** for a sanitizer library. Mentioning "relative" is just
> > confusing.
> >
> > Wouldn't something like `getSanitizerLibName()` or
> > `getNameForSanitizerLib()` be much clearer?
> Then it's not clear whether the returned path is relative or absolute though.
The name implies that it's a file name and not a path. It could be
`getSanitizerLibFileName()` or `getFileNameForSanitizerLib()` but that seemed a
little verbose.
https://reviews.llvm.org/D15225
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
