On 2/26/20 3:09 PM, Jeff Law wrote:
On Wed, 2020-02-19 at 17:26 -0700, Martin Sebor wrote:
The buffer overflow detection for multi-char stores uses the size
of a source array even when what's actually being accessed (read
and stored) is a pointer to the array. That leads to incorrect
warnings in some cases.
The attached patch corrects the function that computes the size of
the access to set it to that of a pointer instead if the source is
an address expression.
Tested on x86_64-linux.
if (TREE_CODE (exp) == ADDR_EXPR)
- exp = TREE_OPERAND (exp, 0);
+ {
+ /* If the size of the access hasn't been determined yet it's that
+ of a pointer. */
+ if (!nbytes)
+ nbytes = tree_to_uhwi (TYPE_SIZE_UNIT (TREE_TYPE (exp)));
+ exp = TREE_OPERAND (exp, 0);
+ }
This doesn't make any sense to me. You're always going to get the size of a
pointer here. Don't you want the size of the TYPE of the operand?
The function correctly handles cases when the expression is an array
because the array is what's being stored. The bug is in stripping
the ADDR_EXPR and then using the size of the operand when what's
being stored is actually a pointer.
This test case illustrates the difference:
struct S { char *p, *q, r[8]; } a;
void f (void)
{
struct S b = { 0, "1234567", "abcdefgh" };
__builtin_memcpy (&a, &b, sizeof a);
}
The memcpy results in:
MEM <char *> [(char * {ref-all})&b] = 0B;
MEM <char *> [(char * {ref-all})&b + 8B] = "1234567";
MEM <unsigned char[24]> [(char * {ref-all})&a] = MEM <unsigned
char[24]> [(char * {ref-all})&b];
The RHS of the second MEM_REF is ADDR_EXPR (STRING_CST). The RHS
of the third MEM_REF is the array case (that's handled correctly).
I think computing the size of the store could be simplified (it
seems it should be the same as type of either the RHS or the LHS)
but I didn't want to mess with things too much this late.
Martin
