Build failure with probe patch

Build failure with probe patch

[Steve Ellcey]

Is anyone else seeing this error when building the top-of-tree gdb:


cc1: warnings being treated as errors
/scratch/sellcey/gdb-bug/src/gdb/gdb/dtrace-probe.c: In function 'dtrace_get_probes':
/scratch/sellcey/gdb-bug/src/gdb/gdb/dtrace-probe.c:624: warning: dereferencing type-punned pointer will break strict-aliasing rules
make[1]: *** [dtrace-probe.o] Error 1


I get this when building gdb on a CentOS 5 system with GCC 4.1.2 but I do not
see it when building on a Ubuntu 12.04 system with GCC 4.6.3.  I am not sure
if this is due to the old GCC or if it is a legitimate error.

Steve Ellcey
sellcey@imgtec.com

Re: Build failure with probe patch

[Sergio Durigan Junior]

On Wednesday, February 18 2015, Steve Ellcey wrote:

> Is anyone else seeing this error when building the top-of-tree gdb:
>
>
> cc1: warnings being treated as errors
> /scratch/sellcey/gdb-bug/src/gdb/gdb/dtrace-probe.c: In function 'dtrace_get_probes':
> /scratch/sellcey/gdb-bug/src/gdb/gdb/dtrace-probe.c:624: warning: dereferencing type-punned pointer will break strict-aliasing rules
> make[1]: *** [dtrace-probe.o] Error 1

Thanks for your report.

I don't see anything like this in any of our builders here:

  <http://gdb-build.sergiodj.net/waterfall>

> I get this when building gdb on a CentOS 5 system with GCC 4.1.2 but I do not
> see it when building on a Ubuntu 12.04 system with GCC 4.6.3.  I am not sure
> if this is due to the old GCC or if it is a legitimate error.

According to:

  <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=41874>

This warning has been removed from GCC.  And by looking at the code
referenced by it, I don't see anything wrong there.  So I'd say you can
ignore this (and probably update your GCC).

-- 
Sergio
GPG key ID: 0x65FC5E36
Please send encrypted e-mail if possible
http://sergiodj.net/

Re: Build failure with probe patch

[Steve Ellcey]

On Wed, 2015-02-18 at 18:57 -0500, Sergio Durigan Junior wrote:
> On Wednesday, February 18 2015, Steve Ellcey wrote:
> 
> > Is anyone else seeing this error when building the top-of-tree gdb:
> >
> >
> > cc1: warnings being treated as errors
> > /scratch/sellcey/gdb-bug/src/gdb/gdb/dtrace-probe.c: In function 'dtrace_get_probes':
> > /scratch/sellcey/gdb-bug/src/gdb/gdb/dtrace-probe.c:624: warning: dereferencing type-punned pointer will break strict-aliasing rules
> > make[1]: *** [dtrace-probe.o] Error 1
> 
> Thanks for your report.
> 
> I don't see anything like this in any of our builders here:
> 
>   <http://gdb-build.sergiodj.net/waterfall>
> 
> > I get this when building gdb on a CentOS 5 system with GCC 4.1.2 but I do not
> > see it when building on a Ubuntu 12.04 system with GCC 4.6.3.  I am not sure
> > if this is due to the old GCC or if it is a legitimate error.
> 
> According to:
> 
>   <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=41874>
> 
> This warning has been removed from GCC.  And by looking at the code
> referenced by it, I don't see anything wrong there.  So I'd say you can
> ignore this (and probably update your GCC).
> 

I would like to avoid updating GCC if possible.  I build on old systems
because some of our customers use old systems.  I don't know if gdb has
a 'minimal GCC' that it can be compiled with like GCC and some other
projects have.  I tried changing the definition of 'dof' to be '
bfd_byte *' instead of 'struct dtrace_dof_hdr *' and then casting it on
the call to dtrace_process_dof instead of the call to
bfd_malloc_and_get_section.  That got rid of the type punning message
but I wound up seeing:

/scratch/gcc/nightly/src/binutils-gdb/gdb/dtrace-probe.c: In function
'dtrace_get_probes':
/scratch/gcc/nightly/src/binutils-gdb/gdb/dtrace-probe.c:64: warning:
'arg.expr' is used uninitialized in this function
make[1]: *** [dtrace-probe.o] Error 1

Which I do not understand at all.  I will investigate some more and see
if there is a clean way to get this to compile with GCC 4.1.2.

Steve Ellcey
sellcey@imgtec.com

Re: Build failure with probe patch

[Sergio Durigan Junior]

On Wednesday, February 18 2015, Steve Ellcey wrote:

> I would like to avoid updating GCC if possible.  I build on old systems
> because some of our customers use old systems.  I don't know if gdb has
> a 'minimal GCC' that it can be compiled with like GCC and some other
> projects have.

Sure, it's your option.  I know some people here still use old compilers
to build GDB, but I don't remember of any explicit rule about a minimal
GCC version.

>  I tried changing the definition of 'dof' to be '
> bfd_byte *' instead of 'struct dtrace_dof_hdr *' and then casting it on
> the call to dtrace_process_dof instead of the call to
> bfd_malloc_and_get_section.  That got rid of the type punning message
> but I wound up seeing:
>
> /scratch/gcc/nightly/src/binutils-gdb/gdb/dtrace-probe.c: In function
> 'dtrace_get_probes':
> /scratch/gcc/nightly/src/binutils-gdb/gdb/dtrace-probe.c:64: warning:
> 'arg.expr' is used uninitialized in this function
> make[1]: *** [dtrace-probe.o] Error 1

This error message is completely misleading.  I built GCC 4.1.2 and
managed to reproduce it, but I couldn't find the reason.  Anyway, I have
not spent too much time on the problem, but let me know if you need some
help.

> Which I do not understand at all.  I will investigate some more and see
> if there is a clean way to get this to compile with GCC 4.1.2.

That would be much appreciated.  Thank you!

-- 
Sergio
GPG key ID: 0x65FC5E36
Please send encrypted e-mail if possible
http://sergiodj.net/

Re: Build failure with probe patch

[Jose E. Marchesi]

Hi Steve.

    > According to:
    > 
    >   <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=41874>
    > 
    > This warning has been removed from GCC.  And by looking at the code
    > referenced by it, I don't see anything wrong there.  So I'd say you can
    > ignore this (and probably update your GCC).
    > 
    
    I would like to avoid updating GCC if possible.  I build on old systems
    because some of our customers use old systems.  I don't know if gdb has
    a 'minimal GCC' that it can be compiled with like GCC and some other
    projects have.  I tried changing the definition of 'dof' to be '
    bfd_byte *' instead of 'struct dtrace_dof_hdr *' and then casting it on
    the call to dtrace_process_dof instead of the call to
    bfd_malloc_and_get_section.

I think that version would still break the strict aliasing rules, since
`struct dtrace_dof_hdr' does not alias bfd_byte, even if the latter is a
character type.

As nothing guarantees that some future GCC version will not be bitching
about it, what about using something like this instead?

if (elf_section_data (sect)->this_hdr.sh_type == SHT_SUNW_dof)
  {
    /* Read the contents of the DOF section and then process it to
       extract the information of any probe defined into it.  */

    bfd_size_type size;
    const void *buf = gdb_bfd_map_section (sect, &size);
    dtrace_process_dof (sect, objfile, probesp, (struct dtrace_dof_hdr *) buf);
  }

AFAIU anything can alias a void pointer, as void pointers can't be
dereferenced...

Re: Build failure with probe patch

[Pedro Alves]

On 02/19/2015 04:01 PM, Jose E. Marchesi wrote:
> 
>     > According to:
>     > 
>     >   <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=41874>
>     > 
>     > This warning has been removed from GCC.  And by looking at the code
>     > referenced by it, I don't see anything wrong there.  So I'd say you can
>     > ignore this (and probably update your GCC).
>     > 
>     
>     I would like to avoid updating GCC if possible.  I build on old systems
>     because some of our customers use old systems.  I don't know if gdb has
>     a 'minimal GCC' that it can be compiled with like GCC and some other
>     projects have.  I tried changing the definition of 'dof' to be '
>     bfd_byte *' instead of 'struct dtrace_dof_hdr *' and then casting it on
>     the call to dtrace_process_dof instead of the call to
>     bfd_malloc_and_get_section.
> 
> I think that version would still break the strict aliasing rules, since
> `struct dtrace_dof_hdr' does not alias bfd_byte, even if the latter is a
> character type.

Hmm, I thought that for aliasing, a typedef to char could
alias anything, as typedefs don't really create new types?

> As nothing guarantees that some future GCC version will not be bitching
> about it, what about using something like this instead?
> 
> if (elf_section_data (sect)->this_hdr.sh_type == SHT_SUNW_dof)
>   {
>     /* Read the contents of the DOF section and then process it to
>        extract the information of any probe defined into it.  */
> 
>     bfd_size_type size;
>     const void *buf = gdb_bfd_map_section (sect, &size);
>     dtrace_process_dof (sect, objfile, probesp, (struct dtrace_dof_hdr *) buf);
>   }
> 
> AFAIU anything can alias a void pointer, as void pointers can't be
> dereferenced...
> 

gdb_bfd_map_section may be better for making use of mmap if possible, but
note that even though that might shut up some versions of some compilers,
WRT to strict aliasing, that ends up being no different from Steve's
suggestion, plus with an intermediate void* cast.  gdb_bfd_map_section
also returns a gdb_byte *.   (And with C++, that'll end up needing
an explicit '(void *)' cast.).

Thanks,
Pedro Alves

Re: Build failure with probe patch

[Jose E. Marchesi]

    >     > According to:
    >     > 
    >     >   <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=41874>
    >     > 
    >     > This warning has been removed from GCC.  And by looking at the code
    >     > referenced by it, I don't see anything wrong there.  So I'd say you can
    >     > ignore this (and probably update your GCC).
    >     > 
    >     
    >     I would like to avoid updating GCC if possible.  I build on old systems
    >     because some of our customers use old systems.  I don't know if gdb has
    >     a 'minimal GCC' that it can be compiled with like GCC and some other
    >     projects have.  I tried changing the definition of 'dof' to be '
    >     bfd_byte *' instead of 'struct dtrace_dof_hdr *' and then casting it on
    >     the call to dtrace_process_dof instead of the call to
    >     bfd_malloc_and_get_section.
    > 
    > I think that version would still break the strict aliasing rules, since
    > `struct dtrace_dof_hdr' does not alias bfd_byte, even if the latter is a
    > character type.
    
    Hmm, I thought that for aliasing, a typedef to char could
    alias anything, as typedefs don't really create new types?

Yes, but not the other way around...  it is not allowed to alias a
character type with a non-character type like in the following call to
dtrace_process_dof:

gdb_byte *buf;
[...]
dtrace_process_dof (sect, objfile, probesp, (struct dtrace_dof_hdr *) buf);

    > As nothing guarantees that some future GCC version will not be bitching
    > about it, what about using something like this instead?
    > 
    > if (elf_section_data (sect)->this_hdr.sh_type == SHT_SUNW_dof)
    >   {
    >     /* Read the contents of the DOF section and then process it to
    >        extract the information of any probe defined into it.  */
    > 
    >     bfd_size_type size;
    >     const void *buf = gdb_bfd_map_section (sect, &size);
    >     dtrace_process_dof (sect, objfile, probesp, (struct dtrace_dof_hdr *) buf);
    >   }
    > 
    > AFAIU anything can alias a void pointer, as void pointers can't be
    > dereferenced...
    > 
    
    gdb_bfd_map_section may be better for making use of mmap if possible, but
    note that even though that might shut up some versions of some compilers,
    WRT to strict aliasing, that ends up being no different from Steve's
    suggestion, plus with an intermediate void* cast.  gdb_bfd_map_section
    also returns a gdb_byte *.   (And with C++, that'll end up needing
    an explicit '(void *)' cast.).

I see, void is not allowed to alias a char...  we are doomed! :D

Oh well, I guess we will have to use an union to avoid any possible
strict-aliasing warning...  Can we assume C99 in GDB?

Re: Build failure with probe patch

[Steve Ellcey]

On Thu, 2015-02-19 at 18:37 +0100, Jose E. Marchesi wrote:

> I see, void is not allowed to alias a char...  we are doomed! :D
> 
> Oh well, I guess we will have to use an union to avoid any possible
> strict-aliasing warning...  Can we assume C99 in GDB?

Even with this issue fixed I ran into one other compiler problem.  I get
this error from GCC 4.1.2:


cc1: warnings being treated as errors
/scratch/sellcey/gdb-bug/src/gdb/gdb/dtrace-probe.c: In function 'dtrace_get_probes':
/scratch/sellcey/gdb-bug/src/gdb/gdb/dtrace-probe.c:64: warning: 'arg.expr' is used uninitialized in this function


To fix this problem I changed dtrace_process_dof_probe to have
'arg.expr = NULL' in the loop that stores argument descriptions.  Here
is a complete patch that compiled for me using the old GCC.

Steve Ellcey
sellcey@imgtec.com




diff --git a/gdb/dtrace-probe.c b/gdb/dtrace-probe.c
index a6544ba..fd6ae6e 100644
--- a/gdb/dtrace-probe.c
+++ b/gdb/dtrace-probe.c
@@ -415,6 +415,7 @@ dtrace_process_dof_probe (struct objfile *objfile,
 	  struct dtrace_probe_arg arg;
 	  struct expression *expr;
 
+	  arg.expr = NULL;
 	  arg.type_str = xstrdup (p);
 
 	  /* Use strtab_size as a sentinel.  */
@@ -617,17 +618,17 @@ dtrace_get_probes (VEC (probe_p) **probesp, struct objfile *objfile)
     {
       if (elf_section_data (sect)->this_hdr.sh_type == SHT_SUNW_dof)
 	{
-	  struct dtrace_dof_hdr *dof;
+	  bfd_byte *dof;
 
 	  /* Read the contents of the DOF section and then process it to
 	     extract the information of any probe defined into it.  */
-	  if (!bfd_malloc_and_get_section (abfd, sect, (bfd_byte **) &dof))
+	  if (!bfd_malloc_and_get_section (abfd, sect, &dof))
 	    complaint (&symfile_complaints,
 		       _("could not obtain the contents of"
 			 "section '%s' in objfile `%s'."),
 		       sect->name, abfd->filename);
       
-	  dtrace_process_dof (sect, objfile, probesp, dof);
+	  dtrace_process_dof (sect, objfile, probesp, (struct dtrace_dof_hdr *) dof);
 	  xfree (dof);
 	}
     }

Re: Build failure with probe patch

[Jose E. Marchesi]

    To fix this problem I changed dtrace_process_dof_probe to have
    'arg.expr = NULL' in the loop that stores argument descriptions.  Here
    is a complete patch that compiled for me using the old GCC.   

Ah!  But the warning seems all misplaced, isnt it?  Weird... :D

    diff --git a/gdb/dtrace-probe.c b/gdb/dtrace-probe.c
    index a6544ba..fd6ae6e 100644
    --- a/gdb/dtrace-probe.c
    +++ b/gdb/dtrace-probe.c
    @@ -415,6 +415,7 @@ dtrace_process_dof_probe (struct objfile *objfile,
     	  struct dtrace_probe_arg arg;
     	  struct expression *expr;
     
    +	  arg.expr = NULL;

I would add a comment explaining why that sentence is necessary, as it
is not obvious at all to the casual reader.

          arg.type_str = xstrdup (p);
     
     	  /* Use strtab_size as a sentinel.  */
    @@ -617,17 +618,17 @@ dtrace_get_probes (VEC (probe_p) **probesp, struct objfile *objfile)
         {
           if (elf_section_data (sect)->this_hdr.sh_type == SHT_SUNW_dof)
     	{
    -	  struct dtrace_dof_hdr *dof;
    +	  bfd_byte *dof;
     
     	  /* Read the contents of the DOF section and then process it to
     	     extract the information of any probe defined into it.  */
    -	  if (!bfd_malloc_and_get_section (abfd, sect, (bfd_byte **) &dof))
    +	  if (!bfd_malloc_and_get_section (abfd, sect, &dof))
     	    complaint (&symfile_complaints,
     		       _("could not obtain the contents of"
     			 "section '%s' in objfile `%s'."),
     		       sect->name, abfd->filename);
           
    -	  dtrace_process_dof (sect, objfile, probesp, dof);
    +	  dtrace_process_dof (sect, objfile, probesp, (struct dtrace_dof_hdr *) dof);
     	  xfree (dof);
     	}
         }

This looks good to me.

Re: Build failure with probe patch

[Sergio Durigan Junior]

Hi Steve,

Thanks for the patch.  And thanks Jose and Pedro taking care of this.

On Thursday, February 19 2015, Jose E. Marchesi wrote:

>     To fix this problem I changed dtrace_process_dof_probe to have
>     'arg.expr = NULL' in the loop that stores argument descriptions.  Here
>     is a complete patch that compiled for me using the old GCC.   
>
> Ah!  But the warning seems all misplaced, isnt it?  Weird... :D

Totally :-).

>     diff --git a/gdb/dtrace-probe.c b/gdb/dtrace-probe.c
>     index a6544ba..fd6ae6e 100644
>     --- a/gdb/dtrace-probe.c
>     +++ b/gdb/dtrace-probe.c
>     @@ -415,6 +415,7 @@ dtrace_process_dof_probe (struct objfile *objfile,
>      	  struct dtrace_probe_arg arg;
>      	  struct expression *expr;
>      
>     +	  arg.expr = NULL;
>
> I would add a comment explaining why that sentence is necessary, as it
> is not obvious at all to the casual reader.

Agreed.

>           arg.type_str = xstrdup (p);
>      
>      	  /* Use strtab_size as a sentinel.  */
>     @@ -617,17 +618,17 @@ dtrace_get_probes (VEC (probe_p) **probesp, struct objfile *objfile)
>          {
>            if (elf_section_data (sect)->this_hdr.sh_type == SHT_SUNW_dof)
>      	{
>     -	  struct dtrace_dof_hdr *dof;
>     +	  bfd_byte *dof;
>      
>      	  /* Read the contents of the DOF section and then process it to
>      	     extract the information of any probe defined into it.  */
>     -	  if (!bfd_malloc_and_get_section (abfd, sect, (bfd_byte **) &dof))
>     +	  if (!bfd_malloc_and_get_section (abfd, sect, &dof))
>      	    complaint (&symfile_complaints,
>      		       _("could not obtain the contents of"
>      			 "section '%s' in objfile `%s'."),
>      		       sect->name, abfd->filename);
>            
>     -	  dtrace_process_dof (sect, objfile, probesp, dof);
>     +	  dtrace_process_dof (sect, objfile, probesp, (struct dtrace_dof_hdr *) dof);
>      	  xfree (dof);
>      	}
>          }
>
> This looks good to me.

This is OK with a ChangeLog entry, and after you address Jose's request.

-- 
Sergio
GPG key ID: 0x65FC5E36
Please send encrypted e-mail if possible
http://sergiodj.net/

Re: Build failure with probe patch

[Steve Ellcey]

On Thu, 2015-02-19 at 15:53 -0500, Sergio Durigan Junior wrote:

> >
> > This looks good to me.
> 
> This is OK with a ChangeLog entry, and after you address Jose's request.

OK, here is the patch (and ChangeLog) after I added a comment about
initializing arg.expr.  I will go ahead and check it in later today.

Steve Ellcey
sellcey@imgtec.com


2015-02-19  Steve Ellcey  <sellcey@imgtec.com>

	* dtrace-probe.c (dtrace_process_dof_probe): Initialize arg.expr.
	(dtrace_get_probes) Change dof type.


diff --git a/gdb/dtrace-probe.c b/gdb/dtrace-probe.c
index a6544ba..5b41e6b 100644
--- a/gdb/dtrace-probe.c
+++ b/gdb/dtrace-probe.c
@@ -415,6 +415,9 @@ dtrace_process_dof_probe (struct objfile *objfile,
 	  struct dtrace_probe_arg arg;
 	  struct expression *expr;
 
+	  /* Set arg.expr to ensure all fields in expr are initialized and
+	     the compiler will not warn when arg is used.  */
+	  arg.expr = NULL;
 	  arg.type_str = xstrdup (p);
 
 	  /* Use strtab_size as a sentinel.  */
@@ -617,17 +620,17 @@ dtrace_get_probes (VEC (probe_p) **probesp, struct objfile *objfile)
     {
       if (elf_section_data (sect)->this_hdr.sh_type == SHT_SUNW_dof)
 	{
-	  struct dtrace_dof_hdr *dof;
+	  bfd_byte *dof;
 
 	  /* Read the contents of the DOF section and then process it to
 	     extract the information of any probe defined into it.  */
-	  if (!bfd_malloc_and_get_section (abfd, sect, (bfd_byte **) &dof))
+	  if (!bfd_malloc_and_get_section (abfd, sect, &dof))
 	    complaint (&symfile_complaints,
 		       _("could not obtain the contents of"
 			 "section '%s' in objfile `%s'."),
 		       sect->name, abfd->filename);
       
-	  dtrace_process_dof (sect, objfile, probesp, dof);
+	  dtrace_process_dof (sect, objfile, probesp, (struct dtrace_dof_hdr *) dof);
 	  xfree (dof);
 	}
     }

Re: Build failure with probe patch

[Sergio Durigan Junior]

On Thursday, February 19 2015, Steve Ellcey wrote:

> On Thu, 2015-02-19 at 15:53 -0500, Sergio Durigan Junior wrote:
>
>> >
>> > This looks good to me.
>> 
>> This is OK with a ChangeLog entry, and after you address Jose's request.
>
> OK, here is the patch (and ChangeLog) after I added a comment about
> initializing arg.expr.  I will go ahead and check it in later today.

Thanks, Steve.  Small nits below.

> Steve Ellcey
> sellcey@imgtec.com
>
>
> 2015-02-19  Steve Ellcey  <sellcey@imgtec.com>
>
> 	* dtrace-probe.c (dtrace_process_dof_probe): Initialize arg.expr.
> 	(dtrace_get_probes) Change dof type.

I prefer "Change type of variable 'dof'".

>
>
> diff --git a/gdb/dtrace-probe.c b/gdb/dtrace-probe.c
> index a6544ba..5b41e6b 100644
> --- a/gdb/dtrace-probe.c
> +++ b/gdb/dtrace-probe.c
> @@ -415,6 +415,9 @@ dtrace_process_dof_probe (struct objfile *objfile,
>  	  struct dtrace_probe_arg arg;
>  	  struct expression *expr;
>  
> +	  /* Set arg.expr to ensure all fields in expr are initialized and
> +	     the compiler will not warn when arg is used.  */
> +	  arg.expr = NULL;
>  	  arg.type_str = xstrdup (p);
>  
>  	  /* Use strtab_size as a sentinel.  */
> @@ -617,17 +620,17 @@ dtrace_get_probes (VEC (probe_p) **probesp, struct objfile *objfile)
>      {
>        if (elf_section_data (sect)->this_hdr.sh_type == SHT_SUNW_dof)
>  	{
> -	  struct dtrace_dof_hdr *dof;
> +	  bfd_byte *dof;
>  
>  	  /* Read the contents of the DOF section and then process it to
>  	     extract the information of any probe defined into it.  */
> -	  if (!bfd_malloc_and_get_section (abfd, sect, (bfd_byte **) &dof))
> +	  if (!bfd_malloc_and_get_section (abfd, sect, &dof))
>  	    complaint (&symfile_complaints,
>  		       _("could not obtain the contents of"
>  			 "section '%s' in objfile `%s'."),
>  		       sect->name, abfd->filename);
>        
> -	  dtrace_process_dof (sect, objfile, probesp, dof);
> +	  dtrace_process_dof (sect, objfile, probesp, (struct dtrace_dof_hdr *) dof);

This line is too long.  Break it after "probesp,".

>  	  xfree (dof);
>  	}
>      }

OK with those changes.

Thanks,

-- 
Sergio
GPG key ID: 0x65FC5E36
Please send encrypted e-mail if possible
http://sergiodj.net/

Re: Build failure with probe patch

[Steve Ellcey]

On Thu, 2015-02-19 at 16:52 -0500, Sergio Durigan Junior wrote:

> > 2015-02-19  Steve Ellcey  <sellcey@imgtec.com>
> >
> > 	* dtrace-probe.c (dtrace_process_dof_probe): Initialize arg.expr.
> > 	(dtrace_get_probes) Change dof type.
> 
> I prefer "Change type of variable 'dof'".

Ok.

> > -	  dtrace_process_dof (sect, objfile, probesp, dof);
> > +	  dtrace_process_dof (sect, objfile, probesp, (struct dtrace_dof_hdr *) dof);
> 
> This line is too long.  Break it after "probesp,".

OK, I will break that line up (should have seen that myself).

Steve Ellcey
sellcey@imgtec

Re: Build failure with probe patch

[Pedro Alves]

On 02/19/2015 05:37 PM, Jose E. Marchesi wrote:
> 
>     >     > According to:
>     >     > 
>     >     >   <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=41874>
>     >     > 
>     >     > This warning has been removed from GCC.  And by looking at the code
>     >     > referenced by it, I don't see anything wrong there.  So I'd say you can
>     >     > ignore this (and probably update your GCC).
>     >     > 
>     >     
>     >     I would like to avoid updating GCC if possible.  I build on old systems
>     >     because some of our customers use old systems.  I don't know if gdb has
>     >     a 'minimal GCC' that it can be compiled with like GCC and some other
>     >     projects have.  I tried changing the definition of 'dof' to be '
>     >     bfd_byte *' instead of 'struct dtrace_dof_hdr *' and then casting it on
>     >     the call to dtrace_process_dof instead of the call to
>     >     bfd_malloc_and_get_section.
>     > 
>     > I think that version would still break the strict aliasing rules, since
>     > `struct dtrace_dof_hdr' does not alias bfd_byte, even if the latter is a
>     > character type.
>     
>     Hmm, I thought that for aliasing, a typedef to char could
>     alias anything, as typedefs don't really create new types?
> 
> Yes, but not the other way around...  it is not allowed to alias a
> character type with a non-character type like in the following call to
> dtrace_process_dof:

Ah, right.

> gdb_byte *buf;
> [...]
> dtrace_process_dof (sect, objfile, probesp, (struct dtrace_dof_hdr *) buf);

Hmm, still, but I think this may be alright.

gdb_bfd_map_section and bfd_get_full_section_contents return
allocated objects, which have no declared type to begin with
(C99 6.5/6, footnote 75).

Then per C99 6.5/6, since the allocated object is never written
through an lvalue with a type that is not a character type, then
the effective type of the object is "simply the type of the
lvalue used for the access."

So it seems to me that casting to struct dtrace_dof_hdr * and then
reading from that lvalue is OK: from that point on, the object
pointed at by BUF has effective type struct dtrace_dof_hdr.

Thanks,
Pedro Alves

Re: Build failure with probe patch

[Jose E. Marchesi]

    > Yes, but not the other way around...  it is not allowed to alias a
    > character type with a non-character type like in the following call to
    > dtrace_process_dof:
    
    Ah, right.
    
    > gdb_byte *buf;
    > [...]
    > dtrace_process_dof (sect, objfile, probesp, (struct dtrace_dof_hdr *) buf);
    
    Hmm, still, but I think this may be alright.
    
    gdb_bfd_map_section and bfd_get_full_section_contents return
    allocated objects, which have no declared type to begin with
    (C99 6.5/6, footnote 75).
    
    Then per C99 6.5/6, since the allocated object is never written
    through an lvalue with a type that is not a character type, then
    the effective type of the object is "simply the type of the
    lvalue used for the access."
    
    So it seems to me that casting to struct dtrace_dof_hdr * and then
    reading from that lvalue is OK: from that point on, the object
    pointed at by BUF has effective type struct dtrace_dof_hdr.

Good point!  Yes, I agree.  Both bfd_byte and gdb_byte are character
types, so the object returned by either bfd_malloc_and_get_section or
gdb_bfd_map_section must retain the property of not having a declared
type.

Re: Build failure with probe patch

[Pedro Alves]

On 02/18/2015 11:57 PM, Sergio Durigan Junior wrote:
> On Wednesday, February 18 2015, Steve Ellcey wrote:
> 
>> Is anyone else seeing this error when building the top-of-tree gdb:
>>
>>
>> cc1: warnings being treated as errors
>> /scratch/sellcey/gdb-bug/src/gdb/gdb/dtrace-probe.c: In function 'dtrace_get_probes':
>> /scratch/sellcey/gdb-bug/src/gdb/gdb/dtrace-probe.c:624: warning: dereferencing type-punned pointer will break strict-aliasing rules
>> make[1]: *** [dtrace-probe.o] Error 1
> 
> Thanks for your report.
> 
> I don't see anything like this in any of our builders here:
> 
>   <http://gdb-build.sergiodj.net/waterfall>
> 
>> I get this when building gdb on a CentOS 5 system with GCC 4.1.2 but I do not
>> see it when building on a Ubuntu 12.04 system with GCC 4.6.3.  I am not sure
>> if this is due to the old GCC or if it is a legitimate error.
> 
> According to:
> 
>   <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=41874>
> 
> This warning has been removed from GCC.  And by looking at the code
> referenced by it, I don't see anything wrong there.  So I'd say you can
> ignore this (and probably update your GCC).
> 

Actually, the code in that GCC bug is valid (char can alias anything),
but the GDB code being warned about is invalid (C99/N1256, 6.5/7) -
the dtrace code is aliasing pointers of incompatible types.

See:
  https://gcc.gnu.org/ml/gcc-help/2013-03/msg00118.html

I fixed up all similar cases I found in the GDB codebase a while ago:

 https://sourceware.org/ml/gdb-patches/2013-03/msg00449.html
 https://sourceware.org/ml/gdb-patches/2013-03/msg00578.html

Compiling with -Wstrict-aliasing=1 shows that dtrace-probe.c warning
with recent GCC's too, but then again, it also dumps a truckload
of false positives.

Thanks,
Pedro Alves