Skip to content

Commit

Permalink
cmd/compile: call ginsnop, not ginsnop2 on ppc64le for mid-stack inli…
Browse files Browse the repository at this point in the history
…ning tracebacks

A recent change to fix stacktraces for inlined functions
introduced a regression on ppc64le when compiling position
independent code. That happened because ginsnop2 was called for
the purpose of inserting a NOP to identify the location of
the inlined function, when ginsnop should have been used.
ginsnop2 is intended to be used before deferreturn to ensure
r2 is properly restored when compiling position independent code.
In some cases the location where r2 is loaded from might not be
initialized. If that happens and r2 is used to generate an address,
the result is likely a SEGV.

This fixes that problem.

Fixes #30283

Change-Id: If70ef27fc65ef31969712422306ac3a57adbd5b6
Reviewed-on: https://go-review.googlesource.com/c/163337
Reviewed-by: Cherry Zhang <[email protected]>
Reviewed-by: Carlos Eduardo Seo <[email protected]>
Reviewed-by: Keith Randall <[email protected]>
Run-TryBot: Andrew Bonventre <[email protected]>
TryBot-Result: Gobot Gobot <[email protected]>
  • Loading branch information
laboger authored and ianlancetaylor committed Feb 25, 2019
1 parent 73b803e commit 2d34740
Show file tree
Hide file tree
Showing 11 changed files with 15 additions and 5 deletions.
1 change: 1 addition & 0 deletions src/cmd/compile/internal/amd64/galign.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ func Init(arch *gc.Arch) {
arch.ZeroRange = zerorange
arch.ZeroAuto = zeroAuto
arch.Ginsnop = ginsnop
arch.Ginsnopdefer = ginsnop

arch.SSAMarkMoves = ssaMarkMoves
arch.SSAGenValue = ssaGenValue
Expand Down
1 change: 1 addition & 0 deletions src/cmd/compile/internal/arm/galign.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ func Init(arch *gc.Arch) {
arch.ZeroRange = zerorange
arch.ZeroAuto = zeroAuto
arch.Ginsnop = ginsnop
arch.Ginsnopdefer = ginsnop

arch.SSAMarkMoves = func(s *gc.SSAGenState, b *ssa.Block) {}
arch.SSAGenValue = ssaGenValue
Expand Down
1 change: 1 addition & 0 deletions src/cmd/compile/internal/arm64/galign.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ func Init(arch *gc.Arch) {
arch.ZeroRange = zerorange
arch.ZeroAuto = zeroAuto
arch.Ginsnop = ginsnop
arch.Ginsnopdefer = ginsnop

arch.SSAMarkMoves = func(s *gc.SSAGenState, b *ssa.Block) {}
arch.SSAGenValue = ssaGenValue
Expand Down
7 changes: 4 additions & 3 deletions src/cmd/compile/internal/gc/go.go
Original file line number Diff line number Diff line change
Expand Up @@ -255,9 +255,10 @@ type Arch struct {
Use387 bool // should 386 backend use 387 FP instructions instead of sse2.
SoftFloat bool

PadFrame func(int64) int64
ZeroRange func(*Progs, *obj.Prog, int64, int64, *uint32) *obj.Prog
Ginsnop func(*Progs) *obj.Prog
PadFrame func(int64) int64
ZeroRange func(*Progs, *obj.Prog, int64, int64, *uint32) *obj.Prog
Ginsnop func(*Progs) *obj.Prog
Ginsnopdefer func(*Progs) *obj.Prog // special ginsnop for deferreturn

// SSAMarkMoves marks any MOVXconst ops that need to avoid clobbering flags.
SSAMarkMoves func(*SSAGenState, *ssa.Block)
Expand Down
2 changes: 1 addition & 1 deletion src/cmd/compile/internal/gc/ssa.go
Original file line number Diff line number Diff line change
Expand Up @@ -5604,7 +5604,7 @@ func (s *SSAGenState) PrepareCall(v *ssa.Value) {
// insert an actual hardware NOP that will have the right line number.
// This is different from obj.ANOP, which is a virtual no-op
// that doesn't make it into the instruction stream.
thearch.Ginsnop(s.pp)
thearch.Ginsnopdefer(s.pp)
}

if sym, ok := v.Aux.(*obj.LSym); ok {
Expand Down
1 change: 1 addition & 0 deletions src/cmd/compile/internal/mips/galign.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ func Init(arch *gc.Arch) {
arch.ZeroRange = zerorange
arch.ZeroAuto = zeroAuto
arch.Ginsnop = ginsnop
arch.Ginsnopdefer = ginsnop
arch.SSAMarkMoves = func(s *gc.SSAGenState, b *ssa.Block) {}
arch.SSAGenValue = ssaGenValue
arch.SSAGenBlock = ssaGenBlock
Expand Down
1 change: 1 addition & 0 deletions src/cmd/compile/internal/mips64/galign.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ func Init(arch *gc.Arch) {
arch.ZeroRange = zerorange
arch.ZeroAuto = zeroAuto
arch.Ginsnop = ginsnop
arch.Ginsnopdefer = ginsnop

arch.SSAMarkMoves = func(s *gc.SSAGenState, b *ssa.Block) {}
arch.SSAGenValue = ssaGenValue
Expand Down
3 changes: 2 additions & 1 deletion src/cmd/compile/internal/ppc64/galign.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@ func Init(arch *gc.Arch) {

arch.ZeroRange = zerorange
arch.ZeroAuto = zeroAuto
arch.Ginsnop = ginsnop2
arch.Ginsnop = ginsnop
arch.Ginsnopdefer = ginsnop2

arch.SSAMarkMoves = ssaMarkMoves
arch.SSAGenValue = ssaGenValue
Expand Down
1 change: 1 addition & 0 deletions src/cmd/compile/internal/s390x/galign.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ func Init(arch *gc.Arch) {
arch.ZeroRange = zerorange
arch.ZeroAuto = zeroAuto
arch.Ginsnop = ginsnop
arch.Ginsnopdefer = ginsnop

arch.SSAMarkMoves = ssaMarkMoves
arch.SSAGenValue = ssaGenValue
Expand Down
1 change: 1 addition & 0 deletions src/cmd/compile/internal/wasm/ssa.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ func Init(arch *gc.Arch) {
arch.ZeroRange = zeroRange
arch.ZeroAuto = zeroAuto
arch.Ginsnop = ginsnop
arch.Ginsnopdefer = ginsnop

arch.SSAMarkMoves = ssaMarkMoves
arch.SSAGenValue = ssaGenValue
Expand Down
1 change: 1 addition & 0 deletions src/cmd/compile/internal/x86/galign.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ func Init(arch *gc.Arch) {
arch.ZeroRange = zerorange
arch.ZeroAuto = zeroAuto
arch.Ginsnop = ginsnop
arch.Ginsnopdefer = ginsnop

arch.SSAMarkMoves = ssaMarkMoves
}

0 comments on commit 2d34740

Please sign in to comment.