Build failure with probe patch

Message ID 1424368119.27855.123.camel@ubuntu-sellcey
State New, archived
Headers

Commit Message

Steve Ellcey Feb. 19, 2015, 5:48 p.m. UTC
  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
  

Comments

Jose E. Marchesi Feb. 19, 2015, 8:46 p.m. UTC | #1
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.
  
Sergio Durigan Junior Feb. 19, 2015, 8:53 p.m. UTC | #2
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.
  

Patch

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);
 	}
     }