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. -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20130718/c48090cb/attachment.html>
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
On Thu, Jul 18, 2013 at 10:31 PM, Yun-Wei Lee <lee2041412 at gmail.com> wrote:> Thank for your suggestion. > > I have tried to use GetAlignedArgumentStackSize. > It is so strange that this function may output uncorrectly. > For example, the StackSize will be 40 when handling the third function call > in main. > If I modified the this function, it will fail one test. > I wonder this may be another bug in llvm...... > And that's why I didn't use GetAlignedArgumentStackSize previously. > > Thank you!I'm sorry, I'm not that familiar with the code. A brief look tells me that it's likely none of the patches are quite right; http://llvm.org/bugs/attachment.cgi?id=10897 looks closer, but still not quite right. -Eli> > On Thu, Jul 18, 2013 at 7:56 PM, Eli Friedman <eli.friedman at gmail.com> > wrote: >> >> 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 > >
Hi All, Please find enclosed an updated and simplified patch along with a additional test for properly aligned variadic arguments. Tom On Fri, Jul 19, 2013 at 8:36 PM, Eli Friedman <eli.friedman at gmail.com> wrote:> On Thu, Jul 18, 2013 at 10:31 PM, Yun-Wei Lee <lee2041412 at gmail.com> > wrote: > > Thank for your suggestion. > > > > I have tried to use GetAlignedArgumentStackSize. > > It is so strange that this function may output uncorrectly. > > For example, the StackSize will be 40 when handling the third function > call > > in main. > > If I modified the this function, it will fail one test. > > I wonder this may be another bug in llvm...... > > And that's why I didn't use GetAlignedArgumentStackSize previously. > > > > Thank you! > > I'm sorry, I'm not that familiar with the code. A brief look tells me > that it's likely none of the patches are quite right; > http://llvm.org/bugs/attachment.cgi?id=10897 looks closer, but still > not quite right. > > -Eli > > > > > On Thu, Jul 18, 2013 at 7:56 PM, Eli Friedman <eli.friedman at gmail.com> > > wrote: > >> > >> 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 > > > > > _______________________________________________ > LLVM Developers mailing list > LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20140613/eecf02d9/attachment.html> -------------- next part -------------- A non-text attachment was scrubbed... Name: variadicAlignment.patch Type: text/x-patch Size: 2539 bytes Desc: not available URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20140613/eecf02d9/attachment.bin>