From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 11180 invoked by alias); 27 Dec 2012 20:13:17 -0000 Received: (qmail 11170 invoked by uid 22791); 27 Dec 2012 20:13:15 -0000 X-SWARE-Spam-Status: No, hits=-2.7 required=5.0 tests=AWL,BAYES_00,KHOP_THREADED X-Spam-Check-By: sourceware.org Received: from na3sys009aog131.obsmtp.com (HELO na3sys009aog131.obsmtp.com) (74.125.149.247) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Thu, 27 Dec 2012 20:13:09 +0000 Received: from mx10.qnx.com ([209.226.137.110]) (using TLSv1) by na3sys009aob131.postini.com ([74.125.148.12]) with SMTP ID DSNKUNyr1JlXlTiFhxj/xY/B8doqZkFMPBkr@postini.com; Thu, 27 Dec 2012 12:13:09 PST Received: by mx10.qnx.com (Postfix, from userid 500) id B4A0621172; Thu, 27 Dec 2012 15:13:07 -0500 (EST) Received: from exhts.ott.qnx.com (exhts2 [10.222.2.120]) (using TLSv1 with cipher AES128-SHA (128/128 bits)) (No client certificate requested) by mx10.qnx.com (Postfix) with ESMTPS id 82E712116F; Thu, 27 Dec 2012 15:13:07 -0500 (EST) Received: from [10.222.96.215] (10.222.96.215) by exhts2.ott.qnx.com (10.222.2.25) with Microsoft SMTP Server id 14.2.318.1; Thu, 27 Dec 2012 15:13:07 -0500 Message-ID: <50DCABD3.9060401@qnx.com> Date: Thu, 27 Dec 2012 20:13:00 -0000 From: Aleksandar Ristovski User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/17.0 Thunderbird/17.0 MIME-Version: 1.0 To: John Gilmore CC: Subject: Re: Ensure correct symbol-file when attaching to a (remote) process References: <50D3FC31.1020103@indel.ch> <20121221161114.GA32638@host2.jankratochvil.net> <50D4D099.3030601@qnx.com> <201212220138.qBM1caIl012256@new.toad.com> <50D520D8.1080602@qnx.com> <50D8B4C9.9080708@qnx.com> <201212260135.qBQ1ZOJJ024756@new.toad.com> In-Reply-To: <201212260135.qBQ1ZOJJ024756@new.toad.com> Content-Type: text/plain; charset="ISO-8859-1"; format=flowed Content-Transfer-Encoding: 7bit Mailing-List: contact gdb-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-owner@sourceware.org X-SW-Source: 2012-12/txt/msg00081.txt.bz2 On 12-12-25 08:35 PM, John Gilmore wrote: >> I have addressed some of your concerns, and some issues I found while >> writing the testcase. Please take a look: >> http://sourceware.org/ml/gdb-patches/2012-12/msg00809.html > > Thank you! > > There are several places where you assume that there are only > two possible pointer sizes ("if (ptr_size == 4)" and "else"). > These should abort -- or return false -- if they encounter a > pointer size they don't understand. And what is this constant "4" > doing in there? > > In read_program_headers_from_target, you initialize load_base_trusted > to 0, then in the first executable statement, you test it and abort if > it's zero. It took me some detailed reading to discover that in > another initializer, you pass load_base_trusted's address to > lm_addr_check, which modifies it as a side effect. Could you move that > call to lm_addr_check to just before the if statement, so it's clearly > in a place where people can see that a side effect will occur? > > In svr4_validate_ehdr_match, you compare the "magic numbers" by > directly feeding struct pointers to memcmp. You should compare > the magic number field the same way you compare all the other > fields -- as e.g. ehdr1->_32.e_magic. > > I'm still bothered that you read the symbol file's ELF header three > times, once when BFD opens the file, once when you read the ELF header > because you don't use BFD's copy, and once again when you need it to > find the phdrs. And you're still reading it from location "0" in the > file. Ditto for accessing the target memory twice, unnecessarily. > > I am also wondering why all this infrastructure has been made specific > to shared libraries (it's in the solib ops vector). The original > request on the mailing list was for this validation to occur for > ordinary symbol-files; this implementation prevents it from ever > being used for that. > > John Thank you for your comments. I will try to address your concerns and come up with a better patch. Due to other priorities, it will most probably not be before another few weeks. --- Aleksandar