Committed as r172806 (llvm) and r172808 (clang). Thanks! ~Will
On Fri, Jan 18, 2013 at 2:28 AM, Alexey Samsonov <[email protected]> wrote: > LGTM. Thanks! > > > On Fri, Jan 18, 2013 at 2:28 AM, Richard Smith <[email protected]> > wrote: >> >> Clang side looks great to me, thanks! >> >> Alexey: Does this look OK from your end? >> >> On Thu, Jan 17, 2013 at 8:53 AM, Will Dietz <[email protected]> wrote: >> > Great, thank you both for your feedback. I also agree the approach >> > originally submitted was messy, and Richard you nailed the conflict >> > that drove me to accept it. Thank you for giving me a better >> > alternative :). >> > >> > Updated patches attached. >> > >> > Notes: >> > * It was unclear to what IR clang should generate for a blacklisted >> > function/module when using address sanitizer. I've added a test >> > verifying a blacklisted function does /not/ get given the >> > address-safety attribute, which is a deviation from existing >> > functionality. Looking at the Asan LLVM transform suggests this is >> > fine (and seems like the right thing to do from clang's perspective >> > regardless), but would appreciate confirmation this is correct. >> > * I extended the AST serialization code to write out the sanitization >> > flags, only because that's what the code does presently. Note that >> > all users of ASTReader seem to ignore benign language options (such as >> > sanitization flags). One result is that AFAICT testing this would >> > require creating additional code to dump LangOpts or similar. Is this >> > worth pursuing? >> > * At the time of CodeGenFunction construction we presently do not give >> > the ctor sufficient information to query the blacklist (function >> > name), so a conditionally bound reference didn't work. Using a >> > pointer seemed preferable to the refactoring required to alleviate >> > this issue. >> > >> > Thanks! >> > >> > ~Will >> > >> > On Thu, Jan 17, 2013 at 2:24 AM, Alexey Samsonov <[email protected]> >> > wrote: >> >> I think that making blacklist available in Clang is a good idea. >> >> I agree with Richard's comment as well. >> >> >> >> >> >> On Thu, Jan 17, 2013 at 12:50 AM, Richard Smith <[email protected]> >> >> wrote: >> >>> >> >>> On the clang side, it seems messy and error-prone to distribute the >> >>> checking for blacklisting across all the callers of EmitCheck, but I >> >>> understand that you're trying to avoid generating the check condition >> >>> for blacklisted functions. Maybe we could make a separate struct >> >>> containing just the Sanitize* flags, and in the CodeGenFunction >> >>> constructor bind a reference to either the global set of flags or a >> >>> set of all-false flags, depending on whether we're blacklisted? This >> >>> would also make it easier to blacklist specific sanitizers in specific >> >>> functions later. >> >>> >> >>> On Wed, Jan 16, 2013 at 12:08 PM, Will Dietz <[email protected]> >> >>> wrote: >> >>> > Minor touchup to clang patch, thanks! >> >>> > >> >>> > ~Will >> >>> > >> >>> > On Wed, Jan 16, 2013 at 11:37 AM, Will Dietz <[email protected]> >> >>> > wrote: >> >>> >> Attached are two patches. >> >>> >> >> >>> >> First applies to clang and uses the blacklist to avoid >> >>> >> instrumenting >> >>> >> the source files or functions specified. Lit test included. >> >>> >> >> >>> >> The other is a small change to llvm to make the "Blacklist" class >> >>> >> visible to Clang. Not sure I understand the header organization >> >>> >> system well enough, let me know if it belongs elsewhere. >> >>> >> >> >>> >> Thanks! >> >>> >> >> >>> >> ~Will >> >>> > >> >>> > _______________________________________________ >> >>> > cfe-commits mailing list >> >>> > [email protected] >> >>> > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >> >>> > >> >>> _______________________________________________ >> >>> cfe-commits mailing list >> >>> [email protected] >> >>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >> >> >> >> >> >> >> >> >> >> -- >> >> Alexey Samsonov, MSK > > > > > -- > Alexey Samsonov, MSK _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
