Maxim Ostapenko via llvm-dev
2015-Nov-23 11:41 UTC
[llvm-dev] asan for allocas on powerpc64
Jay, do you have a PowerPC64 target? If so, could you please check attached patch on PPC box? This is a draft patch, but it would be nice to make sure that we are moving to right direction here. Thanks, -Maxim On 18/11/15 00:12, Jay Foad wrote:>>> Currently test/asan/TestCases/alloca_vla_interact.cc is XFAILed for >>> powerpc64. I've had a look at why it doesn't work. I think the only >>> problem is in the call to __asan_allocas_unpoison that is inserted at >>> the end of the "for" loop (just before a stackrestore instruction). >>> >>> The call function is created something like this (paraphrasing from >>> lib/Transfoms/Instrumentation/AddressSanitizer.cpp): >>> >>> // call __asan_allocas_unpoison(uptr top, uptr bottom); >>> // NB "top" here means lowest address and "bottom" means highest! >>> >>> IRB.CreateCall( >>> AsanAllocasUnpoisonFunc, >>> { >>> IRB.CreateLoad(DynamicAllocaLayout), >>> IRB.CreatePointerToInt(SaveRestoreInst->getOperand(0), IntptrTy) >>> } >>> ); >>> >>> I think the problem is that the operand to stackrestore is the new >>> native sp register value to restore, and this code is assuming that >>> that will be a higher address than all the allocas that are being >>> unallocated. But on PowerPC64, the native sp is always lower than the >>> address of the most recent alloca by MaxCallFrameSize bytes, to leave >>> space for outgoing call arguments. So I think the second argument to >>> __asan_allocas_unpoison needs to be SaveRestoreInst->getOperand(0) + >>> MaxCallFrameSize, but I don't know how to implement that. >>> >>> Thoughts? >>> >> Yeah, you are right, we rely on stackrestore to get "bottom" parameter for >> __asan_allocas_unpoison and indeed PowerPC64 seems to be special here. >> However, right now I don't see a suitable way how to get MaxCallFrameSize >> from LLVM backend in ASan pass, investigation is in progress. > One idea I had for a solution was to change llvm.stacksave so that it > *always* returned native sp + MaxCallFrameSize, and do the > corresponding adjustment in llvm.stackrestore. In most cases this > would just replace a mov instruction with an addi instruction. > > I don't know if this would cause other problems, or if it would be > acceptable to the target maintainers. > > Jay. >-------------- next part -------------- A non-text attachment was scrubbed... Name: dynamic_area_offset.diff Type: text/x-patch Size: 16112 bytes Desc: not available URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20151123/2193563a/attachment.bin>
Maxim, It seems to be doing roughly the right thing, but it isn't quite enough to fix the test case. getDynamicAreaOffset() is returning 0x20, but from looking at the generated code for the test case, the real offset from native sp to the allocated area is 0x60. I will run it under gdb and see if I can spot the problem... Thanks! Jay. On 23 November 2015 at 11:41, Maxim Ostapenko < m.ostapenko at partner.samsung.com> wrote:> Jay, do you have a PowerPC64 target? If so, could you please check > attached patch on PPC box? This is a draft patch, but it would be nice to > make sure that we are moving to right direction here. > > Thanks, > -Maxim > > > On 18/11/15 00:12, Jay Foad wrote: > >> Currently test/asan/TestCases/alloca_vla_interact.cc is XFAILed for >>>> powerpc64. I've had a look at why it doesn't work. I think the only >>>> problem is in the call to __asan_allocas_unpoison that is inserted >>>> at >>>> the end of the "for" loop (just before a stackrestore instruction). >>>> >>>> The call function is created something like this (paraphrasing from >>>> lib/Transfoms/Instrumentation/AddressSanitizer.cpp): >>>> >>>> // call __asan_allocas_unpoison(uptr top, uptr bottom); >>>> // NB "top" here means lowest address and "bottom" means highest! >>>> >>>> IRB.CreateCall( >>>> AsanAllocasUnpoisonFunc, >>>> { >>>> IRB.CreateLoad(DynamicAllocaLayout), >>>> IRB.CreatePointerToInt(SaveRestoreInst->getOperand(0), IntptrTy) >>>> } >>>> ); >>>> >>>> I think the problem is that the operand to stackrestore is the new >>>> native sp register value to restore, and this code is assuming that >>>> that will be a higher address than all the allocas that are being >>>> unallocated. But on PowerPC64, the native sp is always lower than >>>> the >>>> address of the most recent alloca by MaxCallFrameSize bytes, to >>>> leave >>>> space for outgoing call arguments. So I think the second argument >>>> to >>>> __asan_allocas_unpoison needs to be SaveRestoreInst->getOperand(0) >>>> + >>>> MaxCallFrameSize, but I don't know how to implement that. >>>> >>>> Thoughts? >>>> >>>> Yeah, you are right, we rely on stackrestore to get "bottom" parameter >>> for >>> __asan_allocas_unpoison and indeed PowerPC64 seems to be special here. >>> However, right now I don't see a suitable way how to get MaxCallFrameSize >>> from LLVM backend in ASan pass, investigation is in progress. >>> >> One idea I had for a solution was to change llvm.stacksave so that it >> *always* returned native sp + MaxCallFrameSize, and do the >> corresponding adjustment in llvm.stackrestore. In most cases this >> would just replace a mov instruction with an addi instruction. >> >> I don't know if this would cause other problems, or if it would be >> acceptable to the target maintainers. >> >> Jay. >> >> >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20151123/c9fab987/attachment.html>
In LowerGET_DYNAMIC_AREA_OFFSET() you're calling MFI->getMaxCallFrameSize(), but it looks like that doesn't return useful information until after the PrologEpilogInserter's PEI::calculateCallsInformation() has run. So maybe the lowering has to be done as part of frame index elimination? (I'm not too familiar with this code.) Jay. On 23 November 2015 at 13:07, Jay Foad <jay.foad at gmail.com> wrote:> Maxim, > > It seems to be doing roughly the right thing, but it isn't quite enough to > fix the test case. getDynamicAreaOffset() is returning 0x20, but from > looking at the generated code for the test case, the real offset from > native sp to the allocated area is 0x60. > > I will run it under gdb and see if I can spot the problem... > > Thanks! > Jay. > > On 23 November 2015 at 11:41, Maxim Ostapenko < > m.ostapenko at partner.samsung.com> wrote: > >> Jay, do you have a PowerPC64 target? If so, could you please check >> attached patch on PPC box? This is a draft patch, but it would be nice to >> make sure that we are moving to right direction here. >> >> Thanks, >> -Maxim >> >> >> On 18/11/15 00:12, Jay Foad wrote: >> >>> Currently test/asan/TestCases/alloca_vla_interact.cc is XFAILed for >>>>> powerpc64. I've had a look at why it doesn't work. I think the >>>>> only >>>>> problem is in the call to __asan_allocas_unpoison that is >>>>> inserted at >>>>> the end of the "for" loop (just before a stackrestore >>>>> instruction). >>>>> >>>>> The call function is created something like this (paraphrasing >>>>> from >>>>> lib/Transfoms/Instrumentation/AddressSanitizer.cpp): >>>>> >>>>> // call __asan_allocas_unpoison(uptr top, uptr bottom); >>>>> // NB "top" here means lowest address and "bottom" means >>>>> highest! >>>>> >>>>> IRB.CreateCall( >>>>> AsanAllocasUnpoisonFunc, >>>>> { >>>>> IRB.CreateLoad(DynamicAllocaLayout), >>>>> IRB.CreatePointerToInt(SaveRestoreInst->getOperand(0), IntptrTy) >>>>> } >>>>> ); >>>>> >>>>> I think the problem is that the operand to stackrestore is the new >>>>> native sp register value to restore, and this code is assuming >>>>> that >>>>> that will be a higher address than all the allocas that are being >>>>> unallocated. But on PowerPC64, the native sp is always lower than >>>>> the >>>>> address of the most recent alloca by MaxCallFrameSize bytes, to >>>>> leave >>>>> space for outgoing call arguments. So I think the second argument >>>>> to >>>>> __asan_allocas_unpoison needs to be >>>>> SaveRestoreInst->getOperand(0) + >>>>> MaxCallFrameSize, but I don't know how to implement that. >>>>> >>>>> Thoughts? >>>>> >>>>> Yeah, you are right, we rely on stackrestore to get "bottom" parameter >>>> for >>>> __asan_allocas_unpoison and indeed PowerPC64 seems to be special here. >>>> However, right now I don't see a suitable way how to get >>>> MaxCallFrameSize >>>> from LLVM backend in ASan pass, investigation is in progress. >>>> >>> One idea I had for a solution was to change llvm.stacksave so that it >>> *always* returned native sp + MaxCallFrameSize, and do the >>> corresponding adjustment in llvm.stackrestore. In most cases this >>> would just replace a mov instruction with an addi instruction. >>> >>> I don't know if this would cause other problems, or if it would be >>> acceptable to the target maintainers. >>> >>> Jay. >>> >>> >> >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20151123/5f721b23/attachment.html>