Rajesh Viswabramana
2013-Jun-18 11:26 UTC
[LLVMdev] ARM struct byval size > 64 triggers failure
An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20130618/5b2a15e3/attachment.html> -------------- next part -------------- A non-text attachment was scrubbed... Name: 201306181656803_BEI0XT4N.gif Type: image/gif Size: 14036 bytes Desc: not available URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20130618/5b2a15e3/attachment.gif>
Hi Rajesh, Stepan and Manman were changing that part of the code recently, so they can have better answer than I do. On 18 June 2013 12:26, Rajesh Viswabramana <rajesh.vis at samsung.com> wrote:> *Output on arm :* > > # ./check114.exe > values 0x10861, 0x4071706f > which is wrong. >This works on my Chromebook, is this soft-float? Can you show us your compilation options and to which target you're compiling to, please?> When the offset(#76) for the instruction, "ldr r1, [r11, #76] ; 0x4c" > is calculated, 4 bytes alignment is considered. > In prologue stackpointer calculation 8 byte alignment is considered. > > Due to this mimatch of alignment, If try to access any parameter after > byval which results wrong value. > > Issue(or offset of 4 bytes) wont occur if even number of register used for > byval spilling. > ex: > struct S114 check114 (int a, struct S114 arg0, struct S114* arg1) { // > accessing arg1 is fine in this case > ..... > } >This looks like a bug. ;)> 1) Is this 8 byte alignment mandatory ? Is this due to " ARM AAPCS > 5.2.1.2 Stack constraints at a public interface" ? Can this be removed? >I'm not sure what you want removed here. AAPCS should be followed to guarantee compatibility. It looks to me as though the code is considering different constraints in different places, and all we need to do is to match the logic on both places. 2) We will leave alignment as it is but in prologue we will adjust SP once> again, this is little meaningless. > > 3) While accessing arg1 we will consider alignment and add extra offset -> > looks better. >I think both are bad fixes, as they don't fix the correct problem. Once the bug is fixed on trunk, can you use trunk LLVM? Or do you have to use a point-version? Anyway, changing your code to work around compiler issues is always a bad idea... Offset to access arg1 is calculated by selection DAG that will be target> independent. But Alignment adjustment should be done by target lowering. > Any suggestions on how to fix this ? >I'm not too familiar with that part of the code, but Manman or Stepan can give you a hand here. cheers, --renato -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20130618/a9d4ba0d/attachment.html>
Hi Rajesh, The callee code looks okay to me> Assembly for check114 > --------------------------------------------------------------- > sub sp, sp, #16 > push {r11, lr} > mov r11, sp > sub sp, sp, #8 > str r3, [r11, #20] > str r2, [r11, #16] > str r1, [r11, #12] > ldr r1, [r11, #76]VARegSaveSize is 16 because we store the first 16 bytes of struct byval in r0 to r3. Align in computeRegArea is 8 since ABI says the stack pointer needs to be 8 byte aligned at function entry point. But the second argument does not have to be 8 byte aligned, in fact it is 4 byte aligned for i32. r11, #76 is equivalent to sp_at_entry + 52 since r11 = spat_entry - 16 - 8, which is 4-byte aligned after storing the leftover (67-16=51) bytes of struct byval. Can you also paste the assembly for the caller side and check whether the second argument is stored at sp_at_entry+52? As Renato suggested, please file a bug report. Thanks, Manman On Jun 18, 2013, at 4:26 AM, Rajesh Viswabramana <rajesh.vis at samsung.com> wrote:> Hi, > > Handling of pass by val of struct size >64 bytes case is seems wrong for arm targets. > > Summary: > Incase of struct pass by value for size > 64 along with other function params, failure seen in some corner cases. Access to function params result in wrong stack location access. > Stack pointer adjustment done by prologue emitter and offset used to access function params have different logics for calculaton. > > Test code > --------------------------------------------------------------- > #include <stdio.h> > struct S114 { > char a[67]; > }a114[5]; > struct S114 check114 (struct S114 arg0, struct S114* arg1) { > if(&a114[0] != arg1) // arg1 value is wrong > printf( "values %p, %p\n", &a114[0], arg1); > } > int main () { > int i= 0, j = 0; > for (;j<2; j++) // just filling a114 with some values for identification > for(i=0; i<sizeof(struct S114); i++) > memset(&a114[j].a[i],(0x11+i+j*30), sizeof(int)); > check114 (a114[1], &a114[0]); //=> a114[0] is accessed from wrong location inside check114 function > } > --------------------------------------------------------------- > clang -v > clang version 3.3 (tags/RELEASE_33/final) > Target: i386-pc-linux-gnu > Thread model: posix > > Output on arm : > # ./check114.exe > values 0x10861, 0x4071706f > which is wrong. > > Assembly for check114 > --------------------------------------------------------------- > sub sp, sp, #16 > push {r11, lr} > mov r11, sp > sub sp, sp, #8 > str r3, [r11, #20] > str r2, [r11, #16] > str r1, [r11, #12] > ldr r1, [r11, #76] > str r1, [sp, #4] > .loc 1 7 0 prologue_end > ldr r2, .LCPI0_0 > cmp r2, r1 > beq .LBB0_2 > b .LBB0_1 > --------------------------------------------------------------- > > From reg, stack dump: > ------------------------------------------------------------------------------------------------------------------------------ > @entry of check114 > => 0x8398 <check114>: sub sp, sp, #16 > 0x839c <check114+4>: push {r11, lr} > sp 0xbefff808 0xbefff808 > @if condition > 0x83b4 <check114+28>: ldr r1, [r11, #76] ; 0x4c <--- wrong value copied to r1, offset #76 should be #80 > 0x83b8 <check114+32>: str r1, [sp, #4] > 0x83bc <check114+36>: ldr r2, [pc, #44] ; 0x83f0 <check114+88> > => 0x83c0 <check114+40>: cmp r2, r1 > > r11 0xbefff7f0 -1090521104 > sp 0xbefff7e8 0xbefff7e8 > Stack dump: > 0xbefff7e4: 0x4001ed08 0x40024f90 0x4071706f 0xbefff8a0 > 0xbefff7f4: 0x0000869c 0x00000000 0x3231302f 0x36353433 > 0xbefff804: 0x3a393837 0x3e3d3c3b 0x4241403f 0x46454443 > 0xbefff814: 0x4a494847 0x4e4d4c4b 0x5251504f 0x56555453 > 0xbefff824: 0x5a595857 0x5e5d5c5b 0x6261605f 0x66656463 > 0xbefff834: 0x6a696867 0x6e6d6c6b 0x4071706f 0x00010861 //[R11+4c] -> [0xbefff7f0+4c] -> [0xbefff83c] -> 0x4071706f > Correct value is at location {[R11+4c]+4} --> 0x00010861, 4 bytes offset going wrong. > ------------------------------------------------------------------------------------------------------------------------------ > When i checked from the ARM Lowering part for generation of > sub sp, sp, #16 > Emitted by, > if (VARegSaveSize) > emitSPUpdate(isARM, MBB, MBBI, dl, TII, -VARegSaveSize, // --> VARegSaveSize is calculated in computeRegArea > MachineInstr::FrameSetup) > > ARMTargetLowering::computeRegArea(..) { > ... > VARegSize = NumGPRs * 4; > VARegSaveSize = (VARegSize + Align - 1) & ~(Align - 1); // --> 8 byte alignment done here > } > Stack pointer decremented to NumGPRs*4 + alignment > NumGPRs = 3 registers > VARegSaveSize = 16 (after considering 8 byte alignment ) > > When the offset(#76) for the instruction, "ldr r1, [r11, #76] ; 0x4c" is calculated, 4 bytes alignment is considered. > In prologue stackpointer calculation 8 byte alignment is considered. > Due to this mimatch of alignment, If try to access any parameter after byval which results wrong value. > > Issue(or offset of 4 bytes) wont occur if even number of register used for byval spilling. > ex: > struct S114 check114 (int a, struct S114 arg0, struct S114* arg1) { // accessing arg1 is fine in this case > ..... > } > > Could someone comment on below queries about fixing the problem, > 1) Is this 8 byte alignment mandatory ? Is this due to " ARM AAPCS 5.2.1.2 Stack constraints at a public interface" ? Can this be removed? > 2) We will leave alignment as it is but in prologue we will adjust SP once again, this is little meaningless. > 3) While accessing arg1 we will consider alignment and add extra offset -> looks better. > Offset to access arg1 is calculated by selection DAG that will be target independent. But Alignment adjustment should be done by target lowering. Any suggestions on how to fix this ? > > Regards, > Rajesh > > <201306181656803_BEI0XT4N.gif> > _______________________________________________ > 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/20130618/947a601e/attachment.html>
Stepan Dyatkovskiy
2013-Jun-18 20:15 UTC
[LLVMdev] ARM struct byval size > 64 triggers failure
<div>Hi all,</div><div>One more interesting job :-)</div><div>I'll look too at this case tomorrow. Today my brain is about to be exploded..</div><div>š</div><div>-Stepan.</div><div>š</div><div>18.06.2013, 22:56, "Manman Ren" <mren@apple.com>:</div><blockquote type="cite"><div>š</div>Hi Rajesh,<div>š</div><div>The callee code looks okay to me</div><div><blockquote type="cite"><div style="line-height:1.4;margin:10px;font-family:Arial,arial;font-size:9pt;"><p style="margin-top:5px;margin-bottom:5px;font-size:9pt;">Assembly for check114š<br />---------------------------------------------------------------<br />ššššššš subšššš sp, sp, #16<br />ššššššš pushššš {r11, lr}<br />ššššššš movšššš r11, sp<br />ššššššš subšššš sp, sp, #8<br />ššššššš stršššš r3, [r11, #20]<br />ššššššš stršššš r2, [r11, #16]<br />ššššššš stršššš r1, [r11, #12]<br />ššššššš ldršššš r1, [r11, #76]</p></div></blockquote><div><div>VARegSaveSize is 16 because we store the first 16 bytes of struct byval in r0 to r3.</div><div>Align in computeRegArea is 8 since ABI says the stack pointer needs to be 8 byte aligned at function entry point.</div><div>But the second argument does not have to be 8 byte aligned, in fact it is 4 byte aligned for i32.</div><div>š</div><div>r11, #76 is equivalent to sp_at_entry + 52 since r11 = spat_entry - 16 - 8, which is 4-byte aligned after</div><div>storing the leftover (67-16=51) bytes of struct byval.</div><div>š</div><div>Can you also paste the assembly for the caller side and check whether the second argument is stored</div><div>at sp_at_entry+52?</div><div>š</div><div>As Renato suggested, please file a bug report.</div><div>š</div><div>Thanks,</div><div>Manman</div><div>š</div><div>š</div><div>On Jun 18, 2013, at 4:26 AM, Rajesh Viswabramana <<a href="mailto:rajesh.vis@samsung.com">rajesh.vis@samsung.com</a>> wrote:</div><br /><blockquote type="cite"><div style="line-height:1.4;margin:10px;font-family:Arial,arial;font-size:9pt;text-align:start;text-transform:none;"><p style="margin-top:5px;font-family:Arial,arial;margin-bottom:5px;font-size:9pt;">Hi,</p><div style="margin-top:5px;font-family:Arial,arial;margin-bottom:5px;font-size:9pt;">š</div><p style="margin-top:5px;font-family:Arial,arial;margin-bottom:5px;font-size:9pt;">Handling of pass by val of struct size >64 bytes case is seems wrong for arm targets.</p><p style="margin-top:5px;font-family:Arial,arial;margin-bottom:5px;font-size:9pt;">š</p><p style="margin-top:5px;font-family:Arial,arial;margin-bottom:5px;font-size:9pt;"><strong>Summary:</strong></p><p style="margin-top:5px;font-family:Arial,arial;margin-bottom:5px;font-size:9pt;">Incase of struct pass by value for size > 64 along with other function params, failure seen in some corner cases. Access to function params result in wrong stack location access.š</p><p style="margin-top:5px;font-family:Arial,arial;margin-bottom:5px;font-size:9pt;">Stack pointer adjustment done by prologue emitter and offset used to access function params have different logics for calculaton.</p><p style="margin-top:5px;font-family:Arial,arial;margin-bottom:5px;font-size:9pt;">š</p><p style="margin-top:5px;font-family:Arial,arial;margin-bottom:5px;font-size:9pt;"><strong>Test code<br /></strong>---------------------------------------------------------------<br />#include <stdio.h><br />struct S114 {<br />š char a[67];<br />}a114[5];</p><p style="margin-top:5px;font-family:Arial,arial;margin-bottom:5px;font-size:9pt;">struct S114 check114 (struct S114 arg0, struct S114* arg1) {š<br />š if(&a114[0] != arg1)šššššššššššššššššššššššš // arg1 value is wrong<br />ššš printf( "values %p, %p\n", &a114[0], arg1);<br />}<br />int main () {<br />š int i= 0, j = 0;<br />š for (;j<2; j++)ššššššššššššššššššššššššššššššššššš // just filling a114 with some values for identification<br />ššš for(i=0; i<sizeof(struct S114); i++)<br />ššššš memset(&a114[j].a[i],(0x11+i+j*30), sizeof(int));š</p><p style="margin-top:5px;font-family:Arial,arial;margin-bottom:5px;font-size:9pt;">š check114 (a114[1], &a114[0]);ššššššš š//=> a114[0]š is accessed from wrong location insidešcheck114 function<br />}<br />---------------------------------------------------------------<br />clang -v<br />clang version 3.3 (tags/RELEASE_33/final)<br />Target: i386-pc-linux-gnu<br />Thread model: posix</p><div style="margin-top:5px;font-family:Arial,arial;margin-bottom:5px;font-size:9pt;">š</div><p style="margin-top:5px;font-family:Arial,arial;margin-bottom:5px;font-size:9pt;"><strong>Output on arm :<br /></strong># ./check114.exe<span>š</span><br />values 0x10861, 0x<span>4071706</span>f<br />which is wrong.</p><div style="margin-top:5px;font-family:Arial,arial;margin-bottom:5px;font-size:9pt;">š</div><p style="margin-top:5px;font-family:Arial,arial;margin-bottom:5px;font-size:9pt;">Assembly for check114<span>š</span><br />---------------------------------------------------------------<br />ššššššš subšššš sp, sp, #16<br />ššššššš pushššš {r11, lr}<br />ššššššš movšššš r11, sp<br />ššššššš subšššš sp, sp, #8<br />ššššššš stršššš r3, [r11, #20]<br />ššššššš stršššš r2, [r11, #16]<br />ššššššš stršššš r1, [r11, #12]<br />ššššššš ldršššš r1, [r11, #76]<br />ššššššš stršššš r1, [sp, #4]<br />ššššššš .locššš 1 7 0 prologue_end<br />ššššššš ldršššš r2, .LCPI0_0<br />ššššššš cmpšššš r2, r1<br />ššššššš beqšššš .LBB0_2<br />ššššššš bšššššš .LBB0_1<br />---------------------------------------------------------------</p><div style="margin-top:5px;font-family:Arial,arial;margin-bottom:5px;font-size:9pt;">š</div><p style="margin-top:5px;font-family:Arial,arial;margin-bottom:5px;font-size:9pt;">From reg, stack dump:<br />------------------------------------------------------------------------------------------------------------------------------<br />@entry of check114<br />šš=> 0x8398 <check114>:šsubšsp, sp, #16<br />ššššššš 0x839c <check114+4>:špushš{r11, lr}<br />ššspšššššššššššš 0xbefff808š0xbefff808</p><p style="margin-top:5px;font-family:Arial,arial;margin-bottom:5px;font-size:9pt;">@if condition<br />šššššš 0x83b4 <check114+28>:šldršr1, [r11, #76]š; 0x4cšššššššš <--- wrong value copied to r1, offset #76 should be #80<br />ššššššš0x83b8 <check114+32>:šstršr1, [sp, #4]<br />šššššš 0x83bc <check114+36>:šldršr2, [pc, #44]š; 0x83f0 <check114+88><br />š=> 0x83c0 <check114+40>:šcmpšr2, r1<br />šš<br />ššr11ššššššššššš 0xbefff7f0š-<span>1090521104</span><br />ššspšššššššššššš 0xbefff7e8š0xbefff7e8</p><p style="margin-top:5px;font-family:Arial,arial;margin-bottom:5px;font-size:9pt;">ššStack dump:<br />šš0xbefff7e4:š0x4001ed08š0x40024f90š0x<span>4071706</span>fš0xbefff8a0<br />šš0xbefff7f4:š0x<span>0000869</span>cš0x<span>00000000</span>š0x<span>3231302</span>fš0x<span>36353433</span><br />šš0xbefff804:š0x3a393837š0x3e3d3c3bš0x<span>4241403</span>fš0x<span>46454443</span><br />šš0xbefff814:š0x4a494847š0x4e4d4c4bš0x<span>5251504</span>fš0x<span>56555453</span><br />šš0xbefff824:š0x5a595857š0x5e5d5c5bš0x<span>6261605</span>fš0x<span>66656463</span><br />šš0xbefff834:š0x6a696867š0x6e6d6c6b<strong>š0x<span>4071706</span>f</strong>š<strong>0x<span>00010861</span></strong>ššššššššššššššššš//[R11+4c] -> [0xbefff7f0+4c] -> [0xbefff83c] -> 0x<span>4071706</span>f</p><p style="margin-top:5px;font-family:Arial,arial;margin-bottom:5px;font-size:9pt;">Correct value is at locationš{[R11+4c]<strong>+4</strong>} --> 0x<span>00010861</span>, 4 bytes offset going wrong.<br />------------------------------------------------------------------------------------------------------------------------------</p><p style="margin-top:5px;font-family:Arial,arial;margin-bottom:5px;font-size:9pt;">When i checked from the ARM Lowering partšfor generation of<br />ššsubšsp, sp, #16</p><p style="margin-top:5px;font-family:Arial,arial;margin-bottom:5px;font-size:9pt;">Emitted by,</p><p style="margin-top:5px;font-family:Arial,arial;margin-bottom:5px;font-size:9pt;">š if (VARegSaveSize)<br />ššš emitSPUpdate(isARM, MBB, MBBI, dl, TII, -VARegSaveSize,ššššššš // -->šVARegSaveSize is calculated in computeRegArea<br />šššššššššššššššš MachineInstr::FrameSetup)<br /><br />ARMTargetLowering::computeRegArea(..) {<br />š ...<br />š VARegSize = NumGPRs * 4;<br />š VARegSaveSize = (VARegSize + Align - 1) & ~(Align - 1);ššššššššššššššš š// --> 8 byte alignment done here<br />}</p><p style="margin-top:5px;font-family:Arial,arial;margin-bottom:5px;font-size:9pt;">Stack pointer decremented to NumGPRs*4 + alignment</p><p style="margin-top:5px;font-family:Arial,arial;margin-bottom:5px;font-size:9pt;">NumGPRs = 3 registers</p><p style="margin-top:5px;font-family:Arial,arial;margin-bottom:5px;font-size:9pt;">VARegSaveSizeš = 16 (after considering 8 byte alignmentš)</p><p style="margin-top:5px;font-family:Arial,arial;margin-bottom:5px;font-size:9pt;"><br />When the offset(#76) for the instruction, "ldršr1, [r11, #76]š; 0x4c"š isšcalculated,š4 bytes alignment is considered.<br />In prologuešstackpointeršcalculation 8 byte alignment is considered.</p><p style="margin-top:5px;font-family:Arial,arial;margin-bottom:5px;font-size:9pt;">Due to this mimatch of alignment, If try to access any parameter after byval whichšresultsšwrong value.</p><div style="margin-top:5px;font-family:Arial,arial;margin-bottom:5px;font-size:9pt;">š</div><p style="margin-top:5px;font-family:Arial,arial;margin-bottom:5px;font-size:9pt;">Issue(or offset of 4 bytes) wont occur if even number of register used for byval spilling.<br />ex:<span>š</span><br />struct S114 check114 (int a, struct S114 arg0, struct S114* arg1) { // accessing arg1 is fine in this case<br />.....<br />}</p><div style="margin-top:5px;font-family:Arial,arial;margin-bottom:5px;font-size:9pt;">š</div><p style="margin-top:5px;font-family:Arial,arial;margin-bottom:5px;font-size:9pt;">Could someonešcomment onšbelow queries about fixing the problem,</p><p style="margin-top:5px;font-family:Arial,arial;margin-bottom:5px;font-size:9pt;">1) Is this 8 byte alignment mandatoryš?š Is this due to " ARM AAPCS 5.2.1.2 Stack constraints at a public interface" ? Can this be removed?</p><p style="margin-top:5px;font-family:Arial,arial;margin-bottom:5px;font-size:9pt;">2) We will leave alignment as it is but in prologue we will adjust SP once again, this is little meaningless.</p><p style="margin-top:5px;font-family:Arial,arial;margin-bottom:5px;font-size:9pt;">3) While accessing arg1 we will consider alignment and add extra offset -> looks better.</p><p style="margin-top:5px;font-family:Arial,arial;margin-bottom:5px;font-size:9pt;">šOffset tošaccess arg1 is calculated by selection DAG that will be target independent. But Alignment adjustment should be done by target lowering. Any suggestions on how to fix this ?</p><div style="margin-top:5px;font-family:Arial,arial;margin-bottom:5px;font-size:9pt;">š</div><p style="margin-top:5px;font-family:Arial,arial;margin-bottom:5px;font-size:9pt;">Regards,<br />Rajesh</p><div style="margin-top:5px;font-family:Arial,arial;margin-bottom:5px;font-size:9pt;">š</div><table><tbody><tr><td style="margin-top:5px;font-family:Arial,arial;margin-bottom:5px;font-size:9pt;"><p style="margin-top:5px;font-family:Arial,arial;margin-bottom:5px;font-size:9pt;"><span><<span>201306181656803</span>_BEI0XT4N.gif></span></p></td></tr></tbody></table>_______________________________________________<br />LLVM Developers mailing list<br /><a href="mailto:LLVMdev@cs.uiuc.edu">LLVMdev@cs.uiuc.edu</a><span>š</span>šššššššš<a href="http://llvm.cs.uiuc.edu/">http://llvm.cs.uiuc.edu</a><br /><a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev">http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev</a></div></blockquote></div></div>,<p>_______________________________________________<br />LLVM Developers mailing list<br /><a href="mailto:LLVMdev@cs.uiuc.edu">LLVMdev@cs.uiuc.edu</a> šššššššš<a href="http://llvm.cs.uiuc.edu/">http://llvm.cs.uiuc.edu</a><br /><a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev">http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev</a></p></blockquote>