cmd/compile: do not change field offset in ABI analysis

Currently, the ABI analysis assigns parameter/result offsets
to the fields of function *Type. In some cases, we may have
an ABI0 function reference and an ABIInternal reference share
the same function *Type. For example, for an ABI0 function F,
"f := F" will make f and (ABI0) F having the same *Type. But f,
as a func value, should use ABIInternal. Analyses on F and f will
collide and cause ICE.

Also, changing field offsets in ABI analysis has to be done very
carefully to avoid data races. It has been causing
trickiness/difficulty.

This CL removes the change of field offsets in ABI analysis
altogether. The analysis result is stored in ABIParamAssignment,
which is the only way to access parameter/result stack offset now.

Fixes #47317.
Fixes #47227.

Change-Id: I23a3e081a6cf327ac66855da222daaa636ed1ead
Reviewed-on: https://go-review.googlesource.com/c/go/+/336629
Trust: Cherry Mui <cherryyz@google.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Than McIntosh <thanm@google.com>

1 files changed, 3 insertions, 18 deletions

diff --git a/src/cmd/compile/internal/abi/abiutils.go b/src/cmd/compile/internal/abi/abiutils.go
index b8ea1955d1..d657ddc867 100644
--- a/src/cmd/compile/internal/abi/abiutils.go
+++ b/src/cmd/compile/internal/abi/abiutils.go
@@ -446,35 +446,20 @@ func (config *ABIConfig) ABIAnalyze(t *types.Type, setNname bool) *ABIParamResul
 	return result
 }
 
-// parameterUpdateMu protects the Offset field of function/method parameters (a subset of structure Fields)
-var parameterUpdateMu sync.Mutex
-
-// FieldOffsetOf returns a concurrency-safe version of f.Offset
-func FieldOffsetOf(f *types.Field) int64 {
-	parameterUpdateMu.Lock()
-	defer parameterUpdateMu.Unlock()
-	return f.Offset
-}
-
 func (config *ABIConfig) updateOffset(result *ABIParamResultInfo, f *types.Field, a ABIParamAssignment, isReturn, setNname bool) {
 	// Everything except return values in registers has either a frame home (if not in a register) or a frame spill location.
 	if !isReturn || len(a.Registers) == 0 {
 		// The type frame offset DOES NOT show effects of minimum frame size.
 		// Getting this wrong breaks stackmaps, see liveness/plive.go:WriteFuncMap and typebits/typebits.go:Set
-		parameterUpdateMu.Lock()
-		defer parameterUpdateMu.Unlock()
 		off := a.FrameOffset(result)
 		fOffset := f.Offset
 		if fOffset == types.BOGUS_FUNARG_OFFSET {
-			// Set the Offset the first time. After that, we may recompute it, but it should never change.
-			f.Offset = off
-			if f.Nname != nil {
-				// always set it in this case.
+			if setNname && f.Nname != nil {
 				f.Nname.(*ir.Name).SetFrameOffset(off)
 				f.Nname.(*ir.Name).SetIsOutputParamInRegisters(false)
 			}
-		} else if fOffset != off {
-			base.Fatalf("offset for %s at %s changed from %d to %d", f.Sym.Name, base.FmtPos(f.Pos), fOffset, off)
+		} else {
+			base.Fatalf("field offset for %s at %s has been set to %d", f.Sym.Name, base.FmtPos(f.Pos), fOffset)
 		}
 	} else {
 		if setNname && f.Nname != nil {