From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr1-x42a.google.com (mail-wr1-x42a.google.com [IPv6:2a00:1450:4864:20::42a]) by sourceware.org (Postfix) with ESMTPS id ADB383858D1E; Wed, 4 May 2022 22:47:46 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org ADB383858D1E Received: by mail-wr1-x42a.google.com with SMTP id x18so3876221wrc.0; Wed, 04 May 2022 15:47:46 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=a9kxupuCXbMFDfRcN3vgb/ud4InfM/QLI9kXyOECTG8=; b=YFJy74BzZxv0xUkb6oAVz8+aS4bEwJiyguZXpXcjOVD/1M1rWk9zsDmwzRjD1hgYsS Al+hHHcC0J86TF8DLk9AGuXY7p75P9lK1JZelErn7b2dkJm0VOobeceNftodyENI6H5Y 6rjvcmqODg5koRmBoZF6lYnZi3L3ZBgMNuS4eseD4QP+ISAOvrZimJB2lDruLtMiolOK f3jzFr/cCm9rW0xg4mKiXDj7wTdSrZVbMyPFYOYB1I5C3RKnMqwKdVLOOt0huhO8R0G5 gahyp4BJvD64HJWhzRjRL42xBrXguxVaMjXWkk+4oACy/laJonpxaLMe8q/rm4i/tRIe Kd4g== X-Gm-Message-State: AOAM532f2MNAwj2UVDw9T1u08uaU11Z/atzEeAZWENMJrn2gIXGIrrT9 9kJs+NOAS7+/SP/O81Onenwbiyn6PKWM50EmkW4= X-Google-Smtp-Source: ABdhPJwL5N49xXE2OHqBYAZ308UDtknRT+IYfkyRTZOztMR4Dus/QYw+uJH332nPJ7eR1wiJZV6YWCnefuOcaBIQ/xI= X-Received: by 2002:a5d:4806:0:b0:20a:da03:711b with SMTP id l6-20020a5d4806000000b0020ada03711bmr17599231wrq.395.1651704465240; Wed, 04 May 2022 15:47:45 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: Jonathan Wakely Date: Wed, 4 May 2022 23:47:34 +0100 Message-ID: Subject: Re: [PATCH] libstdc++: fix pointer type exception catch [PR105387] To: Jakob Hasse Cc: "libstdc++@gcc.gnu.org" , Rocha Euripedes , Ivan Grokhotkov , gcc-patches Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-0.9 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, KAM_SHORT, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: libstdc++@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libstdc++ mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 04 May 2022 22:47:48 -0000 On Wed, 4 May 2022 at 12:14, Jonathan Wakely wrote: > > On Tue, 3 May 2022 at 11:57, Jakob Hasse via Libstdc++ > wrote: > > > > This is a patch for the bug 105387 reported in bugzilla: 105387 https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105387. This report should contain all the necessary information about the issue. But the patch there was just a preliminary one. > > I created a proper patch with the fix and a test case, based on TAG releases/gcc-11.2.0 (7ca388565af176bd4efd4f8db1e5e9e11e98ef45). The changelog is part of the commit message in the patch. > > Thanks for the patch! > > Some boring administrative comments: > > In the summary line of the git commit message: > - The component should be libstdc++ not libstdc++-v3. > - The PR number should include "PR" i.e. [PR105387]. > Your email Subject: gets this right, but the patch doesn't. > This is (not very well) documented at > https://gcc.gnu.org/contribute.html#patches > > Your new testcase has a FSF copyright notice. Unless you have already > completed the paperwork to assign your work (either for just this > change, or this and all future changes) to the FSF then you need to > either do that legal paperwork, or alternatively contribute under the > DCO terms without assigning copyright. See > https://gcc.gnu.org/contribute.html#legal > For simplicity and to expedite the process, I suggest just removing > the copyright notice and license notice from the testcase, and adding > a Signed-off-by: tag to the commit message, as per > https://gcc.gnu.org/dco.html > > As for the actual code ... The commit msg says: "In case RTTI is enabled, this does not seem to be a problem because RTTI-based checks would run before and prevent running into the bad down-cast. However, since the check is very simple and I'm not 100% sure about the RTTI-case, it has been left for both cases (RTTI and no-RTTI)." I think we don't want to do the redundant __is_pointer_p() call for the RTTI-enabled case. For that case we know it's a pointer, or we'd have returned early, so making an extra virtual call is just unnecessary overhead. So I think your new check should be done in an #else branch of the existing #if __cpp_rtti block. More importantly, I think your patch breaks this case, which currently works with or without RTTI: struct X { int i; }; try { throw &X::i; } catch (const int X::*) { } For this case __is_pointer_p() is false, but the cast to __pbase_type_info is safe, and required for correctness. I think with your patch the exception won't be caught. Maybe that's a sacrifice we have to make in order to avoid undefined behaviour for your original example, but it's unfortunate if that's the case.