Adding Daniel, as this is mostly about libraries. + // LibAsan is "../lib/clang/linux/ArchName/libclang_rt.asan.a s/linux/darwin/ (in comments)
--kcc On Thu, Dec 1, 2011 at 1:44 AM, Alexander Potapenko <[email protected]>wrote: > 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. >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
