[PATCH][gdb] Fix -Wparentheses warnings

[PATCH][gdb] Fix -Wparentheses warnings

[Kyrill Tkachov]

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

Hi all,

I recently moved to using GCC 6.1 and while building a toolchain for arm-none-eabi I got
some -Werror=parentheses errors while building gdb.

This patch fixes them. I think this is due to the way ALL_OBJSECTIONS is defined
(due to using ALL_OBJFILE_OSECTIONS):
#define ALL_OBJFILE_OSECTIONS(objfile, osect)   \
   for (osect = objfile->sections; osect < objfile->sections_end; osect++) \
     if (osect->the_bfd_section == NULL)                                 \
{ \
         /* Nothing. */                                                 \
} \
     else


so that dangling 'else' probably causes the alarm in GCC.

This patch fixes the -Werror errors for me and allows the build to finish successfully.

Is this ok to commit?

Thanks,
Kyrill

2016-04-29  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>

     * symfile.c (find_pc_overlay): Add braces to avoid -Wparentheses
     warning.
     (find_pc_mapped_section): Likewise.
     (list_overlays_command): Likewise.

[-- Attachment #2: gdb-warnings.patch --]
[-- Type: text/x-patch, Size: 3303 bytes --]

diff --git a/gdb/symfile.c b/gdb/symfile.c
index 0d67bfd834594cd68ad1da3aec9e2811dc32bfd8..ca304b8fc9592dbfdbf484a2cac6f2771d570410 100644
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -3285,19 +3285,21 @@ find_pc_overlay (CORE_ADDR pc)
   struct obj_section *osect, *best_match = NULL;
 
   if (overlay_debugging)
-    ALL_OBJSECTIONS (objfile, osect)
-      if (section_is_overlay (osect))
-      {
-	if (pc_in_mapped_range (pc, osect))
+    {
+      ALL_OBJSECTIONS (objfile, osect)
+	if (section_is_overlay (osect))
 	  {
-	    if (section_is_mapped (osect))
-	      return osect;
-	    else
+	    if (pc_in_mapped_range (pc, osect))
+	      {
+		if (section_is_mapped (osect))
+		  return osect;
+		else
+		  best_match = osect;
+	      }
+	    else if (pc_in_unmapped_range (pc, osect))
 	      best_match = osect;
 	  }
-	else if (pc_in_unmapped_range (pc, osect))
-	  best_match = osect;
-      }
+    }
   return best_match;
 }
 
@@ -3312,9 +3314,11 @@ find_pc_mapped_section (CORE_ADDR pc)
   struct obj_section *osect;
 
   if (overlay_debugging)
-    ALL_OBJSECTIONS (objfile, osect)
-      if (pc_in_mapped_range (pc, osect) && section_is_mapped (osect))
-	return osect;
+    {
+      ALL_OBJSECTIONS (objfile, osect)
+	if (pc_in_mapped_range (pc, osect) && section_is_mapped (osect))
+	  return osect;
+    }
 
   return NULL;
 }
@@ -3330,31 +3334,33 @@ list_overlays_command (char *args, int from_tty)
   struct obj_section *osect;
 
   if (overlay_debugging)
-    ALL_OBJSECTIONS (objfile, osect)
+    {
+      ALL_OBJSECTIONS (objfile, osect)
       if (section_is_mapped (osect))
-      {
-	struct gdbarch *gdbarch = get_objfile_arch (objfile);
-	const char *name;
-	bfd_vma lma, vma;
-	int size;
-
-	vma = bfd_section_vma (objfile->obfd, osect->the_bfd_section);
-	lma = bfd_section_lma (objfile->obfd, osect->the_bfd_section);
-	size = bfd_get_section_size (osect->the_bfd_section);
-	name = bfd_section_name (objfile->obfd, osect->the_bfd_section);
-
-	printf_filtered ("Section %s, loaded at ", name);
-	fputs_filtered (paddress (gdbarch, lma), gdb_stdout);
-	puts_filtered (" - ");
-	fputs_filtered (paddress (gdbarch, lma + size), gdb_stdout);
-	printf_filtered (", mapped at ");
-	fputs_filtered (paddress (gdbarch, vma), gdb_stdout);
-	puts_filtered (" - ");
-	fputs_filtered (paddress (gdbarch, vma + size), gdb_stdout);
-	puts_filtered ("\n");
-
-	nmapped++;
-      }
+	{
+	  struct gdbarch *gdbarch = get_objfile_arch (objfile);
+	  const char *name;
+	  bfd_vma lma, vma;
+	  int size;
+
+	  vma = bfd_section_vma (objfile->obfd, osect->the_bfd_section);
+	  lma = bfd_section_lma (objfile->obfd, osect->the_bfd_section);
+	  size = bfd_get_section_size (osect->the_bfd_section);
+	  name = bfd_section_name (objfile->obfd, osect->the_bfd_section);
+
+	  printf_filtered ("Section %s, loaded at ", name);
+	  fputs_filtered (paddress (gdbarch, lma), gdb_stdout);
+	  puts_filtered (" - ");
+	  fputs_filtered (paddress (gdbarch, lma + size), gdb_stdout);
+	  printf_filtered (", mapped at ");
+	  fputs_filtered (paddress (gdbarch, vma), gdb_stdout);
+	  puts_filtered (" - ");
+	  fputs_filtered (paddress (gdbarch, vma + size), gdb_stdout);
+	  puts_filtered ("\n");
+
+	  nmapped++;
+	}
+    }
   if (nmapped == 0)
     printf_filtered (_("No sections are mapped.\n"));
 }

Re: [PATCH][gdb] Fix -Wparentheses warnings

[Pedro Alves]

On 04/29/2016 04:26 PM, Kyrill Tkachov wrote:
> Hi all,
> 
> I recently moved to using GCC 6.1 and while building a toolchain for arm-none-eabi I got
> 
> some -Werror=parentheses errors while building gdb.
> 
> This patch fixes them. I think this is due to the way ALL_OBJSECTIONS is defined
> 
> (due to using ALL_OBJFILE_OSECTIONS):
> #define ALL_OBJFILE_OSECTIONS(objfile, osect)   \
>   for (osect = objfile->sections; osect < objfile->sections_end; osect++) \
>     if (osect->the_bfd_section == NULL)                                 \
> { \
>         /* Nothing. */                                                 \
> } \
>     else
> 
> 
> so that dangling 'else' probably causes the alarm in GCC.

Bah.  That's written that way exactly to avoid dangling if/else
problems.

I think it'd be reasonable for gcc to not warn when the if/else
came from a macro, as users of the macro can't possibly be confused
in the way the warning intents to help with.  I'd call it a
gcc regression.

> 
> This patch fixes the -Werror errors for me and allows the build to finish successfully.
> 
> 
> Is this ok to commit?

OK.

Thanks,
Pedro Alves

Re: [PATCH][gdb] Fix -Wparentheses warnings

[Pedro Alves]

On 05/02/2016 11:57 AM, Pedro Alves wrote:
> On 04/29/2016 04:26 PM, Kyrill Tkachov wrote:

> Bah.  That's written that way exactly to avoid dangling if/else
> problems.
> 
> I think it'd be reasonable for gcc to not warn when the if/else
> came from a macro, as users of the macro can't possibly be confused
> in the way the warning intents to help with.  I'd call it a
> gcc regression.
> 

...

>> Is this ok to commit?
> 
> OK.


I suspect this may be the same as PR20029:

 https://sourceware.org/bugzilla/show_bug.cgi?id=20029

Could you push it to the gdb-7.11-branch branch too, please?

Thanks,
Pedro Alves

Re: [PATCH][gdb] Fix -Wparentheses warnings

[Kyrill Tkachov]

Hi Pedro,

On 02/05/16 18:03, Pedro Alves wrote:
> On 05/02/2016 11:57 AM, Pedro Alves wrote:
>> On 04/29/2016 04:26 PM, Kyrill Tkachov wrote:
>> Bah.  That's written that way exactly to avoid dangling if/else
>> problems.
>>
>> I think it'd be reasonable for gcc to not warn when the if/else
>> came from a macro, as users of the macro can't possibly be confused
>> in the way the warning intents to help with.  I'd call it a
>> gcc regression.
>>

I see what you mean.
I filed GCC bug https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70922
to get the GCC devs opinion.

> ...
>
>>> Is this ok to commit?
>> OK.
>
> I suspect this may be the same as PR20029:
>
>   https://sourceware.org/bugzilla/show_bug.cgi?id=20029
>
> Could you push it to the gdb-7.11-branch branch too, please?

I've pushed the patch to master and gdb-7.11-branch.
I've not used the sourceware bugzilla before (I don't have an account there AFAIK),
so if someone could confirm that the warnings are fixed and close that accordingly
I'd appreciate it.

Kyrill

> Thanks,
> Pedro Alves
>

Re: [PATCH][gdb] Fix -Wparentheses warnings

[Pedro Alves]

On 05/03/2016 10:01 AM, Kyrill Tkachov wrote:
> On 02/05/16 18:03, Pedro Alves wrote:
>> On 05/02/2016 11:57 AM, Pedro Alves wrote:
>>> On 04/29/2016 04:26 PM, Kyrill Tkachov wrote:
>>> Bah.  That's written that way exactly to avoid dangling if/else
>>> problems.
>>>
>>> I think it'd be reasonable for gcc to not warn when the if/else
>>> came from a macro, as users of the macro can't possibly be confused
>>> in the way the warning intents to help with.  I'd call it a
>>> gcc regression.
>>>
> 
> I see what you mean.
> I filed GCC bug https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70922
> to get the GCC devs opinion.

Thanks.  I added some more info there.

>> I suspect this may be the same as PR20029:
>>
>>   https://sourceware.org/bugzilla/show_bug.cgi?id=20029
>>
>> Could you push it to the gdb-7.11-branch branch too, please?
> 
> I've pushed the patch to master and gdb-7.11-branch.

Great, thanks again.

> I've not used the sourceware bugzilla before (I don't have an account
> there AFAIK),
> so if someone could confirm that the warnings are fixed and close that
> accordingly
> I'd appreciate it.

I'll take it from here.

-- 
Pedro Alves