From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 7F8B3C77B7E for ; Fri, 28 Apr 2023 21:03:12 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id D1EB26B0072; Fri, 28 Apr 2023 17:03:11 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id CCEF96B0074; Fri, 28 Apr 2023 17:03:11 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id B6F4A6B0075; Fri, 28 Apr 2023 17:03:11 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0017.hostedemail.com [216.40.44.17]) by kanga.kvack.org (Postfix) with ESMTP id A55476B0072 for ; Fri, 28 Apr 2023 17:03:11 -0400 (EDT) Received: from smtpin28.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay03.hostedemail.com (Postfix) with ESMTP id 73EA4A03BB for ; Fri, 28 Apr 2023 21:03:11 +0000 (UTC) X-FDA: 80732024982.28.C16E036 Received: from mail-pf1-f174.google.com (mail-pf1-f174.google.com [209.85.210.174]) by imf14.hostedemail.com (Postfix) with ESMTP id 99A64100023 for ; Fri, 28 Apr 2023 21:03:09 +0000 (UTC) Authentication-Results: imf14.hostedemail.com; dkim=pass header.d=google.com header.s=20221208 header.b=YY4CcWGw; spf=pass (imf14.hostedemail.com: domain of ndesaulniers@google.com designates 209.85.210.174 as permitted sender) smtp.mailfrom=ndesaulniers@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1682715789; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=bPmTyVp46+VWPrLDPm2VDU5lMS2dX+ng5TIucBI1L2w=; b=GDX7OxHQmttJvhYSyAWRBoJynmOVXMpESfoUTEIiWGXYdFADxGe5nDtB1tQ2mk/bA0hqgd 7YHGmbqC/TIc+Tg4hOe6dfXeF1aq3E9HywT8xiUvPdliJNwOeOiWyV3bGHySbt3uX4DOul nc2KKMQ/BXzqPdP3r9EybYZXpuYsZKk= ARC-Authentication-Results: i=1; imf14.hostedemail.com; dkim=pass header.d=google.com header.s=20221208 header.b=YY4CcWGw; spf=pass (imf14.hostedemail.com: domain of ndesaulniers@google.com designates 209.85.210.174 as permitted sender) smtp.mailfrom=ndesaulniers@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1682715789; a=rsa-sha256; cv=none; b=NXgz2/pklkRp6GkItCloZ6WCW8+Aj/xxYIYFnlxfsiFJwuyZ+b8+ketDuBjqjFC8b0zdVo a4jv/QkbOOVpsWmzESJ5iorcrgiVNfXybWGomdjSHX0sY1FYKKXnxWpl8uV2A0WgK+/VE/ vmv02cRnZIiqIbW+Z811ItBCw6ZAx7k= Received: by mail-pf1-f174.google.com with SMTP id d2e1a72fcca58-63b57c49c4cso386777b3a.3 for ; Fri, 28 Apr 2023 14:03:09 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20221208; t=1682715788; x=1685307788; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=bPmTyVp46+VWPrLDPm2VDU5lMS2dX+ng5TIucBI1L2w=; b=YY4CcWGwj+xp3d9e0icFRi21jnpkEdqvx15VgL+ZUOzwriuWYZrsNgWXm6YjPzy6Nl GxtWeH4SiguJNxWBA2L0nvqZNww1snQmbbfUHc4whZ7bp7Nhxk/dqpPkjjGrzw5jDBpO 0o1NkovuBkTp0OFgtRuY7CiyEzdLFqBXtzLHiZ6iT3AezHEv9qM45ZYBKi2Cpw5LFxS+ tS9MdBYkaBynbJwNvJGBt1y1QAOFT6EJRC/WsxEdAcsByLyp/4e0PQ+Nd2hW4G2TgKLd Jxrjj0vwiSMXnRYHN78bNlyjr80g7lmEi8P6w6qmv6m0fuq34mABq534LA1TqNTlk8+I Ugzg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1682715788; x=1685307788; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=bPmTyVp46+VWPrLDPm2VDU5lMS2dX+ng5TIucBI1L2w=; b=a0dLPDlErkmJGSyu7v3Cw3h3vIbZ2y4JDhxzVfUGqkTptOwGF/JTbqQ4cqAME7hR58 nGS5i2fI4MKJvzAfVPHDSejEkITNMAc7nd6YM/0eXpXDubZgnnKV5F13ExdFvJx/C3lp mMvNYGxFvOvG4jmzjLF/ctEuw/oCRS+xK7asE8+vhbdWXnpgbCFaIfDBk/ZvU/pJKTBN xp/ErA4FjtAhcaUkC2k/MWntcnHMHkQRtN1hoEZZbjkXtAvf/mLd4fayDmV7nqEjEXyO im4+raCRSg+yojdvFzNxnqSfDPER9rvtlBV/kKfqyagEEkfJagxbsCW7FpoGLsHTkg13 0bgw== X-Gm-Message-State: AC+VfDwCQbqLUptEPAAlbDF0ayzPb3QOUSGpyPs4CrUj/RcApDCIdvbO hPx/T3x0zPWkpx0Eatdqx3pYm1KwOohomngcn4ee8zlN3JIaig7KNsUQZg== X-Google-Smtp-Source: ACHHUZ4/C+pcsGR45h3MfmotqGCbxqb7IuQVuzmvPi6rcbjzi2na6M/HMJEKu7VHxwuDV9/E3+xIKeVnwVdZrqTl+OA= X-Received: by 2002:a17:90a:fb53:b0:240:7f0d:9235 with SMTP id iq19-20020a17090afb5300b002407f0d9235mr6465444pjb.22.1682715788129; Fri, 28 Apr 2023 14:03:08 -0700 (PDT) MIME-Version: 1.0 References: <20230425041838.GA150312@mit.edu> In-Reply-To: From: Nick Desaulniers Date: Fri, 28 Apr 2023 14:02:56 -0700 Message-ID: Subject: Re: [GIT PULL] ext4 changes for the 6.4 merge window To: Linus Torvalds Cc: "Theodore Ts'o" , Nathan Chancellor , linux-ext4@vger.kernel.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org, Kees Cook Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Queue-Id: 99A64100023 X-Stat-Signature: 7i15cssw7t7qsnftmmmyasfmy3br6n6b X-Rspam-User: X-Rspamd-Server: rspam08 X-HE-Tag: 1682715789-625374 X-HE-Meta: U2FsdGVkX19PfCTDCF2ZV24458Poig6UjUH6tpF45Zi7msdj5uqzLU1VKH/pst/jCWWRVLOaJPgMGsGvzr1wiBdA3fzFeL1JyqOZPruLH5FBIsq7ULYm5IXv+hBEl/MIsm7yysMl87IM5Ld80InjIGUWQnhu24hHi4apVvN+78uRHdoJULbTXGzPUZKYajDYAiUDIYUGe5mTPNoSLDFwSJK4+cA6X0HWUoGiZKwMANu60+M5tST5mHwlNu2gkZ7B68s3a7ZKFYhbB9sDOFgOl/ekuI2HOWrWbuqbMKle5xPB4pyPGpheKcAYuvX49MtzlEpiftP9ujbR/XNaD1qdsPCOWh5Q6PuMUXQT+5L6YxCS4ea9n+9+2b3oOO4rvm9UWdk9dc4KJzuOGnVfagd5MfjBTjjDmbRDAIhVODIb5te3GOUQsmPpExqwJT7CS4DPFdwjiYu/Lg8Gn8bDPphRxPBrVELTlJguUWtDUYQZKc8H92bV9+1LOx2ndq61hPwpU8rb5jY3LDEOPZohthH7cicNaheQBC0SZMkrARcPKKcJmEy7AxnWsk471BSeYOybtYm5n32b1sLY+Ey6EMLwaifkJ1M+hHjqe/dCTS/Z4JJERU32DUMawjg1UtNJ4StVrQoGIjgT5096mzKBiCSZt1Ga9/Dk1blDL0E12x9hPZtwFML7x2baV4tSzolOkaB9wQ8dY3vn3vDC/KMN3C0JZoEvbZWKcEZGTq2pKC+QF+T1fZE6X19zVY9/w7mSqmz2qjZTOlzetsh1jQ6nLCtrMPeLwoMeG2uj1MKC6fp93vhwVsJBR63LdIUTMQbDmWPv/YmVlyglRYAIP3kKn8RHL8oT90PFBdbZWC6siHzVwSnMYaMBE9vnrsOJRH4moY9OEQ3UNdcI9qE2+iT24crch8KQINSv/EICojIO+RoiZ2l6g5iqCbMu6NAQVlOBM1C4AC92kMtywBI3y3cr49v IA1Wdu/G jyRb5tjkZXVHKlp+W0q7nSllx0GMhAtzrwynG7UFex+Gw1Ve0Hn+E6Mg/9qkqorvS6J4E9HDZxaOeFDIPrw3nZdaeGmrq1JDyYCStZM544gpCCGmd429xcz/MKqfoIDtZmbT0V/QHt84UtxPGI62wf8Ik0ZS0yfdUw6+f1bBQvn6S+Fxd7ivqc0jr0YrXxedSNCRnNTPvtMn9Bgox/hN3X3Er8ZjiDuLd/lhxAIN8WLwhg1Jpr7B6kHkPbWpH992kI8ERxhWmsf1vEbABiwfzXyeshvJnlUtUT54UCFRrQjkBXp1wCH/ywT8fCA== X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Wed, Apr 26, 2023 at 3:31=E2=80=AFPM Linus Torvalds wrote: > > On Wed, Apr 26, 2023 at 3:08=E2=80=AFPM Nick Desaulniers > wrote: > > > > Is this what you had in mind? > > ``` > > void * _Nonnull foo (void) > > ... > > void bar (void) { > > if (foo() =3D=3D NULL) // maybe should warn that foo() returns _Non= null? > > bar(); > > ... > > linus.c:8:15: warning: comparison of _Nonnull function call 'foo' > > equal to a null pointer is always false > > Yes. > > HOWEVER. > > I suspect you will find that it gets complicated for more indirect > uses, and that may be why people have punted on this. > > For example, let's say that you instead have > > void *bar(void) { return foo(); } > > and 'bar()' gets inlined. > > The obvious reaction to that is "ok, clearly the result is still > _Nonnull, and should warn if it is tested. > > But that obvious reaction is actually completely wrong, because it may > be that the real code looks something like > > void *bar(void) { > #if CONFIG_XYZ > if (somecondition) return NULL; > #endif > return foo(); } > > and the caller really *should* check for NULL - it's just that the > compiler never saw that case. I think having a return value be conditionally _Nonnull is "garbage in; garbage out." The straightforward case would be to have `bar` be conditionally _Nonnull on the same preprocessor condition. void * #ifndef CONFIG_XYZ _Nonnull #endif bar (void) { #ifdef CONFIG_XYZ if (somecondition) return NULL; #endif return foo(); } Then code reviewers could go: "yikes; please make bar unconditionally _Nonnull, or simply don't use _Nonnull here." > > So only testing the direct return value of a function should warn. > > And even that "direct return value" is not trivial. What happens if > you have something like this: > > void bar(void) { do_something(foo()); } > > and "do_something()" ends up being inlined - and checks for its > argument for being NULL? Again, that "test against NULL" may well be > absolutely required in that context - because *other* call-sites will > pass in pointers that might be NULL. > > Now, I don't know how clang works internally, but I suspect based just > on the size of your patch that your patch would get all of this > horribly wrong. Of course, it was a quick hack. > > So doing a naked > > void *ptr =3D foo(); > if (!ptr) ... > > should warn. > > But doing the exact same thing, except the test for NULL came in some > other context that just got inlined, cannot warn. Thinking more about this, I really think _Nonnull should behave like a qualifier (const, restrict, volatile). So the above example would be: void * _Nonnull ptr =3D foo(); if (!ptr) // warning: tautology That would require changes to the variables that calls to functions that return ERR_PTR's were stored in. That simplifies the semantic analysis, since the compiler can simply look at the declaration of `ptr` and not try to see that `ptr`'s value at some point in the control flow graph was something returned from calling a _Nonnull returning function. Because _Nonnull isn't modeled as a qualifier in clang today, this doesn't = warn: ``` void foo(void* _Nonnull); void *bar(void); void baz (void) { void *x =3D bar(); foo(x); } ``` It would be nice to say that "baz calls foo which expects its parameter to be _Nonnull, but you passed a pointer that could be null". I also wonder if casting works... Rust got this right; pretty sure they have NonNull and NonZero types. > > I _suspect_ that the reason clang does what it does is that this is > just very complicated to do well. > > It sounds easy to a human, but ... The bones are there; we could finish the damned thing if it sounds like something that would be useful/usable in the kernel? I guess the impetus is that ERR_PTR checks should be comparing against < 0 rather than =3D=3D NULL, since that's a tautology? --=20 Thanks, ~Nick Desaulniers