Use GCC5/DWARF5 DW_AT_noreturn to mark functions that don't return normally.
Commit Message
On Thu, 2014-12-04 at 14:02 +0000, Pedro Alves wrote:
> On 11/27/2014 02:53 PM, Mark Wielaard wrote:
> > Add a flag field is_noreturn to struct func_type. Make calling_convention
> > a small bit field to not increase the size of the struct. Set is_noreturn
> > if the new GCC5/DWARF5 DW_AT_noreturn is set on a DW_TAG_subprogram.
> > Use this information to warn the user before doing a finish or return from
> > a function that does not return normally to its caller.
> >
> > (gdb) finish
> > Warning. Function endless does not return normally.
> > Try to finish anyway? (y or n)
> >
> > (gdb) return
> > warning: Function does not return normally to caller!
> > Make endless return now? (y or n)
>
> I'd suggest making the warnings a bit more consistent.
>
> - "Warning." vs "warning: "
>
> Prefer the latter, as that's what the "warning" function uses.
>
> - "." vs "!"
>
> I'd keep it calm and get rid of the "!". :-)
Fixed both.
> >
> > gdb/ChangeLog
> >
> > * dwarf2read.c (read_subroutine_type): Set TYPE_NO_RETURN from
> > DW_AT_noreturn.
> > * gdbtypes.h (struct func_type): Add is_noreturn field flag. Make
> > calling_convention an 8 bit bit field.
> > (TYPE_NO_RETURN): New macro.
> > * infcmd.c (finish_command): Query if function does not return
> > normally.
> > * stack.c (return_command): Likewise.
> >
> > include/ChangeLog
> >
> > * dwarf2.def (DW_AT_noreturn): New DWARF5 attribute.
>
> I wonder if we could have a test? Could e.g., make sure we don't
> crash when the user confirms a return in a noreturn function.
I am not sure how to write such a test. This is mainly interactive code,
which will only trigger from_tty. I also am not sure such a test really
tests this new feature. Trying to return from a noreturn function
triggers undefined behavior. GDB probably won't crash, but the inferior
might since the result is unpredictable (that is precisely why I added
this, you forcibly return from a function and end up somewhere
unexpected). Which makes testing the expected output of the user
ignoring the warning somewhat hard.
> > --- a/gdb/infcmd.c
> > +++ b/gdb/infcmd.c
> > @@ -1869,7 +1869,14 @@ finish_command (char *arg, int from_tty)
> > if (execution_direction == EXEC_REVERSE)
> > printf_filtered (_("Run back to call of "));
> > else
> > - printf_filtered (_("Run till exit from "));
> > + {
> > + if (function != NULL && TYPE_NO_RETURN (function->type)
> > + && ! query (_("Warning. Function %s does not return normally.\n"
> > + "Try to finish anyway? "),
> > + SYMBOL_PRINT_NAME (function)))
> > + error (_("Not confirmed."));
> > + printf_filtered (_("Run till exit from "));
> > + }
>
> No space between "!" and query:
>
> && !query ...
Fixed.
> > --- a/gdb/stack.c
> > +++ b/gdb/stack.c
> > @@ -2462,8 +2462,12 @@ return_command (char *retval_exp, int from_tty)
> > confirmed = query (_("%sMake selected stack frame return now? "),
> > query_prefix);
> > else
> > - confirmed = query (_("%sMake %s return now? "), query_prefix,
> > - SYMBOL_PRINT_NAME (thisfun));
> > + {
> > + if (TYPE_NO_RETURN (thisfun->type))
> > + warning ("Function does not return normally to caller!");
>
> i18n / _() .
Added.
Rebased patch attached.
Thanks,
Mark
Comments
On 12/9/14, 2:49 AM, Mark Wielaard wrote:
> On Thu, 2014-12-04 at 14:02 +0000, Pedro Alves wrote:
>> On 11/27/2014 02:53 PM, Mark Wielaard wrote:
>>> Add a flag field is_noreturn to struct func_type. Make calling_convention
>>> a small bit field to not increase the size of the struct. Set is_noreturn
>>> if the new GCC5/DWARF5 DW_AT_noreturn is set on a DW_TAG_subprogram.
>>> Use this information to warn the user before doing a finish or return from
>>> a function that does not return normally to its caller.
>>>
>>> (gdb) finish
>>> Warning. Function endless does not return normally.
>>> Try to finish anyway? (y or n)
>>>
>>> (gdb) return
>>> warning: Function does not return normally to caller!
>>> Make endless return now? (y or n)
>>
>> I'd suggest making the warnings a bit more consistent.
>>
>> - "Warning." vs "warning: "
>>
>> Prefer the latter, as that's what the "warning" function uses.
>>
>> - "." vs "!"
>>
>> I'd keep it calm and get rid of the "!". :-)
>
> Fixed both.
>
>>>
>>> gdb/ChangeLog
>>>
>>> * dwarf2read.c (read_subroutine_type): Set TYPE_NO_RETURN from
>>> DW_AT_noreturn.
>>> * gdbtypes.h (struct func_type): Add is_noreturn field flag. Make
>>> calling_convention an 8 bit bit field.
>>> (TYPE_NO_RETURN): New macro.
>>> * infcmd.c (finish_command): Query if function does not return
>>> normally.
>>> * stack.c (return_command): Likewise.
>>>
>>> include/ChangeLog
>>>
>>> * dwarf2.def (DW_AT_noreturn): New DWARF5 attribute.
>>
>> I wonder if we could have a test? Could e.g., make sure we don't
>> crash when the user confirms a return in a noreturn function.
>
> I am not sure how to write such a test. This is mainly interactive code,
> which will only trigger from_tty. I also am not sure such a test really
> tests this new feature. Trying to return from a noreturn function
> triggers undefined behavior. GDB probably won't crash, but the inferior
> might since the result is unpredictable (that is precisely why I added
> this, you forcibly return from a function and end up somewhere
> unexpected). Which makes testing the expected output of the user
> ignoring the warning somewhat hard.
Chiming in here, just write the test so that it passes whether or not
the inferior crashes - as you note, its behavior is undefined anyway.
If GDB crashes or hangs, on any platform, that's a bug that we have to
fix in GDB.
Stan
stan@codesourcery.com
Hi Stan,
On Tue, 2014-12-09 at 10:57 -0800, Stan Shebs wrote:
> On 12/9/14, 2:49 AM, Mark Wielaard wrote:
> > On Thu, 2014-12-04 at 14:02 +0000, Pedro Alves wrote:
> >> I wonder if we could have a test? Could e.g., make sure we don't
> >> crash when the user confirms a return in a noreturn function.
> >
> > I am not sure how to write such a test. This is mainly interactive code,
> > which will only trigger from_tty. I also am not sure such a test really
> > tests this new feature. Trying to return from a noreturn function
> > triggers undefined behavior. GDB probably won't crash, but the inferior
> > might since the result is unpredictable (that is precisely why I added
> > this, you forcibly return from a function and end up somewhere
> > unexpected). Which makes testing the expected output of the user
> > ignoring the warning somewhat hard.
>
> Chiming in here, just write the test so that it passes whether or not
> the inferior crashes - as you note, its behavior is undefined anyway.
> If GDB crashes or hangs, on any platform, that's a bug that we have to
> fix in GDB.
I am afraid I still don't understand what we would be testing (whether
the user if there is a tty gets to say yes or no?) or how to write such
a test where we don't seem interested in the actual result. Is there an
example to follow?
Thanks,
Mark
On 12/10/2014 11:24 AM, Mark Wielaard wrote:
> Hi Stan,
>
> On Tue, 2014-12-09 at 10:57 -0800, Stan Shebs wrote:
>> On 12/9/14, 2:49 AM, Mark Wielaard wrote:
>>> On Thu, 2014-12-04 at 14:02 +0000, Pedro Alves wrote:
>>>> I wonder if we could have a test? Could e.g., make sure we don't
>>>> crash when the user confirms a return in a noreturn function.
>>>
>>> I am not sure how to write such a test. This is mainly interactive code,
>>> which will only trigger from_tty. I also am not sure such a test really
>>> tests this new feature. Trying to return from a noreturn function
>>> triggers undefined behavior. GDB probably won't crash, but the inferior
>>> might since the result is unpredictable (that is precisely why I added
>>> this, you forcibly return from a function and end up somewhere
>>> unexpected). Which makes testing the expected output of the user
>>> ignoring the warning somewhat hard.
>>
>> Chiming in here, just write the test so that it passes whether or not
>> the inferior crashes - as you note, its behavior is undefined anyway.
>> If GDB crashes or hangs, on any platform, that's a bug that we have to
>> fix in GDB.
>
> I am afraid I still don't understand what we would be testing (whether
> the user if there is a tty gets to say yes or no?) or how to write such
> a test where we don't seem interested in the actual result. Is there an
> example to follow?
The point is just to test the new/related code paths, and making sure
the commands do what you propose they do.
E.g., to make sure the new dwarf code (and future changes around it) don't
crash. And then in the "return->no" path, make sure we really cancel the
command (that the backtrace looks the same after canceling), which also potentially
exercises bad/missing cleanups, for example. By testing the "return->yes" path,
we'd be making sure the unwinder doesn't crash (now and in the future)
in this situation, and that the current frame ends up being the one expected.
For the "finish" case, we'd be making sure that GDB doesn't crash when trying
to figure out where the function's return value is stored ("finish" prints the
function's return value when the function actually finishes).
There are some tests for "return" in the testsuite -- return.exp or
return2.exp -- and at least one for "finish" -- finish.exp. I'd suggest
using these as inspiration.
Thanks,
Pedro Alves
From 6b454703a5a53bee1355b5a871a10f6bbfcde64e Mon Sep 17 00:00:00 2001
From: Mark Wielaard <mjw@redhat.com>
Date: Tue, 9 Dec 2014 11:45:41 +0100
Subject: [PATCH] Use GCC5/DWARF5 DW_AT_noreturn to mark functions that don't
return normally.
Add a flag field is_noreturn to struct func_type. Make calling_convention
a small bit field to not increase the size of the struct. Set is_noreturn
if the new GCC5/DWARF5 DW_AT_noreturn is set on a DW_TAG_subprogram.
Use this information to warn the user before doing a finish or return from
a function that does not return normally to its caller.
(gdb) finish
warning: Function endless does not return normally.
Try to finish anyway? (y or n)
(gdb) return
warning: Function does not return normally to caller.
Make endless return now? (y or n)
gdb/ChangeLog
* dwarf2read.c (read_subroutine_type): Set TYPE_NO_RETURN from
DW_AT_noreturn.
* gdbtypes.h (struct func_type): Add is_noreturn field flag. Make
calling_convention an 8 bit bit field.
(TYPE_NO_RETURN): New macro.
* infcmd.c (finish_command): Query if function does not return
normally.
* stack.c (return_command): Likewise.
include/ChangeLog
* dwarf2.def (DW_AT_noreturn): New DWARF5 attribute.
The dwarf2.h addition and the code to emit the new attribute is already in
the gcc tree.
---
gdb/ChangeLog | 11 +++++++++++
gdb/dwarf2read.c | 6 ++++++
gdb/gdbtypes.h | 12 ++++++++++--
gdb/infcmd.c | 9 ++++++++-
gdb/stack.c | 8 ++++++--
include/ChangeLog | 4 ++++
include/dwarf2.def | 2 ++
7 files changed, 47 insertions(+), 5 deletions(-)
@@ -1,3 +1,14 @@
+2014-12-09 Mark Wielaard <mjw@redhat.com>
+
+ * dwarf2read.c (read_subroutine_type): Set TYPE_NO_RETURN from
+ DW_AT_noreturn.
+ * gdbtypes.h (struct func_type): Add is_noreturn field flag. Make
+ calling_convention an 8 bit bit field.
+ (TYPE_NO_RETURN): New macro.
+ * infcmd.c (finish_command): Query if function does not return
+ normally.
+ * stack.c (return_command): Likewise.
+
2014-12-08 Doug Evans <dje@google.com>
* python/py-objfile.c (objfpy_get_owner): Increment refcount of result.
@@ -14308,6 +14308,12 @@ read_subroutine_type (struct die_info *die, struct dwarf2_cu *cu)
else
TYPE_CALLING_CONVENTION (ftype) = DW_CC_normal;
+ /* Record whether the function returns normally to its caller or not
+ if the DWARF producer set that information. */
+ attr = dwarf2_attr (die, DW_AT_noreturn, cu);
+ if (attr && (DW_UNSND (attr) != 0))
+ TYPE_NO_RETURN (ftype) = 1;
+
/* We need to add the subroutine type to the die immediately so
we don't infinitely recurse when dealing with parameters
declared as the same subroutine type. */
@@ -1019,9 +1019,16 @@ struct func_type
{
/* * The calling convention for targets supporting multiple ABIs.
Right now this is only fetched from the Dwarf-2
- DW_AT_calling_convention attribute. */
+ DW_AT_calling_convention attribute. The value is one of the
+ DW_CC enum dwarf_calling_convention constants. */
- unsigned calling_convention;
+ unsigned calling_convention : 8;
+
+ /* * Whether this function normally returns to its caller. It is
+ set from the DW_AT_noreturn attribute if set on the
+ DW_TAG_subprogram. */
+
+ unsigned int is_noreturn : 1;
/* * Only those DW_TAG_GNU_call_site's in this function that have
DW_AT_GNU_tail_call set are linked in this list. Function
@@ -1245,6 +1252,7 @@ extern void allocate_gnat_aux_type (struct type *);
#define TYPE_GNAT_SPECIFIC(thistype) TYPE_MAIN_TYPE(thistype)->type_specific.gnat_stuff
#define TYPE_DESCRIPTIVE_TYPE(thistype) TYPE_GNAT_SPECIFIC(thistype)->descriptive_type
#define TYPE_CALLING_CONVENTION(thistype) TYPE_MAIN_TYPE(thistype)->type_specific.func_stuff->calling_convention
+#define TYPE_NO_RETURN(thistype) TYPE_MAIN_TYPE(thistype)->type_specific.func_stuff->is_noreturn
#define TYPE_TAIL_CALL_LIST(thistype) TYPE_MAIN_TYPE(thistype)->type_specific.func_stuff->tail_call_list
#define TYPE_BASECLASS(thistype,index) TYPE_FIELD_TYPE(thistype, index)
#define TYPE_N_BASECLASSES(thistype) TYPE_CPLUS_SPECIFIC(thistype)->n_baseclasses
@@ -1869,7 +1869,14 @@ finish_command (char *arg, int from_tty)
if (execution_direction == EXEC_REVERSE)
printf_filtered (_("Run back to call of "));
else
- printf_filtered (_("Run till exit from "));
+ {
+ if (function != NULL && TYPE_NO_RETURN (function->type)
+ && !query (_("warning: Function %s does not return normally.\n"
+ "Try to finish anyway? "),
+ SYMBOL_PRINT_NAME (function)))
+ error (_("Not confirmed."));
+ printf_filtered (_("Run till exit from "));
+ }
print_stack_frame (get_selected_frame (NULL), 1, LOCATION, 0);
}
@@ -2462,8 +2462,12 @@ return_command (char *retval_exp, int from_tty)
confirmed = query (_("%sMake selected stack frame return now? "),
query_prefix);
else
- confirmed = query (_("%sMake %s return now? "), query_prefix,
- SYMBOL_PRINT_NAME (thisfun));
+ {
+ if (TYPE_NO_RETURN (thisfun->type))
+ warning ("Function does not return normally to caller.");
+ confirmed = query (_("%sMake %s return now? "), query_prefix,
+ SYMBOL_PRINT_NAME (thisfun));
+ }
if (!confirmed)
error (_("Not confirmed"));
}
@@ -1,3 +1,7 @@
+2014-12-09 Mark Wielaard <mjw@redhat.com>
+
+ * dwarf2.def (DW_AT_noreturn): New DWARF5 attribute.
+
2014-12-06 Eric Botcazou <ebotcazou@adacore.com>
* dis-asm.h (print_insn_visium): Declare.
@@ -308,6 +308,8 @@ DW_AT (DW_AT_data_bit_offset, 0x6b)
DW_AT (DW_AT_const_expr, 0x6c)
DW_AT (DW_AT_enum_class, 0x6d)
DW_AT (DW_AT_linkage_name, 0x6e)
+/* DWARF 5. */
+DW_AT (DW_AT_noreturn, 0x87)
DW_AT_DUP (DW_AT_lo_user, 0x2000) /* Implementation-defined range start. */
DW_AT_DUP (DW_AT_hi_user, 0x3fff) /* Implementation-defined range end. */
--
1.8.3.1