llvm dev - Dec 2011 - [LLVMdev] bug in ARMFrameLowering.cpp:processFunctionBeforeCalleeSavedScan

Hello LLVMDev,

I've encountered what looks like a bug in LLVM 2.9, it doesn't appear to
be fixed in 3.0 either.

The problem occurs in function
ARMFrameLowering.cpp:processFunctionBeforeCalleeSavedScan. There's a
circular dependency in setting the variable BigStack and
AFI->setHashStackFrame(true). The expression which initializes BigStack calls
estimateRSSStackSizeLimit which in turn checks AFI->hasStackFrame().
Unfortunately setHasStackFrame(true) only gets called after BigStack is
initialized.

The specific issue this caused in my case was that BigStack got initialized as
false because estimateRSSStackSizeLimit returned (1<<12)-1 instead of
(1<<8)-1. The code thus never entered the if (BigStack &&
!ExtraCSSpill) block further down where setScavengingFrameIndex should have been
set. As a result of it was not being set there, there was an assertion failure
(ScavengingFrameIndex >= 0) and subsequent memory corruption in
RegScavenger::scavengeRegister.

I fixed this by performing the CanEliminateFrame and
RegInfo->cannotEliminateFrame checks before the call to
estimateRSSStackSizeLimit, since these values are available before BigStack is
initialized. Does that sound reasonable? I've attached a patch with my
change.

Oh, I should mention that the previous version of LLVM we used, 2.6, didn't
include the check to hasStackFrame() in estimateRSSStackSizeLimit and worked on
the same input.

Thanks,
-alok.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20111205/3c7aacd7/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: ARMFrameLowering.patch
Type: application/octet-stream
Size: 613 bytes
Desc: ARMFrameLowering.patch
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20111205/3c7aacd7/attachment.obj>

Hello Alok,
> I fixed this by performing the CanEliminateFrame and
> RegInfo->cannotEliminateFrame checks before the call to
> estimateRSSStackSizeLimit, since these values are available before BigStack
> is initialized. Does that sound reasonable? I’ve attached a patch with my
> change.
Will you please provide a testcase which reproduces the problem?

Thanks!
-- 
With best regards, Anton Korobeynikov
Faculty of Mathematics and Mechanics, Saint Petersburg State University

Hi Alok,

Echoing Anton's statements, please provide a testcase! :)

Apart from that, looking at your patch and description I actually think that it
is estimateRSStackSizeLimit that is wrong.

It is performing a check for a Thumb2 addressing mode whether the target has a
FP register. That's the main check - for some reason it is appending another
check AFI->hasStackFrame() which seems wrong. The function should assume the
function has a stack frame, because its entire function is to estimate its size!

It looks like that check has been copy-pasted from elsewhere in the file. Anton,
would you agree with this? I haven't looked at ARMFrameLowering before.

Cheers,

James

-----Original Message-----
From: llvmdev-bounces at cs.uiuc.edu [mailto:llvmdev-bounces at cs.uiuc.edu] On
Behalf Of Anton Korobeynikov
Sent: 05 December 2011 11:11
To: Alok Manchanda
Cc: llvmdev at cs.uiuc.edu
Subject: Re: [LLVMdev] bug in
ARMFrameLowering.cpp:processFunctionBeforeCalleeSavedScan

Hello Alok,
> I fixed this by performing the CanEliminateFrame and
> RegInfo->cannotEliminateFrame checks before the call to
> estimateRSSStackSizeLimit, since these values are available before BigStack
> is initialized. Does that sound reasonable? I’ve attached a patch with my
> change.
Will you please provide a testcase which reproduces the problem?

Thanks!
-- 
With best regards, Anton Korobeynikov
Faculty of Mathematics and Mechanics, Saint Petersburg State University

_______________________________________________
LLVM Developers mailing list
LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev