public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [stage1][PATCH] Add gcc_assert that &global_options are not dirty modified.
@ 2020-03-19  8:56 Martin Liška
  2020-03-19  9:09 ` Jakub Jelinek
  0 siblings, 1 reply; 22+ messages in thread
From: Martin Liška @ 2020-03-19  8:56 UTC (permalink / raw)
  To: gcc-patches; +Cc: Jan Hubicka

[-- Attachment #1: Type: text/plain, Size: 1346 bytes --]

Hi.

As seen in the mentioned PR we do have issues related to modification
of global_options in context of #pragma GCC optimize/target and
the corresponding function attributes. The patch brings a sanity check
that these context related option modifications should not affect global
state. The patch lists known limitations (mentioned in the PR).

So far I bootstrapped and tested the patch on x86_64-linux-gnu and
ppc64le-linux-gnu.

I'm planning to work on the problematic options in next stage1 and this
patch will help me to catch another violations.

Ready to be installed in next stage1?
Thanks,
Martin

gcc/ChangeLog:

2020-03-17  Martin Liska  <mliska@suse.cz>

	PR tree-optimization/92860
	* opth-gen.awk: Generate new function gcc_options_check. Include
	known exceptions.

gcc/c-family/ChangeLog:

2020-03-17  Martin Liska  <mliska@suse.cz>

	PR tree-optimization/92860
	* c-attribs.c (handle_optimize_attribute):
	Save global options before parsing of an optimization attribute.
	* c-pragma.c (opt_stack): Add saved_global_options field.
	(handle_pragma_push_options): Save current global_options.
	(handle_pragma_pop_options): Check current options.
---
  gcc/c-family/c-attribs.c |  3 +++
  gcc/c-family/c-pragma.c  |  4 ++++
  gcc/opth-gen.awk         | 35 +++++++++++++++++++++++++++++++++++
  3 files changed, 42 insertions(+)



[-- Attachment #2: 0001-Add-gcc_assert-that-global_options-are-not-dirty-mod.patch --]
[-- Type: text/x-patch, Size: 3351 bytes --]

diff --git a/gcc/c-family/c-attribs.c b/gcc/c-family/c-attribs.c
index 9abf81d0248..c99b1256186 100644
--- a/gcc/c-family/c-attribs.c
+++ b/gcc/c-family/c-attribs.c
@@ -4448,6 +4448,7 @@ handle_optimize_attribute (tree *node, tree name, tree args,
 
       /* If we previously had some optimization options, use them as the
 	 default.  */
+      gcc_options saved_global_options = global_options;
       if (old_opts)
 	cl_optimization_restore (&global_options,
 				 TREE_OPTIMIZATION (old_opts));
@@ -4459,6 +4460,8 @@ handle_optimize_attribute (tree *node, tree name, tree args,
 
       /* Restore current options.  */
       cl_optimization_restore (&global_options, &cur_opts);
+      if (flag_checking)
+	gcc_assert (gcc_options_check (&saved_global_options, &global_options));
     }
 
   return NULL_TREE;
diff --git a/gcc/c-family/c-pragma.c b/gcc/c-family/c-pragma.c
index 7c35741745b..8b3b4f218ba 100644
--- a/gcc/c-family/c-pragma.c
+++ b/gcc/c-family/c-pragma.c
@@ -1003,6 +1003,7 @@ struct GTY(()) opt_stack {
   tree target_strings;
   tree optimize_binary;
   tree optimize_strings;
+  gcc_options saved_global_options;
 };
 
 static GTY(()) struct opt_stack * options_stack;
@@ -1028,6 +1029,7 @@ handle_pragma_push_options (cpp_reader *ARG_UNUSED(dummy))
   options_stack = p;
 
   /* Save optimization and target flags in binary format.  */
+  p->saved_global_options = global_options;
   p->optimize_binary = build_optimization_node (&global_options);
   p->target_binary = build_target_option_node (&global_options);
 
@@ -1079,6 +1081,8 @@ handle_pragma_pop_options (cpp_reader *ARG_UNUSED(dummy))
 				      p->optimize_binary);
       optimization_current_node = p->optimize_binary;
     }
+  if (flag_checking)
+    gcc_assert (gcc_options_check (&p->saved_global_options, &global_options));
 
   current_target_pragma = p->target_strings;
   current_optimize_pragma = p->optimize_strings;
diff --git a/gcc/opth-gen.awk b/gcc/opth-gen.awk
index 856a69168a5..586213da3d3 100644
--- a/gcc/opth-gen.awk
+++ b/gcc/opth-gen.awk
@@ -119,6 +119,41 @@ print "#endif"
 print "#endif"
 print ""
 
+print "#if !defined(IN_LIBGCC2) && !defined(IN_TARGET_LIBS) && !defined(IN_RTS)"
+print "#ifndef GENERATOR_FILE"
+print "static inline bool gcc_options_check (gcc_options *ptr1, gcc_options *ptr2)"
+print "{"
+print "  bool result = true;"
+
+# all these options are mentioned in PR92860
+checked_options["flag_merge_constants"]++
+checked_options["param_max_fields_for_field_sensitive"]++
+checked_options["flag_omit_frame_pointer"]++
+checked_options["unroll_only_small_loops"]++
+
+for (i = 0; i < n_opts; i++) {
+	name = var_name(flags[i]);
+	if (name == "")
+		continue;
+
+	if (name in checked_options)
+		continue;
+	checked_options[name]++
+
+	print "  if (ptr1->x_" name " != ptr2->x_" name ")"
+	print "  {"
+	print "    if (result)"
+	print "      fprintf (stderr, \"Error: global_options are modified in local context:\\n\");"
+	print "    fprintf (stderr, \"  " name " (%ld/%ld)\\n\", (long int)ptr1->x_" name ", (long int)ptr2->x_" name ");"
+	print "    result = false;"
+	print "  }"
+}
+
+print "  return result;"
+print "}"
+print "#endif"
+print "#endif"
+
 # All of the optimization switches gathered together so they can be saved and restored.
 # This will allow attribute((cold)) to turn on space optimization.
 


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [stage1][PATCH] Add gcc_assert that &global_options are not dirty modified.
  2020-03-19  8:56 [stage1][PATCH] Add gcc_assert that &global_options are not dirty modified Martin Liška
@ 2020-03-19  9:09 ` Jakub Jelinek
  2020-03-19 15:32   ` Martin Liška
  0 siblings, 1 reply; 22+ messages in thread
From: Jakub Jelinek @ 2020-03-19  9:09 UTC (permalink / raw)
  To: Martin Liška; +Cc: gcc-patches, Jan Hubicka

On Thu, Mar 19, 2020 at 09:56:00AM +0100, Martin Liška wrote:
> I'm planning to work on the problematic options in next stage1 and this
> patch will help me to catch another violations.
> 
> Ready to be installed in next stage1?

Isn't that costly even for the !flag_checking case?
struct gcc_options is over 5KB now.
So even just that
  gcc_options saved_global_options = global_options;
is a fair amount of work.
Can't you instead:
  gcc_options saved_global_options;
  if (flag_checking)
    saved_global_options = global_options;
?
Or for the opt_stack case have a pointer to the options and XNEW/XDELETE
it instead of having it directly in opt_stack?
I mean, optimize for the !flag_checking case...

	Jakub


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [stage1][PATCH] Add gcc_assert that &global_options are not dirty modified.
  2020-03-19  9:09 ` Jakub Jelinek
@ 2020-03-19 15:32   ` Martin Liška
  2020-03-19 15:47     ` Jakub Jelinek
  2020-03-19 16:13     ` Martin Sebor
  0 siblings, 2 replies; 22+ messages in thread
From: Martin Liška @ 2020-03-19 15:32 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: gcc-patches, Jan Hubicka

[-- Attachment #1: Type: text/plain, Size: 304 bytes --]

On 3/19/20 10:09 AM, Jakub Jelinek wrote:
> I mean, optimize for the !flag_checking case...

Sure, I transformed both situations into heap memory allocation.
In gcc/c-family/c-attribs.c I faced maybe uninitialized warning when
I only assigned (and checked) the variable in flag_checking context.

Martin

[-- Attachment #2: 0001-Add-gcc_assert-that-global_options-are-not-dirty-mod.patch --]
[-- Type: text/x-patch, Size: 4716 bytes --]

From a336c110cbefda2a1febddc56e0fd8289bb08c94 Mon Sep 17 00:00:00 2001
From: Martin Liska <mliska@suse.cz>
Date: Tue, 10 Dec 2019 19:41:08 +0100
Subject: [PATCH] Add gcc_assert that &global_options are not dirty modified.

gcc/ChangeLog:

2020-03-17  Martin Liska  <mliska@suse.cz>

	PR tree-optimization/92860
	* opth-gen.awk: Generate new function gcc_options_check. Include
	known exceptions.

gcc/c-family/ChangeLog:

2020-03-17  Martin Liska  <mliska@suse.cz>

	PR tree-optimization/92860
	* c-attribs.c (handle_optimize_attribute):
	Save global options before parsing of an optimization attribute.
	* c-pragma.c (opt_stack): Add saved_global_options field.
	(handle_pragma_push_options): Save current global_options.
	(handle_pragma_pop_options): Check current options.
---
 gcc/c-family/c-attribs.c | 12 ++++++++++++
 gcc/c-family/c-pragma.c  | 11 +++++++++++
 gcc/opth-gen.awk         | 35 +++++++++++++++++++++++++++++++++++
 3 files changed, 58 insertions(+)

diff --git a/gcc/c-family/c-attribs.c b/gcc/c-family/c-attribs.c
index 9abf81d0248..4ac44128407 100644
--- a/gcc/c-family/c-attribs.c
+++ b/gcc/c-family/c-attribs.c
@@ -4448,6 +4448,13 @@ handle_optimize_attribute (tree *node, tree name, tree args,
 
       /* If we previously had some optimization options, use them as the
 	 default.  */
+      gcc_options *saved_global_options = NULL;
+      if (flag_checking)
+	{
+	  saved_global_options = (gcc_options *) xmalloc (sizeof (gcc_options));
+	  *saved_global_options = global_options;
+	}
+
       if (old_opts)
 	cl_optimization_restore (&global_options,
 				 TREE_OPTIMIZATION (old_opts));
@@ -4459,6 +4466,11 @@ handle_optimize_attribute (tree *node, tree name, tree args,
 
       /* Restore current options.  */
       cl_optimization_restore (&global_options, &cur_opts);
+      if (saved_global_options != NULL)
+	{
+	  gcc_assert (gcc_options_check (saved_global_options, &global_options));
+	  free (saved_global_options);
+	}
     }
 
   return NULL_TREE;
diff --git a/gcc/c-family/c-pragma.c b/gcc/c-family/c-pragma.c
index 7c35741745b..94a1c486fc1 100644
--- a/gcc/c-family/c-pragma.c
+++ b/gcc/c-family/c-pragma.c
@@ -1003,6 +1003,7 @@ struct GTY(()) opt_stack {
   tree target_strings;
   tree optimize_binary;
   tree optimize_strings;
+  gcc_options *saved_global_options;
 };
 
 static GTY(()) struct opt_stack * options_stack;
@@ -1028,6 +1029,11 @@ handle_pragma_push_options (cpp_reader *ARG_UNUSED(dummy))
   options_stack = p;
 
   /* Save optimization and target flags in binary format.  */
+  if (flag_checking)
+    {
+      p->saved_global_options = (gcc_options *) xmalloc (sizeof (gcc_options));
+      *p->saved_global_options = global_options;
+    }
   p->optimize_binary = build_optimization_node (&global_options);
   p->target_binary = build_target_option_node (&global_options);
 
@@ -1079,6 +1085,11 @@ handle_pragma_pop_options (cpp_reader *ARG_UNUSED(dummy))
 				      p->optimize_binary);
       optimization_current_node = p->optimize_binary;
     }
+  if (flag_checking)
+    {
+      gcc_assert (gcc_options_check (p->saved_global_options, &global_options));
+      free (p->saved_global_options);
+    }
 
   current_target_pragma = p->target_strings;
   current_optimize_pragma = p->optimize_strings;
diff --git a/gcc/opth-gen.awk b/gcc/opth-gen.awk
index 856a69168a5..586213da3d3 100644
--- a/gcc/opth-gen.awk
+++ b/gcc/opth-gen.awk
@@ -119,6 +119,41 @@ print "#endif"
 print "#endif"
 print ""
 
+print "#if !defined(IN_LIBGCC2) && !defined(IN_TARGET_LIBS) && !defined(IN_RTS)"
+print "#ifndef GENERATOR_FILE"
+print "static inline bool gcc_options_check (gcc_options *ptr1, gcc_options *ptr2)"
+print "{"
+print "  bool result = true;"
+
+# all these options are mentioned in PR92860
+checked_options["flag_merge_constants"]++
+checked_options["param_max_fields_for_field_sensitive"]++
+checked_options["flag_omit_frame_pointer"]++
+checked_options["unroll_only_small_loops"]++
+
+for (i = 0; i < n_opts; i++) {
+	name = var_name(flags[i]);
+	if (name == "")
+		continue;
+
+	if (name in checked_options)
+		continue;
+	checked_options[name]++
+
+	print "  if (ptr1->x_" name " != ptr2->x_" name ")"
+	print "  {"
+	print "    if (result)"
+	print "      fprintf (stderr, \"Error: global_options are modified in local context:\\n\");"
+	print "    fprintf (stderr, \"  " name " (%ld/%ld)\\n\", (long int)ptr1->x_" name ", (long int)ptr2->x_" name ");"
+	print "    result = false;"
+	print "  }"
+}
+
+print "  return result;"
+print "}"
+print "#endif"
+print "#endif"
+
 # All of the optimization switches gathered together so they can be saved and restored.
 # This will allow attribute((cold)) to turn on space optimization.
 
-- 
2.25.1


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [stage1][PATCH] Add gcc_assert that &global_options are not dirty modified.
  2020-03-19 15:32   ` Martin Liška
@ 2020-03-19 15:47     ` Jakub Jelinek
  2020-03-19 16:13     ` Martin Sebor
  1 sibling, 0 replies; 22+ messages in thread
From: Jakub Jelinek @ 2020-03-19 15:47 UTC (permalink / raw)
  To: Martin Liška; +Cc: gcc-patches, Jan Hubicka

On Thu, Mar 19, 2020 at 04:32:05PM +0100, Martin Liška wrote:
> +      gcc_options *saved_global_options = NULL;
> +      if (flag_checking)
> +	{
> +	  saved_global_options = (gcc_options *) xmalloc (sizeof (gcc_options));

XNEW (gcc_options) please.

> +      p->saved_global_options = (gcc_options *) xmalloc (sizeof (gcc_options));

Ditto.

> +      *p->saved_global_options = global_options;

	Jakub


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [stage1][PATCH] Add gcc_assert that &global_options are not dirty modified.
  2020-03-19 15:32   ` Martin Liška
  2020-03-19 15:47     ` Jakub Jelinek
@ 2020-03-19 16:13     ` Martin Sebor
  2020-03-20 15:40       ` Martin Liška
  1 sibling, 1 reply; 22+ messages in thread
From: Martin Sebor @ 2020-03-19 16:13 UTC (permalink / raw)
  To: Martin Liška, Jakub Jelinek; +Cc: gcc-patches, Jan Hubicka

On 3/19/20 9:32 AM, Martin Liška wrote:
> On 3/19/20 10:09 AM, Jakub Jelinek wrote:
>> I mean, optimize for the !flag_checking case...
> 
> Sure, I transformed both situations into heap memory allocation.
> In gcc/c-family/c-attribs.c I faced maybe uninitialized warning when
> I only assigned (and checked) the variable in flag_checking context.

I was mostly just curious about what was being checked and how so
I might be misunderstanding something but it looks to me like
the code generated by the loop below will be a very long series
(of hundreds?) of if statements, each doing the same thing but
each hardcoding different options names.  If that's correct, is
there an easy way to turn that repetitive series into a loop to
keep code (and string) size growth to a minimum?

Also, since the function prints output and the caller then aborts
on failure, would calling internal_error for the first mismatch
instead be more in keeping with how internal errors with additional
output are reported?

One last question/suggestion: if the efficiency of the checking is
at all a concern, would calling memcmp on the arrays first and only
looping to find the position of the mismatch, be viable? (I have no
idea if changes to some of the options are acceptable; if they are
this wouldn't work).

Martin

PS Since the function doesn't modify the option arrays it could
declare them const.

diff --git a/gcc/opth-gen.awk b/gcc/opth-gen.awk
index 856a69168a5..586213da3d3 100644
--- a/gcc/opth-gen.awk
+++ b/gcc/opth-gen.awk
@@ -119,6 +119,41 @@ print "#endif"
  print "#endif"
  print ""

+print "#if !defined(IN_LIBGCC2) && !defined(IN_TARGET_LIBS) && 
!defined(IN_RTS)"
+print "#ifndef GENERATOR_FILE"
+print "static inline bool gcc_options_check (gcc_options *ptr1, 
gcc_options *ptr2)"
+print "{"
+print "  bool result = true;"
+
+# all these options are mentioned in PR92860
+checked_options["flag_merge_constants"]++
+checked_options["param_max_fields_for_field_sensitive"]++
+checked_options["flag_omit_frame_pointer"]++
+checked_options["unroll_only_small_loops"]++
+
+for (i = 0; i < n_opts; i++) {
+	name = var_name(flags[i]);
+	if (name == "")
+		continue;
+
+	if (name in checked_options)
+		continue;
+	checked_options[name]++
+
+	print "  if (ptr1->x_" name " != ptr2->x_" name ")"
+	print "  {"
+	print "    if (result)"
+	print "      fprintf (stderr, \"Error: global_options are modified in 
local context:\\n\");"
+	print "    fprintf (stderr, \"  " name " (%ld/%ld)\\n\", (long 
int)ptr1->x_" name ", (long int)ptr2->x_" name ");"
+	print "    result = false;"
+	print "  }"
+}
+
+print "  return result;"
+print "}"
+print "#endif"
+print "#endif"
+
  # All of the optimization switches gathered together so they can be 
saved and restored.
  # This will allow attribute((cold)) to turn on space optimization.


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [stage1][PATCH] Add gcc_assert that &global_options are not dirty modified.
  2020-03-19 16:13     ` Martin Sebor
@ 2020-03-20 15:40       ` Martin Liška
  2020-03-20 15:43         ` Jakub Jelinek
  0 siblings, 1 reply; 22+ messages in thread
From: Martin Liška @ 2020-03-20 15:40 UTC (permalink / raw)
  To: Martin Sebor, Jakub Jelinek; +Cc: gcc-patches, Jan Hubicka

[-- Attachment #1: Type: text/plain, Size: 4817 bytes --]

On 3/19/20 5:13 PM, Martin Sebor wrote:
> On 3/19/20 9:32 AM, Martin Liška wrote:
>> On 3/19/20 10:09 AM, Jakub Jelinek wrote:
>>> I mean, optimize for the !flag_checking case...
>>>> Sure, I transformed both situations into heap memory allocation.
>> In gcc/c-family/c-attribs.c I faced maybe uninitialized warning when
>> I only assigned (and checked) the variable in flag_checking context.
> 

Hi Martin.

> I was mostly just curious about what was being checked and how so
> I might be misunderstanding something but it looks to me like
> the code generated by the loop below will be a very long series
> (of hundreds?) of if statements, each doing the same thing but
> each hardcoding different options names.  If that's correct, is
> there an easy way to turn that repetitive series into a loop to
> keep code (and string) size growth to a minimum?

Thank you very much for the feedback, it was useful. I realized that
the patch made a huge code bloat (mainly because of the string constants
and the fact that it didn't end quickly (with an internal_error).

The loop is here not possible because we compare struct fields.

> 
> Also, since the function prints output and the caller then aborts
> on failure, would calling internal_error for the first mismatch
> instead be more in keeping with how internal errors with additional
> output are reported?

I decided to call internal_error without string description of the error.
With that I have a reasonable code growth:

bloaty /tmp/cc1plus-after -- /tmp/cc1plus-before
      VM SIZE                     FILE SIZE
  --------------               --------------
   +0.1% +20.9Ki .text         +20.9Ki  +0.1%
   [ = ]       0 .debug_line   +2.38Ki  +0.0%
   [ = ]       0 .debug_info      +369  +0.0%
   [ = ]       0 .debug_str        +75  +0.0%
   +0.0%     +64 .rodata           +64  +0.0%
   [ = ]       0 .debug_ranges     +48  +0.0%
   +0.0%     +45 .dynstr           +45  +0.0%
   [ = ]       0 .strtab           +45  +0.0%
   +0.0%     +40 .eh_frame         +40  +0.0%
   +0.0%     +24 .dynsym           +24  +0.0%
   [ = ]       0 .symtab           +24  +0.0%
   +0.0%      +8 .eh_frame_hdr      +8  +0.0%
   +0.0%      +4 .gnu.hash          +4  +0.0%
   +0.0%      +4 .hash              +4  +0.0%
   +0.0%      +2 .gnu.version       +2  +0.0%
    +14%      +1 [LOAD [R]]         +1   +14%
   [ = ]       0 .debug_loc       -355  -0.0%
   [ = ]       0 [Unmapped]    -1.05Ki -36.5%
   +0.1% +21.0Ki TOTAL         +22.6Ki  +0.0%

So for debugging one will have to take a look at corresponding option-save.c line.
It does not seem to me a problem.

> 
> One last question/suggestion: if the efficiency of the checking is
> at all a concern, would calling memcmp on the arrays first and only
> looping to find the position of the mismatch, be viable? (I have no
> idea if changes to some of the options are acceptable; if they are
> this wouldn't work).

Well, I skip some fields in the struct and there can be string pointers, so I'm suggesting
not to use memcmp.

There's updated tested patch.
Martin

> 
> Martin
> 
> PS Since the function doesn't modify the option arrays it could
> declare them const.
> 
> diff --git a/gcc/opth-gen.awk b/gcc/opth-gen.awk
> index 856a69168a5..586213da3d3 100644
> --- a/gcc/opth-gen.awk
> +++ b/gcc/opth-gen.awk
> @@ -119,6 +119,41 @@ print "#endif"
>   print "#endif"
>   print ""
> 
> +print "#if !defined(IN_LIBGCC2) && !defined(IN_TARGET_LIBS) && !defined(IN_RTS)"
> +print "#ifndef GENERATOR_FILE"
> +print "static inline bool gcc_options_check (gcc_options *ptr1, gcc_options *ptr2)"
> +print "{"
> +print "  bool result = true;"
> +
> +# all these options are mentioned in PR92860
> +checked_options["flag_merge_constants"]++
> +checked_options["param_max_fields_for_field_sensitive"]++
> +checked_options["flag_omit_frame_pointer"]++
> +checked_options["unroll_only_small_loops"]++
> +
> +for (i = 0; i < n_opts; i++) {
> +    name = var_name(flags[i]);
> +    if (name == "")
> +        continue;
> +
> +    if (name in checked_options)
> +        continue;
> +    checked_options[name]++
> +
> +    print "  if (ptr1->x_" name " != ptr2->x_" name ")"
> +    print "  {"
> +    print "    if (result)"
> +    print "      fprintf (stderr, \"Error: global_options are modified in local context:\\n\");"
> +    print "    fprintf (stderr, \"  " name " (%ld/%ld)\\n\", (long int)ptr1->x_" name ", (long int)ptr2->x_" name ");"
> +    print "    result = false;"
> +    print "  }"
> +}
> +
> +print "  return result;"
> +print "}"
> +print "#endif"
> +print "#endif"
> +
>   # All of the optimization switches gathered together so they can be saved and restored.
>   # This will allow attribute((cold)) to turn on space optimization.
> 


[-- Attachment #2: 0001-Add-gcc_assert-that-global_options-are-not-dirty-mod.patch --]
[-- Type: text/x-patch, Size: 4962 bytes --]

From 94f01ce8a662293dc6bc44a0e3047c5fe54fc15d Mon Sep 17 00:00:00 2001
From: Martin Liska <mliska@suse.cz>
Date: Tue, 10 Dec 2019 19:41:08 +0100
Subject: [PATCH] Add gcc_assert that &global_options are not dirty modified.

gcc/ChangeLog:

2020-03-20  Martin Liska  <mliska@suse.cz>

	PR tree-optimization/92860
	* optc-save-gen.awk: Generate new function cl_optimization_compare.
	* opth-gen.awk: Generate declaration of the function.

gcc/c-family/ChangeLog:

2020-03-20  Martin Liska  <mliska@suse.cz>

	PR tree-optimization/92860
	* c-attribs.c (handle_optimize_attribute):
	Save global options and compare it after parsing of function
	attribute.
	* c-pragma.c (opt_stack::saved_global_options): New field.
	(handle_pragma_push_options): Save global_options.
	(handle_pragma_pop_options): Compare them after pop.
---
 gcc/c-family/c-attribs.c | 12 ++++++++++++
 gcc/c-family/c-pragma.c  | 11 +++++++++++
 gcc/optc-save-gen.awk    | 25 +++++++++++++++++++++++++
 gcc/opth-gen.awk         |  3 +++
 4 files changed, 51 insertions(+)

diff --git a/gcc/c-family/c-attribs.c b/gcc/c-family/c-attribs.c
index 9abf81d0248..095986dead4 100644
--- a/gcc/c-family/c-attribs.c
+++ b/gcc/c-family/c-attribs.c
@@ -4448,6 +4448,13 @@ handle_optimize_attribute (tree *node, tree name, tree args,
 
       /* If we previously had some optimization options, use them as the
 	 default.  */
+      gcc_options *saved_global_options = NULL;
+      if (flag_checking)
+	{
+	  saved_global_options = XNEW (gcc_options);
+	  *saved_global_options = global_options;
+	}
+
       if (old_opts)
 	cl_optimization_restore (&global_options,
 				 TREE_OPTIMIZATION (old_opts));
@@ -4459,6 +4466,11 @@ handle_optimize_attribute (tree *node, tree name, tree args,
 
       /* Restore current options.  */
       cl_optimization_restore (&global_options, &cur_opts);
+      if (saved_global_options != NULL)
+	{
+	  cl_optimization_compare (saved_global_options, &global_options);
+	  free (saved_global_options);
+	}
     }
 
   return NULL_TREE;
diff --git a/gcc/c-family/c-pragma.c b/gcc/c-family/c-pragma.c
index 7c35741745b..e3169e68fb6 100644
--- a/gcc/c-family/c-pragma.c
+++ b/gcc/c-family/c-pragma.c
@@ -1003,6 +1003,7 @@ struct GTY(()) opt_stack {
   tree target_strings;
   tree optimize_binary;
   tree optimize_strings;
+  gcc_options * GTY ((skip)) saved_global_options;
 };
 
 static GTY(()) struct opt_stack * options_stack;
@@ -1028,6 +1029,11 @@ handle_pragma_push_options (cpp_reader *ARG_UNUSED(dummy))
   options_stack = p;
 
   /* Save optimization and target flags in binary format.  */
+  if (flag_checking)
+    {
+      p->saved_global_options = XNEW (gcc_options);
+      *p->saved_global_options = global_options;
+    }
   p->optimize_binary = build_optimization_node (&global_options);
   p->target_binary = build_target_option_node (&global_options);
 
@@ -1079,6 +1085,11 @@ handle_pragma_pop_options (cpp_reader *ARG_UNUSED(dummy))
 				      p->optimize_binary);
       optimization_current_node = p->optimize_binary;
     }
+  if (flag_checking)
+    {
+      cl_optimization_compare (p->saved_global_options, &global_options);
+      free (p->saved_global_options);
+    }
 
   current_target_pragma = p->target_strings;
   current_optimize_pragma = p->optimize_strings;
diff --git a/gcc/optc-save-gen.awk b/gcc/optc-save-gen.awk
index 6033f519866..4a0e5ab64f3 100644
--- a/gcc/optc-save-gen.awk
+++ b/gcc/optc-save-gen.awk
@@ -945,5 +945,30 @@ for (i = 0; i < n_opt_val; i++) {
 	      print "    free (const_cast <char *>(ptr->" name"));";
 	}
 }
+print "}";
+
+print "void";
+print "cl_optimization_compare (gcc_options *ptr1, gcc_options *ptr2)"
+print "{"
+
+# all these options are mentioned in PR92860
+checked_options["flag_merge_constants"]++
+checked_options["param_max_fields_for_field_sensitive"]++
+checked_options["flag_omit_frame_pointer"]++
+checked_options["unroll_only_small_loops"]++
+
+for (i = 0; i < n_opts; i++) {
+	name = var_name(flags[i]);
+	if (name == "")
+		continue;
+
+	if (name in checked_options)
+		continue;
+	checked_options[name]++
+
+	print "  if (ptr1->x_" name " != ptr2->x_" name ")"
+	print "    internal_error (\"Error: global_options are modified in local context\\n\");";
+}
+
 print "}";
 }
diff --git a/gcc/opth-gen.awk b/gcc/opth-gen.awk
index 856a69168a5..472fd591413 100644
--- a/gcc/opth-gen.awk
+++ b/gcc/opth-gen.awk
@@ -318,6 +318,9 @@ print "";
 print "/* Free heap memory used by optimization options.  */";
 print "extern void cl_optimization_option_free (cl_optimization *ptr1);"
 print "";
+print "/* Compare and report difference for a part of cl_optimization options.  */";
+print "extern void cl_optimization_compare (gcc_options *ptr1, gcc_options *ptr2);";
+print "";
 print "/* Generator files may not have access to location_t, and don't need these.  */"
 print "#if defined(UNKNOWN_LOCATION)"
 print "bool                                                                  "
-- 
2.25.1


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [stage1][PATCH] Add gcc_assert that &global_options are not dirty modified.
  2020-03-20 15:40       ` Martin Liška
@ 2020-03-20 15:43         ` Jakub Jelinek
  2020-03-20 15:55           ` Martin Liška
  0 siblings, 1 reply; 22+ messages in thread
From: Jakub Jelinek @ 2020-03-20 15:43 UTC (permalink / raw)
  To: Martin Liška; +Cc: Martin Sebor, gcc-patches, Jan Hubicka

On Fri, Mar 20, 2020 at 04:40:17PM +0100, Martin Liška wrote:
> Thank you very much for the feedback, it was useful. I realized that
> the patch made a huge code bloat (mainly because of the string constants
> and the fact that it didn't end quickly (with an internal_error).
> 
> The loop is here not possible because we compare struct fields.

Are you sure?  I mean, you could have a loop over cl_options array which
contains the needed offsetof values and also the types.

	Jakub


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [stage1][PATCH] Add gcc_assert that &global_options are not dirty modified.
  2020-03-20 15:43         ` Jakub Jelinek
@ 2020-03-20 15:55           ` Martin Liška
  2020-05-21 12:04             ` Martin Liška
  0 siblings, 1 reply; 22+ messages in thread
From: Martin Liška @ 2020-03-20 15:55 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: Martin Sebor, gcc-patches, Jan Hubicka

On 3/20/20 4:43 PM, Jakub Jelinek wrote:
> On Fri, Mar 20, 2020 at 04:40:17PM +0100, Martin Liška wrote:
>> Thank you very much for the feedback, it was useful. I realized that
>> the patch made a huge code bloat (mainly because of the string constants
>> and the fact that it didn't end quickly (with an internal_error).
>>
>> The loop is here not possible because we compare struct fields.
> 
> Are you sure?  I mean, you could have a loop over cl_options array which
> contains the needed offsetof values and also the types.

Ok, it would be possible, but if you take a look at options-save.c there's no
function that will leverage that. It's a generated code so I guess we can
live with that?

Martin

> 
> 	Jakub
> 


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [stage1][PATCH] Add gcc_assert that &global_options are not dirty modified.
  2020-03-20 15:55           ` Martin Liška
@ 2020-05-21 12:04             ` Martin Liška
  2020-06-09 21:01               ` Jeff Law
  0 siblings, 1 reply; 22+ messages in thread
From: Martin Liška @ 2020-05-21 12:04 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: gcc-patches, Jan Hubicka

PING^1

On 3/20/20 4:55 PM, Martin Liška wrote:
> Ok, it would be possible, but if you take a look at options-save.c there's no
> function that will leverage that. It's a generated code so I guess we can
> live with that?


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [stage1][PATCH] Add gcc_assert that &global_options are not dirty modified.
  2020-05-21 12:04             ` Martin Liška
@ 2020-06-09 21:01               ` Jeff Law
  2020-06-18 11:32                 ` Luis Machado
  0 siblings, 1 reply; 22+ messages in thread
From: Jeff Law @ 2020-06-09 21:01 UTC (permalink / raw)
  To: Martin Liška, Jakub Jelinek; +Cc: gcc-patches, Jan Hubicka

On Thu, 2020-05-21 at 14:04 +0200, Martin Liška wrote:
> PING^1
> 
> On 3/20/20 4:55 PM, Martin Liška wrote:
> > Ok, it would be possible, but if you take a look at options-save.c there's no
> > function that will leverage that. It's a generated code so I guess we can
> > live with that?
Given the size increase is under control now, I think this is fine for the trunk.

jeff


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [stage1][PATCH] Add gcc_assert that &global_options are not dirty modified.
  2020-06-09 21:01               ` Jeff Law
@ 2020-06-18 11:32                 ` Luis Machado
  2020-06-18 12:21                   ` Martin Liška
  2020-06-18 15:40                   ` Martin Liška
  0 siblings, 2 replies; 22+ messages in thread
From: Luis Machado @ 2020-06-18 11:32 UTC (permalink / raw)
  To: law, Martin Liška, Jakub Jelinek; +Cc: gcc-patches, Jan Hubicka

FTR, I'm running into this ICE when attempting to build the Linux Kernel 
for arm64.

More specifically:

/repos/linux-arm/kernel/bpf/core.c:1368:1: internal compiler error: 
‘global_options’ are modified in local context
  1368 | {
       | ^
0xc0554b cl_optimization_compare(gcc_options*, gcc_options*)
         /build/gcc-master/gcc/options-save.c:9786
0x7812df handle_optimize_attribute
         ../../../repos/gcc/gcc/c-family/c-attribs.c:4475
0x666477 decl_attributes(tree_node**, tree_node*, int, tree_node*)
         ../../../repos/gcc/gcc/attribs.c:714
0x688c9b start_function(c_declspecs*, c_declarator*, tree_node*)
         ../../../repos/gcc/gcc/c/c-decl.c:9177
0x6f85f3 c_parser_declaration_or_fndef
         ../../../repos/gcc/gcc/c/c-parser.c:2434
0x7038af c_parser_external_declaration
         ../../../repos/gcc/gcc/c/c-parser.c:1773
0x7044c7 c_parser_translation_unit
         ../../../repos/gcc/gcc/c/c-parser.c:1646
0x7044c7 c_parse_file()
         ../../../repos/gcc/gcc/c/c-parser.c:21822
0x764897 c_common_parse_file()
         ../../../repos/gcc/gcc/c-family/c-opts.c:1190
Please submit a full bug report,
with preprocessed source if appropriate.
Please include the complete backtrace with any bug report.
See <https://gcc.gnu.org/bugs/> for instructions.

I don't have a reduced testcase for this, but I thought I'd check if 
this is known/being worked on before filing a bug.

On 6/9/20 6:01 PM, Jeff Law via Gcc-patches wrote:
> On Thu, 2020-05-21 at 14:04 +0200, Martin Liška wrote:
>> PING^1
>>
>> On 3/20/20 4:55 PM, Martin Liška wrote:
>>> Ok, it would be possible, but if you take a look at options-save.c there's no
>>> function that will leverage that. It's a generated code so I guess we can
>>> live with that?
> Given the size increase is under control now, I think this is fine for the trunk.
> 
> jeff
> 

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [stage1][PATCH] Add gcc_assert that &global_options are not dirty modified.
  2020-06-18 11:32                 ` Luis Machado
@ 2020-06-18 12:21                   ` Martin Liška
  2020-06-18 15:02                     ` Jeff Law
  2020-06-18 15:40                   ` Martin Liška
  1 sibling, 1 reply; 22+ messages in thread
From: Martin Liška @ 2020-06-18 12:21 UTC (permalink / raw)
  To: Luis Machado, law, Jakub Jelinek; +Cc: gcc-patches, Jan Hubicka

On 6/18/20 1:32 PM, Luis Machado wrote:
> FTR, I'm running into this ICE when attempting to build the Linux Kernel for arm64.

Hello.

Thanks for the report.

> 
> More specifically:
> 
> /repos/linux-arm/kernel/bpf/core.c:1368:1: internal compiler error: ‘global_options’ are modified in local context
>   1368 | {
>        | ^
> 0xc0554b cl_optimization_compare(gcc_options*, gcc_options*)
>          /build/gcc-master/gcc/options-save.c:9786
> 0x7812df handle_optimize_attribute
>          ../../../repos/gcc/gcc/c-family/c-attribs.c:4475
> 0x666477 decl_attributes(tree_node**, tree_node*, int, tree_node*)
>          ../../../repos/gcc/gcc/attribs.c:714
> 0x688c9b start_function(c_declspecs*, c_declarator*, tree_node*)
>          ../../../repos/gcc/gcc/c/c-decl.c:9177
> 0x6f85f3 c_parser_declaration_or_fndef
>          ../../../repos/gcc/gcc/c/c-parser.c:2434
> 0x7038af c_parser_external_declaration
>          ../../../repos/gcc/gcc/c/c-parser.c:1773
> 0x7044c7 c_parser_translation_unit
>          ../../../repos/gcc/gcc/c/c-parser.c:1646
> 0x7044c7 c_parse_file()
>          ../../../repos/gcc/gcc/c/c-parser.c:21822
> 0x764897 c_common_parse_file()
>          ../../../repos/gcc/gcc/c-family/c-opts.c:1190
> Please submit a full bug report,
> with preprocessed source if appropriate.
> Please include the complete backtrace with any bug report.
> See <https://gcc.gnu.org/bugs/> for instructions.
> 
> I don't have a reduced testcase for this, but I thought I'd check if this is known/being worked on before filing a bug.

It's not a known issue. Please attach me the used command line and pre-processed source file
(using -E option).

Martin

> 
> On 6/9/20 6:01 PM, Jeff Law via Gcc-patches wrote:
>> On Thu, 2020-05-21 at 14:04 +0200, Martin Liška wrote:
>>> PING^1
>>>
>>> On 3/20/20 4:55 PM, Martin Liška wrote:
>>>> Ok, it would be possible, but if you take a look at options-save.c there's no
>>>> function that will leverage that. It's a generated code so I guess we can
>>>> live with that?
>> Given the size increase is under control now, I think this is fine for the trunk.
>>
>> jeff
>>


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [stage1][PATCH] Add gcc_assert that &global_options are not dirty modified.
  2020-06-18 12:21                   ` Martin Liška
@ 2020-06-18 15:02                     ` Jeff Law
  2020-06-18 15:36                       ` Martin Liška
  0 siblings, 1 reply; 22+ messages in thread
From: Jeff Law @ 2020-06-18 15:02 UTC (permalink / raw)
  To: Martin Liška, Luis Machado, Jakub Jelinek; +Cc: gcc-patches, Jan Hubicka

On Thu, 2020-06-18 at 14:21 +0200, Martin Liška wrote:
> On 6/18/20 1:32 PM, Luis Machado wrote:
> > FTR, I'm running into this ICE when attempting to build the Linux Kernel for arm64.
> 
> Hello.
> 
> Thanks for the report.
> 
> > More specifically:
> > 
> > /repos/linux-arm/kernel/bpf/core.c:1368:1: internal compiler error: ‘global_options’ are modified in local context
> >   1368 | {
> >        | ^
> > 0xc0554b cl_optimization_compare(gcc_options*, gcc_options*)
> >          /build/gcc-master/gcc/options-save.c:9786
> > 0x7812df handle_optimize_attribute
> >          ../../../repos/gcc/gcc/c-family/c-attribs.c:4475
> > 0x666477 decl_attributes(tree_node**, tree_node*, int, tree_node*)
> >          ../../../repos/gcc/gcc/attribs.c:714
> > 0x688c9b start_function(c_declspecs*, c_declarator*, tree_node*)
> >          ../../../repos/gcc/gcc/c/c-decl.c:9177
> > 0x6f85f3 c_parser_declaration_or_fndef
> >          ../../../repos/gcc/gcc/c/c-parser.c:2434
> > 0x7038af c_parser_external_declaration
> >          ../../../repos/gcc/gcc/c/c-parser.c:1773
> > 0x7044c7 c_parser_translation_unit
> >          ../../../repos/gcc/gcc/c/c-parser.c:1646
> > 0x7044c7 c_parse_file()
> >          ../../../repos/gcc/gcc/c/c-parser.c:21822
> > 0x764897 c_common_parse_file()
> >          ../../../repos/gcc/gcc/c-family/c-opts.c:1190
> > Please submit a full bug report,
> > with preprocessed source if appropriate.
> > Please include the complete backtrace with any bug report.
> > See <https://gcc.gnu.org/bugs/> for instructions.
> > 
> > I don't have a reduced testcase for this, but I thought I'd check if this is known/being worked on before filing a bug.
> 
> It's not a known issue. Please attach me the used command line and pre-processed source file
> (using -E option).
You might try arc-elf gcc.dg/pr91734.c.  I've been behind and just peeked at the
arc-elf regressions in the tester a moment ago and it's tripping this as well.

Executing on host: /home/jenkins/workspace/arc-elf/arc-elf-obj/gcc/gcc/xgcc -B/home/jenkins/workspace/arc-elf/arc-elf-obj/gcc/gcc/ /home/jenkins/gcc/gcc/testsuite/gcc.dg/pr91734.c gcc_tg.o    -fno-diagnostics-show-caret -fno-diagnostics-show-line-numbers -fdiagnostics-color=never  -fdiagnostics-urls=never   -ansi -pedantic-errors -O2 -std=gnu99     -Wl,-wrap,exit -Wl,-wrap,_exit -Wl,-wrap,main -Wl,-wrap,abort -lm  -o ./pr91734.exe    (timeout = 300)
spawn -ignore SIGHUP /home/jenkins/workspace/arc-elf/arc-elf-obj/gcc/gcc/xgcc -B/home/jenkins/workspace/arc-elf/arc-elf-obj/gcc/gcc/ /home/jenkins/gcc/gcc/testsuite/gcc.dg/pr91734.c gcc_tg.o -fno-diagnostics-show-caret -fno-diagnostics-show-line-numbers -fdiagnostics-color=never -fdiagnostics-urls=never -ansi -pedantic-errors -O2 -std=gnu99 -Wl,-wrap,exit -Wl,-wrap,_exit -Wl,-wrap,main -Wl,-wrap,abort -lm -o ./pr91734.exe^M
/home/jenkins/gcc/gcc/testsuite/gcc.dg/pr91734.c:8:1: internal compiler error: 'global_options' are modified in local context^M
0xc8590f cl_optimization_compare(gcc_options*, gcc_options*)^M
        /home/jenkins/workspace/arc-elf/arc-elf-obj/gcc/gcc/options-save.c:9479^M
0x870808 handle_optimize_attribute^M
        ../../../../../gcc/gcc/c-family/c-attribs.c:4475^M
0x784d3f decl_attributes(tree_node**, tree_node*, int, tree_node*)^M
        ../../../../../gcc/gcc/attribs.c:714^M
0x7a0b90 start_function(c_declspecs*, c_declarator*, tree_node*)^M
        ../../../../../gcc/gcc/c/c-decl.c:9177^M
0x7f97f7 c_parser_declaration_or_fndef^M
        ../../../../../gcc/gcc/c/c-parser.c:2434^M
0x801e33 c_parser_external_declaration^M
        ../../../../../gcc/gcc/c/c-parser.c:1773^M
0x802881 c_parser_translation_unit^M
        ../../../../../gcc/gcc/c/c-parser.c:1646^M
0x802881 c_parse_file()^M
        ../../../../../gcc/gcc/c/c-parser.c:21822^M
0x85ab6b c_common_parse_file()^M
        ../../../../../gcc/gcc/c-family/c-opts.c:1190^M
> 


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [stage1][PATCH] Add gcc_assert that &global_options are not dirty modified.
  2020-06-18 15:02                     ` Jeff Law
@ 2020-06-18 15:36                       ` Martin Liška
  2020-06-24  7:44                         ` Martin Liška
  0 siblings, 1 reply; 22+ messages in thread
From: Martin Liška @ 2020-06-18 15:36 UTC (permalink / raw)
  To: law, Luis Machado, Jakub Jelinek; +Cc: gcc-patches, Jan Hubicka

On 6/18/20 5:02 PM, Jeff Law wrote:
> On Thu, 2020-06-18 at 14:21 +0200, Martin Liška wrote:
>> On 6/18/20 1:32 PM, Luis Machado wrote:
>>> FTR, I'm running into this ICE when attempting to build the Linux Kernel for arm64.
>>
>> Hello.
>>
>> Thanks for the report.
>>
>>> More specifically:
>>>
>>> /repos/linux-arm/kernel/bpf/core.c:1368:1: internal compiler error: ‘global_options’ are modified in local context
>>>    1368 | {
>>>         | ^
>>> 0xc0554b cl_optimization_compare(gcc_options*, gcc_options*)
>>>           /build/gcc-master/gcc/options-save.c:9786
>>> 0x7812df handle_optimize_attribute
>>>           ../../../repos/gcc/gcc/c-family/c-attribs.c:4475
>>> 0x666477 decl_attributes(tree_node**, tree_node*, int, tree_node*)
>>>           ../../../repos/gcc/gcc/attribs.c:714
>>> 0x688c9b start_function(c_declspecs*, c_declarator*, tree_node*)
>>>           ../../../repos/gcc/gcc/c/c-decl.c:9177
>>> 0x6f85f3 c_parser_declaration_or_fndef
>>>           ../../../repos/gcc/gcc/c/c-parser.c:2434
>>> 0x7038af c_parser_external_declaration
>>>           ../../../repos/gcc/gcc/c/c-parser.c:1773
>>> 0x7044c7 c_parser_translation_unit
>>>           ../../../repos/gcc/gcc/c/c-parser.c:1646
>>> 0x7044c7 c_parse_file()
>>>           ../../../repos/gcc/gcc/c/c-parser.c:21822
>>> 0x764897 c_common_parse_file()
>>>           ../../../repos/gcc/gcc/c-family/c-opts.c:1190
>>> Please submit a full bug report,
>>> with preprocessed source if appropriate.
>>> Please include the complete backtrace with any bug report.
>>> See <https://gcc.gnu.org/bugs/> for instructions.
>>>
>>> I don't have a reduced testcase for this, but I thought I'd check if this is known/being worked on before filing a bug.
>>
>> It's not a known issue. Please attach me the used command line and pre-processed source file
>> (using -E option).
> You might try arc-elf gcc.dg/pr91734.c.  I've been behind and just peeked at the
> arc-elf regressions in the tester a moment ago and it's tripping this as well.

Thanks for it. This one is one another example where an optimization level affects
a target option:

$ cat x.c
__attribute__((optimize ("O3"))) void
f1 ()
{
}

int
main ()
{
   return 0;
}

11682	  if (ptr1->x_TARGET_ALIGN_CALL != ptr2->x_TARGET_ALIGN_CALL)
11683	    internal_error ("%<global_options%> are modified in local context");

set here:
     { OPT_LEVELS_3_PLUS_SPEED_ONLY, OPT_malign_call, NULL, 1 },

I'm going to add this to exception list.


> 
> Executing on host: /home/jenkins/workspace/arc-elf/arc-elf-obj/gcc/gcc/xgcc -B/home/jenkins/workspace/arc-elf/arc-elf-obj/gcc/gcc/ /home/jenkins/gcc/gcc/testsuite/gcc.dg/pr91734.c gcc_tg.o    -fno-diagnostics-show-caret -fno-diagnostics-show-line-numbers -fdiagnostics-color=never  -fdiagnostics-urls=never   -ansi -pedantic-errors -O2 -std=gnu99     -Wl,-wrap,exit -Wl,-wrap,_exit -Wl,-wrap,main -Wl,-wrap,abort -lm  -o ./pr91734.exe    (timeout = 300)
> spawn -ignore SIGHUP /home/jenkins/workspace/arc-elf/arc-elf-obj/gcc/gcc/xgcc -B/home/jenkins/workspace/arc-elf/arc-elf-obj/gcc/gcc/ /home/jenkins/gcc/gcc/testsuite/gcc.dg/pr91734.c gcc_tg.o -fno-diagnostics-show-caret -fno-diagnostics-show-line-numbers -fdiagnostics-color=never -fdiagnostics-urls=never -ansi -pedantic-errors -O2 -std=gnu99 -Wl,-wrap,exit -Wl,-wrap,_exit -Wl,-wrap,main -Wl,-wrap,abort -lm -o ./pr91734.exe^M
> /home/jenkins/gcc/gcc/testsuite/gcc.dg/pr91734.c:8:1: internal compiler error: 'global_options' are modified in local context^M
> 0xc8590f cl_optimization_compare(gcc_options*, gcc_options*)^M
>          /home/jenkins/workspace/arc-elf/arc-elf-obj/gcc/gcc/options-save.c:9479^M
> 0x870808 handle_optimize_attribute^M
>          ../../../../../gcc/gcc/c-family/c-attribs.c:4475^M
> 0x784d3f decl_attributes(tree_node**, tree_node*, int, tree_node*)^M
>          ../../../../../gcc/gcc/attribs.c:714^M
> 0x7a0b90 start_function(c_declspecs*, c_declarator*, tree_node*)^M
>          ../../../../../gcc/gcc/c/c-decl.c:9177^M
> 0x7f97f7 c_parser_declaration_or_fndef^M
>          ../../../../../gcc/gcc/c/c-parser.c:2434^M
> 0x801e33 c_parser_external_declaration^M
>          ../../../../../gcc/gcc/c/c-parser.c:1773^M
> 0x802881 c_parser_translation_unit^M
>          ../../../../../gcc/gcc/c/c-parser.c:1646^M
> 0x802881 c_parse_file()^M
>          ../../../../../gcc/gcc/c/c-parser.c:21822^M
> 0x85ab6b c_common_parse_file()^M
>          ../../../../../gcc/gcc/c-family/c-opts.c:1190^M
>>
> 


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [stage1][PATCH] Add gcc_assert that &global_options are not dirty modified.
  2020-06-18 11:32                 ` Luis Machado
  2020-06-18 12:21                   ` Martin Liška
@ 2020-06-18 15:40                   ` Martin Liška
  2020-06-18 15:47                     ` Luis Machado
  2020-06-24  7:25                     ` Martin Liška
  1 sibling, 2 replies; 22+ messages in thread
From: Martin Liška @ 2020-06-18 15:40 UTC (permalink / raw)
  To: Luis Machado, law, Jakub Jelinek; +Cc: gcc-patches, Jan Hubicka

I see the following ICE for aarch64 kernel build:

$ cat neon.i
#pragma GCC push_options
#pragma GCC target "arch=armv8.2-a+bf16"
#pragma GCC pop_options

$ ./xgcc -B. ~/Programming/testcases/neon.i -c -mbranch-protection=pac-ret
/home/marxin/Programming/testcases/neon.i:3:9: internal compiler error: ‘global_options’ are modified in local context
     3 | #pragma GCC pop_options
       |         ^~~
0x1111f73 cl_optimization_compare(gcc_options*, gcc_options*)
	/dev/shm/objdir3/gcc/options-save.c:11996
0xb02ff4 handle_pragma_pop_options
	/home/marxin/Programming/gcc/gcc/c-family/c-pragma.c:1090
0xb03953 c_invoke_pragma_handler(unsigned int)
	/home/marxin/Programming/gcc/gcc/c-family/c-pragma.c:1512
0xa5ae39 c_parser_pragma
	/home/marxin/Programming/gcc/gcc/c/c-parser.c:12544
0xa3f9fc c_parser_external_declaration
	/home/marxin/Programming/gcc/gcc/c/c-parser.c:1754
0xa3f5c8 c_parser_translation_unit
	/home/marxin/Programming/gcc/gcc/c/c-parser.c:1646
0xa7db4d c_parse_file()
	/home/marxin/Programming/gcc/gcc/c/c-parser.c:21822
0xafd0b6 c_common_parse_file()
	/home/marxin/Programming/gcc/gcc/c-family/c-opts.c:1190
Please submit a full bug report,
with preprocessed source if appropriate.
Please include the complete backtrace with any bug report.
See <https://gcc.gnu.org/bugs/> for instructions.

#1  0x0000000001111f74 in cl_optimization_compare (ptr1=0x2d5e3f0, ptr2=0x2cc7760 <global_options>) at options-save.c:11996
11996	    internal_error ("%<global_options%> are modified in local context");
(gdb) p ptr2->x_aarch64_branch_protection_string
$2 = 0x2cf52e0 "pac-ret"
(gdb) p ptr1->x_aarch64_branch_protection_string
$3 = 0x2d3c190 "pac-ret"


    │11995         if (ptr1->x_aarch64_branch_protection_string != ptr2->x_aarch64_branch_protection_string)
   >│11996           internal_error ("%<global_options%> are modified in local context");

This is bogus as these are 2 strings that are equal. Let me fix it.

Martin

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [stage1][PATCH] Add gcc_assert that &global_options are not dirty modified.
  2020-06-18 15:40                   ` Martin Liška
@ 2020-06-18 15:47                     ` Luis Machado
  2020-06-18 16:02                       ` Martin Liška
  2020-06-24  7:25                     ` Martin Liška
  1 sibling, 1 reply; 22+ messages in thread
From: Luis Machado @ 2020-06-18 15:47 UTC (permalink / raw)
  To: Martin Liška, law, Jakub Jelinek; +Cc: gcc-patches, Jan Hubicka

On 6/18/20 12:40 PM, Martin Liška wrote:
> I see the following ICE for aarch64 kernel build:
> 
> $ cat neon.i
> #pragma GCC push_options
> #pragma GCC target "arch=armv8.2-a+bf16"
> #pragma GCC pop_options
> 
> $ ./xgcc -B. ~/Programming/testcases/neon.i -c -mbranch-protection=pac-ret
> /home/marxin/Programming/testcases/neon.i:3:9: internal compiler error: 
> ‘global_options’ are modified in local context
>      3 | #pragma GCC pop_options
>        |         ^~~
> 0x1111f73 cl_optimization_compare(gcc_options*, gcc_options*)
>      /dev/shm/objdir3/gcc/options-save.c:11996
> 0xb02ff4 handle_pragma_pop_options
>      /home/marxin/Programming/gcc/gcc/c-family/c-pragma.c:1090
> 0xb03953 c_invoke_pragma_handler(unsigned int)
>      /home/marxin/Programming/gcc/gcc/c-family/c-pragma.c:1512
> 0xa5ae39 c_parser_pragma
>      /home/marxin/Programming/gcc/gcc/c/c-parser.c:12544
> 0xa3f9fc c_parser_external_declaration
>      /home/marxin/Programming/gcc/gcc/c/c-parser.c:1754
> 0xa3f5c8 c_parser_translation_unit
>      /home/marxin/Programming/gcc/gcc/c/c-parser.c:1646
> 0xa7db4d c_parse_file()
>      /home/marxin/Programming/gcc/gcc/c/c-parser.c:21822
> 0xafd0b6 c_common_parse_file()
>      /home/marxin/Programming/gcc/gcc/c-family/c-opts.c:1190
> Please submit a full bug report,
> with preprocessed source if appropriate.
> Please include the complete backtrace with any bug report.
> See <https://gcc.gnu.org/bugs/> for instructions.
> 
> #1  0x0000000001111f74 in cl_optimization_compare (ptr1=0x2d5e3f0, 
> ptr2=0x2cc7760 <global_options>) at options-save.c:11996
> 11996        internal_error ("%<global_options%> are modified in local 
> context");
> (gdb) p ptr2->x_aarch64_branch_protection_string
> $2 = 0x2cf52e0 "pac-ret"
> (gdb) p ptr1->x_aarch64_branch_protection_string
> $3 = 0x2d3c190 "pac-ret"
> 
> 
>     │11995         if (ptr1->x_aarch64_branch_protection_string != 
> ptr2->x_aarch64_branch_protection_string)
>    >│11996           internal_error ("%<global_options%> are modified in 
> local context");
> 
> This is bogus as these are 2 strings that are equal. Let me fix it.
> 
> Martin

That's another one I noticed alongside the first one I reported. That's 
good that you managed to reproduce it.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [stage1][PATCH] Add gcc_assert that &global_options are not dirty modified.
  2020-06-18 15:47                     ` Luis Machado
@ 2020-06-18 16:02                       ` Martin Liška
  2020-06-18 17:18                         ` Luis Machado
  0 siblings, 1 reply; 22+ messages in thread
From: Martin Liška @ 2020-06-18 16:02 UTC (permalink / raw)
  To: Luis Machado, law, Jakub Jelinek; +Cc: gcc-patches, Jan Hubicka

On 6/18/20 5:47 PM, Luis Machado wrote:
> That's another one I noticed alongside the first one I reported. That's good that you managed to reproduce it.

Can you please send me your .config and I can reproduce that locally.

Thanks,
Martin

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [stage1][PATCH] Add gcc_assert that &global_options are not dirty modified.
  2020-06-18 16:02                       ` Martin Liška
@ 2020-06-18 17:18                         ` Luis Machado
  2020-06-18 18:34                           ` Martin Liška
  0 siblings, 1 reply; 22+ messages in thread
From: Luis Machado @ 2020-06-18 17:18 UTC (permalink / raw)
  To: Martin Liška, law, Jakub Jelinek; +Cc: gcc-patches, Jan Hubicka

[-- Attachment #1: Type: text/plain, Size: 498 bytes --]

On 6/18/20 1:02 PM, Martin Liška wrote:
> On 6/18/20 5:47 PM, Luis Machado wrote:
>> That's another one I noticed alongside the first one I reported. 
>> That's good that you managed to reproduce it.
> 
> Can you please send me your .config and I can reproduce that locally.
> 
> Thanks,
> Martin

Here it is. It is a defconfig with a gas that supports aarch64 memtag 
(binutils-gdb master will do). I hope this helps. Otherwise I can 
compile the information and dump it in a ticket later.

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 49324 bytes --]

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [stage1][PATCH] Add gcc_assert that &global_options are not dirty modified.
  2020-06-18 17:18                         ` Luis Machado
@ 2020-06-18 18:34                           ` Martin Liška
  2020-06-18 21:24                             ` Luis Machado
  0 siblings, 1 reply; 22+ messages in thread
From: Martin Liška @ 2020-06-18 18:34 UTC (permalink / raw)
  To: Luis Machado, law, Jakub Jelinek; +Cc: gcc-patches, Jan Hubicka

On 6/18/20 7:18 PM, Luis Machado wrote:
> On 6/18/20 1:02 PM, Martin Liška wrote:
>> On 6/18/20 5:47 PM, Luis Machado wrote:
>>> That's another one I noticed alongside the first one I reported. That's good that you managed to reproduce it.
>>
>> Can you please send me your .config and I can reproduce that locally.
>>
>> Thanks,
>> Martin
> 
> Here it is. It is a defconfig with a gas that supports aarch64 memtag (binutils-gdb master will do). I hope this helps. Otherwise I can compile the information and dump it in a ticket later.

In that case please attach output of -E option for the problematic compilation unit.

Martin

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [stage1][PATCH] Add gcc_assert that &global_options are not dirty modified.
  2020-06-18 18:34                           ` Martin Liška
@ 2020-06-18 21:24                             ` Luis Machado
  0 siblings, 0 replies; 22+ messages in thread
From: Luis Machado @ 2020-06-18 21:24 UTC (permalink / raw)
  To: Martin Liška, law, Jakub Jelinek; +Cc: gcc-patches, Jan Hubicka

On 6/18/20 3:34 PM, Martin Liška wrote:
> On 6/18/20 7:18 PM, Luis Machado wrote:
>> On 6/18/20 1:02 PM, Martin Liška wrote:
>>> On 6/18/20 5:47 PM, Luis Machado wrote:
>>>> That's another one I noticed alongside the first one I reported. 
>>>> That's good that you managed to reproduce it.
>>>
>>> Can you please send me your .config and I can reproduce that locally.
>>>
>>> Thanks,
>>> Martin
>>
>> Here it is. It is a defconfig with a gas that supports aarch64 memtag 
>> (binutils-gdb master will do). I hope this helps. Otherwise I can 
>> compile the information and dump it in a ticket later.
> 
> In that case please attach output of -E option for the problematic 
> compilation unit.
> 
> Martin

Here it is: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95753

Sorry for the delay. I was busy with something else.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [stage1][PATCH] Add gcc_assert that &global_options are not dirty modified.
  2020-06-18 15:40                   ` Martin Liška
  2020-06-18 15:47                     ` Luis Machado
@ 2020-06-24  7:25                     ` Martin Liška
  1 sibling, 0 replies; 22+ messages in thread
From: Martin Liška @ 2020-06-24  7:25 UTC (permalink / raw)
  To: Luis Machado, law, Jakub Jelinek; +Cc: gcc-patches, Jan Hubicka

On 6/18/20 5:40 PM, Martin Liška wrote:
> This is bogus as these are 2 strings that are equal. Let me fix it.

Hey.

Just for the report, this is fixed on master right now.

Martin

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [stage1][PATCH] Add gcc_assert that &global_options are not dirty modified.
  2020-06-18 15:36                       ` Martin Liška
@ 2020-06-24  7:44                         ` Martin Liška
  0 siblings, 0 replies; 22+ messages in thread
From: Martin Liška @ 2020-06-24  7:44 UTC (permalink / raw)
  To: law, Luis Machado, Jakub Jelinek; +Cc: gcc-patches, Jan Hubicka

On 6/18/20 5:36 PM, Martin Liška wrote:
> I'm going to add this to exception list.

Done here:
https://gcc.gnu.org/pipermail/gcc-patches/2020-June/548800.html

Martin

^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2020-06-24  7:44 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-19  8:56 [stage1][PATCH] Add gcc_assert that &global_options are not dirty modified Martin Liška
2020-03-19  9:09 ` Jakub Jelinek
2020-03-19 15:32   ` Martin Liška
2020-03-19 15:47     ` Jakub Jelinek
2020-03-19 16:13     ` Martin Sebor
2020-03-20 15:40       ` Martin Liška
2020-03-20 15:43         ` Jakub Jelinek
2020-03-20 15:55           ` Martin Liška
2020-05-21 12:04             ` Martin Liška
2020-06-09 21:01               ` Jeff Law
2020-06-18 11:32                 ` Luis Machado
2020-06-18 12:21                   ` Martin Liška
2020-06-18 15:02                     ` Jeff Law
2020-06-18 15:36                       ` Martin Liška
2020-06-24  7:44                         ` Martin Liška
2020-06-18 15:40                   ` Martin Liška
2020-06-18 15:47                     ` Luis Machado
2020-06-18 16:02                       ` Martin Liška
2020-06-18 17:18                         ` Luis Machado
2020-06-18 18:34                           ` Martin Liška
2020-06-18 21:24                             ` Luis Machado
2020-06-24  7:25                     ` Martin Liška

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).