[cfe-commits] Patch: Warn on zero-length memaccess

[Jordan Rose]

Hi, Chris. test/Analysis is really for static analyzer tests, not for compile-time analysis tests (even those that use libAnalysis). bstring.c and string.c are run multiple times to test the analyzer for when the builtin versions are and aren't available. Finally clang_analyzer_eval is used to test the static analyzer's understanding of an expression; it will print TRUE or FALSE if it can find a path where it can prove the value is non-zero/zero and UNKNOWN otherwise.

The upshot of all of this is that your new tests should go in Sema, for compile-time semantic warnings.

I think Chandler's right that this really only applies to memset; anything else will already warn on the sizeof not being castable to a pointer. And these zero-length memcpy calls will be optimized out by LLVM anyway, so Clang shouldn't need to worry about checking these prematurely.

The right way to suppress this isn't using void*. I think Chandler's point about ignoring macros and template parameters will cover most of the important cases; the diagnostic macros will cover the few other rare cases.

Thanks for coming up with this!
Jordan


On Jun 13, 2012, at 10:21 AM, Chris Pickel wrote:

> Hi, cfe-commits,
> 
> After spending far too long tracking down a bug which was ultimately
> caused by the line:
> 
>  memset(&x, sizeof(x), 0);  // Should be memset(&x, 0, sizeof(x))
> 
> I wrote a clang patch which adds a diagnostic for this situation. It
> warns on any call to a memaccess function (memset, memcpy, memcmp,
> &c.) in which a literal "0" is passed as the size. Like other
> memaccess diagnostics, it can be suppressed by casting &x to void*.
> 
> The git-formatted patch is attached, but it fails a few tests, and I'm
> not sure what the correct fix is. The test log is attached too; a
> summary is:
> 
> Analysis/bstring.c:
>    "TRUE" warnings expected in addition to zero-length memaccess warnings
> 
> Analysis/string.c
>    Warns on __builtin_strncpy instead of strcpy
>    But if I change the expectation, then it warns on strcpy instead
> 
> SemaCXX/warn-zero-length-memaccess:
> SemaCXX/zero-length-arrays:
>    Implicit copy-constructors appear to generate calls to
> __builtin_memcpy(…, 0)
>    This warning shouldn't really be checking __builtin_memcpy()
>    On the other hand, should __builtin_memcpy(…, 0) be generated to begin with?
> <llvm-test.txt><0001-Warn-on-zero-length-memaccess.patch>_______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits