From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 24854 invoked by alias); 18 Jan 2012 12:09:24 -0000 Received: (qmail 24844 invoked by uid 22791); 18 Jan 2012 12:09:23 -0000 X-SWARE-Spam-Status: No, hits=-0.7 required=5.0 tests=AWL,BAYES_50 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; Wed, 18 Jan 2012 12:09:10 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id E81DD2BB280; Wed, 18 Jan 2012 07:09:09 -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 1SHLiKfR9mEk; Wed, 18 Jan 2012 07:09:09 -0500 (EST) Received: from joel.gnat.com (localhost.localdomain [127.0.0.1]) by rock.gnat.com (Postfix) with ESMTP id 76B5A2BB1F3; Wed, 18 Jan 2012 07:09:09 -0500 (EST) Received: by joel.gnat.com (Postfix, from userid 1000) id 0FB19145615; Wed, 18 Jan 2012 16:08:31 +0400 (RET) Date: Wed, 18 Jan 2012 12:18:00 -0000 From: Joel Brobecker To: Michael Eager Cc: "gdb-patches@sourceware.org" Subject: Re: [PATCH] PowerPC 32 with Secure PLT Message-ID: <20120118120831.GB31383@adacore.com> References: <4F163E35.1080505@eagerm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <4F163E35.1080505@eagerm.com> User-Agent: Mutt/1.5.20 (2009-06-14) 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-01/txt/msg00666.txt.bz2 > 2012-01-17 Michael Eager > > * configure.tgt (powerpc-*-linux*): Add glibc-tdep.o. > * ppc-linux-tdep.c: Include glibc-tdep.h. > (powerpc32_plt_stub, powerpc32_plt_stub_so): Add PLT stub templates. > (powerpc_linux_in_plt_stub): New function. > (powerpc_linux_in_dynsym_resolve_code): New function. > (ppc_skip_trampoline_code): New function. > (ppc_linux_init_abi): Use PPC specific functions rather than generic. > Use glibc_skip_solib_resolver. Overall, this looks good to me, but I am not a Linux specialist, so please give it another week before checking in to give the other maintainers a chance to comment as well. Below are some comments. > +#define POWERPC32_PLT_STUB_LEN \ > + (sizeof powerpc32_plt_stub / sizeof (powerpc32_plt_stub[0])) There is a macro, ARRAY_SIZE, that does this for you. > +/* Check if PC is in PLT stub. For non-secure PLT, stub is in .plt Trailing space at the end of this line... > +/* Follow PLT stub to actual routine. */ > + > +static CORE_ADDR > +ppc_skip_trampoline_code (struct frame_info *frame, CORE_ADDR pc) There are several lines in the code and the comments of this funtion that have trailing spaces. Can you get rid of them? Also, several lines are too long. The absolute limit is 80 characters, but the recommended limit is 70 characters. Can you reformat some of the lines where it doesn't make the code uglier? > + target = (CORE_ADDR) read_memory_unsigned_integer (target, 4, byte_order); This is an area where I am not sure, but I think it would be better to use read_memory_typed_address. I think the address type that you want is: builtin_type (gdbarch)->builtin_func_ptr. Same below. -- Joel