[LLVMdev] Request to review patch for bug #14792

[Eli Friedman]

On Thu, Jul 18, 2013 at 8:36 AM, Yun-Wei Lee <lee2041412 at gmail.com> wrote:
> http://llvm.org/bugs/show_bug.cgi?id=14792
>
> Problem:
>   In the i386 ABI Page 3-10, it said that the stack is aligned. However, the
> two example code show that does not handle the alignment correctly when
> using variadic function. For example, if the size of the first argument is
> 17, the overflow_arg_area in va_list will be set to "address of first
> argument + 16" instead of "address of first argument + 24" after calling
> va_start.
>   In addition, #6636 showed the same problem because in AMD64, arguments is
> passed by register at first, then pass by memory when run out of register
> (AMD64 ABI 3.5.7 rule 10).
>
> Why this problem happened?
>   When calling va_start to set va_list, overflow_arg_area is not set
> correctly. To set the overflow_arg_area correctly, we need to get the
> FrameIndex correctly. Now, here comes the problem, llvm doesn't handle it
> correctly. It accounts for StackSize to compute the FrameIndex, and if the
> StackSize is not aligned, it will compute the wrong FrameIndex. As a result
> overflow_arg_area will not be set correctly.
>
> My Solution:
> 1. Record the Align if it is located in Memory.
> 2. If it is variadic function and needs to set FrameIndex, adjust the
> stacksize.

Please read http://llvm.org/docs/DeveloperPolicy.html .  In
particular, patches should be sent to llvm-commits, and patches should
generally include a regression test.

In terms of the code, you might want to consider using llvm::RoundUpToAlignment.

-Eli