public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: Alan Modra <amodra@gmail.com>
To: binutils@sourceware.org, gdb-patches@sourceware.org
Subject: gdb: warn unused result for bfd IO functions
Date: Thu, 10 Aug 2023 13:06:41 +0930	[thread overview]
Message-ID: <ZNRbSREoB52gfDWx@squeak.grove.modra.org> (raw)
In-Reply-To: <ZNRamusOvj0f+x5T@squeak.grove.modra.org>

From 3dd00a9f25bc78028ba4b3820b45d34f51c4a25d Mon Sep 17 00:00:00 2001
From: Alan Modra <amodra@gmail.com>
Date: Wed, 9 Aug 2023 09:58:36 +0930
Subject: 

This fixes the compilation warnings introduced by my bfdio.c patch.

The removed bfd_seeks in coff_symfile_read date back to 1994, commit
7f4c859520, prior to which the file used stdio rather than bfd to read
symbols.  Since it now uses bfd to read the file there should be no
need to synchronise to bfd's idea of the file position.  I also fixed
a potential uninitialised memory access.

OK to apply?

diff --git a/gdb/coff-pe-read.c b/gdb/coff-pe-read.c
index b82b43c84cf..34360eb72f1 100644
--- a/gdb/coff-pe-read.c
+++ b/gdb/coff-pe-read.c
@@ -254,23 +254,31 @@ read_pe_truncate_name (char *dll_name)
 \f
 /* Low-level support functions, direct from the ld module pe-dll.c.  */
 static unsigned int
-pe_get16 (bfd *abfd, int where)
+pe_get16 (bfd *abfd, int where, bool *fail)
 {
   unsigned char b[2];
 
-  bfd_seek (abfd, (file_ptr) where, SEEK_SET);
-  bfd_read (b, (bfd_size_type) 2, abfd);
+  if (bfd_seek (abfd, where, SEEK_SET) != 0
+      || bfd_read (b, 2, abfd) != 2)
+    {
+      *fail = true;
+      return 0;
+    }
   return b[0] + (b[1] << 8);
 }
 
 static unsigned int
-pe_get32 (bfd *abfd, int where)
+pe_get32 (bfd *abfd, int where, bool *fail)
 {
   unsigned char b[4];
 
-  bfd_seek (abfd, (file_ptr) where, SEEK_SET);
-  bfd_read (b, (bfd_size_type) 4, abfd);
-  return b[0] + (b[1] << 8) + (b[2] << 16) + (b[3] << 24);
+  if (bfd_seek (abfd, where, SEEK_SET) != 0
+      || bfd_read (b, 4, abfd) != 4)
+    {
+      *fail = true;
+      return 0;
+    }
+  return b[0] + (b[1] << 8) + (b[2] << 16) + ((unsigned) b[3] << 24);
 }
 
 static unsigned int
@@ -286,7 +294,7 @@ pe_as32 (void *ptr)
 {
   unsigned char *b = (unsigned char *) ptr;
 
-  return b[0] + (b[1] << 8) + (b[2] << 16) + (b[3] << 24);
+  return b[0] + (b[1] << 8) + (b[2] << 16) + ((unsigned) b[3] << 24);
 }
 \f
 /* Read the (non-debug) export symbol table from a portable
@@ -340,32 +348,46 @@ read_pe_exported_syms (minimal_symbol_reader &reader,
       /* This is not a recognized PE format file.  Abort now, because
 	 the code is untested on anything else.  *FIXME* test on
 	 further architectures and loosen or remove this test.  */
+    notdll:
+      if (debug_coff_pe_read)
+	gdb_printf (gdb_stdlog, _("%s doesn't appear to be a DLL\n"),
+		    bfd_get_filename (dll));
       return;
     }
 
   /* Get pe_header, optional header and numbers of export entries.  */
-  pe_header_offset = pe_get32 (dll, 0x3c);
+  bool fail = false;
+  pe_header_offset = pe_get32 (dll, 0x3c, &fail);
+  if (fail)
+    goto notdll;
   opthdr_ofs = pe_header_offset + 4 + 20;
   if (is_pe64)
-    num_entries = pe_get32 (dll, opthdr_ofs + 108);
+    num_entries = pe_get32 (dll, opthdr_ofs + 108, &fail);
   else
-    num_entries = pe_get32 (dll, opthdr_ofs + 92);
+    num_entries = pe_get32 (dll, opthdr_ofs + 92, &fail);
+  if (fail)
+    goto notdll;
 
   if (num_entries < 1)		/* No exports.  */
     return;
   if (is_pe64)
     {
-      export_opthdrrva = pe_get32 (dll, opthdr_ofs + 112);
-      export_opthdrsize = pe_get32 (dll, opthdr_ofs + 116);
+      export_opthdrrva = pe_get32 (dll, opthdr_ofs + 112, &fail);
+      export_opthdrsize = pe_get32 (dll, opthdr_ofs + 116, &fail);
     }
   else
     {
-      export_opthdrrva = pe_get32 (dll, opthdr_ofs + 96);
-      export_opthdrsize = pe_get32 (dll, opthdr_ofs + 100);
+      export_opthdrrva = pe_get32 (dll, opthdr_ofs + 96, &fail);
+      export_opthdrsize = pe_get32 (dll, opthdr_ofs + 100, &fail);
     }
-  nsections = pe_get16 (dll, pe_header_offset + 4 + 2);
+  if (fail)
+    goto notdll;
+
+  nsections = pe_get16 (dll, pe_header_offset + 4 + 2, &fail);
   secptr = (pe_header_offset + 4 + 20 +
-	    pe_get16 (dll, pe_header_offset + 4 + 16));
+	    pe_get16 (dll, pe_header_offset + 4 + 16, &fail));
+  if (fail)
+    goto notdll;
   expptr = 0;
   export_size = 0;
 
@@ -374,12 +396,13 @@ read_pe_exported_syms (minimal_symbol_reader &reader,
     {
       char sname[8];
       unsigned long secptr1 = secptr + 40 * i;
-      unsigned long vaddr = pe_get32 (dll, secptr1 + 12);
-      unsigned long vsize = pe_get32 (dll, secptr1 + 16);
-      unsigned long fptr = pe_get32 (dll, secptr1 + 20);
+      unsigned long vaddr = pe_get32 (dll, secptr1 + 12, &fail);
+      unsigned long vsize = pe_get32 (dll, secptr1 + 16, &fail);
+      unsigned long fptr = pe_get32 (dll, secptr1 + 20, &fail);
 
-      bfd_seek (dll, (file_ptr) secptr1, SEEK_SET);
-      bfd_read (sname, (bfd_size_type) sizeof (sname), dll);
+      if (fail
+	  || bfd_seek (dll, secptr1, SEEK_SET) != 0
+	  || bfd_read (sname, sizeof (sname), dll) != sizeof (sname))
 
       if ((strcmp (sname, ".edata") == 0)
 	  || (vaddr <= export_opthdrrva && export_opthdrrva < vaddr + vsize))
@@ -420,16 +443,18 @@ read_pe_exported_syms (minimal_symbol_reader &reader,
   for (i = 0; i < nsections; i++)
     {
       unsigned long secptr1 = secptr + 40 * i;
-      unsigned long vsize = pe_get32 (dll, secptr1 + 8);
-      unsigned long vaddr = pe_get32 (dll, secptr1 + 12);
-      unsigned long characteristics = pe_get32 (dll, secptr1 + 36);
+      unsigned long vsize = pe_get32 (dll, secptr1 + 8, &fail);
+      unsigned long vaddr = pe_get32 (dll, secptr1 + 12, &fail);
+      unsigned long characteristics = pe_get32 (dll, secptr1 + 36, &fail);
       char sec_name[SCNNMLEN + 1];
       int sectix;
       unsigned int bfd_section_index;
       asection *section;
 
-      bfd_seek (dll, (file_ptr) secptr1 + 0, SEEK_SET);
-      bfd_read (sec_name, (bfd_size_type) SCNNMLEN, dll);
+      if (fail
+	  || bfd_seek (dll, secptr1 + 0, SEEK_SET) != 0
+	  || bfd_read (sec_name, SCNNMLEN, dll) != SCNNMLEN)
+	goto notdll;
       sec_name[SCNNMLEN] = '\0';
 
       sectix = read_pe_section_index (sec_name);
@@ -468,8 +493,9 @@ read_pe_exported_syms (minimal_symbol_reader &reader,
   gdb::def_vector<unsigned char> expdata_storage (export_size);
   expdata = expdata_storage.data ();
 
-  bfd_seek (dll, (file_ptr) expptr, SEEK_SET);
-  bfd_read (expdata, (bfd_size_type) export_size, dll);
+  if (bfd_seek (dll, expptr, SEEK_SET) != 0
+      || bfd_read (expdata, export_size, dll) != export_size)
+    goto notdll;
   erva = expdata - export_rva;
 
   nexp = pe_as32 (expdata + 24);
@@ -626,20 +652,27 @@ pe_text_section_offset (struct bfd *abfd)
     }
 
   /* Get pe_header, optional header and numbers of sections.  */
-  pe_header_offset = pe_get32 (abfd, 0x3c);
-  nsections = pe_get16 (abfd, pe_header_offset + 4 + 2);
+  bool fail = false;
+  pe_header_offset = pe_get32 (abfd, 0x3c, &fail);
+  if (fail)
+    return DEFAULT_COFF_PE_TEXT_SECTION_OFFSET;
+  nsections = pe_get16 (abfd, pe_header_offset + 4 + 2, &fail);
   secptr = (pe_header_offset + 4 + 20 +
-	    pe_get16 (abfd, pe_header_offset + 4 + 16));
+	    pe_get16 (abfd, pe_header_offset + 4 + 16, &fail));
+  if (fail)
+    return DEFAULT_COFF_PE_TEXT_SECTION_OFFSET;
 
   /* Get the rva and size of the export section.  */
   for (i = 0; i < nsections; i++)
     {
       char sname[SCNNMLEN + 1];
       unsigned long secptr1 = secptr + 40 * i;
-      unsigned long vaddr = pe_get32 (abfd, secptr1 + 12);
+      unsigned long vaddr = pe_get32 (abfd, secptr1 + 12, &fail);
 
-      bfd_seek (abfd, (file_ptr) secptr1, SEEK_SET);
-      bfd_read (sname, (bfd_size_type) SCNNMLEN, abfd);
+      if (fail
+	  || bfd_seek (abfd, secptr1, SEEK_SET) != 0
+	  || bfd_read (sname, SCNNMLEN, abfd) != SCNNMLEN)
+	return DEFAULT_COFF_PE_TEXT_SECTION_OFFSET;
       sname[SCNNMLEN] = '\0';
       if (strcmp (sname, ".text") == 0)
 	return vaddr;
diff --git a/gdb/coffread.c b/gdb/coffread.c
index 583db6bceb0..6a995ae2241 100644
--- a/gdb/coffread.c
+++ b/gdb/coffread.c
@@ -711,8 +711,6 @@ coff_symfile_read (struct objfile *objfile, symfile_add_flags symfile_flags)
 
       /* FIXME: dubious.  Why can't we use something normal like
 	 bfd_get_section_contents?  */
-      bfd_seek (abfd, abfd->where, 0);
-
       stabstrsize = bfd_section_size (info->stabstrsect);
 
       coffstab_build_psymtabs (objfile,
@@ -807,22 +805,6 @@ coff_symtab_read (minimal_symbol_reader &reader,
 
   scoped_free_pendings free_pending;
 
-  /* Work around a stdio bug in SunOS4.1.1 (this makes me nervous....
-     it's hard to know I've really worked around it.  The fix should
-     be harmless, anyway).  The symptom of the bug is that the first
-     fread (in read_one_sym), will (in my example) actually get data
-     from file offset 268, when the fseek was to 264 (and ftell shows
-     264).  This causes all hell to break loose.  I was unable to
-     reproduce this on a short test program which operated on the same
-     file, performing (I think) the same sequence of operations.
-
-     It stopped happening when I put in this (former) rewind().
-
-     FIXME: Find out if this has been reported to Sun, whether it has
-     been fixed in a later release, etc.  */
-
-  bfd_seek (objfile->obfd.get (), 0, 0);
-
   /* Position to read the symbol table.  */
   val = bfd_seek (objfile->obfd.get (), symtab_offset, 0);
   if (val < 0)
@@ -1308,12 +1290,13 @@ init_stringtab (bfd *abfd, file_ptr offset, gdb::unique_xmalloc_ptr<char> *stora
   if (bfd_seek (abfd, offset, 0) < 0)
     return -1;
 
-  val = bfd_read ((char *) lengthbuf, sizeof lengthbuf, abfd);
-  length = bfd_h_get_32 (symfile_bfd, lengthbuf);
-
+  val = bfd_read (lengthbuf, sizeof lengthbuf, abfd);
   /* If no string table is needed, then the file may end immediately
      after the symbols.  Just return with `stringtab' set to null.  */
-  if (val != sizeof lengthbuf || length < sizeof lengthbuf)
+  if (val != sizeof lengthbuf)
+    return 0;
+  length = bfd_h_get_32 (symfile_bfd, lengthbuf);
+  if (length < sizeof lengthbuf)
     return 0;
 
   storage->reset ((char *) xmalloc (length));
diff --git a/gdb/dbxread.c b/gdb/dbxread.c
index 75bbd510155..ddc61d9d539 100644
--- a/gdb/dbxread.c
+++ b/gdb/dbxread.c
@@ -809,7 +809,8 @@ stabs_seek (int sym_offset)
       symbuf_left -= sym_offset;
     }
   else
-    bfd_seek (symfile_bfd, sym_offset, SEEK_CUR);
+    if (bfd_seek (symfile_bfd, sym_offset, SEEK_CUR) != 0)
+      perror_with_name (bfd_get_filename (symfile_bfd));
 }
 
 #define INTERNALIZE_SYMBOL(intern, extern, abfd)			\
@@ -2155,8 +2156,8 @@ dbx_expand_psymtab (legacy_psymtab *pst, struct objfile *objfile)
       symbol_size = SYMBOL_SIZE (pst);
 
       /* Read in this file's symbols.  */
-      bfd_seek (objfile->obfd.get (), SYMBOL_OFFSET (pst), SEEK_SET);
-      read_ofile_symtab (objfile, pst);
+      if (bfd_seek (objfile->obfd.get (), SYMBOL_OFFSET (pst), SEEK_SET) == 0)
+	read_ofile_symtab (objfile, pst);
     }
 
   pst->readin = true;
diff --git a/gdb/xcoffread.c b/gdb/xcoffread.c
index 8ce4b28d133..63eb538ca05 100644
--- a/gdb/xcoffread.c
+++ b/gdb/xcoffread.c
@@ -779,8 +779,9 @@ enter_line_range (struct subfile *subfile, unsigned beginoffset,
 
   while (curoffset <= limit_offset)
     {
-      bfd_seek (abfd, curoffset, SEEK_SET);
-      bfd_read (ext_lnno, linesz, abfd);
+      if (bfd_seek (abfd, curoffset, SEEK_SET) != 0
+	  || bfd_read (ext_lnno, linesz, abfd) != linesz)
+	return;
       bfd_coff_swap_lineno_in (abfd, ext_lnno, &int_lnno);
 
       /* Find the address this line represents.  */

-- 
Alan Modra
Australia Development Lab, IBM

       reply	other threads:[~2023-08-10  3:36 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <ZNRamusOvj0f+x5T@squeak.grove.modra.org>
2023-08-10  3:36 ` Alan Modra [this message]
2023-08-11 12:15   ` Andrew Burgess

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ZNRbSREoB52gfDWx@squeak.grove.modra.org \
    --to=amodra@gmail.com \
    --cc=binutils@sourceware.org \
    --cc=gdb-patches@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).