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 _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
