Skip to content
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

New Go's 1.2(1)2 heap allocator crashes the app #442

Closed
sergystepanov opened this issue Feb 7, 2024 · 4 comments · Fixed by #444
Closed

New Go's 1.2(1)2 heap allocator crashes the app #442

sergystepanov opened this issue Feb 7, 2024 · 4 comments · Fixed by #444

Comments

@sergystepanov
Copy link
Collaborator

sergystepanov commented Feb 7, 2024

Supposedly, the type checker in the newer Go versions randomly crashes the app when it checks unpinned? uintptrs of the YUV planes in the h264 encoder (to be confirmed). The issue is reproducible on Windows and any PSX ROM.
For the time being, it is recommended to change the default encoder to VP8: encoder.video.codec=vpx.

fatal error: unexpected signal during runtime execution
[signal 0xc0000005 code=0x0 addr=0x20 pc=0x7ff7e80caad7]

runtime stack:
runtime.throw({0x7ff7e8aee6b5?, 0xc000060168?})
        C:/Program Files/Go/src/runtime/panic.go:1023 +0x65 fp=0x64cc5ffa38 sp=0x64cc5ffa08 pc=0x7ff7e80f0ca5
runtime.sigpanic()
        C:/Program Files/Go/src/runtime/signal_windows.go:395 +0x265 fp=0x64cc5ffa80 sp=0x64cc5ffa38 pc=0x7ff7e8108be5
runtime.(*mspan).typePointersOfUnchecked(0xc0004bd620?, 0xc00042d900?)
        C:/Program Files/Go/src/runtime/mbitmap_allocheaders.go:202 +0x37 fp=0x64cc5ffaa0 sp=0x64cc5ffa80 pc=0x7ff7e80caad7
runtime.scanobject(0xc000060168?, 0xc000060168)
        C:/Program Files/Go/src/runtime/mgcmark.go:1446 +0xd2 fp=0x64cc5ffb30 sp=0x64cc5ffaa0 pc=0x7ff7e80d6972
runtime.gcDrain(0xc000060168, 0x7)
        C:/Program Files/Go/src/runtime/mgcmark.go:1242 +0x1f4 fp=0x64cc5ffb98 sp=0x64cc5ffb30 pc=0x7ff7e80d62b4
runtime.gcDrainMarkWorkerIdle(...)
        C:/Program Files/Go/src/runtime/mgcmark.go:1114
runtime.gcBgMarkWorker.func2()
        C:/Program Files/Go/src/runtime/mgc.go:1406 +0x6f fp=0x64cc5ffbe8 sp=0x64cc5ffb98 pc=0x7ff7e80d296f
runtime.systemstack(0xc000066e00)
        C:/Program Files/Go/src/runtime/asm_amd64.s:509 +0x49 fp=0x64cc5ffbf8 sp=0x64cc5ffbe8 pc=0x7ff7e8128fc9
@giongto35
Copy link
Owner

hmm, where in the stack trace shows it is from "type checker"? And this can be filed as bug to GO right?

@sergystepanov
Copy link
Collaborator Author

@giongto35, it's in the (*mspan).typePointersOfUnchecked line.
I don't think it is a bug in Go, but rather a race condition during frame encoding and multiple reinitializations of the x264 encoder.
It crashes randomly (right away or on a second attempt) with x264 (but not with vpx), on Windows (but not on cloudretro.io with Linux), on PSX Castlevania where it changes output resolution like crazy, each time recreating the encoder (but if you disable that recreation in the config pcsx.coreAspectRatio: false, it works).
So I assume something has changed in the pointer checker of Go after v1.20, maybe it's way more strict now, and that naïve way of encoder's recreation won't fly anymore. ;/

Well, at least they fixed c-go stack collision introduced in version 1.21, which would burry Go for the project otherwise.

image

@sergystepanov sergystepanov linked a pull request Feb 8, 2024 that will close this issue
3 tasks
@sergystepanov
Copy link
Collaborator Author

sergystepanov commented Feb 8, 2024

After a long debugging session, decided to revert everything back to 1.20 for now.
It crashes under Linux/Windows on Go 1.22 with h264 encoder only (any versions, any settings), when Go's GC is looping over uintptrs and crashing due to the mem corruption. https://tip.golang.org/src/runtime/mbitmap_allocheaders.go
It seems, there are some similar bugs out there in Go, but I couldn't reproduce this issue with an isolated unit test in order to post this bug formally onto the tracker.

Anyways. :/

Steps to reproduce:

  1. Compile with 1.22. (branch go1.22)
  2. Run some PSX games which change output resolution. (Castelvania, or bundled psxtest)
  3. Crash.

@sergystepanov
Copy link
Collaborator Author

sergystepanov commented Feb 10, 2024

@giongto35, I think, I fixed it somehow ;/
Removed a wrapper to Go structure over the C x264_param_t structure and now it works, so it seems. Not sure why, but my guess that new allocator from 1.22 goes beyond mem bounds of this struct. That's possible because the Go wrapper is written a long time ago for older versions of x264 and if they change the order or the list of fields, you can easily go out of bounds (due to Go's structs padding or sizeof mismatch) when mapping an old struct to a new one from C.

type Param struct {
/* CPU flags */
Cpu uint32
IThreads int32 /* encode multiple frames in parallel */

Will test it, then. Maybe, this will fix it. :/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants