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

[Simon Marchi <simark@simark.ca>]

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