From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 6280 invoked by alias); 4 Feb 2013 23:57:37 -0000 Received: (qmail 6263 invoked by uid 22791); 4 Feb 2013 23:57:35 -0000 X-SWARE-Spam-Status: No, hits=-3.0 required=5.0 tests=AWL,BAYES_00,KHOP_THREADED,RP_MATCHES_RCVD,TW_EG X-Spam-Check-By: sourceware.org Received: from sibelius.xs4all.nl (HELO glazunov.sibelius.xs4all.nl) (83.163.83.176) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Mon, 04 Feb 2013 23:57:29 +0000 Received: from glazunov.sibelius.xs4all.nl (kettenis@localhost [127.0.0.1]) by glazunov.sibelius.xs4all.nl (8.14.5/8.14.3) with ESMTP id r14NvIrF002732; Tue, 5 Feb 2013 00:57:18 +0100 (CET) Received: (from kettenis@localhost) by glazunov.sibelius.xs4all.nl (8.14.5/8.14.3/Submit) id r14NvFNC023715; Tue, 5 Feb 2013 00:57:15 +0100 (CET) Date: Mon, 04 Feb 2013 23:57:00 -0000 Message-Id: <201302042357.r14NvFNC023715@glazunov.sibelius.xs4all.nl> From: Mark Kettenis To: davem@davemloft.net CC: gdb-patches@sourceware.org In-reply-to: <20130201.161300.1158114789368969492.davem@davemloft.net> (message from David Miller on Fri, 01 Feb 2013 16:13:00 -0500 (EST)) Subject: Re: [PATCH] Allow struct 'return' on 32-bit sparc. References: <20130201.161300.1158114789368969492.davem@davemloft.net> 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-02/txt/msg00104.txt.bz2 > Date: Fri, 01 Feb 2013 16:13:00 -0500 (EST) > From: David Miller > > If gdb cannot write the return value, it pops the stackframe only. > What this means is that the return value we end up with is going to > be essentially random. It does warn you though. > For example, if the calling convention is like 32-bit sparc, a fixed > stack frame slot is used for these return values. Whatever happened > to be there before the function call is the value that we will end up > with. > > Taking a step back I went to look at why gdb can't override the return > value in this situation in the first place. > > Unlike other targets, we can actually properly override the return > value on 32-bit sparc because it is placed in a fixed location offset > from the stack pointer. > > On other targets, the function is passed the memory area address as > an argument and returns that address as a return value. When gdb > pops the stack for the return command, this address return value > won't be set and therefore gdb doesn't have access to the location. > > It looks like the various RETURN_VALUE_* cases were split up > specifically with this exact distinction in mind. Yup. Not sure why I never finished it though. > So I changed it such that return_command will perform a successful > return value override not just for RETURN_VALUE_REGISTER_CONVENTION, > but for RETURN_VALUE_ABI_PRESERVES_ADDRESS as well. > > Then I converted the one target using RETURN_VALUE_ABI_PRESERVES_ADDRESS > to support writing a value in this case. > > Any objections? The sparc-tdep.c bits are correct as far as I can see. I think it would be better if your new function would still have "struct_return" in its name. My suggestion would be "struct_return_convention". Also, > diff --git a/gdb/stack.c b/gdb/stack.c > index c8a6a7e..17950a2 100644 > --- a/gdb/stack.c > +++ b/gdb/stack.c > @@ -2274,6 +2274,7 @@ down_command (char *count_exp, int from_tty) > void > return_command (char *retval_exp, int from_tty) > { > + enum return_value_convention rv_conv; > struct frame_info *thisframe; > struct gdbarch *gdbarch; > struct symbol *thisfun; > @@ -2327,6 +2328,7 @@ return_command (char *retval_exp, int from_tty) > if (thisfun != NULL) > function = read_var_value (thisfun, thisframe); > > + rv_conv = RETURN_VALUE_REGISTER_CONVENTION; > if (TYPE_CODE (return_type) == TYPE_CODE_VOID) > /* If the return-type is "void", don't try to find the > return-value's location. However, do still evaluate the > @@ -2334,14 +2336,18 @@ return_command (char *retval_exp, int from_tty) > is discarded, side effects such as "return i++" still > occur. */ > return_value = NULL; > - else if (thisfun != NULL > - && using_struct_return (gdbarch, function, return_type)) > + else if (thisfun != NULL) > { > - query_prefix = "The location at which to store the " > - "function's return value is unknown.\n" > - "If you continue, the return value " > - "that you specified will be ignored.\n"; > - return_value = NULL; > + rv_conv = convention_for_return (gdbarch, function, return_type); > + if (rv_conv == RETURN_VALUE_STRUCT_CONVENTION > + || rv_conv == RETURN_VALUE_ABI_RETURNS_ADDRESS) > + { > + query_prefix = "The location at which to store the " > + "function's return value is unknown.\n" > + "If you continue, the return value " > + "that you specified will be ignored.\n"; > + return_value = NULL; > + } > } > } > > @@ -2371,9 +2377,8 @@ return_command (char *retval_exp, int from_tty) > struct type *return_type = value_type (return_value); > struct gdbarch *gdbarch = get_regcache_arch (get_current_regcache ()); > > - gdb_assert (gdbarch_return_value (gdbarch, function, return_type, NULL, > - NULL, NULL) > - == RETURN_VALUE_REGISTER_CONVENTION); > + gdb_assert (rv_conv != RETURN_VALUE_STRUCT_CONVENTION > + && rv_conv != RETURN_VALUE_ABI_RETURNS_ADDRESS); > gdbarch_return_value (gdbarch, function, return_type, > get_current_regcache (), NULL /*read*/, > value_contents (return_value) /*write*/); > diff --git a/gdb/value.c b/gdb/value.c > index dbf1c37..07b7dbf 100644 > --- a/gdb/value.c > +++ b/gdb/value.c > @@ -3323,13 +3323,12 @@ coerce_array (struct value *arg) > } > > > -/* Return true if the function returning the specified type is using > - the convention of returning structures in memory (passing in the > - address as a hidden first parameter). */ > +/* Return the return value convention that will be used for the > + specified type. */ > > -int > -using_struct_return (struct gdbarch *gdbarch, > - struct value *function, struct type *value_type) > +enum return_value_convention > +convention_for_return (struct gdbarch *gdbarch, > + struct value *function, struct type *value_type) > { > enum type_code code = TYPE_CODE (value_type); > > @@ -3339,11 +3338,22 @@ using_struct_return (struct gdbarch *gdbarch, > if (code == TYPE_CODE_VOID) > /* A void return value is never in memory. See also corresponding > code in "print_return_value". */ > - return 0; > + return RETURN_VALUE_REGISTER_CONVENTION; Return RETURN_VALUE_REGISTER_CONVENTION if there is nothing to return seems to be a bit odd. But since convention_for_return() is never called with code being TYPE_CODE (there is an explicit check for that in stack.c:return_command()) you could simply leave the check in using_struct_return().