diff mbox

GCC5 build error. unmap_overlay_command doesn't check section_is_overlay.

Message ID 1422106237-20723-1-git-send-email-mjw@redhat.com
State New
Headers show

Commit Message

Mark Wielaard Jan. 24, 2015, 1:30 p.m. UTC
Building with gcc (GCC) 5.0.0 20150123 (experimental) gives the following
error:

In file included from /home/mark/src/binutils-gdb/gdb/symfile.c:32:0:
/home/mark/src/binutils-gdb/gdb/symfile.c: In function ‘unmap_overlay_command’:
/home/mark/src/binutils-gdb/gdb/objfiles.h:628:3: error: ‘sec’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
   for (osect = objfile->sections; osect < objfile->sections_end; osect++) \
   ^
/home/mark/src/binutils-gdb/gdb/symfile.c:3442:23: note: ‘sec’ was declared here
   struct obj_section *sec;
                       ^
cc1: all warnings being treated as errors

This comes from the following construct:

  /* First, find a section matching the user supplied argument.  */
  ALL_OBJSECTIONS (objfile, sec)
    if (!strcmp (bfd_section_name (objfile->obfd, sec->the_bfd_section), args))
    {
      if (!sec->ovly_mapped)
        error (_("Section %s is not mapped"), args);
      sec->ovly_mapped = 0;
      return;
    }
  error (_("No overlay section called %s"), args);

ALL_OBJSECTIONS is defined in gdb/objfiles.h as two nested for statements
that try to make sure that if one breaks from the second for loop the first
for loop is also terminated. The definition is slightly hairy (you might
want to read the comments in objfiles.h that explain it).

  for (osect = objfile->sections; osect < objfile->sections_end; osect++) \
    if (osect->the_bfd_section == NULL)                                 \
      {                                                                 \
        /* Nothing.  */                                                 \
      }                                                                 \
    else

  for ((objfile) = current_program_space->objfiles,                     \
         (objfile) != NULL ? ((osect) = (objfile)->sections_end) : 0;   \
       (objfile) != NULL                                                \
         && (osect) == (objfile)->sections_end;                         \
       ((osect) == (objfile)->sections_end                              \
        ? ((objfile) = (objfile)->next,                                 \
           (objfile) != NULL ? (osect) = (objfile)->sections_end : 0)   \
        : 0))                                                           \
    ALL_OBJFILE_OSECTIONS (objfile, osect)

This is the only ALL_OBJSECTIONS construct that gives this error. The only
difference with the others is that they all contain an (indirect) call to
section_is_overlay (sec). Adding that check here solves the error. Adding
the check also a bit more correct in this case. A non-overlay section
with the given name would otherwise give the error "is not mapped" (only
overlay sections can be mapped), while the correct error would be the
second "No overlay section".

But I am not entirely sure why adding the check solves the issue. It
might be because section_is_overlay does an explicit NULL pointer check.
Without an explicit check for NULL the compiler might decide sec is only
used in a < comparison or by dereferencing, both of which imply sec can
never be NULL (because that would be undefined behavior). So one of the
conditions in the for loops is always true and most of the code can be
optimized away? The code relies on the fact that osect = objfile->sections;
osect < objfile->sections_end; osect++; will eventually end with
(osect) == (objfile)->sections_end. But I am not sure that the C language
pointer rules really guarantee that.
---
 gdb/symfile.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
diff mbox

Patch

diff --git a/gdb/symfile.c b/gdb/symfile.c
index d55e361..5c80892 100644
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -3451,7 +3451,9 @@  unmap_overlay_command (char *args, int from_tty)
 
   /* First, find a section matching the user supplied argument.  */
   ALL_OBJSECTIONS (objfile, sec)
-    if (!strcmp (bfd_section_name (objfile->obfd, sec->the_bfd_section), args))
+    if (section_is_overlay (sec)
+	&& !strcmp (bfd_section_name (objfile->obfd, sec->the_bfd_section),
+		    args))
     {
       if (!sec->ovly_mapped)
 	error (_("Section %s is not mapped"), args);