diff mbox series

DWARF: Allow hard frame pointer even if frame pointer isn't used

Message ID CAMe9rOpuzruGwKLemne71evtdVuKTAK9Z6MpUZng-eGGGQU5ww@mail.gmail.com
State New
Headers show
Series DWARF: Allow hard frame pointer even if frame pointer isn't used | expand

Commit Message

H.J. Lu Sept. 4, 2018, 3:13 p.m. UTC 
On Mon, Sep 3, 2018 at 10:01 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Mon, Sep 3, 2018 at 9:44 AM, Michael Matz <matz@suse.de> wrote:
>> Hi,
>>
>> On Mon, 3 Sep 2018, H.J. Lu wrote:
>>
>>> > So, what's the testcase testing then?  Before the patch it doesn't ICE,
>>> > after the patch it doesn't ICE.  What should I look out for so I can see
>>> > that what the testcase is producing without the patch is wrong?
>>>
>>> Before the patch, debug info is wrong since it uses hard frame pointer
>>> which isn't set up for the function.  You can do "readelf -w" on .o file to
>>> verify the debug info.
>>
>> Yeah, that's what I thought as well, but it's correct:
>>
>> % ./gcc/cc1plus -quiet -O2 -g -fno-omit-frame-pointer -fvar-tracking x.cc
>> % gcc -c x.s
>> % readelf -wfi x.o
>> ...
>>  <1><8a>: Abbrev Number: 9 (DW_TAG_subprogram)
>>     <8b>   DW_AT_specification: <0x3a>
>>     <8f>   DW_AT_decl_line   : 6
>>     <90>   DW_AT_decl_column : 5
>>     <91>   DW_AT_object_pointer: <0xa7>
>>     <95>   DW_AT_low_pc      : 0x0
>>     <9d>   DW_AT_high_pc     : 0x3
>>     <a5>   DW_AT_frame_base  : 1 byte block: 9c         (DW_OP_call_frame_cfa)
>>     <a7>   DW_AT_GNU_all_call_sites: 1
>> ...
>>  <2><fe>: Abbrev Number: 11 (DW_TAG_formal_parameter)
>>     <ff>   DW_AT_name        : d
>>     <101>   DW_AT_decl_file   : 1
>>     <102>   DW_AT_decl_line   : 6
>>     <103>   DW_AT_decl_column : 63
>>     <104>   DW_AT_type        : <0x78>
>>     <108>   DW_AT_location    : 2 byte block: 91 8      (DW_OP_fbreg: 8)
>> ...
>>   DW_CFA_def_cfa: r7 (rsp) ofs 8
>>   DW_CFA_offset: r16 (rip) at cfa-8
>>   DW_CFA_nop
>>   DW_CFA_nop
>> ...
>>
>> So, argument 'd' is supposed to be at DW_AT_frame_base + 8, which is
>> %rsp+8+8, aka %rsp+16, which is correct given that it's the eigth argument
>> (including the implicit this parameter).
>
> Can we use DW_AT_frame_base when the frame pointer isn't available?
> If yes,
>
>          gcc_assert ((SUPPORTS_STACK_ALIGNMENT
>                        && (elim == hard_frame_pointer_rtx
>                            || elim == stack_pointer_rtx))
>                       || elim == (frame_pointer_needed
>                                   ? hard_frame_pointer_rtx
>                                   : stack_pointer_rtx));
>
> should be changed to
>
>           gcc_assert (elim == hard_frame_pointer_rtx
>                       || elim == stack_pointer_rtx);
>
> This will also fix:
>
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86593
>

Since hard frame pointer is encoded with DW_OP_fbreg which uses the
DW_AT_frame_base attribute, not hard frame pointer directly, we should
allow hard frame pointer when generating DWARF info even if frame pointer
isn't used.

OK for trunk?

Comments

Jason Merrill Sept. 4, 2018, 8:03 p.m. UTC | #1 
OK.

On Tue, Sep 4, 2018 at 11:13 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Mon, Sep 3, 2018 at 10:01 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
>> On Mon, Sep 3, 2018 at 9:44 AM, Michael Matz <matz@suse.de> wrote:
>>> Hi,
>>>
>>> On Mon, 3 Sep 2018, H.J. Lu wrote:
>>>
>>>> > So, what's the testcase testing then?  Before the patch it doesn't ICE,
>>>> > after the patch it doesn't ICE.  What should I look out for so I can see
>>>> > that what the testcase is producing without the patch is wrong?
>>>>
>>>> Before the patch, debug info is wrong since it uses hard frame pointer
>>>> which isn't set up for the function.  You can do "readelf -w" on .o file to
>>>> verify the debug info.
>>>
>>> Yeah, that's what I thought as well, but it's correct:
>>>
>>> % ./gcc/cc1plus -quiet -O2 -g -fno-omit-frame-pointer -fvar-tracking x.cc
>>> % gcc -c x.s
>>> % readelf -wfi x.o
>>> ...
>>>  <1><8a>: Abbrev Number: 9 (DW_TAG_subprogram)
>>>     <8b>   DW_AT_specification: <0x3a>
>>>     <8f>   DW_AT_decl_line   : 6
>>>     <90>   DW_AT_decl_column : 5
>>>     <91>   DW_AT_object_pointer: <0xa7>
>>>     <95>   DW_AT_low_pc      : 0x0
>>>     <9d>   DW_AT_high_pc     : 0x3
>>>     <a5>   DW_AT_frame_base  : 1 byte block: 9c         (DW_OP_call_frame_cfa)
>>>     <a7>   DW_AT_GNU_all_call_sites: 1
>>> ...
>>>  <2><fe>: Abbrev Number: 11 (DW_TAG_formal_parameter)
>>>     <ff>   DW_AT_name        : d
>>>     <101>   DW_AT_decl_file   : 1
>>>     <102>   DW_AT_decl_line   : 6
>>>     <103>   DW_AT_decl_column : 63
>>>     <104>   DW_AT_type        : <0x78>
>>>     <108>   DW_AT_location    : 2 byte block: 91 8      (DW_OP_fbreg: 8)
>>> ...
>>>   DW_CFA_def_cfa: r7 (rsp) ofs 8
>>>   DW_CFA_offset: r16 (rip) at cfa-8
>>>   DW_CFA_nop
>>>   DW_CFA_nop
>>> ...
>>>
>>> So, argument 'd' is supposed to be at DW_AT_frame_base + 8, which is
>>> %rsp+8+8, aka %rsp+16, which is correct given that it's the eigth argument
>>> (including the implicit this parameter).
>>
>> Can we use DW_AT_frame_base when the frame pointer isn't available?
>> If yes,
>>
>>          gcc_assert ((SUPPORTS_STACK_ALIGNMENT
>>                        && (elim == hard_frame_pointer_rtx
>>                            || elim == stack_pointer_rtx))
>>                       || elim == (frame_pointer_needed
>>                                   ? hard_frame_pointer_rtx
>>                                   : stack_pointer_rtx));
>>
>> should be changed to
>>
>>           gcc_assert (elim == hard_frame_pointer_rtx
>>                       || elim == stack_pointer_rtx);
>>
>> This will also fix:
>>
>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86593
>>
>
> Since hard frame pointer is encoded with DW_OP_fbreg which uses the
> DW_AT_frame_base attribute, not hard frame pointer directly, we should
> allow hard frame pointer when generating DWARF info even if frame pointer
> isn't used.
>
> OK for trunk?
>
> --
> H.J.
diff mbox series

Patch

From 6c16105e88c2635bd58fc904e7d28901a7f198f5 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Tue, 4 Sep 2018 08:01:33 -0700
Subject: [PATCH] DWARF: Allow hard frame pointer even if frame pointer isn't
 used

r251028

commit cd557ff63f388ad27c376d0a225e74d3594a6f9d
Author: hjl <hjl@138bc75d-0d04-0410-961f-82ee72b054a4>
Date:   Thu Aug 10 15:29:05 2017 +0000

    i386: Don't use frame pointer without stack access

    When there is no stack access, there is no need to use frame pointer
    even if -fno-omit-frame-pointer is used and caller's frame pointer is
    unchanged.

frame pointer may not be available even if -fno-omit-frame-pointer is
used.  When this happened, arg pointer may be eliminated by hard frame
pointer.  Since hard frame pointer is encoded with DW_OP_fbreg which
uses the DW_AT_frame_base attribute, not hard frame pointer directly,
we should allow hard frame pointer when generating DWARF info even if
frame pointer isn't used.

gcc/

	PR debug/86593
	* dwarf2out.c (based_loc_descr): Allow hard frame pointer even
	if frame pointer isn't used.
	(compute_frame_pointer_to_fb_displacement): Likewise.

gcc/testsuite/

	PR debug/86593
	* g++.dg/pr86593.C: New test.
---
 gcc/dwarf2out.c                | 25 ++++++++++++-------------
 gcc/testsuite/g++.dg/pr86593.C | 11 +++++++++++
 2 files changed, 23 insertions(+), 13 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/pr86593.C

diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
index 77317ed2575..40cfdf56337 100644
--- a/gcc/dwarf2out.c
+++ b/gcc/dwarf2out.c
@@ -14325,13 +14325,13 @@  based_loc_descr (rtx reg, poly_int64 offset,
 
       if (elim != reg)
 	{
+	  /* Allow hard frame pointer here even if frame pointer
+	    isn't used since hard frame pointer is encoded with
+	    DW_OP_fbreg which uses the DW_AT_frame_base attribute,
+	    not hard frame pointer directly.  */
 	  elim = strip_offset_and_add (elim, &offset);
-	  gcc_assert ((SUPPORTS_STACK_ALIGNMENT
-		       && (elim == hard_frame_pointer_rtx
-			   || elim == stack_pointer_rtx))
-	              || elim == (frame_pointer_needed
-				  ? hard_frame_pointer_rtx
-				  : stack_pointer_rtx));
+	  gcc_assert (elim == hard_frame_pointer_rtx
+		      || elim == stack_pointer_rtx);
 
 	  /* If drap register is used to align stack, use frame
 	     pointer + offset to access stack variables.  If stack
@@ -20512,14 +20512,13 @@  compute_frame_pointer_to_fb_displacement (poly_int64 offset)
      in which to eliminate.  This is because it's stack pointer isn't 
      directly accessible as a register within the ISA.  To work around
      this, assume that while we cannot provide a proper value for
-     frame_pointer_fb_offset, we won't need one either.  */
+     frame_pointer_fb_offset, we won't need one either.  We can use
+     hard frame pointer in debug info even if frame pointer isn't used
+     since hard frame pointer in debug info is encoded with DW_OP_fbreg
+     which uses the DW_AT_frame_base attribute, not hard frame pointer
+     directly.  */
   frame_pointer_fb_offset_valid
-    = ((SUPPORTS_STACK_ALIGNMENT
-	&& (elim == hard_frame_pointer_rtx
-	    || elim == stack_pointer_rtx))
-       || elim == (frame_pointer_needed
-		   ? hard_frame_pointer_rtx
-		   : stack_pointer_rtx));
+    = (elim == hard_frame_pointer_rtx || elim == stack_pointer_rtx);
 }
 
 /* Generate a DW_AT_name attribute given some string value to be included as
diff --git a/gcc/testsuite/g++.dg/pr86593.C b/gcc/testsuite/g++.dg/pr86593.C
new file mode 100644
index 00000000000..feed8c3743e
--- /dev/null
+++ b/gcc/testsuite/g++.dg/pr86593.C
@@ -0,0 +1,11 @@ 
+// { dg-options "-O -g -fno-omit-frame-pointer" }
+
+struct Foo
+{
+  int bar(int a, int b, int c, int i1, int i2, int i3, int d);
+};
+
+int Foo::bar(int a, int b, int c, int i1, int i2, int i3, int d)
+{
+  return 0;
+}
-- 
2.17.1