▲ 234 points
back
251 comments
How interesting. GCC does indeed remove that branch.
> For example, GCC will happily remove the dest == NULL branch in the following code
I think the blog should mention `-fno-delete-null-pointer-checks`
https://gcc.gnu.org/onlinedocs/gcc/Optimize-Options.html#ind...
> -fdelete-null-pointer-checks
> [...]
> This option is enabled by default on most targets.
What a footgun.
I understand that, in an effort to compete with other compilers for relevance, GCC pursued performance over safety. Has that era passed? Could GCC choose safer over fast?
Alternatively, has someone compiled a list of flags one might want to enable in latest GCC to avoid such kinds of dangerous optimizations?
Explanation for the above: passing NULL as the destination argument to memcpy() is undefined behaviour at present. gcc assumes that the fact that memcpy() is called therefore means that the destination argument can't be NULL, so "knows" that the dest == NULL check can never be true, and so removes the test and the do_thing1() branch entirely.
Interestingly, replacing len in the memcpy() call results in gcc instead removing the memcpy() call and retaining the check - presumably a different optimisation routine decides that it's a no-op in that case. https://godbolt.org/z/cPdx6v13r is, therefore, interesting - despite this only ever calling test() with a len of 0, the elision of the dest == NULL check is still there, but test() has been inlined without the memcpy (because len == 0) but with do_thing2() (because the behaviour is undefined and so it can assume dest isn't NULL even though there's a NULL literally right there!)
Fucking compilers, man.
How does gcc infer anything about memcpy? Can't I replace the c-library memcpy with my own, so how does it know that dest == NULL can never be true?
> that memcpy() is called therefore means that the destination argument can't be NULL
The whole idea that undefined behavior cannot happen and you can therefore do optimization based on "knowing" it cannot happen is incredibly bonkers.
> Fucking compilers, man.
They're just acting as agents that derive the logical consequences of the code.
The fact that the given example code is "surprising" is analogous to this mathematical derivation:
a = b
a*a = b*a
a*a - b*b = b*a - b*b
(a - b)(a + b) = b(a - b)
(a - b)(a + b)/(a - b) = b(a - b)/(a - b)
^ Divide by 0, undefined behavior!
Everything below is not necessarily true.
a + b = b
b + b = b
2b = b
2 = 1
2 - 1 = 1 - 1
1 = 0
I just skimmed through the proposed wording in [N3322]. It looks like it silently fixes a defect too, NULL == NULL was also undefined up until C23. Hilarious.
[N3322] https://www.open-std.org/jtc1/sc22/wg14/www/docs/n3322.pdf
This is probably related to the issue with NULL - NULL mentioned in the article.
Imagine you’re working in real mode on x86, in the compact or large memory model[1]. This means that a data pointer is basically struct{uint16_t off,seg;} encoding linear address (seg<<4)+off. This makes it annoying to have individual allocations (“objects”) >64K in size (because of the weird carries), so these models don’t allow that. (The huge model does, and it’s significantly slower.) Thus you legitimately have sizeof(size_t) == 2 but sizeof(uintptr_t) == 4 (hi Rust), and God help you if you compare or subtract pointers not within the same allocation. [Also, sizeof(void *) == 4 but sizeof(void (*)(void)) == 2 in the compact model, and the other way around in the medium model.]
Note the addressing scheme is non-bijective. The C standard is generally careful not to require the implementation to canonicalize pointers: if, say, char a[16] happens to be immediately followed by int b[8], an independently declared variable, it may well be that &a+16 (legal “one past” pointer) is {16,1} but &b is {0,2}, which refers to the exact same byte, but the compiler doesn’t have to do anything special because dereferencing &a+16 is UB (duh) and comparing (char *)(&a+16) with (char *)&b or subtracting one from the other is also UB (pointers to different objects).
The issue with NULL == NULL and also with NULL - NULL is that now the null pointer is required to be canonical, or these expressions must canonicalize their operands. I don’t know why you’d ever make an implementation that has non-canonical NULLs, but I guess the text prior to this change allowed such.
[1] https://devblogs.microsoft.com/oldnewthing/20200728-00/?p=10...
> now the null pointer is required to be canonical
Yikes! This particular oddity seems annoying but sort of harmless in x86 real mode, but not necessarily in protected mode. Imagine code that wants to load a pointer into a register: it loads the offset into an ordinary register and the selector portion into a segment register. It’s permissible to load the 0 (null) selector, but loading garbage will fault immediately. So, if you allow non canonical NULL, then knowing that a pointer is either valid or NULL does not allow you to hoist a segment load above a condition that might mean you never actually dereference the pointer.
(I have plenty of experience with low-level OS code in all kinds of nasty x86 modes but, thankfully, not so much experience writing ordinary C code targeting protected mode. It sometimes boggles my mind that anyone ever got decent performance with anything involving far data pointers. Segment loads are slow, and there are not a lot of segment registers to go around.)
If so, it's one that's been introduced at some point post C99 -- the C99 spec explicitly defines the behaviour of NULL == NULL. Section 6.5.9 para 6 says "Two pointers compare equal if and only if both are null pointers, both are pointers to the same object [etc etc]".
I don't imagine NULL is defined as "pointing to an object", so I don't expect that clause to apply.
NULL == NULL was already defined -- but NULL <= NULL wasn't :)
My mistake.
Cannot find any confirmation to your statement. Otoh "All null pointer values (of compatible typewithin the same address space) are already required to compare equal. " in the limked paper.
NULL is not single type in any conventional sense (and is actually tricky to define in a way that makes it usable in the way most programmers expect).
Thus:
T1* a = NULL;
T2* b = NULL
a == b; /* may be undefined at present, depending on the nature of T1 & T2 */I feel like I've misunderstood something here... shouldn't memcpy(anything, anything, 0) just do nothing, because you're copying 0 bytes?
That's a reasonable intuitive interpretation of how it should behave, but according to the spec it's undefined behaviour and compilers have a great degree of freedom in what happens as a result.
More information on this behavior in the link below.
> Note that, apart from contrived examples with deleted null checks, the current rules do not actually help the compiler meaningfully optimize code. A memcpy implementation cannot rely on pointer validity to speculatively read because, even though memcpy(NULL, NULL, 0) is undefined, slices at the end of a buffer are fine. [And if the end of the buffer] were at the end of a page with nothing allocated afterwards, a speculative read from memcpy would break
Why didn't they just... define it, back when they wrote it?
I feel strongly they should split undefined behavior in behavior that is not defined, and things that the compiler is allowed to assume. The former basically already exists as "implementation defined behavior". The latter should be written out explicitly in the documentation:
> memcpy(dest, src, count)
> Copies count bytes from src to dest. [...] Note this is not a plain function, but a special form that applies the constraints dest != NULL and src != NULL to the surrounding scope. Equivalent to:
assume(dest != NULL)
assume(src != NULL)
actual_memcpy(dest, src, count)
I'd love to have a flag "-fno-surprizing-ub" or "-fhighlevel-assembler" combined with the above `assume` function or some other syntax to let me help the compiler, so that I can write C like in the 90s - close to metal but with less surprizes.
A trivial implementation wouldn't dereference dest or src in case the length is 0. That's how a student would write it with a for loop (byte-by-byte copy). A non-trivial implementation might do something with the pointers before entering the copy loop.
It does nothing, but is only defined when the pointers point into or one past the end of valid objects (live allocations), because that's how the standard defines the C VM, in terms of objects, not a flat byte array.
I have asked this question in the past and was told that memcpy() is allowed to preemptively read before it has determined it needs to write to make it faster on some CPUs. The presumption is that if you are going to be copying data, there is at least one cache line there already, so reading can start early.
Purely mechanically, yes, but in terms of the definition of the behaviour in the C abstract machine, no, because certain operations on null pointers are undefined, even if the obvious low-level compilation turns into nothing.
"man bcopy" on BSD:
'If len is zero, no bytes are copied.'
Seems reasonable.
Yes and no.
No, because ISO never said it must behave this way.
Yes, because every libc I've personally encountered acts this way. At a glance, glibc's x86 implementation[1, 2], musl, and picolibc all handle 0-length memcpy as you'd expect. I'm sure other folks could dig up the code for Newlib, uclibc, and others, and they'd see the same thing.
On a related note, ISO C has THREE different things that most people tend to lump together as "undefined behavior." They are:
Implementation-defined behavior: ISO doesn't require any particular behavior, but they do require implementations to consistently apply a particular behavior, and document that behavior.
Unspecified behavior: ISO doesn't require any particular behavior, but they do require implementations to consistently use a particular behavior, but they don't require that behavior to be documented.
Undefined behavior: ISO doesn't require any particular behavior, and they don't require implementations to define any particular behavior either.
[1]: https://github.com/lattera/glibc/blob/master/string/memcpy.c [2]: https://github.com/lattera/glibc/blob/895ef79e04a953cac14938...
> However, the most vocal opposition came from a static analysis perspective: Making null pointers well-defined for zero length means that static analyzers can no longer unconditionally report NULL being passed to functions like memcpy—they also need to take the length into account now.
How does this make any sense? We don't want to remove a low hanging footgun because static analyzers can no longer detect it?
No, it means the static analyzers can't report on a different error because a subset of that class of errors is no longer an error, and the static analysis can't usually distinguish between that subset and the rest.
memcpy(NULL, NULL, 0); // Formerly bad, now ok.
memcpy(NULL, NULL, s); // Formerly bad, now unknown (unless it can be proven that s != 0). memcpy(NULL, b, c); // Same issue.
Previously: warn if either address might be NULL.
Now: warn if either address might be NULL and the length might be nonzero, and prepare for your users to be annoyed and shut this warning off due to the false alarms.
Any useful static analysis tool does a careful balance between false positives and false negatives (aka false alarms and missed bugs). Too many false positives, and that warning will be disabled, or users will get used to ignoring it, or it will be routinely annotated away at call sites without anyone bothering to figure out whether it's valid or not. Soon the tool will cease to be useful and may be entirely abandoned. In actual practice, the sophistication of a static analysis tool is far less relevant than its precision. It's quite common to have an incredibly powerful static analysis tool that is used for only a small handful of blazingly obvious warnings, sometimes ones that the compiler already has implemented! (All the tool's fancy warnings got disabled one by one and nobody noticed.)
My understanding is that with this change, static analyzers have three options:
1. False positive on code that would have been an issue previously
2. False negative on a ton of similar footguns
3. Add complexity to differentiate between these cases
None of these options are fun.
Yes, but that tradeoff exists for most things those tools do. If you can easily and perfectly detect an error, it should just go into the compiler (and perhaps language spec).
Isn't it more sensible to just check that the params that are about to be sent to memcpy be reasonable?
That is why I tend to wrap my system calls with my own internal function (which can be inlined in certain PLs), where I can standardize such tests. Otherwise, the resulting code that performs the checks and does the requisite error handling is bloated.
Note that I am also loath to #DEFINE such code because C is already rife with them and my perspective is that the less of them the better.
At the end of the day, quick and dirty fixes will prove the adage "short cuts make long delays", and OpenBSD's approach is the only really viable long-term solution, where you just have to rewrite your code if it has ill-advised constructs.
For designing libraries such as C's stdlib, I don't believe in 'undefined behavior', clearly define your semantics and say, "If you pass a NULL to memcpy, this is what will happen." Same for providing a (n == 0), or should (src == dst).
And if, for some strange reason, fixing the semantics breaks calling code, then I can't imagine that their code wasn't f_cked in the first place.
> internal function
every time you introduce something nonstandard, you add one little hardship to anyone trying to read or modify your code.
if a programmer is familiar with the language, it's standard library, and the normal idioms, then they should be able to just jump in.
As the article points out, all major memcpy implementations already do this check inside memcpy. Sure, the caller can also check, but given that it's both redundant in practice and makes some common patterns harder to use than they would otherwise be, there's no reason to not just standardize what's already happening anyway and make everyone's lives easier in the process.
Only about 1000 more functions to do this to.
Well, that seems like something that should have been there from the beginning .
> because NULL + 0 is undefined behavior in C.
Why? It's 2024. Make it not be? Sure, some older stuff already written might no longer compile and need to be updated. Put it behind a "newer" standard flag/version or whatever.
Or is it that it can't be caught at compile time and only run time... hmm...
They are making it not be. That’s the whole point of the article.
>On the one hand, UB can be important for compiler optimizations
e.g?
Generally, undefined behavior removes the need for systematically checking for special cases, the most common being out of bounds access.
But it can go further than that. Dereferencing a NULL pointer is undefined behavior, so if a pointer is dereferenced, it can be assumed by the compiler not to be NULL and the code can be optimized. For example:
void foo(int *p) {
*p++;
if (p == NULL) {
printf("val is NULL\n");
} else {
printf("val is %d\n", *p);
}
}
void foo(int *p) {
*p++;
printf("val is %d\n", *p);
}
In the case of memcpy(NULL, NULL, 0), there probably isn't much to gain making it undefined. It most likely doesn't help with the memcpy implementation (len=0 is a generally no-op), and inference based on the fact that the arguments can't be NULL is more likely to screw the programmer up than to improve performance.
The simplest example of a compiler optimization enabled by UB would be the following:
int my_function() {
int x = 1;
another_function();
return x;
}
int my_function() {
another_function();
return 1;
}
And the most important example of a compiler optimization enabled by UB is related to that: being UB to access local variables through out-of-bounds pointers allows the compiler to place them in registers, instead of being forced to go through the stack for every operation.
This explanation of why signed int overflow is undefined is interesting (although the behaviour is still very annoying): https://kristerw.blogspot.com/2016/02/how-undefined-signed-o... (HN discussion: https://news.ycombinator.com/item?id=11146384)
More examples here: http://blog.llvm.org/2011/05/what-every-c-programmer-should-...
http://blog.llvm.org/2011/05/what-every-c-programmer-should-...
In a real world program removing all UB is some cases impossible without adding new breaking features to the C language. But, taking a real world program and removingh all UB which IS possible to remove will introduce an overhead. In some programs this overhead is irrelevant. In others, it is probably the reason why C was picked.
If you want speed without overhead, you need to have more statically checked guarantees. This is what languages such as Rust attempt to achieve (quite successfully).
The example in this blurb is a pretty good one: https://www.hboehm.info/c++mm/why_undef.html