| Message ID | 20241019014002.3684656-1-siddhesh@sourceware.org |
|---|---|
| State | New |
| Headers | show |
| Series | [RFC] realloc: Make REALLOC_ZERO_BYTES_FREES into a tunable | expand |
Thanks for working on this. As often, the trickiest part is naming and documentation. Some suggestions: On 2024-10-18 18:40, Siddhesh Poyarekar wrote: +* A new tunable glibc.malloc.realloc_zero_bytes_frees ... The existing macro name REALLOC_ZERO_BYTES_FREES is a misnomer, since it doesn't affect whether a successful realloc(P,0) frees P: no matter how the tunable is set, the realloc always frees P and using P afterwards is undefined behavior. As the tunable affects only whether a successful realloc(P,0) returns a null pointer, I suggest a name like glibc.malloc.realloc_0_returns_null instead. > - if (bytes == 0 && oldmem != NULL) > + if (mp_.realloc_zero_bytes_frees && bytes == 0 && oldmem != NULL) Since the case is so rare I suggest putting 'bytes == 0 && oldmem != NULL' first in the condition; that way, the CPU typically might not even need to pull mp into cache. > + ISO C17 says the realloc call has implementation-defined behavior, > + and it might not even free p. */ The same is true for C23, so how about saying "C23" instead of "C17"? > - if (size == 0) > + if (realloc_zero_bytes_frees == 1 && size == 0) Likewise check size before realloc_zero_bytes_frees here. Also, don't bother checking that realloc_zero_bytes_frees exactly equals 1, since it is always either 0 or 1; just check that it is nonzero. > -Otherwise, if @var{newsize} is zero > -@code{realloc} frees the block and returns @code{NULL}. > -Otherwise, if @code{realloc} cannot reallocate the requested size > + > +Otherwise, if @var{newsize} is zero @code{realloc} will by default free > +the block and return @code{NULL}. This behavior can be overridden using > +the @code{glibc.malloc.realloc_zero_bytes_frees} tunable, where instead > +@code{realloc} will then free @var{ptr} and attempt to return a zero > +sized block. > + > +Finally, if @code{realloc} cannot reallocate the requested size Use present rather than future tense for consistency & brevity. Also, give a bit more motivation. Perhaps the new paragraph could be something like this: Otherwise, if @var{newsize} is zero a successful @code{realloc} frees the block; it then returns returns either @code{NULL} or a non-null pointer to a newly allocated size-zero block, depending on the @code{glibc.malloc.realloc_0_returns_null} tunable. The @code{NULL} return is compatible with @theglibc{} versions 2.6 (2007) through 2.40 (2024); the non-null return is compatible with older versions of @theglibc{} and with most non-GNU implementations. Portable code should not assume either behavior. > +@deftp Tunable glibc.malloc.realloc_zero_bytes_frees > + > +By default when the @code{realloc} function is called with the new size as > +@var{0}, it frees the input pointer and returns @var{NULL}. Setting the value > +of this tunable to @var{0} will result in @code{realloc} instead freeing the > +input pointer and attempting to allocate and return a zero sized block. Any > +values other than @var{0} are ignored. Similarly, here I suggest wording like the following: @deftp Tunable glibc.malloc.realloc_0_returns_null If 1, a successful @code{realloc (@var{p}, 0)} returns a null pointer when @var{p} is non-null; if 0, the same call instead returns a non-null pointer to a zero sized block. Although the current default is 1 for compatibility with @theglibc{} versions 2.6 (2007) through 2.40 (2024), the default is planned to be changed to 0 in a future version of @theglibc{} as this will be more compatible with non-GNU systems and will be less likely to break naive programs that assume that when @code{realloc} returns a null pointer then @code{realloc} has failed.
Hi Sid, Paul, Please add some acknowledgement of the reporters and the reports: Reported-by: Alejandro Colomar <alx@kernel.org> Reported-by: наб <nabijaczleweli@nabijaczleweli.xyz> Reported-by: Douglas McIlroy <douglas.mcilroy@dartmouth.edu> Link: <https://github.com/shadow-maint/shadow/pull/1095> Link: <https://nabijaczleweli.xyz/content/blogn_t/017-malloc0.html> Link: <https://inbox.sourceware.org/libc-alpha/t7low35raw7dodsie7umqbnddpm7q2eenkbv5lafesrqrisudn@zqvuq3izem6t/T/#t> On Fri, Oct 18, 2024 at 07:42:32PM GMT, Paul Eggert wrote: > Thanks for working on this. As often, the trickiest part is naming and > documentation. Some suggestions: > > On 2024-10-18 18:40, Siddhesh Poyarekar wrote: > > +* A new tunable glibc.malloc.realloc_zero_bytes_frees ... > > The existing macro name REALLOC_ZERO_BYTES_FREES is a misnomer, since it > doesn't affect whether a successful realloc(P,0) frees P: no matter how the > tunable is set, the realloc always frees P and using P afterwards is > undefined behavior. As the tunable affects only whether a successful > realloc(P,0) returns a null pointer, I suggest a name like > glibc.malloc.realloc_0_returns_null instead. +1 > > - if (bytes == 0 && oldmem != NULL) > > + if (mp_.realloc_zero_bytes_frees && bytes == 0 && oldmem != NULL) > > Since the case is so rare I suggest putting 'bytes == 0 && oldmem != NULL' > first in the condition; that way, the CPU typically might not even need to > pull mp into cache. +1 > > + ISO C17 says the realloc call has implementation-defined behavior, > > + and it might not even free p. */ > > The same is true for C23, so how about saying "C23" instead of "C17"? No. C17 had it implementation-defined. C23 makes it Undefined Behavior. > > - if (size == 0) > > + if (realloc_zero_bytes_frees == 1 && size == 0) > > Likewise check size before realloc_zero_bytes_frees here. > > Also, don't bother checking that realloc_zero_bytes_frees exactly equals 1, > since it is always either 0 or 1; just check that it is nonzero. +1 Just treat it like a boolean: if (size == 0 && realloc_0_returns_null) > > -Otherwise, if @var{newsize} is zero > > -@code{realloc} frees the block and returns @code{NULL}. > > -Otherwise, if @code{realloc} cannot reallocate the requested size > > + > > +Otherwise, if @var{newsize} is zero @code{realloc} will by default free > > +the block and return @code{NULL}. This behavior can be overridden using > > +the @code{glibc.malloc.realloc_zero_bytes_frees} tunable, where instead > > +@code{realloc} will then free @var{ptr} and attempt to return a zero > > +sized block. > > + > > +Finally, if @code{realloc} cannot reallocate the requested size > > Use present rather than future tense for consistency & brevity. Also, give a > bit more motivation. Perhaps the new paragraph could be something like this: > > Otherwise, if @var{newsize} is zero a successful @code{realloc} frees the s/zero/zero,/ > block; it then returns returns either @code{NULL} or a non-null pointer to a > newly allocated size-zero block, depending on the > @code{glibc.malloc.realloc_0_returns_null} tunable. The @code{NULL} return s/\. /. / (Double space after period.) > is compatible with @theglibc{} versions 2.6 (2007) through 2.40 (2024); the I got the version incorrectly. It seems it's much older, and I don't know exactly when, since a toggle was added, and then it was switched, so it didn't all happen at once. I think Sid has a better idea of when that happened. > non-null return is compatible with older versions of @theglibc{} and with > most non-GNU implementations. Portable code should not assume either > behavior. > > > > +@deftp Tunable glibc.malloc.realloc_zero_bytes_frees > > + > > +By default when the @code{realloc} function is called with the new size as > > +@var{0}, it frees the input pointer and returns @var{NULL}. Setting the value > > +of this tunable to @var{0} will result in @code{realloc} instead freeing the > > +input pointer and attempting to allocate and return a zero sized block. Any > > +values other than @var{0} are ignored. > > Similarly, here I suggest wording like the following: > > @deftp Tunable glibc.malloc.realloc_0_returns_null > > If 1, a successful @code{realloc (@var{p}, 0)} returns a null pointer when > @var{p} is non-null; if 0, the same call instead returns a non-null pointer > to a zero sized block. Although the current default is 1 for compatibility > with @theglibc{} versions 2.6 (2007) through 2.40 (2024), the default is > planned to be changed to 0 in a future version of @theglibc{} as this will > be more compatible with non-GNU systems and will be less likely to break > naive programs that assume that when @code{realloc} returns a null pointer > then @code{realloc} has failed. LGTM. Thank you for working on this! Have a lovely day! Alex
* Siddhesh Poyarekar: > Allow users to select this behaviour at runtime, which should allow them > flexibility to have glibc realloc do the same thing that many other > implementations do for realloc (p, 0). The default behaviour remains > unchanged for now. What's the point of this tunable? Applications that depend on the new behavior enabled by the tunable are just buggy today. They can't rely on the tunable setting because tunables are deliberately not part of the glibc interface contract. Please do not make this change without further discussion. Thanks, Florian
On 2024-10-21 05:34, Florian Weimer wrote: > * Siddhesh Poyarekar: > >> Allow users to select this behaviour at runtime, which should allow them >> flexibility to have glibc realloc do the same thing that many other >> implementations do for realloc (p, 0). The default behaviour remains >> unchanged for now. > > What's the point of this tunable? Sorry, I should have referenced the discussion that led to this; it's basically a rehash of an old discussion and given that it keeps resurfacing, I felt that something needed to be done to move the status quo somewhat: https://inbox.sourceware.org/libc-alpha/t7low35raw7dodsie7umqbnddpm7q2eenkbv5lafesrqrisudn@zqvuq3izem6t/T/#m4367eacdcdcf1633c51bfea8cfe95fcb52dc9d6d > Applications that depend on the new behavior enabled by the tunable are > just buggy today. They can't rely on the tunable setting because > tunables are deliberately not part of the glibc interface contract. It's unfortunately not as clear cut since the behaviour is considered implementation defined by C17 and earlier and many implementations (musl, BSD for example) have had the new behaviour, something glibc moved away from in 1999, apparently to comply with C9x. C23 makes the behaviour with realloc(p, 0) undefined instead of leaving it implementation defined, so one stand we could take is to do the same and update GNU tools (and the sanitizer, etc.) to promote/enforce that. This patch takes the "middle" route, introducing BSD-like behaviour with realloc(p, 0). I'd ideally like to do both, since it solves an immediate problem as well as works towards a longer term resolution, eliminating zero-sized allocations. I wonder if a "fortified" realloc (and malloc) would be worthwhile for -std=c23 and later, where a call to malloc or realloc with 0 size aborts... Thanks, Sid
Siddhesh Poyarekar <siddhesh@sourceware.org> writes: > [...] > I wonder if a "fortified" realloc (and malloc) would be worthwhile for > -std=c23 and later, where a call to malloc or realloc with 0 size > aborts... We should get UBSAN instrumentation done first. I think Bruno filed some bugs for it already. It's not a bad idea though. But I admit I don't yet have any sense of scale of fallout. sam
Hi Sam, Sid, On Mon, Oct 21, 2024 at 02:04:23PM GMT, Sam James wrote: > Siddhesh Poyarekar <siddhesh@sourceware.org> writes: > > > [...] > > I wonder if a "fortified" realloc (and malloc) would be worthwhile for > > -std=c23 and later, where a call to malloc or realloc with 0 size > > aborts... > > We should get UBSAN instrumentation done first. I think Bruno filed some > bugs for it already. > > It's not a bad idea though. But I admit I don't yet have any sense of > scale of fallout. I think it's a bad idea. It would unnecessarily break stuff randomly. Why would you force a program to abort when it does perfectly reasonable stuff? Allocating 0 bytes is something that fit naturally in a generic algorithm, for example for storing an arbitrary number of things where that nuimber may very well be 0, and you don't want to handle 0 specially. Just fix the bug. Don't make it worse, please. Have a lovely day! Alex > > sam
Hi Sid, On Mon, Oct 21, 2024 at 08:51:40AM GMT, Siddhesh Poyarekar wrote: > On 2024-10-21 05:34, Florian Weimer wrote: > > * Siddhesh Poyarekar: > > > > > Allow users to select this behaviour at runtime, which should allow them > > > flexibility to have glibc realloc do the same thing that many other > > > implementations do for realloc (p, 0). The default behaviour remains > > > unchanged for now. > > > > What's the point of this tunable? > > Sorry, I should have referenced the discussion that led to this; it's > basically a rehash of an old discussion and given that it keeps resurfacing, > I felt that something needed to be done to move the status quo somewhat: > > https://inbox.sourceware.org/libc-alpha/t7low35raw7dodsie7umqbnddpm7q2eenkbv5lafesrqrisudn@zqvuq3izem6t/T/#m4367eacdcdcf1633c51bfea8cfe95fcb52dc9d6d > > > Applications that depend on the new behavior enabled by the tunable are > > just buggy today. They can't rely on the tunable setting because > > tunables are deliberately not part of the glibc interface contract. > > It's unfortunately not as clear cut since the behaviour is considered > implementation defined by C17 and earlier and many implementations (musl, > BSD for example) have had the new behaviour, something glibc moved away from Not new. Rather old. The BSDs' behavior is the original from Unix. It's C89 that innovated, and C17 attempted to revert, but did it in an inconsistent way that left the language of the standard so broken, that C23 had to give up and make it undefined. The BSDs never changed their behavior significantly from Unix V6 and V7. > in 1999, apparently to comply with C9x. > > C23 makes the behaviour with realloc(p, 0) undefined instead of leaving it > implementation defined, so one stand we could take is to do the same and > update GNU tools (and the sanitizer, etc.) to promote/enforce that. This > patch takes the "middle" route, introducing BSD-like behaviour with > realloc(p, 0). > > I'd ideally like to do both, since it solves an immediate problem as well as > works towards a longer term resolution, eliminating zero-sized allocations. Please do NOT eliminate zero-size allocations. That's what ANSI C tried to do in the first place, and they broke realloc(p,0) in their attempt. We need 0-size allocations. If you think you don't need them yourself, don't use them. I do need them. Do NOT touch the behavior of malloc(0). Do NOT touch the behavior of realloc(NULL, 0). Fix the behavior of realloc(p, 0) to be just what it should have always been. Thanks. Have a lovely day! Alex > > I wonder if a "fortified" realloc (and malloc) would be worthwhile for > -std=c23 and later, where a call to malloc or realloc with 0 size aborts... N. O. > > Thanks, > Sid
Alejandro Colomar <alx@kernel.org> writes: > Hi Sam, Sid, > > On Mon, Oct 21, 2024 at 02:04:23PM GMT, Sam James wrote: >> Siddhesh Poyarekar <siddhesh@sourceware.org> writes: >> >> > [...] >> > I wonder if a "fortified" realloc (and malloc) would be worthwhile for >> > -std=c23 and later, where a call to malloc or realloc with 0 size >> > aborts... >> >> We should get UBSAN instrumentation done first. I think Bruno filed some >> bugs for it already. >> >> It's not a bad idea though. But I admit I don't yet have any sense of >> scale of fallout. > > I think it's a bad idea. It would unnecessarily break stuff randomly. > Why would you force a program to abort when it does perfectly reasonable > stuff? Allocating 0 bytes is something that fit naturally in a > generic algorithm, for example for storing an arbitrary number of things > where that nuimber may very well be 0, and you don't want to handle 0 > specially. > I think a way to easily test at runtime for the behaviour we're discussing without running everything under Valgrind is useful, using frameworks (fortify & UBSAN) we're familiar with. Doesn't mean either would have to be in the usual levels or anything. > Just fix the bug. Don't make it worse, please. > > Have a lovely day! > Alex > >> >> sam
On Mon, 21 Oct 2024, Alejandro Colomar wrote: > Not new. Rather old. The BSDs' behavior is the original from Unix. > It's C89 that innovated, and C17 attempted to revert, but did it in an > inconsistent way that left the language of the standard so broken, that > C23 had to give up and make it undefined. C17 made it explicitly obsolescent (see 7.31.12 paragraph 2, "Invoking realloc with a size argument equal to zero is an obsolescent feature."). C23 following up by removing the obsolescent feature was simply the natural followup to that obsolescence (though quicker than the removal of obsolescent features often is). That's nothing to do with "inconsistent", "broken" or "give up".
* Siddhesh Poyarekar: > On 2024-10-21 05:34, Florian Weimer wrote: >> * Siddhesh Poyarekar: >> >>> Allow users to select this behaviour at runtime, which should allow them >>> flexibility to have glibc realloc do the same thing that many other >>> implementations do for realloc (p, 0). The default behaviour remains >>> unchanged for now. >> What's the point of this tunable? > > Sorry, I should have referenced the discussion that led to this; it's > basically a rehash of an old discussion and given that it keeps > resurfacing, I felt that something needed to be done to move the > status quo somewhat: > > https://inbox.sourceware.org/libc-alpha/t7low35raw7dodsie7umqbnddpm7q2eenkbv5lafesrqrisudn@zqvuq3izem6t/T/#m4367eacdcdcf1633c51bfea8cfe95fcb52dc9d6d I was aware of the discussion, but I don't see how a tunable is an improvement here, sorry. >> Applications that depend on the new behavior enabled by the tunable are >> just buggy today. They can't rely on the tunable setting because >> tunables are deliberately not part of the glibc interface contract. > > It's unfortunately not as clear cut since the behaviour is considered > implementation defined by C17 and earlier and many implementations > (musl, BSD for example) have had the new behaviour, something glibc > moved away from in 1999, apparently to comply with C9x. I don't see any ambiguity here: if you build for GNU/Linux and the application is not compatible with the current glibc behavior, this is an application bug. As you point out, it's clearly not a conformance issue anymore. The fact that the application might possibly work if compiled for and running on some BSD variant is not particularly compelling. There must be dozens of issues where BSD and GNU/Linux behavior differs (and some with regards to musl, too). The fundamental problem here is that GCC and other compilers have long decided to support zero-sized objects, something that is naturally part of C. And this eventually led to somewhat conflicting expectations about malloc and realloc behavior. Anyway, to make the tunable truly effective, distributions would have to port system libraries to be compatible with both behaviors. Replacement mallocs would also have to react to the glibc tunable, and we currently do not provide an API for that. I don't think this is worth the effort. UBSAN and others flagging this, sure, but porting the system to the new behavior does not seem like a useful activity. Thanks, Florian
Hi Florian, On Mon, Oct 21, 2024 at 11:02:33PM GMT, Florian Weimer wrote: > I don't see any ambiguity here: if you build for GNU/Linux and the > application is not compatible with the current glibc behavior, this is > an application bug. As you point out, it's clearly not a conformance > issue anymore. The fact that the application might possibly work if > compiled for and running on some BSD variant is not particularly > compelling. There must be dozens of issues where BSD and GNU/Linux > behavior differs (and some with regards to musl, too). I'm not sure code written for glibc would have bugs in the BSD. At least code that is bug-free in glibc. Unless the code has strong guarantees that in all realloc(p,0) calls p is non-NULL, realloc() might actually return a zero-size region. Thus, good glibc code must not make the assumption that it will return NULL. Which probably means there's a decent amount of code there that has memory leaks due to this, since I don't think code is well written in this regard. alx@debian:~/tmp/gcc/realloc$ cat brain_dmg.c #include <errno.h> #include <stdio.h> #include <stdlib.h> int main(void) { void *p, *q; errno = 0; p = malloc(0); // In a sane system, p==NULL is enough of a check. if (p == NULL && errno == ENOMEM) // In glibc we can simplify exit(EXIT_FAILURE); printf("malloc(0): %p\n", p); printf(" errno: %d\n", errno); q = NULL; for (int i = 0; i < 8; i++) { q = realloc(q, 0); // In a sane system, p==NULL is enough of a check. if (q == NULL && errno == ENOMEM) exit(EXIT_FAILURE); printf("realloc(q, 0): %p\n", q); printf(" errno: %d\n", errno); } free(q); free(p); } alx@debian:~/tmp/gcc/realloc$ cc -Wall -Wextra brain_dmg.c alx@debian:~/tmp/gcc/realloc$ ./a.out malloc(0): 0x55f3e910d2a0 errno: 0 realloc(q, 0): 0x55f3e910d6d0 errno: 0 realloc(q, 0): (nil) errno: 0 realloc(q, 0): 0x55f3e910d6d0 errno: 0 realloc(q, 0): (nil) errno: 0 realloc(q, 0): 0x55f3e910d6d0 errno: 0 realloc(q, 0): (nil) errno: 0 realloc(q, 0): 0x55f3e910d6d0 errno: 0 realloc(q, 0): (nil) errno: 0 You see the alternating pattern. If one wants to make sure that the pointer is freed and nothing is leaked, the only way is free(p). Or you could check if the pointer is null before-hand, which would make it rather useless to call realloc. I don't think many programmers check errno after checking for NULL to detect realloc(3) errors (or malloc(3), if they attempt to be portable to AIX and other such systems). > The fundamental problem here is that GCC and other compilers have long > decided to support zero-sized objects, something that is naturally part > of C. And this eventually led to somewhat conflicting expectations > about malloc and realloc behavior. This is not entirely correct. This support comes from Unix V6 and V7, and GCC only continued tradition. What led to the conflicting expectations was C89 trying to kill 0-size objects by decree. Have a lovely night! Alex
On 2024-10-21 14:02, Florian Weimer wrote: > As you point out, it's clearly not a conformance > issue anymore. Although that's true for the main point of this discussion, there is definitely a conformance issue in shouting distance: when P is non-null, glibc's realloc(P, 0) doesn't set errno as POSIX requires. I'm not planning to file a bug report about this, because in my experience apps don't care about this misfeature of POSIX and therefore don't care whether glibc conforms. More generally, in my experience the few apps that call realloc(P, 0) and check the return value generally expect the BSD behavior even when running on glibc. Yes these apps are "wrong" but this is mainly why people are itching to make the proposed change. For this reason I expect the change will make glibc-using software a bit more reliable, not a bit less. If I didn't think that I wouldn't advocate the change. The main practical conflict here, as I understand it, is that in MS-Windows realloc(P, 0) returns null, whereas in the overwhelming Unix/BSD tradition it returns non-null. Evidently the returns-null camp won a standards committee battle decades ago, but the returns-nonnull camp ignored that part of the standard for decades and finally now, as a grudging compromise, C23 says it's undefined. In this conflict, our natural home is the API of BSD, not of MS-Windows. It's easier to port between BSD and GNU than between MS-Windows and GNU, and in the long run we'll be better off if we keep it that way. If tunables are not the best way to make progress in this area, then it'd be helpful to find a better way.
On Okt 18 2024, Siddhesh Poyarekar wrote: > Allow users to select this behaviour at runtime, which should allow them > flexibility to have glibc realloc do the same thing that many other > implementations do for realloc (p, 0). The default behaviour remains > unchanged for now. That doesn't make sense. The required behaviour of realloc (p, 0) is a static, compile-time property of the program, so it needs to be controlled at compile time.
On 2024-10-21 17:02, Florian Weimer wrote: >> https://inbox.sourceware.org/libc-alpha/t7low35raw7dodsie7umqbnddpm7q2eenkbv5lafesrqrisudn@zqvuq3izem6t/T/#m4367eacdcdcf1633c51bfea8cfe95fcb52dc9d6d > > I was aware of the discussion, but I don't see how a tunable is an > improvement here, sorry. It's an attempt at a compromise, since the conversation isn't really going anywhere otherwise. It is a net improvement because the glibc implementation defined behaviour can then be compliant to the BSD behaviour as well. Of course, there's the argument that C23 makes realloc(p, 0) undefined and adding a tunable is just spinning the wheels in that context. >>> Applications that depend on the new behavior enabled by the tunable are >>> just buggy today. They can't rely on the tunable setting because >>> tunables are deliberately not part of the glibc interface contract. >> >> It's unfortunately not as clear cut since the behaviour is considered >> implementation defined by C17 and earlier and many implementations >> (musl, BSD for example) have had the new behaviour, something glibc >> moved away from in 1999, apparently to comply with C9x. > > I don't see any ambiguity here: if you build for GNU/Linux and the > application is not compatible with the current glibc behavior, this is > an application bug. As you point out, it's clearly not a conformance > issue anymore. The fact that the application might possibly work if > compiled for and running on some BSD variant is not particularly > compelling. There must be dozens of issues where BSD and GNU/Linux > behavior differs (and some with regards to musl, too). > > The fundamental problem here is that GCC and other compilers have long > decided to support zero-sized objects, something that is naturally part > of C. And this eventually led to somewhat conflicting expectations > about malloc and realloc behavior. > > Anyway, to make the tunable truly effective, distributions would have to > port system libraries to be compatible with both behaviors. Replacement > mallocs would also have to react to the glibc tunable, and we currently > do not provide an API for that. I don't think this is worth the effort. > UBSAN and others flagging this, sure, but porting the system to the new > behavior does not seem like a useful activity. I agree that ubsan, etc. improvements would be more worthwhile. Thanks, Sid
On 2024-10-22 04:26, Andreas Schwab wrote: > On Okt 18 2024, Siddhesh Poyarekar wrote: > >> Allow users to select this behaviour at runtime, which should allow them >> flexibility to have glibc realloc do the same thing that many other >> implementations do for realloc (p, 0). The default behaviour remains >> unchanged for now. > > That doesn't make sense. The required behaviour of realloc (p, 0) is a > static, compile-time property of the program, so it needs to be > controlled at compile time. Is there a way to do that without having, e.g., a symbol linked into the program that realloc checks to decide which way to go? Thanks, Sid
I doubt that a return to the original semantics of realloc(p,0) would cause harm. It is hard to imagine a scenario in which one tests for a null return from realloc(p,0) not because of caution about the definition of realloc, but because the prorgram would fail if a zero-length block were returned. In the absence of such a threat of failure, programs will not be hurt by a silent change that returns non-null. Doug On Tue, Oct 22, 2024 at 6:45 AM Siddhesh Poyarekar <siddhesh@sourceware.org> wrote: > > On 2024-10-22 04:26, Andreas Schwab wrote: > > On Okt 18 2024, Siddhesh Poyarekar wrote: > > > >> Allow users to select this behaviour at runtime, which should allow them > >> flexibility to have glibc realloc do the same thing that many other > >> implementations do for realloc (p, 0). The default behaviour remains > >> unchanged for now. > > > > That doesn't make sense. The required behaviour of realloc (p, 0) is a > > static, compile-time property of the program, so it needs to be > > controlled at compile time. > > Is there a way to do that without having, e.g., a symbol linked into the > program that realloc checks to decide which way to go? > > Thanks, > Sid
On 2024-10-22 03:45, Siddhesh Poyarekar wrote: > Is there a way to do that without having, e.g., a symbol linked into the > program that realloc checks to decide which way to go? We could introduce a compile-time feature test macro, e.g., -D_REALLOC_PTR_0_NULL=1 for current behavior, -D_REALLOC_PTR_0_NULL=0 for BSD behavior. The default can start off being 1, and we can change it to 0 later. This approach would overcome Andreas's objection. I'm not a fan of compile-time feature test macros as we'd be better off simply changing the behavior as we did last time without significant ill effect. That being said, such a macro would be better than doing nothing. Gnulib-using programs could simply set _REALLOC_PTR_0_NULL, as they already set _GNU_SOURCE etc.
On 2024-10-22 11:38, Paul Eggert wrote: > On 2024-10-22 03:45, Siddhesh Poyarekar wrote: >> Is there a way to do that without having, e.g., a symbol linked into >> the program that realloc checks to decide which way to go? > > We could introduce a compile-time feature test macro, e.g., > -D_REALLOC_PTR_0_NULL=1 for current behavior, -D_REALLOC_PTR_0_NULL=0 > for BSD behavior. The default can start off being 1, and we can change > it to 0 later. This approach would overcome Andreas's objection. > > I'm not a fan of compile-time feature test macros as we'd be better off > simply changing the behavior as we did last time without significant ill > effect. That being said, such a macro would be better than doing > nothing. Gnulib-using programs could simply set _REALLOC_PTR_0_NULL, as > they already set _GNU_SOURCE etc. That'll still need a symbol in the program at runtime, won't it? There needs to be some way for the realloc (in libc) to decide which path to go down. Or are you suggesting *glibc* build time configuration? Thanks, Sid
On 2024-10-22 08:52, Siddhesh Poyarekar wrote: > That'll still need a symbol in the program at runtime, won't it? There > needs to be some way for the realloc (in libc) to decide which path to > go down. Yes, it'd need a new runtime symbol, like the other feature-test macros. Or rather two symbols, as reallocarray is also affected.
On 2024-10-22 11:56, Paul Eggert wrote: > On 2024-10-22 08:52, Siddhesh Poyarekar wrote: >> That'll still need a symbol in the program at runtime, won't it? >> There needs to be some way for the realloc (in libc) to decide which >> path to go down. > > Yes, it'd need a new runtime symbol, like the other feature-test macros. > Or rather two symbols, as reallocarray is also affected. > Yeah, that's the symbol I was looking to avoid. reallocarray just calls realloc, so I don't think it needs to be handled separately and in fact shouldn't, because it calls potentially interposed implementations and it shouldn't overshadow the interposed implementation's behaviour. Thanks, Sid
On Tue, 22 Oct 2024, Paul Eggert wrote: > Although that's true for the main point of this discussion, there is > definitely a conformance issue in shouting distance: when P is non-null, > glibc's realloc(P, 0) doesn't set errno as POSIX requires. My understanding of the intent of POSIX (if not the normative wording) is given by the APPLICATION USAGE: "The ISO C standard makes it implementation-defined whether a call to realloc(p, 0) frees the space pointed to by p if it returns a null pointer because memory for the new object was not allocated. POSIX.1 instead requires that implementations set errno if a null pointer is returned and the space has not been freed, and POSIX applications should only free the space if errno was changed.". That is, since glibc frees the space when returning a null pointer from realloc with size 0, it should not change errno in that case.
On Tue, 22 Oct 2024, Paul Eggert wrote: > On 2024-10-22 08:52, Siddhesh Poyarekar wrote: > > That'll still need a symbol in the program at runtime, won't it? There > > needs to be some way for the realloc (in libc) to decide which path to go > > down. > > Yes, it'd need a new runtime symbol, like the other feature-test macros. Or > rather two symbols, as reallocarray is also affected. You'd need two symbols for realloc and two for reallocarray simply because, if you add a way to configure this, different parts of a program (e.g. shared libraries it uses) might want different semantics, so it ought to be compile-time configuration for each object, not link-time configuration or glibc-build-time configuration. (Though I'm not convinced of the utility of adding variants for this, as opposed to _FORTIFY_SOURCE checks for programs using this undefined behavior, which would naturally be per-object based on the _FORTIFY_SOURCE setting when it was built.)
On 2024-10-22 09:52, Joseph Myers wrote: > it ought to be compile-time configuration for each object, not link-time > configuration or glibc-build-time configuration Yes, that's what I was suggesting. On 2024-10-22 09:01, Siddhesh Poyarekar wrote: > eallocarray just calls realloc, so I don't think it needs to be handled separately reallocarray would need separate handling if, as Joseph suggested, different compilation units want different semantics. On 2024-10-22 09:38, Joseph Myers wrote: > My understanding of the intent of POSIX (if not the normative wording) is > given by the APPLICATION USAGE: Although that's a plausible reading of the intent, POSIX's normative wording directly contradicts it. And AIX 7.3 realloc(p,0) (the only one I know of besides GNU and MS-Windows that returns a null pointer) conforms to the normative wording: it sets errno to EINVAL, which breaks the plausible intent entirely. What a mess, huh? Things would be simpler if we simply reverted Ulrich's 1999 change and went back to the BSD semantics; this would cure more trouble than it'd cause.
On 2024-10-22 14:59, Paul Eggert wrote: > On 2024-10-22 09:52, Joseph Myers wrote: >> it ought to be compile-time configuration for each object, not link-time >> configuration or glibc-build-time configuration > > Yes, that's what I was suggesting. > > > On 2024-10-22 09:01, Siddhesh Poyarekar wrote: > >> eallocarray just calls realloc, so I don't think it needs to be >> handled separately > > reallocarray would need separate handling if, as Joseph suggested, > different compilation units want different semantics. > That's too much machinery for what's essentially undefined behaviour in C23 :/ > On 2024-10-22 09:38, Joseph Myers wrote: >> My understanding of the intent of POSIX (if not the normative wording) >> is given by the APPLICATION USAGE: > > Although that's a plausible reading of the intent, POSIX's normative > wording directly contradicts it. And AIX 7.3 realloc(p,0) (the only one > I know of besides GNU and MS-Windows that returns a null pointer) > conforms to the normative wording: it sets errno to EINVAL, which breaks > the plausible intent entirely. > > What a mess, huh? Things would be simpler if we simply reverted Ulrich's > 1999 change and went back to the BSD semantics; this would cure more > trouble than it'd cause. > Tiny nit, it wasn't Ulrich's change: 1999-04-28 Andreas Jaeger <aj@arthur.rhein-neckar.de> * malloc/malloc.c (REALLOC_ZERO_BYTES_FREES): Define it to follow ISO C9x and Unix98.
Hi Sid, Paul, On Tue, Oct 22, 2024 at 03:05:49PM GMT, Siddhesh Poyarekar wrote: > > > On 2024-10-22 14:59, Paul Eggert wrote: > > On 2024-10-22 09:52, Joseph Myers wrote: > > > it ought to be compile-time configuration for each object, not link-time > > > configuration or glibc-build-time configuration > > > > Yes, that's what I was suggesting. > > > > > > On 2024-10-22 09:01, Siddhesh Poyarekar wrote: > > > > > eallocarray just calls realloc, so I don't think it needs to be > > > handled separately > > > > reallocarray would need separate handling if, as Joseph suggested, > > different compilation units want different semantics. > > > > That's too much machinery for what's essentially undefined behaviour in C23 > :/ I plan to make it defined again in C2y. I'll prepare a paper for standardizing on the BSD/Unix V7 semantics, but I'll do that after glibc has changed. Bionic changing would also be an interesting thing. But yes, I think it's too much machinery. In reality you don't need to make anything configurable. Just changing the behavior tonight wouldn't break stuff. At most a few 0B leaks here and there. Most code will in fact be more stable, since people are already testing for p==NULL after realloc, not p==NULL&&errno, so most programs are actually not well written for glibc (see my example program in the response to Florian). And it is perfectly possible to write code that works well under both the new and the old semantics, so it doesn't make sense to provide a feature test macro. Just tell people to program for both behaviors, by testing (p == NULL && errno) for failure. That errno check will become dead code under the new behavior, and in a decade when the new behavior is everywhere it will be possible to remove it. I think I wouldn't even make a new symbol, since old binaries will probably benefit from the bugfix and get more stability for free. But if you want to be cautiout, a new symbol would be the only thing I'd do. Have a lovely day! Alex
On 2024-10-24 08:50, Alejandro Colomar wrote: > I plan to make it defined again in C2y. I'll prepare a paper for > standardizing on the BSD/Unix V7 semantics, but I'll do that after glibc > has changed. Bionic changing would also be an interesting thing. > > > But yes, I think it's too much machinery. In reality you don't need to > make anything configurable. Just changing the behavior tonight wouldn't > break stuff. At most a few 0B leaks here and there. Most code will in > fact be more stable, since people are already testing for p==NULL after > realloc, not p==NULL&&errno, so most programs are actually not well > written for glibc (see my example program in the response to Florian). AFAICT, there are strong sustained objections to unconditionally changing behaviour, so that is likely not an option anymore. Sid
Hi Sid, On Thu, Oct 24, 2024 at 08:57:48AM GMT, Siddhesh Poyarekar wrote: > On 2024-10-24 08:50, Alejandro Colomar wrote: > > I plan to make it defined again in C2y. I'll prepare a paper for > > standardizing on the BSD/Unix V7 semantics, but I'll do that after glibc > > has changed. Bionic changing would also be an interesting thing. > > > > > > But yes, I think it's too much machinery. In reality you don't need to > > make anything configurable. Just changing the behavior tonight wouldn't > > break stuff. At most a few 0B leaks here and there. Most code will in > > fact be more stable, since people are already testing for p==NULL after > > realloc, not p==NULL&&errno, so most programs are actually not well > > written for glibc (see my example program in the response to Florian). > > AFAICT, there are strong sustained objections to unconditionally changing > behaviour, so that is likely not an option anymore. The strong sustained objections to changing the standard all have one condition: Change the implementations first. This is for example the case of Robert C. Seacord and JeanHeyd Meneide. Once we fix the implementations, the standard will be trivial to change. The only libc implementations that have realloc(p,0) and realloc(NULL,0) be inconsistent are glibc, Win32, and Bionic, AFAIK. Then there are also some interposed libraries that behave like glibc for compatiblity, but I expect them to follow glibc. I'm already in talks with Bionic. Once majority of implementations are fixed, I think we can force Win32 to just follow; in any case, let's ignore Win32 for now. So, this is still an option. Regarding the objections to changing glibc, I think it's mainly you. I have seen a lot of FUD about introducing security vulnerabilities in the form of double frees, but nobody has shown a single piece of code that would do that. So I'd say we're in the process of convincing you that the change is safe. It would help if you'd show a minimal reproducible example program in which you expect to introduce a vulnerability after this change. Have a lovely day! Alex > > Sid
On 2024-10-24 11:15, Alejandro Colomar wrote: >> AFAICT, there are strong sustained objections to unconditionally changing >> behaviour, so that is likely not an option anymore. > > The strong sustained objections to changing the standard all have one > condition: > > Change the implementations first. > > This is for example the case of Robert C. Seacord and JeanHeyd Meneide. > Once we fix the implementations, the standard will be trivial to change. > The only libc implementations that have realloc(p,0) and realloc(NULL,0) > be inconsistent are glibc, Win32, and Bionic, AFAIK. Then there are > also some interposed libraries that behave like glibc for compatiblity, > but I expect them to follow glibc. I'm already in talks with Bionic. > Once majority of implementations are fixed, I think we can force Win32 > to just follow; in any case, let's ignore Win32 for now. > > So, this is still an option. > > > Regarding the objections to changing glibc, I think it's mainly you. Nope, please read the threads carefully; I actually said that I won't sustain an objection if I'm the only one holding that opinion. Among the glibc maintainers involved in the conversation, only Paul aligns with the resolution of changing realloc to behave like BSD. Sid
Hi Sid, On Thu, Oct 24, 2024 at 11:24:39AM GMT, Siddhesh Poyarekar wrote: > > Regarding the objections to changing glibc, I think it's mainly you. > > Nope, please read the threads carefully; I actually said that I won't > sustain an objection if I'm the only one holding that opinion. Yup, I know. That's okay. > Among the glibc maintainers involved in the conversation, only Paul aligns > with the resolution of changing realloc to behave like BSD. I didn't say the others fully support that either. I just don't see a strong sustained opposition. I only see some fear, which is natural. To remove the fear, we need to make sure what's exactly what might break and how. In others, I see doubts about specific proposals, but not necessarily about the overall change in behavior. Cheers, Alex
* Siddhesh Poyarekar: > On 2024-10-24 11:15, Alejandro Colomar wrote: >>> AFAICT, there are strong sustained objections to unconditionally changing >>> behaviour, so that is likely not an option anymore. >> The strong sustained objections to changing the standard all have >> one >> condition: >> Change the implementations first. >> This is for example the case of Robert C. Seacord and JeanHeyd >> Meneide. >> Once we fix the implementations, the standard will be trivial to change. >> The only libc implementations that have realloc(p,0) and realloc(NULL,0) >> be inconsistent are glibc, Win32, and Bionic, AFAIK. Then there are >> also some interposed libraries that behave like glibc for compatiblity, >> but I expect them to follow glibc. I'm already in talks with Bionic. >> Once majority of implementations are fixed, I think we can force Win32 >> to just follow; in any case, let's ignore Win32 for now. >> So, this is still an option. >> Regarding the objections to changing glibc, I think it's mainly you. > > Nope, please read the threads carefully; I actually said that I won't > sustain an objection if I'm the only one holding that opinion. I'm still objecting, I don't think this change is valuable. I'm open to looking at this again once the C standard fully specifies the behavior of zero-sized objects, the return value of malloc (0), and so on. Getting aligned behavior between implementations on these fundamental interfaces seems quite valuable to programmers. But addressing one zero-object-size case and leaving all the others as unclear as before doesn't seem to be useful, especially given that a ffuture standard may require different behavior anyway. A piece-by-piece transition to the newly required behaviors is also more onerous on programmers than one well-defined transition in a single glibc release. Thanks, Florian
On Thu, 24 Oct 2024, Alejandro Colomar wrote: > The strong sustained objections to changing the standard all have one > condition: > > Change the implementations first. I think a fundamental objection for both standard and implementation changes is: we discussed this at length more than once, we settled it (first marking the feature obsolescent in C17, then making it undefined behavior in C23), there is no significant new evidence that the basis for those decisions was wrong or that the decisions have caused problems that could justify spending time on revisiting the settled issue so soon after settling it.
CC += Martin Uecker On Thu, Oct 24, 2024 at 05:36:49PM GMT, Florian Weimer wrote: > * Siddhesh Poyarekar: > > > On 2024-10-24 11:15, Alejandro Colomar wrote: > >>> AFAICT, there are strong sustained objections to unconditionally changing > >>> behaviour, so that is likely not an option anymore. > >> The strong sustained objections to changing the standard all have > >> one > >> condition: > >> Change the implementations first. > >> This is for example the case of Robert C. Seacord and JeanHeyd > >> Meneide. > >> Once we fix the implementations, the standard will be trivial to change. > >> The only libc implementations that have realloc(p,0) and realloc(NULL,0) > >> be inconsistent are glibc, Win32, and Bionic, AFAIK. Then there are > >> also some interposed libraries that behave like glibc for compatiblity, > >> but I expect them to follow glibc. I'm already in talks with Bionic. > >> Once majority of implementations are fixed, I think we can force Win32 > >> to just follow; in any case, let's ignore Win32 for now. > >> So, this is still an option. > >> Regarding the objections to changing glibc, I think it's mainly you. > > > > Nope, please read the threads carefully; I actually said that I won't > > sustain an objection if I'm the only one holding that opinion. > > I'm still objecting, I don't think this change is valuable. Objecting is one thing, which is okay. But that doesn't mean you won't change your mind. Only that we need to talk about it. Continue reading below. > I'm open to looking at this again once the C standard fully specifies > the behavior of zero-sized objects, the return value of malloc (0), and > so on. That's a chicken-and-egg problem. The standard won't do it if implementations don't do it first. > Getting aligned behavior between implementations on these > fundamental interfaces seems quite valuable to programmers. Indeed. > But > addressing one zero-object-size case and leaving all the others as > unclear as before doesn't seem to be useful, especially given that a > ffuture standard may require different behavior anyway. I assume you mean 0-sized arrays as the other zero-sized-object that you refer to. Martin and I are working on improving the support for 0-sized arrays in GCC. That's a problem for implementing the __countof__ operator to work on array parameters. So there's already work to improve the GCC support on 0-sized arrays before being able to do that. > A > piece-by-piece transition to the newly required behaviors is also more > onerous on programmers than one well-defined transition in a single > glibc release. Here's my plan, step by step: A.1) Fix glibc and/or Bionic to have realloc(p,0) be consistent with realloc(NULL,0). A.2) Ask POSIX and ISO to tighten the specification of realloc(p,0) to be consistent with realloc(NULL,0). This will mean that Win32 will be forced by decree of ISO to follow the rest. B.1) Ask POSIX to tighten the specification of malloc(0) to return non-NULL on success. This will be easy, since the only POSIX systems that return NULL from malloc(0) are dying. The BSDs, macOS, and GNU, all of them return non-NULL. B.2) Once POSIX has accepted A.2 and B.1, we can ask ISO to tighten malloc(0) to return non-NULL. This might force some embedded platforms to conform. Other dying systems such as AIX will also need to either conform, or assume they're dead. I think all major implementations alive already work like that. C.1) Make GCC have [n] have a stronger meaning for array parameters. That is, not ignore the number of elements of array parameters, and instead make it mean the same as [static n], except that [n] doesn't require non-null-ness nor require at least 1 element. C.1) Add support for array parameters in __countof__ in GCC. As a natural consequence, __countof__ will be able to return 0 for array parameters whose number of elements is 0 at run time. C.3) Ask ISO to make [n] be meaningful in array parameters, following C.1 from GCC. C.4) Add support for array parameters in __countof__ in ISO C, following C.2 from GCC. C.5) Add official support for 0-sized arrays in ISO. This would be only bureaucracy, since actual support would already have been introduced in C3. But we can't have it all at once, and we can't ask ISO C to innovate on this, but rather follow implementations that already work. If there's not an implementation that works, we can't ask them to come up with a good one. How does that sound? Have a lovely day! Alex
CC += JeanHeyd Hi Joseph, On Thu, Oct 24, 2024 at 03:44:05PM GMT, Joseph Myers wrote: > On Thu, 24 Oct 2024, Alejandro Colomar wrote: > > > The strong sustained objections to changing the standard all have one > > condition: > > > > Change the implementations first. > > I think a fundamental objection for both standard and implementation > changes is: we discussed this at length more than once, we settled it You discussed this in a closed committee, without feedback from users. That's not how it should work. Also, in those discussions there was FUD about vulnerabilities (double-free) being a necessary result of this change. I haven't been shown a Minimal Reproducible Example of that, and strongly doubt, just like Doug and Paul, that it is possible. > (first marking the feature obsolescent in C17, then making it undefined > behavior in C23), there is no significant new evidence that the basis for > those decisions was wrong I've requested JeanHeyd to provide one, but you're invited to do so yourself. Otherwise, I claim that the decission that WG14 took is based on FUD, and thus wrong. Here are the requirements of the program: It must run on any system of your like (except AIX; that's a broken one), and behave well. The same program must have a double free on the BSD/Unix V7 behavior in a circumstance that wouldn't trigger on the other system. Bonus points if the system under which the program works fine is glibc. That should be easy if the decission by WG14 was based on true facts. > or that the decisions have caused problems that > could justify spending time on revisiting the settled issue so soon after > settling it. The internet is on fire about ISO C23 making realloc(p,0) UB. I don't know if you've searched about it, but I've personally received messages from several people despairing about C23, and mentioning that as one of the most obvious wrongs that ISO has done in C23. Have a lovely day! Alex
On Thu, 24 Oct 2024, Alejandro Colomar wrote:
> The internet is on fire about ISO C23 making realloc(p,0) UB. I don't
This sort of exaggerated claim seems like a great way to lose credibility
and encourage people to consider all proposals on this topic as noise to
summarily reject.
I think it would be a bad idea to relitigate the obsolescence and
subsequent removal of realloc(p, 0), just as it's a bad idea to relitigate
the choice of name for _Lengthof.
On Thu, 24 Oct 2024, Alejandro Colomar wrote: > know if you've searched about it, but I've personally received messages > from several people despairing about C23, and mentioning that as one of > the most obvious wrongs that ISO has done in C23. I suggest that people generating noise rather than light are good examples of people to ignore. This really isn't a matter for "despair" or talking about "most obvious wrongs" (although such terminology is still to be preferred to certain offensive language seen earlier in this discussion). It's rather a matter for calm, thoughtful, in-depth analysis (thus essentially ruling out anything found on short-form social media) that recognizes the complicated compromises involved in dealing with established bases of code that may expect different semantics, and recognizes the tricky issues involved in defining language and library semantics as a cohesive whole (in the presence of such bases of application code and implementations), and does not attack people or their decisions based on hindsight or because you personally disagree with their decisions but tries to understand the basis they were or might have been working from when coming up with those decisions in the past. There is a fundamental difference between *disagreeing* (based on the information now available with hindsight about what the consequences of certain choices turned out to be) with a choice made for C89 to allow a particular option for malloc of size 0 based on semantic elegance of not having zero-size objects in the specification, and asserting that the committee or the choice they made was in some sense objectively wrong. I don't consider the latter to be a reasonable assertion once you make proper allowance for the context of trying to define a consistent semantic basis for standard C in the first place. Much the same applies to disagreement with other choices made since then.
CC += Lundin Hi Joseph, On Thu, Oct 24, 2024 at 04:22:56PM GMT, Joseph Myers wrote: > On Thu, 24 Oct 2024, Alejandro Colomar wrote: > > > The internet is on fire about ISO C23 making realloc(p,0) UB. I don't > > This sort of exaggerated claim seems like a great way to lose credibility <https://langdev.stackexchange.com/questions/3286/what-was-the-rationale-for-making-reallocptr-0-have-ub-in-c23> <https://stackoverflow.com/questions/78691087/reallocptr-0-in-c23-now-what> <https://dl.acm.org/doi/pdf/10.1145/3588242> <https://software.codidact.com/posts/289414/289415#answer-289415> <https://software.codidact.com/posts/292694/292695#answer-292695> > I think it would be a bad idea to relitigate the obsolescence and > subsequent removal of realloc(p, 0), Why do you think banning realloc(p,0) is better than fixing it? Yes, it will be hard to fix it, but how is banning it better than fixing it? Even if you say realloc() cannot be fixed without breaking other stuff, we'd need realloc2() to be invented and fix this, even if it's just a thin wrapper that does realloc(p,n?:1). There's a need for 0-sized objects. > just as it's a bad idea to relitigate > the choice of name for _Lengthof. That name was chosen by a small set of programmers, influenced by programmers who don't even like C as their main language. I want to avoid having another mistake from WG14 be a mess for the future. Why should WG14's preference be more important than GCC's? Would you mind surveying GCC and glibc maintainers and contributors on the topic? Also, I can't even understand how a WG14 member gets offended by me ignoring the results of his survey presented at his job to programmers that explicitly say they hope to not have to program ever again in C, as part of the survey comment. Their argument is that many other languages --which don't have the ambiguity problem of null-terminated strings also using length-- use length for this, so we should follow. Please consider countof. Have a lovely day! Alex
On Thu, 24 Oct 2024, Alejandro Colomar wrote: > It must run on any system of your like (except AIX; that's a broken > one), and behave well. The same program must have a double free on the > BSD/Unix V7 behavior in a circumstance that wouldn't trigger on the > other system. Bonus points if the system under which the program works > fine is glibc. That should be easy if the decission by WG14 was based > on true facts. A double free comes from a hypothetical change to semantics in one direction; leaks from a hypothetical change in a different direction. The old implementation where a double free doesn't occur would be one that only returns NULL from realloc with size 0 when there is an error, and doesn't free the old allocation in that case. (For example, an implementation that never returns NULL from realloc with size 0 and a non-null pointer argument.) An application written for that interpretation of old versions of the standards might then check for NULL return and free the memory itself in that case (and that code might be unreachable if the implementation doesn't return NULL). The new implementation would be one that does return NULL and does free the old allocation. That's why a change in that direction would have caused problems with double frees for some applications. A hypothetical change in semantics in the direction of always returning a non-null pointer (and thus freeing the previous allocation) should not introduce double frees, but can introduce memory leaks in any application that relied on realloc with size 0 being equivalent to free and not doing a new allocation even of size 0. There are at least three options for what hypothetical new semantics might be (ignoring further variations such as semantics allowing more than one of these options, or semantics involving errno). Every one might introduce either double frees or memory leaks in applications that were expecting different semantics. * Always returns non-null (and frees previous allocation): can introduce memory leaks. * Always returns null, as an error without freeing previous allocation: can introduce memory leaks. * Always returns null and frees previous allocation: can introduce double frees in applications that interpreted the null return as indicating an error and thus freed the previous allocation themselves. You can reasonably argue that this renders some choices of new semantics better than others because some risks are more serious than others. I think the choice of obsolescence followed by undefined behavior was a carefully considered, reasonable conclusion in a situation where every choice would have adverse impacts for some (application, implementation) pairs, and that there is nothing new sufficient to justify revisiting this choice already.
Hi Joseph, On Thu, Oct 24, 2024 at 04:45:15PM GMT, Joseph Myers wrote: > On Thu, 24 Oct 2024, Alejandro Colomar wrote: > > > know if you've searched about it, but I've personally received messages > > from several people despairing about C23, and mentioning that as one of > > the most obvious wrongs that ISO has done in C23. > > I suggest that people generating noise rather than light are good examples > of people to ignore. This really isn't a matter for "despair" or talking > about "most obvious wrongs" (although such terminology is still to be > preferred to certain offensive language seen earlier in this discussion). > > It's rather a matter for calm, thoughtful, in-depth analysis (thus Agree. I want to do that. I'll try to be more measured with my language. However, refusing to talk about this saying that this was discussed in C17, doesn't seem like an in-depth analysis. Can we please do an in-depth analysis, forgetting any mistakes on either side that might have been made prior to now? I'm interested in fixing this, and am trying to find the best solution. I'd appreciate some collaboration. Some of us think that this is not dangerous, and can be fixed in glibc without any problems. I'm, just like everybody, not interested in making things worse, so it would be great if you could show how improving the behavior of glibc could make things worse. Or maybe tell me what you need so that we can agree to a path to a fix. > essentially ruling out anything found on short-form social media) that I don't use short-form social media. I was actually referring to senior engineers that often publish on stackexchange and similar sites. Engineers that have taught me a lot of what I currently know, and which have a deep understanding of C. Have a lovely day! Alex
On Thu, 24 Oct 2024, Alejandro Colomar wrote: > CC += Lundin > > Hi Joseph, > > On Thu, Oct 24, 2024 at 04:22:56PM GMT, Joseph Myers wrote: > > On Thu, 24 Oct 2024, Alejandro Colomar wrote: > > > > > The internet is on fire about ISO C23 making realloc(p,0) UB. I don't > > > > This sort of exaggerated claim seems like a great way to lose credibility > > <https://langdev.stackexchange.com/questions/3286/what-was-the-rationale-for-making-reallocptr-0-have-ub-in-c23> That's someone asking a question and having it answered. You can expect people to ask questions about changes in a new standard version. > <https://stackoverflow.com/questions/78691087/reallocptr-0-in-c23-now-what> That's pointing out an editorial issue in non-normative parts of the standard (Annex J). It's true that certain lists there should no longer mention realloc now that it's separately undefined behavior to call it with size 0. The fix to that editorial issue would be very straightforward: just remove realloc from a few items in Annex J. > <https://dl.acm.org/doi/pdf/10.1145/3588242> One person disagreeing with the decision is not "The internet is on fire". > <https://software.codidact.com/posts/289414/289415#answer-289415> That's just describing new features in C23. > <https://software.codidact.com/posts/292694/292695#answer-292695> That just seems like people discussing size 0 semantics. All of this just looks like a few discussions of changes in C23. Nothing for which "The internet is on fire" is a good description. > Why do you think banning realloc(p,0) is better than fixing it? Yes, it > will be hard to fix it, but how is banning it better than fixing it? A fix in any direction would break some (application, implementation) pairs that currently work. > Even if you say realloc() cannot be fixed without breaking other stuff, > we'd need realloc2() to be invented and fix this, even if it's just a > thin wrapper that does realloc(p,n?:1). There's a need for 0-sized > objects. New functions are a sensible solution when previous APIs and associated implementation divergence make it hard or impossible to use certain features safely and portably. Cf. the discussion we had at WG14 in Minneapolis about the problems with struct tm and implementation-defined members thereof making it unclear what is / is not a valid way to create or modify a struct tm, and how completely new interfaces might be a sensible solution there (provided they avoid the problems with struct tmx that was first added then removed in C9X drafts). In the case of realloc, however, the only problem is with the size 0 case; all other cases are unproblematic.
On Thu, 24 Oct 2024, Joseph Myers wrote: > > <https://stackoverflow.com/questions/78691087/reallocptr-0-in-c23-now-what> > > That's pointing out an editorial issue in non-normative parts of the > standard (Annex J). It's true that certain lists there should no longer > mention realloc now that it's separately undefined behavior to call it > with size 0. The fix to that editorial issue would be very > straightforward: just remove realloc from a few items in Annex J. (And also add realloc with size 0 to the list of undefined behavior in Annex J to match the normative part of the standard, since it seems it didn't get added there.)
Hi Joseph, On Thu, Oct 24, 2024 at 05:02:10PM GMT, Joseph Myers wrote: > On Thu, 24 Oct 2024, Alejandro Colomar wrote: > > > It must run on any system of your like (except AIX; that's a broken > > one), and behave well. The same program must have a double free on the > > BSD/Unix V7 behavior in a circumstance that wouldn't trigger on the > > other system. Bonus points if the system under which the program works > > fine is glibc. That should be easy if the decission by WG14 was based > > on true facts. > > A double free comes from a hypothetical change to semantics in one > direction; leaks from a hypothetical change in a different direction. > > The old implementation where a double free doesn't occur would be one that > only returns NULL from realloc with size 0 when there is an error, and > doesn't free the old allocation in that case. (For example, an > implementation that never returns NULL from realloc with size 0 and a > non-null pointer argument.) An application written for that > interpretation of old versions of the standards might then check for NULL > return and free the memory itself in that case (and that code might be > unreachable if the implementation doesn't return NULL). > > The new implementation would be one that does return NULL and does free > the old allocation. > > That's why a change in that direction would have caused problems with > double frees for some applications. > > A hypothetical change in semantics in the direction of always returning a > non-null pointer (and thus freeing the previous allocation) should not > introduce double frees, but can introduce memory leaks in any application > that relied on realloc with size 0 being equivalent to free and not doing > a new allocation even of size 0. > > There are at least three options for what hypothetical new semantics might > be (ignoring further variations such as semantics allowing more than one > of these options, or semantics involving errno). Every one might > introduce either double frees or memory leaks in applications that were > expecting different semantics. > > * Always returns non-null (and frees previous allocation): can introduce > memory leaks. This is the one we're proposing, and yes, it can potentially introduce memory leaks. One case that would obviously produce such leaks is implementing free(3) as a call to realloc(p,0). (And I've seen that somewhere in GCC, so there are indeed things that need to be fixed first.) However, those uses are actually rare, and we can probably minimize them by searching all code in Debian that does something like that: a hardcoded 0 in a call to realloc. Maybe a diagnostic in GCC could be added for hardcoded 0's in realloc(3) in -Wall. I think that would be part of a good transition plan. Also, an implementation of free(3) as a call realloc(p,0) is itself buggy, and potentially leaky, since free(3) accepts NULL, but realloc(p,0) would leak 0B, so it can't be called with NULL (it's not so bad, but for something implementing a generic free(3) it would be obviously wrong). Then there's run-time 0 values. In those, I'm not sure what kind of code could be written that would introduce leaks where they didn't previously exist. I can think of the following, which would be even worse than a forever loop: p = NULL; while (true) { n = ...; p = realloc(p, n); if (p == NULL) // n has gone down to 0 (or error). break; /* do work */ } Which would introduce a forever loop (until realloc fails). However, that program already relies on the first n not being 0 to not misbehave, so a forever loop might in fact be better than the current behavior. This is possibly the most worrying case that I can think of. > > * Always returns null, as an error without freeing previous allocation: > can introduce memory leaks. > > * Always returns null and frees previous allocation: can introduce double > frees in applications that interpreted the null return as indicating an > error and thus freed the previous allocation themselves. > > You can reasonably argue that this renders some choices of new semantics > better than others because some risks are more serious than others. I > think the choice of obsolescence followed by undefined behavior was a > carefully considered, reasonable conclusion in a situation where every > choice would have adverse impacts for some (application, implementation) > pairs, and that there is nothing new sufficient to justify revisiting this > choice already. Please let's revisit. If realloc() cannot be saved, can we please discuss the introduction of remalloc() or something like that? I do need the semantics of that thing. Calling realloc(3) through a wrapper that does ?:1 is a decent workaround, but it might hide off-by-one bugs in certain cases, so a realloc() that works well with 0 would be better. Cheers, Alex
On 2024-10-24 10:02, Joseph Myers wrote: > the choice of obsolescence followed by undefined behavior was a > carefully considered, reasonable conclusion in a situation where every > choice would have adverse impacts for some (application, implementation) > pairs, and that there is nothing new sufficient to justify revisiting this > choice already I assume you are writing about what C23 says, and I agree with that assessment. However, the crux of this discussion is elsewhere: it is about what future versions of glibc should do and what future C and POSIX standards should say. Even if we agree that the current C23 standard should say realloc(p,0) is undefined and that the current POSIX should partially specify its behavior (in a way that current glibc doesn't always conform to!), that doesn't mean we can't improve this messy situation moving forward. At bottom this is a disagreement about whether it's worth the trouble to clean up this mess in the obvious way. Concerns have been raised that having glibc behave like BSD/musl/etc. would adversely affect some applications that assume glibc semantics. One specific application has been mentioned (systemd) but on further investigation it has no problems. I'd welcome further examples of significant real-world problems that would come from the proposed change. So far we haven't seen any, which means that so far all the hobgoblins have been imaginary.
Hi Joseph, On Thu, Oct 24, 2024 at 05:18:26PM GMT, Joseph Myers wrote: > > Why do you think banning realloc(p,0) is better than fixing it? Yes, it > > will be hard to fix it, but how is banning it better than fixing it? > > A fix in any direction would break some (application, implementation) > pairs that currently work. > > > Even if you say realloc() cannot be fixed without breaking other stuff, > > we'd need realloc2() to be invented and fix this, even if it's just a > > thin wrapper that does realloc(p,n?:1). There's a need for 0-sized > > objects. > > New functions are a sensible solution when previous APIs and associated > implementation divergence make it hard or impossible to use certain > features safely and portably. Cf. the discussion we had at WG14 in > Minneapolis about the problems with struct tm and implementation-defined > members thereof making it unclear what is / is not a valid way to create > or modify a struct tm, and how completely new interfaces might be a > sensible solution there (provided they avoid the problems with struct tmx > that was first added then removed in C9X drafts). In the case of realloc, > however, the only problem is with the size 0 case; all other cases are > unproblematic. Still, programs need that case. How about this?: We agree to ban realloc(p,0) temporarily; say until C2y, to give implementations time to add a fortify assertion that it never happens. Code needing the workaround ?:1 will either already have done so, or will do it after a fortified realloc(3) breaks their code. Then, after C2y is released, we reinstate realloc(p,0), but do it with semantics compatible with malloc(0) --whatever malloc(0) does on that system--. No extant code would break, since they had to migrate away. In C3x, we reinstate realloc(p,0) to be compatible with realloc(NULL,0) and malloc(0). Programs that were using ?:1 can gradually remove it, at the pace required by their portability needs. Does that sound like a good compromise? Cheers, Alex > > -- > Joseph S. Myers > josmyers@redhat.com >
On Thu, 24 Oct 2024, Alejandro Colomar wrote: > We agree to ban realloc(p,0) temporarily; say until C2y, to give > implementations time to add a fortify assertion that it never happens. > Code needing the workaround ?:1 will either already have done so, or > will do it after a fortified realloc(3) breaks their code. I agree that fortification (when building applications with C23 features enabled; hopefully the default with GCC 15 and later if a few remaining testsuite issues get resolved) is an implementation change it would make sense to explore to detect programs exercising this undefined behavior. I'm much more doubtful about reinstating defined semantics for realloc with size 0 in future.
On 2024-10-24 08:15, Alejandro Colomar wrote: > The only libc implementations that have realloc(p,0) and realloc(NULL,0) > be inconsistent are glibc, Win32, and Bionic, AFAIK. There's also AIX, which has things almost exactly backwards. AIX realloc(p,0) normally acts like glibc except it sets errno=EINVAL as POSIX requires (glibc does not conform to POSIX here). However, if you #define _LINUX_SOURCE_COMPAT, AIX realloc(p,0) stops acting mostly like glibc: instead, it acts like BSD! It does so via <stdlib.h>'s "#define malloc __linux_realloc", where __linux_realloc behaves like BSD. Both AIX behaviors conform to current POSIX, whereas glibc does not conform. The proposed change to glibc would fix this incompatibility with AIX.
On Thu, Oct 24, 2024 at 10:51:27AM GMT, Paul Eggert wrote: > On 2024-10-24 08:15, Alejandro Colomar wrote: > > The only libc implementations that have realloc(p,0) and realloc(NULL,0) > > be inconsistent are glibc, Win32, and Bionic, AFAIK. > > There's also AIX, which has things almost exactly backwards. Yep; that's the other one. Although I think we can ignore it for the standards. Does AIX intend to comply with C23 and/or POSIX.1-2024? How far from the graveyard is it? > > AIX realloc(p,0) normally acts like glibc except it sets errno=EINVAL as > POSIX requires (glibc does not conform to POSIX here). > > However, if you #define _LINUX_SOURCE_COMPAT, AIX realloc(p,0) stops acting > mostly like glibc: lol It might come from the times of Linux libc, or before glibc got broken? > instead, it acts like BSD! It does so via <stdlib.h>'s > "#define malloc __linux_realloc", where __linux_realloc behaves like BSD. > > Both AIX behaviors conform to current POSIX, whereas glibc does not conform. > > The proposed change to glibc would fix this incompatibility with AIX. Have a lovely night! Alex
Hi Joseph, On Thu, Oct 24, 2024 at 05:40:08PM GMT, Joseph Myers wrote: > On Thu, 24 Oct 2024, Alejandro Colomar wrote: > > > We agree to ban realloc(p,0) temporarily; say until C2y, to give > > implementations time to add a fortify assertion that it never happens. > > Code needing the workaround ?:1 will either already have done so, or > > will do it after a fortified realloc(3) breaks their code. > > I agree that fortification (when building applications with C23 features > enabled; hopefully the default with GCC 15 and later if a few remaining > testsuite issues get resolved) is an implementation change it would make > sense to explore to detect programs exercising this undefined behavior. > > I'm much more doubtful about reinstating defined semantics for realloc > with size 0 in future. Could you clarify why? Have a lovely night! Alex
Hi DJ, On Thu, Oct 24, 2024 at 02:27:37PM GMT, DJ Delorie wrote: > Alejandro Colomar <alx@kernel.org> writes: > > However, refusing to talk about this saying that this was discussed in > > C17, doesn't seem like an in-depth analysis. > > The request wasn't "don't talk about this". It was "let's not rehash > the old discussion." Focus on today. That sounds good to me. > > Can we please do an in-depth analysis, > > Each time someone wants to make a program-facing change in malloc, I say > the same thing, to wit: glibc's malloc is the system malloc. It has to > be very reliable and conservative, and if you want to change it, you > basically have to test it against *everything* in GNU/Hurd and GNU/Linux > (I recommend Fedora, because reasons, so that's about 70,000 packages to > test). > > So far everyone has been arguing... er, "discussing"... based on made up > scenarios, any of which *could* happen. Not really *any*. Some people (e.g., JeanHeyd) said this would necessarily result in double-frees, which I thin we agree cannot happen (Joseph just admitted that earlier today). But yeah, several scenarios which are not good could happen, such as leaks of 0+16 B, or infinite loops. > What we need is for someone to > FIND OUT which cases actually exist in practice, and what breaking them > would mean. > > Hard data beats opinion every day. > > I personaly think realloc(...,0) should match malloc(0) when > appropriate, but I'm against change *in general* if applications could > unknownly break in the future because of it. I really don't like > surprises. Agreed. Who is able to test that? Have a lovely night! Alex
On Thu, 24 Oct 2024, Alejandro Colomar wrote: > > I agree that fortification (when building applications with C23 features > > enabled; hopefully the default with GCC 15 and later if a few remaining > > testsuite issues get resolved) is an implementation change it would make > > sense to explore to detect programs exercising this undefined behavior. > > > > I'm much more doubtful about reinstating defined semantics for realloc > > with size 0 in future. > > Could you clarify why? Once something is in such a mess as realloc with size 0 has been ever since there were multiple implementations in use with incompatible semantics and multiple standard versions supporting different interpretations, resolving things as "don't use this feature" seems preferable to me to ending up with an even more complicated resolution "don't use in this range of versions, but maybe you can use with this newer version". This admits the possibility of the standard defining some kind of erroneous runtime behavior, which conforming applications must not use but where there are bounded choices for what an implementation may do (terminate the program; free the memory and return either an allocated pointer or NULL; fail to free the memory and return NULL). The ability to terminate the program, in particular, would be a useful option if bounding the choices like that.
Hi Joseph, On Thu, Oct 24, 2024 at 09:21:50PM GMT, Joseph Myers wrote: > > > I'm much more doubtful about reinstating defined semantics for realloc > > > with size 0 in future. > > > > Could you clarify why? > > Once something is in such a mess as realloc with size 0 has been ever > since there were multiple implementations in use with incompatible > semantics and multiple standard versions supporting different > interpretations, resolving things as "don't use this feature" seems > preferable to me to ending up with an even more complicated resolution > "don't use in this range of versions, but maybe you can use with this > newer version". That's why a decade or so is a good thing. We could wait until the last system with a bad glibc dies, and then reinstate it with the good behavior. Portable programs shouldn't have problems dealing with ranges of versions; I agree with that. So, imagine that the last system with the current glibc behavior is EOL in 2035. On 2036, we could reinstate realloc() with any behavior we want. > This admits the possibility of the standard defining some kind of > erroneous runtime behavior, which conforming applications must not use but > where there are bounded choices for what an implementation may do > (terminate the program; free the memory and return either an allocated > pointer or NULL; fail to free the memory and return NULL). I would reword that to: "terminate the program; or free the memory and return the result of malloc(0). On error, the memory is not freed, NULL is returned, and errno is set to indicate the error.". > The ability to > terminate the program, in particular, would be a useful option if bounding > the choices like that. That would be good. Have a lovely night! Alex
On 2024-10-24 14:04, Alejandro Colomar wrote: > Does AIX intend to comply with C23 and/or POSIX.1-2024? How > far from the graveyard is it? Of all the operating systems in the world, AIX is the most likely to conform to POSIX.1-2024 eventually. That is because it's the only OS certified to conform to POSIX.1-2017. And AIX is far from the graveyard: IBM currently plans to support AIX through at least the year 2035 <https://www.ibm.com/power>. On this particular issue, AIX realloc(p,0) obviously conforms to C23, where behavior is undefined. And it conforms to POSIX.1-2024, which allows either return-non-null behavior or return-null behavior. >> if you #define _LINUX_SOURCE_COMPAT, AIX realloc(p,0) stops acting >> mostly like glibc: > > lol > > It might come from the times of Linux libc, or before glibc got broken? Possibly. Or possibly the AIX maintainers thought that realloc(p,0) should be consistent with malloc(0), and so implemented the traditional+BSD behavior without thinking about it. We'd likely need access to AIX internals to know more. One other detail: although AIX 7.3 realloc(p,0) conforms to POSIX.1-2017+ by setting errno when returning null, AIX 7.1 realloc(p,0) did not: it left errno alone, as current glibc does. Presumably this is because POSIX didn't require errno to be set until POSIX.1-2017, and earlier AIX conformed to earlier POSIX editions. AIX 7.1 realloc(p,0) therefore was compatible with current glibc if and only if you did *not* #define _LINUX_SOURCE_COMPAT, whereas AIX 7.3 conforms to POSIX.1-2017+ regardless of whether _LINUX_SOURCE_COMPAT is defined so it is never compatible with current glibc realloc(p,0). I use the past tense for AIX 7.1 (2010) because it's no longer supported. I lack access to AIX 7.2 (2015), which is still supported and which was the first to conform to POSIX.1-2017.
Am Donnerstag, dem 24.10.2024 um 18:02 +0200 schrieb Alejandro Colomar: > ... > > > But > > addressing one zero-object-size case and leaving all the others as > > unclear as before doesn't seem to be useful, especially given that a > > ffuture standard may require different behavior anyway. > > I assume you mean 0-sized arrays as the other zero-sized-object that you > refer to. > > Martin and I are working on improving the support for 0-sized arrays in > GCC. That's a problem for implementing the __countof__ operator to work > on array parameters. So there's already work to improve the GCC support > on 0-sized arrays before being able to do that. I think for purposes relevant to this discussion, GCC and clang already support 0-sized arrays just fine and any standardization will certainly not break this (modulo corner cases). I can not predict how likely standardization of this is, but I would support this. (What I am working on is to fix some corner case bugs related to the internal representation of arrays in GCC. I do not think this is relevant here.) ... > > C.1) Make GCC have [n] have a stronger meaning for array parameters. > That is, not ignore the number of elements of array parameters, > and instead make it mean the same as [static n], except that [n] > doesn't require non-null-ness nor require at least 1 element. This is already the case. GCC will warn about passing shorter arrays to function (by default even without -Wall) since GCC 11. https://godbolt.org/z/bz7fEaE1M I do not think there were many complaints about this. Martin
Hi DJ, Paul, On Thu, Oct 24, 2024 at 11:19:16PM +0200, Alejandro Colomar wrote: > On Thu, Oct 24, 2024 at 02:27:37PM GMT, DJ Delorie wrote: > > What we need is for someone to > > FIND OUT which cases actually exist in practice, and what breaking them > > would mean. > > > > Hard data beats opinion every day. > > > > I personaly think realloc(...,0) should match malloc(0) when > > appropriate, but I'm against change *in general* if applications could > > unknownly break in the future because of it. I really don't like > > surprises. gnulib did change that back in November, in the realloc-posix module. Paul, I guess nothing has broken, did it? > > Agreed. > > Who is able to test that? I'd like to ping about this. Is anyone able to test such a change in Fedora? Have a lovely day! Alex
On 2025-01-08 03:50, Alejandro Colomar wrote: > gnulib did change that back in November, in the realloc-posix module. > Paul, I guess nothing has broken, did it? Yes, nothing broke, as far as we know. Not surprising, as it's implausible that things would break.
diff --git a/NEWS b/NEWS index 2fe0396b2d..00abfd896e 100644 --- a/NEWS +++ b/NEWS @@ -31,6 +31,10 @@ Major new features: * The iconv program now supports converting files in place. The program automatically uses a temporary file if required. +* A new tunable glibc.malloc.realloc_zero_bytes_frees can be set to 0 to + make the realloc function return a zero sized pointer instead of NULL + when the requested new size is 0 bytes. + Deprecated and removed features, and other changes affecting compatibility: * The big-endian ARC port (arceb-linux-gnu) has been removed. diff --git a/elf/dl-tunables.list b/elf/dl-tunables.list index 40ac5b3776..5dcf534332 100644 --- a/elf/dl-tunables.list +++ b/elf/dl-tunables.list @@ -82,6 +82,12 @@ glibc { type: SIZE_T minval: 0 } + realloc_zero_bytes_frees { + type: INT_32 + minval: 0 + maxval: 1 + default: 1 + } } elision { diff --git a/malloc/Makefile b/malloc/Makefile index 1630aaf6ac..fcf0806fe1 100644 --- a/malloc/Makefile +++ b/malloc/Makefile @@ -61,6 +61,7 @@ tests := \ tst-pvalloc \ tst-pvalloc-fortify \ tst-realloc \ + tst-realloc-nonfreeing \ tst-reallocarray \ tst-safe-linking \ tst-tcfree1 tst-tcfree2 tst-tcfree3 \ @@ -136,12 +137,17 @@ tests-exclude-hugetlb1 = \ tst-mallocfork2 \ tst-mallocfork3 \ tst-mallocstate \ + tst-realloc-nonfreeing \ # tests-exclude-hugetlb1 + # The tst-free-errno relies on the used malloc page size to mmap an # overlapping region. tests-exclude-hugetlb2 = \ - $(tests-exclude-hugetlb1) \ - tst-free-errno + $(tests-exclude-hugetlb1) \ + tst-free-errno \ + tst-realloc-nonfreeing \ +# tests-exclude-hugetlb2 + tests-malloc-hugetlb1 = \ $(filter-out $(tests-exclude-hugetlb1), $(tests)) tests-malloc-hugetlb2 = \ @@ -338,6 +344,13 @@ tst-malloc-usable-ENV = MALLOC_CHECK_=3 \ tst-malloc-usable-tunables-ENV = GLIBC_TUNABLES=glibc.malloc.check=3 \ LD_PRELOAD=$(objpfx)/libc_malloc_debug.so +tst-realloc-nonfreeing-ENV += \ + GLIBC_TUNABLES=glibc.malloc.realloc_zero_bytes_frees=0 +tst-realloc-nonfreeing-malloc-check-ENV += \ + GLIBC_TUNABLES=glibc.malloc.realloc_zero_bytes_frees=0 +tst-realloc-nonfreeing-mcheck-ENV += \ + GLIBC_TUNABLES=glibc.malloc.realloc_zero_bytes_frees=0 + tst-mxfast-ENV = GLIBC_TUNABLES=glibc.malloc.tcache_count=0:glibc.malloc.mxfast=0 CPPFLAGS-malloc-debug.c += -DUSE_TCACHE=0 diff --git a/malloc/arena.c b/malloc/arena.c index cfb1ff8854..99eb748f9b 100644 --- a/malloc/arena.c +++ b/malloc/arena.c @@ -253,6 +253,7 @@ TUNABLE_CALLBACK_FNDECL (set_tcache_unsorted_limit, size_t) #endif TUNABLE_CALLBACK_FNDECL (set_mxfast, size_t) TUNABLE_CALLBACK_FNDECL (set_hugetlb, size_t) +TUNABLE_CALLBACK_FNDECL (set_realloc_zero_bytes_frees, int32_t) #if USE_TCACHE static void tcache_key_initialize (void); @@ -312,6 +313,8 @@ ptmalloc_init (void) # endif TUNABLE_GET (mxfast, size_t, TUNABLE_CALLBACK (set_mxfast)); TUNABLE_GET (hugetlb, size_t, TUNABLE_CALLBACK (set_hugetlb)); + TUNABLE_GET (realloc_zero_bytes_frees, int32_t, + TUNABLE_CALLBACK (set_realloc_zero_bytes_frees)); if (mp_.hp_pagesize > 0) { diff --git a/malloc/malloc-check.c b/malloc/malloc-check.c index da1158b333..3a4941414b 100644 --- a/malloc/malloc-check.c +++ b/malloc/malloc-check.c @@ -257,7 +257,7 @@ realloc_check (void *oldmem, size_t bytes) if (oldmem == 0) return malloc_check (bytes); - if (bytes == 0) + if (mp_.realloc_zero_bytes_frees == 1 && bytes == 0) { free_check (oldmem); return NULL; diff --git a/malloc/malloc-debug.c b/malloc/malloc-debug.c index c9b4166a1e..1fde57722c 100644 --- a/malloc/malloc-debug.c +++ b/malloc/malloc-debug.c @@ -73,9 +73,12 @@ __malloc_debug_disable (enum malloc_debug_hooks flag) __malloc_debugging_hooks &= ~flag; } +/* The ordering matters here because mcheck.c includes elf/dl-tunables.h, which + ends up messing with the same include in malloc-check.c. This probably + needs to be fixed at some point. */ +#include "malloc-check.c" #include "mcheck.c" #include "mtrace.c" -#include "malloc-check.c" #if SHLIB_COMPAT (libc_malloc_debug, GLIBC_2_0, GLIBC_2_24) extern void (*__malloc_initialize_hook) (void); diff --git a/malloc/malloc.c b/malloc/malloc.c index bcb6e5b83c..be37f2b117 100644 --- a/malloc/malloc.c +++ b/malloc/malloc.c @@ -177,7 +177,6 @@ USE_PUBLIC_MALLOC_WRAPPERS NOT defined USE_MALLOC_LOCK NOT defined MALLOC_DEBUG NOT defined - REALLOC_ZERO_BYTES_FREES 1 TRIM_FASTBINS 0 Options for customizing MORECORE: @@ -330,20 +329,6 @@ ((__typeof (ptr)) ((((size_t) pos) >> 12) ^ ((size_t) ptr))) #define REVEAL_PTR(ptr) PROTECT_PTR (&ptr, ptr) -/* - The REALLOC_ZERO_BYTES_FREES macro controls the behavior of realloc (p, 0) - when p is nonnull. If the macro is nonzero, the realloc call returns NULL; - otherwise, the call returns what malloc (0) would. In either case, - p is freed. Glibc uses a nonzero REALLOC_ZERO_BYTES_FREES, which - implements common historical practice. - - ISO C17 says the realloc call has implementation-defined behavior, - and it might not even free p. -*/ - -#ifndef REALLOC_ZERO_BYTES_FREES -#define REALLOC_ZERO_BYTES_FREES 1 -#endif /* TRIM_FASTBINS controls whether free() of a very small chunk can @@ -628,9 +613,9 @@ void* __libc_calloc(size_t, size_t); ANSI) and p is NOT freed. if n is for fewer bytes than already held by p, the newly unused - space is lopped off and freed if possible. Unless the #define - REALLOC_ZERO_BYTES_FREES is set, realloc with a size argument of - zero (re)allocates a minimum-sized chunk. + space is lopped off and freed if possible. Unless the + REALLOC_ZERO_BYTES_FREES tunable is set, realloc with a size argument + of zero (re)allocates a minimum-sized chunk. Large chunks that were internally obtained via mmap will always be grown using malloc-copy-free sequences unless the system supports @@ -1861,6 +1846,7 @@ struct malloc_par INTERNAL_SIZE_T mmap_threshold; INTERNAL_SIZE_T arena_test; INTERNAL_SIZE_T arena_max; + int realloc_zero_bytes_frees; /* Transparent Large Page support. */ INTERNAL_SIZE_T thp_pagesize; @@ -1919,7 +1905,8 @@ static struct malloc_par mp_ = .mmap_threshold = DEFAULT_MMAP_THRESHOLD, .trim_threshold = DEFAULT_TRIM_THRESHOLD, #define NARENAS_FROM_NCORES(n) ((n) * (sizeof (long) == 4 ? 2 : 8)) - .arena_test = NARENAS_FROM_NCORES (1) + .arena_test = NARENAS_FROM_NCORES (1), + .realloc_zero_bytes_frees = 1 #if USE_TCACHE , .tcache_count = TCACHE_FILL_COUNT, @@ -3413,12 +3400,11 @@ __libc_realloc (void *oldmem, size_t bytes) if (!__malloc_initialized) ptmalloc_init (); -#if REALLOC_ZERO_BYTES_FREES - if (bytes == 0 && oldmem != NULL) + if (mp_.realloc_zero_bytes_frees && bytes == 0 && oldmem != NULL) { - __libc_free (oldmem); return 0; + __libc_free (oldmem); + return 0; } -#endif /* realloc of null is supposed to be same as malloc */ if (oldmem == 0) @@ -5554,6 +5540,23 @@ do_set_hugetlb (size_t value) return 0; } +/* The REALLOC_ZERO_BYTES_FREES tunable controls the behavior of realloc (p, 0) + when p is nonnull. If the macro is nonzero, the realloc call returns NULL; + otherwise, the call returns what malloc (0) would. In either case, + p is freed. Glibc uses a nonzero REALLOC_ZERO_BYTES_FREES by default to + remain compatible with older programs. + + ISO C17 says the realloc call has implementation-defined behavior, + and it might not even free p. */ +static __always_inline int +do_set_realloc_zero_bytes_frees (int32_t value) +{ + if (value == 0) + mp_.realloc_zero_bytes_frees = 0; + + return 0; +} + int __libc_mallopt (int param_number, int value) { diff --git a/malloc/mcheck-impl.c b/malloc/mcheck-impl.c index eff4873f62..6d1cd8876e 100644 --- a/malloc/mcheck-impl.c +++ b/malloc/mcheck-impl.c @@ -16,6 +16,7 @@ License along with the GNU C Library; if not, see <https://www.gnu.org/licenses/>. */ +#include <elf/dl-tunables.h> #include <malloc-internal.h> #include <mcheck.h> #include <libintl.h> @@ -31,6 +32,7 @@ /* Function to call when something awful happens. */ static void (*abortfunc) (enum mcheck_status); +static int realloc_zero_bytes_frees = 1; struct hdr { @@ -285,7 +287,7 @@ realloc_mcheck_before (void **ptrp, size_t *sizep, size_t *oldsize, return true; } - if (size == 0) + if (realloc_zero_bytes_frees == 1 && size == 0) { __debug_free (ptr); *victimp = NULL; @@ -401,6 +403,11 @@ __mcheck_initialize (void (*func) (enum mcheck_status), bool in_pedantic) } pedantic = in_pedantic; + + if (TUNABLE_GET_FULL (glibc, malloc, realloc_zero_bytes_frees, int32_t, NULL) + == 0) + realloc_zero_bytes_frees = 0; + return 0; } diff --git a/malloc/tst-realloc-nonfreeing.c b/malloc/tst-realloc-nonfreeing.c new file mode 100644 index 0000000000..eec9fcb767 --- /dev/null +++ b/malloc/tst-realloc-nonfreeing.c @@ -0,0 +1,35 @@ +/* Smoke test for realloc when glibc.malloc.realloc_zero_bytes_frees tunable + is set to zero. + Copyright The GNU Toolchain Authors. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + <https://www.gnu.org/licenses/>. */ + +#include <stdlib.h> +#include <support/check.h> + +static int +do_test (void) +{ + char *foo = malloc (42); + char *p = realloc (foo, 0); + + TEST_VERIFY (p != NULL); + free (p); + + return 0; +} + +#include <support/test-driver.c> diff --git a/manual/memory.texi b/manual/memory.texi index 3710d7ec66..5ade11101e 100644 --- a/manual/memory.texi +++ b/manual/memory.texi @@ -834,9 +834,14 @@ contents. If you pass a null pointer for @var{ptr}, @code{realloc} behaves just like @samp{malloc (@var{newsize})}. -Otherwise, if @var{newsize} is zero -@code{realloc} frees the block and returns @code{NULL}. -Otherwise, if @code{realloc} cannot reallocate the requested size + +Otherwise, if @var{newsize} is zero @code{realloc} will by default free +the block and return @code{NULL}. This behavior can be overridden using +the @code{glibc.malloc.realloc_zero_bytes_frees} tunable, where instead +@code{realloc} will then free @var{ptr} and attempt to return a zero +sized block. + +Finally, if @code{realloc} cannot reallocate the requested size it returns @code{NULL} and sets @code{errno}; the original block is left undisturbed. @end deftypefun diff --git a/manual/tunables.texi b/manual/tunables.texi index 0b1b2898c0..b76b643313 100644 --- a/manual/tunables.texi +++ b/manual/tunables.texi @@ -294,6 +294,15 @@ supported ones. If provided value is invalid, @code{MAP_HUGETLB} will not be used. @end deftp +@deftp Tunable glibc.malloc.realloc_zero_bytes_frees + +By default when the @code{realloc} function is called with the new size as +@var{0}, it frees the input pointer and returns @var{NULL}. Setting the value +of this tunable to @var{0} will result in @code{realloc} instead freeing the +input pointer and attempting to allocate and return a zero sized block. Any +values other than @var{0} are ignored. +@end deftp + @node Dynamic Linking Tunables @section Dynamic Linking Tunables @cindex dynamic linking tunables
Allow users to select this behaviour at runtime, which should allow them flexibility to have glibc realloc do the same thing that many other implementations do for realloc (p, 0). The default behaviour remains unchanged for now. Signed-off-by: Siddhesh Poyarekar <siddhesh@sourceware.org> --- The have-cake-and-eat-it-too solution. Lightly tested, mainly intended to get feedback from folks. NEWS | 4 +++ elf/dl-tunables.list | 6 ++++ malloc/Makefile | 17 ++++++++++-- malloc/arena.c | 3 ++ malloc/malloc-check.c | 2 +- malloc/malloc-debug.c | 5 +++- malloc/malloc.c | 49 +++++++++++++++++---------------- malloc/mcheck-impl.c | 9 +++++- malloc/tst-realloc-nonfreeing.c | 35 +++++++++++++++++++++++ manual/memory.texi | 11 ++++++-- manual/tunables.texi | 9 ++++++ 11 files changed, 119 insertions(+), 31 deletions(-) create mode 100644 malloc/tst-realloc-nonfreeing.c