-
Notifications
You must be signed in to change notification settings - Fork 18k
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
cmd/compile: internal compiler error: not lowered: v108, Zero SSA PTR SSA #59174
Comments
The culprit found by git bisect CL 454255 @jake-ciolek |
Thanks for the report. I just had a quick look and it's because the clear length ends up being negative, in this case it's -8: v108 = Zero {uint8} [-8] v101 v106 The arch-specific lowering rules don't match negative Zero arguments. I think my patch just surfaced the issue because I don't think passing negative integers to the memclrNoHeapPointers is the expected behavior. I'll look more later, but I think there's another bug here. The memclrNoHeapPointers doesn't run because a panic gets raised by panicmakeslicelen but I don't think we should ever call it with negative lengths. 1.19 assembly for reference:
To summarize, I think we should panic on any clears to memclrNoHeapPointers with size < 0 , I don't think it's safe to use negative-sized clears as the routines don't seem to be adapted to that. |
Even if we make memclrNoHeapPointers panics on negative size, the right fix here is applying the optimization only if size is not negative IMHO. |
@cuonglm Perhaps we can turn it into a zero sized clear? Then it should elide the unreachable memclrNoHeapPointers call. |
I'm not sure it's correct behavior. The program must panic at runtime. If you turn it into zero size, it does not panic! |
So I tried clamping negative clears to 0 and the program doesn't miscompile anymore. Now the assembly looks like:
The progam still panics on runtime:
If the branch is never taken (due to the panicmakeslicelen call) then we might as well remove the memcrlNoHeapPointers call as it should never fire - this reduces the code size. We probably don't ever want to call memcrlNoHeapPointers with negative sized clears so it also deals with that. |
@jake-ciolek ah cool, I thought that the call to makeslice would be elided. |
Change https://go.dev/cl/478375 mentions this issue: |
Doesn't crash with go1.20.2.
The text was updated successfully, but these errors were encountered: