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
