From 1b44167d055464f79c026d2023953ba7efdbcfe6 Mon Sep 17 00:00:00 2001 From: Austin Clements Date: Sat, 28 Apr 2018 22:00:36 -0400 Subject: [PATCH] cmd/internal/obj/arm: fix/rationalize checkpool distance check When deciding whether to flush the constant pool, the distance check in checkpool can fail to account for padding inserted before the next instruction by nacl. For example, see this failure: https://go-review.googlesource.com/c/go/+/109350/2#message-07085b591227824bb1d646a7192cbfa7e0b97066 Here, the pool should be flushed before a CALL instruction, but checkpool only considers the CALL instruction to be 4 bytes and doesn't account for the 8 extra bytes of alignment padding added before it by asmoutnacl. As a result, it flushes the pool after the CALL instruction, which is 4 bytes too late. Furthermore, there's no explanation for the rather convoluted expression used to decide if we need to emit the constant pool. This CL modifies checkpool to take the PC following the tentative instruction as an argument. The caller knows this already and this way checkpool doesn't have to guess (and get it wrong in the presence of padding). In the process, it rewrites the test to be structured and commented. Change-Id: I32a3d50ffb5a94d42be943e9bcd49036c7e9b95c Reviewed-on: https://go-review.googlesource.com/110017 Run-TryBot: Austin Clements TryBot-Result: Gobot Gobot Reviewed-by: Cherry Zhang --- src/cmd/internal/obj/arm/asm5.go | 42 ++++++++++++++++++++++---------- 1 file changed, 29 insertions(+), 13 deletions(-) diff --git a/src/cmd/internal/obj/arm/asm5.go b/src/cmd/internal/obj/arm/asm5.go index a88ae74d31..96f6b90e8d 100644 --- a/src/cmd/internal/obj/arm/asm5.go +++ b/src/cmd/internal/obj/arm/asm5.go @@ -668,12 +668,11 @@ func span5(ctxt *obj.Link, cursym *obj.LSym, newprog obj.ProgAlloc) { op = p p = p.Link - var i int var m int var o *Optab for ; p != nil || c.blitrl != nil; op, p = p, p.Link { if p == nil { - if c.checkpool(op, 0) { + if c.checkpool(op, pc) { p = op continue } @@ -700,8 +699,12 @@ func span5(ctxt *obj.Link, cursym *obj.LSym, newprog obj.ProgAlloc) { // must check literal pool here in case p generates many instructions if c.blitrl != nil { - i = m - if c.checkpool(op, i) { + // Emit the constant pool just before p if p + // would push us over the immediate size limit. + if c.checkpool(op, pc+int32(m)) { + // Back up to the instruction just + // before the pool and continue with + // the first instruction of the pool. p = op continue } @@ -872,7 +875,7 @@ func span5(ctxt *obj.Link, cursym *obj.LSym, newprog obj.ProgAlloc) { pc += 4 } - for i = 0; i < m/4; i++ { + for i := 0; i < m/4; i++ { v = int(out[i]) bp[0] = byte(v) bp = bp[1:] @@ -888,14 +891,26 @@ func span5(ctxt *obj.Link, cursym *obj.LSym, newprog obj.ProgAlloc) { } } -/* - * when the first reference to the literal pool threatens - * to go out of range of a 12-bit PC-relative offset, - * drop the pool now, and branch round it. - * this happens only in extended basic blocks that exceed 4k. - */ -func (c *ctxt5) checkpool(p *obj.Prog, sz int) bool { - if c.pool.size >= 0xff0 || immaddr(int32((p.Pc+int64(sz)+4)+4+int64(12+c.pool.size)-int64(c.pool.start+8))) == 0 { +// checkpool flushes the literal pool when the first reference to +// it threatens to go out of range of a 12-bit PC-relative offset. +// +// nextpc is the tentative next PC at which the pool could be emitted. +// checkpool should be called *before* emitting the instruction that +// would cause the PC to reach nextpc. +// If nextpc is too far from the first pool reference, checkpool will +// flush the pool immediately after p. +// The caller should resume processing a p.Link. +func (c *ctxt5) checkpool(p *obj.Prog, nextpc int32) bool { + poolLast := nextpc + poolLast += 4 // the AB instruction to jump around the pool + poolLast += 12 // the maximum nacl alignment padding for ADATABUNDLE + poolLast += int32(c.pool.size) - 4 // the offset of the last pool entry + + refPC := int32(c.pool.start) // PC of the first pool reference + + v := poolLast - refPC - 8 // 12-bit PC-relative offset (see omvl) + + if c.pool.size >= 0xff0 || immaddr(v) == 0 { return c.flushpool(p, 1, 0) } else if p.Link == nil { return c.flushpool(p, 2, 0) @@ -1016,6 +1031,7 @@ func (c *ctxt5) addpool(p *obj.Prog, a *obj.Addr) { c.elitrl = q c.pool.size += 4 + // Store the link to the pool entry in Pcond. p.Pcond = q }