[PATCH] gdb: define COFF file offsets with file_ptr

[PATCH] gdb: define COFF file offsets with file_ptr

[Jameson Nash]

The arguments to these functions are file_ptr, so these declarations
were accidentally implicitly down-casting them to signed int. This
allows for reading files between 2 and 4 GB in size in my testing (I
don't have a larger dll currently to test). These may not be natively
supported by Windows, but can appear when using split-dwarf information.

This solves a "can't get string table" error resulting from attempting
to pass a negative offset to bfd_seek. I encountered this occuring while
trying to use a debug file for libLLVM.dll, but searching online reveals
at least one other person may have run into a similar problem with
Firefox?
https://sourceforge.net/p/mingw-w64/mailman/mingw-w64-public/thread/CA+cU71k2bU0azQxjy4-77ynQj1O+TKmgtaTKe59n7Bjub1y7Tg@mail.gmail.com/
With this patch, the debug file appears to load successfully and I can
see debug information in gdb for the library.

gdb/ChangeLog:
	*coffread.c: Define COFF file offsets with file_ptr to support
files over 2GB.

---
 gdb/coffread.c | 34 +++++++++++++++++-----------------
 1 file changed, 17 insertions(+), 17 deletions(-)

diff --git a/gdb/coffread.c b/gdb/coffread.c
index c61c9a7ca1..8e0e9543ae 100644
--- a/gdb/coffread.c
+++ b/gdb/coffread.c
@@ -155,8 +155,8 @@ static int type_vector_length;
 #define INITIAL_TYPE_VECTOR_LENGTH 160
 
 static char *linetab = NULL;
-static long linetab_offset;
-static unsigned long linetab_size;
+static file_ptr linetab_offset;
+static file_ptr linetab_size;
 
 static char *stringtab = NULL;
 
@@ -188,23 +188,23 @@ static struct symbol *process_coff_symbol (struct coff_symbol *,
 
 static void patch_opaque_types (struct symtab *);
 
-static void enter_linenos (long, int, int, struct objfile *);
+static void enter_linenos (file_ptr, int, int, struct objfile *);
 
-static int init_lineno (bfd *, long, int, gdb::unique_xmalloc_ptr<char> *);
+static int init_lineno (bfd *, file_ptr, file_ptr, gdb::unique_xmalloc_ptr<char> *);
 
 static char *getsymname (struct internal_syment *);
 
 static const char *coff_getfilename (union internal_auxent *);
 
-static int init_stringtab (bfd *, long, gdb::unique_xmalloc_ptr<char> *);
+static int init_stringtab (bfd *, file_ptr, gdb::unique_xmalloc_ptr<char> *);
 
 static void read_one_sym (struct coff_symbol *,
 			  struct internal_syment *,
 			  union internal_auxent *);
 
 static void coff_symtab_read (minimal_symbol_reader &,
-			      long, unsigned int, struct objfile *);
-\f
+			      file_ptr, unsigned int, struct objfile *);
+
 /* We are called once per section from coff_symfile_read.  We
    need to examine each section we are passed, check to see
    if it is something we are interested in processing, and
@@ -540,9 +540,9 @@ coff_symfile_read (struct objfile *objfile, symfile_add_flags symfile_flags)
   const char *filename = bfd_get_filename (abfd);
   int val;
   unsigned int num_symbols;
-  int symtab_offset;
-  int stringtab_offset;
-  int stabstrsize;
+  file_ptr symtab_offset;
+  file_ptr stringtab_offset;
+  unsigned int stabstrsize;
   
   info = coff_objfile_data_key.get (objfile);
   symfile_bfd = abfd;		/* Kludge for swap routines.  */
@@ -741,7 +741,7 @@ coff_symfile_finish (struct objfile *objfile)
   /* Let stabs reader clean up.  */
   stabsread_clear_cache ();
 }
-\f
+
 
 /* Given pointers to a symbol table in coff style exec file,
    analyze them and create struct symtab's describing the symbols.
@@ -750,7 +750,7 @@ coff_symfile_finish (struct objfile *objfile)
 
 static void
 coff_symtab_read (minimal_symbol_reader &reader,
-		  long symtab_offset, unsigned int nsyms,
+		  file_ptr symtab_offset, unsigned int nsyms,
 		  struct objfile *objfile)
 {
   struct gdbarch *gdbarch = objfile->arch ();
@@ -796,7 +796,7 @@ coff_symtab_read (minimal_symbol_reader &reader,
   bfd_seek (objfile->obfd, 0, 0);
 
   /* Position to read the symbol table.  */
-  val = bfd_seek (objfile->obfd, (long) symtab_offset, 0);
+  val = bfd_seek (objfile->obfd, symtab_offset, 0);
   if (val < 0)
     perror_with_name (objfile_name (objfile));
 
@@ -1183,7 +1183,7 @@ coff_symtab_read (minimal_symbol_reader &reader,
 
   coffread_objfile = NULL;
 }
-\f
+
 /* Routines for reading headers and symbols from executable.  */
 
 /* Read the next symbol, swap it, and return it in both
@@ -1270,7 +1270,7 @@ read_one_sym (struct coff_symbol *cs,
 /* Support for string table handling.  */
 
 static int
-init_stringtab (bfd *abfd, long offset, gdb::unique_xmalloc_ptr<char> *storage)
+init_stringtab (bfd *abfd, file_ptr offset, gdb::unique_xmalloc_ptr<char> *storage)
 {
   long length;
   int val;
@@ -1366,7 +1366,7 @@ coff_getfilename (union internal_auxent *aux_entry)
    them into GDB's data structures.  */
 
 static int
-init_lineno (bfd *abfd, long offset, int size,
+init_lineno (bfd *abfd, file_ptr offset, file_ptr size,
 	     gdb::unique_xmalloc_ptr<char> *storage)
 {
   int val;
@@ -1399,7 +1399,7 @@ init_lineno (bfd *abfd, long offset, int size,
 #endif
 
 static void
-enter_linenos (long file_offset, int first_line,
+enter_linenos (file_ptr file_offset, int first_line,
 	       int last_line, struct objfile *objfile)
 {
   struct gdbarch *gdbarch = objfile->arch ();
-- 
2.29.2

Re: [PATCH] gdb: define COFF file offsets with file_ptr

[Simon Marchi]

On 2020-11-26 1:11 p.m., Jameson Nash via Gdb-patches wrote:
> The arguments to these functions are file_ptr, so these declarations
> were accidentally implicitly down-casting them to signed int. This
> allows for reading files between 2 and 4 GB in size in my testing (I
> don't have a larger dll currently to test). These may not be natively
> supported by Windows, but can appear when using split-dwarf information.
>
> This solves a "can't get string table" error resulting from attempting
> to pass a negative offset to bfd_seek. I encountered this occuring while
> trying to use a debug file for libLLVM.dll, but searching online reveals
> at least one other person may have run into a similar problem with
> Firefox?
> https://sourceforge.net/p/mingw-w64/mailman/mingw-w64-public/thread/CA+cU71k2bU0azQxjy4-77ynQj1O+TKmgtaTKe59n7Bjub1y7Tg@mail.gmail.com/
> With this patch, the debug file appears to load successfully and I can
> see debug information in gdb for the library.
>
> gdb/ChangeLog:
> 	*coffread.c: Define COFF file offsets with file_ptr to support
> files over 2GB.

I forgot to mention that you need to list the changed entities as well
(it's described somewhere in the contribution checklist).  A quick way
to get started is to use the mklog.py script:

$ git show | ../contrib/mklog.py
gdb/ChangeLog:

        * coffread.c (enter_linenos):
        (init_lineno):
        (init_stringtab):
        (coff_symtab_read):
        (coff_symfile_read):
        (coff_symfile_finish):

As you can see, it doesn't get everything right.  I would fix it up and
fill it up like this:

gdb/ChangeLog:

	* coffread.c (linetab_offset): Change type to file_ptr.
	(linetab_size): Likewise.
	(enter_linenos): Change parameter type to file_ptr.
	(init_lineno): Likewise.
	(init_stringtab): Likewise.
	(coff_symtab_read): Likewise.
	(coff_symfile_read): Change variable types to file_ptr.

I don't see anything wrong with the patch, except maybe one nit below.
I would like if we could wait a week or so, maybe others who know more
about this will have something to say.

> diff --git a/gdb/coffread.c b/gdb/coffread.c
> index c61c9a7ca1..8e0e9543ae 100644
> --- a/gdb/coffread.c
> +++ b/gdb/coffread.c
> @@ -155,8 +155,8 @@ static int type_vector_length;
>  #define INITIAL_TYPE_VECTOR_LENGTH 160
>
>  static char *linetab = NULL;
> -static long linetab_offset;
> -static unsigned long linetab_size;
> +static file_ptr linetab_offset;
> +static file_ptr linetab_size;

linetab_size was unsigned, so maybe use ufile_ptr to keep it that way?

I don't know what linetab_offset is, presumably it's the offset of the
line table in the file, so it can't really be negative.  So I'd try to
see if it would make sense to make that unsigned too.  But if the goal
is to minimize the changes, it can stay signed.

Simon

Re: [PATCH] gdb: define COFF file offsets with file_ptr

[Simon Marchi]

On 2020-11-26 1:51 p.m., Simon Marchi wrote:
> I don't see anything wrong with the patch, except maybe one nit below.
> I would like if we could wait a week or so, maybe others who know more
> about this will have something to say.

I forgot to say: please ping if nothing happened a week from now,
because I'll probably don't remember and the thread will be quite far in
my inbox.

Simon

Re: [PATCH] gdb: define COFF file offsets with file_ptr

[Jameson Nash]

> linetab_size was unsigned, so maybe use ufile_ptr to keep it that way?

It's (currently) computed as the difference of two file_ptr values. The
result should be positive, certainly, as it's later used as either a file
offset (with a printed warning) and as a size_t. But I'm more inclined to
keep it common with the other values here, and use `file_ptr`, which is a
signed integer type like `off_t` and `ssize_t`.

Re: [PATCH] gdb: define COFF file offsets with file_ptr

[Simon Marchi]

On 2020-11-27 3:49 p.m., Jameson Nash wrote:
> It's (currently) computed as the difference of two file_ptr values. The result should be positive, certainly, as it's later used as either a file offset (with a printed warning) and as a size_t. But I'm more inclined to keep it common with the other values here, and use `file_ptr`, which is a signed integer type like `off_t` and `ssize_t`.

Ok, that's fine with me.

Simon

Re: [PATCH] gdb: define COFF file offsets with file_ptr

[Simon Marchi]

On 2020-11-27 3:55 p.m., Simon Marchi wrote:
> On 2020-11-27 3:49 p.m., Jameson Nash wrote:
>> It's (currently) computed as the difference of two file_ptr values. The result should be positive, certainly, as it's later used as either a file offset (with a printed warning) and as a size_t. But I'm more inclined to keep it common with the other values here, and use `file_ptr`, which is a signed integer type like `off_t` and `ssize_t`.
> 
> Ok, that's fine with me.
> 
> Simon
> 

I have now pushed this patch.

Simon

[PATCH] gdb: define COFF file offsets with file_ptr

[Jameson Nash]

The arguments to these functions are file_ptr, so these declarations
were accidentally implicitly down-casting them to signed int. This
allows for reading files between 2 and 4 GB in size in my testing (I
don't have a larger dll currently to test). These may not be natively
supported by Windows, but can appear when using split-dwarf information.

This solves a "can't get string table" error resulting from attempting
to pass a negative offset to bfd_seek. I encountered this occuring while
trying to use a debug file for libLLVM.dll, but searching online reveals
at least one other person may have run into a similar problem with
Firefox?
https://sourceforge.net/p/mingw-w64/mailman/mingw-w64-public/thread/CA+cU71k2bU0azQxjy4-77ynQj1O+TKmgtaTKe59n7Bjub1y7Tg@mail.gmail.com/
With this patch, the debug file appears to load successfully and I can
see debug information in gdb for the library.

gdb/ChangeLog:
        * coffread.c (linetab_offset): Change type to file_ptr.
        (linetab_size): Likewise.
        (enter_linenos): Change parameter type to file_ptr.
        (init_lineno): Likewise.
        (init_stringtab): Likewise.
        (coff_symtab_read): Likewise.
        (coff_symfile_read): Change variable types to file_ptr.

---
 gdb/coffread.c | 34 +++++++++++++++++-----------------
 1 file changed, 17 insertions(+), 17 deletions(-)

diff --git a/gdb/coffread.c b/gdb/coffread.c
index c61c9a7ca1..8e0e9543ae 100644
--- a/gdb/coffread.c
+++ b/gdb/coffread.c
@@ -155,8 +155,8 @@ static int type_vector_length;
 #define INITIAL_TYPE_VECTOR_LENGTH 160
 
 static char *linetab = NULL;
-static long linetab_offset;
-static unsigned long linetab_size;
+static file_ptr linetab_offset;
+static file_ptr linetab_size;
 
 static char *stringtab = NULL;
 
@@ -188,23 +188,23 @@ static struct symbol *process_coff_symbol (struct coff_symbol *,
 
 static void patch_opaque_types (struct symtab *);
 
-static void enter_linenos (long, int, int, struct objfile *);
+static void enter_linenos (file_ptr, int, int, struct objfile *);
 
-static int init_lineno (bfd *, long, int, gdb::unique_xmalloc_ptr<char> *);
+static int init_lineno (bfd *, file_ptr, file_ptr, gdb::unique_xmalloc_ptr<char> *);
 
 static char *getsymname (struct internal_syment *);
 
 static const char *coff_getfilename (union internal_auxent *);
 
-static int init_stringtab (bfd *, long, gdb::unique_xmalloc_ptr<char> *);
+static int init_stringtab (bfd *, file_ptr, gdb::unique_xmalloc_ptr<char> *);
 
 static void read_one_sym (struct coff_symbol *,
 			  struct internal_syment *,
 			  union internal_auxent *);
 
 static void coff_symtab_read (minimal_symbol_reader &,
-			      long, unsigned int, struct objfile *);
-\f
+			      file_ptr, unsigned int, struct objfile *);
+
 /* We are called once per section from coff_symfile_read.  We
    need to examine each section we are passed, check to see
    if it is something we are interested in processing, and
@@ -540,9 +540,9 @@ coff_symfile_read (struct objfile *objfile, symfile_add_flags symfile_flags)
   const char *filename = bfd_get_filename (abfd);
   int val;
   unsigned int num_symbols;
-  int symtab_offset;
-  int stringtab_offset;
-  int stabstrsize;
+  file_ptr symtab_offset;
+  file_ptr stringtab_offset;
+  unsigned int stabstrsize;
   
   info = coff_objfile_data_key.get (objfile);
   symfile_bfd = abfd;		/* Kludge for swap routines.  */
@@ -741,7 +741,7 @@ coff_symfile_finish (struct objfile *objfile)
   /* Let stabs reader clean up.  */
   stabsread_clear_cache ();
 }
-\f
+
 
 /* Given pointers to a symbol table in coff style exec file,
    analyze them and create struct symtab's describing the symbols.
@@ -750,7 +750,7 @@ coff_symfile_finish (struct objfile *objfile)
 
 static void
 coff_symtab_read (minimal_symbol_reader &reader,
-		  long symtab_offset, unsigned int nsyms,
+		  file_ptr symtab_offset, unsigned int nsyms,
 		  struct objfile *objfile)
 {
   struct gdbarch *gdbarch = objfile->arch ();
@@ -796,7 +796,7 @@ coff_symtab_read (minimal_symbol_reader &reader,
   bfd_seek (objfile->obfd, 0, 0);
 
   /* Position to read the symbol table.  */
-  val = bfd_seek (objfile->obfd, (long) symtab_offset, 0);
+  val = bfd_seek (objfile->obfd, symtab_offset, 0);
   if (val < 0)
     perror_with_name (objfile_name (objfile));
 
@@ -1183,7 +1183,7 @@ coff_symtab_read (minimal_symbol_reader &reader,
 
   coffread_objfile = NULL;
 }
-\f
+
 /* Routines for reading headers and symbols from executable.  */
 
 /* Read the next symbol, swap it, and return it in both
@@ -1270,7 +1270,7 @@ read_one_sym (struct coff_symbol *cs,
 /* Support for string table handling.  */
 
 static int
-init_stringtab (bfd *abfd, long offset, gdb::unique_xmalloc_ptr<char> *storage)
+init_stringtab (bfd *abfd, file_ptr offset, gdb::unique_xmalloc_ptr<char> *storage)
 {
   long length;
   int val;
@@ -1366,7 +1366,7 @@ coff_getfilename (union internal_auxent *aux_entry)
    them into GDB's data structures.  */
 
 static int
-init_lineno (bfd *abfd, long offset, int size,
+init_lineno (bfd *abfd, file_ptr offset, file_ptr size,
 	     gdb::unique_xmalloc_ptr<char> *storage)
 {
   int val;
@@ -1399,7 +1399,7 @@ init_lineno (bfd *abfd, long offset, int size,
 #endif
 
 static void
-enter_linenos (long file_offset, int first_line,
+enter_linenos (file_ptr file_offset, int first_line,
 	       int last_line, struct objfile *objfile)
 {
   struct gdbarch *gdbarch = objfile->arch ();
-- 
2.29.2

Re: [PATCH] gdb: define COFF file offsets with file_ptr

[Simon Marchi]

On 2020-11-25 2:14 p.m., Jameson Nash via Gdb-patches wrote:
> The arguments to these functions are file_ptr, so these declarations
> were accidentally implicitly down-casting them to signed int. This
> allows for reading files between 2 and 4 GB in size in my testing (I
> don't have a larger dll currently to test). These may not be natively
> supported by Windows, but can appear when using split-dwarf information.
>
> This solves a "can't get string table" error resulting from attempting to
> pass a negative offset to bfd_seek. I encountered this occuring while
> trying to use a debug file for libLLVM.dll, but searching online reveals at
> least one other person may have run into a similar problem with Firefox?
> https://sourceforge.net/p/mingw-w64/mailman/mingw-w64-public/thread/CA+cU71k2bU0azQxjy4-77ynQj1O+TKmgtaTKe59n7Bjub1y7Tg@mail.gmail.com/
> With this patch, the debug file appears to load successfully and I can see
> debug information in gdb for the library.

Hi Jameson,

Thanks for the patch, I think that makes sense.

I am however unable to apply the patch, probably because you sent it
using an email client that wrapped lines or something.  Can you please
send it again but using git-send-email this time?

https://sourceware.org/gdb/wiki/ContributionChecklist#Submitting_patches

> gdb/ChangeLog:
>         *gdb/coffread.c: define COFF file offsets with file_ptr to support
> files over 2GB

For the ChangeLog entry:

- Remove the "gdb/" portion of the file name
- Start with a capital letter
- End with a period.
- Wrap at 74 columns

Simon

[PATCH] gdb: define COFF file offsets with file_ptr

[Jameson Nash]

The arguments to these functions are file_ptr, so these declarations
were accidentally implicitly down-casting them to signed int. This
allows for reading files between 2 and 4 GB in size in my testing (I
don't have a larger dll currently to test). These may not be natively
supported by Windows, but can appear when using split-dwarf information.

This solves a "can't get string table" error resulting from attempting to
pass a negative offset to bfd_seek. I encountered this occuring while
trying to use a debug file for libLLVM.dll, but searching online reveals at
least one other person may have run into a similar problem with Firefox?
https://sourceforge.net/p/mingw-w64/mailman/mingw-w64-public/thread/CA+cU71k2bU0azQxjy4-77ynQj1O+TKmgtaTKe59n7Bjub1y7Tg@mail.gmail.com/
With this patch, the debug file appears to load successfully and I can see
debug information in gdb for the library.


gdb/ChangeLog:
        *gdb/coffread.c: define COFF file offsets with file_ptr to support
files over 2GB

---
 gdb/coffread.c | 34 +++++++++++++++++-----------------
 1 file changed, 17 insertions(+), 17 deletions(-)

diff --git a/gdb/coffread.c b/gdb/coffread.c
index c61c9a7ca1..8e0e9543ae 100644
--- a/gdb/coffread.c
+++ b/gdb/coffread.c
@@ -155,8 +155,8 @@ static int type_vector_length;
 #define INITIAL_TYPE_VECTOR_LENGTH 160

 static char *linetab = NULL;
-static long linetab_offset;
-static unsigned long linetab_size;
+static file_ptr linetab_offset;
+static file_ptr linetab_size;

 static char *stringtab = NULL;

@@ -188,23 +188,23 @@ static struct symbol *process_coff_symbol (struct
coff_symbol *,

 static void patch_opaque_types (struct symtab *);

-static void enter_linenos (long, int, int, struct objfile *);
+static void enter_linenos (file_ptr, int, int, struct objfile *);

-static int init_lineno (bfd *, long, int, gdb::unique_xmalloc_ptr<char> *);
+static int init_lineno (bfd *, file_ptr, file_ptr,
gdb::unique_xmalloc_ptr<char> *);

 static char *getsymname (struct internal_syment *);

 static const char *coff_getfilename (union internal_auxent *);

-static int init_stringtab (bfd *, long, gdb::unique_xmalloc_ptr<char> *);
+static int init_stringtab (bfd *, file_ptr, gdb::unique_xmalloc_ptr<char>
*);

 static void read_one_sym (struct coff_symbol *,
   struct internal_syment *,
   union internal_auxent *);

 static void coff_symtab_read (minimal_symbol_reader &,
-      long, unsigned int, struct objfile *);
-
+      file_ptr, unsigned int, struct objfile *);
+
 /* We are called once per section from coff_symfile_read.  We
    need to examine each section we are passed, check to see
    if it is something we are interested in processing, and
@@ -540,9 +540,9 @@ coff_symfile_read (struct objfile *objfile,
symfile_add_flags symfile_flags)
   const char *filename = bfd_get_filename (abfd);
   int val;
   unsigned int num_symbols;
-  int symtab_offset;
-  int stringtab_offset;
-  int stabstrsize;
+  file_ptr symtab_offset;
+  file_ptr stringtab_offset;
+  unsigned int stabstrsize;

   info = coff_objfile_data_key.get (objfile);
   symfile_bfd = abfd; /* Kludge for swap routines.  */
@@ -741,7 +741,7 @@ coff_symfile_finish (struct objfile *objfile)
   /* Let stabs reader clean up.  */
   stabsread_clear_cache ();
 }
-
+

 /* Given pointers to a symbol table in coff style exec file,
    analyze them and create struct symtab's describing the symbols.
@@ -750,7 +750,7 @@ coff_symfile_finish (struct objfile *objfile)

 static void
 coff_symtab_read (minimal_symbol_reader &reader,
-  long symtab_offset, unsigned int nsyms,
+  file_ptr symtab_offset, unsigned int nsyms,
   struct objfile *objfile)
 {
   struct gdbarch *gdbarch = objfile->arch ();
@@ -796,7 +796,7 @@ coff_symtab_read (minimal_symbol_reader &reader,
   bfd_seek (objfile->obfd, 0, 0);

   /* Position to read the symbol table.  */
-  val = bfd_seek (objfile->obfd, (long) symtab_offset, 0);
+  val = bfd_seek (objfile->obfd, symtab_offset, 0);
   if (val < 0)
     perror_with_name (objfile_name (objfile));

@@ -1183,7 +1183,7 @@ coff_symtab_read (minimal_symbol_reader &reader,

   coffread_objfile = NULL;
 }
-
+
 /* Routines for reading headers and symbols from executable.  */

 /* Read the next symbol, swap it, and return it in both
@@ -1270,7 +1270,7 @@ read_one_sym (struct coff_symbol *cs,
 /* Support for string table handling.  */

 static int
-init_stringtab (bfd *abfd, long offset, gdb::unique_xmalloc_ptr<char>
*storage)
+init_stringtab (bfd *abfd, file_ptr offset, gdb::unique_xmalloc_ptr<char>
*storage)
 {
   long length;
   int val;
@@ -1366,7 +1366,7 @@ coff_getfilename (union internal_auxent *aux_entry)
    them into GDB's data structures.  */

 static int
-init_lineno (bfd *abfd, long offset, int size,
+init_lineno (bfd *abfd, file_ptr offset, file_ptr size,
      gdb::unique_xmalloc_ptr<char> *storage)
 {
   int val;
@@ -1399,7 +1399,7 @@ init_lineno (bfd *abfd, long offset, int size,
 #endif

 static void
-enter_linenos (long file_offset, int first_line,
+enter_linenos (file_ptr file_offset, int first_line,
        int last_line, struct objfile *objfile)
 {
   struct gdbarch *gdbarch = objfile->arch ();
-- 
2.29.2