[PATCH] Record that -gtoggle is already used in gcc_options.

[PATCH] Record that -gtoggle is already used in gcc_options.

[Martin Liška]

Patch can bootstrap on x86_64-linux-gnu and survives regression tests.

Ready to be installed?
Thanks,
Martin

When doing flip based on -gtoggle, record it. Otherwise, we will
apply it for the second time in finish_options.

	PR debug/102955

gcc/ChangeLog:

	* common.opt: Add new gtoggle_used variable.
	* opts.c (finish_options): Do not interpret flag_gtoggle twice.

gcc/testsuite/ChangeLog:

	* gcc.dg/pr102955.c: New test.
---
  gcc/common.opt                  |  4 ++++
  gcc/opts.c                      |  3 ++-
  gcc/testsuite/gcc.dg/pr102955.c | 14 ++++++++++++++
  3 files changed, 20 insertions(+), 1 deletion(-)
  create mode 100644 gcc/testsuite/gcc.dg/pr102955.c

diff --git a/gcc/common.opt b/gcc/common.opt
index 1a5b9bfcca9..2568ecb98b8 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -3316,6 +3316,10 @@ gdescribe-dies
  Common Driver Var(flag_describe_dies) Init(0)
  Add description attributes to some DWARF DIEs that have no name attribute.
  
+; True if -gtoggle option was already handled.
+Variable
+bool gtoggle_used
+
  gtoggle
  Common Driver Var(flag_gtoggle)
  Toggle debug information generation.
diff --git a/gcc/opts.c b/gcc/opts.c
index 3f80fce82bc..ef38b8dbab0 100644
--- a/gcc/opts.c
+++ b/gcc/opts.c
@@ -1375,8 +1375,9 @@ finish_options (struct gcc_options *opts, struct gcc_options *opts_set,
        profile_flag = 0;
      }
  
-  if (flag_gtoggle)
+  if (flag_gtoggle && !gtoggle_used)
      {
+      gtoggle_used = true;
        if (debug_info_level == DINFO_LEVEL_NONE)
  	{
  	  debug_info_level = DINFO_LEVEL_NORMAL;
diff --git a/gcc/testsuite/gcc.dg/pr102955.c b/gcc/testsuite/gcc.dg/pr102955.c
new file mode 100644
index 00000000000..de9689edec4
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr102955.c
@@ -0,0 +1,14 @@
+/* PR debug/102955 */
+/* { dg-do compile } */
+/* { dg-options "-g -gtoggle" } */
+
+#pragma GCC optimize "0"
+struct j
+{
+  explicit j ();
+  ~j ();
+};
+void g (void)
+{
+  new j();
+}
-- 
2.33.1

Re: [PATCH] Record that -gtoggle is already used in gcc_options.

[Richard Biener]

On Tue, Nov 2, 2021 at 3:11 PM Martin Liška <mliska@suse.cz> wrote:
>
> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
>
> Ready to be installed?

I think -gtoggle matches a Defered option and thus should be processed
in handle_common_deferred_options.  I'd argue that --help printing shouldn't
be part of decode_options but done only in toplev.c (not from the
parse_optimize_options caller).

Richard.

> Thanks,
> Martin
>
> When doing flip based on -gtoggle, record it. Otherwise, we will
> apply it for the second time in finish_options.
>
>         PR debug/102955
>
> gcc/ChangeLog:
>
>         * common.opt: Add new gtoggle_used variable.
>         * opts.c (finish_options): Do not interpret flag_gtoggle twice.
>
> gcc/testsuite/ChangeLog:
>
>         * gcc.dg/pr102955.c: New test.
> ---
>   gcc/common.opt                  |  4 ++++
>   gcc/opts.c                      |  3 ++-
>   gcc/testsuite/gcc.dg/pr102955.c | 14 ++++++++++++++
>   3 files changed, 20 insertions(+), 1 deletion(-)
>   create mode 100644 gcc/testsuite/gcc.dg/pr102955.c
>
> diff --git a/gcc/common.opt b/gcc/common.opt
> index 1a5b9bfcca9..2568ecb98b8 100644
> --- a/gcc/common.opt
> +++ b/gcc/common.opt
> @@ -3316,6 +3316,10 @@ gdescribe-dies
>   Common Driver Var(flag_describe_dies) Init(0)
>   Add description attributes to some DWARF DIEs that have no name attribute.
>
> +; True if -gtoggle option was already handled.
> +Variable
> +bool gtoggle_used
> +
>   gtoggle
>   Common Driver Var(flag_gtoggle)
>   Toggle debug information generation.
> diff --git a/gcc/opts.c b/gcc/opts.c
> index 3f80fce82bc..ef38b8dbab0 100644
> --- a/gcc/opts.c
> +++ b/gcc/opts.c
> @@ -1375,8 +1375,9 @@ finish_options (struct gcc_options *opts, struct gcc_options *opts_set,
>         profile_flag = 0;
>       }
>
> -  if (flag_gtoggle)
> +  if (flag_gtoggle && !gtoggle_used)
>       {
> +      gtoggle_used = true;
>         if (debug_info_level == DINFO_LEVEL_NONE)
>         {
>           debug_info_level = DINFO_LEVEL_NORMAL;
> diff --git a/gcc/testsuite/gcc.dg/pr102955.c b/gcc/testsuite/gcc.dg/pr102955.c
> new file mode 100644
> index 00000000000..de9689edec4
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/pr102955.c
> @@ -0,0 +1,14 @@
> +/* PR debug/102955 */
> +/* { dg-do compile } */
> +/* { dg-options "-g -gtoggle" } */
> +
> +#pragma GCC optimize "0"
> +struct j
> +{
> +  explicit j ();
> +  ~j ();
> +};
> +void g (void)
> +{
> +  new j();
> +}
> --
> 2.33.1
>

Re: [PATCH] Record that -gtoggle is already used in gcc_options.

[Martin Liška]

On 11/2/21 15:33, Richard Biener wrote:
> I think -gtoggle matches a Defered option and thus should be processed
> in handle_common_deferred_options.

Well, that's quite problematic as I handle_common_deferred_options is called
after decode_options (that calls finish_options).

Note there's direct dependency at very end of finish_options in between -gtoggle
and debug_nonbind_markers_p:


   if (flag_gtoggle)
     {
       if (debug_info_level == DINFO_LEVEL_NONE)
	{
	  debug_info_level = DINFO_LEVEL_NORMAL;

	  if (write_symbols == NO_DEBUG)
	    write_symbols = PREFERRED_DEBUGGING_TYPE;
	}
       else
	debug_info_level = DINFO_LEVEL_NONE;
     }

   if (!OPTION_SET_P (debug_nonbind_markers_p))
     debug_nonbind_markers_p
       = (optimize
	 && debug_info_level >= DINFO_LEVEL_NORMAL
	 && dwarf_debuginfo_p ()
	 && !(flag_selective_scheduling || flag_selective_scheduling2));

I don't see who you mean the possible fix?

Martin

Re: [PATCH] Record that -gtoggle is already used in gcc_options.

[Richard Biener]

On Tue, Nov 2, 2021 at 4:11 PM Martin Liška <mliska@suse.cz> wrote:
>
> On 11/2/21 15:33, Richard Biener wrote:
> > I think -gtoggle matches a Defered option and thus should be processed
> > in handle_common_deferred_options.
>
> Well, that's quite problematic as I handle_common_deferred_options is called
> after decode_options (that calls finish_options).
>
> Note there's direct dependency at very end of finish_options in between -gtoggle
> and debug_nonbind_markers_p:
>
>
>    if (flag_gtoggle)
>      {
>        if (debug_info_level == DINFO_LEVEL_NONE)
>         {
>           debug_info_level = DINFO_LEVEL_NORMAL;
>
>           if (write_symbols == NO_DEBUG)
>             write_symbols = PREFERRED_DEBUGGING_TYPE;
>         }
>        else
>         debug_info_level = DINFO_LEVEL_NONE;
>      }
>
>    if (!OPTION_SET_P (debug_nonbind_markers_p))
>      debug_nonbind_markers_p
>        = (optimize
>          && debug_info_level >= DINFO_LEVEL_NORMAL
>          && dwarf_debuginfo_p ()
>          && !(flag_selective_scheduling || flag_selective_scheduling2));
>
> I don't see who you mean the possible fix?

So at first I thought we might have a place that post-processes
'decoded_options' so we could reflect -gtoggle on those but
out-of-order (removing/adding -g).  But that's going to be mightly
complicated as well.

I wonder what the original issue is you fix?  You say we ap;ly
it for a second time but we should apply it onto the same
state as previously since we restore that for optimize attribute
processing?

Richard.

>
> Martin

Re: [PATCH] Record that -gtoggle is already used in gcc_options.

[Andrew Pinski]

On Tue, Nov 2, 2021 at 7:11 AM Martin Liška <mliska@suse.cz> wrote:
>
> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
>
> Ready to be installed?
> Thanks,
> Martin
>
> When doing flip based on -gtoggle, record it. Otherwise, we will
> apply it for the second time in finish_options.
>
>         PR debug/102955
>
> gcc/ChangeLog:
>
>         * common.opt: Add new gtoggle_used variable.
>         * opts.c (finish_options): Do not interpret flag_gtoggle twice.
>
> gcc/testsuite/ChangeLog:
>
>         * gcc.dg/pr102955.c: New test.

Hmm, this is a C++ testcase so shouldn't it be at g++.dg/pr102955.C ?

Thanks,
Andrew Pinski

> ---
>   gcc/common.opt                  |  4 ++++
>   gcc/opts.c                      |  3 ++-
>   gcc/testsuite/gcc.dg/pr102955.c | 14 ++++++++++++++
>   3 files changed, 20 insertions(+), 1 deletion(-)
>   create mode 100644 gcc/testsuite/gcc.dg/pr102955.c
>
> diff --git a/gcc/common.opt b/gcc/common.opt
> index 1a5b9bfcca9..2568ecb98b8 100644
> --- a/gcc/common.opt
> +++ b/gcc/common.opt
> @@ -3316,6 +3316,10 @@ gdescribe-dies
>   Common Driver Var(flag_describe_dies) Init(0)
>   Add description attributes to some DWARF DIEs that have no name attribute.
>
> +; True if -gtoggle option was already handled.
> +Variable
> +bool gtoggle_used
> +
>   gtoggle
>   Common Driver Var(flag_gtoggle)
>   Toggle debug information generation.
> diff --git a/gcc/opts.c b/gcc/opts.c
> index 3f80fce82bc..ef38b8dbab0 100644
> --- a/gcc/opts.c
> +++ b/gcc/opts.c
> @@ -1375,8 +1375,9 @@ finish_options (struct gcc_options *opts, struct gcc_options *opts_set,
>         profile_flag = 0;
>       }
>
> -  if (flag_gtoggle)
> +  if (flag_gtoggle && !gtoggle_used)
>       {
> +      gtoggle_used = true;
>         if (debug_info_level == DINFO_LEVEL_NONE)
>         {
>           debug_info_level = DINFO_LEVEL_NORMAL;
> diff --git a/gcc/testsuite/gcc.dg/pr102955.c b/gcc/testsuite/gcc.dg/pr102955.c
> new file mode 100644
> index 00000000000..de9689edec4
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/pr102955.c
> @@ -0,0 +1,14 @@
> +/* PR debug/102955 */
> +/* { dg-do compile } */
> +/* { dg-options "-g -gtoggle" } */
> +
> +#pragma GCC optimize "0"
> +struct j
> +{
> +  explicit j ();
> +  ~j ();
> +};
> +void g (void)
> +{
> +  new j();
> +}
> --
> 2.33.1
>

Re: [PATCH] Record that -gtoggle is already used in gcc_options.

[Martin Liška]

On 11/2/21 17:45, Richard Biener wrote:
> On Tue, Nov 2, 2021 at 4:11 PM Martin Liška <mliska@suse.cz> wrote:
>>
>> On 11/2/21 15:33, Richard Biener wrote:
>>> I think -gtoggle matches a Defered option and thus should be processed
>>> in handle_common_deferred_options.
>>
>> Well, that's quite problematic as I handle_common_deferred_options is called
>> after decode_options (that calls finish_options).
>>
>> Note there's direct dependency at very end of finish_options in between -gtoggle
>> and debug_nonbind_markers_p:
>>
>>
>>     if (flag_gtoggle)
>>       {
>>         if (debug_info_level == DINFO_LEVEL_NONE)
>>          {
>>            debug_info_level = DINFO_LEVEL_NORMAL;
>>
>>            if (write_symbols == NO_DEBUG)
>>              write_symbols = PREFERRED_DEBUGGING_TYPE;
>>          }
>>         else
>>          debug_info_level = DINFO_LEVEL_NONE;
>>       }
>>
>>     if (!OPTION_SET_P (debug_nonbind_markers_p))
>>       debug_nonbind_markers_p
>>         = (optimize
>>           && debug_info_level >= DINFO_LEVEL_NORMAL
>>           && dwarf_debuginfo_p ()
>>           && !(flag_selective_scheduling || flag_selective_scheduling2));
>>
>> I don't see who you mean the possible fix?
> 
> So at first I thought we might have a place that post-processes
> 'decoded_options' so we could reflect -gtoggle on those but
> out-of-order (removing/adding -g).  But that's going to be mightly
> complicated as well.

That would be very complicated.

> 
> I wonder what the original issue is you fix?  You say we ap;ly
> it for a second time but we should apply it onto the same
> state as previously since we restore that for optimize attribute
> processing?

Well, finish_options is always called once we parse options and we want to finalize them.
So that happens from toplev where we create initial global options. Later on, after the pragma
is parsed (where we start with current global options), the finish_options is called.

Problem of -gtoggle is that it does not directly influence an option, but it negates it.

That said, I think my patch with gtoggle_used is a reasonable workaround.

Cheers,
Martin

> 
> Richard.
> 
>>
>> Martin

Re: [PATCH] Record that -gtoggle is already used in gcc_options.

[Richard Biener]

On Thu, Nov 4, 2021 at 1:51 PM Martin Liška <mliska@suse.cz> wrote:
>
> On 11/2/21 17:45, Richard Biener wrote:
> > On Tue, Nov 2, 2021 at 4:11 PM Martin Liška <mliska@suse.cz> wrote:
> >>
> >> On 11/2/21 15:33, Richard Biener wrote:
> >>> I think -gtoggle matches a Defered option and thus should be processed
> >>> in handle_common_deferred_options.
> >>
> >> Well, that's quite problematic as I handle_common_deferred_options is called
> >> after decode_options (that calls finish_options).
> >>
> >> Note there's direct dependency at very end of finish_options in between -gtoggle
> >> and debug_nonbind_markers_p:
> >>
> >>
> >>     if (flag_gtoggle)
> >>       {
> >>         if (debug_info_level == DINFO_LEVEL_NONE)
> >>          {
> >>            debug_info_level = DINFO_LEVEL_NORMAL;
> >>
> >>            if (write_symbols == NO_DEBUG)
> >>              write_symbols = PREFERRED_DEBUGGING_TYPE;
> >>          }
> >>         else
> >>          debug_info_level = DINFO_LEVEL_NONE;
> >>       }
> >>
> >>     if (!OPTION_SET_P (debug_nonbind_markers_p))
> >>       debug_nonbind_markers_p
> >>         = (optimize
> >>           && debug_info_level >= DINFO_LEVEL_NORMAL
> >>           && dwarf_debuginfo_p ()
> >>           && !(flag_selective_scheduling || flag_selective_scheduling2));
> >>
> >> I don't see who you mean the possible fix?
> >
> > So at first I thought we might have a place that post-processes
> > 'decoded_options' so we could reflect -gtoggle on those but
> > out-of-order (removing/adding -g).  But that's going to be mightly
> > complicated as well.
>
> That would be very complicated.
>
> >
> > I wonder what the original issue is you fix?  You say we ap;ly
> > it for a second time but we should apply it onto the same
> > state as previously since we restore that for optimize attribute
> > processing?
>
> Well, finish_options is always called once we parse options and we want to finalize them.
> So that happens from toplev where we create initial global options. Later on, after the pragma
> is parsed (where we start with current global options), the finish_options is called.

But we shouldn't start with the current global options but with ones
we saved for
optimize attribute/pragma processing, no?

> Problem of -gtoggle is that it does not directly influence an option, but it negates it.
>
> That said, I think my patch with gtoggle_used is a reasonable workaround.

Well, then we could as well unset flag_gtoggle after processing it, no?

Thanks,
Richard.

> Cheers,
> Martin
>
> >
> > Richard.
> >
> >>
> >> Martin
>

Re: [PATCH] Record that -gtoggle is already used in gcc_options.

[Martin Liška]

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

On 11/4/21 14:09, Richard Biener wrote:
> But we shouldn't start with the current global options but with ones
> we saved for
> optimize attribute/pragma processing, no?

We hit the issue when we combine cmdline and pragma optimize options.

> 
>> Problem of -gtoggle is that it does not directly influence an option, but it negates it.
>>
>> That said, I think my patch with gtoggle_used is a reasonable workaround.
> Well, then we could as well unset flag_gtoggle after processing it, no?

Yeah, that work! :)

Patch can bootstrap on x86_64-linux-gnu and survives regression tests.

Ready to be installed?
Thanks,
Martin

[-- Attachment #2: 0001-Reset-when-gtoggle-is-used-in-gcc_options.patch --]
[-- Type: text/x-patch, Size: 1308 bytes --]

From b458389805f3d0677b04850d95990a88df40806f Mon Sep 17 00:00:00 2001
From: Martin Liska <mliska@suse.cz>
Date: Mon, 1 Nov 2021 16:28:34 +0100
Subject: [PATCH] Reset when -gtoggle is used in gcc_options.

	PR debug/102955

gcc/ChangeLog:

	* opts.c (finish_options): Reset flag_gtoggle when it is used.

gcc/testsuite/ChangeLog:

	* g++.dg/pr102955.C: New test.
---
 gcc/opts.c                      |  1 +
 gcc/testsuite/g++.dg/pr102955.C | 14 ++++++++++++++
 2 files changed, 15 insertions(+)
 create mode 100644 gcc/testsuite/g++.dg/pr102955.C

diff --git a/gcc/opts.c b/gcc/opts.c
index 3f80fce82bc..442e6597b63 100644
--- a/gcc/opts.c
+++ b/gcc/opts.c
@@ -1377,6 +1377,7 @@ finish_options (struct gcc_options *opts, struct gcc_options *opts_set,
 
   if (flag_gtoggle)
     {
+      flag_gtoggle = false;
       if (debug_info_level == DINFO_LEVEL_NONE)
 	{
 	  debug_info_level = DINFO_LEVEL_NORMAL;
diff --git a/gcc/testsuite/g++.dg/pr102955.C b/gcc/testsuite/g++.dg/pr102955.C
new file mode 100644
index 00000000000..de9689edec4
--- /dev/null
+++ b/gcc/testsuite/g++.dg/pr102955.C
@@ -0,0 +1,14 @@
+/* PR debug/102955 */
+/* { dg-do compile } */
+/* { dg-options "-g -gtoggle" } */
+
+#pragma GCC optimize "0"
+struct j
+{
+  explicit j ();
+  ~j ();
+};
+void g (void)
+{
+  new j();
+}
-- 
2.33.1

Re: [PATCH] Record that -gtoggle is already used in gcc_options.

[Richard Biener]

On Thu, Nov 4, 2021 at 4:11 PM Martin Liška <mliska@suse.cz> wrote:
>
> On 11/4/21 14:09, Richard Biener wrote:
> > But we shouldn't start with the current global options but with ones
> > we saved for
> > optimize attribute/pragma processing, no?
>
> We hit the issue when we combine cmdline and pragma optimize options.
>
> >
> >> Problem of -gtoggle is that it does not directly influence an option, but it negates it.
> >>
> >> That said, I think my patch with gtoggle_used is a reasonable workaround.
> > Well, then we could as well unset flag_gtoggle after processing it, no?
>
> Yeah, that work! :)
>
> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
>
> Ready to be installed?

OK if you add a comment like

 /* Make sure to process -gtoggle only once.  */

Richard.

> Thanks,
> Martin

Re: [PATCH] Record that -gtoggle is already used in gcc_options.

[Martin Liška]

On 11/5/21 11:23, Richard Biener wrote:
> OK if you add a comment like
> 
>   /* Make sure to process -gtoggle only once.  */

Sure, added and installed as 14c7041a1f00ef4ee9a036e0b369c97646db5b5c.

Cheers,
Martin