[cfe-commits] [PATCH] AddressSanitizer: set the AddressSafety attribute in CodeGenModule::SetLLVMFunctionAttributesForDefinition

[Chandler Carruth]

(switching back to cfe-commits, this isn't an llvm patch...)

This patch is pretty good. I just want to nit pick a bit w/ the way you're
using FileCheck. It won't cause you problems today, but it may save you
some painful debugging tomorrow to get into this habit.

One the tricky things about FileCheck is 'CHECK-NOT'. It often doesn't do
what people think it does. In particular, it only checks that there is not
a match starting from the previously matched position until the next
matched position. Thus, in some cases, your test might silently pass even
though there was a bug, because the first CHECK-NOT would scan the entire
file, and the second CHECK-NOT would start from the end of the file, and go
to the end of the file.

There are lots of little gotchas w/ CHECK-NOT. The way I try to use it is
to always have explicit bounds:

CHECK: function_name
CHECK-NOT: bad_attribute
CHECK: ret

The idea is to pincer the -NOT predicate between two that we're certain
will match. This is made easier because FileCheck is not actually operating
on a line-by-line basis, so the above will detect the following buggy
pattern:

def function_name bad_attribute {
  ...
  ret
}

(completely making up syntax here, but you get the point).

I'd try to use this to harden these filecheck tests a bit.


Also, just to make me happy about test coverage, want to add the attribute
to another objc construct, and check that we then suppress the LLVM
function attribute in that case?

Feel free to check in whenever, I or someone else can follow-up in
post-commit review if needed. Thanks for putting up w/ all the comments
here. =]
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120202/9e90a356/attachment.html>