Google broke a conditional statement that verifies passwords on Chrome OS(arstechnica.com)
arstechnica.com
Google broke a conditional statement that verifies passwords on Chrome OS
https://arstechnica.com/gadgets/2021/07/google-pushed-a-one-character-typo-to-production-bricking-chrome-os-devices/
272 comments
Just reading the title it sounded very much like "Single Point of Failure: The (Fictional) Day Google Forgot To Check Passwords": https://www.youtube.com/watch?v=y4GB_NDU43Q
This actually happened for real with Dropbox about 10 years ago: https://techcrunch.com/2011/06/20/dropbox-security-bug-made-...
That exact news article appears in the video.
Fortunately (or not, depending if you have a Chromebook and store files locally there), the inverse has happened and it locked up your book (fail-deadly versus fail-safe).
> fail-deadly versus fail-safe
Speaking of... https://www.youtube.com/watch?v=TUKQfMFKfjk
Speaking of... https://www.youtube.com/watch?v=TUKQfMFKfjk
I love Tom's speculative fiction talks. He tells such a good story.
From the article:
> The line should read "if (key_data_.has_value() && !key_data_->label().empty()) {" but instead of "&&"—the C++ version of the "AND" operator—the bad update used a single ampersand, breaking the second half of the conditional statement.
Someone correct me if I'm wrong, but this seems like it wouldn't be possible in a language that didn't conflate boolean values with bitvectors.
Edit: I was wrong[1].
[1] https://news.ycombinator.com/item?id=27923785
> The line should read "if (key_data_.has_value() && !key_data_->label().empty()) {" but instead of "&&"—the C++ version of the "AND" operator—the bad update used a single ampersand, breaking the second half of the conditional statement.
Someone correct me if I'm wrong, but this seems like it wouldn't be possible in a language that didn't conflate boolean values with bitvectors.
Edit: I was wrong[1].
[1] https://news.ycombinator.com/item?id=27923785
You can use `and` and its siblings instead of && and similar in C++11.
Most people here, and in other boards, will try to convince you that it hurts readability because 'that‘s how we always did it' (read I‘m used to it and don‘t like change).
I was surprised to read that `and`, `and_eq`, `xor`, etc are all supported "secondary/alternative operators"
Never seen them used, but I use them in my C++ code when I have to write it to accomplish something else:
https://en.cppreference.com/w/cpp/language/operator_alternat...
Never seen them used, but I use them in my C++ code when I have to write it to accomplish something else:
https://en.cppreference.com/w/cpp/language/operator_alternat...
I used to use them. Then I was burned by some Very Opinionated managers and coworkers who didn't like seeing new things. It wasn't a hill I was willing to die on; there's more important things to argue about in C++.
They have been around since the first standard and basically only exist as a hack around an ancient non ascii compatible encoding. So unless you got burned in 1995 nothing about them was new.
They also aren't very consistent between C derived languages. Mostly because C# inherited its versions from VisualBasic, where And is a bitwise operator instead of a short circuiting one.
They also aren't very consistent between C derived languages. Mostly because C# inherited its versions from VisualBasic, where And is a bitwise operator instead of a short circuiting one.
> * So unless you got burned in 1995 nothing about them was new.*
I didn't mean to imply that they were new features to the language. I meant to imply that they were new style of coding to the manager & coworkers.
Some other features that have burned me from managers & coworkers is `using` to change the visibility of parent class members & functions and CRTP.
I didn't mean to imply that they were new features to the language. I meant to imply that they were new style of coding to the manager & coworkers.
Some other features that have burned me from managers & coworkers is `using` to change the visibility of parent class members & functions and CRTP.
Development is a massive cargo cult
I use them, some of them anyway. I find them to be more readable (in particular’not’ instead of ‘!’). Also it means that I can reserve ‘&&’ for rvalue references.
While it is true that you can use `and` in C++11, that's a bit of an understatement: these keywords have in fact been present since the very first ISO C++ standard C++98!
Note however that MSVC does not support these by default. You need to #include <ciso646> before their use, or you need to pass the `/permissive-` compiler option.
You should be using /permissive- anyway unless you are dealing with a legacy codebase that breaks with that.
TIL!
Always thought this was a C++11 feature.
Then this argument against the 'new' keywords is even weaker.
> that‘s how we always did it
... which, frankly, is a very good reason. Don't needlessly change something people are used to.
Why is the blinker control on the left side and the wiper control on the right side? Because that's what people expect.
... which, frankly, is a very good reason. Don't needlessly change something people are used to.
Why is the blinker control on the left side and the wiper control on the right side? Because that's what people expect.
On the other hand, if it were actually the case that people kept turning on the wipers instead of signaling their turns, it would be a sign that we should figure out how to make these two different operations not use symmetrical levers, and that it would be okay to change people's expectations because those expectations weren't very firm.
In aircraft, where it matters a whole lot more that you don't confuse the various levers, the handle of the landing gear lever is shaped like a little wheel and the handle of the flaps lever is shaped like a little wing edge: https://aviation.stackexchange.com/a/22689
Typing "and"/"bitand" seems like the same sort of thing. It's a minor change, but it prevents errors.
In aircraft, where it matters a whole lot more that you don't confuse the various levers, the handle of the landing gear lever is shaped like a little wheel and the handle of the flaps lever is shaped like a little wing edge: https://aviation.stackexchange.com/a/22689
Typing "and"/"bitand" seems like the same sort of thing. It's a minor change, but it prevents errors.
Wish it was consistent, though.
'josefx is right in saying[0] these are still primarily hacks around encoding issues.
[0] - https://news.ycombinator.com/item?id=27928276
'josefx is right in saying[0] these are still primarily hacks around encoding issues.
| meaning | what is | what should have been |
|---------+---------+-----------------------|
| && | and | and |
| &= | and_eq | bitand_eq |
| & | bitand | bitand |
| | | bitor | bitor |
| ~ | compl | bitcompl |
| ! | not | not |
| != | not_eq | not_eq |
| || | or | or |
| |= | or_eq | bitor_eq |
| ^ | xor | bitxor |
| ^= | xor_eq | bitxor_eq |
[0] - https://news.ycombinator.com/item?id=27928276
No it's because this way your left hand only deals with blinker, and your right one wih the gear shift. Separation of concerns. It's reversed in the UK of course.
The gear stick may be on the driver's left in the UK, but the indicator and windscreen wiper/wash stalks do not necessarily follow.
All my cars for the last ~15-20 years have had the indicator stalk on the left (i.e. same side as the gear stick), and the windscreen wiper/wash controls on the right, but I suspect that this might be a manufacturer-specific convention.
A quick bit of googled internet wisdom suggest Japanese brands tend to the right, European brands tend to the left, and EU standardisation has settled on left.
Strictly speaking, I would expect the indicator to be activated prior to commencing a manoeuvre, so would expect the two actions to not overlap.
All my cars for the last ~15-20 years have had the indicator stalk on the left (i.e. same side as the gear stick), and the windscreen wiper/wash controls on the right, but I suspect that this might be a manufacturer-specific convention.
A quick bit of googled internet wisdom suggest Japanese brands tend to the right, European brands tend to the left, and EU standardisation has settled on left.
Strictly speaking, I would expect the indicator to be activated prior to commencing a manoeuvre, so would expect the two actions to not overlap.
Also - super old reference:
https://news.ycombinator.com/item?id=768151
:)
https://news.ycombinator.com/item?id=768151
:)
Fyi it does in fact change depending on the car manufacturer.
> ... which, frankly, is a very good reason. Don't needlessly change something people are used to.
Unless this reason is causing bugs and security issues.
Unless this reason is causing bugs and security issues.
I am also of the opinion that `and` is more readable than `&&` (and isn't as easy to typo in a catastrophic way) - although my main point was about the weaker type system.
C++ definitely hasn't a weaker type system than "newer" languages like Java - if any, it is much more richer and complex than most languages out there. What's happening here is a type conversion that has to be in place due to C not having a boolean type until 1999. C++ attempts to construct a boolean from the argument of an `if()`, and given that bool can be constructed from int, the conversion succeedes.
You can define your own conversion operators to boolean, too, which are very useful for stuff like smart pointers and similar classes that may or may not have a value.
You can define your own conversion operators to boolean, too, which are very useful for stuff like smart pointers and similar classes that may or may not have a value.
struct A {
std::string value;
explicit operator bool() const {
return this->value.size();
}
};
// ...
A a {};
if (!a) {
// ...
}"Not as weak as Java" is not a very interesting benchmark, as Java also has a poor type system. C++'s type system is pitiable relative to those of Rust, Haskell, OCaml, and SML.
Moreover, in addition to being less expressive than them, C++'s type system is also weak, in formal sense, by allowing many implicit type conversions - which is one of the issues that I was complaining about. The fact that it "has to be in place due to C not having a boolean type until 1999" doesn't make it any less weak.
Moreover, in addition to being less expressive than them, C++'s type system is also weak, in formal sense, by allowing many implicit type conversions - which is one of the issues that I was complaining about. The fact that it "has to be in place due to C not having a boolean type until 1999" doesn't make it any less weak.
The fact implicit conversions exist doesn't really weaken the type system, the two are different concepts. If you define an overload or a type specialization on both the compiler will call the right version without any ambiguity. You can even define your own implicit conversions itself, and sometimes they have their uses, like when you need to wrap values in proxies but you still don't want the user to actually have to know that.
The template system and stuff like auto and such are crazy powerful. The Rust typesystem doesn't suffer from the C compatibility baggage and has been designed almost 30 years later, so it is amazing C++ can be so powerful and feel modern despite it being a forty years old language.
I write both Rust and C++ and I have to say, you can do a lot of type safe stuff in C++ too. Most patterns can be backported from Rust and while the ergonomics are not obviously at par, there is still a lot you can do. In C++you can do a crazy amount of metaintrospection at compile time that Rust can only do using procedural macros. Also constexpr and C++20's constinit and consteval are still more powerful than Rust's const.
The template system and stuff like auto and such are crazy powerful. The Rust typesystem doesn't suffer from the C compatibility baggage and has been designed almost 30 years later, so it is amazing C++ can be so powerful and feel modern despite it being a forty years old language.
I write both Rust and C++ and I have to say, you can do a lot of type safe stuff in C++ too. Most patterns can be backported from Rust and while the ergonomics are not obviously at par, there is still a lot you can do. In C++you can do a crazy amount of metaintrospection at compile time that Rust can only do using procedural macros. Also constexpr and C++20's constinit and consteval are still more powerful than Rust's const.
It is nothing to be amazed, other than we are finally able to enjoy such type systems in mainstream languages.
PL/I and Algol 68 were equally powerfull, or if you research into was being done in Xerox PARC workstations.
If anything, we are 30 years too late for where we could have been if it wasn't for UNIX winning the worstation market.
PL/I and Algol 68 were equally powerfull, or if you research into was being done in Xerox PARC workstations.
If anything, we are 30 years too late for where we could have been if it wasn't for UNIX winning the worstation market.
The type system in C++ is pretty strong actually, it just has too many implicit conversions. It's not like the compiler doesn't know that this value is not a bool.
Which of "Rust, Haskell (core Haskell), OCaml, and SML" is able to parametrize types on values, à la template<auto> ?
It only supports integers so far, which c++ could do pretty much since before it was even standardized 25 years ago. Doesn't seem to support as wide of a type menagerie as C++20's NTTP. Does it even support parametrization over function pointers ?
> It only supports integers so far
Integer-like things. C++ char is similar to Rust's u8 or i8 but Rust's char and bool are very deliberately not just integers.
> Does it even support parametrization over function pointers ?
Can you give a clear example ? I think the answer is "No" but I struggled to put together an application for the feature I'm imagining.
You would like to define a type, which is parameterised not by the type of function, but by specific functions? So, for example I can make a foo<sum> and a foo<average> and those are distinct types which presumably internally are using the provided function to behave differently?
Except all the examples I think of come out better with just parametrisation over traits instead. So a clear example from you might illuminate whether this is a sizeable hole or just a difference of philosophy.
Integer-like things. C++ char is similar to Rust's u8 or i8 but Rust's char and bool are very deliberately not just integers.
> Does it even support parametrization over function pointers ?
Can you give a clear example ? I think the answer is "No" but I struggled to put together an application for the feature I'm imagining.
You would like to define a type, which is parameterised not by the type of function, but by specific functions? So, for example I can make a foo<sum> and a foo<average> and those are distinct types which presumably internally are using the provided function to behave differently?
Except all the examples I think of come out better with just parametrisation over traits instead. So a clear example from you might illuminate whether this is a sizeable hole or just a difference of philosophy.
> Integer-like things. C++ char is similar to Rust's u8 or i8 but Rust's char and bool are very deliberately not just integers.
sure, but there's an easy mapping from char / bool to integers. C++ supports parametrizing on values of struct type, which is a clear increase in expressive power:
yes, it's pretty much the only way in C++ to have zero-cost callbacks / strategy pattern using function pointers since the compiler can directly inline the function pointer everywhere in the type's implementation instead of having to go and read it from some variable at run-time
sure, but there's an easy mapping from char / bool to integers. C++ supports parametrizing on values of struct type, which is a clear increase in expressive power:
struct foo {
int count;
float init;
};
template<auto arg> struct bar {
float x[arg.count];
bar() {
for(int i = 0; i < arg.count; i++)
x[i] = arg.init;
}
};
constexpr foo f{ .count = 123, .init = 4.56 };
bar<f> b;
yes, it's pretty much the only way in C++ to have zero-cost callbacks / strategy pattern using function pointers since the compiler can directly inline the function pointer everywhere in the type's implementation instead of having to go and read it from some variable at run-time
You're correct to observe that more sophisticated value parametrisation might be useful. At least &str (imagine roughly a nicer std::string_view as a built-in type) is on the horizon for Rust.
Because Rust cares a lot about soundness, this gets very tricky for user-defined types. The current idea is that they could be allowed if they derive Eq. What "derive Eq" means in practice is that the compiler is comfortable believing that either you can compare instances of these types to one another with a bit-match or they're composed of types with that property.
You wouldn't be allowed to implement Eq instead for this, because such implementations are safe in Rust, and thus their promises are worthless (Rust has unsafe traits like Allocator, and if you choose to implement those and get it wrong your program has Undefined Behaviour, but by definition that won't happen for a safe trait, even if you deliberately implement it contrary to the specification)
So that rules out floating point numbers because you shouldn't try to compare those to each other with bit matching - NaNs for example.
What does C++ do here? Just YOLO, if you make poor choices the resulting code is nonsense and too bad?
--
We still don't have an example, the constant function pointer thing feels unergonomic to me, I think I'd cook up a Trait representing whatever it is that these functions have in common, and then implement that Trait as necessary to get the same effect by parametrising on the Trait -- but I may very well be missing some affordance your preferred approach has since the equivalent C++ is just to use a Class and finalize the implementations yet you aren't doing that.
Because Rust cares a lot about soundness, this gets very tricky for user-defined types. The current idea is that they could be allowed if they derive Eq. What "derive Eq" means in practice is that the compiler is comfortable believing that either you can compare instances of these types to one another with a bit-match or they're composed of types with that property.
You wouldn't be allowed to implement Eq instead for this, because such implementations are safe in Rust, and thus their promises are worthless (Rust has unsafe traits like Allocator, and if you choose to implement those and get it wrong your program has Undefined Behaviour, but by definition that won't happen for a safe trait, even if you deliberately implement it contrary to the specification)
So that rules out floating point numbers because you shouldn't try to compare those to each other with bit matching - NaNs for example.
What does C++ do here? Just YOLO, if you make poor choices the resulting code is nonsense and too bad?
--
We still don't have an example, the constant function pointer thing feels unergonomic to me, I think I'd cook up a Trait representing whatever it is that these functions have in common, and then implement that Trait as necessary to get the same effect by parametrising on the Trait -- but I may very well be missing some affordance your preferred approach has since the equivalent C++ is just to use a Class and finalize the implementations yet you aren't doing that.
In extremely limited form. That should change in the future, though.
Much more usefully, in C++ you can also parametrize over templates, which I think you still can't in rust.
You can't parametrize over namespaces (at least not directly) which is an annoying and arbitrary restriction.
You can't parametrize over namespaces (at least not directly) which is an annoying and arbitrary restriction.
> You can't parametrize over namespaces (at least not directly) which is an annoying and arbitrary restriction.
It's not a restriction, namespaces are neither types nor values so they would need specific support. Given that you can (ab)use classes with static members as namespaces which are also a type it's simply that nobody cared enough to add support for templating over real namespaces.
It's not a restriction, namespaces are neither types nor values so they would need specific support. Given that you can (ab)use classes with static members as namespaces which are also a type it's simply that nobody cared enough to add support for templating over real namespaces.
You are right it is not a restriction in the sense that is explicitly disallowed; what I meant is that it would take very little to add support for parametrizing over namespaces.
Stateless structures are a workaround (so is adl driven by a template parameter), still direct support would be nice.
Stateless structures are a workaround (so is adl driven by a template parameter), still direct support would be nice.
For those languages, I don't really see a need to paremtrize types on values. Because of the AMAZING generics support.
But that may just be the blub paradox [1] in action
[1] https://wiki.c2.com/?BlubParadox
But that may just be the blub paradox [1] in action
[1] https://wiki.c2.com/?BlubParadox
Well the fact you couldn't reasonably use arrays without Generic Const hit stable in Rust 1.51 says otherwise.
Defining templates on values is very useful, especially in C++ where you can provide template specializations. You can do a whole lot of metaprogramming and compile-time stuff that way.
Defining templates on values is very useful, especially in C++ where you can provide template specializations. You can do a whole lot of metaprogramming and compile-time stuff that way.
I don't think weak has a formal sense in any formal sense.
Java has a poor type system, but not as poor as that of C++.
This is uttely false. I desume you haven't used C++ in the last 10 years, or at least you didn't delve deep enough into it to really understand how powerful (while bonkers) the C++ type system is.
C++ has a much, MUCH more stronger type system with true generics, value types, const-correctness, compile time reflection and dispatching, ...
In Java, everything is a reference, except when it's not (which is a design mistake that .NET fixed, IMHO). Some stuff in Java is plain "magic", like type erasure and boxing, while C++ might well be drowning in its own sea of utter madness but at least tries to be somewhat consistent (for instance, there are no "magic" types, when you do `int { 3U }` you are "constructing" an integer, when you do `bool x { 33 }` you applying the implicitly defined `bool(int)` constructor from bool. You can define your own conversions, and you can define your own custom types that behave and can be used like built-in ones (see smart pointers, iterators, ...).
Java _seems_ stronger typed because it generally doesn't allow integer promotions and implicit conversions, but these are concepts that are orthogonal to the type system, `bool` and `char` are different, distinct types and if you specialize a template for T<bool>, it won't apply to `char` unless a conversion happens, and if it does so, it is still operating on `bool`, not char - it is constructing a type from another, the fact this happens is simply hidden from you, like Java and boxing (which ironically is an implicit conversion).
The Java delegates pretty much everything to the JVM, and that's reflected in the language design. Java is a simple language that does not do a lot at compile time, relying on runtime facilities to mitigate these shortcomings. See for instance how everything can always decay to a reference to Object, implicitly, everywhere, requiring casts (i.e. runtime assertions) to restore type safety - that's basically a safer `void*`.
1995's Java was clearly too limited, I understand they wanted a fresh start from the ugliness of '90s C++, but they straight removed too much for the sake of simplicity. The current crop of languages, which largely rejected the Java model is kind of a symbol of what went wrong with Java, IMHO.
The fact certain features had been "hacked" on top of the language using what was already there (see generics, boxing, ...), often introducing features that act like "magic", and can't be overridden by the user is bad, and shows how limited the original language was. The same way you can't override operators, you can't define custom boxing rules for your types (mostly because you won't be able to define custom value types until Valhalla is released).
Modern C++ allows you to write safe and solid code using compile-time features and the type system. While stuff like <type_traits>, SFINAE, template metaprogramming and such are definitely not "nice", they are extremely powerful and if used correctly eliminate completely certain issues from ever happening. If you only use smart pointers, references, containers, moves and by-value semantics you won't get crashes from nulls, ever. You won't have memory leaks, and after lots of fighting with the compiler, there's a high chance your code will work straight away (unless you messed up the logic). This is not that far from Rust, as far as my experience goes - I still hope for Rust to mostly replace C++ in the end, but for now C++20 is a very solid substitute and a good choice (and feels much more modern and powerful than Java could or has ever been).
C++ has a much, MUCH more stronger type system with true generics, value types, const-correctness, compile time reflection and dispatching, ...
In Java, everything is a reference, except when it's not (which is a design mistake that .NET fixed, IMHO). Some stuff in Java is plain "magic", like type erasure and boxing, while C++ might well be drowning in its own sea of utter madness but at least tries to be somewhat consistent (for instance, there are no "magic" types, when you do `int { 3U }` you are "constructing" an integer, when you do `bool x { 33 }` you applying the implicitly defined `bool(int)` constructor from bool. You can define your own conversions, and you can define your own custom types that behave and can be used like built-in ones (see smart pointers, iterators, ...).
Java _seems_ stronger typed because it generally doesn't allow integer promotions and implicit conversions, but these are concepts that are orthogonal to the type system, `bool` and `char` are different, distinct types and if you specialize a template for T<bool>, it won't apply to `char` unless a conversion happens, and if it does so, it is still operating on `bool`, not char - it is constructing a type from another, the fact this happens is simply hidden from you, like Java and boxing (which ironically is an implicit conversion).
The Java delegates pretty much everything to the JVM, and that's reflected in the language design. Java is a simple language that does not do a lot at compile time, relying on runtime facilities to mitigate these shortcomings. See for instance how everything can always decay to a reference to Object, implicitly, everywhere, requiring casts (i.e. runtime assertions) to restore type safety - that's basically a safer `void*`.
1995's Java was clearly too limited, I understand they wanted a fresh start from the ugliness of '90s C++, but they straight removed too much for the sake of simplicity. The current crop of languages, which largely rejected the Java model is kind of a symbol of what went wrong with Java, IMHO.
The fact certain features had been "hacked" on top of the language using what was already there (see generics, boxing, ...), often introducing features that act like "magic", and can't be overridden by the user is bad, and shows how limited the original language was. The same way you can't override operators, you can't define custom boxing rules for your types (mostly because you won't be able to define custom value types until Valhalla is released).
Modern C++ allows you to write safe and solid code using compile-time features and the type system. While stuff like <type_traits>, SFINAE, template metaprogramming and such are definitely not "nice", they are extremely powerful and if used correctly eliminate completely certain issues from ever happening. If you only use smart pointers, references, containers, moves and by-value semantics you won't get crashes from nulls, ever. You won't have memory leaks, and after lots of fighting with the compiler, there's a high chance your code will work straight away (unless you messed up the logic). This is not that far from Rust, as far as my experience goes - I still hope for Rust to mostly replace C++ in the end, but for now C++20 is a very solid substitute and a good choice (and feels much more modern and powerful than Java could or has ever been).
All versions of C++ inherit type system from C and keep it for backward compatibility. Indeed you can opt into a stronger dialect with right compiler flags, but this is used as widely as Prolog, because the rest of ecosystem uses an incompatible dialect and will fight against you. C++20 is a substitute of C++17, not of other languages.
The desire of maintaining compatibility with C at all cost is both the main selling point of C++ and the source of almost all of its madness. C was full of weird quirks (null terminated strings, enums that decayed to int, char* that could represent arbitrary data, weird syntax inherited from B, nonsensical arrays, ...) but its simplicity has always kinda kept everything down to a tameable level of chaos.
C++ took this quirky base and pushed it (abused it?) in ways that go way beyond what can be considered "sane"; see for instance the bajillion ways operator new() can be redefined, the fact there are at least 10 ways to initialize something, or the fact that features can be _discovered_ and not designed (like CRTP). Still, this wealth of features together with the unwavering (and arguably masochistic) Herculean efforts of the ISO C++ committee to modernize the language make it, by far, one of the most flexible languages around. Every single time I use C++ I discover a new amazing way to do something that had never crossed my mind; call me crazy, but I think this makes programming fun, a detail that is often (wrongly) neglected.
C++ took this quirky base and pushed it (abused it?) in ways that go way beyond what can be considered "sane"; see for instance the bajillion ways operator new() can be redefined, the fact there are at least 10 ways to initialize something, or the fact that features can be _discovered_ and not designed (like CRTP). Still, this wealth of features together with the unwavering (and arguably masochistic) Herculean efforts of the ISO C++ committee to modernize the language make it, by far, one of the most flexible languages around. Every single time I use C++ I discover a new amazing way to do something that had never crossed my mind; call me crazy, but I think this makes programming fun, a detail that is often (wrongly) neglected.
I just wish there would be a -fsafe compiler flag instead of having to deal with all the sanitizers and debug mode flags, but it is what it is.
[deleted]
you can even use it for rvalues!
Forgive my poor C++, but has_value() sounds like a bool, and empty() sounds like a bool. Why wouldn't "bool1 & bool2" work correctly?
Dereferencing via key_label_-> has undefined behavior when !key_label_.has_value(), so the compiler treats that case the same as if key_label_.has_value(). Or to put it another way, the compiler reasons that key_label_ must have had a value (since you dereferenced it), and thus optimizes out the check for `has_value()` (since clearly it was unnecessary).
I don't think that's right. It seems far-fetched that the compiler would know that accessing key_data->label() implies that k_d->has_value() == true
I believe the explanation is:
I believe the explanation is:
A && B : compiler will evaluate B only if A==true
A & B : compiler will evaluate both A (safe) and B (unsafe), then perform bitwise operationNo, it's not far-fetched, this is a pretty well-known optimization.
https://blog.regehr.org/archives/970
https://blog.regehr.org/archives/970
Ahhh, so it was a problem with lazy evaluation (and the lack thereof when using &), and not type coercion, like I initially thought.
Yeah it has nothing to do with type coercion. I'm baffled a linter doesn't catch this though. & for bools should just be prohibited altogether. If people really want & for bool, they can just cast to an integer type and use & with that (which generates its own warnings appropriately).
the problem is that bitwise & and | are routinely used for their non-shortcircuiting behaviour in boolean expressions, so it is hard for the compiler to flag this as an error.
Many linters and static analysis tools do flag them though as these days it is not considered good practice.
Many linters and static analysis tools do flag them though as these days it is not considered good practice.
Why would you even need non-shortcircuiting behavior in boolean expressions? Because either side of the operator has side effects that you want to happen unconditionally? Please just write it out as an extra statement then instead of hammering it into place with an implicit coercion to an integer type only so you can abuse bitwise AND and OR only to force that side effect to... you can see why this is probably not the most elegant idea.
What I would like to write in unit tests:
I really do wonder why the standard doesn't just specify the behavior of bool & bool. Is it just because of holdover from C?
for (auto x : y)
ok &= check(x);
for (auto x : y)
ok = ok && check(x);
for (auto x : y)
ok = check(x) && ok;
I really do wonder why the standard doesn't just specify the behavior of bool & bool. Is it just because of holdover from C?
> I really do wonder why the standard doesn't just specify the behavior of bool & bool.
Confused about your question. Did you have a typo? bool & bool is valid in C++.
Confused about your question. Did you have a typo? bool & bool is valid in C++.
I'm guessing the question is related to this complaint:
> What I would like to write in unit tests:
> ok &= check(x);
> That gets you angry messages from static analysis
> What I would like to write in unit tests:
> ok &= check(x);
> That gets you angry messages from static analysis
That has nothing to do with the standard though? The behavior is well-defined, it's just compilers warning about use of & tending to be error-prone (rightly or wrongly).
I lost track how many times I have been bitten by the ok = ok && ... trap.
Matt Godbolt gave an example of a significant performance hit caused by unnecessary short-circuiting in his CppCon 2019 talk[1]. EDIT: Deleted tangent.
[1]: https://youtu.be/HG6c4Kwbv4I?t=45m
[1]: https://youtu.be/HG6c4Kwbv4I?t=45m
This example is interesting enough that it deserves to be written up for convenience. The reason there is a performance hit is that the example involves a lot of mispredicted branches.
The setup is that we have a large number of tiny triangles, we cast a ray in a random direction, and we want to detect whether it intersects any of the triangles. The summarized code looks like this:
We calculate the (x,y) coordinates of the point where our ray intersects the plane in which the triangle lies.
We convert those (x,y) coordinates to (u,v) coordinates, where the vectors u and v are parallel to two sides of the triangle. (And equal in length.)
In our transformed (u,v) space, determining whether a point lies inside the triangle is very easy. With the origin at the corner of the triangle from which the u and v sides emanate, a point is out of bounds if its u-coordinate lies outside the interval [0, 1], or if its v-coordinate lies outside [0, 1-u]. That's what the ifs after calcU and calcV are checking. When we detect that a point is out of bounds, we move on to the next triangle.
The triangles are small and the ray is random, so the intersection of the ray with the plane will strike at a random point. It is almost always the case that the point will lie outside the triangle. This means that the full conditional (u < 0 || u > 1) will almost always be false.
But the two subconditions u < 0 and u > 1 will each be true 50% of the time. They are individually impossible to predict, which causes branch prediction on the first one, u < 0, to fail about 50% of the time. But they are jointly easy to predict. There is a huge performance gain from switching to (u < 0 | u > 1) -- in this case, 50% of the time we will do the work of calculating whether u > 1 even though we didn't have to. But the reward we get for that extra work is that branch prediction drops from a 50% failure rate (actually 45%) to a very low failure rate.
Including the v-coordinate makes the full check, ((u < 0) | (u > 1) | (v < 0) | (u + v > 1)), even more easy to predict. We've decided to guarantee that we will always do the full amount of work, and most of it is unnecessary. But it's easier to do 2x or more the amount of work and test a condition that fails consistently than to do less work and keep having to clear the instruction pipeline.
The setup is that we have a large number of tiny triangles, we cast a ray in a random direction, and we want to detect whether it intersects any of the triangles. The summarized code looks like this:
for (/* all triangles */) {
auto u = calcU(/*...*/);
if (u < 0 || u > 1) {
continue;
}
auto v = calcV(/*...*/);
if (v < 0 || u + v > 1) {
continue;
}
auto dist = calcD(/*...*/);
if (dist < nearest) {
nearest = dist;
}
}
We calculate the (x,y) coordinates of the point where our ray intersects the plane in which the triangle lies.
We convert those (x,y) coordinates to (u,v) coordinates, where the vectors u and v are parallel to two sides of the triangle. (And equal in length.)
In our transformed (u,v) space, determining whether a point lies inside the triangle is very easy. With the origin at the corner of the triangle from which the u and v sides emanate, a point is out of bounds if its u-coordinate lies outside the interval [0, 1], or if its v-coordinate lies outside [0, 1-u]. That's what the ifs after calcU and calcV are checking. When we detect that a point is out of bounds, we move on to the next triangle.
The triangles are small and the ray is random, so the intersection of the ray with the plane will strike at a random point. It is almost always the case that the point will lie outside the triangle. This means that the full conditional (u < 0 || u > 1) will almost always be false.
But the two subconditions u < 0 and u > 1 will each be true 50% of the time. They are individually impossible to predict, which causes branch prediction on the first one, u < 0, to fail about 50% of the time. But they are jointly easy to predict. There is a huge performance gain from switching to (u < 0 | u > 1) -- in this case, 50% of the time we will do the work of calculating whether u > 1 even though we didn't have to. But the reward we get for that extra work is that branch prediction drops from a 50% failure rate (actually 45%) to a very low failure rate.
Including the v-coordinate makes the full check, ((u < 0) | (u > 1) | (v < 0) | (u + v > 1)), even more easy to predict. We've decided to guarantee that we will always do the full amount of work, and most of it is unnecessary. But it's easier to do 2x or more the amount of work and test a condition that fails consistently than to do less work and keep having to clear the instruction pipeline.
> Why would you even need non-shortcircuiting behavior in boolean expressions? Because either side of the operator has side effects that you want to happen unconditionally?
Interestingly, the example provided by slavik81 is able to tolerate non-shortcircuiting behavior specifically because no side effects are involved. It's more of a case of "we don't care about the result of this work, but we have to do it anyway, because it's better to do a lot of extra work concurrently than to do only the necessary work sequentially." And doing the extra work is OK because it doesn't have any side effects.
If side effects were involved, you'd be stuck needing to short-circuit despite the fact that it's slower.
There are also areas in cybersecurity where you'd like the time taken for some operation to be insensitive to the input; short-circuiting is bad there too.
Interestingly, the example provided by slavik81 is able to tolerate non-shortcircuiting behavior specifically because no side effects are involved. It's more of a case of "we don't care about the result of this work, but we have to do it anyway, because it's better to do a lot of extra work concurrently than to do only the necessary work sequentially." And doing the extra work is OK because it doesn't have any side effects.
If side effects were involved, you'd be stuck needing to short-circuit despite the fact that it's slower.
There are also areas in cybersecurity where you'd like the time taken for some operation to be insensitive to the input; short-circuiting is bad there too.
It's usually wrong to assume that expressions that ultimately have the same effect (such as a || b and a | b when both a and b are simple booleans, so no side effects) will make the compiler generate different code depending on which variant to use.
The compiler knows that both variants resolve to the same result with no side effects, so it should try to generate whatever code is more efficient. Depending on several factors, that may be machine code that effectively exhibits short circuit behavior or not, for either expression (so even a|b might end up having a short circuit on the lowest level).
This would be different if you, e.g., sprinkle some volatile keywords (which effectively introduces side effects), but then you'd probably still want to take more care and split the expressions up, unless you really don't care about the order of your forced memory accesses (i.e. if I recall correctly, there is no guarantee that a|b will access a first, then b). And that's not even going into barriers.
The compiler knows that both variants resolve to the same result with no side effects, so it should try to generate whatever code is more efficient. Depending on several factors, that may be machine code that effectively exhibits short circuit behavior or not, for either expression (so even a|b might end up having a short circuit on the lowest level).
This would be different if you, e.g., sprinkle some volatile keywords (which effectively introduces side effects), but then you'd probably still want to take more care and split the expressions up, unless you really don't care about the order of your forced memory accesses (i.e. if I recall correctly, there is no guarantee that a|b will access a first, then b). And that's not even going into barriers.
Reduced unit test complexity? Short circuiting implies more branches, more branches means more tests for 100% coverage.
Of course for that to be a downside someone would have to write tests for security critical code, which obviously did not happen.
Of course for that to be a downside someone would have to write tests for security critical code, which obviously did not happen.
That is just gaming your coverage stats, there are still the different branches they just aren't visible now.
That's not true.
Compare `( a() || b() || c() ) ? yes() : no()` to `( a() | b() | c() ) ? yes() : no()`.
In the second case, there are two paths through the code:
Compare `( a() || b() || c() ) ? yes() : no()` to `( a() | b() | c() ) ? yes() : no()`.
In the second case, there are two paths through the code:
a()
b()
c()
yes()
a()
b()
c()
no()
a() /* true */
yes()
a() /* false */
b() /* true */
yes()
a() /* false */
b() /* false */
c() /* true */
yes()
a() /* false */
b() /* false */
c() /* false */
no()
So, it tests that you're running yes() and no(). But it doesn't test that your choice to use 'a()|b()|c()' for your condition was correct, instead of, for example, 'a()&b()&c()' or even '(a()&b())|c()' because you didn't test all the combinations. Using bitwise instead of logical operations hides this in your branch coverage report.
You are not attempting to have branch coverage; you are attempting to cover the space of possible inputs.
The code is correct if you hit yes() and no() correctly depending on three conditions. There are 8 possibilities in both cases and thus you need 8 tests in both cases. Shortcircuiting is optimization, it doesn't reduce complexity of the logic.
That is a concern, but it's not one related to how many branches exist in the code.
"How did we get here?" and "Are we in the right place?" are distinct questions.
"How did we get here?" and "Are we in the right place?" are distinct questions.
When you check for error conditions shortcircuiting is unlikely and thus useless.
if(error1|error2)abort();
if(error1|error2)abort();
But it semantically resolves to the exact same, and so the compiler will likely treat it the same and optimize for whatever makes most sense. This may both make a written "a|b" short-circuiting, and a written "a||b" non-short-circuiting, as long as the result stays the same (which for simple bool a,b it always does).
> & and | are routinely used for their non-shortcircuiting behaviour in boolean expressions
Really? I can't recall a single case of this with 'bool' in any codebase in my recent memory. I only see it with integers. And even if it's somehow routine for your codebases with bool (why?!), surely it's not routine inside a conditional expression, so at least they can prohibit it there?
Really? I can't recall a single case of this with 'bool' in any codebase in my recent memory. I only see it with integers. And even if it's somehow routine for your codebases with bool (why?!), surely it's not routine inside a conditional expression, so at least they can prohibit it there?
Whenever I see it I assume it's a typo. If it was truly intentional I would expect a comment pointing out why it's needed.
I think this is a unpopular opinion, but I consider taking advantage of short-circuiting in boolean operations to be a code smell. At least for me, it seems very fragile and I'd much rather break the conditional logic out explicitly.
But I like to write as much like assembly as I can in every language. Each line of code should do one and only one thing.
But I like to write as much like assembly as I can in every language. Each line of code should do one and only one thing.
Short circuiting boolean operators are very useful, and not something I would consider a code smell. They let constructs like this work, which are very common in most codebases:
if (foo && foo->bar) {
// whatever
}
If you're going for "each line of code should do exactly one thing", you'd probably prefer that as
Short-circuit 'or' is a little harder to avoid (if you specifically want the short-circuit behavior), since you'd have to duplicate the code in the body of the if. But that doesn't come up as often IME.
if(foo) {
if(foo->bar) {
//whatever
}
}
Short-circuit 'or' is a little harder to avoid (if you specifically want the short-circuit behavior), since you'd have to duplicate the code in the body of the if. But that doesn't come up as often IME.
Yep, I'd prefer that option, despite its wordiness. Obviously this depends a lot on individual preferences, and in this simple example it really doesn't make a lot of difference.
I imagine the compiler spits out pretty similar code either way. It's more just a recognition of my own limitations... I'm a lot less likely to screw up something up, the more explicitly it's written out.
When I was a younger man, I wouldn't have bothered.
I imagine the compiler spits out pretty similar code either way. It's more just a recognition of my own limitations... I'm a lot less likely to screw up something up, the more explicitly it's written out.
When I was a younger man, I wouldn't have bothered.
The problem with that code and the reason that it is often considered bad C++ practice is that it is very easy to accidentally move the inner conditional outside of the outer conditional, thus creating undefined behaviour.
Putting the null check in the same line as the usage means that it is much harder to separate them accidentally.
Putting the null check in the same line as the usage means that it is much harder to separate them accidentally.
That technique doesn't work (as well) when you want a single "else" block that is used when either of the conditions is false, which I think is pretty common for this sort of conditional
I used to only put short-circuits when I really needed short-circuit behavior, thinking it improved readability. But after getting by the implicit type conversions over and over (as well as nags from every compiler) I just decided it was a bad idea in C++. I'd still do it in a safer language, but in C++ I'm convinced you should just use && even when you only need & for bools.
So instead of short circuiting like that code should when it uses &&, it instead makes an assumption about the first part of the boolean expression because otherwise the code would have undefined behavior?
Yeah. More specifically, it just inserts whatever behavior it likes for the undefined scenario, which in this case is "the same behavior as in the defined scenario".
I am utterly confused. How would the compiler know that has_value() guarantees that the ->label dereferencing work, since this is a bitwise operation ?
I would imagine has_value is checking to see if a pointer is NULL, and -> is dereferencing that same pointer. When the optimizer sees the two together it realizes the former is useless.
Would this optimization not have been on the beta/canary channel? I assumed they were but perhaps not since this was not caught.
I don't know how their builds work or where their templates are instantiated but it's possible that the compiler doesn't see this optimization until LTO and that they don't do LTO until stable releases? It would seem strange though, I'd have thought beta would have LTO.
Yeah that would be a very weird choice. LTO itself can have bugs, or (as we speculate here) unmask existing bugs. Plus, even that aside, you'd hopefully test exactly the build that you will release before you release it.
As I wrote in another comment, this could still be the result of a freak accident and not have shown up in testing, though (but since I know nothing about the process or the code there, it could be anything).
As I wrote in another comment, this could still be the result of a freak accident and not have shown up in testing, though (but since I know nothing about the process or the code there, it could be anything).
If I had to guess, the second expression depends on the first being true. So if `has_value()` is false, perhaps `key_data_->label()` is undefined behavior.
In other languages, booleans are their own values (iirc in C/C++ they're aliases for 0/not-0) for which the `&` operand is simply not implemented.
There's also linters and other quality control analyzers that will warn on a bitwise operator on booleans.
There's also linters and other quality control analyzers that will warn on a bitwise operator on booleans.
This is one of those times when, for all its verbosity, Ada got it right. For boolean conjunction you must choose between writing "A and B", or "A and then B". The "and then" version is short circuiting, while the bare "and" version is not.
This is a typo of missing an ampersand. If the author were writing Ada, it would have been a typo of missing a “then”. How is it different?
The equivalent Ada code does not exist as the Ada boolean type only has a logical AND operator (no bitwise operations exist):
http://www.ada-auth.org/standards/12rm/html/RM-A-1.html#I538...
http://www.ada-auth.org/standards/12rm/html/RM-A-1.html#I538...
Visual Basic also has this distinction (but the keywords are "And" and "AndAlso")
Other than this particular edge case it's not obvious to me why bitwise operators shouldn't be applicable to boolean values. Aren't they just single-bit vectors?
Of course it makes sense that logical operators wouldn't be applicable to numeric types, but why the reverse?
Of course it makes sense that logical operators wouldn't be applicable to numeric types, but why the reverse?
If you stop integer values from being implicitly convertible to booleans, you break the language and its compatibility with C.
The inverse is debatable, though. I honestly don't see why someone would do bitwise operations on boolean, unless they really wish to die a slow death from thousand cuts. Integer promotion rules make even the simplest operations a wild mess, and there's nothing in the standard that prevents bool from having a sizeof equal to int (thus further complicating everything).
It might be sensible for newer language revisions to consider deprecating certain operators from being applied to bool, given that there is a strong change that 99% of the time their use is not on purpose, but I guess it depends on how much disruptive such change would turn out to be.
The inverse is debatable, though. I honestly don't see why someone would do bitwise operations on boolean, unless they really wish to die a slow death from thousand cuts. Integer promotion rules make even the simplest operations a wild mess, and there's nothing in the standard that prevents bool from having a sizeof equal to int (thus further complicating everything).
It might be sensible for newer language revisions to consider deprecating certain operators from being applied to bool, given that there is a strong change that 99% of the time their use is not on purpose, but I guess it depends on how much disruptive such change would turn out to be.
I don't think the other poster was imagining a language that retained compatibility with C, but rather a new language altogether. Of course it would be impossible to change this aspect of the language if retaining compatibility with C was a requirement.
> I honestly don't see why someone would do bitwise operations on boolean
For example, you might want the behaviour shown here of not short-circuiting. Or you might want to create a bitwise function that also allows calling on single-bit (i.e. boolean) values.
> I honestly don't see why someone would do bitwise operations on boolean
For example, you might want the behaviour shown here of not short-circuiting. Or you might want to create a bitwise function that also allows calling on single-bit (i.e. boolean) values.
> you might want the behaviour shown here of not short-circuiting. Or you might want to create a bitwise function that also allows calling on single-bit (i.e. boolean) values.
In both cases, I think the risk-benefit ratio is way too skewed towards the risk of creating ugly, hard to detect bugs. When I don't want short circuiting from happening, I just assign the RHS of the expression to a variable beforehand; if I want a boolean to become an integer, I make it explicit with a cast.
Sometimes, using certain C and C++ debatable features feels like attempting to shave yourself using a dagger. The fact you could do that doesn't mean you should, or even that it is a good idea.
In both cases, I think the risk-benefit ratio is way too skewed towards the risk of creating ugly, hard to detect bugs. When I don't want short circuiting from happening, I just assign the RHS of the expression to a variable beforehand; if I want a boolean to become an integer, I make it explicit with a cast.
Sometimes, using certain C and C++ debatable features feels like attempting to shave yourself using a dagger. The fact you could do that doesn't mean you should, or even that it is a good idea.
You could make the opposite argument that short-circuiting is the surprising behavior [1] and you should make it explicit with an if.
[1] and in this very thread many people were actually surprised by it!
[1] and in this very thread many people were actually surprised by it!
Short circuiting for logical operators has existed at least since the first C versions in the early '70s. It's maybe not intuitive, it's probably a crummy idea, but that's how basically everything based on C works (including modern languages like Go and Rust), so there's not that much that can be done about that.
Also, after seeing how weird JavaScript is in comparison, I have to say that C++ definitely isn't the strangest kid in the block when accounting for weirdness and unintuitive behaviours.
Also, after seeing how weird JavaScript is in comparison, I have to say that C++ definitely isn't the strangest kid in the block when accounting for weirdness and unintuitive behaviours.
>> Other than this particular edge case it's not obvious to me why bitwise operators shouldn't be applicable to boolean values.
That is a really good question. At some point they decided that true=1 and false=0 but I'm not sure how deep that goes. In particular what does ! actually do in this context?
That is a really good question. At some point they decided that true=1 and false=0 but I'm not sure how deep that goes. In particular what does ! actually do in this context?
In what context? Unless overloaded ! coerces its operand to bool and returns the negation.
The most general solution here is to use algebraic types and pattern match on the value being “None” or “Some data”.
Certainly I find
if (key_data_.has_value() && !key_data_->label().empty())
return key_data_->label();
if let Some(label) = key_data_ {
label
}
It's not, because that would involve re-engineering and re-writing millions of lines of code.
The simpler solution is to fix this gap in their automated testing. Another one is to configure a linter to error on bitwise operations on boolean values.
The simpler solution is to fix this gap in their automated testing. Another one is to configure a linter to error on bitwise operations on boolean values.
[deleted]
That so sounds like an error I would make and not some superDev working at the Googs making around 4x the amount of money I make.
Certainly won't compile in Java.
Oddly... It would? Try:
Map<String, Boolean> map = ...
if (map.containsKey("hello") & map.get("hello")) { ...I bet the post-mortem for this is going to be fun. How this wasn't covered by multiple unit tests, a code review and a roll out strategy is.... impressive.
This isn't the failure of an individual. It is a failure of management.
Bitwise & versus logical && is a classic, right up there with an assignment in a comparison (when an equality check is intended), = for ==.
That this was missed is pretty surprising, given that it's Google and the stakes involved in the encryption/key management code in a secure platform device.
I wonder if we'll even get a postmortem, as this simply cannot happen unless several someones all Seriously Fucked Up simultaneously.
- code error (expected, humans are fallible)
- review error (less expected, this is the kind of thing code review exists for)
- static analysis / linting check failure
- integration test failure (interactive login no longer works on this build)
This is a massive, overlapping fuckup. Heads should probably roll here, especially given that when you opt in to ChromeOS as a user, Google now has "one job" (DFIU).
That this was missed is pretty surprising, given that it's Google and the stakes involved in the encryption/key management code in a secure platform device.
I wonder if we'll even get a postmortem, as this simply cannot happen unless several someones all Seriously Fucked Up simultaneously.
- code error (expected, humans are fallible)
- review error (less expected, this is the kind of thing code review exists for)
- static analysis / linting check failure
- integration test failure (interactive login no longer works on this build)
This is a massive, overlapping fuckup. Heads should probably roll here, especially given that when you opt in to ChromeOS as a user, Google now has "one job" (DFIU).
"Heads should probably roll here..."
Google has a blameless post-mortem process (see https://sre.google/sre-book/postmortem-culture/) which essentially boils down to:
0) We want to fix the situation. By the time the post-mortem happens, that should already be done.
1) We want to make sure this never happens again. In order to do that we need to understand as much as we can about what happened.
2) We want everyone to share everything they know about the event. If the participants worry about fallout, they will be compelled to cover up.
3) Most failures happen at a number of points, so you need to dig in deeply if you want to get full value out of the "event".
4) Good post-mortems lead to process fixes to reduce (or eliminate?) the chance of similar events, or to make fixes throughout the code base.
The other thing is that after the post-mortem, all of the folks involved have learned from it. Usually they're better engineers because of that expensive lesson you've just paid for.
Disclaimer: Googler, participant in several post-mortems, opinions my own.
Google has a blameless post-mortem process (see https://sre.google/sre-book/postmortem-culture/) which essentially boils down to:
0) We want to fix the situation. By the time the post-mortem happens, that should already be done.
1) We want to make sure this never happens again. In order to do that we need to understand as much as we can about what happened.
2) We want everyone to share everything they know about the event. If the participants worry about fallout, they will be compelled to cover up.
3) Most failures happen at a number of points, so you need to dig in deeply if you want to get full value out of the "event".
4) Good post-mortems lead to process fixes to reduce (or eliminate?) the chance of similar events, or to make fixes throughout the code base.
The other thing is that after the post-mortem, all of the folks involved have learned from it. Usually they're better engineers because of that expensive lesson you've just paid for.
Disclaimer: Googler, participant in several post-mortems, opinions my own.
Everyone should support blameless postmortems. I'm more ruing the impending "oh we're soooo sorry" PR babble where it reads as a subtle "** off pleb".
Blameless post-mortems are good, but there have been rather a lot of bootloader level disasters on ChromeOS/Android and where they intersect at this point and it is probably worth wondering why this process does not seem to be generating sufficient improvement to make customer devices work better.
One notes that even years later Google still haven't figured out why the Pixel C occasionally just forgot it's own passcode and encryption key and locked users out. Thousands of reports, and publicly acknowledged as an issue. Never addressed. Pixel devices have had multiple bootloader issues and are currently experiencing a widespread Widevine downgrade for months and not fixed.
One notes that even years later Google still haven't figured out why the Pixel C occasionally just forgot it's own passcode and encryption key and locked users out. Thousands of reports, and publicly acknowledged as an issue. Never addressed. Pixel devices have had multiple bootloader issues and are currently experiencing a widespread Widevine downgrade for months and not fixed.
Sounds nice, but how much is this really true in practice? If one guy on the team seriously screws up, won't the rest of the team naturally trust him less?
I've seen the = instead of == in the wild with disastrous results: the assignment was on an ORM backed object, e.g.
// accidentally mutates all names to Sam and persists to the DB
myList.filter { myDomainObj.name = 'Sam' }
>But it makes me wonder why our programming languages would use characters which can often lead to this type of error.
Guido van Rossum wondered the same, which is why it isn't possible in Python. Same for "&&" vs. "&", since in Python it's "and" vs. "&".
Guido van Rossum wondered the same, which is why it isn't possible in Python. Same for "&&" vs. "&", since in Python it's "and" vs. "&".
If you consider a linter part of a programming language, most, when properly configured, do not permit this. It's just the syntax that does; professionals do not rely on syntax validity alone but employ linting, code review, static analysis, and integrations tests on top.
The gradient of professionalism in our industry is very wide, and the slope is quite shallow.
To your point, however, many languages (such as Go, developed and used by Google, the organization under discussion) have designed their syntax now to completely avoid this type of error even being possible.
The gradient of professionalism in our industry is very wide, and the slope is quite shallow.
To your point, however, many languages (such as Go, developed and used by Google, the organization under discussion) have designed their syntax now to completely avoid this type of error even being possible.
> type of error even being possible
I mean its still possible to forget the ":" character and its still possible to mentally scan a PR and see "=" and miss that it should have been a "==".
I mean its still possible to forget the ":" character and its still possible to mentally scan a PR and see "=" and miss that it should have been a "==".
= vs == isn't possible in Go because of the statement/expression distinction. The benefits of allowing assignment to be an expression are too small compared to the problems it causes.
x == 1 // oops, meant =
// x == 1 evaluated but not used
if x = 1 { ... } // oops, meant ==
// syntax error: assignment x = 1 used as valuePython had similar, then introduced := for explicit assignment-as-expression. The debate over that one is considered a contributing factor to Guido's retirement as bdfl
Ah I see your point. Appreciate the correction.
what is the difference between linting and static analysis? Coverity, cppcheck etc, what are those? You mention integration tests, but not unit test, do you value unit test?
You could consider linting a subset of static analysis that considers only smaller units of code and not program flow. A linter might tell you your variable names are wrong or your brackets are wrong or you used discouraged method xyz. A different static analysis tool might tell you there's a path through a function that leaks memory.
I'm a fan of programming languages using := for assignment for this reason
Look, I know that this is an enormously bad bug, but...
Google has a codebase that numbers multiple billions of lines. Using := means typing multiple billions of extra characters. That adds up. And it wouldn't even have prevented this bug!
All checks have costs. As Emerson said, "A stitch in time saves nine. So we make 1000 stitches, that by doing so we may save nine."
Google has a codebase that numbers multiple billions of lines. Using := means typing multiple billions of extra characters. That adds up. And it wouldn't even have prevented this bug!
All checks have costs. As Emerson said, "A stitch in time saves nine. So we make 1000 stitches, that by doing so we may save nine."
Once more I hope this is satire, but as so often on the Internet it is surprisingly hard to tell.
This reminds me of Silicon Valley’s main character preferring tabs to spaces because they use less bytes, despite his own product being for... compression!
I personally prefer spaces so that under any circumstances, the code reads the same as the author intended (whether you’re in an editor or viewing a file with a CLI tool). Are we really counting bytes in this day and age?
I personally prefer spaces so that under any circumstances, the code reads the same as the author intended (whether you’re in an editor or viewing a file with a CLI tool). Are we really counting bytes in this day and age?
Considering that most code get read by more than one person, I think hardcoding the author's preferences in how it should be read is strange and in some cases inaccessible.
This. Author chose two spaces per indent and proceeded to nest blocks 5 levels deep? Guess I have to run it through a code formatter now just to read it.
Alternatively, author chose eight spaces per indent and proceeded to nest things? My tiled windows only have ~100 columns, thanks so much for breaking my workflow with your poor choices.
Alternatively, author chose eight spaces per indent and proceeded to nest things? My tiled windows only have ~100 columns, thanks so much for breaking my workflow with your poor choices.
But why not? These days reformatting on the fly is fast and easy. Especially if you enforce formatting conventions on commit (so you can automatically reformat into a canonical version).
Because typically code will be read far more than it is written and by far more people than contributed to it. In many cases I won't have the relevant tooling installed let alone know how to use it because I don't use that language myself. (For example, a library written in Rust that exposes C bindings whose prebuilt artifact I might download and link against.)
That being said, I 100% wholeheartedly approve of enforcing code formatting (among other things) on commit. Pre-commit hooks and linters are both awesome.
That being said, I 100% wholeheartedly approve of enforcing code formatting (among other things) on commit. Pre-commit hooks and linters are both awesome.
If you do not have the relevant tooling you can still read the canonical formatting.
If you want custom formatting, I don't think that expecting your editor to reformat on the fly is a big ask. At the end of the day that exactly what tab proponents expect.
If you want custom formatting, I don't think that expecting your editor to reformat on the fly is a big ask. At the end of the day that exactly what tab proponents expect.
Automated reformats cause big problems in version control.
Every line appears to come from the reformat commit. So it's hard to see when the codes function was changed.
Every line appears to come from the reformat commit. So it's hard to see when the codes function was changed.
I believe what's being described is on the fly (ie uncommited) reformatting to suit individual reading preferences. Additional enforced autoformatting at the time of commit ensures that all version controlled code always conforms to the chosen style. Thus there are never any reformat commits unless the chosen style is modified.
you format as part of the commit, not as a separate commit.
> so that under any circumstances, the code reads the same as the author intended
I write all my code in notepad.exe with the font Wingdings. I hope that if you ever read any of my code you'll respect my intent with how it's displayed.
I write all my code in notepad.exe with the font Wingdings. I hope that if you ever read any of my code you'll respect my intent with how it's displayed.
The argument for tabs really goes that time is spent much more on reading code than authoring, so it should be optimized and customizable (tab width) for the reader.
Tabs for indentation, spaces for alignment.
I've settled on "follow the existing/upstream project's style; if it's a new project, follow the language's idiomatic style". I personally (strongly) prefer spaces over tabs, but I've inherited/forked plenty of projects that use tabs in JavaScript for example, and I just set my editor and formatter to use tabs in that project.
I think the ideal situation is a language with an official or at least commonly accepted formatter, so that existing and new projects will essentially always follow the same style and you never have to think about it. Like "go fmt" always using tabs. I don't like tabs, but I like that kind of enforced consistency way more than I dislike tabs.
That's also why I like using opinionated third-party formatters like prettier and black. Combined with pre-commit hooks, you almost never have to think about style and can just focus on the code. Plus diffs are cleaner.
I think the ideal situation is a language with an official or at least commonly accepted formatter, so that existing and new projects will essentially always follow the same style and you never have to think about it. Like "go fmt" always using tabs. I don't like tabs, but I like that kind of enforced consistency way more than I dislike tabs.
That's also why I like using opinionated third-party formatters like prettier and black. Combined with pre-commit hooks, you almost never have to think about style and can just focus on the code. Plus diffs are cleaner.
I used to feel very strongly about this. Then I started using Common Lisp and discovered the abomination that is tabstop. This archaic "feature" leads to tab width that varies across the line and seems to be hardcoded in every text editor out there. Worse, it's only useful if using tabs for alignment which is completely broken anyway! At some point I intend to patch the source for the editors I use to remove this nonsense.
Admittedly this is something of an edge case - how many languages other than Lisps involve alternating layers of indentation and alignment? The more common C like languages don't suffer from this at all.
At this point I'm largely convinced that more or less all of our tools, languages, and conventions are poorly thought out, brittle, and inelegant. /rant
Admittedly this is something of an edge case - how many languages other than Lisps involve alternating layers of indentation and alignment? The more common C like languages don't suffer from this at all.
At this point I'm largely convinced that more or less all of our tools, languages, and conventions are poorly thought out, brittle, and inelegant. /rant
usually I turn off tabs for indentation in Lisp editors. On a Lisp Machine:
(setq zwei:*indent-with-tabs* nil)The problem is not even with "easy to mix up operators" for this, it's about languages without a strong and static enough type system. While the particular problem Google hit is a bit more subtle (involving undefined behavior), in the majority of cases a strongly typed language would not allow expressions that accidentally mix up = and ==, as they often resolve to different types.
In C "if (foo = bar)" and "if (foo == bar)" are both legal, because pretty much any type can be automatically coerced into a boolean. In many modern languages that is not the case. (Of course if you have assignments as expressions you are still screwed if foo and bar are both boolean to begin with, so some languages got away with that, too. And for fairness I should say that C at the very least warns if you don't explicitly put parentheses around the assignment, nowadays.)
In C "if (foo = bar)" and "if (foo == bar)" are both legal, because pretty much any type can be automatically coerced into a boolean. In many modern languages that is not the case. (Of course if you have assignments as expressions you are still screwed if foo and bar are both boolean to begin with, so some languages got away with that, too. And for fairness I should say that C at the very least warns if you don't explicitly put parentheses around the assignment, nowadays.)
In many languages, such as python, assignments are statements, not expressions, and so they don’t evaluate to anything at all.
Yeah, I covered that at the end.
> in the majority of cases a strongly typed language would not allow expressions that accidentally mix up = and ==, as they resolve to different types.
That's not true at all.
Whenever you're intending to compare equality, they're usually going to be the same type already unless you're being really slopping in a scripting language like JavaScript or PHP -- but even then they're usually two numbers, two strings, or two booleans.
The whole problem here is where foo and bar are both integers, or booleans, or whatever you want, and instead of typing "if (foo == bar)" you type "if (foo = bar)".
Strong typing does nothing whatsoever to help in this common situation.
That's not true at all.
Whenever you're intending to compare equality, they're usually going to be the same type already unless you're being really slopping in a scripting language like JavaScript or PHP -- but even then they're usually two numbers, two strings, or two booleans.
The whole problem here is where foo and bar are both integers, or booleans, or whatever you want, and instead of typing "if (foo == bar)" you type "if (foo = bar)".
Strong typing does nothing whatsoever to help in this common situation.
No. It can help because the result of the expression would have a different type, one that "if" does not accept. If a and b are integers, then the expression "a = b" is an integer too, but "a == b" is a boolean. In a strongly typed language, you can assume that "if" only accepts booleans.
Yeah. The direction of modern languages has been away from features like this which are "clever" but don't express anything you couldn't have said by another means - and yet allow programmers to accidentally write something nobody would ever want.
Assignment shouldn't have a return type. Compound assignment operators? Likewise. You never need this return value, and sometimes you may use it by mistake. So, abolish it. In C++ not only do operators that shouldn't have a return value have one anyway, the type of that value might be anything. You can even overload both ++ operators to return different unrelated types so that
"Your scientists were so preoccupied with whether or not they could, that they didn't stop to think if they should"
Assignment shouldn't have a return type. Compound assignment operators? Likewise. You never need this return value, and sometimes you may use it by mistake. So, abolish it. In C++ not only do operators that shouldn't have a return value have one anyway, the type of that value might be anything. You can even overload both ++ operators to return different unrelated types so that
mytype aThing { blah };
auto a = aThing++; // returns a String "Fuck tha police"
auto b = ++aThing; // returns a double, 8.63
"Your scientists were so preoccupied with whether or not they could, that they didn't stop to think if they should"
> Heads should probably roll here,
I disagree. It's obviously a major balls up by multiple people, but at the end of the day this is an organizational failure. You don't fire people for this, you learn from it and fix the organizational holes that caused it.
I disagree. It's obviously a major balls up by multiple people, but at the end of the day this is an organizational failure. You don't fire people for this, you learn from it and fix the organizational holes that caused it.
Someone is responsible for the organizational units and their processes.
By that logic, almost no individual failure is a firing offense. Many people in the organization are responsible for ensuring that the processes themselves overlap in ways that exclude the possibility of failures like this.
I'm talking about the meta-failure in management's oversight of the processes. There is belt person, and there is suspenders person, and there is a third person who makes sure there are both belt person and suspenders person on staff and that they don't go on vacation at the same time.
Somewhere above the team lead and below the CEO there is someone who didn't do their job.
By that logic, almost no individual failure is a firing offense. Many people in the organization are responsible for ensuring that the processes themselves overlap in ways that exclude the possibility of failures like this.
I'm talking about the meta-failure in management's oversight of the processes. There is belt person, and there is suspenders person, and there is a third person who makes sure there are both belt person and suspenders person on staff and that they don't go on vacation at the same time.
Somewhere above the team lead and below the CEO there is someone who didn't do their job.
> By that logic, almost no individual failure is a firing offence
I'd probably agree with that. Firing should be for extreme negligence, or sustained underperformance.
I'd probably agree with that. Firing should be for extreme negligence, or sustained underperformance.
>extreme negligence
If this doesn't qualify, what does?
If this doesn't qualify, what does?
This isn’t extreme negligence of any single individual. If there are linters/compiler warnings that should have caught this, this is an organizational failure that can be remedied and the manager/TL should be taking a bit of a hit here as having those enabled is best practices. Since it’s ChromeOS I imagine there’s a dedicated team managing the build infra and it’s not using the base set of things that Google3 protects you with, so the fault may lie in that organization a bit as an oversight.
Then there’s obviously the automated testing strategy, manual testing strategy and sanitizer strategy that missed this.
The net result is extreme negligence but to me this kind of failure speaks to several layers going wrong at once rather than one person fucking up badly. Organizationally you try to defend against these kinds of failures with multiple layers of defenses.
Now if this was a malicious internal attack that should be investigated but I’d hate to have to seriously reprimand anyone who typo’ed & instead of &&.
Then there’s obviously the automated testing strategy, manual testing strategy and sanitizer strategy that missed this.
The net result is extreme negligence but to me this kind of failure speaks to several layers going wrong at once rather than one person fucking up badly. Organizationally you try to defend against these kinds of failures with multiple layers of defenses.
Now if this was a malicious internal attack that should be investigated but I’d hate to have to seriously reprimand anyone who typo’ed & instead of &&.
The GGGP is talking about failures at a higher level. Not reprimanding the coder who missed an ampersand, but the manager who was ultimately responsibly for ensuring there was a functioning process for catching this sort of nonsense.
In situations like these there's not really one manager, so who are you reprimanding? I'm sure there's failures all around. Seriously, blameless post mortems are a cultural thing you need to adopt. Reprimanding just instills fear which causes people to take actions that you don't want organizationally (shifting blame/responsibility, hiding issues, politicking etc). Better for people to just be proactive about when a problem exists & marshal your org to prevent such issues in the future. It's unlikely to be the cause of 1 individuals incompetence so any individual(s) you find to blame are just victims of circumstance.
Where you do want to reprimand is if the technical team was repeatedly raising concerns related to failures like this & their concerns weren't being addressed OR someone was being deceitful & hiding issues. That's about the only cases where you want to fire/replace anyone in my book.
Where you do want to reprimand is if the technical team was repeatedly raising concerns related to failures like this & their concerns weren't being addressed OR someone was being deceitful & hiding issues. That's about the only cases where you want to fire/replace anyone in my book.
Reprimanding is one thing and firing another though. I certainly think the manager in charge deserves a reprimand.
I wouldn't even say a reprimand is necessary here.
Programming safety is like safety in auto racing or rock climbing: you build the safest thing you can, but the activity you're doing is inherently unsafe, and it will generate failures despite the presence safety measures, so you do your best to study those failures to mitigate them the best you can.
The only time adverse action like a reprimand is necessary is if the manager or tech lead don't take any action to prevent future issues. The only time a firing is necessary is if the security issue was created with intent, rather than accidentally.
Programming safety is like safety in auto racing or rock climbing: you build the safest thing you can, but the activity you're doing is inherently unsafe, and it will generate failures despite the presence safety measures, so you do your best to study those failures to mitigate them the best you can.
The only time adverse action like a reprimand is necessary is if the manager or tech lead don't take any action to prevent future issues. The only time a firing is necessary is if the security issue was created with intent, rather than accidentally.
This specific type of failure is evidence that they didn't build the safest thing they can.
Linting catches this.
Linting catches this.
I would say this is negligence, but perhaps not extremely so.
You are all very quick to judge. I don't work for Google so I don't know what happened, but consider the possibility that this may have been a freak accident. That piece of code for example might only be on a path that is not activated when testing, or it may be executed but not have the same fatal effect.
And before anyone comes along and says "well, every possible path should be tested then": That would be absolutely impossible even if the halting problem wasn't a thing (which it very much is). Even if you somehow have utopian "100% code coverage", that does not mean you have tested every possible input resulting in every possible state combination.
I bet every one of us has a story about an old bug that only manifested years[1] later because of the confluence of many unfortunate things, one that takes half an hour to tell.
[1] Or even decades, in some cases that then made it to the HN front page.
And before anyone comes along and says "well, every possible path should be tested then": That would be absolutely impossible even if the halting problem wasn't a thing (which it very much is). Even if you somehow have utopian "100% code coverage", that does not mean you have tested every possible input resulting in every possible state combination.
I bet every one of us has a story about an old bug that only manifested years[1] later because of the confluence of many unfortunate things, one that takes half an hour to tell.
[1] Or even decades, in some cases that then made it to the HN front page.
There are freak accidents and then there are complete lapses is basic critical functionality that demonstrate a total lack of competence.
The article indicates that every person who updated to the latest version could not login. This is a bug that clearly presents itself, every time, in essentially every use case, and is perfectly reproducible. It affects critical functionality that any layperson can recognize is one of the most critical components of a general purpose computing system. The greatest degree of scrutiny should be applied to this system, yet what we see is the functional equivalent of a car factory forgetting to put a wheel on every car it produces.
Sure, there could be a freak accident that makes the 4th wheel robot malfunction, but other stages of the process should catch such an egregious visible failure. Any process that would allow such basic errors to occur without checking or fixing them elsewhere is terrible on its face.
If that is too abstract, imagine you hired a construction company to build a house and they forgot to put in an entire wall. Maybe you just got unlucky and you were the 1 in 1 billion person who gets such a stupid error to happen to them, but the much more likely theory is that they are grossly incompetent, or at the very least far less reliable than you were led to believe.
The article indicates that every person who updated to the latest version could not login. This is a bug that clearly presents itself, every time, in essentially every use case, and is perfectly reproducible. It affects critical functionality that any layperson can recognize is one of the most critical components of a general purpose computing system. The greatest degree of scrutiny should be applied to this system, yet what we see is the functional equivalent of a car factory forgetting to put a wheel on every car it produces.
Sure, there could be a freak accident that makes the 4th wheel robot malfunction, but other stages of the process should catch such an egregious visible failure. Any process that would allow such basic errors to occur without checking or fixing them elsewhere is terrible on its face.
If that is too abstract, imagine you hired a construction company to build a house and they forgot to put in an entire wall. Maybe you just got unlucky and you were the 1 in 1 billion person who gets such a stupid error to happen to them, but the much more likely theory is that they are grossly incompetent, or at the very least far less reliable than you were led to believe.
I don't know what you are trying to add. Yes, if testing would have uncovered this and the release was put out without that testing, then someone is incompetent. But my point is exactly that the "egregiously visible failure" as you say may not have been visible at all during testing. You seem to think that by "not possible to test all paths" I mean that "logging in" was not tested. That's not at all what I mean.
I mean that even for basic user flows, there is an astronomically high number of possible states and external inputs such that, taken together, something that worked fine during testing may stop working at some point thereafter, or in the hands of the actual users. Latent bugs are called latent for a reason.
I mean that even for basic user flows, there is an astronomically high number of possible states and external inputs such that, taken together, something that worked fine during testing may stop working at some point thereafter, or in the hands of the actual users. Latent bugs are called latent for a reason.
One point that doesn't seem to have been explained yet - mentioned in the OP article - is that Chrome OS has canary, dev, and beta channels prior to stable, with real users.
For something like this to have happened without those non-stable channel users experiencing it, it seems like there must have been a code change that was pushed direct to the stable channel - which negates the purpose of the other channels in the first place.
At the very least it seems like a severe process failure. I don't think the dev who changed the code is necessarily at fault - it's more a question of how it was allowed to get into the stable channel without going through the normal process.
For something like this to have happened without those non-stable channel users experiencing it, it seems like there must have been a code change that was pushed direct to the stable channel - which negates the purpose of the other channels in the first place.
At the very least it seems like a severe process failure. I don't think the dev who changed the code is necessarily at fault - it's more a question of how it was allowed to get into the stable channel without going through the normal process.
This is the sort of thing that most linting tools will catch, so it's a bit worse than that I think. You shouldn't need any test coverage to catch this.
The way you fix organizational/process failures is not by firing the person in charge of them every time they fail to catch something.
How many breaking bugs have _NOT_ shipped due to the current processes. Your approach suggests that there is some perfect process manager out there who will let no bugs like this ship (whilst not dropping any other business priorities mind you) that they just haven't hired yet into this position.
This isn't as big of a bug as people on HN want it to be. A few months from few people will remember, and even fewer will care.
How many breaking bugs have _NOT_ shipped due to the current processes. Your approach suggests that there is some perfect process manager out there who will let no bugs like this ship (whilst not dropping any other business priorities mind you) that they just haven't hired yet into this position.
This isn't as big of a bug as people on HN want it to be. A few months from few people will remember, and even fewer will care.
> By that logic, almost no individual failure is a firing offense.
That's pretty close to how it should be. That's the core premise of the no-blame post-mortem culture that's great about some companies.
As long as you go through the port-mortem process, which includes doing your best to prevent this from happening again (which may include changing organizational processes, testing strategies, etc), then everyone can move on with their lives.
Firing happens when you have a history of performing below your level. Never for a single engineering failure.
That's pretty close to how it should be. That's the core premise of the no-blame post-mortem culture that's great about some companies.
As long as you go through the port-mortem process, which includes doing your best to prevent this from happening again (which may include changing organizational processes, testing strategies, etc), then everyone can move on with their lives.
Firing happens when you have a history of performing below your level. Never for a single engineering failure.
There is a story (perhaps apocryphal) of a salesman who lost a $10 million deal for IBM. He was sure he was going to be fired. Some bigwig (Watson himself, maybe?) talked him through what had gone wrong. The salesman asked, "Aren't you going to fire me?" The bigwig said, "I just spent 10 million dollars training you."
> By that logic, almost no individual failure is a firing offense.
Correct.
Correct.
I don't think we know enough from this to say.
It's possible that this bug made it into the wild because the organizational structure made remedies impossible. In that case you probably don't fire anyone, though maybe you reassign some folks.
It's also possible that the organizational structure indicates that a person or a group of people is responsible for this and they failed to do that job. If that's the case then it might very well make sense to fire folks. It also might not.
I think it's important for poor performance to have consequences[1] and I also think that people leave jobs all the time and it's fine to decide that someone should leave their place in the org with a positive reference and look elsewhere for work (perhaps in another part of goog).
[1] for moral and company culture reasons if nothing else.
It's possible that this bug made it into the wild because the organizational structure made remedies impossible. In that case you probably don't fire anyone, though maybe you reassign some folks.
It's also possible that the organizational structure indicates that a person or a group of people is responsible for this and they failed to do that job. If that's the case then it might very well make sense to fire folks. It also might not.
I think it's important for poor performance to have consequences[1] and I also think that people leave jobs all the time and it's fine to decide that someone should leave their place in the org with a positive reference and look elsewhere for work (perhaps in another part of goog).
[1] for moral and company culture reasons if nothing else.
Especially since it's so easy to lint for. This isn't some cross-file memory mishandling bug, it's a single token that's being used where it shouldn't be
> this simply cannot happen unless several someones all Seriously Fucked Up simultaneously.
This is exactly how any and every bug enters a mature software product: it slips past multiple layers.
> This is a massive, overlapping fuckup. Heads should probably roll here
Some process should change, to fix the problem. But firing people is not necessarily the best solution.
This is exactly how any and every bug enters a mature software product: it slips past multiple layers.
> This is a massive, overlapping fuckup. Heads should probably roll here
Some process should change, to fix the problem. But firing people is not necessarily the best solution.
RedHat once committed wrong value of MTWO variable, it was 2, there was another variable called TWO which also had value of 2. It was fixed a few months later with a commit like "change value of MTWO to -2".Sorry, but can't find it find now
glances at screen LGTM
Straight up negligence.
Reading stuff like "heads gonna roll" in the comments is kind of shocking to me. Have you never screwed up in your job?
How long have they shipped a secure Linux to a couple of hundred different devices simultaneously, while improving security, fixing critical bugs soon and delivering a pretty great user experience since they first launched Chrome OS? Sure, were soft bricks before, critical fixes introduced other bugs, duh. The same happens on Windows, macOS, Linux distributions - we're all human, we make mistakes, we try to learn from each mistake.
Don't give them a hard time. They already were in panic mode already.
How long have they shipped a secure Linux to a couple of hundred different devices simultaneously, while improving security, fixing critical bugs soon and delivering a pretty great user experience since they first launched Chrome OS? Sure, were soft bricks before, critical fixes introduced other bugs, duh. The same happens on Windows, macOS, Linux distributions - we're all human, we make mistakes, we try to learn from each mistake.
Don't give them a hard time. They already were in panic mode already.
Yup. My second or third month at my first job, I accidentally knocked out internet access for thousands of people for a couple of hours because I deleted something in production rather than in dev. It was terrifying and I was sure I would be fired.
Rather than being canned, of course, the team managing the tool that allowed me to delete the resource learned the following:
1) people should not be able to delete things without confirmation in production
2) it should be very obvious which environment you're in
3) normal humans shouldn't have access to production except for operations folks.
That's the value of blameless post mortems.
Rather than being canned, of course, the team managing the tool that allowed me to delete the resource learned the following:
1) people should not be able to delete things without confirmation in production
2) it should be very obvious which environment you're in
3) normal humans shouldn't have access to production except for operations folks.
That's the value of blameless post mortems.
I'm baffled that there's no test for this, manual or otherwise, before this rolls out to production. That you can change ChromeOS code in the login path, and the very first time it's exercised is by an end customer.
Edit: Someone has explained the issue below
I don't understand how that broke anything?
The C++ codegen for && and & is the same:
I don't understand how that broke anything?
The C++ codegen for && and & is the same:
bool good(bool a, bool b) { return a && b; }
good(bool, bool):
mov eax, edi
and eax, esi
ret
bool bad(bool a, bool b) { return a & b; }
bad(bool, bool):
mov eax, edi
and eax, esi
ret
The code is:
I wanna point out that std::optional, in the typical C++ stance, has a "I know what I'm doing" API and a "Humans make mistakes API": optional->... is UB if the optional is empty, optional.value()... raises an exception. Arguably, in code like this you probably only want to use the latter kind of API. (The pattern of "operator can UB, equivalent method asserts pre-condition" is very common in the C++ standard library)
key_data.has_value() && !key_data_->label().empty()
I wanna point out that std::optional, in the typical C++ stance, has a "I know what I'm doing" API and a "Humans make mistakes API": optional->... is UB if the optional is empty, optional.value()... raises an exception. Arguably, in code like this you probably only want to use the latter kind of API. (The pattern of "operator can UB, equivalent method asserts pre-condition" is very common in the C++ standard library)
Correct, key_data_ is a base::Optional<KeyData>. Chromium's base::Optional is very similar to std::optional, but predates c++17.
Except that Google uses C++ with exceptions disabled - IIUC, the throwing code would effectively behave like a call to std::terminate(), I haven't looked at the code to know if it would actually be a preferred behavior in this case.
Source: I'm a Chrome contributor.
Source: I'm a Chrome contributor.
Where is the undefined behavior? AFAU, UB is a compile time concept. As other commenter has asked, how can the compiler infer that has_value() call has any relation to key_data_ non-NULL-ness?
EDIT:
> optional->... is UB if the optional is empty
I understand what you mean: dereferencing optional is undefined if has_value() is not checked for true in the same thread before. But that undefined behavior happens at runtime, i.e. compile time UB is not invoked, the statements are kept without any optimizations. Right? The terminology is confusing.
EDIT:
> optional->... is UB if the optional is empty
I understand what you mean: dereferencing optional is undefined if has_value() is not checked for true in the same thread before. But that undefined behavior happens at runtime, i.e. compile time UB is not invoked, the statements are kept without any optimizations. Right? The terminology is confusing.
UB is not an event that happens. UB is a rule that allows compilers to assume that situations defined as UB never happen.
It's UB to access an optional that has no value, so if the compiler sees optional accessed unconditionally, it may assume the optional must have had a value, and delete all code that contradicts this conclusion.
Compilers don't do this out of spite. It's an optimization that removes redundant checks (e.g. when code in an inline function checks args != null, but you use the function with obviously non-null args).
It's UB to access an optional that has no value, so if the compiler sees optional accessed unconditionally, it may assume the optional must have had a value, and delete all code that contradicts this conclusion.
Compilers don't do this out of spite. It's an optimization that removes redundant checks (e.g. when code in an inline function checks args != null, but you use the function with obviously non-null args).
Yeah, I know what UB is, and that's why I'm saying that this is not UB. Have you even read what I wrote?
I recommend stepping back for a moment and asking yourself: Why is it that you, the person who believe they "know what UB is" and is sure "this is not UB" did not understand what's going on here, while the other people reading who explained why it's UB have understood exactly what's going on ?
Doesn't it seem most likely that you in fact do not understand Undefined Behaviour and didn't understand what's going on ?
Doesn't it seem most likely that you in fact do not understand Undefined Behaviour and didn't understand what's going on ?
I don't think you understand UB, because clearly, this is not UB.
But I congratulate you. You have enough knowledge to engage in Internet discussions involving fancy names without understanding underlying concepts.
Hint: 1) Read my original post. 2) Contemplate whether the compiler is removing anything in this instance.
But I congratulate you. You have enough knowledge to engage in Internet discussions involving fancy names without understanding underlying concepts.
Hint: 1) Read my original post. 2) Contemplate whether the compiler is removing anything in this instance.
> Read my original post.
Your original post has some questions. This is good, when you don't understand the world, you can ask questions and other people can help you to understand. Let's look at the questions:
> Where is the undefined behavior?
Using an Optional's value when it doesn't have one is Undefined. Specifically in this case use of the arrow operator on key_data_ is undefined.
> how can the compiler infer that has_value() call has any relation to key_data_ non-NULL-ness?
The compiler is allowed to use almost whatever means it wants to infer this type of relationship. For example if the Optional has_value() checks a member pointer isn't nullptr, and the arrow operator overload uses that pointer, the compiler can infer that if the arrow operator was used then has_value() is true.
At this point you had exciting opportunities to learn stuff you didn't understand about C++
Unfortunately, you also insisted upon some claims that are false, perhaps things you wrongly believed about C++ already that now came up against a fact about the world which seemed to prove them wrong. In particular:
> But that undefined behavior happens at runtime, i.e. compile time UB is not invoked
This isn't a thing. If you learned it somewhere, the source was wrong, if you've instead developed this as some sort of intuition about how C++ works, dismiss this understanding, it will continue to mislead you if retained.
Undefined Behaviour is a claim about the entire program which is useful to the compiler since its purpose is to transform the program. If it was Undefined before the transformation, it will still be Undefined afterwards, no matter what the transformation did, so you didn't break anything with your transformation.
> the statements are kept without any optimizations.
No. The compiler is free to use what it knows to optimize this code. Undefined Behaviour frees it up to optimize here a whole lot more than otherwise.
In particular it needn't worry about scenarios where key_data.has_value() is false, those have Undefined Behaviour, so nothing it could do is wrong.
Your original post has some questions. This is good, when you don't understand the world, you can ask questions and other people can help you to understand. Let's look at the questions:
> Where is the undefined behavior?
Using an Optional's value when it doesn't have one is Undefined. Specifically in this case use of the arrow operator on key_data_ is undefined.
> how can the compiler infer that has_value() call has any relation to key_data_ non-NULL-ness?
The compiler is allowed to use almost whatever means it wants to infer this type of relationship. For example if the Optional has_value() checks a member pointer isn't nullptr, and the arrow operator overload uses that pointer, the compiler can infer that if the arrow operator was used then has_value() is true.
At this point you had exciting opportunities to learn stuff you didn't understand about C++
Unfortunately, you also insisted upon some claims that are false, perhaps things you wrongly believed about C++ already that now came up against a fact about the world which seemed to prove them wrong. In particular:
> But that undefined behavior happens at runtime, i.e. compile time UB is not invoked
This isn't a thing. If you learned it somewhere, the source was wrong, if you've instead developed this as some sort of intuition about how C++ works, dismiss this understanding, it will continue to mislead you if retained.
Undefined Behaviour is a claim about the entire program which is useful to the compiler since its purpose is to transform the program. If it was Undefined before the transformation, it will still be Undefined afterwards, no matter what the transformation did, so you didn't break anything with your transformation.
> the statements are kept without any optimizations.
No. The compiler is free to use what it knows to optimize this code. Undefined Behaviour frees it up to optimize here a whole lot more than otherwise.
In particular it needn't worry about scenarios where key_data.has_value() is false, those have Undefined Behaviour, so nothing it could do is wrong.
> As other commenter has asked, how can the compiler infer that has_value() call has any relation to key_data_ non-NULL-ness?
Because of LTO.
Because of LTO.
The short circuiting was preventing undefined behavior on the second expression.
That is, yes, it is the same with booleans. Now try with expressions.
That is, yes, it is the same with booleans. Now try with expressions.
I made what I think is a minimal example here: https://godbolt.org/z/dcnzonjar
Interestingly I couldn't get it to fail on GCC (even with -fnoexceptions and -Ofast), only on Clang. Maybe someone else has some info as to why this is?
There is a clear difference in the assembly output for the correct and incorrect implementations, with a testb instruction run much earlier in the correct version. You can see this at line 22 of the output, if you highlight it the && in the source is highlighted in bold.
Interestingly I couldn't get it to fail on GCC (even with -fnoexceptions and -Ofast), only on Clang. Maybe someone else has some info as to why this is?
There is a clear difference in the assembly output for the correct and incorrect implementations, with a testb instruction run much earlier in the correct version. You can see this at line 22 of the output, if you highlight it the && in the source is highlighted in bold.
The difference is your example uses bool values, the code in question uses expressions that (should) evaluate to bool. && is short-circuited if a is false, such that b is not evaluated.
[deleted]
This is clearly a typo that happened to stomp some logic that is not covered by a test. The review 3002282 includes the bug, which deletes one of the & from a && operator. It says it was cherry-picked from review 2994464, which does not change that line. So, slip of the backspace key plus poor testing.
Yeah, the biggest thing to me here is that there's no "login succeeded" unit test, integration test, manual test, etc. Whatever customer logged in first was the first person to actually run the code.
Umm... no automated tests? Junior engineers responsible for security of half of the world without proper code review?
Half of the world? We're talking primarily north-american school kids.
https://trends.google.com/trends/explore?q=chromebook
https://trends.google.com/trends/explore?q=chromebook
>security of half the world
Chrome OS has less than 2% market share.
Chrome OS has less than 2% market share.
I heard elsewhere that its market share is closer to 10% (and that it's now the second most popular desktop OS). 2% would be considering mobile OS too, I think
https://www.maketecheasier.com/chrome-os-tops-macos-2nd-most...
https://www.maketecheasier.com/chrome-os-tops-macos-2nd-most...
Couldn't this be caused by a bad merge?
Not sure how Google manages their merging and release process but there are conceivably several stages where a bad merge can occur and go unnoticed - possibly at a stage close to release and after testing.
Not sure how Google manages their merging and release process but there are conceivably several stages where a bad merge can occur and go unnoticed - possibly at a stage close to release and after testing.
Or maybe a big automated refactor going too far? I wonder whether they run all tests on those.
At an established company merging is an incredibly incredibly locked down process. There are usually multiple barriers of automation and people, entire release teams who all have to be stoned at the wheel to miss anything odd getting through.
That's not "bricking" as the title suggests. Sorry, word police incoming. :P
If there is such a process as unbricking, then the title is right. If becoming a brick is irrecoverable, then you are right.
If you can't log into a device, it is a paperweight.
If you can't log into a device, it is a paperweight.
Here is the original article from Android Police:
https://www.androidpolice.com/2021/07/20/a-new-chrome-os-91-...
The reason for the failed "update" was another Chrome 0-day that relies on the Google's Javascript engine:
https://www.androidpolice.com/2021/07/16/another-day-another...
Curious whether they release the details of how this 0-day works after it is fixed, so we can see the mistake they made putting users at risk.
https://www.androidpolice.com/2021/07/20/a-new-chrome-os-91-...
The reason for the failed "update" was another Chrome 0-day that relies on the Google's Javascript engine:
https://www.androidpolice.com/2021/07/16/another-day-another...
Curious whether they release the details of how this 0-day works after it is fixed, so we can see the mistake they made putting users at risk.
So what it means is that someone could potentially introduce another 0day while patching a 0day because the code reviewing of those escapes the scrutiny of a regular process.
What it means is: They push crap to users with no testing.
Testing would have caught this.
Testing would have caught this.
Since the issue is caused by an undefined behavior optimization, possibly the tests ran on a different optimization level or on a different architecture target? Would still be surprising, but less so than there not being tests at all.
Maybe pushing updates to users is their test process? I doubt they're the only ones doing this, sadly.
There is a policy that would probably catch these types of errors - Chromeos developers should eat their own dog food, i.e., developers should be required to use a Chromebook on the "dev" or "beta" channels as their daily machines. Sure, use a cloud VM to get work done, but login, email, browsing, etc. done on the Chromebook. Google itself has fine machines for this purpose.
The article says the problem bricked their laptop - but I was under the assumption that you can create a non-google account/guest account on Chrome OS to use the device.
This has happened so many times in recent years to high profile services and OSes. You'd think it'd be a test case.
The bug, which used a single & in an if statement instead of a double && reminds me of this one from years ago-- https://lwn.net/Articles/57135/
In that case it was a single = versus a double == but also in if statement.
In that case it was a single = versus a double == but also in if statement.
So what happens to people who are affected by this? Are they just out of luck and have to find another device or try to unbrick their chromebook somehow?
EDIT: Per the Android Police article, looks like there is a way to update now without powerwashing. Disregard the below paragraph.
If they stored any data locally it's lost unless Google releases a way to allow a computer in this state to repair itself. Fortunately for many users the ChromeOS convention is to keep important things stored on the cloud. But that's not much comfort to those who chose to have experienced data loss due to this.
The description of 'bricking' in this case is unwarranted. Their device is fine following a 'powerwash', essentially a factory reset.
If they stored any data locally it's lost unless Google releases a way to allow a computer in this state to repair itself. Fortunately for many users the ChromeOS convention is to keep important things stored on the cloud. But that's not much comfort to those who chose to have experienced data loss due to this.
The description of 'bricking' in this case is unwarranted. Their device is fine following a 'powerwash', essentially a factory reset.
gcc and clang warn about if (a = b) and recommend if ((a = b))
gcc and clang don't warn about if (a & b) and don't recommend if ((a & b))
That doesn't make sense.
gcc and clang don't warn about if (a & b) and don't recommend if ((a & b))
That doesn't make sense.
(1 && 2) != (1 & 2)
If you've been in tech for a while, this is one of those things that makes you sigh and shake your fist at "move fast and break stuff" as a strategy.
We need more lawsuits for faulty deliverables and less "reboot and hope it works again" attitude.
That's more about challenging the status quo not bad project management.
[deleted]
shocking there is not test that caught this
Rust would have prevented this.
Rust's Option can't be simply dereferenced causing UB, so `val.is_some() & val.kaboom()` doesn't compile.
Rust doesn't have implicit conversion between booleans and integers, so a problematic `true & 2` does not compile either.
Rust does allow `true & true`, but booleans are strictly 0 or 1 in value, so you wouldn't get a wrong result, apart from eager evaluation. There's needless_bitwise_bool clippy lint for it.
Rust doesn't have implicit conversion between booleans and integers, so a problematic `true & 2` does not compile either.
Rust does allow `true & true`, but booleans are strictly 0 or 1 in value, so you wouldn't get a wrong result, apart from eager evaluation. There's needless_bitwise_bool clippy lint for it.
> Rust's Option can't be simply dereferenced causing UB, so `val.is_some() & val.kaboom()` doesn't compile.
I guess the equivalent of `operator->` would be `unwrap()`. It isn't UB, so this bug couldn't happen in Rust indeed.
> Rust doesn't have implicit conversion between booleans and integers, so a problematic `true & 2` does not compile either.
That isn't what's happening here as both `key_data_.has_value()` and `!key_data_->label().empty()` return bool.
I guess the equivalent of `operator->` would be `unwrap()`. It isn't UB, so this bug couldn't happen in Rust indeed.
> Rust doesn't have implicit conversion between booleans and integers, so a problematic `true & 2` does not compile either.
That isn't what's happening here as both `key_data_.has_value()` and `!key_data_->label().empty()` return bool.
Yes! While the other way around an accidental `&&` when `&` would have failed as well.
Can the link be updated to the one below? The link above goes directly to the comments section and collapses the article, ?comments=1 is the culprit.
https://arstechnica.com/gadgets/2021/07/google-pushed-a-one-...
https://arstechnica.com/gadgets/2021/07/google-pushed-a-one-...
HN won't let me edit the URL. :/
Dang or someone else will probably see it. I didn’t think you’d be able to do it yourself.
Fixed now. Thanks!
Wait so are the devices bricked or not? The title says bricked but as far as I can tell people just can't log in temporarily until the fix is rolled out.
Ok, we've done our bit to counter brick dilution in the title above.
Appreciated. Bricked used to mean utterly irreparable. The term certainly has seen dilution, and not for the better.
No, they are not bricked. The term has become ridiculously diluted in practice to mean "hard, breaking bug".
It's a soft lock for sure, given it requires a software update to use you could likely consider this a soft brick too.
It's not a hard brick to your point.
It's not a hard brick to your point.
The reason this happened is that it wasn't followed by "Google hit by 143 defective product lawsuits".
I don't think they could possibly be sued for this. They would just be responsible for fixing or refunding the product. In this case they issued a "fix".
It is the same as when a car is defective. They don't get sued, they do a recall and fix it.
It is the same as when a car is defective. They don't get sued, they do a recall and fix it.
>They don't get sued, they do a recall and fix it
Are you kidding? Major (car, drug, you name it) companies have hundreds, if not thousands of lawsuits going at any given moment.
Public companies will sometimes list major lawsuits in their annual report, but most of them don't get mentioned because the amount at stake is trivial for them.
Are you kidding? Major (car, drug, you name it) companies have hundreds, if not thousands of lawsuits going at any given moment.
Public companies will sometimes list major lawsuits in their annual report, but most of them don't get mentioned because the amount at stake is trivial for them.
You have to remember, pretty much anyone can sue anyone... doesn't mean that they'll be successful.
I don't have statistics on that, but I find it implausible that there would be hundreds or thousands of lawsuits against a typical corporation if say, only a couple % won or got a settlement.
I find it hard to believe that people are successful over lawsuits like this. Pretty much every product comes with an agreement that by using the device that you agree to it.