proc: allow calls to optimized functions (#1684)

Trust argument order to determine argument frame layout when calling
functions, this allows calling optimized functions and removes the
special cases for runtime.mallocgc.

Fixes #1589

_fixtures/fncall.go

@@ -145,5 +145,6 @@ func main() {
 	runtime.Breakpoint()
 	call1(one, two)
 	fn2clos(2)
+	strings.LastIndexByte(stringslice[1], 'w')
 	fmt.Println(one, two, zero, callpanic, callstacktrace, stringsJoin, intslice, stringslice, comma, a.VRcvr, a.PRcvr, pa, vable_a, vable_pa, pable_pa, fn2clos, fn2glob, fn2valmeth, fn2ptrmeth, fn2nil, ga, escapeArg, a2, square, intcallpanic, onetwothree, curriedAdd, getAStruct, getAStructPtr, getVRcvrableFromAStruct, getPRcvrableFromAStructPtr, getVRcvrableFromAStructPtr, pa2, noreturncall, str)
 }

pkg/dwarf/godwarf/type.go

@@ -55,16 +55,24 @@ func (recCheck recCheck) acquire(off dwarf.Offset) (release func()) {
 	}
 }
 
+func sizeAlignToSize(sz, align int64) int64 {
+	return sz
+}
+
+func sizeAlignToAlign(sz, align int64) int64 {
+	return align
+}
+
 // A Type conventionally represents a pointer to any of the
 // specific Type structures (CharType, StructType, etc.).
-//TODO: remove this use dwarf.Type
 type Type interface {
 	Common() *CommonType
 	String() string
 	Size() int64
+	Align() int64
 
 	stringIntl(recCheck) string
-	sizeIntl(recCheck) int64
+	sizeAlignIntl(recCheck) (int64, int64)
 }
 
 // A CommonType holds fields common to multiple types.
@@ -80,8 +88,9 @@ type CommonType struct {
 
 func (c *CommonType) Common() *CommonType { return c }
 
-func (c *CommonType) Size() int64             { return c.ByteSize }
-func (c *CommonType) sizeIntl(recCheck) int64 { return c.ByteSize }
+func (c *CommonType) Size() int64                           { return c.ByteSize }
+func (c *CommonType) Align() int64                          { return c.ByteSize }
+func (c *CommonType) sizeAlignIntl(recCheck) (int64, int64) { return c.ByteSize, c.ByteSize }
 
 // Basic types
 
@@ -103,6 +112,8 @@ func (t *BasicType) stringIntl(recCheck) string {
 	return "?"
 }
 
+func (t *BasicType) Align() int64 { return t.CommonType.ByteSize }
+
 // A CharType represents a signed character type.
 type CharType struct {
 	BasicType
@@ -168,15 +179,15 @@ func (t *QualType) stringIntl(recCheck recCheck) string {
 	return t.Qual + " " + t.Type.stringIntl(recCheck)
 }
 
-func (t *QualType) Size() int64 { return t.sizeIntl(make(recCheck)) }
+func (t *QualType) Size() int64 { return sizeAlignToSize(t.sizeAlignIntl(make(recCheck))) }
 
-func (t *QualType) sizeIntl(recCheck recCheck) int64 {
+func (t *QualType) sizeAlignIntl(recCheck recCheck) (int64, int64) {
 	release := recCheck.acquire(t.CommonType.Offset)
 	if release == nil {
-		return t.CommonType.ByteSize
+		return t.CommonType.ByteSize, t.CommonType.ByteSize
 	}
 	defer release()
-	return t.Type.sizeIntl(recCheck)
+	return t.Type.sizeAlignIntl(recCheck)
 }
 
 // An ArrayType represents a fixed size array type.
@@ -198,15 +209,20 @@ func (t *ArrayType) stringIntl(recCheck recCheck) string {
 	return "[" + strconv.FormatInt(t.Count, 10) + "]" + t.Type.stringIntl(recCheck)
 }
 
-func (t *ArrayType) Size() int64 { return t.sizeIntl(make(recCheck)) }
+func (t *ArrayType) Size() int64  { return sizeAlignToSize(t.sizeAlignIntl(make(recCheck))) }
+func (t *ArrayType) Align() int64 { return sizeAlignToAlign(t.sizeAlignIntl(make(recCheck))) }
 
-func (t *ArrayType) sizeIntl(recCheck recCheck) int64 {
+func (t *ArrayType) sizeAlignIntl(recCheck recCheck) (int64, int64) {
 	release := recCheck.acquire(t.CommonType.Offset)
 	if release == nil {
-		return t.CommonType.ByteSize
+		return t.CommonType.ByteSize, 1
 	}
 	defer release()
-	return t.Count * t.Type.sizeIntl(recCheck)
+	sz, align := t.Type.sizeAlignIntl(recCheck)
+	if t.CommonType.ByteSize != 0 {
+		return t.CommonType.ByteSize, align
+	}
+	return sz * t.Count, align
 }
 
 // A VoidType represents the C void type.
@@ -294,6 +310,21 @@ func (t *StructType) Defn(recCheck recCheck) string {
 	return s
 }
 
+func (t *StructType) Size() int64  { return sizeAlignToSize(t.sizeAlignIntl(make(recCheck))) }
+func (t *StructType) Align() int64 { return sizeAlignToAlign(t.sizeAlignIntl(make(recCheck))) }
+
+func (t *StructType) sizeAlignIntl(recCheck recCheck) (int64, int64) {
+	release := recCheck.acquire(t.CommonType.Offset)
+	if release == nil {
+		return t.CommonType.ByteSize, 1
+	}
+	defer release()
+	if len(t.Field) == 0 {
+		return t.CommonType.ByteSize, 1
+	}
+	return t.CommonType.ByteSize, sizeAlignToAlign(t.Field[0].Type.sizeAlignIntl(recCheck))
+}
+
 // A SliceType represents a Go slice type. It looks like a StructType, describing
 // the runtime-internal structure, with extra fields.
 type SliceType struct {
@@ -425,18 +456,18 @@ func (t *TypedefType) String() string { return t.stringIntl(nil) }
 
 func (t *TypedefType) stringIntl(recCheck recCheck) string { return t.Name }
 
-func (t *TypedefType) Size() int64 { return t.sizeIntl(make(recCheck)) }
+func (t *TypedefType) Size() int64 { sz, _ := t.sizeAlignIntl(make(recCheck)); return sz }
 
-func (t *TypedefType) sizeIntl(recCheck recCheck) int64 {
+func (t *TypedefType) sizeAlignIntl(recCheck recCheck) (int64, int64) {
 	release := recCheck.acquire(t.CommonType.Offset)
 	if release == nil {
-		return t.CommonType.ByteSize
+		return t.CommonType.ByteSize, t.CommonType.ByteSize
 	}
 	defer release()
 	if t.Type == nil {
-		return 0
+		return 0, 1
 	}
-	return t.Type.sizeIntl(recCheck)
+	return t.Type.sizeAlignIntl(recCheck)
 }
 
 // A MapType represents a Go map type. It looks like a TypedefType, describing

pkg/proc/eval.go

@@ -102,6 +102,8 @@ func (scope *EvalScope) Locals() ([]*Variable, error) {
 		return nil, errors.New("unable to find function context")
 	}
 
+	trustArgOrder := scope.BinInfo.Producer() != "" && goversion.ProducerAfterOrEqual(scope.BinInfo.Producer(), 1, 12)
+
 	var vars []*Variable
 	var depths []int
 	varReader := reader.Variables(scope.image().dwarf, scope.Fn.offset, reader.ToRelAddr(scope.PC, scope.image().StaticBase), scope.Line, true, false)
@@ -112,6 +114,14 @@ func (scope *EvalScope) Locals() ([]*Variable, error) {
 			// skip variables that we can't parse yet
 			continue
 		}
+		if trustArgOrder && val.Unreadable != nil && val.Addr == 0 && entry.Tag == dwarf.TagFormalParameter {
+			addr := afterLastArgAddr(vars)
+			if addr == 0 {
+				addr = uintptr(scope.Regs.CFA)
+			}
+			addr = uintptr(alignAddr(int64(addr), val.DwarfType.Align()))
+			val = newVariable(val.Name, addr, val.DwarfType, scope.BinInfo, scope.Mem)
+		}
 		vars = append(vars, val)
 		depth := varReader.Depth()
 		if entry.Tag == dwarf.TagFormalParameter {
@@ -163,6 +173,16 @@ func (scope *EvalScope) Locals() ([]*Variable, error) {
 	return vars, nil
 }
 
+func afterLastArgAddr(vars []*Variable) uintptr {
+	for i := len(vars) - 1; i >= 0; i-- {
+		v := vars[i]
+		if (v.Flags&VariableArgument != 0) || (v.Flags&VariableReturnArgument != 0) {
+			return v.Addr + uintptr(v.DwarfType.Size())
+		}
+	}
+	return 0
+}
+
 // setValue writes the value of srcv to dstv.
 // * If srcv is a numerical literal constant and srcv is of a compatible type
 //   the necessary type conversion is performed.
@@ -378,6 +398,9 @@ func (scope *EvalScope) findGlobal(name string) (*Variable, error) {
 			r.Value = constant.MakeString(fn.Name)
 			r.Base = uintptr(fn.Entry)
 			r.loaded = true
+			if fn.Entry == 0 {
+				r.Unreadable = fmt.Errorf("function %s is inlined", fn.Name)
+			}
 			return r, nil
 		}
 	}
@@ -777,27 +800,31 @@ func (scope *EvalScope) evalBuiltinCall(node *ast.CallExpr) (*Variable, error) {
 		return nil, nil
 	}
 
-	args := make([]*Variable, len(node.Args))
+	callBuiltinWithArgs := func(builtin func([]*Variable, []ast.Expr) (*Variable, error)) (*Variable, error) {
+		args := make([]*Variable, len(node.Args))
 
-	for i := range node.Args {
-		v, err := scope.evalAST(node.Args[i])
-		if err != nil {
-			return nil, err
+		for i := range node.Args {
+			v, err := scope.evalAST(node.Args[i])
+			if err != nil {
+				return nil, err
+			}
+			args[i] = v
 		}
-		args[i] = v
+
+		return builtin(args, node.Args)
 	}
 
 	switch fnnode.Name {
 	case "cap":
-		return capBuiltin(args, node.Args)
+		return callBuiltinWithArgs(capBuiltin)
 	case "len":
-		return lenBuiltin(args, node.Args)
+		return callBuiltinWithArgs(lenBuiltin)
 	case "complex":
-		return complexBuiltin(args, node.Args)
+		return callBuiltinWithArgs(complexBuiltin)
 	case "imag":
-		return imagBuiltin(args, node.Args)
+		return callBuiltinWithArgs(imagBuiltin)
 	case "real":
-		return realBuiltin(args, node.Args)
+		return callBuiltinWithArgs(realBuiltin)
 	}
 
 	return nil, nil

pkg/proc/fncall.go

@@ -523,21 +523,34 @@ func funcCallArgs(fn *Function, bi *BinaryInfo, includeRet bool) (argFrameSize i
 		}
 		typ = resolveTypedef(typ)
 		var off int64
-		if trustArgOrder && fn.Name == "runtime.mallocgc" {
-			// runtime is always optimized and optimized code sometimes doesn't have
-			// location info for arguments, but we still want to call runtime.mallocgc.
-			off = argFrameSize
+
+		locprog, _, err := bi.locationExpr(entry, dwarf.AttrLocation, fn.Entry)
+		if err != nil {
+			err = fmt.Errorf("could not get argument location of %s: %v", argname, err)
 		} else {
-			locprog, _, err := bi.locationExpr(entry, dwarf.AttrLocation, fn.Entry)
+			var pieces []op.Piece
+			off, pieces, err = op.ExecuteStackProgram(op.DwarfRegisters{CFA: CFA, FrameBase: CFA}, locprog)
 			if err != nil {
-				return 0, nil, fmt.Errorf("could not get argument location of %s: %v", argname, err)
+				err = fmt.Errorf("unsupported location expression for argument %s: %v", argname, err)
 			}
-			off, _, err = op.ExecuteStackProgram(op.DwarfRegisters{CFA: CFA, FrameBase: CFA}, locprog)
-			if err != nil {
-				return 0, nil, fmt.Errorf("unsupported location expression for argument %s: %v", argname, err)
+			if pieces != nil {
+				err = fmt.Errorf("unsupported location expression for argument %s (uses DW_OP_piece)", argname)
+			}
+			off -= CFA
+		}
+		if err != nil {
+			if !trustArgOrder {
+				return 0, nil, err
 			}
 
-			off -= CFA
+			// With Go version 1.12 or later we can trust that the arguments appear
+			// in the same order as declared, which means we can calculate their
+			// address automatically.
+			// With this we can call optimized functions (which sometimes do not have
+			// an argument address, due to a compiler bug) as well as runtime
+			// functions (which are always optimized).
+			off = argFrameSize
+			off = alignAddr(off, typ.Align())
 		}
 
 		if e := off + typ.Size(); e > argFrameSize {
@@ -559,6 +572,11 @@ func funcCallArgs(fn *Function, bi *BinaryInfo, includeRet bool) (argFrameSize i
 	return argFrameSize, formalArgs, nil
 }
 
+// alignAddr rounds up addr to a multiple of align. Align must be a power of 2.
+func alignAddr(addr, align int64) int64 {
+	return (addr + int64(align-1)) &^ int64(align-1)
+}
+
 func escapeCheck(v *Variable, name string, g *G) error {
 	switch v.Kind {
 	case reflect.Ptr:
@@ -736,14 +754,6 @@ func funcCallStep(callScope *EvalScope, fncall *functionCallState) bool {
 		fncall.retvars = filterVariables(fncall.retvars, func(v *Variable) bool {
 			return (v.Flags & VariableReturnArgument) != 0
 		})
-		if fncall.fn.Name == "runtime.mallocgc" && fncall.retvars[0].Unreadable != nil {
-			// return values never have a location for optimized functions and the
-			// runtime is always optimized. However we want to call runtime.mallocgc,
-			// so we fix the address of the return value manually.
-			fncall.retvars[0].Unreadable = nil
-			lastArg := fncall.formalArgs[len(fncall.formalArgs)-1]
-			fncall.retvars[0].Addr = uintptr(retScope.Regs.CFA + lastArg.off + int64(bi.Arch.PtrSize()))
-		}
 
 		loadValues(fncall.retvars, callScope.callCtx.retLoadCfg)
 		for _, v := range fncall.retvars {

pkg/proc/proc_unexported_test.go

@@ -0,0 +1,75 @@
+package proc
+
+import (
+	"testing"
+)
+
+func TestAlignAddr(t *testing.T) {
+	c := func(align, in, tgt int64) {
+		out := alignAddr(in, align)
+		if out != tgt {
+			t.Errorf("alignAddr(%x, %x) = %x, expected %x", in, align, out, tgt)
+		}
+	}
+
+	for i := int64(0); i <= 0xf; i++ {
+		c(1, i, i)
+		c(1, i+0x10000, i+0x10000)
+	}
+
+	for _, example := range []struct{ align, in, tgt int64 }{
+		{2, 0, 0},
+		{2, 1, 2},
+		{2, 2, 2},
+		{2, 3, 4},
+		{2, 4, 4},
+		{2, 5, 6},
+		{2, 6, 6},
+		{2, 7, 8},
+		{2, 8, 8},
+		{2, 9, 0xa},
+		{2, 0xa, 0xa},
+		{2, 0xb, 0xc},
+		{2, 0xc, 0xc},
+		{2, 0xd, 0xe},
+		{2, 0xe, 0xe},
+		{2, 0xf, 0x10},
+
+		{4, 0, 0},
+		{4, 1, 4},
+		{4, 2, 4},
+		{4, 3, 4},
+		{4, 4, 4},
+		{4, 5, 8},
+		{4, 6, 8},
+		{4, 7, 8},
+		{4, 8, 8},
+		{4, 9, 0xc},
+		{4, 0xa, 0xc},
+		{4, 0xb, 0xc},
+		{4, 0xc, 0xc},
+		{4, 0xd, 0x10},
+		{4, 0xe, 0x10},
+		{4, 0xf, 0x10},
+
+		{8, 0, 0},
+		{8, 1, 8},
+		{8, 2, 8},
+		{8, 3, 8},
+		{8, 4, 8},
+		{8, 5, 8},
+		{8, 6, 8},
+		{8, 7, 8},
+		{8, 8, 8},
+		{8, 9, 0x10},
+		{8, 0xa, 0x10},
+		{8, 0xb, 0x10},
+		{8, 0xc, 0x10},
+		{8, 0xd, 0x10},
+		{8, 0xe, 0x10},
+		{8, 0xf, 0x10},
+	} {
+		c(example.align, example.in, example.tgt)
+		c(example.align, example.in+0x10000, example.tgt+0x10000)
+	}
+}

service/test/variables_test.go

@@ -1110,7 +1110,7 @@ func TestCallFunction(t *testing.T) {
 		{"callpanic()", []string{`~panic:interface {}:interface {}(string) "callpanic panicked"`}, nil},
 		{`stringsJoin(nil, "")`, []string{`:string:""`}, nil},
 		{`stringsJoin(stringslice, comma)`, []string{`:string:"one,two,three"`}, nil},
-		{`stringsJoin(s1, comma)`, nil, errors.New("could not find symbol value for s1")},
+		{`stringsJoin(s1, comma)`, nil, errors.New(`error evaluating "s1" as argument v in function main.stringsJoin: could not find symbol value for s1`)},
 		{`stringsJoin(intslice, comma)`, nil, errors.New("can not convert value of type []int to []string")},
 		{`noreturncall(2)`, nil, nil},
 
@@ -1172,6 +1172,15 @@ func TestCallFunction(t *testing.T) {
 		// string allocation requires trusted argument order, which we don't have in Go 1.11
 		{`stringsJoin(stringslice, ",")`, []string{`:string:"one,two,three"`}, nil},
 		{`str = "a new string"; str`, []string{`str:string:"a new string"`}, nil},
+
+		// support calling optimized functions
+		{`strings.Join(nil, "")`, []string{`:string:""`}, nil},
+		{`strings.Join(stringslice, comma)`, []string{`:string:"one,two,three"`}, nil},
+		{`strings.Join(s1, comma)`, nil, errors.New(`error evaluating "s1" as argument a in function strings.Join: could not find symbol value for s1`)},
+		{`strings.Join(intslice, comma)`, nil, errors.New("can not convert value of type []int to []string")},
+		{`strings.Join(stringslice, ",")`, []string{`:string:"one,two,three"`}, nil},
+		{`strings.LastIndexByte(stringslice[1], 'w')`, []string{":int:1"}, nil},
+		{`strings.LastIndexByte(stringslice[1], 'o')`, []string{":int:2"}, nil},
 	}
 
 	var testcases113 = []testCaseCallFunction{