From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 6309 invoked by alias); 12 Jun 2012 07:37:20 -0000 Received: (qmail 6139 invoked by uid 22791); 12 Jun 2012 07:37:19 -0000 X-SWARE-Spam-Status: No, hits=-6.3 required=5.0 tests=AWL,BAYES_00,KHOP_RCVD_UNTRUST,RCVD_IN_DNSWL_HI,RCVD_IN_HOSTKARMA_W,SPF_HELO_PASS,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 12 Jun 2012 07:37:00 +0000 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id q5C7axT5020731 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Tue, 12 Jun 2012 03:36:59 -0400 Received: from host2.jankratochvil.net (ovpn-116-33.ams2.redhat.com [10.36.116.33]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id q5C7atOG015894 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES128-SHA bits=128 verify=NO); Tue, 12 Jun 2012 03:36:57 -0400 Date: Tue, 12 Jun 2012 07:37:00 -0000 From: Jan Kratochvil To: Mark Kettenis Cc: gdb-patches@sourceware.org Subject: Re: ping: [patch 2/2] Fix gdb.cp/gdb2495.exp regression with gcc-4.7 #5 Message-ID: <20120612073654.GA4374@host2.jankratochvil.net> References: <20120309210117.GB30432@host2.jankratochvil.net> <20120326190414.GB11001@host2.jankratochvil.net> <201203271853.q2RIrbWf024897@glazunov.sibelius.xs4all.nl> <20120611191008.GA29811@host2.jankratochvil.net> <201206112129.q5BLT8ck007831@glazunov.sibelius.xs4all.nl> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <201206112129.q5BLT8ck007831@glazunov.sibelius.xs4all.nl> User-Agent: Mutt/1.5.21 (2010-09-15) X-IsSubscribed: yes 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-06/txt/msg00323.txt.bz2 On Mon, 11 Jun 2012 23:29:08 +0200, Mark Kettenis wrote: > > On Tue, 27 Mar 2012 20:53:37 +0200, Mark Kettenis wrote: > > > Also, if i386_push_dummy_call() doesn't preserve alignment (and it sure > > > looks like it doesn't), then aligning the stack here doesn't help. > > > > The patch helps (it makes working cases which did not work before), I am > > unable to find a user visible problem with it. > > Sorry, but that must be sheer luck. Here is no luck, your patch also does not do anything good with the alignment as the new KFAIL tdep/14222 proves. 16 bytes shift is still better than 1 byte shift (as was in amd64-dicos-tdep.c and i386-dicos-tdep.c) as 1 byte shift always breaks SSE inferiors. But whether 16 bytes subtraction or 16 bytes modulo works is just the "sheer luck" depending on this or that case. I will check GCC parameters alignment and fix PR tdep/14222 first otherwise the code does not make sense. > Like I said, that probably requires i386_push_dummy_call() to properly > align the stack just like amd64_push_dummy_call() does. Yes, PR tdep/14222. > > It is again about different coding style. > > Well, KISS is an important engineering principle if you ask me. (a) The previous code in amd64-dicos-tdep.c and i386-dicos-tdep.c already used gdbarch_breakpoint_from_pc. So moving a code is more simple patch than moving + changing the code. (b) Your code: *bp_addr = sp - 1; I find definitely less readable than gdbarch_breakpoint_from_pc (gdbarch, &bppc, &bplen); What is "- 1" there? There is no comment for it. If anything you should have written: /* 1 is size of the i386 breakpoint instruction. */ *bp_addr = sp - 1; But in such case I find more obvious it is more readable to put there just: gdbarch_breakpoint_from_pc (gdbarch, &bppc, &bplen); Jan