5 ms·

Unexpected string to byte casting behavior in Go 1.5

32 points by spenczar5 11y ago

sleaze 11y ago
http://play.golang.org/p/hR7iVDC-Vu http://play.golang.org/p/hR7iVDC-Vu Yes, that is a very nasty bug indeed. I think I'll be sticking with 1.4.2 for quite some time.. Does anyone else feel like 1.5 was released prematurely before it was honestly ready to go out the door? At this point Go is more than 5 years old; these kinds of amateur issues seem unacceptable. Can anyone comment on what it's like when new versions of Java have been released throughout the past 20 years? I am curious if regressions and stability issues are common in that ecosystem. My experience with Scala has been that things are quite bulletproof by the time they actually get released for wide-consumption.
- pdq 11y ago
  Agreed, considering this was a major rewrite of the core compiler in converting for C to Go, I don't understand why they rushed from release candidate (August 6) to release (August 20) in 2 weeks. I'm all for schedules, but in this case more regressions and a longer release candidate timeline would have been warranted. This also stresses the importance of random codegen testing and fuzzing, where test cases get run on v1.4 and v1.5 simultaneously and compared against each other. A simple fuzzer should have caught this. The good news is this bug was caught on the same day as the release.
  - omarforgotpwd 11y ago
    It wasn't exactly a rewrite... the C code got passed through a tool that converted it from C to Go in a way that tried not to change anything. Now they have a bunch of non-idiomatic Go code that should do what the C code was doing before more or less. Judging from my use of Go 1.5 so far, it did a pretty good job but made the compiler noticably slower.
- skybrian 11y ago
  It's certainly surprising, particularly since Go has its own extensive test suites and quite a few people have used the prereleases (including being deployed within Google and running all those tests as well). It suggests the error is somehow not that commonly triggered in a visible way, or lots of tests would have broken. Or alternately it was a last-minute change.
  - pcwalton 11y ago
    Looks like a register allocation bug, which is the kind of thing that can only trigger in specific circumstances. It looks like the Go compilers are especially vulnerable to this because individual AST nodes have to allocate registers, which would not be a problem if they used a more traditional IR.
    - enneff 11y ago
      There's work underway to implement the SSA form for amd64 in 1.6.
- nemothekid 11y ago
  >Does anyone else feel like 1.5 was released prematurely before it was honestly ready to go out the door? Nope. I would consider it the same as moving from 1.7 -> 1.8 and having things break/bugs occur. How many people in the Java ecosystem are still on 1.6 for whatever reason? It may have been fixed before release if more people tested the release candidate (but that never happens), and unfortunately for software "x.0" ends up being the "public beta". I remember in 1.3.0 where I reported a bug (that also had to do with string/byte slice weirdness), that was fixed pretty quickly and landed in 1.3.1 (https://github.com/golang/go/issues/8325 https://github.com/golang/go/issues/8325).
- enneff 11y ago
  > Does anyone else feel like 1.5 was released prematurely before it was honestly ready to go out the door? At this point Go is more than 5 years old; these kinds of amateur issues seem unacceptable. Honestly, no. All software has bugs. The Go codebase is very solid, in general. It's hard to get away with as much change as we've had between 1.4 and 1.5 without a few issues slipping through. At Google we've been running 1.5 in production for a while and didn't catch this issue. As have a lot of our other users. But inevitably when you release the stable version more bugs are uncovered; this is just life. Conservative users should certainly wait a few days (or weeks) after a major new version before upgrading. Historically we have released version 1.N.1 within a couple of weeks of each major release. If I were you I'd just wait for 1.5.1. edit: I think "very nasty" is overstating things. The likelihood of real code triggering this bug is extremely low.
detaro 11y ago
I wonder if it makes sense to "black-flag" certain instructions in the output, like here the comparison of a value with itself, as instructions that should never be generated and throw an error if that happens?
schmichael 11y ago
Ugly bug, but I'm very impressed to see a first time contributor able to fix a compiler bug in 6 hours! Seems to lend some credence to the assertion that rewriting everything in Go would make it easier to maintain.
- vhost- 11y ago
  I too found that to be pretty awesome. I've spent a few hours looking at the compiler source and the runtime code and a surprising amount of it is very understandable to me.
- Maken 11y ago
  It looks to me more like an accomplishment of git-bisect, which let the contributor isolate the bug to a few lines of code in a single commit.
  - 4ad 11y ago
    Git-bisect helped a lot, yes, but you can see that the commit in question was huge, and complex, not "a few lines of code".
- sshykes 11y ago
  Git-bisect for the win! On another note, I find it very odd that this fell off the front page so quickly. Is there some kind of go/google voters club on HN that flags anything that seems negative towards go?
deleted 11y ago
[deleted]
yo-code-sucks 11y ago
GO Figure

32 points by spenczar5 11y ago

http://play.golang.org/p/hR7iVDC-Vu http://play.golang.org/p/hR7iVDC-Vu Yes, that is a very nasty bug indeed. I think I'll be sticking with 1.4.2 for quite some time.. Does anyone else feel like 1.5 was released prematurely before it was honestly ready to go out the door? At this point Go is more than 5 years old; these kinds of amateur issues seem unacceptable. Can anyone comment on what it's like when new versions of Java have been released throughout the past 20 years? I am curious if regressions and stability issues are common in that ecosystem. My experience with Scala has been that things are quite bulletproof by the time they actually get released for wide-consumption.

pdq 11y ago
Agreed, considering this was a major rewrite of the core compiler in converting for C to Go, I don't understand why they rushed from release candidate (August 6) to release (August 20) in 2 weeks. I'm all for schedules, but in this case more regressions and a longer release candidate timeline would have been warranted. This also stresses the importance of random codegen testing and fuzzing, where test cases get run on v1.4 and v1.5 simultaneously and compared against each other. A simple fuzzer should have caught this. The good news is this bug was caught on the same day as the release.
- omarforgotpwd 11y ago
  It wasn't exactly a rewrite... the C code got passed through a tool that converted it from C to Go in a way that tried not to change anything. Now they have a bunch of non-idiomatic Go code that should do what the C code was doing before more or less. Judging from my use of Go 1.5 so far, it did a pretty good job but made the compiler noticably slower.
skybrian 11y ago
It's certainly surprising, particularly since Go has its own extensive test suites and quite a few people have used the prereleases (including being deployed within Google and running all those tests as well). It suggests the error is somehow not that commonly triggered in a visible way, or lots of tests would have broken. Or alternately it was a last-minute change.
- pcwalton 11y ago
  Looks like a register allocation bug, which is the kind of thing that can only trigger in specific circumstances. It looks like the Go compilers are especially vulnerable to this because individual AST nodes have to allocate registers, which would not be a problem if they used a more traditional IR.
  - enneff 11y ago
    There's work underway to implement the SSA form for amd64 in 1.6.
nemothekid 11y ago
>Does anyone else feel like 1.5 was released prematurely before it was honestly ready to go out the door? Nope. I would consider it the same as moving from 1.7 -> 1.8 and having things break/bugs occur. How many people in the Java ecosystem are still on 1.6 for whatever reason? It may have been fixed before release if more people tested the release candidate (but that never happens), and unfortunately for software "x.0" ends up being the "public beta". I remember in 1.3.0 where I reported a bug (that also had to do with string/byte slice weirdness), that was fixed pretty quickly and landed in 1.3.1 (https://github.com/golang/go/issues/8325 https://github.com/golang/go/issues/8325).
enneff 11y ago
> Does anyone else feel like 1.5 was released prematurely before it was honestly ready to go out the door? At this point Go is more than 5 years old; these kinds of amateur issues seem unacceptable. Honestly, no. All software has bugs. The Go codebase is very solid, in general. It's hard to get away with as much change as we've had between 1.4 and 1.5 without a few issues slipping through. At Google we've been running 1.5 in production for a while and didn't catch this issue. As have a lot of our other users. But inevitably when you release the stable version more bugs are uncovered; this is just life. Conservative users should certainly wait a few days (or weeks) after a major new version before upgrading. Historically we have released version 1.N.1 within a couple of weeks of each major release. If I were you I'd just wait for 1.5.1. edit: I think "very nasty" is overstating things. The likelihood of real code triggering this bug is extremely low.

detaro 11y ago

I wonder if it makes sense to "black-flag" certain instructions in the output, like here the comparison of a value with itself, as instructions that should never be generated and throw an error if that happens?

schmichael 11y ago

Ugly bug, but I'm very impressed to see a first time contributor able to fix a compiler bug in 6 hours! Seems to lend some credence to the assertion that rewriting everything in Go would make it easier to maintain.

vhost- 11y ago
I too found that to be pretty awesome. I've spent a few hours looking at the compiler source and the runtime code and a surprising amount of it is very understandable to me.
Maken 11y ago
It looks to me more like an accomplishment of git-bisect, which let the contributor isolate the bug to a few lines of code in a single commit.
- 4ad 11y ago
  Git-bisect helped a lot, yes, but you can see that the commit in question was huge, and complex, not "a few lines of code".
sshykes 11y ago
Git-bisect for the win! On another note, I find it very odd that this fell off the front page so quickly. Is there some kind of go/google voters club on HN that flags anything that seems negative towards go?

deleted 11y ago

[deleted]

yo-code-sucks 11y ago

GO Figure