HN

Comments (68)

• Menu

  nneonneo
  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!
• Menu

  panstromek
  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.
• MutableLambda
  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.
• Menu

  fizzynut
  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 sensesucceeded = false; error = false; //This makes no senseOtherwise 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.
• Menu

  yongjik
  I think UB doesn't have much to do with this bug after all.The original code defined a struct with two bools that were not initialized. Therefore, when you instantiate one, the initial values of the two bools could be anything. In particular, they could be both true.This is a bit like defining a local int and getting surprised that its initial value is not always zero. (Even if the compiler did nothing funny with UB, its initial value could be anything.)
• mac3n
  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.
• pornel
  To me the real horror is that the exact same syntax can be either a perfectly normal thing to do, or a horrible mistake that gives the compiler a license to kill, and this doesn't depend on something locally explicit, but on details of a definition that lives somewhere else and may have multiple layers of indirection.
• Menu

  vhantz
  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.
• Menu

  titzer
  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.
• kayo_20211030
  Great post. It was both funny and humble. Of course, it probably wasn't at all funny at the time.
• Menu

  inglor_cz
  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.