* gdb: warn unused result for bfd IO functions
[not found] <ZNRamusOvj0f+x5T@squeak.grove.modra.org>
@ 2023-08-10 3:36 ` Alan Modra
2023-08-11 12:15 ` Andrew Burgess
0 siblings, 1 reply; 2+ messages in thread
From: Alan Modra @ 2023-08-10 3:36 UTC (permalink / raw)
To: binutils, gdb-patches
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
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: gdb: warn unused result for bfd IO functions
2023-08-10 3:36 ` gdb: warn unused result for bfd IO functions Alan Modra
@ 2023-08-11 12:15 ` Andrew Burgess
0 siblings, 0 replies; 2+ messages in thread
From: Andrew Burgess @ 2023-08-11 12:15 UTC (permalink / raw)
To: Alan Modra, binutils, gdb-patches
Alan Modra via Binutils <binutils@sourceware.org> writes:
> 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?
I had a look through, this all looks OK, except I think we can easily
avoid adding a new 'goto' with the patch below.
If you're happy to take this change then:
Approved-By: Andrew Burgess <aburgess@redhat.com>
Thanks,
Andrew
---
diff --git a/gdb/coff-pe-read.c b/gdb/coff-pe-read.c
index 34360eb72f1..0d76ebdbfce 100644
--- a/gdb/coff-pe-read.c
+++ b/gdb/coff-pe-read.c
@@ -343,30 +343,29 @@ read_pe_exported_syms (minimal_symbol_reader &reader,
|| strcmp (target, "pei-i386") == 0
|| strcmp (target, "pe-arm-wince-little") == 0
|| strcmp (target, "pei-arm-wince-little") == 0);
+
+ /* Possibly print a debug message about DLL not having a valid format. */
+ auto maybe_print_debug_msg = [&] () -> void {
+ if (debug_coff_pe_read)
+ gdb_printf (gdb_stdlog, _("%s doesn't appear to be a DLL\n"),
+ bfd_get_filename (dll));
+ };
+
if (!is_pe32 && !is_pe64)
- {
- /* 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;
- }
+ return maybe_print_debug_msg ();
/* Get pe_header, optional header and numbers of export entries. */
bool fail = false;
pe_header_offset = pe_get32 (dll, 0x3c, &fail);
if (fail)
- goto notdll;
+ return maybe_print_debug_msg ();
opthdr_ofs = pe_header_offset + 4 + 20;
if (is_pe64)
num_entries = pe_get32 (dll, opthdr_ofs + 108, &fail);
else
num_entries = pe_get32 (dll, opthdr_ofs + 92, &fail);
if (fail)
- goto notdll;
+ return maybe_print_debug_msg ();
if (num_entries < 1) /* No exports. */
return;
@@ -381,13 +380,13 @@ read_pe_exported_syms (minimal_symbol_reader &reader,
export_opthdrsize = pe_get32 (dll, opthdr_ofs + 100, &fail);
}
if (fail)
- goto notdll;
+ return maybe_print_debug_msg ();
nsections = pe_get16 (dll, pe_header_offset + 4 + 2, &fail);
secptr = (pe_header_offset + 4 + 20 +
pe_get16 (dll, pe_header_offset + 4 + 16, &fail));
if (fail)
- goto notdll;
+ return maybe_print_debug_msg ();
expptr = 0;
export_size = 0;
@@ -454,7 +453,7 @@ read_pe_exported_syms (minimal_symbol_reader &reader,
if (fail
|| bfd_seek (dll, secptr1 + 0, SEEK_SET) != 0
|| bfd_read (sec_name, SCNNMLEN, dll) != SCNNMLEN)
- goto notdll;
+ return maybe_print_debug_msg ();
sec_name[SCNNMLEN] = '\0';
sectix = read_pe_section_index (sec_name);
@@ -495,7 +494,7 @@ read_pe_exported_syms (minimal_symbol_reader &reader,
if (bfd_seek (dll, expptr, SEEK_SET) != 0
|| bfd_read (expdata, export_size, dll) != export_size)
- goto notdll;
+ return maybe_print_debug_msg ();
erva = expdata - export_rva;
nexp = pe_as32 (expdata + 24);
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2023-08-11 12:15 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <ZNRamusOvj0f+x5T@squeak.grove.modra.org>
2023-08-10 3:36 ` gdb: warn unused result for bfd IO functions Alan Modra
2023-08-11 12:15 ` Andrew Burgess
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).