-
Notifications
You must be signed in to change notification settings - Fork 17.8k
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
runtime: incorrect sanity checks in the page allocator #38130
Comments
Ah! Thank you. I'll fix that right away. Earlier on in the review process the return value was indeed an int, but this was used as a shift in multiple places, so I changed the whole Luckily these are just sanity checks, but if there is a bug in the page allocator, it could lead to bad pointers being returned (and possibly memory corruption). |
Change https://golang.org/cl/226297 mentions this issue: |
Uploaded a fix. We may want to backport this. Like I said before, if there's a bug in the page allocator that would cause these sanity checks to fail, it would manifest as memory corruption as opposed to the immediate failure it was supposed to be. CC @bcmills who has been tracking a bunch of the memory corruption issues. |
I wonder whether this should become a vet check. I think with vet, we can at least find very simple instance (eg: unsigned number < 0); prove is able to find similar always-true or always-false conditions in more complicated situations which vet wouldn't be able to; but prove of course cannot emit a compile-time error. Maybe this should go into what @dr2chase was doing, that is a log of "important information" written by the compiler during compilation. |
While investigating the optimizations performed by my prove CL 196679, I found out these two instances of what looks like dead code:
go/src/runtime/mpagealloc.go
Lines 726 to 734 in d99fe1f
go/src/runtime/mpagealloc.go
Lines 768 to 773 in d99fe1f
That branch is never taken because
pallocBits.find()
return an unsigned number:go/src/runtime/mpallocbits.go
Lines 197 to 206 in d99fe1f
Given the documentation, it looks like the failure is reported as
^uint(0)
so the above checks are probably wrong./cc @aclements @mknyszek
The text was updated successfully, but these errors were encountered: