[PATCH] D28166: Properly merge K&R functions that have attributes

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In https://reviews.llvm.org/D28166#633595, @rsmith wrote:

> The test failure in test/CodeGen/microsoft-call-conv-x64.c definitely indicates a problem. The code has defined behavior, but the IR you say we now produce has undefined behavior due to a type mismatch between the call and the callee.
>
> It looks to me like unprototyped `__stdcall` lowering is broken (prior to your change). Consider:
>
>   void __stdcall g(int n) {}
>   void __stdcall (*p)() = g;
>   void f() { p(0); }
>
>
> The types of `p` and `g` are compatible (`g`'s parameter type list does not end in an ellipsis and its parameter type `int` is a promoted type, so it is compatible with an unprototyped function), so the above program is valid, and a call to `f` has defined behavior.
>
> And yet we lower the definition of `g` to `define void @g(i32 %n) ` and the call to
>
>   %0 = load void (...)*, void (...)** @p, align 8
>   %callee.knr.cast = bitcast void (...)* %0 to void (i64)*
>   call void %callee.knr.cast(i64 0)
>
>
> ... resulting in undefined behavior.


Thank you for the explanation -- that makes sense to me.

Do you think this patch should be gated on (or perhaps combined with) a fix for the lowering bug, or do you think this patch is reasonable on its own? Given that it turns working code into UB, I think my preference is to gate it on a fix for the lowering bug, but I'm also not certain I am the right person to implement that fix (though I could give it a shot).


https://reviews.llvm.org/D28166