[llvm] r293727 - [IPSCCP] Teach how to not propagate return values of naked functions.

[Sean Silva via llvm-commits]

On Tue, Jan 31, 2017 at 5:01 PM, Davide Italiano via llvm-commits <
llvm-commits at lists.llvm.org> wrote:

> Author: davide
> Date: Tue Jan 31 19:01:22 2017
> New Revision: 293727
>
> URL: http://llvm.org/viewvc/llvm-project?rev=293727&view=rev
> Log:
> [IPSCCP] Teach how to not propagate return values of naked functions.
>
> Differential Revision:  https://reviews.llvm.org/D29360
>
> Added:
>     llvm/trunk/test/Transforms/IPConstantProp/naked-return.ll
> Modified:
>     llvm/trunk/lib/Transforms/Scalar/SCCP.cpp
>
> Modified: llvm/trunk/lib/Transforms/Scalar/SCCP.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/
> Transforms/Scalar/SCCP.cpp?rev=293727&r1=293726&r2=293727&view=diff
> ============================================================
> ==================
> --- llvm/trunk/lib/Transforms/Scalar/SCCP.cpp (original)
> +++ llvm/trunk/lib/Transforms/Scalar/SCCP.cpp Tue Jan 31 19:01:22 2017
> @@ -1712,7 +1712,10 @@ static bool runIPSCCP(Module &M, const D
>
>      // If this is an exact definition of this function, then we can
> propagate
>      // information about its result into callsites of it.
> -    if (F.hasExactDefinition())
> +    // Don't touch naked functions. They may contain asm returning a
> +    // value we don't see, so we may end up interprocedurally propagating
> +    // the return value incorrectly.
> +    if (F.hasExactDefinition() && !F.hasFnAttribute(Attribute::Naked))
>        Solver.AddTrackedFunction(&F);
>
>      // If this function only has direct calls that we can see, we can
> track its
>
> Added: llvm/trunk/test/Transforms/IPConstantProp/naked-return.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/
> Transforms/IPConstantProp/naked-return.ll?rev=293727&view=auto
> ============================================================
> ==================
> --- llvm/trunk/test/Transforms/IPConstantProp/naked-return.ll (added)
> +++ llvm/trunk/test/Transforms/IPConstantProp/naked-return.ll Tue Jan 31
> 19:01:22 2017
> @@ -0,0 +1,28 @@
> +; RUN: opt -ipsccp -S %s | FileCheck %s
> +
> +target datalayout = "e-m:x-p:32:32-i64:64-f80:32-n8:16:32-a:0:32-S32"
> +target triple = "i686-pc-windows-msvc19.0.24215"
> +
> +define i32 @dipsy(i32, i32) local_unnamed_addr #0 {
> +BasicBlock0:
> +  call void asm "\0D\0Apushl %ebp\0D\0Amovl 8(%esp),%eax\0D\0Amovl
> 12(%esp), %ebp\0D\0Acalll *%eax\0D\0Apopl %ebp\0D\0Aretl\0D\0A", ""()
> +  ret i32 0
>

If this asm is going to return, shouldn't we have `unreachable` instead of
`ret i32 0`? That would naturally be correct I think without requiring
special handling for naked functions. I.e., the verifier should ensure that
naked functions always end with `unreachable`.

-- Sean Silva


> +}
> +
> +define void @tinkywinky(i32, i32, i32) local_unnamed_addr #0 {
> +BasicBlock1:
> +  call void asm "\0D\0A    movl 12(%esp), %ebp\0D\0A    movl 4(%esp),
> %eax\0D\0A    movl 8(%esp), %esp\0D\0A    jmpl *%eax\0D\0A", ""()
> +  ret void
> +}
> +
> +define void @patatino(i32, i32, i32) local_unnamed_addr #1 {
> +bb:
> +  %3 = tail call i32 @dipsy(i32 %0, i32 %1) #0
> +; Check that we don't accidentally propagate zero.
> +; CHECK: @tinkywinky(i32 %3, i32 %2, i32 %1) #0
> +  tail call void @tinkywinky(i32 %3, i32 %2, i32 %1) #0
> +  ret void
> +}
> +
> +attributes #0 = { naked noinline optnone }
> +attributes #1 = { "no-frame-pointer-elim"="true"
> "no-frame-pointer-elim-non-leaf" }
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170201/9805a5c9/attachment.html>