This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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

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.

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
Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]