From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mailrelay.tugraz.at (mailrelay.tugraz.at [129.27.2.202]) by sourceware.org (Postfix) with ESMTPS id 12AF43858D20 for ; Wed, 31 May 2023 08:12:19 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 12AF43858D20 Authentication-Results: sourceware.org; dmarc=pass (p=quarantine dis=none) header.from=tugraz.at Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=tugraz.at Received: from fbmtpc21.tugraz.at (fbmtpc21.tugraz.at [129.27.144.40]) by mailrelay.tugraz.at (Postfix) with ESMTPSA id 4QWMSH0rfnz1LM0K; Wed, 31 May 2023 10:12:14 +0200 (CEST) DKIM-Filter: OpenDKIM Filter v2.11.0 mailrelay.tugraz.at 4QWMSH0rfnz1LM0K DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=tugraz.at; s=mailrelay; t=1685520735; bh=TCcaqA+GKVIK76RE3h7gWSu7FTlaAkTNmPht6jxoWb4=; h=Subject:From:To:Cc:Date:In-Reply-To:References:From; b=r5tJnkqlpDZTHRN45x8p/C9Kc/2S1pDFOMbg+hACg1kVAzyUBo8+q50FGfZBb7paz PT7VujxSaWPJm29IdYijEmHYoNWdc6vsbOMgcQl2zo4CPzFML4KkmtCaBtB4VwCQGs JDV4BUOcgKBOyOGmFcSJPnGma14GBaML2aYEkJYA= Message-ID: Subject: Re: [C PATCH 3/4] introduce ubsan checking for assigment of VM types 3/4 From: Martin Uecker To: Joseph Myers Cc: gcc-patches@gcc.gnu.org, Martin =?UTF-8?Q?Li=C5=A1ka?= Date: Wed, 31 May 2023 10:12:14 +0200 In-Reply-To: <77865f49-763-1ecf-36a7-e7bc21b2e0e2@codesourcery.com> References: <93a1692e7f0e895379cb6847bfcb6e6d3dafadc3.camel@tugraz.at> <6a868211b3892c5c9161eb5fae908195eec728ce.camel@tugraz.at> <77865f49-763-1ecf-36a7-e7bc21b2e0e2@codesourcery.com> Content-Type: text/plain; charset="UTF-8" User-Agent: Evolution 3.38.3-1+deb11u2 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-TUG-Backscatter-control: G/VXY7/6zeyuAY/PU2/0qw X-Spam-Scanner: SpamAssassin 3.003001 X-Spam-Score-relay: -1.9 X-Scanned-By: MIMEDefang 2.74 on 129.27.10.116 X-Spam-Status: No, score=-3.6 required=5.0 tests=BAYES_00,BODY_8BITS,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,KAM_NUMSUBJECT,RCVD_IN_DNSWL_LOW,RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: Am Dienstag, dem 30.05.2023 um 22:59 +0000 schrieb Joseph Myers: > On Mon, 29 May 2023, Martin Uecker via Gcc-patches wrote: > > >     Support instrumentation of function arguments for functions > >     called via a declaration.  We can support only simple size > > What do you mean by "via a declaration"? > > If the *definition* is visible (and known to be the definition used > at runtime rather than being interposed) then you can determine in > some cases that there is UB from bad bounds.  If only some other > declaration is visible, or the definition might be interposed, VLA > sizes in the declaration are equivalent to [*]; it's suspicious if > they don't match, but it's not UB and so it would seem rather > questionable for UBSan to treat it as such (cf. the rejection in  > GCC of sanitization for some questionable cases of unsigned integer > overflow that aren't UB either). You are right that it is UB only with the additional assumption that the bounds in the seen declaration are the same as the ones in the definition. But we now warn about any mismatch since GCC 11 with -Wall based on the understanding  that any such mismatch should be considered a bug. There also does not seem  to be any valid use case for having mismatching bounds and I think the intention  of WG14 is clearly that they can be used for checking  (cf. WG14 charter). So I think this is a different situation for unsigned integer overflow. Fom a practial point of view is is certainly very useful  for users to be able to verify these bounds at run-time.  But we could make it a separate UBSan option if it is really a concern. BTW: There was a similar discussion years ago about making certain bound checks for arrays part of UBSan because it is not clear that the bounds in the type of 'x' in x[n] are relevant rather than the ones of the underlying array (which may be different). In the end both GCC and clang have these UBSan checks now and I think  everybody is happy about it. > > +         /*  Give up.  If we do not understand a size expression, > > we can > > +             also not instrument any of the others because it may > > have > > +             side effects affecting them.  (We could restart and > > instrument > > +             the only the ones with integer constants.)   */ > > +           warning_at (location, 0, "Function call not > > instrumented."); > > +           return void_node; > > This is not a properly formatted diagnostic message (should start > with a > lowercase letter and not end with '.'). Thanks. I would probably remove this warning and re-introduce it with another patch that also adds an option fir it. Martin