Live data from Hacker News

Google broke a conditional statement that verifies passwords on Chrome OS

arstechnica.com

61–70 of 276 posts

Re: Google broke a conditional statement that verifies passwords on Chrome OS

#64
post #53

Earlier quoted context omitted.

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

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.

Re: Google broke a conditional statement that verifies passwords on Chrome OS

#65
post #17

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]…

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

Re: Google broke a conditional statement that verifies passwords on Chrome OS

#66

Earlier quoted context omitted.

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

Once more I hope this is satire, but as so often on the Internet it is surprisingly hard to tell.

Re: Google broke a conditional statement that verifies passwords on Chrome OS

#67
post #32

Earlier quoted context omitted.

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, d…

> 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 "==".

= 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 value

Re: Google broke a conditional statement that verifies passwords on Chrome OS

#68
post #20
post #18

Earlier quoted context omitted.

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

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 suspender…

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

Re: Google broke a conditional statement that verifies passwords on Chrome OS

#69
The company who no longer recognizes doublequotes nor their own verbatim operator?

The one who flags chess videos?

Closes decade old accounts without any kind of explanation?

Absolutely believable.

Wouldn't surprise if this too is related to relying on their Artificial Stupidity^HIntellingence systems.

Re: Google broke a conditional statement that verifies passwords on Chrome OS

#70

Edit: Someone has explained the issue below 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 [0] https://godbolt.org/z/84MMqTehs

The code is: key_data.has_value() && !key_data_->label().empty() key_data is probably something like an std::optional, so short-circuit evaluation guards the dereferencing of the optional. If you write this with a binary and, no short-circuit evaluation can happen, so undefined behavior ensues. 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 mistak…

Correct, key_data_ is a base::Optional. Chromium's base::Optional is very similar to std::optional, but predates c++17.
Post reply on HN