Live data from Hacker News

Google broke a conditional statement that verifies passwords on Chrome OS

arstechnica.com

121–130 of 276 posts

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

#121
post #86

Earlier quoted context omitted.

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

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

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:

  for (/* all triangles */) {
    auto u = calcU(/*...*/);
    if (u  1) {
      continue;
    }
    auto v = calcV(/*...*/);
    if (v  1) {
      continue;
    }
    auto dist = calcD(/*...*/);
    if (dist 
So what's happening?

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 1) will almost always be false.

But the two subconditions 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 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 1) | (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.

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

#122
post #97

Earlier quoted context omitted.

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

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.

Reprimanding is one thing and firing another though. I certainly think the manager in charge deserves a reprimand.

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

#123
post #66

Earlier quoted context omitted.

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?

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.

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

#124
post #89
post #10

Earlier quoted context omitted.

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

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

Perhaps they should have deleted that variable…

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

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

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

#126

Earlier quoted context omitted.

> 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

Ah I see your point. Appreciate the correction.

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

#127
post #102

Earlier quoted context omitted.

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

"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 p…

Which of "Rust, Haskell (core Haskell), OCaml, and SML" is able to parametrize types on values, à la template ?

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

#128
post #81

Earlier quoted context omitted.

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.

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 } An operator that did not short circuit would have undefined behavior because it might dereference a NULL pointer.

If you're going for "each line of code should do exactly one thing", you'd probably prefer that as

    if(foo) {
      if(foo->bar) {
        //whatever
      }
    }
separating out the null check and the actual conditional. More lines of code and more nesting, yes, but, if you're trying to strictly adhere to a one thing/one line principle, you probably don't care.

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.

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

#129
post #50

Earlier quoted context omitted.

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.

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

In many languages, such as python, assignments are statements, not expressions, and so they don’t evaluate to anything at all.

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

#130
post #89

Earlier quoted context omitted.

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

Perhaps they should have deleted that variable…

While externalizing repeated constants can be useful (e.g. defining PI_4 for a math routine that uses pi / 4 a dozen times), taken to an extreme like that it leads to more bugs, not fewer.
Post reply on HN