Skip to content

R
riscv-gcc-1
Commit 6889a3ac by Martin Sebor Committed by Martin Sebor
Options

PR middle-end/91631 - buffer overflow into an array member of a declared object not detected 

gcc/ChangeLog:

	PR middle-end/91631
	* builtins.c (component_size): Correct trailing array computation,
	rename to component_ref_size and move...
	(compute_objsize): Adjust.
	* gimple-ssa-warn-restrict.c (builtin_memref::refsize): New member.
	(builtin_access::strict): Do not consider mememmove.
	(builtin_access::write_off): New function.
	(builtin_memref::builtin_memref): Initialize refsize.
	(builtin_memref::set_base_and_offset): Adjust refoff and compute
	refsize.
	(builtin_memref::offset_out_of_bounds): Use ooboff input values.
	Handle refsize.
	(builtin_access::builtin_access): Intialize dstoff to destination
	refeence offset here instead of in maybe_diag_overlap.  Adjust
	referencess even to unrelated objects.	Adjust sizrange of bounded
	string functions to reflect bound.  For strcat, adjust destination
	sizrange by that of source.
	(builtin_access::strcat_overlap):  Adjust offsets and sizes
	to reflect the increase in destination sizrange above.
	(builtin_access::overlap): Do not set dstoff here but instead
	in builtin_access::builtin_access.
	(check_bounds_or_overlap): Use builtin_access::write_off.
	(maybe_diag_access_bounds): Add argument.  Add informational notes.
	(dump_builtin_memref, dump_builtin_access): New functions.
	* tree.c (component_ref_size): ...to here.
	* tree.h (component_ref_size): Declare.
	* tree-ssa-strlen (handle_builtin_strcat): Include the terminating
	nul in the size of the source string.

gcc/testsuite/ChangeLog:

	PR middle-end/91631
	* /c-c++-common/Warray-bounds-3.c: Correct expected offsets.
	* /c-c++-common/Warray-bounds-4.c: Same.
	* gcc.dg/Warray-bounds-39.c: Remove xfails.
	* gcc.dg/Warray-bounds-45.c: New test.
	* gcc.dg/Warray-bounds-46.c: New test.

From-SVN: r275981
parent 7d112d66
Showing with 141 additions and 71 deletions
gcc/ChangeLog
2019-09-19 Martin Sebor <msebor@redhat.com>
PR middle-end/91631
* builtins.c (component_size): Correct trailing array computation,
rename to component_ref_size and move...
(compute_objsize): Adjust.
* gimple-ssa-warn-restrict.c (builtin_memref::refsize): New member.
(builtin_access::strict): Do not consider memmove.
(builtin_access::write_off): New function.
(builtin_memref::builtin_memref): Initialize refsize.
(builtin_memref::set_base_and_offset): Adjust refoff and compute
refsize.
(builtin_memref::offset_out_of_bounds): Use ooboff input values.
Handle refsize.
(builtin_access::builtin_access): Initialize dstoff to destination
refeence offset here instead of in maybe_diag_overlap. Adjust
referencess even to unrelated objects. Adjust sizrange of bounded
string functions to reflect bound. For strcat, adjust destination
sizrange by that of source.
(builtin_access::strcat_overlap): Adjust offsets and sizes
to reflect the increase in destination sizrange above.
(builtin_access::overlap): Do not set dstoff here but instead
in builtin_access::builtin_access.
(check_bounds_or_overlap): Use builtin_access::write_off.
(maybe_diag_access_bounds): Add argument. Add informational notes.
(dump_builtin_memref, dump_builtin_access): New functions.
* tree.c (component_ref_size): ...to here.
* tree.h (component_ref_size): Declare.
* tree-ssa-strlen (handle_builtin_strcat): Include the terminating
nul in the size of the source string.
2019-09-19 Lewis Hyatt <lhyatt@gmail.com>
PR c/67224
......
gcc/builtins.c
......@@ -3562,54 +3562,6 @@ check_access (tree exp, tree, tree, tree dstwrite,
return true;
}
/* Determines the size of the member referenced by the COMPONENT_REF
REF, using its initializer expression if necessary in order to
determine the size of an initialized flexible array member.
Returns the size (which might be zero for an object with
an uninitialized flexible array member) or null if the size
cannot be determined. */
static tree
component_size (tree ref)
{
gcc_assert (TREE_CODE (ref) == COMPONENT_REF);
tree member = TREE_OPERAND (ref, 1);
/* If the member is not last or has a size greater than one, return
it. Otherwise it's either a flexible array member or a zero-length
array member, or an array of length one treated as such. */
tree size = DECL_SIZE_UNIT (member);
if (size
&& (!array_at_struct_end_p (ref)
|| (!integer_zerop (size)
&& !integer_onep (size))))
return size;
/* If the reference is to a declared object and the member a true
flexible array, try to determine its size from its initializer. */
poly_int64 off = 0;
tree base = get_addr_base_and_unit_offset (ref, &off);
if (!base || !VAR_P (base))
return NULL_TREE;
/* The size of any member of a declared object other than a flexible
array member is that obtained above. */
if (size)
return size;
if (tree init = DECL_INITIAL (base))
if (TREE_CODE (init) == CONSTRUCTOR)
{
off <<= LOG2_BITS_PER_UNIT;
init = fold_ctor_reference (NULL_TREE, init, off, 0, base);
if (init)
return TYPE_SIZE_UNIT (TREE_TYPE (init));
}
return DECL_EXTERNAL (base) ? NULL_TREE : integer_zero_node;
}
/* Helper to compute the size of the object referenced by the DEST
expression which must have pointer type, using Object Size type
OSTYPE (only the least significant 2 bits are used). Return
......@@ -3744,7 +3696,7 @@ compute_objsize (tree dest, int ostype, tree *pdecl /* = NULL */)
if (TREE_CODE (dest) == COMPONENT_REF)
{
*pdecl = TREE_OPERAND (dest, 1);
return component_size (dest);
return component_ref_size (dest);
}
if (TREE_CODE (dest) != ADDR_EXPR)
......
gcc/testsuite/ChangeLog
2019-09-19 Martin Sebor <msebor@redhat.com>
PR middle-end/91631
* /c-c++-common/Warray-bounds-3.c: Correct expected offsets.
* /c-c++-common/Warray-bounds-4.c: Same.
* gcc.dg/Warray-bounds-39.c: Remove xfails.
* gcc.dg/Warray-bounds-45.c: New test.
* gcc.dg/Warray-bounds-46.c: New test.
2019-09-19 Lewis Hyatt <lhyatt@gmail.com>
PR c/67224
......
gcc/testsuite/c-c++-common/Warray-bounds-3.c
......@@ -115,7 +115,7 @@ void test_memcpy_bounds_anti_range (char *d, const char *s, size_t n)
offset, i.e., 7 + 3. Including the whole final range because would be
confusing (the upper bound would either be negative or a very large
positive number) so only the lower bound is included. */
T (char, 9, a, a + SAR ( 0, 6), 3); /* { dg-warning "forming offset 10 is out of the bounds \\\[0, 9] of object " "memcpy" } */
T (char, 9, a, a + SAR ( 0, 6), 3); /* { dg-warning "forming offset 9 is out of the bounds \\\[0, 9] of object " "memcpy" } */
/* This fails because the offset isn't represented as an SSA_NAME
but rather as a GIMPLE_PHI (offset, 0). With some effort it is
......@@ -129,18 +129,18 @@ void test_memcpy_bounds_anti_range (char *d, const char *s, size_t n)
T (char, 9, a, a + SAR ( 2, 6), 3);
T (char, 9, a, a + SAR ( 3, 6), 3);
T (char, 9, a, a + SAR (-1, 7), 3); /* { dg-warning "forming offset \\\[10, 11] is out of the bounds \\\[0, 9] of object " "memcpy" } */
T (char, 9, a, a + SAR (-2, 8), 3); /* { dg-warning "forming offset \\\[10, 12] is out of the bounds \\\[0, 9] of object " "memcpy" } */
T (char, 9, a, a + SAR (-3, 7), 5); /* { dg-warning "forming offset \\\[10, 13] is out of the bounds \\\[0, 9] of object " "memcpy" } */
T (char, 9, a, a + SAR (-1, 7), 3); /* { dg-warning "forming offset \\\[9, 10] is out of the bounds \\\[0, 9] of object " "memcpy" } */
T (char, 9, a, a + SAR (-2, 8), 3); /* { dg-warning "offset \\\[9, 11] is out of the bounds \\\[0, 9] of object " "memcpy" } */
T (char, 9, a, a + SAR (-3, 7), 5); /* { dg-warning "forming offset \\\[9, 12] is out of the bounds \\\[0, 9] of object " "memcpy" } */
T (char, 9, a + SAR (-2, -1), a, 3);
T (char, 9, a + SAR (-1, 1), a, 3);
T (char, 9, a + SAR ( 0, 1), a, 3);
T (char, 9, a + SAR ( 0, 2), a, 3);
T (char, 9, a + SAR ( 0, 3), a, 3);
T (char, 9, a + SAR ( 0, 6), a, 3); /* { dg-warning "forming offset 10 is out of the bounds \\\[0, 9] of object " "memcpy" } */
T (char, 9, a + SAR (-1, 7), a, 3); /* { dg-warning "forming offset \\\[10, 11] is out of the bounds \\\[0, 9] of object " "memcpy" } */
T (char, 9, a + SAR (-2, 8), a, 3); /* { dg-warning "forming offset \\\[10, 12] is out of the bounds \\\[0, 9] of object " "memcpy" } */
T (char, 9, a + SAR ( 0, 6), a, 3); /* { dg-warning "forming offset 9 is out of the bounds \\\[0, 9] of object " "memcpy" } */
T (char, 9, a + SAR (-1, 7), a, 3); /* { dg-warning "forming offset \\\[9, 10] is out of the bounds \\\[0, 9] of object " "memcpy" } */
T (char, 9, a + SAR (-2, 8), a, 3); /* { dg-warning "offset \\\[9, 11] is out of the bounds \\\[0, 9] of object " "memcpy" } */
ptrdiff_t i = SAR (DIFF_MIN + 1, DIFF_MAX - 4);
T (char, 1, d, d + SAR (DIFF_MIN + 3, DIFF_MAX - 1), 3);
......@@ -312,13 +312,13 @@ void test_strcpy_bounds (char *d, const char *s)
it out of bounds (it isn't) but because the final source offset
after the access has completed, is. It would be clearer if
the warning mentioned the final offset. */
TI (char, 2, "", a + SR (2, DIFF_MAX - 1), s); /* { dg-warning "forming offset 3 is out of the bounds \\\[0, 2] of object \[^\n\r\]+ with type .char ?\\\[2\\\]." "strcpy" } */
TI (char, 2, "", a + SR (2, DIFF_MAX - 1), s); /* { dg-warning "offset 2 is out of the bounds \\\[0, 2] of object \[^\n\r\]+ with type .char ?\\\[2\\\]." "strcpy" } */
TI (char, 2, "", a + SR (3, DIFF_MAX - 1), s); /* { dg-warning "offset \\\[3, \[0-9\]+] is out of the bounds \\\[0, 2] of object \[^\n\r\]+ with type .char ?\\\[2\\\]." "strcpy" } */
TI (char, 3, "", a + SR (0, DIFF_MAX - 1), s);
TI (char, 3, "", a + SR (1, DIFF_MAX - 1), s);
TI (char, 3, "", a + SR (2, DIFF_MAX - 1), s);
TI (char, 3, "", a + SR (3, DIFF_MAX - 1), s); /* { dg-warning "forming offset 4 is out of the bounds \\\[0, 3] of object \[^\n\r\]+ with type .char ?\\\[3\\\]." "strcpy" } */
TI (char, 3, "", a + SR (3, DIFF_MAX - 1), s); /* { dg-warning "offset 3 is out of the bounds \\\[0, 3] of object \[^\n\r\]+ with type .char ?\\\[3\\\]." "strcpy" } */
TI (char, 3, "", a + SR (4, DIFF_MAX - 1), s); /* { dg-warning "offset \\\[4, \[0-9\]+] is out of the bounds \\\[0, 3] of object \[^\n\r\]+ with type .char ?\\\[3\\\]." "strcpy" } */
TI (char, 4, "", a + SR (DIFF_MAX - 2, DIFF_MAX - 1), s); /* { dg-warning "offset \\\[\[0-9\]+, \[0-9\]+] is out of the bounds \\\[0, 4] of object \[^\n\r\]+ with type .char ?\\\[4\\\]." "strcpy" } */
......@@ -364,15 +364,15 @@ void test_strcpy_bounds_memarray_range (void)
TM (a5, "0", ma.a5 + i, ma.a5);
TM (a5, "01", ma.a5 + i, ma.a5);
TM (a5, "012", ma.a5 + i, ma.a5);
TM (a5, "0123", ma.a5 + i, ma.a5); /* { dg-warning "offset 10 from the object at .ma. is out of the bounds of referenced subobject .\(MA::\)?a5. with type .char ?\\\[5]. at offset 4" "strcpy" } */
TM (a5, "0123", ma.a5 + i, ma.a5); /* { dg-warning "offset 9 from the object at .ma. is out of the bounds of referenced subobject .\(MA::\)?a5. with type .char ?\\\[5]. at offset 4" "strcpy" } */
TM (a11, "0", ma.a5, ma.a11);
TM (a11, "01", ma.a5, ma.a11);
TM (a11, "012", ma.a5, ma.a11);
TM (a11, "0123", ma.a5, ma.a11);
TM (a11, "01234", ma.a5, ma.a11); /* { dg-warning "offset 10 from the object at .ma. is out of the bounds of referenced subobject .\(MA::\)?a5. with type .char ?\\\[5]' at offset 4" } */
TM (a11, "012345", ma.a5, ma.a11); /* { dg-warning "offset \\\[10, 11] from the object at .ma. is out of the bounds of referenced subobject .\(MA::\)?a5. with type .char ?\\\[5]' at offset 4" } */
TM (a11, "0123456", ma.a5, ma.a11); /* { dg-warning "offset \\\[10, 12] from the object at .ma. is out of the bounds of referenced subobject .\(MA::\)?a5. with type .char ?\\\[5]' at offset 4" } */
TM (a11, "01234", ma.a5, ma.a11); /* { dg-warning "offset 9 from the object at .ma. is out of the bounds of referenced subobject .\(MA::\)?a5. with type .char ?\\\[5]' at offset 4" } */
TM (a11, "012345", ma.a5, ma.a11); /* { dg-warning "offset \\\[9, 10] from the object at .ma. is out of the bounds of referenced subobject .\(MA::\)?a5. with type .char ?\\\[5]' at offset 4" } */
TM (a11, "0123456", ma.a5, ma.a11); /* { dg-warning "offset \\\[9, 11] from the object at .ma. is out of the bounds of referenced subobject .\(MA::\)?a5. with type .char ?\\\[5]' at offset 4" } */
TM (a11, "0123456", ma.a11 + i, "789abcd");
}
......
gcc/testsuite/c-c++-common/Warray-bounds-4.c
......@@ -44,8 +44,8 @@ void test_memcpy_bounds_memarray_range (void)
TM (ma.a5, ma.a5 + j, ma.a5, 1);
TM (ma.a5, ma.a5 + j, ma.a5, 3);
TM (ma.a5, ma.a5 + j, ma.a5, 5);
TM (ma.a5, ma.a5 + j, ma.a5, 7); /* { dg-warning "offset \\\[6, 8] from the object at .ma. is out of the bounds of referenced subobject .\(MA::\)?a5. with type .char ?\\\[5]. at offset 0" } */
TM (ma.a5, ma.a5 + j, ma.a5, 9); /* { dg-warning "offset \\\[6, 10] from the object at .ma. is out of the bounds of referenced subobject .\(MA::\)?a5. with type .char ?\\\[5]. at offset 0" } */
TM (ma.a5, ma.a5 + j, ma.a5, 7); /* { dg-warning "offset \\\[5, 7] from the object at .ma. is out of the bounds of referenced subobject .\(MA::\)?a5. with type .char ?\\\[5]. at offset 0" } */
TM (ma.a5, ma.a5 + j, ma.a5, 9); /* { dg-warning "offset \\\[5, 9] from the object at .ma. is out of the bounds of referenced subobject .\(MA::\)?a5. with type .char ?\\\[5]. at offset 0" } */
}
void test_strcpy_bounds_memarray_range (void)
......@@ -67,7 +67,7 @@ void test_strcpy_bounds_memarray_range (void)
#if __i386__ || __x86_64__
/* Disabled for non-x86 targets due to bug 83462. */
TM ("", "012345", ma.a7 + i, ma.a7); /* { dg-warning "offset 13 from the object at .ma. is out of the bounds of referenced subobject .\(MA::\)?a7. with type .char ?\\\[7]. at offset 5" "strcpy" { xfail { ! { i?86-*-* x86_64-*-* } } } } */
TM ("", "012345", ma.a7 + i, ma.a7); /* { dg-warning "offset 12 from the object at .ma. is out of the bounds of referenced subobject .\(MA::\)?a7. with type .char ?\\\[7]. at offset 5" "strcpy" { xfail { ! { i?86-*-* x86_64-*-* } } } } */
#endif
}
gcc/testsuite/gcc.dg/Warray-bounds-39.c
......@@ -123,7 +123,7 @@ char* test_strcpy_s0 (char *d)
char* test_strcpy_s0_0 (char *d)
{
return strcpy (d, s0_0[0]); /* { dg-warning "\\\[-Warray-bounds|-Wstringop-overflow" "pr88991" { xfail *-*-* } } */
return strcpy (d, s0_0[0]); /* { dg-warning "\\\[-Warray-bounds|-Wstringop-overflow" } */
}
......@@ -139,10 +139,10 @@ char* test_strncpy_s0_2 (char *d)
char* test_strncpy_s0_0_1 (char *d)
{
return strncpy (d, s0_0[0], 1); /* { dg-warning "\\\[-Warray-bounds|-Wstringop-overflow" "pr88991" { xfail *-*-* } } */
return strncpy (d, s0_0[0], 1); /* { dg-warning "\\\[-Warray-bounds|-Wstringop-overflow" } */
}
char* test_strncpy_s0_0_2 (char *d)
{
return strncpy (d, s0_0[0], 2); /* { dg-warning "\\\[-Warray-bounds|-Wstringop-overflow" "pr88991" { xfail *-*-* } } */
return strncpy (d, s0_0[0], 2); /* { dg-warning "\\\[-Warray-bounds|-Wstringop-overflow" } */
}
gcc/tree-ssa-strlen.c
......@@ -3057,11 +3057,12 @@ handle_builtin_strcat (enum built_in_function bcode, gimple_stmt_iterator *gsi)
/* Compute the size of the source sequence, including the nul. */
tree srcsize = srclen ? srclen : size_zero_node;
srcsize = fold_build2 (PLUS_EXPR, type, srcsize, build_int_cst (type, 1));
tree one = build_int_cst (type, 1);
srcsize = fold_build2 (PLUS_EXPR, type, srcsize, one);
tree dstsize = fold_build2 (PLUS_EXPR, type, dstlen, one);
tree sptr = si && si->ptr ? si->ptr : src;
if (check_bounds_or_overlap (stmt, dst, sptr, dstlen, srcsize))
if (check_bounds_or_overlap (stmt, dst, sptr, dstsize, srcsize))
{
gimple_set_no_warning (stmt, true);
set_no_warning = true;
......
gcc/tree.c
......@@ -67,6 +67,7 @@ along with GCC; see the file COPYING3. If not see
#include "rtl.h"
#include "regs.h"
#include "tree-vector-builder.h"
#include "gimple-fold.h"
/* Tree code classes. */
......@@ -13850,6 +13851,75 @@ component_ref_field_offset (tree exp)
return SUBSTITUTE_PLACEHOLDER_IN_EXPR (DECL_FIELD_OFFSET (field), exp);
}
/* Determines the size of the member referenced by the COMPONENT_REF
REF, using its initializer expression if necessary in order to
determine the size of an initialized flexible array member.
Returns the size (which might be zero for an object with
an uninitialized flexible array member) or null if the size
cannot be determined. */
tree
component_ref_size (tree ref)
{
gcc_assert (TREE_CODE (ref) == COMPONENT_REF);
tree member = TREE_OPERAND (ref, 1);
/* If the member is not an array, or is not last, or is an array with
more than one element, return its size. Otherwise it's either
a bona fide flexible array member, or a zero-length array member,
or an array of length one treated as such. */
tree size = DECL_SIZE_UNIT (member);
if (size)
{
tree memtype = TREE_TYPE (member);
if (TREE_CODE (memtype) != ARRAY_TYPE
|| !array_at_struct_end_p (ref))
return size;
if (!integer_zerop (size))
if (tree dom = TYPE_DOMAIN (memtype))
if (tree min = TYPE_MIN_VALUE (dom))
if (tree max = TYPE_MAX_VALUE (dom))
if (TREE_CODE (min) == INTEGER_CST
&& TREE_CODE (max) == INTEGER_CST)
{
offset_int minidx = wi::to_offset (min);
offset_int maxidx = wi::to_offset (max);
if (maxidx - minidx > 1)
return size;
}
}
/* If the reference is to a declared object and the member a true
flexible array, try to determine its size from its initializer. */
poly_int64 off = 0;
tree base = get_addr_base_and_unit_offset (ref, &off);
if (!base || !VAR_P (base))
return NULL_TREE;
/* The size of any member of a declared object other than a flexible
array member is that obtained above. */
if (size)
return size;
if (tree init = DECL_INITIAL (base))
if (TREE_CODE (init) == CONSTRUCTOR)
{
off <<= LOG2_BITS_PER_UNIT;
init = fold_ctor_reference (NULL_TREE, init, off, 0, base);
if (init)
return TYPE_SIZE_UNIT (TREE_TYPE (init));
}
/* Return "don't know" for an external non-array object since its
flexible array member can be initialized to have any number of
elements. Otherwise, return zero because the flexible array
member has no elements. */
return (DECL_EXTERNAL (base) && TREE_CODE (TREE_TYPE (base)) != ARRAY_TYPE
? NULL_TREE : integer_zero_node);
}
/* Return the machine mode of T. For vectors, returns the mode of the
inner type. The main use case is to feed the result to HONOR_NANS,
avoiding the BLKmode that a direct TYPE_MODE (T) might return. */
......
gcc/tree.h
......@@ -5263,6 +5263,13 @@ extern bool array_at_struct_end_p (tree);
by EXP. This does not include any offset in DECL_FIELD_BIT_OFFSET. */
extern tree component_ref_field_offset (tree);
/* Return the size of the member referenced by the COMPONENT_REF, using
its initializer expression if necessary in order to determine the size
of an initialized flexible array member. The size might be zero for
an object with an uninitialized flexible array member or null if it
cannot be determined. */
extern tree component_ref_size (tree);
extern int tree_map_base_eq (const void *, const void *);
extern unsigned int tree_map_base_hash (const void *);
extern int tree_map_base_marked_p (const void *);
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment