[HN Gopher] A production bug that made me care about undefined b...
___________________________________________________________________
A production bug that made me care about undefined behavior
Author : birdculture
Score : 76 points
Date : 2025-12-29 18:17 UTC (4 hours ago)
(HTM) web link (gaultier.github.io)
(TXT) w3m dump (gaultier.github.io)
| titzer wrote:
| tldr; the UB was reading uninitialized data in a struct. The C++
| rules for when default initialization occurs are crazy complex.
|
| I think a sanitizer probably would have caught this, but IMHO
| this is the language's fault.
|
| Hopefully future versions of C++ will mandate default
| initialization for all cases that are UB today and we can be free
| of this class of bug.
| trueismywork wrote:
| For now, best strategy is to initialize everything explicitly.
| torstenvl wrote:
| Yeah... but I wouldn't characterize the bug itself (in its
| essential form) as UB.
|
| Even if the implementation specified that the data would be
| indeterminate depending on what existed in that memory location
| previously, the bug would still exist.
|
| Even if you hand-coded this in assembly, the bug would still
| exist.
|
| The essence of the bug is uninitialized data being garbage.
| That's always gonna be a latent bug, regardless of whether the
| behavior is defined in an ISO standard.
| forrestthewoods wrote:
| Yeah I agree. This is a classic "uninitialized variable has
| garbage memory value" bug. But it is not a "undefined nasal
| demons behavior" bug.
|
| That said, we all learn this one! I spent like two weeks
| debugging a super rare desync bug in a multiplayer game with
| a P2P lockstep synchronous architecture.
|
| Suffice to say I am now a zealot about providing default
| values all the time. Thankfully it's a lot easier since C++11
| came out and lets you define default values at the
| declaration site!
| titzer wrote:
| I prefer language constructs define that new storage is
| zero-initialized. It doesn't prevent all bugs (i.e.
| application logic bugs) but at least gives deterministic
| results. These days it's zero cost for local variables and
| near-zero cost for fields. This is the case in Virgil.
| kevin_thibedeau wrote:
| C & C++ run on systems where it may not be zero cost. If
| you need low latency startup it could be a liability to
| zero out large chunks of memory.
| ablob wrote:
| I think it's acceptable to leave an escape hatch for
| these situations instead of leaving it to easy to
| misunderstand nooks and crannies of the standard.
|
| You don't want to zero out the memory? Slap a "foo =
| uninitialized" in there to have that exact behavior and
| get the _here be demons_ sign for free.
| forrestthewoods wrote:
| Yeah this issue is super obvious and non-controversial.
|
| Uninitialized state is totally fine as an opt-in
| performance optimization. But having a well defined non-
| garbage default value should _obviously_ be the default.
|
| Did C fuck that up 50 years ago? Yeah probably. They
| should have known better even then. But that's ok. It's a
| historical artifact. All languages are full of them. We
| learn and improve!
| 1718627440 wrote:
| I don't know, I expect all variables to be uninitialized
| until proven otherwise. It makes it easier for me to
| reason about code, especially convoluted code. But I also
| like C a lot and actually explicitly invoke UB quite
| often, so there is that.
| andrewaylett wrote:
| That makes things worse if all-zero is not a valid value
| for the datatype. I'd much prefer a set-up that requires
| you to initialise explicitly. Rust, for example, has a
| `Default` trait that you can implement if there _is_ a
| sensible default, which may well be all-zero. It also has
| a `MaybeUninit` holder which doesn 't do any
| initialisation, but needs an `unsafe` to extract the
| value once you've made sure it's OK. But if you don't
| have a suitable default, and don't want/need to use
| `unsafe`, you have to supply all the values.
| kayo_20211030 wrote:
| Great post. It was both funny and humble. Of course, it probably
| wasn't at all funny at the time.
| vhantz wrote:
| The two fields in the struct are expected to be false unless
| changed, then initialize them as such. Nothing is gained by
| leaving it to the compiler, and a lot is lost.
| gwd wrote:
| I think the point is that _sometimes_ variables are defined by
| the language spec as initialized to zero, and _sometimes_ they
| aren 't.
|
| Perhaps what you mean is, "Nothing is to be gained by relying
| on the language spec to initialize things to zero, and a lot is
| lost"; I'd agree with that.
| vhantz wrote:
| Please don't be pedantic. Compilers implement the standard,
| otherwise it's just a text document.
| hn_go_brrrrr wrote:
| Compilers implement the parts of the standard they agree
| with, in the way they think is best. They also implement it
| in the way _they_ understand the standardese.
|
| Read a complex enough project that's meant to be used
| across compiler venrdos and versions, and you'll find
| plenty of instances where they're working around the
| compiler not implementing the standard.
|
| Also, if you attended the standards committee, you would
| hear plenty of complaints from compiler vendors that
| certain things are implementable. Sometimes the committee
| listens and makes changes, other times they put their
| fingers in their ears and ignore reality.
|
| There are also plenty of places where the standard lets the
| compiler make it's own decision (implementation defined
| behavior). You need to know what your compiler vendor(s)
| chose to do.
|
| tl;dr: With a standard as complex as C++'s, the compilers
| very much do not just "implement the standard". Sometimes
| you can get away with pretending that, but others very much
| not.
| gwd wrote:
| Not trying to be pedantic. When I hear "leave it to the
| compiler", I normally think, "let the compiler optimize it,
| rather than optimizing it yourself". The compiler is doing
| the initialization either way, but in one case you're
| relying on a correct understanding of minutiae of the
| language spec (both for you and all future readers and
| writers of the code), in another case you're explicitly
| instructing the compiler to initialize it to zero.
| nneonneo wrote:
| Even calling uninitialized data "garbage" is misleading. You
| might expect that the compiler would just leave out some
| initialization code and compile the remaining code in the
| expected way, causing the values to be "whatever was in memory
| previously". But no - the compiler can (and absolutely will)
| optimize by assuming the values are whatever would be most
| convenient for optimization reasons, even if it would be
| vanishingly unlikely or even impossible.
|
| As an example, consider this code (godbolt:
| https://godbolt.org/z/TrMrYTKG9): struct foo {
| unsigned char a, b; }; foo make(int x) {
| foo result; if (x) { result.a = 13;
| } else { result.b = 37; }
| return result; }
|
| At high enough optimization levels, the function compiles to "mov
| eax, 9485; ret", which sets both a=13 and b=37 without testing
| the condition at all - as if both branches of the test were
| executed. This is perfectly reasonable because the lack of
| initialization means the values _could_ already have been set
| that way (even if unlikely), so the compiler just goes ahead and
| sets them that way. It's faster!
| arrowsmith wrote:
| How is this an "optimization" if the compiled result is
| incorrect? Why would you design a compiler that can produce
| errors?
| tehjoker wrote:
| It's not incorrect. Where is the flaw?
| Negitivefrags wrote:
| It's not incorrect.
|
| The code says that if x is true then a=13 and if it is false
| than b=37.
|
| This is the case. Its just that a=13 even if x is false. A
| thing that the code had nothing to say about, and so the
| compiler is free to do.
| foltik wrote:
| Ok, so you're saying it's "technically correct?"
|
| Practically speaking, I'd argue that a compiler assuming
| uninitialized stack or heap memory is always equal to some
| arbitrary convenient constant is obviously incorrect,
| actively harmful, and benefits no one.
| publicdebates wrote:
| In this example, the human author clearly intended mutual
| exclusivity in the condition branches, and this
| optimization would in fact destroy that assumption. That
| said, (a) human intentions are not evidence of foolproof
| programming logic, and often miscalculate state, and (b)
| the author could possibly catch most or all errors here
| when compiling without optimizations during debugging
| phase.
| foltik wrote:
| Regardless of intention, the code says this memory is
| uninitialized.
|
| I take issue with the compiler assuming anything about
| the contents of that memory; it should be a black box.
| masklinn wrote:
| The compiler is the arbiter of what's what (as long as it
| does not run afoul the CPU itself).
|
| The memory being uninitialised means reading it is
| illegal _for the writer of the program_. The compiler can
| write to it if that suits it, the program can't see the
| difference without UB.
|
| In fact the compiler can also read from it, because it
| knows that it has in fact initialised that memory. And
| the compiler is not writing a C program and is thus not
| bound by the strictures of the C abstract machine anyway.
| foltik wrote:
| Yes yes, the spec says compilers are free to do whatever
| they want. That doesn't mean they should.
|
| > The user didn't initialize this integer. Let's assume
| it's always 4 since that helps us optimize this division
| over here into a shift...
|
| This is convenient for who exactly? Why not just treat it
| as a black box memory load and not do further
| "optimizations"?
| masklinn wrote:
| > That doesn't mean they should.
|
| Nobody's stopping you from using non-optimising
| compilers, regardless of the strawmen you assert.
| 1718627440 wrote:
| Also even without UB, even for a naive translation, a
| could just happen to be 13 by chance, so the behaviour
| isn't even an example of nasal demons.
| throwatdem12311 wrote:
| Because a _could_ be 13 even if x is false because
| initialisation of the struct doesn't have defined behavior of
| what the initial values of a and b need to be.
|
| Same for b. If x is true, b _could_ be 37 no matter how
| unlikely that is.
| xboxnolifes wrote:
| It is not incorrect. The values are undefined, so the
| compiler is free to do whatever it want to do with them, even
| assign values to them.
| recursivecaveat wrote:
| Even the notion that uninitialized memory contain values is
| kind of dangerous. Once you access them you can't reason about
| what's going to happen at all. Behaviour can happen that's not
| self-consistent with any value at all:
| https://godbolt.org/z/adsP4sxMT
| masklinn wrote:
| Is that an old 'bot? because I noticed it was an old version
| of Clang, and I tried switching to the latest Clang which is
| hilarious: https://godbolt.org/z/fra6fWexM
| afiori wrote:
| This is gold
| qbane wrote:
| icc's result is interesting too
| nneonneo wrote:
| Oh yeah the classic Clang behaviour of "just stop codegen
| at UB". If you look at the assembly, the main function just
| _ends_ after the call to endl (right before where the if
| test should go); the program will run off the end of main
| and execute whatever nonsense is after it in memory as
| instructions. In this case I guess it calls main again (??)
| and then runs off into the woods and crashes.
|
| I've never understood this behaviour from clang. At least
| stick a trap at the end so the program aborts instead of
| just executing random instructions?
|
| The x and y values are funny too, because clang doesn't
| even bother loading anything into esi for
| operator<<(unsigned int), so you get whatever the previous
| call left behind in that register. This means there's no x
| or y variable at all, even though they're nominally being
| "printed out".
| recursivecaveat wrote:
| No I wrote it with the default choice of compiler just now.
| That newer result is truly crazy though lol.
| quietbritishjim wrote:
| If I understand it right, in principle the compiler doesn't
| even need to do that.
|
| It can just leave the result totally uninitialised. That's
| because both code paths have undefined behaviour: whichever of
| result.x or result.y is not set is still copied at "return
| result" which is undefined behaviour, so the overall function
| has undefined behaviour either way.
|
| It could even just replace the function body with abort(), or
| omit the implementation entirely (even the ret instruction,
| allowing execution to just fall through to whatever memory
| happens to follow). Whether any computer does that in practice
| is another matter.
| masklinn wrote:
| > It can just leave the result totally uninitialised. That's
| because both code paths have undefined behaviour: whichever
| of result.x or result.y is not set is still copied at "return
| result" which is undefined behaviour, so the overall function
| has undefined behaviour either way.
|
| That is incorrect, per the resolution of DR222 (partially
| initialized structures) at WG14:
|
| > This DR asks the question of whether or not struct
| assignment is well defined when the source of the assignment
| is a struct, some of whose members have not been given a
| value. There was consensus that this should be well defined
| because of common usage, including the standard-specified
| structure struct tm.
|
| As long as the caller doesn't read an uninitialised member,
| it's completely fine.
| masklinn wrote:
| Things can get even wonkier if the compiler keeps the values in
| registers, as two consecutive loads could use different
| registers based as you say on what's the most convenient for
| optimisation (register allocation, code density).
| jmgao wrote:
| There are some even funnier cases like this one:
| https://gcc.godbolt.org/z/cbscGf8ss
|
| The compiler sees that foo can only be assigned in one place
| (that isn't called locally, but could called from other object
| files linked into the program) and its address never escapes.
| Since dereferencing a null pointer is UB, it can legally assume
| that `*foo` is always 42 and optimizes out the variable
| entirely.
| publicdebates wrote:
| To those who are just as confused as me:
|
| Compilers can do whatever they want when they see UB, and
| accessing an unassigned and unassiganble (file-local)
| variable is UB, therefore the compiler can just decide that
| *foo is in fact _always_ 42, or never 42, or sometimes 42,
| and all would be just as valid options for the compiler.
|
| (I know I'm just restating the parent comment, but I had to
| think it through several times before understanding it
| myself, even after reading that.)
| masklinn wrote:
| Although it should be noted that that's not how compilers
| "reason".
|
| The way they work things out is to assume no UB happens
| (because otherwise your program is invalid and you would
| not request compiling an invalid program would you) then
| work from there.
| mac3n wrote:
| Many years had a customer complaint about undefined data changing
| value in Fortran 77. It turned out that the compiler never
| allocated storage for uninitialized variables, so it was aliased
| to something else.
|
| Compiler was changed to allocate storage for any referenced
| varibles.
| panstromek wrote:
| I have bumped into this myself, too. It's really annoying. The
| biggest footgun isn't even discussed explicitly and it might be
| how the error got introduced - it's when the struct goes from POD
| to non-POD or vice-versa, the rules change, so completely
| innocent change, like adding a string field, can suddenly create
| undefined behaviour in unrelated code that was correct
| previously.
| feelamee wrote:
| wow, can you elaborate how adding a string field can break some
| assumptions?
| nneonneo wrote:
| Not the OP, but note that adding a std::string to a POD type
| makes it non-POD. If you were doing something like using
| malloc() to make the struct (not recommended in C++!), then
| suddenly your std::string is uninitialized, and touching that
| object will be instant UB. Uninitialized primitives are
| benign unless read, but uninitialized objects are extremely
| dangerous.
| inglor_cz wrote:
| Symbian's way of avoiding this was to use a class called CBase to
| derive from. CBase would memset the entire allocated memory for
| the object to binary zeros, thus zeroizing any member variable.
|
| And by convention, all classes derived from CBase would start
| their name with C, so something like CHash or CRectangle.
| jenadine wrote:
| I'm afraid that's still not defined behaviour in many case. For
| example, pointer and bool can be initialized with `=0`, but
| that doesn't mean the binary representation in memory has to be
| 0, and so initializing with memset would still be wrong. (Even
| if it works with all compilers I know of.)
|
| Also, how does CBase knows the size of its allocated memory?
| MutableLambda wrote:
| Yeah, looks pretty straightforward to me, but I used to write C++
| for a living. I mean, there are complicated cases in C++ starting
| with C++11, this one is not really one of them. Just init the
| fields to false. Most of these cases is just C++ trying to bring
| in new features without breaking legacy code, it has become
| pretty difficult to keep up with it all.
| fizzynut wrote:
| Even if you fixed the initialized data problem, this code is
| still a bug waiting to happen. It should be a single bool in the
| struct to handle the state for the function as there are only two
| states that actually make sense.
|
| succeeded = true; error = true; //This makes no sense
|
| succeeded = false; error = false; //This makes no sense
|
| Otherwise if I'm checking a response, I am generally going to
| check just "succeeded" or "error" and miss one of the two above
| states that "shouldn't happen", or if I check both it's both a
| lot of awkward extra code and I'm left with trying to output an
| error for a state that again makes no sense.
| deepsun wrote:
| It happens often when "error" field is not a bool, but a
| string, aka error_message. Could be empty string, or _null_, or
| even _undefined_ if we're in JS.
|
| Then the obvious question why do we need _succeeded_ at all, if
| we can always check for _error_. Sometimes it can be useful,
| when the server doesn't know itself if the operation is
| succeeded (e.g. an IO/database operation timed out), so it
| might be succeeded, but should also show an error message to
| user.
|
| Another possibility if the succeeded is not a bool, but, say,
| "succeeded_at" timestamp. In general, I noticed that almost
| always any boolean value in database can be replaced with a
| timestamp or an error code.
___________________________________________________________________
(page generated 2025-12-29 23:00 UTC)