[dev.ssa] cmd/compile: fix possible invalid pointer spill in large Zero/Move on ARM

Instead of comparing the address of the end of the memory to zero/copy,
comparing the address of the last element, which is a valid pointer.
Also unify large and unaligned Zero/Move, by passing alignment as AuxInt.

Fixes #16515 for ARM.

Change-Id: I19a62b31c5acf5c55c16a89bea1039c926dc91e5
Reviewed-on: https://go-review.googlesource.com/25300
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>

diff --git a/src/cmd/compile/internal/arm/ssa.go b/src/cmd/compile/internal/arm/ssa.go
index bdb3c36cf409822dfc4ecbe724de7bd832d35315..f16dc0f95f2ce6396f195f87b77efb8620f05a05 100644 (file)
--- a/src/cmd/compile/internal/arm/ssa.go
+++ b/src/cmd/compile/internal/arm/ssa.go
@@ -797,7 +797,7 @@ func ssaGenValue(s *gc.SSAGenState, v *ssa.Value) {
                                        }
                                        return
                                }
-                       case ssa.OpARMDUFFZERO, ssa.OpARMLoweredZero, ssa.OpARMLoweredZeroU:
+                       case ssa.OpARMDUFFZERO, ssa.OpARMLoweredZero:
                                // arg0 is ptr
                                if w.Args[0] == v.Args[0] {
                                        if gc.Debug_checknil != 0 && int(v.Line) > 1 {
@@ -805,7 +805,7 @@ func ssaGenValue(s *gc.SSAGenState, v *ssa.Value) {
                                        }
                                        return
                                }
-                       case ssa.OpARMDUFFCOPY, ssa.OpARMLoweredMove, ssa.OpARMLoweredMoveU:
+                       case ssa.OpARMDUFFCOPY, ssa.OpARMLoweredMove:
                                // arg0 is dst ptr, arg1 is src ptr
                                if w.Args[0] == v.Args[0] || w.Args[1] == v.Args[0] {
                                        if gc.Debug_checknil != 0 && int(v.Line) > 1 {
@@ -835,15 +835,23 @@ func ssaGenValue(s *gc.SSAGenState, v *ssa.Value) {
                if gc.Debug_checknil != 0 && v.Line > 1 { // v.Line==1 in generated wrappers
                        gc.Warnl(v.Line, "generated nil check")
                }
-       case ssa.OpARMLoweredZero, ssa.OpARMLoweredZeroU:
+       case ssa.OpARMLoweredZero:
                // MOVW.P       Rarg2, 4(R1)
                // CMP  Rarg1, R1
-               // BLT  -2(PC)
-               // arg1 is the end of memory to zero
+               // BLE  -2(PC)
+               // arg1 is the address of the last element to zero
                // arg2 is known to be zero
-               var sz int64 = 4
-               mov := arm.AMOVW
-               if v.Op == ssa.OpARMLoweredZeroU { // unaligned
+               // auxint is alignment
+               var sz int64
+               var mov obj.As
+               switch {
+               case v.AuxInt%4 == 0:
+                       sz = 4
+                       mov = arm.AMOVW
+               case v.AuxInt%2 == 0:
+                       sz = 2
+                       mov = arm.AMOVH
+               default:
                        sz = 1
                        mov = arm.AMOVB
                }
@@ -858,18 +866,26 @@ func ssaGenValue(s *gc.SSAGenState, v *ssa.Value) {
                p2.From.Type = obj.TYPE_REG
                p2.From.Reg = gc.SSARegNum(v.Args[1])
                p2.Reg = arm.REG_R1
-               p3 := gc.Prog(arm.ABLT)
+               p3 := gc.Prog(arm.ABLE)
                p3.To.Type = obj.TYPE_BRANCH
                gc.Patch(p3, p)
-       case ssa.OpARMLoweredMove, ssa.OpARMLoweredMoveU:
+       case ssa.OpARMLoweredMove:
                // MOVW.P       4(R1), Rtmp
                // MOVW.P       Rtmp, 4(R2)
                // CMP  Rarg2, R1
-               // BLT  -3(PC)
-               // arg2 is the end of src
-               var sz int64 = 4
-               mov := arm.AMOVW
-               if v.Op == ssa.OpARMLoweredMoveU { // unaligned
+               // BLE  -3(PC)
+               // arg2 is the address of the last element of src
+               // auxint is alignment
+               var sz int64
+               var mov obj.As
+               switch {
+               case v.AuxInt%4 == 0:
+                       sz = 4
+                       mov = arm.AMOVW
+               case v.AuxInt%2 == 0:
+                       sz = 2
+                       mov = arm.AMOVH
+               default:
                        sz = 1
                        mov = arm.AMOVB
                }
@@ -891,7 +907,7 @@ func ssaGenValue(s *gc.SSAGenState, v *ssa.Value) {
                p3.From.Type = obj.TYPE_REG
                p3.From.Reg = gc.SSARegNum(v.Args[2])
                p3.Reg = arm.REG_R1
-               p4 := gc.Prog(arm.ABLT)
+               p4 := gc.Prog(arm.ABLE)
                p4.To.Type = obj.TYPE_BRANCH
                gc.Patch(p4, p)
        case ssa.OpVarDef:

diff --git a/src/cmd/compile/internal/ssa/gen/ARM.rules b/src/cmd/compile/internal/ssa/gen/ARM.rules
index 17d151d824327d520b49f85d7baf8fa0e4906591..54d7395d0cdf312a2ce16c8e942c3da01595d59b 100644 (file)
--- a/src/cmd/compile/internal/ssa/gen/ARM.rules
+++ b/src/cmd/compile/internal/ssa/gen/ARM.rules
@@ -302,13 +302,12 @@
 
 // Large zeroing uses a loop
 (Zero [s] ptr mem)
-       && SizeAndAlign(s).Size()%4 == 0 && (SizeAndAlign(s).Size() > 512 || config.noDuffDevice)
-       && SizeAndAlign(s).Align()%4 == 0 ->
-       (LoweredZero ptr (ADDconst <ptr.Type> ptr [SizeAndAlign(s).Size()]) (MOVWconst [0]) mem)
-
-// Unaligned zeroing uses a loop
-(Zero [s] ptr mem) && SizeAndAlign(s).Size() > 4 && SizeAndAlign(s).Align()%4 != 0 ->
-       (LoweredZeroU ptr (ADDconst <ptr.Type> ptr [SizeAndAlign(s).Size()]) (MOVWconst [0]) mem)
+       && (SizeAndAlign(s).Size() > 512 || config.noDuffDevice) || SizeAndAlign(s).Align()%4 != 0 ->
+       (LoweredZero [SizeAndAlign(s).Align()]
+               ptr
+               (ADDconst <ptr.Type> ptr [SizeAndAlign(s).Size()-moveSize(SizeAndAlign(s).Align(), config)])
+               (MOVWconst [0])
+               mem)
 
 // moves
 (Move [s] _ _ mem) && SizeAndAlign(s).Size() == 0 -> mem
@@ -343,13 +342,12 @@
 
 // Large move uses a loop
 (Move [s] dst src mem)
-       && SizeAndAlign(s).Size()%4 == 0 && (SizeAndAlign(s).Size() > 512 || config.noDuffDevice)
-       && SizeAndAlign(s).Align()%4 == 0 ->
-       (LoweredMove dst src (ADDconst <src.Type> src [SizeAndAlign(s).Size()]) mem)
-
-// Unaligned move uses a loop
-(Move [s] dst src mem) && SizeAndAlign(s).Size() > 4 && SizeAndAlign(s).Align()%4 != 0 ->
-       (LoweredMoveU dst src (ADDconst <src.Type> src [SizeAndAlign(s).Size()]) mem)
+       && (SizeAndAlign(s).Size() > 512 || config.noDuffDevice) || SizeAndAlign(s).Align()%4 != 0 ->
+       (LoweredMove [SizeAndAlign(s).Align()]
+               dst
+               src
+               (ADDconst <src.Type> src [SizeAndAlign(s).Size()-moveSize(SizeAndAlign(s).Align(), config)])
+               mem)
 
 // calls
 (StaticCall [argwid] {target} mem) -> (CALLstatic [argwid] {target} mem)

diff --git a/src/cmd/compile/internal/ssa/gen/ARMOps.go b/src/cmd/compile/internal/ssa/gen/ARMOps.go
index 85e1e2f941be4dcdc79bc689292554997965c349..865dc1034ed56ab3857cd0ade1a77494cd870978 100644 (file)
--- a/src/cmd/compile/internal/ssa/gen/ARMOps.go
+++ b/src/cmd/compile/internal/ssa/gen/ARMOps.go
@@ -415,17 +415,18 @@ func init() {
                        },
                },
 
-               // large zeroing (must be 4-byte aligned)
+               // large or unaligned zeroing
                // arg0 = address of memory to zero (in R1, changed as side effect)
-               // arg1 = address of the end of the memory to zero
+               // arg1 = address of the last element to zero
                // arg2 = value to store (always zero)
                // arg3 = mem
                // returns mem
                //      MOVW.P  Rarg2, 4(R1)
                //      CMP     R1, Rarg1
-               //      BLT     -2(PC)
+               //      BLE     -2(PC)
                {
                        name:      "LoweredZero",
+                       aux:       "Int64",
                        argLength: 4,
                        reg: regInfo{
                                inputs:   []regMask{buildReg("R1"), gp, gp},
@@ -433,55 +434,19 @@ func init() {
                        },
                },
 
-               // large move (must be 4-byte aligned)
+               // large or unaligned move
                // arg0 = address of dst memory (in R2, changed as side effect)
                // arg1 = address of src memory (in R1, changed as side effect)
-               // arg2 = address of the end of src memory
+               // arg2 = address of the last element of src
                // arg3 = mem
                // returns mem
                //      MOVW.P  4(R1), Rtmp
                //      MOVW.P  Rtmp, 4(R2)
                //      CMP     R1, Rarg2
-               //      BLT     -3(PC)
+               //      BLE     -3(PC)
                {
                        name:      "LoweredMove",
-                       argLength: 4,
-                       reg: regInfo{
-                               inputs:   []regMask{buildReg("R2"), buildReg("R1"), gp},
-                               clobbers: buildReg("R1 R2 FLAGS"),
-                       },
-               },
-
-               // unaligned zeroing
-               // arg0 = address of memory to zero (in R1, changed as side effect)
-               // arg1 = address of the end of the memory to zero
-               // arg2 = value to store (always zero)
-               // arg3 = mem
-               // returns mem
-               //      MOVB.P  Rarg2, 1(R1)
-               //      CMP     R1, Rarg1
-               //      BLT     -2(PC)
-               {
-                       name:      "LoweredZeroU",
-                       argLength: 4,
-                       reg: regInfo{
-                               inputs:   []regMask{buildReg("R1"), gp, gp},
-                               clobbers: buildReg("R1 FLAGS"),
-                       },
-               },
-
-               // unaligned move
-               // arg0 = address of dst memory (in R2, changed as side effect)
-               // arg1 = address of src memory (in R1, changed as side effect)
-               // arg2 = address of the end of src memory
-               // arg3 = mem
-               // returns mem
-               //      MOVB.P  1(R1), Rtmp
-               //      MOVB.P  Rtmp, 1(R2)
-               //      CMP     R1, Rarg2
-               //      BLT     -3(PC)
-               {
-                       name:      "LoweredMoveU",
+                       aux:       "Int64",
                        argLength: 4,
                        reg: regInfo{
                                inputs:   []regMask{buildReg("R2"), buildReg("R1"), gp},

diff --git a/src/cmd/compile/internal/ssa/opGen.go b/src/cmd/compile/internal/ssa/opGen.go
index 89a79ef1c05110c4817ed42cdb975d53f4862479..d66515402d88f68fcf362eba625747cc4f4dba6a 100644 (file)
--- a/src/cmd/compile/internal/ssa/opGen.go
+++ b/src/cmd/compile/internal/ssa/opGen.go
@@ -786,8 +786,6 @@ const (
        OpARMDUFFCOPY
        OpARMLoweredZero
        OpARMLoweredMove
-       OpARMLoweredZeroU
-       OpARMLoweredMoveU
        OpARMLoweredGetClosurePtr
        OpARMMOVWconvert
        OpARMFlagEQ
@@ -9879,32 +9877,9 @@ var opcodeTable = [...]opInfo{
                },
        },
        {
-               name:   "LoweredZero",
-               argLen: 4,
-               reg: regInfo{
-                       inputs: []inputInfo{
-                               {0, 2},    // R1
-                               {1, 5119}, // R0 R1 R2 R3 R4 R5 R6 R7 R8 R9 R12
-                               {2, 5119}, // R0 R1 R2 R3 R4 R5 R6 R7 R8 R9 R12
-                       },
-                       clobbers: 4294967298, // R1 FLAGS
-               },
-       },
-       {
-               name:   "LoweredMove",
-               argLen: 4,
-               reg: regInfo{
-                       inputs: []inputInfo{
-                               {0, 4},    // R2
-                               {1, 2},    // R1
-                               {2, 5119}, // R0 R1 R2 R3 R4 R5 R6 R7 R8 R9 R12
-                       },
-                       clobbers: 4294967302, // R1 R2 FLAGS
-               },
-       },
-       {
-               name:   "LoweredZeroU",
-               argLen: 4,
+               name:    "LoweredZero",
+               auxType: auxInt64,
+               argLen:  4,
                reg: regInfo{
                        inputs: []inputInfo{
                                {0, 2},    // R1
@@ -9915,8 +9890,9 @@ var opcodeTable = [...]opInfo{
                },
        },
        {
-               name:   "LoweredMoveU",
-               argLen: 4,
+               name:    "LoweredMove",
+               auxType: auxInt64,
+               argLen:  4,
                reg: regInfo{
                        inputs: []inputInfo{
                                {0, 4},    // R2

diff --git a/src/cmd/compile/internal/ssa/rewriteARM.go b/src/cmd/compile/internal/ssa/rewriteARM.go
index c391ffa43623c30e56345957685d3842322d5f79..eb000d7460fd0a56b2a1c856741b38a3361fa7ed 100644 (file)
--- a/src/cmd/compile/internal/ssa/rewriteARM.go
+++ b/src/cmd/compile/internal/ssa/rewriteARM.go
@@ -10696,43 +10696,23 @@ func rewriteValueARM_OpMove(v *Value, config *Config) bool {
                return true
        }
        // match: (Move [s] dst src mem)
-       // cond: SizeAndAlign(s).Size()%4 == 0 && (SizeAndAlign(s).Size() > 512 || config.noDuffDevice)         && SizeAndAlign(s).Align()%4 == 0
-       // result: (LoweredMove dst src (ADDconst <src.Type> src [SizeAndAlign(s).Size()]) mem)
+       // cond: (SizeAndAlign(s).Size() > 512 || config.noDuffDevice) || SizeAndAlign(s).Align()%4 != 0
+       // result: (LoweredMove [SizeAndAlign(s).Align()]               dst             src             (ADDconst <src.Type> src [SizeAndAlign(s).Size()-moveSize(SizeAndAlign(s).Align(), config)])            mem)
        for {
                s := v.AuxInt
                dst := v.Args[0]
                src := v.Args[1]
                mem := v.Args[2]
-               if !(SizeAndAlign(s).Size()%4 == 0 && (SizeAndAlign(s).Size() > 512 || config.noDuffDevice) && SizeAndAlign(s).Align()%4 == 0) {
+               if !((SizeAndAlign(s).Size() > 512 || config.noDuffDevice) || SizeAndAlign(s).Align()%4 != 0) {
                        break
                }
                v.reset(OpARMLoweredMove)
+               v.AuxInt = SizeAndAlign(s).Align()
                v.AddArg(dst)
                v.AddArg(src)
                v0 := b.NewValue0(v.Line, OpARMADDconst, src.Type)
                v0.AddArg(src)
-               v0.AuxInt = SizeAndAlign(s).Size()
-               v.AddArg(v0)
-               v.AddArg(mem)
-               return true
-       }
-       // match: (Move [s] dst src mem)
-       // cond: SizeAndAlign(s).Size() > 4 && SizeAndAlign(s).Align()%4 != 0
-       // result: (LoweredMoveU dst src (ADDconst <src.Type> src [SizeAndAlign(s).Size()]) mem)
-       for {
-               s := v.AuxInt
-               dst := v.Args[0]
-               src := v.Args[1]
-               mem := v.Args[2]
-               if !(SizeAndAlign(s).Size() > 4 && SizeAndAlign(s).Align()%4 != 0) {
-                       break
-               }
-               v.reset(OpARMLoweredMoveU)
-               v.AddArg(dst)
-               v.AddArg(src)
-               v0 := b.NewValue0(v.Line, OpARMADDconst, src.Type)
-               v0.AddArg(src)
-               v0.AuxInt = SizeAndAlign(s).Size()
+               v0.AuxInt = SizeAndAlign(s).Size() - moveSize(SizeAndAlign(s).Align(), config)
                v.AddArg(v0)
                v.AddArg(mem)
                return true
@@ -16763,42 +16743,21 @@ func rewriteValueARM_OpZero(v *Value, config *Config) bool {
                return true
        }
        // match: (Zero [s] ptr mem)
-       // cond: SizeAndAlign(s).Size()%4 == 0 && (SizeAndAlign(s).Size() > 512 || config.noDuffDevice)         && SizeAndAlign(s).Align()%4 == 0
-       // result: (LoweredZero ptr (ADDconst <ptr.Type> ptr [SizeAndAlign(s).Size()]) (MOVWconst [0]) mem)
+       // cond: (SizeAndAlign(s).Size() > 512 || config.noDuffDevice) || SizeAndAlign(s).Align()%4 != 0
+       // result: (LoweredZero [SizeAndAlign(s).Align()]               ptr             (ADDconst <ptr.Type> ptr [SizeAndAlign(s).Size()-moveSize(SizeAndAlign(s).Align(), config)])            (MOVWconst [0])                 mem)
        for {
                s := v.AuxInt
                ptr := v.Args[0]
                mem := v.Args[1]
-               if !(SizeAndAlign(s).Size()%4 == 0 && (SizeAndAlign(s).Size() > 512 || config.noDuffDevice) && SizeAndAlign(s).Align()%4 == 0) {
+               if !((SizeAndAlign(s).Size() > 512 || config.noDuffDevice) || SizeAndAlign(s).Align()%4 != 0) {
                        break
                }
                v.reset(OpARMLoweredZero)
+               v.AuxInt = SizeAndAlign(s).Align()
                v.AddArg(ptr)
                v0 := b.NewValue0(v.Line, OpARMADDconst, ptr.Type)
                v0.AddArg(ptr)
-               v0.AuxInt = SizeAndAlign(s).Size()
-               v.AddArg(v0)
-               v1 := b.NewValue0(v.Line, OpARMMOVWconst, config.fe.TypeUInt32())
-               v1.AuxInt = 0
-               v.AddArg(v1)
-               v.AddArg(mem)
-               return true
-       }
-       // match: (Zero [s] ptr mem)
-       // cond: SizeAndAlign(s).Size() > 4 && SizeAndAlign(s).Align()%4 != 0
-       // result: (LoweredZeroU ptr (ADDconst <ptr.Type> ptr [SizeAndAlign(s).Size()]) (MOVWconst [0]) mem)
-       for {
-               s := v.AuxInt
-               ptr := v.Args[0]
-               mem := v.Args[1]
-               if !(SizeAndAlign(s).Size() > 4 && SizeAndAlign(s).Align()%4 != 0) {
-                       break
-               }
-               v.reset(OpARMLoweredZeroU)
-               v.AddArg(ptr)
-               v0 := b.NewValue0(v.Line, OpARMADDconst, ptr.Type)
-               v0.AddArg(ptr)
-               v0.AuxInt = SizeAndAlign(s).Size()
+               v0.AuxInt = SizeAndAlign(s).Size() - moveSize(SizeAndAlign(s).Align(), config)
                v.AddArg(v0)
                v1 := b.NewValue0(v.Line, OpARMMOVWconst, config.fe.TypeUInt32())
                v1.AuxInt = 0