From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 11126 invoked by alias); 5 Nov 2012 17:11:42 -0000 Received: (qmail 11115 invoked by uid 22791); 5 Nov 2012 17:11:40 -0000 X-SWARE-Spam-Status: No, hits=-2.0 required=5.0 tests=AWL,BAYES_00,RCVD_IN_HOSTKARMA_NO X-Spam-Check-By: sourceware.org Received: from rock.gnat.com (HELO rock.gnat.com) (205.232.38.15) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Mon, 05 Nov 2012 17:11:28 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id 45DE11C6BF2; Mon, 5 Nov 2012 12:11:27 -0500 (EST) Received: from rock.gnat.com ([127.0.0.1]) by localhost (rock.gnat.com [127.0.0.1]) (amavisd-new, port 10024) with LMTP id 33IHJady0P-C; Mon, 5 Nov 2012 12:11:27 -0500 (EST) Received: from joel.gnat.com (localhost.localdomain [127.0.0.1]) by rock.gnat.com (Postfix) with ESMTP id 12FBC1C693C; Mon, 5 Nov 2012 12:11:27 -0500 (EST) Received: by joel.gnat.com (Postfix, from userid 1000) id 19FF5C6822; Mon, 5 Nov 2012 09:11:21 -0800 (PST) Date: Mon, 05 Nov 2012 17:11:00 -0000 From: Joel Brobecker To: Pierre Muller Cc: gdb-patches@sourceware.org Subject: Re: [RFC] Fix .text section offset for windows DLL (was Calling __stdcall functions in the inferior) Message-ID: <20121105171121.GA2972@adacore.com> References: <83a9vs89r9.fsf@gnu.org> <201210120953.q9C9rqfu020865@glazunov.sibelius.xs4all.nl> <834nm07z0s.fsf@gnu.org> <5077FEB9.4030304@redhat.com> <83y5jb7rfe.fsf@gnu.org> <006001cdaada$00c81f00$02585d00$@muller@ics-cnrs.unistra.fr> <20121024194517.GK3555@adacore.com> <011901cdb2ab$48076b90$d81642b0$@muller@ics-cnrs.unistra.fr> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <011901cdb2ab$48076b90$d81642b0$@muller@ics-cnrs.unistra.fr> User-Agent: Mutt/1.5.21 (2010-09-15) Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2012-11/txt/msg00085.txt.bz2 > Oh boy, you scared me there... But definitively, no, I didn't > commit this change yet! Sorry about the scare. I was looking at the code, and saw identical code, so I figured it got checked in. That's perhaps a clue for us to do more code factoring... > I still didn't get a correct way of checking my formatting... No problemo, I don't think it's a major issue. > > > + printf ("%s is a forward to %s\n", funcname, forward_name); > > > > I don't think a printf is appropriate, here. Is that meant to be > > a warning? > > I would have like any suggestion about an idea about how to > implement such a forward... Something like a function called > add_pe_forward_symbol Would it require the definition of a new > minimal symbol type? If yes, I wouldn't even know where I should > add this new type... Are you saying, remove the need for the printf/warning by implementing the missing feature? If complicated, I'd start with the warning/complaint first, get that checked in, and then think about how to lift the limitation. > I basically reused the existing code in the function called > read_pe_exported_syms, and, specifically, I kept most of the local > variables and used the same names... Let's keep it consistent for now, then... > > > + unsigned char *expdata, *erva; > > > > Should we be using gdb_byte *, in this case? I'm wondering if we > > should be adjusting the pe_get* & pe_as* routines as well... > > The problem is that it is used both for 'char *' > for all the names and for RVA (relative virtual addresses) > which are more something like a 32-bit unsigned offset. I'm not opposed to keeping the consistency, but I think we're going to have to re-evaluate the use of "char *" vs "gdb_byte *" at some point. I think that if you have a buffer of bytes, it's better to use gdb_bytes, as the type was created to deal with this sort of data. Usually, you can't rely on what the buffer contains, because host and target can be different. > > > + 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 0; > > > + } > > > > I think a complaint would be appropriate, here. And I'm wondering > > if there might be a better way to check which PE format it is other > > than looking at a string... > > Again, this is a plain copy of the code in read_pe_exported_syms. OK, you can keep it that way, at least for now. "plain copy" -> factorize? -- Joel