[patch+7.9] compile: Filter out -fpreprocessed

[patch+7.9] compile: Filter out -fpreprocessed

[Jan Kratochvil]

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

Hi,

with global system gcc-5.0 if one also installs ccache (needing a different
patch for -fplugin=libcc1plugin) it breaks as GDB will read from inferior
DW_AT_producer containing -fpreprocessed (due to ccache used to compile the
inferior).
    <c>   DW_AT_producer    : (indirect string, offset: 0x52): GNU C11 5.0.0 20150114 (Red Hat 5.0.0-0.1) -fpreprocessed -mtune=generic -march=x86-64 -g

It is wrong that gcc puts -fpreprocessed into DW_AT_producer - I may post a gcc
patch for it.  But even if it gets accepted there are already built inferiors
out there which GDB could be compatible (for the 'compile' mode) with.

gdb.compile/*.exp PASSes for it on {x86_64,x86_64-m32}-fedora22pre-linux-gnu.


Thanks,
Jan

[-- Attachment #2: filter.patch --]
[-- Type: text/plain, Size: 1376 bytes --]

gdb/ChangeLog
2015-01-16  Jan Kratochvil  <jan.kratochvil@redhat.com>

	Filter out inferior gcc option -fpreprocessed.
	* compile/compile.c (filter_args): New function.
	(get_args): Use it.

diff --git a/gdb/compile/compile.c b/gdb/compile/compile.c
index ccac49d..ecbd15c 100644
--- a/gdb/compile/compile.c
+++ b/gdb/compile/compile.c
@@ -324,6 +324,26 @@ get_selected_pc_producer_options (void)
   return cs;
 }
 
+/* Filter out unwanted options from *ARGCP and ARGV.  */
+
+static void
+filter_args (int *argcp, char **argv)
+{
+  char **destv;
+
+  for (destv = argv; *argv != NULL; argv++)
+    {
+      /* -fpreprocessed may get in commonly from ccache.  */
+      if (strcmp (*argv, "-fpreprocessed") == 0)
+	{
+	  (*argcp)--;
+	  continue;
+	}
+      *destv++ = *argv;
+    }
+  *destv = NULL;
+}
+
 /* Produce final vector of GCC compilation options.  First element is target
    size ("-m64", "-m32" etc.), optionally followed by DW_AT_producer options
    and then compile-args string GDB variable.  */
@@ -346,6 +366,7 @@ get_args (const struct compile_instance *compiler, struct gdbarch *gdbarch,
       char **argv_producer;
 
       build_argc_argv (cs_producer_options, &argc_producer, &argv_producer);
+      filter_args (&argc_producer, argv_producer);
       append_args (argcp, argvp, argc_producer, argv_producer);
       freeargv (argv_producer);
     }

[patchv2+7.9] compile: Filter out -fpreprocessed

[Jan Kratochvil]

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

Hi,

fix-up, I forgot in C one had to free() (or xfree()) some of the strings.


Jan

[-- Attachment #2: filter2.patch --]
[-- Type: text/plain, Size: 1395 bytes --]

gdb/ChangeLog
2015-01-17  Jan Kratochvil  <jan.kratochvil@redhat.com>

	Filter out inferior gcc option -fpreprocessed.
	* compile/compile.c (filter_args): New function.
	(get_args): Use it.

diff --git a/gdb/compile/compile.c b/gdb/compile/compile.c
index ccac49d..6cd401d 100644
--- a/gdb/compile/compile.c
+++ b/gdb/compile/compile.c
@@ -324,6 +324,27 @@ get_selected_pc_producer_options (void)
   return cs;
 }
 
+/* Filter out unwanted options from *ARGCP and ARGV.  */
+
+static void
+filter_args (int *argcp, char **argv)
+{
+  char **destv;
+
+  for (destv = argv; *argv != NULL; argv++)
+    {
+      /* -fpreprocessed may get in commonly from ccache.  */
+      if (strcmp (*argv, "-fpreprocessed") == 0)
+	{
+	  xfree (*argv);
+	  (*argcp)--;
+	  continue;
+	}
+      *destv++ = *argv;
+    }
+  *destv = NULL;
+}
+
 /* Produce final vector of GCC compilation options.  First element is target
    size ("-m64", "-m32" etc.), optionally followed by DW_AT_producer options
    and then compile-args string GDB variable.  */
@@ -346,6 +367,7 @@ get_args (const struct compile_instance *compiler, struct gdbarch *gdbarch,
       char **argv_producer;
 
       build_argc_argv (cs_producer_options, &argc_producer, &argv_producer);
+      filter_args (&argc_producer, argv_producer);
       append_args (argcp, argvp, argc_producer, argv_producer);
       freeargv (argv_producer);
     }

Re: [patchv2+7.9] compile: Filter out -fpreprocessed

[Joel Brobecker]

> gdb/ChangeLog
> 2015-01-17  Jan Kratochvil  <jan.kratochvil@redhat.com>
> 
> 	Filter out inferior gcc option -fpreprocessed.
> 	* compile/compile.c (filter_args): New function.
> 	(get_args): Use it.

Looks good to me; and OK for both master and branch.

Thank you,
-- 
Joel

[commit+7.9] [patchv2+7.9] compile: Filter out -fpreprocessed

[Jan Kratochvil]

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

On Tue, 03 Feb 2015 08:56:39 +0100, Joel Brobecker wrote:
> > gdb/ChangeLog
> > 2015-01-17  Jan Kratochvil  <jan.kratochvil@redhat.com>
> > 
> > 	Filter out inferior gcc option -fpreprocessed.
> > 	* compile/compile.c (filter_args): New function.
> > 	(get_args): Use it.
> 
> Looks good to me; and OK for both master and branch.

Checked in:
	a7606d8083c9e217294f6e47a8d2903716c6337c master
	c8b16901e05a15e018394ecefe7348c94b43a4f8 7.9
	https://sourceware.org/gdb/wiki/GDB_7.9_Release?action=diff&rev2=27&rev1=26

In the meantime ccache fix has been submitted (no reply) and gcc fix has been
posted+checked-in as referenced in the attached GDB commit.


Thanks,
Jan

[-- Attachment #2: Type: message/rfc822, Size: 2826 bytes --]

From: Jan Kratochvil <jan.kratochvil@redhat.com>
Subject: [PATCH] compile: Filter out -fpreprocessed
Date: Tue, 3 Feb 2015 18:17:02 +0100

With global system gcc-5.0 if one also installs ccache (needing a different
patch
	https://bugzilla.samba.org/show_bug.cgi?id=11060
for -fplugin=libcc1plugin) it breaks as GDB will read from inferior
DW_AT_producer containing -fpreprocessed (due to ccache used to compile the
inferior).
    <c>   DW_AT_producer    : (indirect string, offset: 0x52): GNU C11 5.0.0 20150114 (Red Hat 5.0.0-0.1) -fpreprocessed -mtune=generic -
march=x86-64 -g

It is wrong that gcc puts -fpreprocessed into DW_AT_producer - fixed it in
trunk GCCs:
	https://gcc.gnu.org/ml/gcc-patches/2015-01/msg01495.html
But even with that fix there are already built inferiors out there which GDB
could be compatible (for the 'compile' mode) with.

gdb/ChangeLog
2015-02-03  Jan Kratochvil  <jan.kratochvil@redhat.com>

	Filter out inferior gcc option -fpreprocessed.
	* compile/compile.c (filter_args): New function.
	(get_args): Use it.
---
 gdb/ChangeLog         |  6 ++++++
 gdb/compile/compile.c | 22 ++++++++++++++++++++++
 2 files changed, 28 insertions(+)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 44dc1ea..2266c11 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,9 @@
+2015-02-03  Jan Kratochvil  <jan.kratochvil@redhat.com>
+
+	Filter out inferior gcc option -fpreprocessed.
+	* compile/compile.c (filter_args): New function.
+	(get_args): Use it.
+
 2015-02-03  Pedro Alves  <palves@redhat.com>
 
 	* event-loop.c: Don't declare nor define a queue type for
diff --git a/gdb/compile/compile.c b/gdb/compile/compile.c
index 70b6d44..c204a13 100644
--- a/gdb/compile/compile.c
+++ b/gdb/compile/compile.c
@@ -324,6 +324,27 @@ get_selected_pc_producer_options (void)
   return cs;
 }
 
+/* Filter out unwanted options from *ARGCP and ARGV.  */
+
+static void
+filter_args (int *argcp, char **argv)
+{
+  char **destv;
+
+  for (destv = argv; *argv != NULL; argv++)
+    {
+      /* -fpreprocessed may get in commonly from ccache.  */
+      if (strcmp (*argv, "-fpreprocessed") == 0)
+	{
+	  xfree (*argv);
+	  (*argcp)--;
+	  continue;
+	}
+      *destv++ = *argv;
+    }
+  *destv = NULL;
+}
+
 /* Produce final vector of GCC compilation options.  First element is target
    size ("-m64", "-m32" etc.), optionally followed by DW_AT_producer options
    and then compile-args string GDB variable.  */
@@ -346,6 +367,7 @@ get_args (const struct compile_instance *compiler, struct gdbarch *gdbarch,
       char **argv_producer;
 
       build_argc_argv (cs_producer_options, &argc_producer, &argv_producer);
+      filter_args (&argc_producer, argv_producer);
       append_args (argcp, argvp, argc_producer, argv_producer);
       freeargv (argv_producer);
     }
-- 
2.1.0

Re: [patch+7.9] compile: Filter out -fpreprocessed

[Doug Evans]

On Fri, Jan 16, 2015 at 2:42 PM, Jan Kratochvil
<jan.kratochvil@redhat.com> wrote:
> [...]
> It is wrong that gcc puts -fpreprocessed into DW_AT_producer - I may post a gcc
> patch for it.

Hi.
I wasn't aware there are now rules for what can and cannot go in DW_AT_producer.
DW_AT_producer has gone from being informational to having a formal
spec (in the sense that something will break if, for example, a
particular option is mentioned).
Is this spec written down somewhere? [At least guidelines for what
things may lead to breakage?]

Re: [patch+7.9] compile: Filter out -fpreprocessed

[Jan Kratochvil]

On Tue, 03 Feb 2015 19:50:40 +0100, Doug Evans wrote:
> On Fri, Jan 16, 2015 at 2:42 PM, Jan Kratochvil
> <jan.kratochvil@redhat.com> wrote:
> > [...]
> > It is wrong that gcc puts -fpreprocessed into DW_AT_producer - I may post a gcc
> > patch for it.
> 
> Hi.
> I wasn't aware there are now rules for what can and cannot go in DW_AT_producer.
> DW_AT_producer has gone from being informational to having a formal
> spec (in the sense that something will break if, for example, a
> particular option is mentioned).
> Is this spec written down somewhere? [At least guidelines for what
> things may lead to breakage?]

No. Do you have a suggestion where to put it? Should it be only a GNU
extension or should it be even DWARF-standardized?


Jan

Re: [patch+7.9] compile: Filter out -fpreprocessed

[Mark Wielaard]

On Tue, 2015-02-03 at 19:59 +0100, Jan Kratochvil wrote:
> On Tue, 03 Feb 2015 19:50:40 +0100, Doug Evans wrote:
> > On Fri, Jan 16, 2015 at 2:42 PM, Jan Kratochvil
> > <jan.kratochvil@redhat.com> wrote:
> > > [...]
> > > It is wrong that gcc puts -fpreprocessed into DW_AT_producer - I may post a gcc
> > > patch for it.
> > 
> > I wasn't aware there are now rules for what can and cannot go in DW_AT_producer.
> > DW_AT_producer has gone from being informational to having a formal
> > spec (in the sense that something will break if, for example, a
> > particular option is mentioned).
> > Is this spec written down somewhere? [At least guidelines for what
> > things may lead to breakage?]
> 
> No. Do you have a suggestion where to put it? Should it be only a GNU
> extension or should it be even DWARF-standardized?

The gcc documentation describes it:
https://gcc.gnu.org/onlinedocs/gcc/Debugging-Options.html

-grecord-gcc-switches
        This switch causes the command-line options used to invoke the
        compiler that may affect code generation to be appended to the
        DW_AT_producer attribute in DWARF debugging information. The
        options are concatenated with spaces separating them from each
        other and from the compiler version. See also
        -frecord-gcc-switches for another way of storing compiler
        options into the object file. This is the default.

-gno-record-gcc-switches
        Disallow appending command-line options to the DW_AT_producer
        attribute in DWARF debugging information.

So Jan is right that gcc adding -fpreprocessed, which doesn't affect
code generation, but is a preprocessor option, shouldn't be there.

Cheers,

Mark

Re: [patch+7.9] compile: Filter out -fpreprocessed

[Doug Evans]

On Tue, Feb 3, 2015 at 11:10 AM, Mark Wielaard <mjw@redhat.com> wrote:
> On Tue, 2015-02-03 at 19:59 +0100, Jan Kratochvil wrote:
>> On Tue, 03 Feb 2015 19:50:40 +0100, Doug Evans wrote:
>> > On Fri, Jan 16, 2015 at 2:42 PM, Jan Kratochvil
>> > <jan.kratochvil@redhat.com> wrote:
>> > > [...]
>> > > It is wrong that gcc puts -fpreprocessed into DW_AT_producer - I may post a gcc
>> > > patch for it.
>> >
>> > I wasn't aware there are now rules for what can and cannot go in DW_AT_producer.
>> > DW_AT_producer has gone from being informational to having a formal
>> > spec (in the sense that something will break if, for example, a
>> > particular option is mentioned).
>> > Is this spec written down somewhere? [At least guidelines for what
>> > things may lead to breakage?]
>>
>> No. Do you have a suggestion where to put it? Should it be only a GNU
>> extension or should it be even DWARF-standardized?
>
> The gcc documentation describes it:
> https://gcc.gnu.org/onlinedocs/gcc/Debugging-Options.html
>
> -grecord-gcc-switches
>         This switch causes the command-line options used to invoke the
>         compiler that may affect code generation to be appended to the
>         DW_AT_producer attribute in DWARF debugging information. The
>         options are concatenated with spaces separating them from each
>         other and from the compiler version. See also
>         -frecord-gcc-switches for another way of storing compiler
>         options into the object file. This is the default.
>
> -gno-record-gcc-switches
>         Disallow appending command-line options to the DW_AT_producer
>         attribute in DWARF debugging information.
>
> So Jan is right that gcc adding -fpreprocessed, which doesn't affect
> code generation, but is a preprocessor option, shouldn't be there.

Thanks.

Still, there's no hint to the reader that things may break if certain rules
are not followed. It still seems like it's for informational purposes for
human readers, with no suggestion that programs use this information too.

[For completeness sake, I'm setting aside the compiler and version string.
That seemed common enough knowledge to not need documentation
as much as this does.  I realize we're now talking about -grecord-gcc-switches,
but this thread was originally about DW_AT_producer in general.]