Bug 65610 - [5 Regression] Compare debug failure with -g3 -fsanitize=undefined -fno-sanitize=vptr -O3
Summary: [5 Regression] Compare debug failure with -g3 -fsanitize=undefined -fno-sanit...
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: ipa (show other bugs)
Version: 5.0
: P3 normal
Target Milestone: 5.0
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-03-27 18:03 UTC by Tobias Burnus
Modified: 2015-03-30 21:59 UTC (History)
5 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed:


Attachments
input.ii - compile with g++ -fcompare-debug -std=c++11 -g3 -fsanitize=undefined -fno-sanitize=vptr -O3 (477 bytes, text/plain)
2015-03-27 18:03 UTC, Tobias Burnus
Details
gcc5-pr65610.patch (2.50 KB, patch)
2015-03-30 10:12 UTC, Jakub Jelinek
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Tobias Burnus 2015-03-27 18:03:33 UTC
Created attachment 35166 [details]
input.ii - compile with g++ -fcompare-debug -std=c++11 -g3 -fsanitize=undefined -fno-sanitize=vptr -O3

I get a
  -fcompare-debug failure (length)
failure for the attached program.

To reproduce:

$ g++ -fcompare-debug -std=c++11 -g3 -fsanitize=undefined -fno-sanitize=vptr -O3 input.ii
Comment 1 Tobias Burnus 2015-03-27 18:05:16 UTC
Note that the "-fno-sanitize=vptr" is crucial: without it doesn't give an ICE.
Comment 2 Jakub Jelinek 2015-03-28 00:33:11 UTC
Seems to be a bug somewhere in ipa-polymorphic-call.c.
In particular, it seems noncall_stmt_may_be_vtbl_ptr_store returns different results between -g0 and -g - the function apparently walks BLOCKs and apparently with -g we have in there BLOCK_ABSTRACT_ORIGIN of some inlined dtor, while for -g0 something has optimized the blocks away.

Honza, can you please have a look?
Comment 3 Jakub Jelinek 2015-03-28 10:52:21 UTC
Perhaps one possibility would be even for -g0 preserve those specific BLOCKs (those satisfying
    if (BLOCK_ABSTRACT_ORIGIN (block)
        && TREE_CODE (BLOCK_ABSTRACT_ORIGIN (block)) == FUNCTION_DECL)
      {
        tree fn = BLOCK_ABSTRACT_ORIGIN (block);

        if (flags_from_decl_or_type (fn) & (ECF_PURE | ECF_CONST))
          return false;
        return (TREE_CODE (TREE_TYPE (fn)) == METHOD_TYPE
                && (DECL_CXX_CONSTRUCTOR_P (fn)
                    || DECL_CXX_DESTRUCTOR_P (fn)));
      }
).
Comment 4 Jan Hubicka 2015-03-28 18:31:41 UTC
> Perhaps one possibility would be even for -g0 preserve those specific BLOCKs
> (those satisfying

Yep, we should do that. Who is removing them?
Comment 5 Jakub Jelinek 2015-03-28 20:55:15 UTC
Haven't debugged that part yet.
Looking at decl_maybe_in_construction_p, we'd probably need to treat functions with DECL_ABSTRACT_ORIGIN being ctor or dtor.
I can look into this on Monday...
Comment 6 Jan Hubicka 2015-03-30 07:09:44 UTC
i guess it is remove_unused_scope_block_p who remove the blocks.  I guess
we want to consider blocks as live when BLOCK_ABSTRACT_ORIGIN is function decl, it is DECL_CXX_CONSTRUCTOR or DESTURCTOR and moreover it is polymorphic, i.e.
method_class_type (TREE_TYPE (fn)) has TYPE_BINFO and that binfo passes polymorphic_type_binfo_p

We may want to make predicates polymorphic_type_ctor/dtor to test that :)

I will try to look into it tomorrow, but given the timezone, I would not mind if you beat me :)
Comment 7 Jakub Jelinek 2015-03-30 09:33:46 UTC
I've tried:
--- gcc/ipa-polymorphic-call.c.jj	2015-03-09 08:05:06.000000000 +0100
+++ gcc/ipa-polymorphic-call.c	2015-03-30 11:24:48.280199943 +0200
@@ -513,6 +513,38 @@ contains_type_p (tree outer_type, HOST_W
 }
 
 
+/* Return a FUNCTION_DECL if BLOCK represents a constructor or destructor.
+   If CHECK_CLONES is true, also check for clones of ctor/dtors.  */
+
+tree
+ctor_dtor_block_p (tree block, bool check_clones)
+{
+  tree fn = BLOCK_ABSTRACT_ORIGIN (block);
+  if (fn == NULL || TREE_CODE (fn) != FUNCTION_DECL)
+    return NULL_TREE;
+
+  if (TREE_CODE (TREE_TYPE (fn)) != METHOD_TYPE
+      || (!DECL_CXX_CONSTRUCTOR_P (fn) && !DECL_CXX_DESTRUCTOR_P (fn)))
+    {
+      if (!check_clones)
+	return NULL_TREE;
+
+      /* Watch for clones where we constant propagated the first
+	 argument (pointer to the instance).  */
+      fn = DECL_ABSTRACT_ORIGIN (fn);
+      if (!fn
+	  || TREE_CODE (TREE_TYPE (fn)) != METHOD_TYPE
+	  || (!DECL_CXX_CONSTRUCTOR_P (fn) && !DECL_CXX_DESTRUCTOR_P (fn)))
+	return NULL_TREE;
+    }
+
+  if (flags_from_decl_or_type (fn) & (ECF_PURE | ECF_CONST))
+    return NULL_TREE;
+
+  return fn;
+}
+
+
 /* We know that the instance is stored in variable or parameter
    (not dynamically allocated) and we want to disprove the fact
    that it may be in construction at invocation of CALL.
@@ -552,28 +584,8 @@ decl_maybe_in_construction_p (tree base,
 
   for (tree block = gimple_block (call); block && TREE_CODE (block) == BLOCK;
        block = BLOCK_SUPERCONTEXT (block))
-    if (BLOCK_ABSTRACT_ORIGIN (block)
-	&& TREE_CODE (BLOCK_ABSTRACT_ORIGIN (block)) == FUNCTION_DECL)
+    if (tree fn = ctor_dtor_block_p (block, !base || is_global_var (base)))
       {
-	tree fn = BLOCK_ABSTRACT_ORIGIN (block);
-
-	if (TREE_CODE (TREE_TYPE (fn)) != METHOD_TYPE
-	    || (!DECL_CXX_CONSTRUCTOR_P (fn)
-		&& !DECL_CXX_DESTRUCTOR_P (fn)))
-	  {
-	    /* Watch for clones where we constant propagated the first
-	       argument (pointer to the instance).  */
-	    fn = DECL_ABSTRACT_ORIGIN (fn);
-	    if (!fn
-		|| (base && !is_global_var (base))
-	        || TREE_CODE (TREE_TYPE (fn)) != METHOD_TYPE
-		|| (!DECL_CXX_CONSTRUCTOR_P (fn)
-		    && !DECL_CXX_DESTRUCTOR_P (fn)))
-	      continue;
-	  }
-	if (flags_from_decl_or_type (fn) & (ECF_PURE | ECF_CONST))
-	  continue;
-
 	tree type = TYPE_MAIN_VARIANT (method_class_type (TREE_TYPE (fn)));
 
 	if (!outer_type || !types_odr_comparable (type, outer_type))
@@ -1163,15 +1175,7 @@ noncall_stmt_may_be_vtbl_ptr_store (gimp
        block = BLOCK_SUPERCONTEXT (block))
     if (BLOCK_ABSTRACT_ORIGIN (block)
 	&& TREE_CODE (BLOCK_ABSTRACT_ORIGIN (block)) == FUNCTION_DECL)
-      {
-	tree fn = BLOCK_ABSTRACT_ORIGIN (block);
-
-	if (flags_from_decl_or_type (fn) & (ECF_PURE | ECF_CONST))
-	  return false;
-	return (TREE_CODE (TREE_TYPE (fn)) == METHOD_TYPE
-		&& (DECL_CXX_CONSTRUCTOR_P (fn)
-		    || DECL_CXX_DESTRUCTOR_P (fn)));
-      }
+      return ctor_dtor_block_p (block, false);
   return (TREE_CODE (TREE_TYPE (current_function_decl)) == METHOD_TYPE
 	  && (DECL_CXX_CONSTRUCTOR_P (current_function_decl)
 	      || DECL_CXX_DESTRUCTOR_P (current_function_decl)));

which I hope doesn't change ipa-polymorphic-call.c behavior,
but before adjusting remove_unused_scope_block_p to also do
else if (!cfun->after_inlining && ctor_dtor_block_p (scope, true))
  unused = false;
I've noticed that the noncall_stmt_may_be_vtbl_ptr_store call unfortunately doesn't care just about the cdtor blocks, but about all blocks where BLOCK_ABSTRACT_ORIGIN is a FUNCTION_DECL.
Preserving all such blocks would be too costly for -g0 I guess; wonder if we e.g. could preserve in tree-ssa-live.c (!cfun->after_inlining ?) the ctor_dtor_block_p (scope, true) and also pass a bool with the return of that function down to the recursive remove_unused_scope_block_p calls and if that flag is true, preserve the outermost scope with FUNCTION_DECL block_abstract_origin there too.
Comment 8 Jakub Jelinek 2015-03-30 10:12:00 UTC
Created attachment 35180 [details]
gcc5-pr65610.patch

Untested fix.  Not at all sure about the if (!cfun->after_inlining) guard, dunno when we can call the ipa-polymorphic-call.c stuff, if it can happen also later on, the guard should be removed.
Comment 9 Jakub Jelinek 2015-03-30 21:56:34 UTC
Author: jakub
Date: Mon Mar 30 21:56:02 2015
New Revision: 221781

URL: https://gcc.gnu.org/viewcvs?rev=221781&root=gcc&view=rev
Log:
	PR ipa/65610
	* ipa-utils.h (inlined_polymorphic_ctor_dtor_block_p): Declare.
	* ipa-polymorphic-call.c (inlined_polymorphic_ctor_dtor_block_p): New
	function.
	(decl_maybe_in_construction_p, noncall_stmt_may_be_vtbl_ptr_store):
	Use it.
	* ipa-prop.c (param_type_may_change_p): Likewise.
	* tree-ssa-live.c: Include ipa-utils.h and its dependencies.
	(remove_unused_scope_block_p): Add in_ctor_dtor_block
	argument.  Before inlining, preserve
	inlined_polymorphic_ctor_dtor_block_p blocks and the outermost block
	with FUNCTION_DECL BLOCK_ABSTRACT_ORIGIN inside of them.  Adjust
	recursive calls.
	(remove_unused_locals): Adjust remove_unused_scope_block_p caller.

	* g++.dg/ubsan/pr65610.C: New test.

Added:
    trunk/gcc/testsuite/g++.dg/ubsan/pr65610.C
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/ipa-polymorphic-call.c
    trunk/gcc/ipa-prop.c
    trunk/gcc/ipa-utils.h
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/tree-ssa-live.c
Comment 10 Jakub Jelinek 2015-03-30 21:59:18 UTC
Fixed.