diff mbox

correct aligned_alloc argument order (PR 80020)

Message ID e08fee72-2089-ea49-5108-93e5833bd9ca@gmail.com
State New
Headers show

Commit Message

Martin Sebor March 13, 2017, 1:13 a.m. UTC 
r243470 decorates standard allocation functions like alloca
and malloc with attribute alloc_size.  However, in applying
the attribute to aligned_alloc I had overlooked that the size
argument is the second one and not the first.  That oversight
has led to __builtin_object_size() reporting the wrong size
for aligned_alloc-allocated objects and, consequently, to
checking functions like __memset_chk calling abort for buffer
bogus overflows.

The attached patch corrects this mistake by decorating the
function with the correct alloc_size attribute.

Martin

Comments

Richard Biener March 13, 2017, 8:44 a.m. UTC | #1 
On Mon, Mar 13, 2017 at 2:13 AM, Martin Sebor <msebor@gmail.com> wrote:
> r243470 decorates standard allocation functions like alloca
> and malloc with attribute alloc_size.  However, in applying
> the attribute to aligned_alloc I had overlooked that the size
> argument is the second one and not the first.  That oversight
> has led to __builtin_object_size() reporting the wrong size
> for aligned_alloc-allocated objects and, consequently, to
> checking functions like __memset_chk calling abort for buffer
> bogus overflows.
>
> The attached patch corrects this mistake by decorating the
> function with the correct alloc_size attribute.

Ok.

Richard.

> Martin
Jeff Law March 14, 2017, 10:16 p.m. UTC | #2 
On 03/13/2017 02:44 AM, Richard Biener wrote:
> On Mon, Mar 13, 2017 at 2:13 AM, Martin Sebor <msebor@gmail.com> wrote:
>> r243470 decorates standard allocation functions like alloca
>> and malloc with attribute alloc_size.  However, in applying
>> the attribute to aligned_alloc I had overlooked that the size
>> argument is the second one and not the first.  That oversight
>> has led to __builtin_object_size() reporting the wrong size
>> for aligned_alloc-allocated objects and, consequently, to
>> checking functions like __memset_chk calling abort for buffer
>> bogus overflows.
>>
>> The attached patch corrects this mistake by decorating the
>> function with the correct alloc_size attribute.
>
> Ok.
I installed the patch on the trunk
jeff
diff mbox

Patch

PR middle-end/80020 - gcc confused about aligned_alloc argument order

gcc/testsuite/ChangeLog:

	PR middle-end/80020
	* gcc.dg/attr-alloc_size-6.c: Correct aligned_alloc argument order.
	* gcc.dg/attr-alloc_size-7.c: Same.
	* gcc.dg/attr-alloc_size-9.c: Same.
	* gcc.dg/builtin-alloc-size.c: Same.
	* gcc.dg/pr80020.c: New test.

gcc/ChangeLog:

	PR middle-end/80020
	* builtin-attrs.def (ATTR_ALLOC_SIZE_2_NOTHROW_LIST): New macro.
	* builtins.def (aligned_alloc): Use it.

diff --git a/gcc/builtin-attrs.def b/gcc/builtin-attrs.def
index 2753288..2396f00 100644
--- a/gcc/builtin-attrs.def
+++ b/gcc/builtin-attrs.def
@@ -156,9 +156,12 @@  DEF_ATTR_TREE_LIST (ATTR_COLD_CONST_NORETURN_NOTHROW_LEAF_LIST, ATTR_CONST,\
 			ATTR_NULL, ATTR_COLD_NORETURN_NOTHROW_LEAF_LIST)
 
 /* Allocation functions like malloc and realloc whose first argument
-   specifies the size of the allocated object.  */
+   with _SIZE_1, or second argument with _SIZE_2, specifies the size
+   of the allocated object.  */
 DEF_ATTR_TREE_LIST (ATTR_MALLOC_SIZE_1_NOTHROW_LIST, ATTR_ALLOC_SIZE,	\
 			ATTR_LIST_1, ATTR_MALLOC_NOTHROW_LIST)
+DEF_ATTR_TREE_LIST (ATTR_ALLOC_SIZE_2_NOTHROW_LIST, ATTR_ALLOC_SIZE,	\
+			ATTR_LIST_2, ATTR_MALLOC_NOTHROW_LIST)
 DEF_ATTR_TREE_LIST (ATTR_MALLOC_SIZE_1_NOTHROW_LEAF_LIST, ATTR_ALLOC_SIZE, \
 		        ATTR_LIST_1, ATTR_MALLOC_NOTHROW_LEAF_LIST)
 /* Alloca is just like malloc except that it never returns null.  */
diff --git a/gcc/builtins.def b/gcc/builtins.def
index d7f80e6..197c8ac 100644
--- a/gcc/builtins.def
+++ b/gcc/builtins.def
@@ -297,7 +297,7 @@  DEF_C99_BUILTIN        (BUILT_IN_ACOSH, "acosh", BT_FN_DOUBLE_DOUBLE, ATTR_MATHF
 DEF_C99_BUILTIN        (BUILT_IN_ACOSHF, "acoshf", BT_FN_FLOAT_FLOAT, ATTR_MATHFN_FPROUNDING_ERRNO)
 DEF_C99_BUILTIN        (BUILT_IN_ACOSHL, "acoshl", BT_FN_LONGDOUBLE_LONGDOUBLE, ATTR_MATHFN_FPROUNDING_ERRNO)
 DEF_C99_C90RES_BUILTIN (BUILT_IN_ACOSL, "acosl", BT_FN_LONGDOUBLE_LONGDOUBLE, ATTR_MATHFN_FPROUNDING_ERRNO)
-DEF_C11_BUILTIN        (BUILT_IN_ALIGNED_ALLOC, "aligned_alloc", BT_FN_PTR_SIZE_SIZE, ATTR_MALLOC_SIZE_1_NOTHROW_LIST)
+DEF_C11_BUILTIN        (BUILT_IN_ALIGNED_ALLOC, "aligned_alloc", BT_FN_PTR_SIZE_SIZE, ATTR_ALLOC_SIZE_2_NOTHROW_LIST)
 DEF_LIB_BUILTIN        (BUILT_IN_ASIN, "asin", BT_FN_DOUBLE_DOUBLE, ATTR_MATHFN_FPROUNDING_ERRNO)
 DEF_C99_C90RES_BUILTIN (BUILT_IN_ASINF, "asinf", BT_FN_FLOAT_FLOAT, ATTR_MATHFN_FPROUNDING_ERRNO)
 DEF_C99_BUILTIN        (BUILT_IN_ASINH, "asinh", BT_FN_DOUBLE_DOUBLE, ATTR_MATHFN_FPROUNDING)
diff --git a/gcc/testsuite/gcc.dg/attr-alloc_size-6.c b/gcc/testsuite/gcc.dg/attr-alloc_size-6.c
index 38890b6..fc3e22c 100644
--- a/gcc/testsuite/gcc.dg/attr-alloc_size-6.c
+++ b/gcc/testsuite/gcc.dg/attr-alloc_size-6.c
@@ -15,8 +15,8 @@  void sink (void*);
 
 void test_lit (char *p, char *q)
 {
-  sink (__builtin_aligned_alloc (MAXOBJSZ, 1));
-  sink (__builtin_aligned_alloc (MAXOBJSZ + 1, 1));   /* { dg-warning "argument 1 value .12346\[lu\]*. exceeds maximum object size 12345" } */
+  sink (__builtin_aligned_alloc (1, MAXOBJSZ));
+  sink (__builtin_aligned_alloc (1, MAXOBJSZ + 1));   /* { dg-warning "argument 2 value .12346\[lu\]*. exceeds maximum object size 12345" } */
 
   sink (__builtin_alloca (MAXOBJSZ));
   sink (__builtin_alloca (MAXOBJSZ + 2));   /* { dg-warning "argument 1 value .12347\[lu\]*. exceeds maximum object size 12345" } */
@@ -46,8 +46,8 @@  enum { max = MAXOBJSZ };
 
 void test_cst (char *p, char *q)
 {
-  sink (__builtin_aligned_alloc (max, 1));
-  sink (__builtin_aligned_alloc (max + 1, 1));   /* { dg-warning "argument 1 value .12346\[lu\]*. exceeds maximum object size 12345" } */
+  sink (__builtin_aligned_alloc (1, max));
+  sink (__builtin_aligned_alloc (1, max + 1));   /* { dg-warning "argument 2 value .12346\[lu\]*. exceeds maximum object size 12345" } */
 
   sink (__builtin_alloca (max));
   sink (__builtin_alloca (max + 2));   /* { dg-warning "argument 1 value .12347\[lu\]*. exceeds maximum object size 12345" } */
diff --git a/gcc/testsuite/gcc.dg/attr-alloc_size-7.c b/gcc/testsuite/gcc.dg/attr-alloc_size-7.c
index d6e618d..a3b2a6b 100644
--- a/gcc/testsuite/gcc.dg/attr-alloc_size-7.c
+++ b/gcc/testsuite/gcc.dg/attr-alloc_size-7.c
@@ -22,8 +22,8 @@  void test_var (void *p)
 {
   size_t max = maxobjsize ();
 
-  sink (__builtin_aligned_alloc (max, 1));
-  sink (__builtin_aligned_alloc (max + 1, 1));   /* { dg-warning "argument 1 value .12346\[lu\]*. exceeds maximum object size 12345" } */
+  sink (__builtin_aligned_alloc (1, max));
+  sink (__builtin_aligned_alloc (1, max + 1));   /* { dg-warning "argument 2 value .12346\[lu\]*. exceeds maximum object size 12345" } */
 
   sink (__builtin_alloca (max));
   sink (__builtin_alloca (max + 2));   /* { dg-warning "argument 1 value .12347\[lu\]*. exceeds maximum object size 12345" } */
@@ -52,8 +52,8 @@  void test_range (void *p, size_t range)
   if (range < max || 2 * max <= range)
     range = maxobjsize ();
 
-  sink (__builtin_aligned_alloc (range, 1));
-  sink (__builtin_aligned_alloc (range + 1, 1));   /* { dg-warning "argument 1 range \\\[12346\[lu\]*, \[0-9\]+\[lu\]*\\\] exceeds maximum object size 12345" } */
+  sink (__builtin_aligned_alloc (1, range));
+  sink (__builtin_aligned_alloc (1, range + 1));   /* { dg-warning "argument 2 range \\\[12346\[lu\]*, \[0-9\]+\[lu\]*\\\] exceeds maximum object size 12345" } */
 
   sink (__builtin_alloca (range));
   sink (__builtin_alloca (range + 2));   /* { dg-warning "argument 1 range \\\[12347\[lu\]*, \[0-9\]+\[lu\]*\\\] exceeds maximum object size 12345" } */
diff --git a/gcc/testsuite/gcc.dg/attr-alloc_size-9.c b/gcc/testsuite/gcc.dg/attr-alloc_size-9.c
index 66765fd..0e75bb2 100644
--- a/gcc/testsuite/gcc.dg/attr-alloc_size-9.c
+++ b/gcc/testsuite/gcc.dg/attr-alloc_size-9.c
@@ -20,7 +20,7 @@  extern int x;
 
 void test (void *p, unsigned n)
 {
-  TEST (__builtin_aligned_alloc (n, 8));
+  TEST (__builtin_aligned_alloc (8, n));
   TEST (__builtin_alloca (n));
   TEST (__builtin_calloc (4, n));
   TEST (__builtin_malloc (n));
diff --git a/gcc/testsuite/gcc.dg/builtin-alloc-size.c b/gcc/testsuite/gcc.dg/builtin-alloc-size.c
index 5a40862..6929e88 100644
--- a/gcc/testsuite/gcc.dg/builtin-alloc-size.c
+++ b/gcc/testsuite/gcc.dg/builtin-alloc-size.c
@@ -16,7 +16,7 @@  void test_aligned_alloc (unsigned a)
 {
   unsigned n = size (7);
 
-  void *p = __builtin_aligned_alloc (n, a);
+  void *p = __builtin_aligned_alloc (a, n);
   if (__builtin_object_size (p, 0) != n)
     __builtin_abort ();
   sink (p);
diff --git a/gcc/testsuite/gcc.dg/pr80020.c b/gcc/testsuite/gcc.dg/pr80020.c
new file mode 100644
index 0000000..5e79c37
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr80020.c
@@ -0,0 +1,26 @@ 
+/* PR middle-end/80020 - gcc confused about aligned_alloc argument order
+   { dg-compile }
+   { dg-options "-O2 -Wall -fdump-tree-optimized" } */
+
+void sink (void*);
+
+void foo (void)
+{
+  enum {
+    Align = 32,
+    Size = 123
+  };
+
+  void *p = __builtin_aligned_alloc (Align, Size);
+  unsigned n = __builtin_object_size (p, 0);
+
+  if (n != Size)
+    __builtin_abort ();
+
+  __builtin___memset_chk (p, 0, Size, n);
+
+  sink (p);
+}
+
+/* { dg-final { scan-tree-dump-not "abort" "optimized" } }
+   { dg-final { scan-tree-dump-not "memset_chk" "optimized" } } */