[Bug c++/95221] g++.dg/ubsan/vptr-12.C fails with -fstrong-eval-order=all
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95221 --- Comment #9 from CVS Commits --- The master branch has been updated by Jason Merrill : https://gcc.gnu.org/g:172f2c42a10fd470c93f1e84575de9766c157591 commit r11-775-g172f2c42a10fd470c93f1e84575de9766c157591 Author: Jason Merrill Date: Mon Jun 1 16:20:38 2020 -0400 c++: vptr ubsan and object of known type [PR95466] Another case where we can't find the OBJ_TYPE_REF_OBJECT in the OBJ_TYPE_REF_EXPR. So let's just evaluate the sanitize call first. gcc/cp/ChangeLog: PR c++/95466 PR c++/95311 PR c++/95221 * class.c (build_vfn_ref): Revert 95311 change. * cp-ubsan.c (cp_ubsan_maybe_instrument_member_call): Build a COMPOUND_EXPR. gcc/testsuite/ChangeLog: PR c++/95466 * g++.dg/ubsan/vptr-17.C: New test.
[Bug c++/95221] g++.dg/ubsan/vptr-12.C fails with -fstrong-eval-order=all
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95221 --- Comment #8 from CVS Commits --- The master branch has been updated by Jason Merrill : https://gcc.gnu.org/g:8e915901deb3518d4bef73ea52eab2ece7a2bbf6 commit r11-728-g8e915901deb3518d4bef73ea52eab2ece7a2bbf6 Author: Jason Merrill Date: Fri May 29 11:59:33 2020 -0400 c++: vptr ubsan and derived class [PR95311]. We weren't able to find OBJ_TYPE_REF_OBJECT walking through OBJ_TYPE_REF_EXPR because we had folded away the ADDR_EXPR. gcc/cp/ChangeLog: PR c++/95311 PR c++/95221 * class.c (build_vfn_ref): Don't fold the INDIRECT_REF. gcc/testsuite/ChangeLog: PR c++/95311 * g++.dg/ubsan/vptr-16.C: New test.
[Bug c++/95221] g++.dg/ubsan/vptr-12.C fails with -fstrong-eval-order=all
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95221 Jason Merrill changed: What|Removed |Added Target Milestone|--- |11.0 Status|ASSIGNED|RESOLVED Resolution|--- |FIXED --- Comment #7 from Jason Merrill --- Fixed.
[Bug c++/95221] g++.dg/ubsan/vptr-12.C fails with -fstrong-eval-order=all
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95221 Jason Merrill changed: What|Removed |Added Assignee|unassigned at gcc dot gnu.org |jason at gcc dot gnu.org Status|NEW |ASSIGNED
[Bug c++/95221] g++.dg/ubsan/vptr-12.C fails with -fstrong-eval-order=all
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95221 Jason Merrill changed: What|Removed |Added Ever confirmed|0 |1 Last reconfirmed||2020-05-20 Status|UNCONFIRMED |NEW --- Comment #6 from Jason Merrill --- (In reply to Marek Polacek from comment #2) > I think the thing is that we have a CALL_EXPR, something like > > OBJ_TYPE_REF (...) (.UBSAN_VPTR ()) > > and now we first evaluate the OBJ_TYPE_REF. Yep, that's the problem. We're currently sanitizing the vtable when determining the 'this' argument to the function, separately from looking at the vtable in order to determine which function to call. In C++14 we evaluate 'this' first, in C++17 we evaluate the function first. build_over_call tries to share any side-effects between the 'this' and the OBJ_TYPE_REF with a save_expr, but then cp_ubsan_maybe_instrument_member_call adds the sanitization only to the 'this' argument, breaking that sharing and introducing a fragile order dependency. A simple fix might be to not preevaluate when CALL_EXPR_FN is an OBJ_TYPE_REF, as the sharing above should mean that side-effects happen when the object argument is evaluated, which should be what we want. It occurs to me that the current preevaluation is wrong for a virtual assignment when CALL_EXPR_REVERSE_ARGS is set.
[Bug c++/95221] g++.dg/ubsan/vptr-12.C fails with -fstrong-eval-order=all
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95221 --- Comment #5 from Marek Polacek --- You're not wrong, but here we're dealing with the undefined behavior sanitizer whose point is to detect broken code like the above.
[Bug c++/95221] g++.dg/ubsan/vptr-12.C fails with -fstrong-eval-order=all
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95221 --- Comment #4 from Andrew Pinski --- Is this code even defined? We call a method after calling the deconstructor on the object? If we do: c->~MyClass (); new(c) MyClass(); c->Doit (); Then it is defined. Or am I wrong about that?
[Bug c++/95221] g++.dg/ubsan/vptr-12.C fails with -fstrong-eval-order=all
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95221 --- Comment #3 from Marek Polacek --- And for completeness, the asm for the -fstrong-eval-order=all case: movq%rbx, %rdi call*%r12 movq-24(%rbp), %rax movq(%rax), %rax addq$16, %rax movq(%rax), %r12 # crash here Any ideas what as to we want to do here?
[Bug c++/95221] g++.dg/ubsan/vptr-12.C fails with -fstrong-eval-order=all
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95221 --- Comment #2 from Marek Polacek --- I think the thing is that we have a CALL_EXPR, something like OBJ_TYPE_REF (...) (.UBSAN_VPTR ()) and now we first evaluate the OBJ_TYPE_REF. In this case this is what seems to happen here: 1) we evaluate OBJ_TYPE_REF before the arguments: OBJ_TYPE_REF(_2;(struct MyClass)c->0) (c.0); I think this calls the destructor of 'c', and that clears up the vptr: this->_vptr.MyClass = 0B; 2) then we fetch the vptr: _5 = c->_vptr.MyClass; // _5 == null 3) we add the offset to the vtable for Doit: _6 = _5 + 16; // _6 == 16 4) we deref the pointer and crash _7 = *_6; With -fstrong-eval-order=some we instead do this _3 = c->_vptr.MyClass; _4 = *_3; OBJ_TYPE_REF(_4;(struct MyClass)c->0) (c.0); so there's no crash, and the subsequent call to the ubsan routine: _5 = c.1->_vptr.MyClass; _6 = (long unsigned int) _5; .UBSAN_VPTR (c.1, _6, 2968783897514143607, &_ZTI7MyClass, 4B); reports the error, because it sees that _6 is null.
[Bug c++/95221] g++.dg/ubsan/vptr-12.C fails with -fstrong-eval-order=all
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95221
--- Comment #1 from Marek Polacek ---
It is caused by this code:
853 if (flag_strong_eval_order == 2
854 && CALL_EXPR_FN (*expr_p)
855 && cp_get_callee_fndecl_nofold (*expr_p) == NULL_TREE)
856 {
857 tree fnptrtype = TREE_TYPE (CALL_EXPR_FN (*expr_p));
858 enum gimplify_status t
859 = gimplify_to_rvalue (_EXPR_FN (*expr_p), pre_p, NULL,
860 is_gimple_call_addr);
861 if (t == GS_ERROR)
862 ret = GS_ERROR;
863 /* GIMPLE considers most pointer conversion useless, but for
864 calls we actually care about the exact function pointer type.
*/
865 else if (TREE_TYPE (CALL_EXPR_FN (*expr_p)) != fnptrtype)
866 CALL_EXPR_FN (*expr_p)
867 = build1 (NOP_EXPR, fnptrtype, CALL_EXPR_FN (*expr_p));
868 }
and causes this difference:
--- 1g 2020-05-19 11:42:32.123168379 -0400
+++ 2g 2020-05-19 11:42:41.254133594 -0400
@@ -18,21 +18,21 @@
operator delete (D.2725, 8);
}
c = D.2725;
+_1 = c->_vptr.MyClass;
+_2 = *_1;
c.0 = c;
-_1 = c.0->_vptr.MyClass;
-_2 = (long unsigned int) _1;
-.UBSAN_VPTR (c.0, _2, 2968783897514143607, &_ZTI7MyClass, 4B);
-_3 = c->_vptr.MyClass;
-_4 = *_3;
-OBJ_TYPE_REF(_4;(struct MyClass)c->0) (c.0);
+_3 = c.0->_vptr.MyClass;
+_4 = (long unsigned int) _3;
+.UBSAN_VPTR (c.0, _4, 2968783897514143607, &_ZTI7MyClass, 4B);
+OBJ_TYPE_REF(_2;(struct MyClass)c->0) (c.0);
+_5 = c->_vptr.MyClass;
+_6 = _5 + 16;
+_7 = *_6;
c.1 = c;
-_5 = c.1->_vptr.MyClass;
-_6 = (long unsigned int) _5;
-.UBSAN_VPTR (c.1, _6, 2968783897514143607, &_ZTI7MyClass, 4B);
-_7 = c->_vptr.MyClass;
-_8 = _7 + 16;
-_9 = *_8;
-OBJ_TYPE_REF(_9;(struct MyClass)c->2) (c.1);
+_8 = c.1->_vptr.MyClass;
+_9 = (long unsigned int) _8;
+.UBSAN_VPTR (c.1, _9, 2968783897514143607, &_ZTI7MyClass, 4B);
+OBJ_TYPE_REF(_7;(struct MyClass)c->2) (c.1);
D.2751 = 0;
return D.2751;
}
