george.karpenkov added a comment.
@delcypher sorry i don't agree with the change requests.
@mracek any 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"))
----------------
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.
================
Comment at: clang/lib/Driver/ToolChains/Darwin.h:134
+ /// \return Directory to find the runtime library in.
+ SmallString<128> runtimeLibDir(bool IsEmbedded=false) const;
+
----------------
delcypher wrote:
> Why is this a `SmallString<128>` but in other places we're just using
> `std::string`?
For driver it does not matter which datastructure to use.
Here SmallString is used because that's what is more convenient to return.
================
Comment at: clang/lib/Driver/ToolChains/Darwin.h:427
+ /// containing the sanitizer {@code SanitizerName}.
+ std::string sanitizerToRelativeLibName(StringRef SanitizerName,
+ bool Shared = true) const;
----------------
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.
================
Comment at: clang/test/Driver/darwin-asan-nofortify.c:3
-// RUN: %clang -fsanitize=address %s -E -dM -target x86_64-darwin | FileCheck
%s
+// RUN: %clang -fsanitize=address %s -E -dM -target x86_64-darwin
-resource-dir %S/Inputs/resource_dir | FileCheck %s
----------------
delcypher wrote:
> Are these `-resource-dir %S/Inputs/resource_dir` needed because compiler-rt
> might not be present and now the driver checks for these?
By default -resource-dir points to the resource dir in the "bin" directory. The
contents of that is sort of random, and depends on what ninja commands have you
ran before.
https://reviews.llvm.org/D15225
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
