[PATCH] fix for PR16393: miscompile with struct byval

Manman Ren mren at apple.com
Thu Jul 11 21:06:22 PDT 2013


On Jul 11, 2013, at 4:37 PM, Jakob Stoklund Olesen wrote:

> 
> On Jul 11, 2013, at 12:07 PM, Manman Ren <mren at apple.com> wrote:
> 
>> <byval_spadj_verify.patch>
> 
> Thanks, Manman
> 
> I still think the logic would be simpler if you write the loop like this:
> 
> for (...) {
> if (I->getOpcode() == FrameSetupOpcode) {
>   ...
> } 
> if (I->getOpcode() == FrameDestroyOpcode) {
>   ...
> }
> }
Done
> 
> Also:
> 
> +    int SPAdj = 0;
> +    bool IsSetup = false;
> +    int SPAdjAfter = SPAdj;
> +    bool IsSetupAfter = IsSetup;
> 
> Should this just be an instance of the struct you just defined?
Done
> 
> +      if (I->getOpcode() == FrameSetupOpcode)
> +        Size = -Size;
> +      SPAdjAfter += Size;
> 
> Why the math? If these things don’t nest, it should never be necessary to add or subtract stack adjustments.
If there is no verification error, SPAdj should be either 0 or the amount by a FrameSetup.
But if we want to continue while there are errors, we can calculate SPAdj as above?

Thanks,
Manman
-------------- next part --------------
A non-text attachment was scrubbed...
Name: byval_spadj_verify.patch
Type: application/octet-stream
Size: 6036 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130711/6c6a4143/attachment.obj>
-------------- next part --------------

> 
> With those changes, the verifier patch looks good to me.
> 
> Thanks,
> /jakob
> 



More information about the llvm-commits mailing list