From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 109335 invoked by alias); 20 Aug 2015 12:09:14 -0000 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 Received: (qmail 109326 invoked by uid 89); 20 Aug 2015 12:09:13 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-0.9 required=5.0 tests=AWL,BAYES_05,KAM_LAZY_DOMAIN_SECURITY,RCVD_IN_DNSWL_LOW autolearn=no version=3.3.2 X-HELO: rock.gnat.com Received: from rock.gnat.com (HELO rock.gnat.com) (205.232.38.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-SHA encrypted) ESMTPS; Thu, 20 Aug 2015 12:09:12 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id 5E05429290; Thu, 20 Aug 2015 08:09:10 -0400 (EDT) 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 G9yQUSX4gkK9; Thu, 20 Aug 2015 08:09:10 -0400 (EDT) Received: from joel.gnat.com (localhost.localdomain [127.0.0.1]) by rock.gnat.com (Postfix) with ESMTP id 4CD7A2926D; Thu, 20 Aug 2015 08:09:10 -0400 (EDT) Received: by joel.gnat.com (Postfix, from userid 1000) id CF5F544451; Thu, 20 Aug 2015 08:09:09 -0400 (EDT) Date: Thu, 20 Aug 2015 12:09:00 -0000 From: Joel Brobecker To: Max Filippov Cc: gdb-patches@sourceware.org, Maxim Grigoriev , Woody LaRue , Marc Gauthier Subject: Re: [PATCH] xtensa: initialize call_abi in xtensa_tdep Message-ID: <20150820120909.GC4571@adacore.com> References: <1433628336-24058-1-git-send-email-jcmvbkbc@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1433628336-24058-1-git-send-email-jcmvbkbc@gmail.com> User-Agent: Mutt/1.5.23 (2014-03-12) X-SW-Source: 2015-08/txt/msg00537.txt.bz2 On Sun, Jun 07, 2015 at 01:05:36AM +0300, Max Filippov wrote: > Use XSHAL_ABI value provided by xtensa-config.h to correctly initialize > xtensa_tdep.call_abi > This fixes calls to functions from GDB that otherwise fail with the > following assertion in call0 configuration: > > gdb/regcache.c:602: internal-error: regcache_raw_read: Assertion > `regnum >= 0 && regnum < regcache->descr->nr_raw_registers' failed. > > gdb/ > * xtensa-tdep.h (XTENSA_GDBARCH_TDEP_INSTANTIATE): Initialize > call_abi using XSHAL_ABI macro. I am not sure I understand the patch. The first thing I should mention is that, unless what your patch suggests, the code as I see it on master currently initializes call_abi to CallAbiDefault: .call_abi = CallAbiDefault, \ And looking at the definitions of XSHAL_ABI and XTHAL_ABI_CALL0 in include/xtensa-config.h, those definitions seem to be entirely static... #undef XSHAL_ABI #undef XTHAL_ABI_WINDOWED #undef XTHAL_ABI_CALL0 #define XSHAL_ABI XTHAL_ABI_WINDOWED #define XTHAL_ABI_WINDOWED 0 #define XTHAL_ABI_CALL0 1 ... and resolving to distinct values. So the condition would always be false, resulting in call_abi always being set to CallAbiDefault. Am I missing something? > gdb/xtensa-tdep.h | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/gdb/xtensa-tdep.h b/gdb/xtensa-tdep.h > index adacaf8..3b6ea66 100644 > --- a/gdb/xtensa-tdep.h > +++ b/gdb/xtensa-tdep.h > @@ -246,7 +246,8 @@ struct gdbarch_tdep > .spill_location = -1, \ > .spill_size = (spillsz), \ > .unused = 0, \ > - .call_abi = 0, \ > + .call_abi = (XSHAL_ABI == XTHAL_ABI_CALL0) ? \ > + CallAbiCall0Only : CallAbiDefault, \ Small style issue: binary operators should be at the start of the next line, rather than the end of the current line. Also, for multi-line conditions like that, the GNU Coding Standard asks that we help automatic code formatters by wrapping the expression inside parentheses. These are redundant for the compiler, but help code formatters. Since the parens around the == operator are unecessary, I would have written the above: .call_abi = (XSHAL_ABI == XTHAL_ABI_CALL0 \ ? CallAbiCall0Only : CallAbiDefault), \ People sometime even write it as the following, finding it clearer (although a matter of taste, of course): .call_abi = (XSHAL_ABI == XTHAL_ABI_CALL0 \ ? CallAbiCall0Only \ : CallAbiDefault), \ I personally have no preference. > .debug_interrupt_level = XCHAL_DEBUGLEVEL, \ > .icache_line_bytes = XCHAL_ICACHE_LINESIZE, \ > .dcache_line_bytes = XCHAL_DCACHE_LINESIZE, \ > -- > 1.8.1.4 -- Joel