Add limit for maximal alignment options (PR c/84310).

Message ID 0a23778f-57c2-5f01-4919-231ec7d001e0@suse.cz
State New
Headers show
Series
  • Add limit for maximal alignment options (PR c/84310).
Related show

Commit Message

Martin Liška Feb. 12, 2018, 12:09 p.m.
Hi.

Following patch fixes 2 issues with -falign-*:
1) when using -malign-x=16 (or corresponding -falign-* value) then ICE appeared
as code in final.c can deal just with limited alignment.
2) thus I also documented and limited the maximum value of -falign-* options.

Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.
i386.exp test-suite works fine on x86_64 machine.

Ready to be installed?
Martin

gcc/ChangeLog:

2018-02-09  Martin Liska  <mliska@suse.cz>

	PR c/84310
	PR target/79747
	* final.c (shorten_branches): Build align_tab array with one
	more element.
	* opts.c (finish_options): Add alignment option limit check.
	(MAX_CODE_ALIGN): Likewise.
	(MAX_CODE_ALIGN_VALUE): Likewise.
	* doc/invoke.texi: Document maximum allowed option value for
	all -falign-* options.

gcc/testsuite/ChangeLog:

2018-02-12  Martin Liska  <mliska@suse.cz>

	PR c/84310
	PR target/79747
	* gcc.target/i386/pr84310.c: New test.
	* gcc.target/i386/pr84310-2.c: Likewise.
---
 gcc/doc/invoke.texi                       |  4 ++++
 gcc/final.c                               |  4 ++--
 gcc/opts.c                                | 20 ++++++++++++++++++++
 gcc/testsuite/gcc.target/i386/pr84310-2.c | 10 ++++++++++
 gcc/testsuite/gcc.target/i386/pr84310.c   |  8 ++++++++
 5 files changed, 44 insertions(+), 2 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/i386/pr84310-2.c
 create mode 100644 gcc/testsuite/gcc.target/i386/pr84310.c

Comments

Martin Liška Feb. 20, 2018, 8:25 a.m. | #1
PING^1

On 02/12/2018 01:09 PM, Martin Liška wrote:
> Hi.
> 
> Following patch fixes 2 issues with -falign-*:
> 1) when using -malign-x=16 (or corresponding -falign-* value) then ICE appeared
> as code in final.c can deal just with limited alignment.
> 2) thus I also documented and limited the maximum value of -falign-* options.
> 
> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.
> i386.exp test-suite works fine on x86_64 machine.
> 
> Ready to be installed?
> Martin
> 
> gcc/ChangeLog:
> 
> 2018-02-09  Martin Liska  <mliska@suse.cz>
> 
> 	PR c/84310
> 	PR target/79747
> 	* final.c (shorten_branches): Build align_tab array with one
> 	more element.
> 	* opts.c (finish_options): Add alignment option limit check.
> 	(MAX_CODE_ALIGN): Likewise.
> 	(MAX_CODE_ALIGN_VALUE): Likewise.
> 	* doc/invoke.texi: Document maximum allowed option value for
> 	all -falign-* options.
> 
> gcc/testsuite/ChangeLog:
> 
> 2018-02-12  Martin Liska  <mliska@suse.cz>
> 
> 	PR c/84310
> 	PR target/79747
> 	* gcc.target/i386/pr84310.c: New test.
> 	* gcc.target/i386/pr84310-2.c: Likewise.
> ---
>  gcc/doc/invoke.texi                       |  4 ++++
>  gcc/final.c                               |  4 ++--
>  gcc/opts.c                                | 20 ++++++++++++++++++++
>  gcc/testsuite/gcc.target/i386/pr84310-2.c | 10 ++++++++++
>  gcc/testsuite/gcc.target/i386/pr84310.c   |  8 ++++++++
>  5 files changed, 44 insertions(+), 2 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.target/i386/pr84310-2.c
>  create mode 100644 gcc/testsuite/gcc.target/i386/pr84310.c
> 
>
Jakub Jelinek Feb. 20, 2018, 8:43 a.m. | #2
On Mon, Feb 12, 2018 at 01:09:43PM +0100, Martin Liška wrote:
> Following patch fixes 2 issues with -falign-*:
> 1) when using -malign-x=16 (or corresponding -falign-* value) then ICE appeared
> as code in final.c can deal just with limited alignment.
> 2) thus I also documented and limited the maximum value of -falign-* options.
> 
> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.
> i386.exp test-suite works fine on x86_64 machine.
> 
> Ready to be installed?
> Martin
> 
> gcc/ChangeLog:
> 
> 2018-02-09  Martin Liska  <mliska@suse.cz>
> 
> 	PR c/84310
> 	PR target/79747
> 	* final.c (shorten_branches): Build align_tab array with one
> 	more element.
> 	* opts.c (finish_options): Add alignment option limit check.
> 	(MAX_CODE_ALIGN): Likewise.
> 	(MAX_CODE_ALIGN_VALUE): Likewise.
> 	* doc/invoke.texi: Document maximum allowed option value for
> 	all -falign-* options.
> 
> gcc/testsuite/ChangeLog:
> 
> 2018-02-12  Martin Liska  <mliska@suse.cz>
> 
> 	PR c/84310
> 	PR target/79747
> 	* gcc.target/i386/pr84310.c: New test.
> 	* gcc.target/i386/pr84310-2.c: Likewise.

Ok, thanks.

	Jakub
Martin Liška Feb. 20, 2018, 10:05 a.m. | #3
On 02/20/2018 09:43 AM, Jakub Jelinek wrote:
> On Mon, Feb 12, 2018 at 01:09:43PM +0100, Martin Liška wrote:
>> Following patch fixes 2 issues with -falign-*:
>> 1) when using -malign-x=16 (or corresponding -falign-* value) then ICE appeared
>> as code in final.c can deal just with limited alignment.
>> 2) thus I also documented and limited the maximum value of -falign-* options.
>>
>> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.
>> i386.exp test-suite works fine on x86_64 machine.
>>
>> Ready to be installed?
>> Martin
>>
>> gcc/ChangeLog:
>>
>> 2018-02-09  Martin Liska  <mliska@suse.cz>
>>
>> 	PR c/84310
>> 	PR target/79747
>> 	* final.c (shorten_branches): Build align_tab array with one
>> 	more element.
>> 	* opts.c (finish_options): Add alignment option limit check.
>> 	(MAX_CODE_ALIGN): Likewise.
>> 	(MAX_CODE_ALIGN_VALUE): Likewise.
>> 	* doc/invoke.texi: Document maximum allowed option value for
>> 	all -falign-* options.
>>
>> gcc/testsuite/ChangeLog:
>>
>> 2018-02-12  Martin Liska  <mliska@suse.cz>
>>
>> 	PR c/84310
>> 	PR target/79747
>> 	* gcc.target/i386/pr84310.c: New test.
>> 	* gcc.target/i386/pr84310-2.c: Likewise.
> 
> Ok, thanks.
> 
> 	Jakub
> 

Thanks Jakub!
Would it be possible to backport that to active branches?

Martin
Jakub Jelinek Feb. 20, 2018, 10:06 a.m. | #4
On Tue, Feb 20, 2018 at 11:05:50AM +0100, Martin Liška wrote:
> Thanks Jakub!
> Would it be possible to backport that to active branches?

Yes, but give it some time on the trunk first.

	Jakub

Patch

diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index df357bea7dc..edfa9d5ada1 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -9096,6 +9096,7 @@  Some assemblers only support this flag when @var{n} is a power of two;
 in that case, it is rounded up.
 
 If @var{n} is not specified or is zero, use a machine-dependent default.
+The maximum allowed @var{n} option value is 65536.
 
 Enabled at levels @option{-O2}, @option{-O3}.
 
@@ -9121,6 +9122,7 @@  are greater than this value, then their values are used instead.
 
 If @var{n} is not specified or is zero, use a machine-dependent default
 which is very likely to be @samp{1}, meaning no alignment.
+The maximum allowed @var{n} option value is 65536.
 
 Enabled at levels @option{-O2}, @option{-O3}.
 
@@ -9134,6 +9136,7 @@  operations.
 
 @option{-fno-align-loops} and @option{-falign-loops=1} are
 equivalent and mean that loops are not aligned.
+The maximum allowed @var{n} option value is 65536.
 
 If @var{n} is not specified or is zero, use a machine-dependent default.
 
@@ -9151,6 +9154,7 @@  need be executed.
 equivalent and mean that loops are not aligned.
 
 If @var{n} is not specified or is zero, use a machine-dependent default.
+The maximum allowed @var{n} option value is 65536.
 
 Enabled at levels @option{-O2}, @option{-O3}.
 
diff --git a/gcc/final.c b/gcc/final.c
index 99a7cadd7c9..933c613cabf 100644
--- a/gcc/final.c
+++ b/gcc/final.c
@@ -911,7 +911,7 @@  shorten_branches (rtx_insn *first)
   char *varying_length;
   rtx body;
   int uid;
-  rtx align_tab[MAX_CODE_ALIGN];
+  rtx align_tab[MAX_CODE_ALIGN + 1];
 
   /* Compute maximum UID and allocate label_align / uid_shuid.  */
   max_uid = get_max_uid ();
@@ -1016,7 +1016,7 @@  shorten_branches (rtx_insn *first)
      alignment of n.  */
   uid_align = XCNEWVEC (rtx, max_uid);
 
-  for (i = MAX_CODE_ALIGN; --i >= 0;)
+  for (i = MAX_CODE_ALIGN + 1; --i >= 0;)
     align_tab[i] = NULL_RTX;
   seq = get_last_insn ();
   for (; seq; seq = PREV_INSN (seq))
diff --git a/gcc/opts.c b/gcc/opts.c
index f2795f98bf4..33efcc0d6e7 100644
--- a/gcc/opts.c
+++ b/gcc/opts.c
@@ -1039,6 +1039,26 @@  finish_options (struct gcc_options *opts, struct gcc_options *opts_set,
   if ((opts->x_flag_sanitize & SANITIZE_KERNEL_ADDRESS) && opts->x_flag_tm)
     sorry ("transactional memory is not supported with "
 	   "%<-fsanitize=kernel-address%>");
+
+  /* Comes from final.c -- no real reason to change it.  */
+#define MAX_CODE_ALIGN 16
+#define MAX_CODE_ALIGN_VALUE (1 << MAX_CODE_ALIGN)
+
+  if (opts->x_align_loops > MAX_CODE_ALIGN_VALUE)
+    error_at (loc, "-falign-loops=%d is not between 0 and %d",
+	      opts->x_align_loops, MAX_CODE_ALIGN_VALUE);
+
+  if (opts->x_align_jumps > MAX_CODE_ALIGN_VALUE)
+    error_at (loc, "-falign-jumps=%d is not between 0 and %d",
+	      opts->x_align_jumps, MAX_CODE_ALIGN_VALUE);
+
+  if (opts->x_align_functions > MAX_CODE_ALIGN_VALUE)
+    error_at (loc, "-falign-functions=%d is not between 0 and %d",
+	      opts->x_align_functions, MAX_CODE_ALIGN_VALUE);
+
+  if (opts->x_align_labels > MAX_CODE_ALIGN_VALUE)
+    error_at (loc, "-falign-labels=%d is not between 0 and %d",
+	      opts->x_align_labels, MAX_CODE_ALIGN_VALUE);
 }
 
 #define LEFT_COLUMN	27
diff --git a/gcc/testsuite/gcc.target/i386/pr84310-2.c b/gcc/testsuite/gcc.target/i386/pr84310-2.c
new file mode 100644
index 00000000000..e39a421e8d2
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr84310-2.c
@@ -0,0 +1,10 @@ 
+/* { dg-do compile } */
+/* { dg-options "-O2 -malign-loops=16" } */
+/* { dg-warning "is obsolete" "" { target *-*-* } 0 } */
+
+void
+c (void)
+{  
+  for (;;)
+    ;
+}
diff --git a/gcc/testsuite/gcc.target/i386/pr84310.c b/gcc/testsuite/gcc.target/i386/pr84310.c
new file mode 100644
index 00000000000..f82327e45f3
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr84310.c
@@ -0,0 +1,8 @@ 
+/* { dg-do compile } */
+/* { dg-options "-O2 -falign-functions=100000" } */
+/* { dg-error "is not between 0 and 65536" "" { target *-*-* } 0 } */
+
+void
+test_func (void)
+{
+}