From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca (simark.ca [158.69.221.121]) by sourceware.org (Postfix) with ESMTPS id 02332384242B for ; Thu, 26 Nov 2020 18:51:02 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 02332384242B Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=simark.ca Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=simark@simark.ca Received: from [10.0.0.11] (173-246-6-90.qc.cable.ebox.net [173.246.6.90]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id A28D11E58D; Thu, 26 Nov 2020 13:51:01 -0500 (EST) Subject: Re: [PATCH] gdb: define COFF file offsets with file_ptr To: Jameson Nash , gdb-patches@sourceware.org References: <20201126181121.31219-1-vtjnash@gmail.com> From: Simon Marchi Message-ID: <2bb948a7-e909-8949-7486-2aec0c4a9ecf@simark.ca> Date: Thu, 26 Nov 2020 13:51:01 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.12.0 MIME-Version: 1.0 In-Reply-To: <20201126181121.31219-1-vtjnash@gmail.com> Content-Type: text/plain; charset=utf-8 Content-Language: fr Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-9.8 required=5.0 tests=BAYES_00, GIT_PATCH_0, KAM_DMARC_STATUS, NICE_REPLY_A, SPF_HELO_PASS, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 26 Nov 2020 18:51:05 -0000 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