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][debug] Handle references to skipped params in remap_ssa_name


[ was: Re: [testsuite/guality, committed] Prevent optimization of local in
vla-1.c ]

On Wed, Jul 04, 2018 at 02:32:27PM +0200, Tom de Vries wrote:
> On 07/03/2018 11:05 AM, Tom de Vries wrote:
> > On 07/02/2018 10:16 AM, Jakub Jelinek wrote:
> >> On Mon, Jul 02, 2018 at 09:44:04AM +0200, Richard Biener wrote:
> >>> Given the array has size i + 1 it's upper bound should be 'i' and 'i'
> >>> should be available via DW_OP_[GNU_]entry_value.
> >>>
> >>> I see it is
> >>>
> >>>     <175>   DW_AT_upper_bound : 10 byte block: 75 1 8 20 24 8 20 26 31
> >>> 1c       (DW_OP_breg5 (rdi): 1; DW_OP_const1u: 32; DW_OP_shl;
> >>> DW_OP_const1u: 32; DW_OP_shra; DW_OP_lit1; DW_OP_minus)
> >>>
> >>> and %rdi is 1.  Not sure why gdb fails to print it's length.  Yes, the
> >>> storage itself doesn't have a location but the
> >>> type specifies the size.
> >>>
> >>> (gdb) ptype a
> >>> type = char [variable length]
> >>> (gdb) p sizeof(a)
> >>> $3 = 0
> >>>
> >>> this looks like a gdb bug to me?
> >>>
> > 
> > With gdb patch:
> > ...
> > diff --git a/gdb/findvar.c b/gdb/findvar.c
> > index 8ad5e25cb2..ebaff923a1 100644
> > --- a/gdb/findvar.c
> > +++ b/gdb/findvar.c
> > @@ -789,6 +789,8 @@ default_read_var_value
> >        break;
> > 
> >      case LOC_OPTIMIZED_OUT:
> > +      if (is_dynamic_type (type))
> > +       type = resolve_dynamic_type (type, NULL,
> > +                                    /* Unused address.  */ 0);
> >        return allocate_optimized_out_value (type);
> > 
> >      default:
> > ...
> > 
> > I get:
> > ...
> > $ ./gdb -batch -ex "b f1" -ex "r" -ex "p sizeof (a)" vla-1.exe
> > Breakpoint 1 at 0x4004a8: file vla-1.c, line 17.
> > 
> > Breakpoint 1, f1 (i=i@entry=5) at vla-1.c:17
> > 17        return a[0];
> > $1 = 6
> > ...
> > 
> 
> Well, for -O1 and -O2.
> 
> For O3, I get instead:
> ...
> $ ./gdb vla-1.exe -q -batch -ex "b f1" -ex "run" -ex "p sizeof (a)"
> Breakpoint 1 at 0x4004b0: f1. (2 locations)
> 
> Breakpoint 1, f1 (i=5) at vla-1.c:17
> 17        return a[0];
> $1 = 0
> ...
> 

Hi,

When compiling guality/vla-1.c with -O3 -g, vla 'a[i + 1]' in f1 is optimized
away, but f1 still contains a debug expression describing the upper bound of the
vla (D.1914):
...
     __attribute__((noinline))
     f1 (intD.6 iD.1900)
     {
       <bb 2>
       saved_stack.1_2 = __builtin_stack_save ();
       # DEBUG BEGIN_STMT
       # DEBUG D#3 => i_1(D) + 1
       # DEBUG D#2 => (long intD.8) D#3
       # DEBUG D#1 => D#2 + -1
       # DEBUG D.1914 => (sizetype) D#1
...

Then f1 is cloned to a version f1.constprop with no parameters, eliminating
parameter i, and 'DEBUG D#3 => i_1(D) + 1' turns into 'D#3 => NULL'.
Consequently, 'print sizeof (a)' yields '0' in gdb.

This patch fixes that by recognizing eliminated parameters in remap_ssa_name,
defining a debug expression linking back to the the eliminated parameter, and
using that debug expression to replace references to the eliminated parameter:
...
     __attribute__((noinline))
     f1.constprop ()
     {
       intD.6 iD.1949;

       <bb 3>
       # DEBUG D#8 s=> iD.1900
       # DEBUG iD.1949 => D#8

       <bb 2>
+      # DEBUG D#6 s=> iD.1900
       saved_stack.1_1 = __builtin_stack_save ();
       # DEBUG BEGIN_STMT
-      # DEBUG D#3 => NULL
+      # DEBUG D#3 => D#6 + 1
       # DEBUG D#2 => (long intD.8) D#3
       # DEBUG D#1 => D#2 + -1
       # DEBUG D.1951 => (sizetype) D#1
...

The inserted debug expression (D#6) is a duplicate of the debug expression
that will be inserted after copy_body in tree_function_versioning (D#8), so the
patch contains a todo to fix the duplication.

Bootstrapped and reg-tested on x86_64.

OK for trunk?

Thanks,
- Tom

[debug] Handle references to skipped params in remap_ssa_name

2018-07-05  Tom de Vries  <tdevries@suse.de>

	* tree-inline.c (remap_ssa_name): Handle references to skipped
	params.

---
 gcc/tree-inline.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/gcc/tree-inline.c b/gcc/tree-inline.c
index 427ef959740..0fa996cab49 100644
--- a/gcc/tree-inline.c
+++ b/gcc/tree-inline.c
@@ -204,11 +204,22 @@ remap_ssa_name (tree name, copy_body_data *id)
 	  gimple *def_temp;
 	  gimple_stmt_iterator gsi;
 	  tree val = SSA_NAME_VAR (name);
+	  bool skipped_parm_decl = false;
 
 	  n = id->decl_map->get (val);
 	  if (n != NULL)
-	    val = *n;
-	  if (TREE_CODE (val) != PARM_DECL)
+	    {
+	      if (TREE_CODE (*n) == DEBUG_EXPR_DECL)
+		return *n;
+
+	      if (TREE_CODE (*n) == VAR_DECL
+		  && DECL_ABSTRACT_ORIGIN (*n)
+		  && TREE_CODE (DECL_ABSTRACT_ORIGIN (*n)) == PARM_DECL)
+		skipped_parm_decl = true;
+	      else
+		val = *n;
+	    }
+	  if (TREE_CODE (val) != PARM_DECL && !skipped_parm_decl)
 	    {
 	      processing_debug_stmt = -1;
 	      return name;
@@ -219,6 +230,8 @@ remap_ssa_name (tree name, copy_body_data *id)
 	  SET_DECL_MODE (vexpr, DECL_MODE (SSA_NAME_VAR (name)));
 	  gsi = gsi_after_labels (single_succ (ENTRY_BLOCK_PTR_FOR_FN (cfun)));
 	  gsi_insert_before (&gsi, def_temp, GSI_SAME_STMT);
+	  // Todo: Reuse vexpr in tree_function_versioning
+	  insert_decl_map (id, val, vexpr);
 	  return vexpr;
 	}
 


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]