Message ID | 1424957146-25705-1-git-send-email-antoine.tremblay@ericsson.com |
---|---|
State | New, archived |
Headers |
Received: (qmail 97668 invoked by alias); 26 Feb 2015 13:25:58 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: <gdb-patches.sourceware.org> List-Unsubscribe: <mailto:gdb-patches-unsubscribe-##L=##H@sourceware.org> List-Subscribe: <mailto:gdb-patches-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/gdb-patches/> List-Post: <mailto:gdb-patches@sourceware.org> List-Help: <mailto:gdb-patches-help@sourceware.org>, <http://sourceware.org/ml/#faqs> Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 97655 invoked by uid 89); 26 Feb 2015 13:25:57 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.0 required=5.0 tests=AWL, BAYES_00, SPF_PASS autolearn=ham version=3.3.2 X-HELO: usevmg21.ericsson.net Received: from usevmg21.ericsson.net (HELO usevmg21.ericsson.net) (198.24.6.65) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-SHA encrypted) ESMTPS; Thu, 26 Feb 2015 13:25:56 +0000 Received: from EUSAAHC002.ericsson.se (Unknown_Domain [147.117.188.78]) by usevmg21.ericsson.net (Symantec Mail Security) with SMTP id C2.53.25146.35EBEE45; Thu, 26 Feb 2015 07:33:55 +0100 (CET) Received: from elx1123rzr-fp.mo.ca.am.ericsson.se (147.117.188.8) by smtps-am.internal.ericsson.com (147.117.188.78) with Microsoft SMTP Server (TLS) id 14.3.210.2; Thu, 26 Feb 2015 08:25:53 -0500 From: Antoine Tremblay <antoine.tremblay@ericsson.com> To: <gdb-patches@sourceware.org> CC: Antoine Tremblay <antoine.tremblay@ericsson.com> Subject: [PATCH] Fix print of value type in a corner case of finish Date: Thu, 26 Feb 2015 08:25:46 -0500 Message-ID: <1424957146-25705-1-git-send-email-antoine.tremblay@ericsson.com> MIME-Version: 1.0 Content-Type: text/plain X-IsSubscribed: yes |
Commit Message
Antoine Tremblay
Feb. 26, 2015, 1:25 p.m. UTC
When doing finish in a function, if gdb fails to return a value, gdb also fails at printing the value type if this type is a struct. For example : (gdb) fin .... Value returned has type: . Cannot determine contents This patch fixes this by calling type_to_string to print the type so that we can support these types. This patch returns the following example output : (gdb) fin .... Value returned has type: struct test. Cannot determine contents gdb/ChangeLog: * gdb/infcmd.c (print_return_value): use type_to_string to print type. --- gdb/infcmd.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
Comments
It there a way to add a test? Also please use "const char *" for type_name, so C++ doesn't complain. :-) Thanks, Pedro Alves On 02/26/2015 01:25 PM, Antoine Tremblay wrote: > When doing finish in a function, if gdb fails to return a value, gdb > also fails at printing the value type if this type is a struct. > > For example : > > (gdb) fin > .... > Value returned has type: . Cannot determine contents > > This patch fixes this by calling type_to_string to print the type > so that we can support these types. > > This patch returns the following example output : > > (gdb) fin > .... > Value returned has type: struct test. Cannot determine contents > > gdb/ChangeLog: > * gdb/infcmd.c (print_return_value): use type_to_string to print type. > --- > gdb/infcmd.c | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/gdb/infcmd.c b/gdb/infcmd.c > index 9a1fb8d..3737b8f 100644 > --- a/gdb/infcmd.c > +++ b/gdb/infcmd.c > @@ -1607,10 +1607,16 @@ print_return_value (struct value *function, struct type *value_type) > } > else > { > + struct cleanup *oldchain; > + char *type_name; > + > + type_name = type_to_string (value_type); > + oldchain = make_cleanup (xfree, type_name); > ui_out_text (uiout, "Value returned has type: "); > - ui_out_field_string (uiout, "return-type", TYPE_NAME (value_type)); > + ui_out_field_string (uiout, "return-type", type_name); > ui_out_text (uiout, "."); > ui_out_text (uiout, " Cannot determine contents\n"); > + do_cleanups (oldchain); > } > } > >
On 02/26/2015 08:35 AM, Pedro Alves wrote: > It there a way to add a test? I would need to add a test that's specific to an architecture that does not support the struct return value... I'll see what I can figure out.. > Also please use "const char *" for type_name, so C++ > doesn't complain. :-) Humm not sure I understand here since type_to_string returns a char* that needs to be freed ? Converting to const char* would make make_cleanup complain... How is C++ complaining ? > Thanks, > Pedro Alves > > On 02/26/2015 01:25 PM, Antoine Tremblay wrote: >> When doing finish in a function, if gdb fails to return a value, gdb >> also fails at printing the value type if this type is a struct. >> >> For example : >> >> (gdb) fin >> .... >> Value returned has type: . Cannot determine contents >> >> This patch fixes this by calling type_to_string to print the type >> so that we can support these types. >> >> This patch returns the following example output : >> >> (gdb) fin >> .... >> Value returned has type: struct test. Cannot determine contents >> >> gdb/ChangeLog: >> * gdb/infcmd.c (print_return_value): use type_to_string to print type. >> --- >> gdb/infcmd.c | 8 +++++++- >> 1 file changed, 7 insertions(+), 1 deletion(-) >> >> diff --git a/gdb/infcmd.c b/gdb/infcmd.c >> index 9a1fb8d..3737b8f 100644 >> --- a/gdb/infcmd.c >> +++ b/gdb/infcmd.c >> @@ -1607,10 +1607,16 @@ print_return_value (struct value *function, struct type *value_type) >> } >> else >> { >> + struct cleanup *oldchain; >> + char *type_name; >> + >> + type_name = type_to_string (value_type); >> + oldchain = make_cleanup (xfree, type_name); >> ui_out_text (uiout, "Value returned has type: "); >> - ui_out_field_string (uiout, "return-type", TYPE_NAME (value_type)); >> + ui_out_field_string (uiout, "return-type", type_name); >> ui_out_text (uiout, "."); >> ui_out_text (uiout, " Cannot determine contents\n"); >> + do_cleanups (oldchain); >> } >> } >> >>
On 02/26/2015 02:11 PM, Antoine Tremblay wrote: > On 02/26/2015 08:35 AM, Pedro Alves wrote: >> It there a way to add a test? > I would need to add a test that's specific to an architecture that does > not support the struct return value... Hmm, AFAICS, it's not that the architecture doesn't support the struct return value, or some kind of memory read error or some such. It's that the ABI returns the object using struct return convention / RETURN_VALUE_STRUCT_CONVENTION: /* Return the value of the result of a function at the end of a 'finish' command/BP. */ struct value * get_return_value (struct value *function, struct type *value_type) { ... switch (gdbarch_return_value (gdbarch, function, value_type, NULL, NULL, NULL)) { ... break; case RETURN_VALUE_STRUCT_CONVENTION: value = NULL; break; I think that we make the test return a big enough structure, we'll hit RETURN_VALUE_STRUCT_CONVENTION on most architectures (but not x86...)? For archs that don't do RETURN_VALUE_STRUCT_CONVENTION, the test could cope, by also passing if GDB manages to extract the real value. In sum, PASS if real value, or at least type. FAIL otherwise. But, ... I'm actually very much surprised we don't test this already. So I did $ grep -rn "\"finish\"" testsuite/ and noticed: ... gdb.base/structs.exp:259:# "finish") and correctly extract/store any corresponding gdb.base/structs.exp:276:# is in three parts: test "return"; test "finish"; check that the two gdb.base/structs.exp:299: # consistency between this and the "finish" case. gdb.base/structs.exp:404: # Check that a "finish" works. gdb.base/structs.exp:427: gdb_test_multiple "finish" "${test}" { gdb.base/structs.exp:467: # Since "finish" works in more cases than "return" (see gdb.base/structs.exp:470: # known implies that the "finish" value is known (but not the ... And lo: set test "finish foo<n>; ${tests}" set finish_value_known 1 gdb_test_multiple "finish" "${test}" { -re "Value returned is .*${gdb_prompt} $" { pass "${test}" } -re "Cannot determine contents.*${gdb_prompt} $" { # Expected bad value. For the moment this is ok. set finish_value_known 0 pass "${test}" } } So the regex here is too lax and missed this bug. Could you tweak it to make sure some type was output? > >> Also please use "const char *" for type_name, so C++ >> doesn't complain. :-) > Humm not sure I understand here since type_to_string returns a char* > that needs to be freed ? > > Converting to const char* would make make_cleanup complain... > > How is C++ complaining ? Oh, nevermind, that was a thinko on my part, sorry. Thanks, Pedro Alves
On 02/26/2015 10:18 AM, Pedro Alves wrote: > I think that we make the test return a big enough > structure, we'll hit RETURN_VALUE_STRUCT_CONVENTION on most > architectures (but not x86...)? Yes that's what led me to believe I needed something arch specific but indeed it may not totally be.. > So the regex here is too lax and missed this bug. Could you > tweak it to make sure some type was output? Wow I had totally missed that , Thanks!! Patch v2 coming up in a few secs... In that patch I check that I actually get the right struct type :) (I tested the patch by simulating the condition and it works fine) Regards, Antoine
diff --git a/gdb/infcmd.c b/gdb/infcmd.c index 9a1fb8d..3737b8f 100644 --- a/gdb/infcmd.c +++ b/gdb/infcmd.c @@ -1607,10 +1607,16 @@ print_return_value (struct value *function, struct type *value_type) } else { + struct cleanup *oldchain; + char *type_name; + + type_name = type_to_string (value_type); + oldchain = make_cleanup (xfree, type_name); ui_out_text (uiout, "Value returned has type: "); - ui_out_field_string (uiout, "return-type", TYPE_NAME (value_type)); + ui_out_field_string (uiout, "return-type", type_name); ui_out_text (uiout, "."); ui_out_text (uiout, " Cannot determine contents\n"); + do_cleanups (oldchain); } }