Comment by geofft | Hacker News Reader

Comment by geofft | Hacker News Reader

Parent Full thread

> Well, what benefit does -fdelete-null-pointer-checks bring? Why would I ever want to remove that check? The more conservative version either segfaults or (quite likely) accidentally does the right thing. That situation is not great but it’s better than nothing.

My concern is largely about how to phrase "The compiler may not delete this dead code" in a way that's meaningful. For instance, I assume the intent of the proposal is not to forbid optimizing this code in the obvious way, right?

        int n = param->length, i;
    
        if (param == NULL) {
            return;
        }

        if (param == NULL) {
            return;
        }
    
        for (i = 0; i < n; i++) {
            do_something(param->data[i]);
        }

What about this?

        int n = param->length, i;
    
        for (i = 0; i < n; i++) {
            if (param == NULL) {
                return;
            }
            do_something(param->data[i]);
        }

which is more likely to show up in the form of an inline function that checks for NULL - can the compiler refactor that check into a single one?

> I think that one could also be usefully addressed by a compiler that flags it as a likely error.

The compiler can't know when compiling that one function whether src->len and dest->data alias. It might be compiling that file as a standalone object file, with no other context about who's calling it. Should it emit a warning when compiling that code? (What's the warning, "I'd like to optimize this but I can't?")

Alternatively, should it flag the function ... somehow ... so that when someone else calls it with aliasing arguments, that's an error? That isn't even a capability available to the C language as written: it's valid to call that function knowing nothing other than the struct definition and the function's prototype.

You can imagine a language where variables carried information about whether they alias. (Actually you don't really have to imagine; the `restrict` keyword does a subset of this in C itself, and other languages have much more explicit ways of tracking this, such as Rust's shared vs. mutable references.) But it would be hard to retrofit such a thing onto existing C code.

The fundamental problem with the C language working group is that they're obligated to keep working on C as it already exists.

One of the reasons for this proposal is that LTO is going to make C completely unusable otherwise. Suppose that a driver produces a structure that points to data buffers. Core code interacting with the drivers knows that drivers are of variable quality and checks that the structure is properly setup: if(x->buffer == 0)panic("bad driver" ) or something more sophisticated. Everything works beautifully until some driver is added with a dereference of x->buffer - which then defeats the guard in the core core because now the compiler " knows" that the x->buffer is not null. Separate compilation and inlining can't work reliably in this situation.

Are you suggesting that driver code has an opportunity to run before core code can implement checks?

If so, then again, this has the fundamental problem that you're expecting the compiler to implement some optimizations - like pushing the dereference out of the buggy driver via LTO - and not others. If the compiler had all optimizations off, it would just generate code to read from x->buffer in the obvious place, which would page-fault at runtime before the check from core code even happened.

If the core code implements the check first, then this optimization won't cause you problems, because this optimization is strictly about checking for a NULL pointer after using it.

For the specific case of computing the address of x->buffer without actually dereferencing it, I do agree that the compiler should not consider that to be an implicit non-null promise. That seems much more likely to be a change you can make without breaking the language spec. See the subthread with 'dbaupp.

(Alternatively, you could work on building tools for driver writers to write in a less error-prone language than C, because drivers being of poor quality is a much broader problem with much broader impacts than just triggering ill-advised optimizations under LTO, and simply optimizing drivers less efficiently isn't going to solve the problem that the driver shouldn't be dereferencing x->buffer without its own NULL check in the first place. If you check my recent GitHub activity you might notice a project to do exactly this in Linux....)

This is a really common code pattern: client sends request to server; server sanity checks request. If you omit sanity checks from the server because you assume there are no errors in the client, you break a lot of systems.

I am expecting the compiler to only make optimizations based on things it knows are true - that are true. So if the source code is "check for null;check for null; do something;" the compiler should be able to delete the second check (hopefully with a warning). In fact, the Standard is very explicit about permitting optimizing compilers to delete repeated operations. However, in the case I described, the compiler does not know that the pointer is non-null, it's just easier to assume it is not null. That leads to an "optimization" that is not an optimization. I agree that requiring compilers to only use true optimizations is more of a challenge than allowing them to use false ones, but nobody said it was going to be easy. In particular, it looks like Clang loses way too much information going from source to llvm language which makes it difficult to carry information about whether, for example, a pointer dereference is safe or not - but that's a defect in the compiler, not in the source language.

What's the warning, "I'd like to optimize this but I can't?"

Yes, that's exactly what I had in mind! :)

My instinctive response was going to be "That can't work, there are too many of them," but apparently you can do this particular one with -Wnull-dereference (which requires -fdelete-null-pointer-checks enabled to work, but you can just compile twice, once as a throwaway and once for production with -fno-delete-null-pointer-checks, or do the former for non-release builds, or something).

More generally, it might be the case that enough of the last several years of unpopular optimizations are controlled by flags that you can compile the code twice, once with the optimization enabled and once without, and warn whenever the optimization changes something. It might even be possible to do this with a shell script that treats the compiler as a black box with -ffunction-sections and uses objdump or something.

I do wonder how hard it is to write code that produces the same object code regardless of something more major like -ftrapv, though.

My view is that the compiler cannot remove any of these NULL checks. There should be a separate tool that gives the programmer hints that she is making unneeded checks and suggest program transformations, but removing NULL checks is unsafe when done automatically. Because I might be on a platform where param->length is actually referring to valid memory, so my program won't segfault and the NULL check is the only way to know.

The compiler can't remove repeated NULL checks and collapse them to a single one?? Why?

a) these are marginal optimizations, at best. b) because it violates C practice. My parsing library is written to distrust inputs so as to protect against buffer exploits. Your compiler "optimizes" away my security check because some buggy code dereferences a null pointer - which actually works in this implementation. Great. Now you have "optimized" to create a security hole in the program by removing a single instruction that is actually free in many pipelined architectures. Wonderful. Thanks so much.

> a) these are marginal optimizations, at best.

The optimization isn't written in the form of "If you see this ridiculous code, make it less ridiculous." It's written in the form of "You can refactor repeated checks of presumed-non-aliased variables into a single check." Otherwise code like

    for (i = 0; i < *plen; i++)

would involve dereferencing plen on every single loop, which for many applications is definitely not marginal!

> some buggy code dereferences a null pointer - which actually works in this implementation.

What implementation is this? (Are you mmapping at NULL, or writing for an architecture without an MMU, or something?)

NULL doesn't have to be address zero. It's denoted by literal zero, in the C source, but it can be something totally different when translated to the machine. Can you arrange for some other address to generate a segfault, and tell your C compiler to use that as NULL? (I don't know how to do this with gcc, and I'd like to know how, honestly - it seems like a significant win for kernel security if NULL pointers in kernel code were an invalid kernelspace address and not an invalid userspace address.)

Many C programs assume that initializing a variable to all-bits-zero, through memset, calloc, the .bss section, or on the kernel the GFP_ZERO flag, will initialize all pointers within it to NULL. See also: http://austingroupbugs.net/view.php?id=940 ("all 0 bits should be a null pointer on POSIX systems").

Not correct. g(x){ ... if x is null panic otherwise x->v = n} can currently be optimized to x->v =n given f(){ ... y =x->v ... g(x) } because null dereferences are assumed impossible in some twisted interpretation of the standard. Imagine the effects with inlining and LTO. That's my issue, not perfectly ordinary standard value caching etc.

Reply on news.ycombinator.com