From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 54474 invoked by alias); 1 Apr 2018 04:22:51 -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 54446 invoked by uid 89); 1 Apr 2018 04:22:49 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-25.4 required=5.0 tests=BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,KAM_STOCKGEN,SPF_HELO_PASS,SPF_PASS,T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 spammy=ptype, exposes, teaches X-HELO: simark.ca Received: from simark.ca (HELO simark.ca) (158.69.221.121) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Sun, 01 Apr 2018 04:22:48 +0000 Received: from [10.0.0.11] (unknown [192.222.164.54]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id 4FDAD1E030; Sun, 1 Apr 2018 00:22:46 -0400 (EDT) Subject: Re: [PATCH v2 03/15] Calling ifunc functions when target has no debug info but resolver has To: Pedro Alves , gdb-patches@sourceware.org References: <20180325191943.8246-1-palves@redhat.com> <20180325191943.8246-4-palves@redhat.com> From: Simon Marchi Message-ID: <805a18bb-d85d-f115-2e27-970ac8e8523c@simark.ca> Date: Sun, 01 Apr 2018 04:22:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.7.0 MIME-Version: 1.0 In-Reply-To: <20180325191943.8246-4-palves@redhat.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-SW-Source: 2018-04/txt/msg00009.txt.bz2 On 2018-03-25 03:19 PM, Pedro Alves wrote: > In v2: > - Added testsuite tweak. > > After the previous patch, on Fedora 27 (glibc 2.26), if you try > calling strlen in the inferior, you now get: > > (top-gdb) p strlen ("hello") > '__strlen_avx2' has unknown return type; cast the call to its declared return type > > This is correct, because __strlen_avx2 is written in assembly. > > We can improve on this though -- if the final ifunc resolved/target > function has no debug info, but the ifunc _resolver_ does have debug > info, we can try extracting the final function's type from the type > that the resolver returns. E.g.,: > > typedef size_t (*strlen_t) (const char*); > > size_t my_strlen (const char *) { /* some implementation */ } > strlen_t strlen_resolver (unsigned long hwcap) { return my_strlen; } > > extern size_t strlen (const char *s); > __typeof (strlen) strlen __attribute__ ((ifunc ("strlen_resolver"))); > > In the strlen example above, the resolver returns strlen_t, which is a > typedef for pointer to a function that returns size_t. "strlen_t" is > the type of both the user-visible "strlen", and of the the target > function that implements it. > > This patch teaches GDB to extract that type. > > This is done for actual inferior function calls (in infcall.c), and > for ptype (in eval_call). By the time we get to either of these > places, we've already lost the original symbol/minsym, and only have > values and types to work with. Hence the changes to c-exp.y and > evaluate_var_msym_value, to ensure that we propagate the ifunc > minsymbol's info. > > The change to make ifunc symbols have no/unknown return type exposes a > latent problem -- gdb.compile/compile-ifunc.exp calls a no-debug-info > function, but we did not warn about it. The test is fixed by this > commit too. As usual, what you did seems to make sense, but I'm a bit lost. I noted some random comments. > diff --git a/gdb/blockframe.c b/gdb/blockframe.c > index 9be8871f756..db02b35742d 100644 > --- a/gdb/blockframe.c > +++ b/gdb/blockframe.c > @@ -323,6 +323,40 @@ find_pc_partial_function (CORE_ADDR pc, const char **name, CORE_ADDR *address, > return find_pc_partial_function_gnu_ifunc (pc, name, address, endaddr, NULL); > } > > +/* See symtab.h. */ > + > +struct type * > +find_gnu_ifunc_target_type (CORE_ADDR resolver_funaddr) > +{ > + /* See if we can figure out the function's return type from the type > + that the resolver returns. */ > + symbol *sym = find_pc_function (resolver_funaddr); > + if (sym != NULL > + && SYMBOL_CLASS (sym) == LOC_BLOCK > + && BLOCK_START (SYMBOL_BLOCK_VALUE (sym)) == resolver_funaddr) > + { This looks a lot like the "find_function_type" function. Maybe it should use it? > @@ -864,7 +878,11 @@ call_function_by_hand_dummy (struct value *function, > } > } > > - funaddr = find_function_addr (function, &values_type); > + struct type *ftype = check_typedef (value_type (function)); > + if (TYPE_CODE (ftype) == TYPE_CODE_PTR) > + ftype = check_typedef (TYPE_TARGET_TYPE (ftype)); Are these last operations necessary to do here? It seems to me like find_function_addr will do pretty much the same work and ignore the input value of ftype anyway. > + > + funaddr = find_function_addr (function, &values_type, &ftype); > if (values_type == NULL) > values_type = default_return_type; > if (values_type == NULL) > diff --git a/gdb/infcall.h b/gdb/infcall.h > index a3861fb1bf3..bea1494b50d 100644 > --- a/gdb/infcall.h > +++ b/gdb/infcall.h > @@ -25,8 +25,15 @@ > struct value; > struct type; > > +/* Determine a function's address and its return type from its value. > + If the function is a GNU ifunc, then return the address of the > + target function, and set *FUNCTION_TYPE to the target function's > + type, and *RETVAL_TYPE to the target function's return type.. > + Calls error() if the function is not valid for calling. */ > + > extern CORE_ADDR find_function_addr (struct value *function, > - struct type **retval_type); > + struct type **retval_type, > + struct type **function_type = NULL); Isn't the function's return value type always the target type of the function's type? If so, it seems a bit redundant to have both retval_type and function_type. The callers easily call TYPE_TARGET_TYPE on *function_type. Or maybe do you see some situations where we're able to determine the reval_type but not the function_type, in which case the retval_type would still be relevant? Simon