Hacker News new | ask | show | jobs
by dccsillag 81 days ago
I'm sorry, but what exactly is the problem with the code? I've been staring at it for quite a while now and still don't see what is counterintuitive about it.
2 comments

There's nothing wrong with it. It does exactly what you think it does when passed null.
A lot of compilers will optimize out a NULL pointer check because dereferencing a NULL pointer is UB.

Because assert will not run the following code in the case of a NULL pointer, AFAIK this exact code is still defined behavior, but if for some reason some code dereferenced the NULL pointer before, it would be optimized out - there are some corner cases that aren't obvious on the surface.

This kind of thing was always theoretically allowed, but really started to become insidious within the past 5-10 years. It's probably one of the more surprising UB things that bites people in the field.

GCC has a flag "-fno-delete-null-pointer-checks" to specifically turn off this behavior.

https://qinsb.blogspot.com/2018/03/ub-will-delete-your-null-...

This is an actual Linux kernel exploit caused by this behavior where the compiler optimized out code that checked for a NULL pointer and returned an error.

https://lwn.net/Articles/342330/

Sure, but none of that is relevant to just the code snippet that was posted. The compiler can exploit UB in other code to do weird things, but that's just C being C. There's nothing unexpected in the snippet posted.

The issue is cause by C declaring that dereferencing a null pointer is UB. It's not really an issue with assertions.

You can get the same optimisation-removes-code for any UB.

> There's nothing unexpected in the snippet posted.

> The issue is cause by C declaring that dereferencing a null pointer is UB. It's not really an issue with assertions. > You can get the same optimisation-removes-code for any UB.

I disagree - It’s a 4 line toy example but in a 30-40 line function these things are not always clear. The actual problem is if you compile with NDEBUG=1, the nullptr check is removed and the optimiser can (and will, currently) do unexpected things.

The printf sample above is a good example of the side effects.

> The actual problem is if you compile with NDEBUG=1

That is entirely expected by any C programmer. Sure they named things wrong - it should have been something like `assert` (always enabled) and `debug_assert` (controlled by NDEBUG), as Rust did. And I have actually done that in my C++ code before.

But I don't think the mere fact that assertions can be disabled was the issue that was being alluded to.

I wrote the comment, assertions being disabled was exactly what was being alluded to.

> that is entirely expected by any C programmer

That’s great. Every C programmer also knows to avoid all the footguns and nasties - yet we still have issues like this come up all the time. I’ve worked as a C++ programmer for 12 years and I’d say it’s probably 50/50 in practice how many people would spot that in a code review.

Depends on where you're coming from, but some people would expect it to enforce that the pointer is non-null, then proceed. Which would actually give you a guaranteed crash in case it is null. But that's not what it does in C++, and I could see it not being entirely obvious.
Assert doesn't work like that in any language.
It does in Rust: assert is always enabled, whereas the debug-only version is called debug_assert.

But yes, “assert” in most languages is debug-only.

He said

> some people would expect it to enforce that the pointer is non-null, then proceed

No language magically makes the pointer non-null and then continues. I don't even know what that would mean.

If you don't even know what that would mean then it's premature to declare that nothing works that way. Understanding the meaning is a prerequisite for that.

In this case, it may help to understand that e.g. border control enforces a traveler's permission to cross the border, then lets them proceed.