This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] 69780 - [4.9/5/6 Regression] ICE on __builtin_alloca_with_align, with small alignment
- From: Martin Sebor <msebor at gmail dot com>
- To: Jakub Jelinek <jakub at redhat dot com>
- Cc: Gcc Patch List <gcc-patches at gcc dot gnu dot org>
- Date: Tue, 16 Feb 2016 18:04:48 -0700
- Subject: Re: [PATCH] 69780 - [4.9/5/6 Regression] ICE on __builtin_alloca_with_align, with small alignment
- Authentication-results: sourceware.org; auth=none
- References: <56C134ED dot 9060106 at gmail dot com> <20160215082933 dot GW3017 at tucnak dot redhat dot com>
On 02/15/2016 01:29 AM, Jakub Jelinek wrote:
On Sun, Feb 14, 2016 at 07:16:13PM -0700, Martin Sebor wrote:
+ case BUILT_IN_ALLOCA_WITH_ALIGN:
+ {
+ /* Get the requested alignment (in bits) if it's a constant
+ integer expression. */
+ HOST_WIDE_INT align =
+ TREE_CODE (args [1]) == INTEGER_CST ? tree_to_uhwi (args [1]) : 0;
Thank you for your comments. Attached is an updated patch that
corrects most of the problems you pointed out. Also included
in this patch is the update to the manual documenting both this
intrinsic and __builtin_alloca. I originally submitted that
part for review separately but since they are closely related
I think they are best reviewed and ultimately committed together.
(In the spirit of full disclosure, the review of the documentation
change got side-tracked by a discussion of an unrelated topic and
would need to be restarted anyway.)
Formatting. = needs to be on the next line.
There are literally dozens of examples of this style in this file
alone. In one of the two instances of this style in this patch,
moving the equals sign to the next line would force me to split
the initializer expression over the next two lines to avoid
exceeding the 80 character per line limit and make the code
harder to read. I also don't see the style you suggest mentioned
in the GNU coding standard or in the GCC coding conventions.
I would prefer to leave this detail to the discretion of the
author.
Martin
PR middle-end/69780 - [4.9/5/6 Regression] ICE on __builtin_alloca_with_align
with small alignment
PR c/69759 - __builtin_alloca and __builtin_alloca_with_align undocumented
gcc/c-family/ChangeLog:
2016-02-16 Martin Sebor <msebor@redhat.com>
PR middle-end/69780
* c-common.c (check_builtin_function_arguments): Validate and reject
invalid arguments to __builtin_alloca_with_align.
gcc/ChangeLog:
2016-02-16 Martin Sebor <msebor@redhat.com>
PR c/69759
* doc/extend.texi (Other Builtins): Document __builtin_alloca and
__builtin_alloca_with_align.
gcc/testsuite/ChangeLog:
2016-02-16 Martin Sebor <msebor@redhat.com>
PR middle-end/69780
* g++.dg/ext/builtin_alloca.C: New test.
* gcc.dg/builtins-68.c: New test.
Index: gcc/c-family/c-common.c
===================================================================
--- gcc/c-family/c-common.c (revision 233476)
+++ gcc/c-family/c-common.c (working copy)
@@ -9816,8 +9816,41 @@ check_builtin_function_arguments (tree f
|| DECL_BUILT_IN_CLASS (fndecl) != BUILT_IN_NORMAL)
return true;
+#undef MAX_STACK_ALIGNMENT
+#define MAX_STACK_ALIGNMENT __UINT32_MAX__
+
switch (DECL_FUNCTION_CODE (fndecl))
{
+ case BUILT_IN_ALLOCA_WITH_ALIGN:
+ {
+ /* Get the requested alignment (in bits) if it's a constant
+ integer expression. */
+ unsigned HOST_WIDE_INT align =
+ TREE_CODE (args[1]) == INTEGER_CST ? tree_to_uhwi (args[1]) : 0;
+
+ /* Determine if the requested alignment is a power of 2 greater
+ than CHAR_BIT. */
+ if ((align & (align - 1)) == 0)
+ align >>= LOG2_BITS_PER_UNIT;
+ else
+ align = 0;
+
+ /* Reject invalid alignments. */
+ if (align < 1 || MAX_STACK_ALIGNMENT < align)
+ {
+ /* Compute the maximum stack alignment in bits. */
+ unsigned HOST_WIDE_INT max_align_bits =
+ (unsigned HOST_WIDE_INT)MAX_STACK_ALIGNMENT << LOG2_BITS_PER_UNIT;
+
+ error_at (EXPR_LOC_OR_LOC (args[1], input_location),
+ "second argument to function %qE must be a constant "
+ "integer power of 2 between %qi and %qwu",
+ fndecl, BITS_PER_UNIT, max_align_bits);
+ return false;
+ }
+ return true;
+ }
+
case BUILT_IN_CONSTANT_P:
return builtin_function_validate_nargs (fndecl, nargs, 1);
Index: gcc/doc/extend.texi
===================================================================
--- gcc/doc/extend.texi (revision 233476)
+++ gcc/doc/extend.texi (working copy)
@@ -10144,6 +10144,8 @@ in the Cilk Plus language manual which c
@node Other Builtins
@section Other Built-in Functions Provided by GCC
@cindex built-in functions
+@findex __builtin_alloca
+@findex __builtin_alloca_with_align
@findex __builtin_call_with_static_chain
@findex __builtin_fpclassify
@findex __builtin_isfinite
@@ -10690,6 +10692,89 @@ In the same fashion, GCC provides @code{
@code{__builtin_} prefixed. The @code{isinf} and @code{isnan}
built-in functions appear both with and without the @code{__builtin_} prefix.
+@deftypefn {Built-in Function} void* __builtin_alloca (size_t size)
+The @code{__builtin_alloca} function must be called at block scope.
+The function allocates an object @var{size} bytes large on the stack
+of the calling function. The object is aligned at the default stack
+alignment boundary for the target determined by the
+@code{__BIGGEST_ALIGNMENT__} macro. @code{__builtin_alloca} returns
+a pointer to the first byte of the allocated object. The lifetime of
+the allocated object ends just before the calling function returns to
+its caller. This is so even when @code{__builtin_alloca_with_align}
+is called within a nested block.
+
+For example, the following function allocates eight objects of @code{n}
+bytes each on the stack, storing a pointer to each in consecutive elements
+of the array @code{a}. It then passes the array to function @code{g()}
+which can safely use the storage pointed to by each of the array elements.
+
+@smallexample
+void f (unsigned n)
+@{
+ void *a [8];
+ for (int i = 0; i != 8; ++i)
+ a [i] = __builtin_alloca (n);
+
+ g (a, n); // safe
+@}
+@end smallexample
+
+Since the @code{__builtin_alloca} function doesn't validate its arguments
+it is the responsibility of its caller to make sure the argument doesn't
+cause it doesn't exceed the stack size limit.
+The @code{__builtin_alloca} function is provided to make it possible to
+allocate arrays with a runtime bound on the stack. Since C99 variable
+length arrays offer similar functionality under a portable, more convenient,
+and safer interface they are recommended instead, in both C99 and C++
+programs where GCC provides them as an extension.
+
+@end deftypefn
+
+@deftypefn {Built-in Function} void* __builtin_alloca_with_align (size_t size, size_t align)
+The @code{__builtin_alloca_with_align} function must be called at block
+scope. The function allocates an object @var{align} bytes large on
+the stack of the calling function. The allocated object is aligned on
+the boundary specified by the argument @var{align} whose unit is given
+in bits (not bytes). @var{size} must be positive and not exceed the stack
+size limit. @var{align} must be a constant integer expression that
+evaluates to a power of 2 greater than or equal to @code{__CHAR_BIT__}.
+Invocations with other values are rejected with an error. The function
+returns a pointer to the first byte of the allocated object. The lifetime
+of the allocated object ends at the end of the block in which the function
+was called. The allocated storage is released no later than just before
+the calling function returns to its called, but may be released at the end
+of the block in which the function was called.
+
+For example, in the following function the call to @code{g()} is unsafe
+because when @code{overalign} is non-zero, the space allocated by
+@code{__builtin_alloca_with_align} may have been released at the end
+of the @code{if} statement in which it was called.
+
+@smallexample
+void f (unsigned n, bool overalign)
+@{
+ void *p;
+ if (overalign)
+ p = __builtin_alloca_with_align (n, 64);
+ else
+ p = __builtin_alloc (n);
+
+ g (p, n); // unsafe
+@}
+@end smallexample
+
+Since the @code{__builtin_alloca_with_align} function doesn't validate its
+arguments it is the responsibility of its caller to make sure the argument
+doesn't cause it to exceed the stack size limit.
+The @code{__builtin_alloca_with_align} function is provided to make
+it possible to allocate overaligned arrays with a runtime bound on
+the stack. Since C99 variable length arrays offer the same functionality
+under a portable, more convenient, and safer interface they are recommended
+instead, in both C99 and C++ programs where GCC provides them as
+an extension.
+
+@end deftypefn
+
@deftypefn {Built-in Function} int __builtin_types_compatible_p (@var{type1}, @var{type2})
You can use the built-in function @code{__builtin_types_compatible_p} to
Index: gcc/testsuite/gcc.dg/builtins-68.c
===================================================================
--- gcc/testsuite/gcc.dg/builtins-68.c (revision 0)
+++ gcc/testsuite/gcc.dg/builtins-68.c (working copy)
@@ -0,0 +1,101 @@
+/* PR middle-end/69780 - [4.9/5/6 Regression] ICE on
+ __builtin_alloca_with_align with small alignment */
+/* { dg-require-effective-target alloca } */
+/* { dg-do compile } */
+
+#define CHAR_BIT __CHAR_BIT__
+#define INT_MAX __INT_MAX__
+#define INT_MIN (-INT_MAX - 1)
+
+static void* p;
+
+/* Verify that valid __builtin_alloca_with_align expressions are accepted. */
+void test_valid (int n)
+{
+ enum {
+ A1 = CHAR_BIT * 1,
+ A2 = CHAR_BIT * 2,
+ A4 = CHAR_BIT * 4,
+ A8 = CHAR_BIT * 8,
+ A16 = CHAR_BIT * 16,
+ A32 = CHAR_BIT * 32
+ };
+
+ /* Valid alignments are power of 2 positive multiples of CHAR_BIT. */
+ p = __builtin_alloca_with_align (n, CHAR_BIT * 1);
+ p = __builtin_alloca_with_align (n, CHAR_BIT * 2);
+ p = __builtin_alloca_with_align (n, CHAR_BIT * 4);
+ p = __builtin_alloca_with_align (n, CHAR_BIT * 8);
+ p = __builtin_alloca_with_align (n, CHAR_BIT * 16);
+ p = __builtin_alloca_with_align (n, CHAR_BIT * 32);
+
+ p = __builtin_alloca_with_align (n, A1);
+ p = __builtin_alloca_with_align (n, A2);
+ p = __builtin_alloca_with_align (n, A4);
+ p = __builtin_alloca_with_align (n, A8);
+ p = __builtin_alloca_with_align (n, A16);
+ p = __builtin_alloca_with_align (n, A32);
+}
+
+/* Non-integer alignments must be rejected. */
+void test_arg2_non_int (int n)
+{
+ /* Verify the full text of the diagnostic just once. */
+ p = __builtin_alloca_with_align (n, 0.0); /* { dg-error "second argument to function .__builtin_alloca_with_align. must be a constant integer power of 2 between .8. and " } */
+
+ /* Disable diagnostic complaining about converting void* to int that
+ preempts the "constant integer expression" error. */
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wint-conversion"
+
+ p = __builtin_alloca_with_align (n, (void*)0); /* { dg-error "must be a constant integer" } */
+ p = __builtin_alloca_with_align (n, ""); /* { dg-error "must be a constant integer" } */
+ p = __builtin_alloca_with_align (n, L""); /* { dg-error "must be a constant integer" } */
+
+#pragma GCC diagnostic pop
+
+}
+
+/* Integer alignment that's not a constant expression must be rejected. */
+void test_arg2_non_const (int n, int a1)
+{
+ extern const int a2;
+ static const int a3 = CHAR_BIT;
+ static volatile const int a4 = CHAR_BIT;
+
+ p = __builtin_alloca_with_align (n, a1); /* { dg-error "must be a constant integer" } */
+ p = __builtin_alloca_with_align (n, a2); /* { dg-error "must be a constant integer" } */
+ p = __builtin_alloca_with_align (n, a3); /* { dg-error "must be a constant integer" } */
+ p = __builtin_alloca_with_align (n, a4); /* { dg-error "must be a constant integer" } */
+}
+
+/* Constant integer alignment that's not a power of 2 positive multiple
+ of CHAR_BIT must be rejected. */
+void test_arg2_non_pow2 (int n)
+{
+ p = __builtin_alloca_with_align (n, INT_MIN); /* { dg-error "must be a constant integer" } */
+ p = __builtin_alloca_with_align (n, -1); /* { dg-error "must be a constant integer" } */
+ p = __builtin_alloca_with_align (n, !1); /* { dg-error "must be a constant integer" } */
+ p = __builtin_alloca_with_align (n, !0); /* { dg-error "must be a constant integer" } */
+ p = __builtin_alloca_with_align (n, 0); /* { dg-error "must be a constant integer" } */
+ p = __builtin_alloca_with_align (n, 1); /* { dg-error "must be a constant integer" } */
+ p = __builtin_alloca_with_align (n, 2); /* { dg-error "must be a constant integer" } */
+ p = __builtin_alloca_with_align (n, 3); /* { dg-error "must be a constant integer" } */
+ p = __builtin_alloca_with_align (n, 4); /* { dg-error "must be a constant integer" } */
+ p = __builtin_alloca_with_align (n, 5); /* { dg-error "must be a constant integer" } */
+ p = __builtin_alloca_with_align (n, 6); /* { dg-error "must be a constant integer" } */
+ p = __builtin_alloca_with_align (n, 7); /* { dg-error "must be a constant integer" } */
+ p = __builtin_alloca_with_align (n, 9); /* { dg-error "must be a constant integer" } */
+ p = __builtin_alloca_with_align (n, 10); /* { dg-error "must be a constant integer" } */
+ p = __builtin_alloca_with_align (n, 11); /* { dg-error "must be a constant integer" } */
+ p = __builtin_alloca_with_align (n, 12); /* { dg-error "must be a constant integer" } */
+ p = __builtin_alloca_with_align (n, 13); /* { dg-error "must be a constant integer" } */
+ p = __builtin_alloca_with_align (n, 14); /* { dg-error "must be a constant integer" } */
+ p = __builtin_alloca_with_align (n, 15); /* { dg-error "must be a constant integer" } */
+ p = __builtin_alloca_with_align (n, 17); /* { dg-error "must be a constant integer" } */
+ p = __builtin_alloca_with_align (n, 31); /* { dg-error "must be a constant integer" } */
+ p = __builtin_alloca_with_align (n, 33); /* { dg-error "must be a constant integer" } */
+ p = __builtin_alloca_with_align (n, 63); /* { dg-error "must be a constant integer" } */
+ p = __builtin_alloca_with_align (n, 65); /* { dg-error "must be a constant integer" } */
+ p = __builtin_alloca_with_align (n, INT_MAX); /* { dg-error "must be a constant integer" } */
+}
Index: gcc/testsuite/g++.dg/ext/builtin_alloca.C
===================================================================
--- gcc/testsuite/g++.dg/ext/builtin_alloca.C (revision 0)
+++ gcc/testsuite/g++.dg/ext/builtin_alloca.C (working copy)
@@ -0,0 +1,183 @@
+// PR middle-end/69780 - [4.9/5/6 Regression] ICE on
+// __builtin_alloca_with_align with small alignment
+// { dg-require-effective-target alloca }
+// { dg-do compile }
+
+#define CHAR_BIT __CHAR_BIT__
+#define INT_MAX __INT_MAX__
+#define INT_MIN (-INT_MAX - 1)
+
+static void* p;
+
+// Verify that valid __builtin_alloca_with_align expressions are accepted.
+void test_valid (int n)
+{
+ enum {
+ A1 = CHAR_BIT * 1,
+ A2 = CHAR_BIT * 2,
+ A4 = CHAR_BIT * 4,
+ A8 = CHAR_BIT * 8,
+ A16 = CHAR_BIT * 16,
+ A32 = CHAR_BIT * 32
+ };
+
+ const int a1 = A1;
+ const int a2 = A2;
+ const int a4 = A4;
+ const int a8 = A8;
+ const int a16 = A16;
+ const int a32 = A32;
+
+ // Valid alignments are power of 2 positive multiples of CHAR_BIT.
+ p = __builtin_alloca_with_align (n, CHAR_BIT * 1);
+ p = __builtin_alloca_with_align (n, CHAR_BIT * 2);
+ p = __builtin_alloca_with_align (n, CHAR_BIT * 4);
+ p = __builtin_alloca_with_align (n, CHAR_BIT * 8);
+ p = __builtin_alloca_with_align (n, CHAR_BIT * 16);
+ p = __builtin_alloca_with_align (n, CHAR_BIT * 32);
+
+ p = __builtin_alloca_with_align (n, A1);
+ p = __builtin_alloca_with_align (n, A2);
+ p = __builtin_alloca_with_align (n, A4);
+ p = __builtin_alloca_with_align (n, A8);
+ p = __builtin_alloca_with_align (n, A16);
+ p = __builtin_alloca_with_align (n, A32);
+
+ p = __builtin_alloca_with_align (n, a1);
+ p = __builtin_alloca_with_align (n, a2);
+ p = __builtin_alloca_with_align (n, a4);
+ p = __builtin_alloca_with_align (n, a8);
+ p = __builtin_alloca_with_align (n, a16);
+ p = __builtin_alloca_with_align (n, a32);
+}
+
+template <int A> struct X { enum { Align = A }; };
+
+template <int A>
+void test_valid_template (int n)
+{
+ // Valid alignments are power of 2 positive multiples of CHAR_BIT.
+ p = __builtin_alloca_with_align (n, A);
+}
+
+template void test_valid_template<CHAR_BIT>(int);
+template void test_valid_template<CHAR_BIT * 2>(int);
+template void test_valid_template<CHAR_BIT * 4>(int);
+template void test_valid_template<CHAR_BIT * 8>(int);
+template void test_valid_template<CHAR_BIT * 16>(int);
+template void test_valid_template<CHAR_BIT * 32>(int);
+
+// Exercise the alignment in a dependent context.
+template <int A>
+void test_valid_template_dep (int n)
+{
+ // Valid alignments are power of 2 positive multiples of CHAR_BIT.
+ p = __builtin_alloca_with_align (n, X<A>::Align);
+}
+
+template void test_valid_template_dep<CHAR_BIT>(int);
+template void test_valid_template_dep<CHAR_BIT * 2>(int);
+template void test_valid_template_dep<CHAR_BIT * 4>(int);
+template void test_valid_template_dep<CHAR_BIT * 8>(int);
+template void test_valid_template_dep<CHAR_BIT * 16>(int);
+template void test_valid_template_dep<CHAR_BIT * 32>(int);
+
+// Invalid size must be rejected (and not cause an ICE).
+void test_arg1_non_int (int n)
+{
+ extern void f ();
+
+ p = __builtin_alloca_with_align ((void*)0, 32); // { dg-error "invalid conversion" }
+
+ p = __builtin_alloca_with_align ("", 32); // { dg-error "invalid conversion" }
+ p = __builtin_alloca_with_align (L"", 32); // { dg-error "invalid conversion" }
+ p = __builtin_alloca_with_align (f, 32); // { dg-error "invalid conversion" }
+}
+
+// Non-integer alignment must be rejected.
+void test_arg2_non_int (int n)
+{
+ // Verify the full text of the diagnostic just once.
+ p = __builtin_alloca_with_align (n, 0.0); // { dg-error "second argument to function .__builtin_alloca_with_align. must be a constant integer power of 2 between .8. and " }
+
+ p = __builtin_alloca_with_align (n, (void*)0); // { dg-error "invalid conversion|must be a constant integer" }
+ p = __builtin_alloca_with_align (n, ""); // { dg-error "invalid conversion|must be a constant integer" }
+ p = __builtin_alloca_with_align (n, L""); // { dg-error "invalid conversion|must be a constant integer" }
+}
+
+// Integer alignment that's not a constant expression must be rejected.
+void test_arg2_non_const (int n, int a1)
+{
+ extern const int a2;
+ static volatile const int a3 = CHAR_BIT;
+
+ p = __builtin_alloca_with_align (n, a1); // { dg-error "must be a constant integer" }
+ p = __builtin_alloca_with_align (n, a2); // { dg-error "must be a constant integer" }
+ p = __builtin_alloca_with_align (n, a3); // { dg-error "must be a constant integer" }
+}
+
+// Constant integer alignment that's not a power of 2 positive multiple
+// of CHAR_BIT must be rejected.
+void test_arg2_non_pow2 (int n)
+{
+ p = __builtin_alloca_with_align (n, INT_MIN); // { dg-error "must be a constant integer" }
+ p = __builtin_alloca_with_align (n, -1); // { dg-error "must be a constant integer" }
+ p = __builtin_alloca_with_align (n, !1); // { dg-error "must be a constant integer" }
+ p = __builtin_alloca_with_align (n, !0); // { dg-error "must be a constant integer" }
+ p = __builtin_alloca_with_align (n, 0); // { dg-error "must be a constant integer" }
+ p = __builtin_alloca_with_align (n, 1); // { dg-error "must be a constant integer" }
+ p = __builtin_alloca_with_align (n, 2); // { dg-error "must be a constant integer" }
+ p = __builtin_alloca_with_align (n, 3); // { dg-error "must be a constant integer" }
+ p = __builtin_alloca_with_align (n, 4); // { dg-error "must be a constant integer" }
+ p = __builtin_alloca_with_align (n, 5); // { dg-error "must be a constant integer" }
+ p = __builtin_alloca_with_align (n, 6); // { dg-error "must be a constant integer" }
+ p = __builtin_alloca_with_align (n, 7); // { dg-error "must be a constant integer" }
+ p = __builtin_alloca_with_align (n, 9); // { dg-error "must be a constant integer" }
+ p = __builtin_alloca_with_align (n, 10); // { dg-error "must be a constant integer" }
+ p = __builtin_alloca_with_align (n, 11); // { dg-error "must be a constant integer" }
+ p = __builtin_alloca_with_align (n, 12); // { dg-error "must be a constant integer" }
+ p = __builtin_alloca_with_align (n, 13); // { dg-error "must be a constant integer" }
+ p = __builtin_alloca_with_align (n, 14); // { dg-error "must be a constant integer" }
+ p = __builtin_alloca_with_align (n, 15); // { dg-error "must be a constant integer" }
+ p = __builtin_alloca_with_align (n, 17); // { dg-error "must be a constant integer" }
+ p = __builtin_alloca_with_align (n, 31); // { dg-error "must be a constant integer" }
+ p = __builtin_alloca_with_align (n, 33); // { dg-error "must be a constant integer" }
+ p = __builtin_alloca_with_align (n, 63); // { dg-error "must be a constant integer" }
+ p = __builtin_alloca_with_align (n, 65); // { dg-error "must be a constant integer" }
+ p = __builtin_alloca_with_align (n, INT_MAX); // { dg-error "must be a constant integer" }
+}
+
+// Exercise invalid alignment specified by a template argument.
+template <int A>
+void test_invalid_template_1 (int n)
+{
+ // Valid alignments are power of 2 positive multiples of CHAR_BIT.
+ p = __builtin_alloca_with_align (n, A); // { dg-error "must be a constant integer" }
+}
+
+template void test_invalid_template_1<1>(int);
+
+template <int A>
+void test_invalid_template_7 (int n)
+{
+ p = __builtin_alloca_with_align (n, A); // { dg-error "must be a constant integer" }
+}
+
+template void test_invalid_template_7<7>(int);
+
+template <int A>
+void test_invalid_template_9 (int n)
+{
+ p = __builtin_alloca_with_align (n, A); // { dg-error "must be a constant integer" }
+}
+
+template void test_invalid_template_9<9>(int);
+
+// Exercise invalid alignment specified by a template dependent argument.
+template <int A>
+void test_invalid_template_dep_1 (int n)
+{
+ p = __builtin_alloca_with_align (n, X<A>::Align); // { dg-error "must be a constant integer" }
+}
+
+template void test_invalid_template_dep_1<1>(int);