Bug 59890 - var-tracking.c:val_reset segfaults
Summary: var-tracking.c:val_reset segfaults
Status: RESOLVED INVALID
Alias: None
Product: gcc
Classification: Unclassified
Component: rtl-optimization (show other bugs)
Version: 4.9.0
: P3 normal
Target Milestone: ---
Assignee: Richard Biener
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-01-20 14:48 UTC by Richard Biener
Modified: 2019-05-08 15:20 UTC (History)
2 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2014-01-20 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Richard Biener 2014-01-20 14:48:45 UTC
With changed iteration order during bootstrap I get into

#0  0x0000000000c99933 in pointer_set_lookup (pset=0x0, p=0x21d3f20, ix=
    0x7fffffffd9c8) at /space/rguenther/src/svn/trunk/gcc/pointer-set.c:86
#1  0x0000000000c99aa7 in pointer_map_contains (pmap=0x0, p=0x21d3f20)
    at /space/rguenther/src/svn/trunk/gcc/pointer-set.c:211
#2  0x000000000105906e in val_reset (set=0x2109338, dv=0x21d3f20)
    at /space/rguenther/src/svn/trunk/gcc/var-tracking.c:2511
#3  0x00000000010599dd in variable_post_merge_new_vals (slot=0x21e8fa0, dfpm=
    0x7fffffffdb80) at /space/rguenther/src/svn/trunk/gcc/var-tracking.c:4429
#4  0x0000000001061ab2 in hash_table<variable_hasher, xcallocator>::traverse_noresize<dfset_post_merge*, &(variable_post_merge_new_vals(variable_def**, dfset_post_merge*))> (this=0x7fffffffdb90, argument=0x7fffffffdb80)
    at /space/rguenther/src/svn/trunk/gcc/hash-table.h:928
#5  0x0000000001061b39 in hash_table<variable_hasher, xcallocator>::traverse<dfset_post_merge*, &(variable_post_merge_new_vals(variable_def**, dfset_post_merge*))> (this=0x7fffffffdb90, argument=0x7fffffffdb80)
    at /space/rguenther/src/svn/trunk/gcc/hash-table.h:950
#6  0x0000000001059bdc in dataflow_post_merge_adjust (set=0x2109338, permp=
    0x2109838) at /space/rguenther/src/svn/trunk/gcc/var-tracking.c:4555
#7  0x000000000105d2f7 in vt_find_locations ()
    at /space/rguenther/src/svn/trunk/gcc/var-tracking.c:7021

where val_reset is called with a NULL local_get_addr_cache.  The fix seems
obvious.
Comment 1 Richard Biener 2014-01-20 15:19:51 UTC
Even with the obvious fix to val_reset

Index: gcc/var-tracking.c
===================================================================
--- gcc/var-tracking.c  (revision 206808)
+++ gcc/var-tracking.c  (working copy)
@@ -2501,7 +2501,8 @@ val_reset (dataflow_set *set, decl_or_va
           
   gcc_assert (var->n_var_parts == 1);
           
-  if (var->onepart == ONEPART_VALUE)
+  if (var->onepart == ONEPART_VALUE
+      && local_get_addr_cache != NULL)
     {     
       rtx x = dv_as_value (dv);
       void **slot;


I get during -m32 multilib x86_64 libjava build

Program received signal SIGSEGV, Segmentation fault.
vt_get_canonicalize_base (loc=0x0)
    at /space/rguenther/src/svn/trunk/gcc/var-tracking.c:1985
1985              || GET_CODE (loc) == AND)
(gdb) bt
#0  vt_get_canonicalize_base (loc=0x0)
    at /space/rguenther/src/svn/trunk/gcc/var-tracking.c:1985
#1  0x0000000000ba7db0 in local_get_addr_clear_given_value (v=<optimized out>, 
    slot=0x1ab8c28, x=0x1a6bdc8)
    at /space/rguenther/src/svn/trunk/gcc/var-tracking.c:2484
#2  0x00000000008bfeec in pointer_map_traverse (pmap=0x0, fn=0x1ab8c28, 
    data=0x1a6bdc8) at /space/rguenther/src/svn/trunk/gcc/pointer-set.c:269
#3  0x0000000000bb6612 in val_reset (set=set@entry=0x1ab74b8, 
    dv=dv@entry=0x1a6bdc8)
    at /space/rguenther/src/svn/trunk/gcc/var-tracking.c:2522
#4  0x0000000000bba55c in val_resolve (set=set@entry=0x1ab74b8, val=0x1a6bdc8, 
    loc=0x7ffff60b6b20, insn=insn@entry=0x7ffff4c07dc8)
    at /space/rguenther/src/svn/trunk/gcc/var-tracking.c:2597
#5  0x0000000000bbac6f in compute_bb_dataflow (
    bb=<error reading variable: Unhandled dwarf expression opcode 0xfa>)
    at /space/rguenther/src/svn/trunk/gcc/var-tracking.c:6695
#6  0x0000000000bbbfa9 in vt_find_locations ()
    at /space/rguenther/src/svn/trunk/gcc/var-tracking.c:7047

Btw, this all is with

Index: gcc/var-tracking.c
===================================================================
--- gcc/var-tracking.c  (revision 206808)
+++ gcc/var-tracking.c  (working copy)
@@ -6934,12 +6935,12 @@ vt_find_locations (void)
   bool success = true;
           
   timevar_push (TV_VAR_TRACKING_DATAFLOW);
-  /* Compute reverse completion order of depth first search of the CFG
+  /* Compute reverse top sord order of the inverted CFG
      so that the data-flow runs faster.  */
-  rc_order = XNEWVEC (int, n_basic_blocks_for_fn (cfun) - NUM_FIXED_BLOCKS);
+  rc_order = XNEWVEC (int, n_basic_blocks_for_fn (cfun));
   bb_order = XNEWVEC (int, last_basic_block_for_fn (cfun));
-  pre_and_rev_post_order_compute (NULL, rc_order, false);
-  for (i = 0; i < n_basic_blocks_for_fn (cfun) - NUM_FIXED_BLOCKS; i++)
+  int num = inverted_post_order_compute (rc_order);
+  for (i = 0; i < num; i++)
     bb_order[rc_order[i]] = i;
   free (rc_order);
           

which provides a nice speedup for some testcases (inverted post-order
is the correct order for a forward dataflow problem).

static bool
local_get_addr_clear_given_value (const void *v ATTRIBUTE_UNUSED,
                                  void **slot, void *x)
{
  if (vt_get_canonicalize_base ((rtx)*slot) == x)
    *slot = NULL;
  return true;
}

if you do that again you have to guard against a cleared valued!
(pointer-map does not allow removal)

Index: gcc/var-tracking.c
===================================================================
--- gcc/var-tracking.c  (revision 206808)
+++ gcc/var-tracking.c  (working copy)
@@ -2481,7 +2481,8 @@ static bool
 local_get_addr_clear_given_value (const void *v ATTRIBUTE_UNUSED,
                                  void **slot, void *x)
 {
-  if (vt_get_canonicalize_base ((rtx)*slot) == x)
+  if (*slot != NULL
+      && vt_get_canonicalize_base ((rtx)*slot) == x)
     *slot = NULL;
   return true;
 }
Comment 2 Richard Biener 2014-01-20 18:52:42 UTC
Mine.
Comment 3 Richard Biener 2014-01-28 09:03:32 UTC
Author: rguenth
Date: Tue Jan 28 09:02:59 2014
New Revision: 207172

URL: http://gcc.gnu.org/viewcvs?rev=207172&root=gcc&view=rev
Log:
2014-01-28  Richard Biener  <rguenther@suse.de>

	PR rtl-optimization/45364
	PR rtl-optimization/59890
	* var-tracking.c (local_get_addr_clear_given_value): Handle
	already cleared slot.
	(val_reset): Handle not allocated local_get_addr_cache.
	(vt_find_locations): Use post-order on the inverted CFG.

Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/var-tracking.c
Comment 4 Richard Biener 2014-01-28 13:14:26 UTC
Author: rguenth
Date: Tue Jan 28 13:13:54 2014
New Revision: 207182

URL: http://gcc.gnu.org/viewcvs?rev=207182&root=gcc&view=rev
Log:
2014-01-28  Richard Biener  <rguenther@suse.de>

	Revert
	2014-01-28  Richard Biener  <rguenther@suse.de>

	PR rtl-optimization/45364
	PR rtl-optimization/59890
	* var-tracking.c (local_get_addr_clear_given_value): Handle
	already cleared slot.
	(val_reset): Handle not allocated local_get_addr_cache.
	(vt_find_locations): Use post-order on the inverted CFG.

Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/var-tracking.c
Comment 5 Richard Biener 2014-01-28 13:25:07 UTC
Seems to break bootstrap now (with -maccumulate-outgoing-args change now for i?86)
during libquadmath build where

/abuild/rguenther/obj/./gcc/xgcc -B/abuild/rguenther/obj/./gcc/ -B/usr/local/x86_64-unknown-linux-gnu/bin/ -B/usr/local/x86_64-unknown-linux-gnu/lib/ -isystem /usr/local/x86_64-unknown-linux-gnu/include -isystem /usr/local/x86_64-unknown-linux-gnu/sys-include -m32 -DHAVE_CONFIG_H -I. -I/space/rguenther/src/svn/trunk/libquadmath -I /space/rguenther/src/svn/trunk/libquadmath/../include -g -O2 -MT printf/mul_n.lo -MD -MP -MF printf/.deps/mul_n.Tpo -c /space/rguenther/src/svn/trunk/libquadmath/printf/mul_n.c -o printf/mul_n.o

doesn't converge anymore in vt_find_locations ().

Thus, reverted for now, testcase coming (to be attached here).
Comment 6 Richard Biener 2014-01-28 13:43:45 UTC
typedef unsigned long int mp_limb_t;
typedef mp_limb_t * mp_ptr;
typedef const mp_limb_t * mp_srcptr;
typedef long int mp_size_t;
mp_limb_t __quadmath_mpn_add_n (mp_ptr, mp_srcptr, mp_srcptr, mp_size_t);
mp_limb_t __quadmath_mpn_addmul_1 (mp_ptr, mp_srcptr, mp_size_t, mp_limb_t);
void __quadmath_mpn_impn_mul_n_basecase (mp_ptr prodp, mp_srcptr up,
                                         mp_srcptr vp, mp_size_t size)
{
  mp_size_t i;
  mp_limb_t cy_limb;
  mp_limb_t v_limb = vp[0];
  for (i = 1; i < size; i++)
    {
      if (v_limb <= 1)
        {
          cy_limb = 0;
          if (v_limb == 1)
            cy_limb = __quadmath_mpn_add_n (prodp, prodp, up, size);
        }
      else
        cy_limb = __quadmath_mpn_addmul_1 (prodp, up, size, v_limb);
      prodp[size] = cy_limb;
    }
}

fails to converge at -O2 -g -m32 with the patch applied.
Comment 7 Richard Biener 2019-05-08 15:20:00 UTC
RPO is correct for forward dataflow.