llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
28.68k stars 11.86k forks source link

coldcc miscompiles catastrophically on x86_64 #72321

Open mcy opened 11 months ago

mcy commented 11 months ago

I was messing around with using coldcc for some optimizations in a frontend, and I wrote some code to see how it changed the codegen.

https://godbolt.org/z/The9EKP9x

target triple = "x86_64-unknown-linux-gnu"

%T = type {ptr, i64}
define coldcc %T @get_span() {
  call i32 @printf(ptr @fmt.0, ptr null, i64 0)
  ret %T zeroinitializer
}

define i32 @main() {
  %span = call %T @get_span()
  %span.0 = extractvalue %T %span, 0
  %span.1 = extractvalue %T %span, 1
  call i32 @printf(ptr @fmt.0, ptr %span.0, i64 %span.1) 
  ret i32 42
}

@fmt.0 = constant [9 x i8] c"%p, %lu\0A\00"
declare i32 @printf(ptr, ...)

This program is miscompiled at both -O0 and -O2. In the former, it outputs (nil), 0\n(nil), 140723522928744\n, while in the latter, it outputs nothing and incorrectly has an exit code of 0. The expected output is (nil), 0\n(nil), 0\n.

In the former case, the second half of the return value of @get_span is clobbered by a pop rdx in the epilogue, despite the fact that this function uses rdx as an ABI register, so it can't be callee-saved. In the latter case, calling @get_span causes main to be replaced with unreachable.

As far as I can tell, coldcc does not document that it implies noreturn, but it feels like that's what I'm seeing here.

asl commented 11 months ago

You are missing coldcc on your call. Essentially you're calling get_span with wrong calling convention.

From langref: https://llvm.org/docs/LangRef.html#i-call

The optional “cconv” marker indicates which calling convention the call should use. If none is specified, the call defaults to using C calling conventions. The calling convention of the call must match the calling convention of the target function, or else the behavior is undefined.

mcy commented 11 months ago

Oops, nice catch. However, that does not fix the bug: https://godbolt.org/z/3d6cYcW4c.

Bigcheese commented 11 months ago

The MIR for main is correct. get_span is correct until we do prologue/epilogue insertion, at which point rdx is overwritten with a pop because CSR_64_MostRegs_SaveList contains rdx. Pretty sure rdx shouldn't be in that list given the ABI, or we need a new RetCC for coldcc that doesn't use rdx.

jaens commented 3 weeks ago

Still broken (judging from Compiler Explorer), the "invalid" label should probably be removed...