[go] runtime: print error code on Windows VirtualAlloc failure
Gerrit Bot (Gerrit)
Gerrit Bot has uploaded the change for review![]( "Open in Gerrit")
Commit message
runtime: print error code on Windows VirtualAlloc failure
runtime: print error code on Windows VirtualAlloc failure
When VirtualAlloc fails on Windows, the resulting crash is currently
opaque. It is difficult to distinguish between physical memory exhaustion
(ERROR_NOT_ENOUGH_MEMORY), the commit limit being reached
(ERROR_COMMITMENT_LIMIT), or other system limits.
This change prints the GetLastError code to stderr when VirtualAlloc
returns nil to aid debugging.
Fixes #70558
Change diff
diff --git a/src/runtime/mem_windows.go b/src/runtime/mem_windows.go
index afc2dee..5729aa5 100644
--- a/src/runtime/mem_windows.go
+++ b/src/runtime/mem_windows.go
@@ -26,7 +26,13 @@
//
//go:nosplit
func sysAllocOS(n uintptr, _ string) unsafe.Pointer {
- return unsafe.Pointer(stdcall(_VirtualAlloc, 0, n, _MEM_COMMIT|_MEM_RESERVE, _PAGE_READWRITE))
+ p := stdcall(_VirtualAlloc, 0, n, _MEM_COMMIT|_MEM_RESERVE, _PAGE_READWRITE)
+ if p == 0 {
+ errno := getlasterror()
+ print("runtime: VirtualAlloc of ", n, " bytes failed with errno=", errno, "\n")
+ return nil
+ }
+ return unsafe.Pointer(p)
}
func sysUnusedOS(v unsafe.Pointer, n uintptr) {
@@ -120,14 +126,19 @@
func sysReserveOS(v unsafe.Pointer, n uintptr, _ string) unsafe.Pointer {
// v is just a hint.
// First try at v.
- // This will fail if any of [v, v+n) is already reserved.
- v = unsafe.Pointer(stdcall(_VirtualAlloc, uintptr(v), n, _MEM_RESERVE, _PAGE_READWRITE))
- if v != nil {
- return v
+ p := stdcall(_VirtualAlloc, uintptr(v), n, _MEM_RESERVE, _PAGE_READWRITE)
+ if p != 0 {
+ return unsafe.Pointer(p)
}
// Next let the kernel choose the address.
- return unsafe.Pointer(stdcall(_VirtualAlloc, 0, n, _MEM_RESERVE, _PAGE_READWRITE))
+ p = stdcall(_VirtualAlloc, 0, n, _MEM_RESERVE, _PAGE_READWRITE)
+ if p == 0 {
+ errno := getlasterror()
+ print("runtime: VirtualAlloc of ", n, " bytes failed with errno=", errno, "\n")
+ return nil
+ }
+ return unsafe.Pointer(p)
}
func sysMapOS(v unsafe.Pointer, n uintptr, _ string) {
Change information
- M src/runtime/mem_windows.go
Related details
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Gerrit Bot (Gerrit)
Gerrit Bot uploaded new patchset![]( "Open in Gerrit")
Related details
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Alex Brainman (Gerrit)
Alex Brainman added 2 comments![]( "Open in Gerrit")
Thanks for sending the CL.
Please, answer my question below.
Alex
print("runtime: VirtualAlloc of ", n, " bytes failed with errno=", errno, "\n")If you look at the log https://logs.chromium.org/logs/golang/buildbucket/cr-buildbucket/8730352753718672849/+/u/step/19/log/2 mentioned at the issue https://github.com/golang/go/issues/70558#issue-2691890092 , you will see that the stack trace already prints that information, like
```
runtime: VirtualAlloc of 8192 bytes failed with errno=1455
```
Why do we need your CL to print similar information?
Related details
- Austin Clements
- Keith Randall
- Martin Möhrmann
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Bill Morgan (Gerrit)
Bill Morgan added 2 comments![]( "Open in Gerrit")
Thanks for the clarification Alex. I replied below.
print("runtime: VirtualAlloc of ", n, " bytes failed with errno=", errno, "\n")If you look at the log https://logs.chromium.org/logs/golang/buildbucket/cr-buildbucket/8730352753718672849/+/u/step/19/log/2 mentioned at the issue https://github.com/golang/go/issues/70558#issue-2691890092 , you will see that the stack trace already prints that information, like
```
runtime: VirtualAlloc of 8192 bytes failed with errno=1455
```Why do we need your CL to print similar information?
Thanks, I see what you mean now Alex. I tested by doing `_ = make([]byte, 200*1024*1024*1024*1024)` and did not get the errno printed out so I modified `sysAllocOS` and `sysReserveOS` behavior to be in line with `sysUsedOS` and tested to confirm I got the errno for that huge allocation failure. After a closer re-reading of the issue, I realize this doesn't really address the deeper 'unactionable' part of the original issue Alan mentioned though.
Related details
- Alex Brainman
- Austin Clements
- Keith Randall
- Martin Möhrmann
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Alex Brainman (Gerrit)
Alex Brainman voted and added 4 comments![]( "Open in Gerrit")
Votes added by Alex Brainman
| Commit-Queue | +1 |
4 comments
Thanks for the clarification Alex. I replied below.
I did not review your CL properly. I will do it once we agree what to do here.
Fixes #70558s/Fixes/For/
otherwise the issue will get close when we submit this CL.
print("runtime: VirtualAlloc of ", n, " bytes failed with errno=", errno, "\n")Bill MorganIf you look at the log https://logs.chromium.org/logs/golang/buildbucket/cr-buildbucket/8730352753718672849/+/u/step/19/log/2 mentioned at the issue https://github.com/golang/go/issues/70558#issue-2691890092 , you will see that the stack trace already prints that information, like
```
runtime: VirtualAlloc of 8192 bytes failed with errno=1455
```Why do we need your CL to print similar information?
Thanks, I see what you mean now Alex. I tested by doing `_ = make([]byte, 200*1024*1024*1024*1024)` and did not get the errno printed out so I modified `sysAllocOS` and `sysReserveOS` behavior to be in line with `sysUsedOS` and tested to confirm I got the errno for that huge allocation failure. After a closer re-reading of the issue, I realize this doesn't really address the deeper 'unactionable' part of the original issue Alan mentioned though.
Thanks for explaining what you see. I run this program
```
package main
import (
"fmt"
)
func main() {
_ = make([]byte, 200*1024*1024*1024*1024)
fmt.Printf("OK\n")
}
```and indeed I see this stack trace
```
runtime: out of memory: cannot allocate 219902325555200-byte block (4096000 in use)
fatal error: out of memory
goroutine 1 gp=0xc0000021c0 m=0 mp=0x7ff6619afbe0 [running]:
...
```
so you are correct Windows error number is missing here.
I suggest you add test to make sure that your change does not get lost.
I also think we should stick to the same message format as in
but instead of
```
runtime: VirtualAlloc of 8192 bytes failed with errno=1455
fatal error: out of memory
```
here it should say
```
runtime: VirtualAlloc of 8192 bytes failed with errno=1455: cannot allocate 219902325555200-byte block (4096000 in use)
fatal error: out of memory
```
Please, confirm my suggestion at https://github.com/golang/go/issues/70558 to make sure everyone agrees.
p := stdcall(_VirtualAlloc, uintptr(v), n, _MEM_RESERVE, _PAGE_READWRITE)We need new test and adjust the message text similar to what I suggested in sysAllocOS.
Related details
- Austin Clements
- Bill Morgan
- Keith Randall
- Martin Möhrmann
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Gerrit Bot (Gerrit)
Gerrit Bot uploaded new patchset![]( "Open in Gerrit")
- Alex Brainman
- Austin Clements
- Bill Morgan
- Keith Randall
- Martin Möhrmann
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Gerrit Bot (Gerrit)
Gerrit Bot uploaded new patchset![]( "Open in Gerrit")
- TryBots-Pass: LUCI-TryBot-Result+1 by Go LUCI
Related details
- Alex Brainman
- Austin Clements
- Bill Morgan
- Keith Randall
- Martin Möhrmann
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Gerrit Bot (Gerrit)
Gerrit Bot uploaded new patchset![]( "Open in Gerrit")
Related details
- Alex Brainman
- Austin Clements
- Bill Morgan
- Keith Randall
- Martin Möhrmann
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Gerrit Bot (Gerrit)
Gerrit Bot uploaded new patchset![]( "Open in Gerrit")
Related details
- Alex Brainman
- Austin Clements
- Bill Morgan
- Keith Randall
- Martin Möhrmann
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Bill Morgan (Gerrit)
Bill Morgan added 4 comments![]( "Open in Gerrit")
Thanks Alex. I have pushed updates and commented on the issue.
s/Fixes/For/
otherwise the issue will get close when we submit this CL.
Done
print("runtime: VirtualAlloc of ", n, " bytes failed with errno=", errno, "\n")Done
p := stdcall(_VirtualAlloc, uintptr(v), n, _MEM_RESERVE, _PAGE_READWRITE)We need new test and adjust the message text similar to what I suggested in sysAllocOS.
- Alex Brainman
- Austin Clements
- Keith Randall
- Martin Möhrmann
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |