[cfe-commits] r133136 - in /cfe/trunk: lib/Sema/SemaChecking.cpp test/SemaCXX/warn-memset-bad-sizeof.cpp

[Chandler Carruth]

First off, thanks for the review.

On Wed, Jun 15, 2011 at 7:27 PM, Nico Weber <thakis at chromium.org> wrote:

> > Nico, please review; does this miss any of the bugs you were trying to
> > find with this warning? The array test case you had should be caught by
> > the array-specific sizeof warning I think.
>
> Yes, now it doesn't find http://codereview.chromium.org/7167009/
>
> Can you provide any examples where this warns when it shouldn't? (…and
> maybe revert this in the meantime?)


Ugh. So, I'd really rather not revert this, as it hits a frustratingly
common false positive I added to the test cases: copying a pointer's value
into or out of a character array, often from an opaque memory buffer. If the
pointer is itself a character pointer, then the warning fires. Because the
standard specifically blesses the use af character arrays and memcpy, it
seems bad to warn on their interactions just because of one particular
sizeof() usage in the third argument.

Looking at the bug you point to, it's not clear why comparing the types of
the first argument and the sizeof argument is the principled way to warn
about this construct. You're not copying characters, and so the expected
type of the argument to sizeof() isn't a 'char' (as the warning suggests).
Instead, the correct code doesn't have any sizeof at all...

Could chat and see if there is some better way to flag this kind of code?
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20110615/b1929449/attachment.html>