From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-lf1-x133.google.com (mail-lf1-x133.google.com [IPv6:2a00:1450:4864:20::133]) by sourceware.org (Postfix) with ESMTPS id 4DB793857C66 for ; Sat, 28 Oct 2023 23:50:21 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 4DB793857C66 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=jguk.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=jguk.org ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 4DB793857C66 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2a00:1450:4864:20::133 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1698537023; cv=none; b=l3WG1zqRcoW3UK6jIHJvNjf6Hd31LaP9I0cFcjMaSsp6Z4Hzqd0xEuak1EiN14BH2HjaQkacC9xOOjoaaqWkHIz32ufliU5K/9zj294ohDmvQqlEs54Kj2l9F8b1hxz7PYXcuYT0wvWhOISStCKJlVwGb7sWCDOlcHE4Vd510Rc= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1698537023; c=relaxed/simple; bh=MFBAeW7/bzI7+N8X+0J6Df1Wem2NEdHynk7Vo7gv51E=; h=DKIM-Signature:MIME-Version:From:Date:Message-ID:Subject:To; b=klsE0JAILvi9UB7yDVs6OIbfAhUqHOrJFvM3i4RaUP1nYWAv5dXUqYdtsdgKLctmCQX0xnn91SQXowJ14BriZbNAe+/BA2tuBy30T9dI/roy4aIFh56zoBT3O+hDp117y4Hcsp/G4jZbNtqLKU9ENasNeBYnhFple2wKauAnEvg= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-lf1-x133.google.com with SMTP id 2adb3069b0e04-509109104e2so843961e87.3 for ; Sat, 28 Oct 2023 16:50:21 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=jguk.org; s=google; t=1698537019; x=1699141819; darn=sourceware.org; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=csoVbTsLooDl5D6U6V7itLmhzkOtmkgNSjGiP/adGvU=; b=UhQ9uJ3k8roBiprbh/H+KGFDqFtQ+ar9TNgGTduTRDANQdG5c3YXYBK0KP1HZ7JSWl llDrgdmb0T5FovrcZVec4KGRYtMCNdA17wkMwDrDfYIUiCggk0DtpOpj7Xir0K22y8FG j+ZUbmt7ZB/zBokGrDhnaedN+8IZDLpqclXw6vVbWhJ+wOFmb/7d2/yoIX4hZLWAtRLl qflXr6h73VF+4zLIqTWRhw4YS9ooEa/bGja5xkdxBdEGQg+o7PhkQ+W0I+DK9eF8An6z MvmGN2g8LBnyzInqj6WnMKKAfUCUTfdPPCUpgTwSE5muwnNLSyY/5KU/6QkqUYXsMAa/ luRg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1698537019; x=1699141819; h=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=csoVbTsLooDl5D6U6V7itLmhzkOtmkgNSjGiP/adGvU=; b=rabKDTXbm9A5JuR/qWFqsIS97TkR4ZzqnEFVnmGYx1PCVaC6cs9OjOucm+PX7bq+i3 ySV/txpVmD2cNCjS3MorhX9a7XbhNrn4W3IBOmsO9mQVnjuNUrBi0GjqGG47AJwjrzjX k2W3oEvkKvy/JLvFkck9MSrt99H4O4vKfIQDAqBheWNSHD1k+eMqnIu2t0O81Ryh+lrv BzYkCUXhFy4wQ6BKUejieBV89w35P3xm7UNKz37+Of3/jVFD7AnyIwAPIlOGlEIqBWAN vLXrKAfTIJhocQaHZs63rad0npQMDCSsc1KCJOWHBkOGjDH/nrMpoaBSWMYHLDCzNzS9 675w== X-Gm-Message-State: AOJu0YwZlWN93ceBcTdrA4pRBMB89xIY6z5RrK4X+PSTHnqFe5F63QWy xgsTmm/PjbniWz8WNeIrGOhl1uu86iaGXGwnkO4bJg== X-Google-Smtp-Source: AGHT+IE0buErz9CgyHH9katpZz2Nj4nmqqyf6Vteh/MUJUM2izM40RJut1788eZkvZzMvO/RwtO6E0ZraakvQYpZ5jc= X-Received: by 2002:a05:6512:2017:b0:4fe:551:3d3c with SMTP id a23-20020a056512201700b004fe05513d3cmr4232438lfb.36.1698537019101; Sat, 28 Oct 2023 16:50:19 -0700 (PDT) MIME-Version: 1.0 References: <25d0b6fa-7b45-3f8e-946a-ad3256e211a4@jguk.org> <0d99df74-fb83-1647-ca19-17d2229f0ae0@linaro.org> <514c11a4-405b-f7f3-9a67-0b6c10ad7740@jguk.org> <21bc9125ab8ced26aa85f3f787f084c4af460a18.camel@xry111.site> In-Reply-To: <21bc9125ab8ced26aa85f3f787f084c4af460a18.camel@xry111.site> From: Jonny Grant Date: Sun, 29 Oct 2023 00:50:07 +0100 Message-ID: Subject: Re: glibc misc/sys/cdefs.h nonull - typo in comment To: Xi Ruoyao Cc: Adhemerval Zanella Netto , GNU C Library , Paul Eggert Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-1.0 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP 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: On Wed, 12 Apr 2023 at 17:26, Xi Ruoyao wrote: > > On Wed, 2023-04-12 at 16:56 +0100, Jonny Grant wrote: > > > Do you think it is risky to have the nonnull attribute in use in glibc? > > Some projects have abandoned attribute nonnull due to the GCC optimizer using the nonnull attribute to remove the runtime null pointer checks. > > https://gitlab.com/gnuwget/wget2/-/issues/200 > > No because in the C standard & POSIX standard the text is clear that > many arguments just should not be NULL. So the implementation does not > need to check if such an argument is NULL. Could you give an example of a POSIX API text you refer to that specifies many arguments should not be NULL? I recall they seem to omit mentioning, just implying it must be a valid pointer. Take puts(), it doesn't mention https://pubs.opengroup.org/onlinepubs/9699919799/functions/puts.html But I hear you, I get it, these APIs have been like this for since the beginning requiring a valid pointer. So passing a NULL pointer is UB and will likely crash, which is what we would try to avoid by wrapping library function calls. > If the standard allows an argument to be NULL but there is attribute > nonnull there, it's a bug. Please open a ticket in > https://sourceware.org/bugzilla if you find such a bug. But when there > is no bugs, we have no reason to remove avoid nonnull for "comforting > some people". If I see any, I will be sure to file a bug report. We generally would wrap library functions on application side to check for NULL for functional safety. > > > Currently cdefs.h has > > > > /* The nonnull function attribute marks pointer parameters that > > must not be NULL. This has the name __nonnull in glibc, > > and __attribute_nonnull__ in files shared with Gnulib to avoid > > collision with a different __nonnull in DragonFlyBSD 5.9. */ > > > > Is it better to clarify this to be something like the following? > > > > /* The nonnull function attribute marks pointer parameters that > > the compiler's optimizer knows will never be NULL. This means NULL checks can be optimized out. > > No because such NULL checks which can be removed by nonnull attribute > should not exist in Glibc. If any one exists, it's a bug (CWE-1164 > "Irrelevant Code"). Again open a ticket if you find one. But I guess > you won't find any, because such a bug will be rejected by "gcc -Wall - > Werror" and there are many people building Glibc with this. > > And the user should not assume any implementation details in Glibc > functions. For example: > > size_t > f (const char *s) > { > size_t r = strlen (s); > return s ? r : 0; > } > > will do strange things no matter if nonnull is used for strlen. Even if > you remove the nonnull attribute of strlen and expect this thing to > work, the different implementations (in many Glibc ports, heavily > optimized assembly code) can still do unexpected things. The people > writing the implementation of strlen will assume the argument is not > null anyway because the standard says so. Could you share the page of the standard you are referring to please? POSIX standard doesn't mention on the strlen page: https://pubs.opengroup.org/onlinepubs/9699919799/functions/strlen.html C99 7.21.6.3 strlen doesn't mention. > > So this is just broken in the nature, regardless of nonnull or not. > > OTOH > > size_t > f (const char *s) > { > return s ? strlen (r) : 0; > } I get it, the condition is checked before the call to strlen() that doesn't check for the null pointer constant. > is well-defined. The compiler is prohibited from removing the check, no > matter if nonnull is used for strlen. If the compiler removed the > check, it's a bug. Again if there is a bug, report it; but if there is > no bugs, you cannot tell people to "fix" it just because "it seems > risky". > > Anyway Glibc (and GCC) are C implementations for real C programs, not > for "allowing people who don't know C to programming in C". Popular libraries, even glibc from time to time suffer crash issues and vulnerabilities. In this thread I was speaking about nonnull removing null pointer checks, I get it that it's nearly impossible to change a POSIX API. For new APIs it would be useful to clarify in the specification that pointers should be checked against the null pointer constant to meet functional safety requirements. Of course we can easily wrap all function calls for libraries we don't maintain if UB can't be changed (I can't imagine anyone is relying upon passing NULL to puts() etc?). getdomainname() and uname() APIs are well specified to check for the null pointer constant. Regards, Jonny