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);
@@ -254,23 +254,31 @@ read_pe_truncate_name (char *dll_name)
/* 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);
}
/* 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;
@@ -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));
@@ -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;
@@ -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. */