This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH] xtensa: initialize call_abi in xtensa_tdep
- From: Max Filippov <jcmvbkbc at gmail dot com>
- To: Joel Brobecker <brobecker at adacore dot com>
- Cc: gdb-patches at sourceware dot org, Maxim Grigoriev <maxim2405 at gmail dot com>, Woody LaRue <larue at cadence dot com>, Marc Gauthier <marc at cadence dot com>
- Date: Thu, 20 Aug 2015 15:47:01 +0300
- Subject: Re: [PATCH] xtensa: initialize call_abi in xtensa_tdep
- Authentication-results: sourceware.org; auth=none
- References: <1433628336-24058-1-git-send-email-jcmvbkbc at gmail dot com> <20150820120909 dot GC4571 at adacore dot com>
On Thu, Aug 20, 2015 at 3:09 PM, Joel Brobecker <brobecker@adacore.com> wrote:
> 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?
The file include/xtensa-config.h is meant to be replaced by processor-specific
version where XSHAL_ABI definition may be different. That's why I mention
'call0 configuration' in the description: when xtensa core is configured without
windowed registers it will necessarily have call0 ABI.
>> 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.
I'll send fixed v2.
--
Thanks.
-- Max