All of lore.kernel.org
 help / color / mirror / Atom feed
From: Paul Eggert <eggert@cs.ucla.edu>
To: Alejandro Colomar <alx@kernel.org>
Cc: gcc@gcc.gnu.org, linux-man@vger.kernel.org, xry111@xry111.site,
	jakub@redhat.com, uecker@tugraz.at, lh_mouse@126.com,
	jwakely.gcc@gmail.com, Richard.Earnshaw@arm.com, sam@gentoo.org,
	ben.boeckel@kitware.com, heiko.eissfeldt@siemens.com,
	dmalcolm@redhat.com, libc-alpha@sourceware.org
Subject: Re: WG14 paper for removing restrict from nptr in strtol(3)
Date: Sun, 7 Jul 2024 12:42:51 +0200	[thread overview]
Message-ID: <37a1f7fa-eac5-440a-a3e9-08125ee7ec81@cs.ucla.edu> (raw)
In-Reply-To: <xjoazfkcloggmceefxusjusbksfslgpdpoph4ixdtp4kbu4kua@vdh73ba7k2zq>

On 7/7/24 03:58, Alejandro Colomar wrote:

> I've incorporated feedback, and here's a new revision, let's call it
> v0.2, of the draft for a WG14 paper.
Although I have not followed the email discussion closely, I read v0.2 
and think that as stated there is little chance that its proposal will 
be accepted.

Fundamentally the proposal is trying to say that there are two styles X 
and Y for declaring strtol and similar functions, and that although both 
styles are correct in some sense, style Y is better than style X. 
However, the advantages of Y are not clearly stated and the advantages 
of style X over Y are not admitted, so the proposal is not making the 
case clearly and fairly.

One other thing: to maximize the chance of a proposal being accepted, 
please tailor it for its expected readership. The C committee is expert 
on ‘restrict’, so don’t try to define ‘restrict’ in your own way. Unless 
merely repeating the language of the standard, any definition given for 
‘restrict’ is likely to cause the committee to quibble with the 
restatement of the standard wording. (It is OK to mention some 
corollaries of the standard definition, so long as the corollaries are 
not immediately obvious.)

Here are some comments about the proposal. At the start these comments 
are detailed; towards the end, as I could see the direction the proposal 
was headed and was convinced it wouldn’t be accepted as stated, the 
comments are less detailed.


"The API may copy"

One normally doesn’t think of the application programming interface as 
copying. Please replace the phrase “the API” with “the caller” or “the 
callee” as appropriate. (Although ‘restrict’ can be used in places other 
than function parameters, I don’t think the proposal is concerned about 
those cases and so it doesn’t need to go into that.)


"To avoid violations of for example C11::6.5.16.1p3,"

Code that violates C11::6.5.16.1p3 will do so regardless of whether 
‘restrict’ is present. I would not mention C11::6.5.16.1p3 as it’s a red 
herring. Fundamentally, ‘restrict’ is not about the consequences of 
caching when one does overlapping moves; it’s about caching in a more 
general sense.


“As long as an object is only accessed via one restricted pointer, other 
restricted pointers are allowed to point to the same object.”

“only accessed” → “accessed only”


“This is less strict than I think it should be, but this proposal 
doesn’t attempt to change that definition.”

I would omit this sentence and all similar sentences. Don’t distract the 
reader with other potential proposals. The proposal as it stands is 
complicated enough.


“return ca > a;”
“return ca > *ap;”

I fail to understand why these examples are present. It’s not simply 
that nobody writes code like that: the examples are not on point. I 
would remove the entire programs containing them, along with the 
sections that discuss them. When writing to the C committee one can 
assume the reader is expert in ‘restrict’, there is no need for examples 
such as these.


“strtol(3) accepts 4 objects via pointer parameters and global variables.”

Omit the “(3)”, here and elsewhere, as the audience is the C standard 
committee.

“accepts” is a strange word to use here: normally one says “accepts” to 
talk about parameters, not global variables. Also, “global variables” is 
not right here. The C standard allows strtol, for example, to read and 
write an internal static cache. (Yes, that would be weird, but it’s 
allowed.) I suggest rephrasing this sentence to talk about accessing, 
not accepting.


“endptr	access(write_only) ... *endptr access(none)”

This is true for glibc, but it’s not necessarily true for all conforming 
strtol implementations. If endptr is non-null, a conforming strtol 
implementation can both read and write *endptr; it can also both read 
and write **endptr. (Although it would need to write before reading, 
reading is allowed.)


“This qualifier helps catch obvious bugs such as strtol(p, p, 0) and 
strtol(&p, &p, 0) .”

No it doesn’t. Ordinary type checking catches those obvious bugs, and 
‘restrict’ provides no additional help there. Please complicate the 
examples to make the point more clearly.


“The caller knows that errno doesn’t alias any of the function arguments.”

Only because all args are declared with ‘restrict’. So if the proposal 
is accepted, the caller doesn’t necessarily know that.


“The callee knows that *endptr is not accessed.”

This is true for glibc, but not necessarily true for every conforming 
strtol implementation.


“It might seem that it’s a problem that the callee doesn’t know if nptr 
can alias errno or not. However, the callee will not write to the latter 
directly until it knows it has failed,”

Again this is true for glibc, but not necessarily true for every 
conforming strtol implementation.

To my mind this is the most serious objection. The current standard 
prohibits calls like strtol((char *) &errno, 0, 0). The proposal would 
relax the standard to allow such calls. In other words, the proposal 
would constrain implementations to support such calls. Why is this 
change worth making? Real-world programs do not make calls like that.


“But nothing prohibits those internal helper functions to specify that 
nptr is restrict and thus distinct from errno.”

Although true, it’s also the case that the C standard does not *require* 
internal helper functions to use ‘restrict’. All that matters is the 
accesses. So I’m not sure what the point of this statement is.


“m = strtol(p, &p, 0); An analyzer more powerful than the current ones
could extend the current -Wrestrict diagnostic to also diagnose this case.”

Why would an analyzer want to do that? This case is a perfectly normal 
thing to do and it has well-defined behavior.


“To prevent triggering diagnostics in a powerful analyzer that would be 
smart enough to diagnose the example function g(), the prototype of 
strtol(3) should be changed to ‘long int strtol(const char *nptr, char 
**restrict endptr, int base);’”

Sorry, but the case has not been made to make any such change to 
strtol’s prototype. On the contrary, what I’m mostly gathering from the 
discussion is that ‘restrict’ can be confusing, which is not news.

n3220 §6.7.4.2 examples 5 through 7 demonstrate that the C committee has 
thought through the points you’re making. (These examples were not 
present in C11.) This may help to explain why the standard specifies 
strtol with ‘restrict’ on both arguments.


  parent reply	other threads:[~2024-07-07 10:52 UTC|newest]

Thread overview: 76+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20240705130249.14116-2-alx@kernel.org>
     [not found] ` <38982a470643f766747b0ca06b27ca859a87b101.camel@xry111.site>
2024-07-05 14:37   ` [PATCH v1] Remove 'restrict' from 'nptr' in strtol(3)-like functions Alejandro Colomar
2024-07-05 15:02     ` Martin Uecker
2024-07-05 15:23       ` Alejandro Colomar
2024-07-05 15:34         ` Martin Uecker
2024-07-05 15:53           ` Alejandro Colomar
2024-07-05 16:01             ` Xi Ruoyao
2024-07-05 16:17               ` Xi Ruoyao
2024-07-05 16:24               ` Jonathan Wakely
2024-07-05 16:30                 ` Martin Uecker
2024-07-05 19:28                   ` Alejandro Colomar
2024-07-05 19:38                     ` Jonathan Wakely
2024-07-05 19:47                       ` Alejandro Colomar
2024-07-05 19:52                         ` Jonathan Wakely
2024-07-05 20:11                           ` Alejandro Colomar
2024-07-05 20:15                           ` Emanuele Torre
2024-07-05 20:31                             ` Ben Boeckel
2024-07-05 20:25                     ` Martin Uecker
2024-07-05 20:28                       ` Jonathan Wakely
2024-07-05 20:41                         ` Alejandro Colomar
2024-07-05 20:55                         ` Alejandro Colomar
2024-07-05 21:39                           ` Jonathan Wakely
2024-07-05 22:02                             ` Alejandro Colomar
2024-07-05 22:04                               ` Alejandro Colomar
2024-07-06  2:24                               ` Xi Ruoyao
2024-07-06  2:39                                 ` Xi Ruoyao
2024-07-06  5:51                                   ` [[gnu::null_terminated_string_arg(1)]] on strtol(1) (was: [PATCH v1] Remove 'restrict' from 'nptr' in strtol(3)-like) functions Alejandro Colomar
2024-07-06  6:10                                 ` [PATCH v1] Remove 'restrict' from 'nptr' in strtol(3)-like functions Alejandro Colomar
2024-07-06  6:11                                   ` Alejandro Colomar
2024-07-05 16:32                 ` Sam James
2024-07-05 16:02             ` Martin Uecker
2024-07-05 16:11             ` Jonathan Wakely
2024-07-05 16:21               ` Richard Earnshaw (lists)
2024-07-05 15:54         ` LIU Hao
2024-07-05 15:55         ` Xi Ruoyao
2024-07-05 16:32           ` Alejandro Colomar
2024-07-05 17:32             ` Alejandro Colomar
2024-07-05 19:41 ` [WG14] Request for document number; strtol restrictness Alejandro Colomar
2024-07-07 15:46   ` Daniel Plakosh
2024-07-09 19:00     ` Alejandro Colomar
2024-07-09 20:04       ` Daniel Plakosh
2024-07-07  1:58 ` WG14 paper for removing restrict from nptr in strtol(3) Alejandro Colomar
2024-07-07  7:15   ` Martin Uecker
2024-07-07 11:07     ` Alejandro Colomar
2024-07-07 12:21       ` Martin Uecker
2024-07-07 13:10         ` Alejandro Colomar
2024-07-07 10:42   ` Paul Eggert [this message]
2024-07-07 12:42     ` Alejandro Colomar
2024-07-07 17:30       ` Paul Eggert
2024-07-07 22:52         ` Alejandro Colomar
2024-07-09 12:09           ` Paul Eggert
2024-07-09 17:36             ` Alejandro Colomar
2024-07-08 14:30       ` David Malcolm
2024-07-08 15:01         ` Alejandro Colomar
2024-07-08 16:05           ` Martin Uecker
2024-07-08 20:17             ` Alejandro Colomar
2024-07-09  5:58               ` Martin Uecker
2024-07-09  9:26                 ` Alejandro Colomar
2024-07-08 22:48           ` David Malcolm
2024-07-09  9:07             ` Alejandro Colomar
2024-07-09  9:18               ` Jakub Jelinek
2024-07-09 10:28                 ` Alejandro Colomar
2024-07-09 11:28                   ` Alejandro Colomar
2024-07-09 22:42   ` n3294 - The restrict function attribute as a replacement of the restrict qualifier Alejandro Colomar
2024-07-26 16:24     ` Joseph Myers
2024-07-26 16:35       ` G. Branden Robinson
2024-07-26 19:53         ` Alejandro Colomar
2024-07-26 18:50       ` Paul Eggert
2024-07-26 20:11       ` Alejandro Colomar
2024-07-26 20:30         ` Joseph Myers
2024-07-26 21:14           ` Alejandro Colomar
2024-07-26 21:22             ` Joseph Myers
2024-07-26 21:49               ` Alejandro Colomar
2024-07-26 22:03                 ` Martin Uecker
2024-07-26 22:26                   ` Alejandro Colomar
2024-07-26 22:59                     ` Martin Uecker
2024-07-27  8:44                       ` Alejandro Colomar

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=37a1f7fa-eac5-440a-a3e9-08125ee7ec81@cs.ucla.edu \
    --to=eggert@cs.ucla.edu \
    --cc=Richard.Earnshaw@arm.com \
    --cc=alx@kernel.org \
    --cc=ben.boeckel@kitware.com \
    --cc=dmalcolm@redhat.com \
    --cc=gcc@gcc.gnu.org \
    --cc=heiko.eissfeldt@siemens.com \
    --cc=jakub@redhat.com \
    --cc=jwakely.gcc@gmail.com \
    --cc=lh_mouse@126.com \
    --cc=libc-alpha@sourceware.org \
    --cc=linux-man@vger.kernel.org \
    --cc=sam@gentoo.org \
    --cc=uecker@tugraz.at \
    --cc=xry111@xry111.site \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.