diff mbox

DWARF add DW_AT_noreturn on noreturn function subprogram.

Message ID 1416956221-26727-1-git-send-email-mjw@redhat.com
State New
Headers show

Commit Message

Mark Wielaard Nov. 25, 2014, 10:57 p.m. UTC
This implements the DWARFv5 noreturn proposal:
http://dwarfstd.org/ShowIssue.php?issue=140331.1

TREE_THIS_VOLATILE on a FUNCTION_DECL node means the function does not
return normally. This catches the traditional noreturn GNU attribute,
the C11 _Noreturn keyword and the C++11 [[noreturn]] attribute.

This relies on the DW_AT_noreturn constant defined in the DWARFv5 DRAFT:
http://www.dwarfstd.org/doc/dwarf5.20141029.pdf
We could also use a new GNU extension if we don't trust these constants
to be stable.

gcc/ChangeLog

	* dwarf2out.c (gen_subprogram_die): Add DW_AT_noreturn when the
	function decl has TREE_THIS_VOLATILE.

gcc/testsuite/ChangeLog

	* g++.dg/debug/dwarf2/noreturn-function.C: New test.
	* gcc.dg/debug/dwarf2/noreturn-function-attribute.c: Likewise.
	* gcc.dg/debug/dwarf2/noreturn-function-keyword.c: Likewise.

include/ChangeLog

	* dwarf2.def (DW_AT_noreturn): New DWARF5 attribute.

Comments

Jason Merrill Nov. 26, 2014, 1:26 a.m. UTC | #1
On 11/25/2014 05:57 PM, Mark Wielaard wrote:
> This implements the DWARFv5 noreturn proposal:
> http://dwarfstd.org/ShowIssue.php?issue=140331.1
>
> TREE_THIS_VOLATILE on a FUNCTION_DECL node means the function does not
> return normally. This catches the traditional noreturn GNU attribute,
> the C11 _Noreturn keyword and the C++11 [[noreturn]] attribute.
>
> This relies on the DW_AT_noreturn constant defined in the DWARFv5 DRAFT:
> http://www.dwarfstd.org/doc/dwarf5.20141029.pdf
> We could also use a new GNU extension if we don't trust these constants
> to be stable.

Looks good to me.  I think we can expect these constants to stay the same.

Jason
Jeff Law Dec. 1, 2014, 9:31 p.m. UTC | #2
On 11/25/14 15:57, Mark Wielaard wrote:
> This implements the DWARFv5 noreturn proposal:
> http://dwarfstd.org/ShowIssue.php?issue=140331.1
>
> TREE_THIS_VOLATILE on a FUNCTION_DECL node means the function does not
> return normally. This catches the traditional noreturn GNU attribute,
> the C11 _Noreturn keyword and the C++11 [[noreturn]] attribute.
>
> This relies on the DW_AT_noreturn constant defined in the DWARFv5 DRAFT:
> http://www.dwarfstd.org/doc/dwarf5.20141029.pdf
> We could also use a new GNU extension if we don't trust these constants
> to be stable.
>
> gcc/ChangeLog
>
> 	* dwarf2out.c (gen_subprogram_die): Add DW_AT_noreturn when the
> 	function decl has TREE_THIS_VOLATILE.
>
> gcc/testsuite/ChangeLog
>
> 	* g++.dg/debug/dwarf2/noreturn-function.C: New test.
> 	* gcc.dg/debug/dwarf2/noreturn-function-attribute.c: Likewise.
> 	* gcc.dg/debug/dwarf2/noreturn-function-keyword.c: Likewise.
>
> include/ChangeLog
>
> 	* dwarf2.def (DW_AT_noreturn): New DWARF5 attribute.
Jason and Jakub have the final say here.

Presumably we don't have any sense when the values will be assigned, 
right?  Any chance we could speed that along a bit?

jeff
Cary Coutant Dec. 1, 2014, 9:44 p.m. UTC | #3
> Presumably we don't have any sense when the values will be assigned, right?
> Any chance we could speed that along a bit?

As Jason said, the value in the current draft is unlikely to change,
and I think he and I can probably lobby to keep it unchanged if there
any danger that the numbering will change. The committee doesn't
really like it when we jump the gun and use values before the final
spec is published, but as a practical matter, it's often necessary and
(at this stage) pretty safe.

-cary
Jeff Law Dec. 1, 2014, 9:50 p.m. UTC | #4
On 12/01/14 14:44, Cary Coutant wrote:
>> Presumably we don't have any sense when the values will be assigned, right?
>> Any chance we could speed that along a bit?
>
> As Jason said, the value in the current draft is unlikely to change,
> and I think he and I can probably lobby to keep it unchanged if there
> any danger that the numbering will change. The committee doesn't
> really like it when we jump the gun and use values before the final
> spec is published, but as a practical matter, it's often necessary and
> (at this stage) pretty safe.
Rather than having to lobby to keep it unchanged because we jumped the 
gun, can we lobby to get the number assigned in the near future rather 
than in the potentially far future?  That feels more cooperative to me :-)

Would that make Michael happier?

jeff
Jakub Jelinek Dec. 1, 2014, 9:54 p.m. UTC | #5
On Mon, Dec 01, 2014 at 02:50:57PM -0700, Jeff Law wrote:
> On 12/01/14 14:44, Cary Coutant wrote:
> >>Presumably we don't have any sense when the values will be assigned, right?
> >>Any chance we could speed that along a bit?
> >
> >As Jason said, the value in the current draft is unlikely to change,
> >and I think he and I can probably lobby to keep it unchanged if there
> >any danger that the numbering will change. The committee doesn't
> >really like it when we jump the gun and use values before the final
> >spec is published, but as a practical matter, it's often necessary and
> >(at this stage) pretty safe.
> Rather than having to lobby to keep it unchanged because we jumped the gun,
> can we lobby to get the number assigned in the near future rather than in
> the potentially far future?  That feels more cooperative to me :-)
> 
> Would that make Michael happier?

The numbers are assigned and there is a working draft with those numbers,
the thing is just that there is no 100% guarantee they won't be removed or
reassigned.  But as Jason/Cary said, we can lobby of for them being kept the
same.

	Jakub
Cary Coutant Dec. 1, 2014, 10:02 p.m. UTC | #6
[+cc Michael Eager]

> Rather than having to lobby to keep it unchanged because we jumped the gun,
> can we lobby to get the number assigned in the near future rather than in
> the potentially far future?  That feels more cooperative to me :-)
>
> Would that make Michael happier?

I'm pretty confident that Michael will say, "don't rely on the value
until the final spec is published." (He's told me exactly that in the
past. I've been guilty of jumping the gun on a couple of DW_LANG codes
and a few DW_AT values.)

But we should let Michael answer for himself...

Michael, for your reference, here's the start of this thread:

   https://gcc.gnu.org/ml/gcc-patches/2014-11/msg03195.html

and its continuation into December:

   https://gcc.gnu.org/ml/gcc-patches/2014-12/msg00099.html

Michael, are you OK with sharing your target dates for publishing the spec?

-cary
Mark Wielaard Dec. 2, 2014, 1:17 p.m. UTC | #7
On Mon, 2014-12-01 at 14:31 -0700, Jeff Law wrote:
> On 11/25/14 15:57, Mark Wielaard wrote:
> > This implements the DWARFv5 noreturn proposal:
> > http://dwarfstd.org/ShowIssue.php?issue=140331.1
> >
> > TREE_THIS_VOLATILE on a FUNCTION_DECL node means the function does not
> > return normally. This catches the traditional noreturn GNU attribute,
> > the C11 _Noreturn keyword and the C++11 [[noreturn]] attribute.
> >
> > This relies on the DW_AT_noreturn constant defined in the DWARFv5 DRAFT:
> > http://www.dwarfstd.org/doc/dwarf5.20141029.pdf
> > We could also use a new GNU extension if we don't trust these constants
> > to be stable.
> 
> Presumably we don't have any sense when the values will be assigned, 
> right?  Any chance we could speed that along a bit?

Sadly I have no idea. Officially the DWARF committee hasn't accepted new
proposals since March 2014 for DWARFv5. But some of my proposals have
been accepted anyway as late as last month. There is not really a way to
know when which proposal is submitted or considered for incorporation by
people outside the committee.

There is the above draft that contains the currently assigned constants.
But I don't know when the committee will ask for comments on the draft.
Or how long the comment period will be. I have just asked on the DWARF
mailinglist.

I don't mind using GNU vendor constants instead. But people on the
committee have convinced me it should be fine to use the constants from
the current draft. Given that Jakub, Jason and Cary are all on the DWARF
committee I assume they would yell and scream at me if they believed
using these constants were not stable. Most usage is gated behind the
experimental GCC5 -gdwarf-5 switch anyway (although not this attribute,
which is also emitted in non-strict mode).

BTW. I do make sure to update bintuils, gdb, elfutils and valgrind to
match the GCC5 output, so the constants should at least be consistent
across the GNU toolchain and friends. All GNU and DWARFv5 extensions are
documented: https://fedorahosted.org/elfutils/wiki/DwarfExtensions

Cheers,

Mark
Michael Eager Dec. 2, 2014, 9:52 p.m. UTC | #8
On 12/01/14 14:02, Cary Coutant wrote:
> [+cc Michael Eager]
>
>> Rather than having to lobby to keep it unchanged because we jumped the gun,
>> can we lobby to get the number assigned in the near future rather than in
>> the potentially far future?  That feels more cooperative to me :-)
>>
>> Would that make Michael happier?
>
> I'm pretty confident that Michael will say, "don't rely on the value
> until the final spec is published." (He's told me exactly that in the
> past. I've been guilty of jumping the gun on a couple of DW_LANG codes
> and a few DW_AT values.)
>
> But we should let Michael answer for himself...

As Cary said, the values are not fixed until the spec is released.
As Jason said, the values are unlikely to change.

The standards process can be a bit messy at times, particularly
when we realize that something we added to a draft needs to be changed.
We try to avoid making unnecessary changes, naturally.  On the other
hand, we don't want something in an internal draft of the DWARF
specification to prevent us from making necessary changes.

We will avoid changing the value of the DW_AT_noreturn or any other
new attribute, if for no reason than it makes additional work for us.
We're also aware that changes may affect work on GCC, GDB, or other
tools, which we would like to avoid.

The most conservative approach is to wait for the DWARF Version
5 spec to be released.  While there is no guarantee that attribute
values will not change in the final spec, the risk if this happening
seems small.


> Michael, for your reference, here's the start of this thread:
>
>     https://gcc.gnu.org/ml/gcc-patches/2014-11/msg03195.html
>
> and its continuation into December:
>
>     https://gcc.gnu.org/ml/gcc-patches/2014-12/msg00099.html
>
> Michael, are you OK with sharing your target dates for publishing the spec?

We will likely have a public review draft available in April or May
with a 60 period for comments, with final release most likely in July.
Jeff Law Dec. 3, 2014, 9:18 p.m. UTC | #9
On 12/02/14 14:52, Michael Eager wrote:
>
> As Cary said, the values are not fixed until the spec is released.
> As Jason said, the values are unlikely to change.
Fair enough.

>
> The standards process can be a bit messy at times, particularly
> when we realize that something we added to a draft needs to be changed.
> We try to avoid making unnecessary changes, naturally.  On the other
> hand, we don't want something in an internal draft of the DWARF
> specification to prevent us from making necessary changes.
Understood.  And I don't want GCC handcuffing the DWARF standardization 
process or generally making things harder for you.

> The most conservative approach is to wait for the DWARF Version
> 5 spec to be released.  While there is no guarantee that attribute
> values will not change in the final spec, the risk if this happening
> seems small.
The problem is that waiting for the final spec means effectively GCC 
doesn't support the feature until gcc-6 in 2016 just due to the expected 
timing of the dwarf5 standard and the gcc release schedule.  They pretty 
much couldn't line up much worse in this regard.

As I mentioned, Jason and Jakub have the final say on including this 
stuff into gcc-5 -- I just wanted to see if there was a way we cooperate 
better with the DWARF committee on this class of issues.

Thanks,
jeff
Mike Stump Dec. 4, 2014, 6:34 p.m. UTC | #10
On Dec 3, 2014, at 1:18 PM, Jeff Law <law@redhat.com> wrote:
> The problem is that waiting for the final spec means effectively GCC doesn't support the feature until gcc-6 in 2016 just due to the expected timing of the dwarf5 standard and the gcc release schedule.  They pretty much couldn't line up much worse in this regard.

So, I’d recommend implementing the draft so that we can get a feel for if it is done, if we like it, if there is anything else that needs to be fixed about it and review of the draft.  We can not default to generation of dwarf5 until it is a standard.  Not unreasonable to default to dwarf4 if people prefer that.  Advanced users that want the draft, with the understanding it isn’t set in stone, can ask for it by -gdwarf5.  I view this is largely the same as a language standard.  Implement early, implement often, as the standard comes out, you have already implemented it, had adventuresome users comment on it; had vendors build worlds with it, fixed all the bugs found, then you can just flip the switch.  I don’t favor waiting until a standard is published before implementing it.
diff mbox

Patch

diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
index d0eaaf1..25f0e7d 100644
--- a/gcc/dwarf2out.c
+++ b/gcc/dwarf2out.c
@@ -18348,6 +18348,9 @@  gen_subprogram_die (tree decl, dw_die_ref context_die)
       if (DECL_ARTIFICIAL (decl))
 	add_AT_flag (subr_die, DW_AT_artificial, 1);
 
+      if (TREE_THIS_VOLATILE (decl) && (dwarf_version >= 5 || !dwarf_strict))
+	add_AT_flag (subr_die, DW_AT_noreturn, 1);
+
       add_accessibility_attribute (subr_die, decl);
     }
 
diff --git a/gcc/testsuite/g++.dg/debug/dwarf2/noreturn-function.C b/gcc/testsuite/g++.dg/debug/dwarf2/noreturn-function.C
new file mode 100644
index 0000000..73a0af4
--- /dev/null
+++ b/gcc/testsuite/g++.dg/debug/dwarf2/noreturn-function.C
@@ -0,0 +1,16 @@ 
+// { dg-do compile }
+// { dg-options "-O -std=c++11 -g -dA -gno-strict-dwarf" }
+// Expect DW_AT_noreturn once in .debug_info and once in .debug_abbrev
+// { dg-final { scan-assembler-times "DW_AT_noreturn" 2 } }
+
+class Foo
+{
+  int i;
+  void bar [[noreturn]] (int j);
+};
+
+void
+Foo::bar (int j)
+{
+  while (1) { ; }
+}
diff --git a/gcc/testsuite/gcc.dg/debug/dwarf2/noreturn-function-attribute.c b/gcc/testsuite/gcc.dg/debug/dwarf2/noreturn-function-attribute.c
new file mode 100644
index 0000000..7c8924a
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/debug/dwarf2/noreturn-function-attribute.c
@@ -0,0 +1,11 @@ 
+// { dg-do compile }
+// { dg-options "-O -std=c99 -g -dA -gno-strict-dwarf" }
+// Expect DW_AT_noreturn once in .debug_info and once in .debug_abbrev
+// { dg-final { scan-assembler-times "DW_AT_noreturn" 2 } }
+
+void __attribute__ ((noreturn))
+baz (void)
+{
+  while (1) { ; }
+}
+
diff --git a/gcc/testsuite/gcc.dg/debug/dwarf2/noreturn-function-keyword.c b/gcc/testsuite/gcc.dg/debug/dwarf2/noreturn-function-keyword.c
new file mode 100644
index 0000000..ced96d1
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/debug/dwarf2/noreturn-function-keyword.c
@@ -0,0 +1,13 @@ 
+// { dg-do compile }
+// { dg-options "-O -std=c11 -g -dA -gno-strict-dwarf" }
+// Expect DW_AT_noreturn once in .debug_info and once in .debug_abbrev
+// { dg-final { scan-assembler-times "DW_AT_noreturn" 2 } }
+
+_Noreturn void exit (int);
+
+void exit (int i)
+{
+  while (i < 0 || i == 0 || i > 0)
+    ;
+}
+
diff --git a/include/dwarf2.def b/include/dwarf2.def
index 8ca143c..8533a3e 100644
--- a/include/dwarf2.def
+++ b/include/dwarf2.def
@@ -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.  */