diff mbox

[PING*4] PR debug/53927: fix value for DW_AT_static_link

Message ID 5654916D.5070309@adacore.com
State New
Headers show

Commit Message

Pierre-Marie de Rodat Nov. 24, 2015, 4:33 p.m. UTC
On 08/31/2015 09:28 AM, Pierre-Marie de Rodat wrote:
> Ping for the patch submitted in
> <https://gcc.gnu.org/ml/gcc-patches/2015-07/msg01629.html>.

As Jason noticed in another thread, I forgot to attach the patch here 
(although it’s filed in the bug tracker). Here it is! Rebased against 
trunk, bootstrapped and regtested again on x86_64-linux.

Thanks in advance!

Comments

Jason Merrill Nov. 25, 2015, 5:54 p.m. UTC | #1
The DWARF change is OK if the tree-nested.c changes make sense to Eric.

Jason
Eric Botcazou Nov. 25, 2015, 8:35 p.m. UTC | #2
> The DWARF change is OK if the tree-nested.c changes make sense to Eric.

As discussed in the audit trail of the PR, I'd rather have avoided the 
additional indirection, but the DWARF requirement seems to force it if we base 
the computation on the static chain and I don't see another approach.

A few comments:

+      int save_warn_padded;
+      tree *adjust, *field_decl_p;
+      char *name;
+      tree fb_decl, fb_ref, fb_tmp;
+      gcall *fb_gimple;
+      gimple_stmt_iterator gsi;

We try to declare variables only at the first use point now I think.

+      /* Debugging information needs to compute the frame base address of the
+	 nestee frame out of the static chain from the nested frame.

"parent frame"

+	 The static chain is the address of the FRAME record, so one could
+	 imagine it would be possible to compute the frame base address just
+	 adding a constant offset to this address.  Unfortunately, this is not
+	 possible: if the FRAME object has alignment constraints that are
+	 stronger than the stack, then the offset between the frame base and
+	 the FRAME object will be dynamic.
+
+	 What we do instead is to append a field to the FRAME object that 
holds
+	 the frame base address: then debug. info. just has to fetch this
+	 field.  */

No useless period: "debug info"

+      /* Debugging information will refer to the CFA as the frame base
+	 address: we will do the same here.  */
+      const tree frame_addr_fndecl
+        = builtin_decl_explicit (BUILT_IN_DWARF_CFA);
+
+      /* Create a field in the FRAME record to hold the frame base address 
for
+	 this stack frame.  Since it will be used only by the debugger 
(through
+	 the debugging information), put it at the end of the record not to
+	 shift all other offsets.  */

"(through the debugging information)" sounds superfluous. "in order not to..."

+      fb_decl = make_node (FIELD_DECL);
+      name = concat ("FRAME_BASE.",
+		     IDENTIFIER_POINTER (DECL_NAME (root->context)),
+		     NULL);
+      DECL_NAME (fb_decl) = get_identifier (name);
+      free (name);

Let's avoid this concat/free business and use a simpler name.

+      TREE_TYPE (fb_decl) = ptr_type_node;
+      TREE_ADDRESSABLE (fb_decl) = 1;
+      DECL_CONTEXT (fb_decl) = root->frame_type;
+      field_decl_p = &TYPE_FIELDS (root->frame_type);
+      while (*field_decl_p != NULL_TREE)
+	field_decl_p = &DECL_CHAIN (*field_decl_p);
+      *field_decl_p = fb_decl;

TYPE_FIELDS (root->frame_type)
  = chainon (TYPE_FIELDS (root->frame_type), fb_decl);
diff mbox

Patch

From d49b9ba6e40683a01c3fbf351c661a501c72ced1 Mon Sep 17 00:00:00 2001
From: Pierre-Marie de Rodat <derodat@adacore.com>
Date: Wed, 25 Feb 2015 14:48:24 +0100
Subject: [PATCH] DWARF: fix loc. descr. generation for DW_AT_static_link

gcc/ChangeLog:

	PR debug/53927
	* tree-nested.c (finalize_nesting_tree_1): Append a field to
	hold the frame base address.
	* dwarf2out.c (gen_subprogram_die): Generate for
	DW_AT_static_link a location description that computes the value
	of this field.
---
 gcc/dwarf2out.c   | 20 ++++++++++++---
 gcc/tree-nested.c | 73 +++++++++++++++++++++++++++++++++++++++++++++++++------
 2 files changed, 83 insertions(+), 10 deletions(-)

diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
index f184750..5249fca 100644
--- a/gcc/dwarf2out.c
+++ b/gcc/dwarf2out.c
@@ -19113,9 +19113,23 @@  gen_subprogram_die (tree decl, dw_die_ref context_die)
       compute_frame_pointer_to_fb_displacement (cfa_fb_offset);
 
       if (fun->static_chain_decl)
-	add_AT_location_description
-	  (subr_die, DW_AT_static_link,
-	   loc_list_from_tree (fun->static_chain_decl, 2, NULL));
+	{
+	  /* DWARF requires here a location expression that computes the
+	     address of the enclosing subprogram's frame base.  The machinery
+	     in tree-nested.c is supposed to store this specific address in the
+	     last field of the FRAME record.  */
+	  const tree frame_type
+	    = TREE_TYPE (TREE_TYPE (fun->static_chain_decl));
+	  const tree fb_decl = tree_last (TYPE_FIELDS (frame_type));
+
+	  tree fb_expr
+	    = build1 (INDIRECT_REF, frame_type, fun->static_chain_decl);
+	  fb_expr = build3 (COMPONENT_REF, TREE_TYPE (fb_decl),
+			    fb_expr, fb_decl, NULL_TREE);
+
+	  add_AT_location_description (subr_die, DW_AT_static_link,
+				       loc_list_from_tree (fb_expr, 0, NULL));
+	}
     }
 
   /* Generate child dies for template paramaters.  */
diff --git a/gcc/tree-nested.c b/gcc/tree-nested.c
index 1f6311c..06484a2 100644
--- a/gcc/tree-nested.c
+++ b/gcc/tree-nested.c
@@ -2718,10 +2718,10 @@  fold_mem_refs (tree *const &e, void *data ATTRIBUTE_UNUSED)
   return true;
 }
 
-/* Do "everything else" to clean up or complete state collected by the
-   various walking passes -- lay out the types and decls, generate code
-   to initialize the frame decl, store critical expressions in the
-   struct function for rtl to find.  */
+/* Do "everything else" to clean up or complete state collected by the various
+   walking passes -- create a field to hold the frame base address, lay out the
+   types and decls, generate code to initialize the frame decl, store critical
+   expressions in the struct function for rtl to find.  */
 
 static void
 finalize_nesting_tree_1 (struct nesting_info *root)
@@ -2737,16 +2737,75 @@  finalize_nesting_tree_1 (struct nesting_info *root)
      out at this time.  */
   if (root->frame_type)
     {
+      int save_warn_padded;
+      tree *adjust, *field_decl_p;
+      char *name;
+      tree fb_decl, fb_ref, fb_tmp;
+      gcall *fb_gimple;
+      gimple_stmt_iterator gsi;
+
+      /* Debugging information needs to compute the frame base address of the
+	 nestee frame out of the static chain from the nested frame.
+
+	 The static chain is the address of the FRAME record, so one could
+	 imagine it would be possible to compute the frame base address just
+	 adding a constant offset to this address.  Unfortunately, this is not
+	 possible: if the FRAME object has alignment constraints that are
+	 stronger than the stack, then the offset between the frame base and
+	 the FRAME object will be dynamic.
+
+	 What we do instead is to append a field to the FRAME object that holds
+	 the frame base address: then debug. info. just has to fetch this
+	 field.  */
+
+      /* Debugging information will refer to the CFA as the frame base
+	 address: we will do the same here.  */
+      const tree frame_addr_fndecl
+        = builtin_decl_explicit (BUILT_IN_DWARF_CFA);
+
+      /* Create a field in the FRAME record to hold the frame base address for
+	 this stack frame.  Since it will be used only by the debugger (through
+	 the debugging information), put it at the end of the record not to
+	 shift all other offsets.  */
+      fb_decl = make_node (FIELD_DECL);
+      name = concat ("FRAME_BASE.",
+		     IDENTIFIER_POINTER (DECL_NAME (root->context)),
+		     NULL);
+      DECL_NAME (fb_decl) = get_identifier (name);
+      free (name);
+      TREE_TYPE (fb_decl) = ptr_type_node;
+      TREE_ADDRESSABLE (fb_decl) = 1;
+      DECL_CONTEXT (fb_decl) = root->frame_type;
+      field_decl_p = &TYPE_FIELDS (root->frame_type);
+      while (*field_decl_p != NULL_TREE)
+	field_decl_p = &DECL_CHAIN (*field_decl_p);
+      *field_decl_p = fb_decl;
+
       /* In some cases the frame type will trigger the -Wpadded warning.
 	 This is not helpful; suppress it. */
-      int save_warn_padded = warn_padded;
-      tree *adjust;
-
+      save_warn_padded = warn_padded;
       warn_padded = 0;
       layout_type (root->frame_type);
       warn_padded = save_warn_padded;
       layout_decl (root->frame_decl, 0);
 
+      /* Initialize the frame base address field.  If the builtin we need is
+	 not available, set it to NULL so that debugging information does not
+	 reference junk.  */
+      fb_ref = build3 (COMPONENT_REF, TREE_TYPE (fb_decl),
+		       root->frame_decl, fb_decl, NULL_TREE);
+      if (frame_addr_fndecl != NULL_TREE)
+	{
+	  fb_gimple = gimple_build_call (frame_addr_fndecl, 1,
+					 integer_zero_node);
+	  gsi = gsi_last (stmt_list);
+	  fb_tmp = init_tmp_var_with_call (root, &gsi, fb_gimple);
+	}
+      else
+	fb_tmp = build_int_cst (TREE_TYPE (fb_ref), 0);
+      gimple_seq_add_stmt (&stmt_list,
+			   gimple_build_assign (fb_ref, fb_tmp));
+
       /* Remove root->frame_decl from root->new_local_var_chain, so
 	 that we can declare it also in the lexical blocks, which
 	 helps ensure virtual regs that end up appearing in its RTL
-- 
2.3.3.199.g52cae64