From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 9687 invoked by alias); 18 Jan 2013 15:00:50 -0000 Received: (qmail 9678 invoked by uid 22791); 18 Jan 2013 15:00:49 -0000 X-SWARE-Spam-Status: No, hits=-3.3 required=5.0 tests=BAYES_00,KHOP_THREADED,RP_MATCHES_RCVD,TW_CP X-Spam-Check-By: sourceware.org Received: from usmamail.tilera.com (HELO USMAMAIL.TILERA.COM) (12.216.194.151) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 18 Jan 2013 15:00:43 +0000 Received: from [192.168.0.103] (218.11.179.59) by USMAExch2.tad.internal.tilera.com (10.3.0.33) with Microsoft SMTP Server (TLS) id 14.0.694.0; Fri, 18 Jan 2013 10:00:39 -0500 Message-ID: <50F9638C.4020005@tilera.com> Date: Fri, 18 Jan 2013 15:00:00 -0000 From: Jiong Wang User-Agent: Mozilla/5.0 (Windows NT 6.1; rv:17.0) Gecko/20130107 Thunderbird/17.0.2 MIME-Version: 1.0 To: Joel Brobecker CC: , Walter Lee Subject: Re: [RFC/TileGX 1/6] fix args alignment bug References: <50F9148F.3010602@tilera.com> <20130118131358.GE3564@adacore.com> In-Reply-To: <20130118131358.GE3564@adacore.com> Content-Type: multipart/mixed; boundary="------------050209070600060409020508" 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: 2013-01/txt/msg00427.txt.bz2 --------------050209070600060409020508 Content-Type: text/plain; charset="ISO-8859-1"; format=flowed Content-Transfer-Encoding: 7bit Content-length: 1734 Hi Joel, thanks for your careful review >> - /* Loop backwards through arguments to determine stack alignment. */ >> - alignlen = 0; >> - >> - for (j = nargs - 1; j >= i; j--) >> - { >> - typelen = TYPE_LENGTH (value_enclosing_type (args[j])); >> - alignlen += (typelen + 3) & (~3); >> - } >> - >> - if (alignlen & 0x4) >> - stack_dest -= 4; > Do you have any hint as to why this code was written the way it was? > It doesn't seem like the type of code that would be added by accident. > Did Jeff (the apparent author) misunderstand the ABI? actually, these these copied from our tilepro target which is a pure 32bit target, and the compiler for that target is not gcc initially. so there maybe some historic reason for those code. but for tilegx, the ABI is actually very clear, 10 regs for arguments, then push on stack. so I just delete them, gdb works ok on both daily usage and dejagnu > >> typelen = TYPE_LENGTH (value_enclosing_type (args[j])); >> - slacklen = ((typelen + 3) & (~3)) - typelen; >> + slacklen = ((typelen + 7) & (~7)) - typelen; > This is a detail, but can you use utils.c:align_up, in this case? > (I find those midly more readable than these incantations, but > it's OK if you prefer the code to stay as is). thanks, I change to align_up to reuse existing helper function > >> /* Add 2 words for linkage space to the stack. */ >> - stack_dest = stack_dest - 8; >> - write_memory (stack_dest, two_zero_words, 8); >> + stack_dest = stack_dest - 16; >> + write_memory (stack_dest, four_zero_words, 16); > It looks like you need to adjust the comment as well. fixed. is this new 01-revise.patch ok for this issue? --------------050209070600060409020508 Content-Type: text/plain; charset="gb18030"; name="01-revise.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="01-revise.patch" Content-length: 2001 diff --git a/gdb/tilegx-tdep.c b/gdb/tilegx-tdep.c index 0c560ba..8be4046 100644 --- a/gdb/tilegx-tdep.c +++ b/gdb/tilegx-tdep.c @@ -292,7 +292,7 @@ tilegx_push_dummy_call (struct gdbarch *gdbarch, int argreg = TILEGX_R0_REGNUM; int i, j; int typelen, slacklen, alignlen; - static const gdb_byte two_zero_words[8] = { 0 }; + static const gdb_byte four_zero_words[16] = { 0 }; /* If struct_return is 1, then the struct return address will consume one argument-passing register. */ @@ -326,18 +326,6 @@ tilegx_push_dummy_call (struct gdbarch *gdbarch, /* Align SP. */ stack_dest = tilegx_frame_align (gdbarch, stack_dest); - /* Loop backwards through arguments to determine stack alignment. */ - alignlen = 0; - - for (j = nargs - 1; j >= i; j--) - { - typelen = TYPE_LENGTH (value_enclosing_type (args[j])); - alignlen += (typelen + 3) & (~3); - } - - if (alignlen & 0x4) - stack_dest -= 4; - /* Loop backwards through remaining arguments and push them on the stack, word aligned. */ for (j = nargs - 1; j >= i; j--) @@ -347,7 +335,7 @@ tilegx_push_dummy_call (struct gdbarch *gdbarch, const gdb_byte *contents = value_contents (args[j]); typelen = TYPE_LENGTH (value_enclosing_type (args[j])); - slacklen = ((typelen + 3) & (~3)) - typelen; + slacklen = align_up (typelen, 8) - typelen; val = xmalloc (typelen + slacklen); back_to = make_cleanup (xfree, val); memcpy (val, contents, typelen); @@ -359,9 +347,9 @@ tilegx_push_dummy_call (struct gdbarch *gdbarch, do_cleanups (back_to); } - /* Add 2 words for linkage space to the stack. */ - stack_dest = stack_dest - 8; - write_memory (stack_dest, two_zero_words, 8); + /* Add 16 bytes for linkage space to the stack. */ + stack_dest = stack_dest - 16; + write_memory (stack_dest, four_zero_words, 16); /* Update stack pointer. */ regcache_cooked_write_unsigned (regcache, TILEGX_SP_REGNUM, stack_dest); --------------050209070600060409020508--