On Thu, Dec 1, 2011 at 1:41 PM, Chandler Carruth <[email protected]> wrote:
> On Thu, Dec 1, 2011 at 1:38 AM, Alexander Potapenko <[email protected]>
> wrote:
>>
>> On Thu, Dec 1, 2011 at 1:32 PM, Chandler Carruth <[email protected]>
>> wrote:
>> > On Thu, Dec 1, 2011 at 1:28 AM, Alexander Potapenko <[email protected]>
>> > wrote:
>> >>
>> >> Attached the new version, PTAL
>> >>
>> >> > + if (Args.hasArg(options::OPT_faddress_sanitizer)) {
>> >> > Please
>> >> >
>> >> >
>> >> > use Args.hasFlag(options::OPT_faddress_sanitizer, options::OPT_fno_address_sanitizer,
>> >> > false))
>> >> Done
>> >> >
>> >> > and hide the flag checking inside addAsanRTDarwinExe/etc
>> >> Then this should be something like "maybeAddAsan...", because this
>> >> function is called unconditionally and other devs may think that we
>> >> always add ASan.
>> >> I've renamed the correspoding Linux function as well.
>> >
>> >
>> > I don't agree... The function is responsible for adding what Address
>> > Sanitizer needs, and it needs nothing if disabled. This matches several
>> > other 'addFoo' functions in the driver.
>>
>> Sounds convincing, I really haven't noticed other "addFoo" functions.
>> Fixed.
>
> FWIW, this patch looks fine to me with one nit: please don't introduce more
> trailing whitespace. It's important (for the annotation history) to not
> strip trailing whitespace that already exists in the files, but I would ask
> that you don't introduce more. =]
> Still, I thing someone more familiar with Darwin should look at this before
> it goes in... I just can't realistically review it for correctness on that
> platform.
Ok, done.
Waiting for someone with Darwin background to take a look.
Index: tools/clang/lib/Driver/Tools.cpp
===================================================================
--- tools/clang/lib/Driver/Tools.cpp (revision 145532)
+++ tools/clang/lib/Driver/Tools.cpp (working copy)
@@ -1121,6 +1121,38 @@
TC.AddCXXStdlibLibArgs(Args, CmdArgs);
}
+static void addAsanRTDarwin(const ToolChain &TC, const ArgList &Args,
+ ArgStringList &CmdArgs) {
+ // Add linker flags only if ASan is on.
+ if (!Args.hasFlag(options::OPT_faddress_sanitizer,
+ options::OPT_fno_address_sanitizer, false)) return;
+
+ if (!Args.hasArg(options::OPT_dynamiclib) &&
+ !Args.hasArg(options::OPT_bundle)) {
+ // Exe case: link with the runtime library.
+
+ // LibAsan is "../lib/clang/linux/ArchName/libclang_rt.asan.a
+ llvm::SmallString<128> LibAsan =
+ llvm::sys::path::parent_path(StringRef(TC.getDriver().Dir));
+ llvm::sys::path::append(
+ LibAsan, "lib", "clang", "darwin", TC.getArchName());
+ llvm::sys::path::append(LibAsan, "libclang_rt.asan.a");
+ CmdArgs.push_back(Args.MakeArgString(LibAsan));
+ CmdArgs.push_back("-lpthread");
+ CmdArgs.push_back("-ldl");
+ CmdArgs.push_back("-framework");
+ CmdArgs.push_back("Foundation");
+ TC.AddCXXStdlibLibArgs(Args, CmdArgs);
+ } else {
+ // Dylib case: treat all undefined functions as dynamic_lookup.
+ // This is needed because after ASan instrumentation the dynamic library
+ // may have calls to ASan interface functions that exist only in the
+ // executable.
+ CmdArgs.push_back("-undefined");
+ CmdArgs.push_back("dynamic_lookup");
+ }
+}
+
void Clang::ConstructJob(Compilation &C, const JobAction &JA,
const InputInfo &Output,
const InputInfoList &Inputs,
@@ -3576,6 +3608,9 @@
}
Args.AddAllArgs(CmdArgs, options::OPT_L);
+
+ // Call this before we add the C run-time.
+ addAsanRTDarwin(getToolChain(), Args, CmdArgs);
if (Args.hasArg(options::OPT_fopenmp))
// This is more complicated in gcc...
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits