-
Notifications
You must be signed in to change notification settings - Fork 17.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
proposal: Go 2: make int type panic on overflow #31500
Comments
Any refs to research on the frequency of int-overflow defects? Any thoughts on performance impact of checking every change to every Could one enable/disable this with a build flag? |
What about the A possible compromise between this and your other proposal (#30613) would be to leave |
I do not have any stats on the frequency of integer wrapping errors. Good question. Not sure how to get those stats. On x86 the performance impact is an extra instruction, a correctly predicted branch, after most arithmetic operations. I doubt the performance impact would be significant, but we will have to see. I do not think there should be a build flag for this, any more than there is for other language features. The The purpose of this proposal is to make most programs avoid overflow without requiring extra work. Adding a new type would defeat that goal. If people are worried about overflow, then they will write code that does not overflow, regardless of whether we add a new type or not. The point is to help people who are not worried about overflow, but should be. |
could we check this on compile time instead of panicking in runtime? I mean something like this. func add(i int) int {
i += 1
return i // overflow of an integer not checked error
}
func add(i int) int {
i += 1
if overflowed(i) {
...
}
return i // ok
} |
@kybin There is a proposal along those lines at #30209. This proposal is not that one. Personally I not convinced that such a check needs to be built into the language. I think it's rare that people want to check for overflow. And for people who do want to check, it can be checked before the operation. And of course, with this proposal, one could write func CheckedAdd(a, b int) (sum int, ok bool) {
defer func() {
if recover() != nil {
sum = 0
ok = false
}
}()
return a + b, true
} although it would be more efficient to write an explicit check beforehand as one can already do today. |
@ianlancetaylor I've missed it, sorry. Another thought is, we can add -overflow flag to build command, so overflows are checked if the flag is on. (just like go build -race). I support your proposal, which is panicking on integer overflow. It might be little better if we could check early when we want. |
In Go we try to avoid adding knobs like a |
I agree to you said. Thank you. |
I vote against this functionality.
Although I do see the reason why this is asked. The reasons why GOlang should not do it is the same as all the other languages earlier reached the same conclusion (not to support it). |
coming from a C background, where int has always been undefined-on-overflow, and most users are still surprised by it: i think i like this idea, but find it slightly-surprising for it to apply to int, but not to int32 or int64. i'd be concerned about impact on existing code, but also i think it would indeed make code safer and mostly catch actual errors. |
That is true in general, but note that it is possible today to detect the size of As much as I would like us to be able to adopt this proposal, it certainly seems to run against the constraints described in https://github.com/golang/proposal/blob/master/design/28221-go2-transitions.md#language-redefinitions. |
Fair enough. If there are plausible examples of programs that do this for To expand, it is clear that this proposal changes the behavior of existing programs. And clearly it will fix some programs, or, at least, avoid certain classes of incorrect behavior. If we can convince ourselves that it will fix some real programs and not break any real programs, then perhaps we can proceed. But I'm open to counter-arguments. |
A counter-argument to this proposal is that it introduces a panic path to code that currently doesn't have one. That raises the possibility of unexpectedly crashing a long-running server on an unlikely and untested path. In some cases that can lead to a denial of service attack via a query of death. That is, this proposal can change code that is silently incorrect into code that loudly crashes. While that is often a good tradeoff, sometimes it is not. |
The usual example of such paths is logging statements, which may very well be untested and unchecked in existing code: log.Printf("wrote %d bytes", fooLen + barLen) |
#30613 (comment) points out the similarity between dynamic overflow checks and race-detection. So perhaps we could enable the overflow check for |
It's an interesting idea, but from a language perspective it means that integer overflow is no longer clearly specified. That might be OK but it seems awkward. |
Lots of answers here, all shared before. How common is overflow? Find out for yourself in the Playground... How central is this issue? It defines Niklaus Wirth's worldview on the matter, with much code in PASCAL and successors shaped by it. (The "math view" vs the "computer view) Scroll down there to my post "Yes, there are cases for integer domain wraparound just as there are cases for exception at the limits." Summary: (1) Silent wraparound is the right answer in general. (2) A special kind of variable that always checks (as an option in variable declaration or as a special checking-type) is a very good thing. (3) A way to test for exception is also a good thing. #3 is always possible. So the question there is more about how much help to provide. I'd early on proposed: sum,carry := a+b which did not win favor, though it has come back around via the bits library and special function calls. |
@MichaelTJones, when people are asking “How common is overflow?”, I suspect they mean “How often do variables overflow in Go programs in production?”, not “What fraction of the space of possible operands results in overflow?”. |
The borrow / carry / remainder approach works great for multi-precision arithmetic, but that's not what most people are writing. For simple overflow checks, it seems more useful to have an operation or program behavior that applies to a whole expression tree, not just one individual operation. |
I do like the idea of having the borrow/carry/remainder functionality; I would absolutely use it if it existed, even though that's separate from the desireability of checked-overflow (or for that matter saturating) math. |
If the question is “how often in apps” then it is too general to answer technically. It would seem the soft answer would be “never in cases that matter, because that would be considered a bug in program behavior.” The occurrence in unnoticed places, not seen or addressed as bugs, is clearly not “never” in a smart way but it is a kind of virtual never. On a pragmatic note, people use 64 bit signed ints these days as they loop from 1 to 10 and multiply by 365 days, say, so there is quite a bit of headroom to the 63 bit upper limit of 9,223,372,036,854,775,807. |
I think there's probably a fair amount of intentional-overflow happening in code, but i'm not honestly sure how much. Obviously, much more in unsigned. I suppose one strategy would be (1) implement it, (2) find out what happens. |
Based on the objections mentioned above in #31500 (comment) we are not going to adopt this proposal. |
Proposal: change the type
int
to panic on overflow.The type
int
does not have a defined size. That means that people do not expect a particular overflow behavior. It is also the default type of integer constants, and the result type oflen
,cap
andcopy
, and the key type ofrange
on a slice, array, or string, so it appears frequently in Go programs.Catching integer overflow will catch many cases where a program behaves incorrectly. Overflow in
int
values is unexpected but does sometimes occur. When it does occur, it is normally a bug in the program. Panicking rather than wrapping will help Go programmers find these bugs.This idea arose in the consideration of adding checked integer types, which panic rather than wrap on overflow. A problem with adding new checked integer types is that, as noted above,
int
is the implied type for several operations. New types would require explicit conversions fromint
to the new type. It's unlikely that people would use them, so code would not become safe.This proposal does not preclude adding more checked integer types in the future, as suggested in #30209 and #30613. It only addresses the handling of the special type
int
itself.This would be a change in current program behavior, but since the type does not have a defined size, a program that relies on the wrapping behavior of
int
is likely to be subtly buggy when run on systems with different sizes ofint
.This proposal is a simpler path forward compared to #19623, #19624, #30209, #30613.
A disadvantage of changing only
int
would be that people might assume that all integer types panic on overflow. Unfortunately, changing the behavior of the other integer types seems much more likely to break existing programs.The text was updated successfully, but these errors were encountered: