From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [216.205.24.124]) by sourceware.org (Postfix) with ESMTP id 693F3384A01D for ; Sat, 5 Dec 2020 03:40:38 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 693F3384A01D Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-193-lXfGzMuNNMquRpyj1Fd5wg-1; Fri, 04 Dec 2020 22:40:36 -0500 X-MC-Unique: lXfGzMuNNMquRpyj1Fd5wg-1 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 8C29E800D53 for ; Sat, 5 Dec 2020 03:40:35 +0000 (UTC) Received: from redhat.com (ovpn-117-246.rdu2.redhat.com [10.10.117.246]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 1F31B19809; Sat, 5 Dec 2020 03:40:35 +0000 (UTC) Date: Fri, 4 Dec 2020 22:40:33 -0500 From: Marek Polacek To: Jason Merrill Cc: GCC Patches Subject: Re: [PATCH] c++: ICE with -fsanitize=vptr and constexpr dynamic_cast [PR98103] Message-ID: <20201205034033.GD2286574@redhat.com> References: <20201202231857.2416552-1-polacek@redhat.com> <1a4d9b45-e23e-b05a-bfee-7c9bc33776a8@redhat.com> MIME-Version: 1.0 In-Reply-To: <1a4d9b45-e23e-b05a-bfee-7c9bc33776a8@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=us-ascii Content-Disposition: inline X-Spam-Status: No, score=-15.0 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 05 Dec 2020 03:40:39 -0000 On Wed, Dec 02, 2020 at 09:01:48PM -0500, Jason Merrill wrote: > On 12/2/20 6:18 PM, Marek Polacek wrote: > > -fsanitize=vptr initializes all vtable pointers to null so that it can > > catch invalid calls; see cp_ubsan_maybe_initialize_vtbl_ptrs. That > > means that evaluating a vtable reference can produce a null pointer > > in this mode, so cxx_eval_dynamic_cast_fn should check that. > > Yes, but we shouldn't accept it silently; sanitize is supposed to flag > undefined behavior, not allow it. If we see a null vptr, we should complain > and set *non_constant_p. True, I shouldn't have left it for the run-time diagnostic. How's this, then? Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? -- >8 -- -fsanitize=vptr initializes all vtable pointers to null so that it can catch invalid calls; see cp_ubsan_maybe_initialize_vtbl_ptrs. That means that evaluating a vtable reference can produce a null pointer in this mode, so cxx_eval_dynamic_cast_fn should check that and give and error. gcc/cp/ChangeLog: PR c++/98103 * constexpr.c (cxx_eval_dynamic_cast_fn): If the evaluating of vtable yields a null pointer, give an error and return. Use objtype. gcc/testsuite/ChangeLog: PR c++/98103 * g++.dg/ubsan/vptr-18.C: New test. --- gcc/cp/constexpr.c | 11 ++++++++++- gcc/testsuite/g++.dg/ubsan/vptr-18.C | 25 +++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 1 deletion(-) create mode 100644 gcc/testsuite/g++.dg/ubsan/vptr-18.C diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c index e0d358027c9..c413313fbe1 100644 --- a/gcc/cp/constexpr.c +++ b/gcc/cp/constexpr.c @@ -1998,11 +1998,20 @@ cxx_eval_dynamic_cast_fn (const constexpr_ctx *ctx, tree call, to the object under construction or destruction, this object is considered to be a most derived object that has the type of the constructor or destructor's class. */ - tree vtable = build_vfield_ref (obj, TREE_TYPE (obj)); + tree vtable = build_vfield_ref (obj, objtype); vtable = cxx_eval_constant_expression (ctx, vtable, /*lval*/false, non_constant_p, overflow_p); if (*non_constant_p) return call; + /* With -fsanitize=vptr, we initialize all vtable pointers to null, + so it's possible that we got a null pointer now. */ + if (integer_zerop (vtable)) + { + if (!ctx->quiet) + error_at (loc, "virtual table pointer is used uninitialized"); + *non_constant_p = true; + return integer_zero_node; + } /* VTABLE will be &_ZTV1A + 16 or similar, get _ZTV1A. */ vtable = extract_obj_from_addr_offset (vtable); const tree mdtype = DECL_CONTEXT (vtable); diff --git a/gcc/testsuite/g++.dg/ubsan/vptr-18.C b/gcc/testsuite/g++.dg/ubsan/vptr-18.C new file mode 100644 index 00000000000..cd2ca0a9fb6 --- /dev/null +++ b/gcc/testsuite/g++.dg/ubsan/vptr-18.C @@ -0,0 +1,25 @@ +// PR c++/98103 +// { dg-do compile { target c++20 } } +// { dg-additional-options "-fsanitize=vptr -fno-sanitize-recover=vptr" } +// Modified constexpr-dynamic17.C. + +struct V { + virtual void f(); +}; + +struct A : V { }; + +struct B : V { + constexpr B(V*, A*); +}; + +struct D : B, A { + constexpr D() : B((A*)this, this) { } +}; + +constexpr B::B(V* v, A* a) +{ + dynamic_cast(a); // { dg-error "uninitialized" } +} + +constexpr D d; base-commit: df933e307b1950ce12472660dcac1765b8eb431d -- 2.28.0