343 lines
12 KiB
Diff
343 lines
12 KiB
Diff
From 42cf538d633015c3d2e6c81a0ca96b31edc004f0 Mon Sep 17 00:00:00 2001
|
||
From: Alan Modra <amodra@gmail.com>
|
||
Date: Wed, 9 Aug 2023 09:58:36 +0930
|
||
Subject: [PATCH] gdb: warn unused result for bfd IO functions
|
||
|
||
Reference:https://sourceware.org/bugzilla/show_bug.cgi?id=30641
|
||
Conflict:in gdb/coff-pe-read.c gdb/coffread.c gdb/dbxread.c gdb/xcoffread.c:
|
||
caused by commit:
|
||
98badbfdc (Use gdb_bfd_ref_ptr in objfile):
|
||
The type of the 'obfd' member in struct 'objfile' is changed from
|
||
'bfd *' to 'gdb_bfd_ref_ptr', needs to get the 'obfd' pointer by
|
||
using the get() method.
|
||
226f9f4fa (Rename bfd_bread and bfd_bwrite):
|
||
Only rename bfd_bread and bfd_bwrite to bfd_read and bfd_write.
|
||
6cb06a8cd (Unify gdb printf functions):
|
||
unify the printf family of functions by using gdb_printf.
|
||
We still using fprintf_unfiltered here.
|
||
|
||
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.
|
||
|
||
Approved-By: Andrew Burgess <aburgess@redhat.com>
|
||
---
|
||
gdb/coff-pe-read.c | 118 ++++++++++++++++++++++++++++-----------------
|
||
gdb/coffread.c | 27 ++---------
|
||
gdb/dbxread.c | 7 +--
|
||
gdb/xcoffread.c | 5 +-
|
||
4 files changed, 87 insertions(+), 70 deletions(-)
|
||
|
||
diff --git a/gdb/coff-pe-read.c b/gdb/coff-pe-read.c
|
||
index 90b406f..8edbf4c 100644
|
||
--- a/gdb/coff-pe-read.c
|
||
+++ b/gdb/coff-pe-read.c
|
||
@@ -291,23 +291,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_bread (b, (bfd_size_type) 2, abfd);
|
||
+ if (bfd_seek (abfd, where, SEEK_SET) != 0
|
||
+ || bfd_bread (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_bread (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_bread (b, 4, abfd) != 4)
|
||
+ {
|
||
+ *fail = true;
|
||
+ return 0;
|
||
+ }
|
||
+ return b[0] + (b[1] << 8) + (b[2] << 16) + ((unsigned) b[3] << 24);
|
||
}
|
||
|
||
static unsigned int
|
||
@@ -323,7 +331,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
|
||
@@ -376,37 +384,50 @@ 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)
|
||
+ fprintf_unfiltered (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. */
|
||
- return;
|
||
- }
|
||
+ return maybe_print_debug_msg ();
|
||
|
||
/* 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)
|
||
+ return maybe_print_debug_msg ();
|
||
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)
|
||
+ return maybe_print_debug_msg ();
|
||
|
||
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)
|
||
+ 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));
|
||
+ pe_get16 (dll, pe_header_offset + 4 + 16, &fail));
|
||
+ if (fail)
|
||
+ return maybe_print_debug_msg ();
|
||
expptr = 0;
|
||
export_size = 0;
|
||
|
||
@@ -415,13 +436,14 @@ 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);
|
||
-
|
||
- bfd_seek (dll, (file_ptr) secptr1, SEEK_SET);
|
||
- bfd_bread (sname, (bfd_size_type) sizeof (sname), dll);
|
||
-
|
||
+ 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);
|
||
+
|
||
+ if (fail
|
||
+ || bfd_seek (dll, secptr1, SEEK_SET) != 0
|
||
+ || bfd_bread (sname, sizeof (sname), dll) != sizeof (sname))
|
||
+
|
||
if ((strcmp (sname, ".edata") == 0)
|
||
|| (vaddr <= export_opthdrrva && export_opthdrrva < vaddr + vsize))
|
||
{
|
||
@@ -461,16 +483,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_bread (sec_name, (bfd_size_type) SCNNMLEN, dll);
|
||
+ if (fail
|
||
+ || bfd_seek (dll, secptr1 + 0, SEEK_SET) != 0
|
||
+ || bfd_bread (sec_name, SCNNMLEN, dll) != SCNNMLEN)
|
||
+ return maybe_print_debug_msg ();
|
||
sec_name[SCNNMLEN] = '\0';
|
||
|
||
sectix = read_pe_section_index (sec_name);
|
||
@@ -509,8 +533,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_bread (expdata, (bfd_size_type) export_size, dll);
|
||
+ if (bfd_seek (dll, expptr, SEEK_SET) != 0
|
||
+ || bfd_bread (expdata, export_size, dll) != export_size)
|
||
+ return maybe_print_debug_msg ();
|
||
erva = expdata - export_rva;
|
||
|
||
nexp = pe_as32 (expdata + 24);
|
||
@@ -658,20 +683,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_bread (sname, (bfd_size_type) SCNNMLEN, abfd);
|
||
+ if (fail
|
||
+ || bfd_seek (abfd, secptr1, SEEK_SET) != 0
|
||
+ || bfd_bread (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 c02aad6..6f58517 100644
|
||
--- a/gdb/coffread.c
|
||
+++ b/gdb/coffread.c
|
||
@@ -691,8 +691,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,
|
||
@@ -782,22 +780,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, 0, 0);
|
||
-
|
||
/* Position to read the symbol table. */
|
||
val = bfd_seek (objfile->obfd, symtab_offset, 0);
|
||
if (val < 0)
|
||
@@ -1287,12 +1269,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_bread ((char *) lengthbuf, sizeof lengthbuf, abfd);
|
||
- length = bfd_h_get_32 (symfile_bfd, lengthbuf);
|
||
-
|
||
+ val = bfd_bread (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 cf35880..0cdebe0 100644
|
||
--- a/gdb/dbxread.c
|
||
+++ b/gdb/dbxread.c
|
||
@@ -812,7 +812,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) \
|
||
@@ -2095,8 +2096,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, SYMBOL_OFFSET (pst), SEEK_SET);
|
||
- read_ofile_symtab (objfile, pst);
|
||
+ if (bfd_seek (objfile->obfd, 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 a854d4d..4b20054 100644
|
||
--- a/gdb/xcoffread.c
|
||
+++ b/gdb/xcoffread.c
|
||
@@ -865,8 +865,9 @@ enter_line_range (struct subfile *subfile, unsigned beginoffset,
|
||
|
||
while (curoffset <= limit_offset)
|
||
{
|
||
- bfd_seek (abfd, curoffset, SEEK_SET);
|
||
- bfd_bread (ext_lnno, linesz, abfd);
|
||
+ if (bfd_seek (abfd, curoffset, SEEK_SET) != 0
|
||
+ || bfd_bread (ext_lnno, linesz, abfd) != linesz)
|
||
+ return;
|
||
bfd_coff_swap_lineno_in (abfd, ext_lnno, &int_lnno);
|
||
|
||
/* Find the address this line represents. */
|
||
--
|
||
2.33.0
|
||
|