ogle/program: in stack frames, add the function name, parameters, and local variables.

A program.Var is returned for each parameter and variable, so we can use
Value to get their values.  evalLocation is changed to return an error
when it can't parse a DWARF location description.  Parameters and
variables we can't parse are ignored.

Fixes some error messages.

Change-Id: I0bd4263268db01cf206fdc8e1f1d55b9106035c6
Reviewed-on: https://go-review.googlesource.com/10609
Reviewed-by: Rob Pike <r@golang.org>
diff --git a/ogle/demo/ogler/ogler_test.go b/ogle/demo/ogler/ogler_test.go
index 8bceaff..fc7c511 100644
--- a/ogle/demo/ogler/ogler_test.go
+++ b/ogle/demo/ogler/ogler_test.go
@@ -191,6 +191,37 @@
 		log.Fatalf("prog.Frames error: %v", err)
 	}
 	fmt.Printf("%#v\n", frames)
+	if len(frames) == 0 {
+		t.Fatalf("no stack frames returned")
+	}
+	if frames[0].Function != "main.foo" {
+		t.Errorf("function name: got %s expected main.foo", frames[0].Function)
+	}
+	if len(frames[0].Params) != 2 {
+		t.Errorf("got %d parameters, expected 2", len(frames[0].Params))
+	} else {
+		x := frames[0].Params[0]
+		y := frames[0].Params[1]
+		if x.Name != "x" {
+			x, y = y, x
+		}
+		if x.Name != "x" {
+			t.Errorf("parameter name: got %s expected x", x.Name)
+		}
+		if y.Name != "y" {
+			t.Errorf("parameter name: got %s expected y", y.Name)
+		}
+		if val, err := prog.Value(x.Var); err != nil {
+			t.Errorf("value of x: %s", err)
+		} else if val != int16(42) {
+			t.Errorf("value of x: got %T(%v) expected int16(42)", val, val)
+		}
+		if val, err := prog.Value(y.Var); err != nil {
+			t.Errorf("value of y: %s", err)
+		} else if val != float32(1.5) {
+			t.Errorf("value of y: got %T(%v) expected float32(1.5)", val, val)
+		}
+	}
 
 	varnames, err := prog.Eval(`re:main\.Z_.*`)
 	if err != nil {
@@ -207,25 +238,25 @@
 		} else {
 			fmt.Printf("%s = %v\n", v, val)
 			if seen[v] {
-				log.Fatalf("Repeated variable %s\n", v)
+				log.Fatalf("repeated variable %s\n", v)
 			}
 			seen[v] = true
 			if len(val) != 1 {
-				log.Fatalf("Should be one value for %s\n", v)
+				log.Fatalf("should be one value for %s\n", v)
 			}
 			expected, ok := expectedVars[v]
 			if !ok {
-				log.Fatalf("Unexpected variable %s\n", v)
+				log.Fatalf("unexpected variable %s\n", v)
 			} else {
 				if !matches(expected, val[0]) {
-					log.Fatalf("Expected %s = %s\n", v, expected)
+					log.Fatalf("expected %s = %s\n", v, expected)
 				}
 			}
 		}
 	}
 	for v, e := range expectedVars {
 		if !seen[v] {
-			log.Fatalf("Didn't get %s = %s\n", v, e)
+			log.Fatalf("didn't get %s = %s\n", v, e)
 		}
 	}
 
@@ -260,7 +291,7 @@
 		}
 	}
 	if !ok {
-		t.Errorf("Stopped at %X expected one of %X.", status.PC, pcs2)
+		t.Errorf("stopped at %X expected one of %X.", status.PC, pcs2)
 	}
 
 	// Check we get the expected results calling VarByName then Value
@@ -269,9 +300,9 @@
 		if v, err := prog.VarByName(name); err != nil {
 			t.Errorf("VarByName(%s): %s", name, err)
 		} else if val, err := prog.Value(v); err != nil {
-			t.Errorf("Value for %s: %s", name, err)
+			t.Errorf("value of %s: %s", name, err)
 		} else if val != exp {
-			t.Errorf("Value for %s: got %T(%v) want %T(%v)", name, val, val, exp, exp)
+			t.Errorf("value of %s: got %T(%v) want %T(%v)", name, val, val, exp, exp)
 		}
 	}
 
@@ -280,7 +311,7 @@
 		t.Error("VarByName for invalid name: expected error")
 	}
 	if _, err = prog.Value(program.Var{}); err == nil {
-		t.Error("Value of invalid var: expected error")
+		t.Error("value of invalid var: expected error")
 	}
 	if v, err := prog.VarByName("main.Z_int16"); err != nil {
 		t.Error("VarByName(main.Z_int16) error:", err)
@@ -289,7 +320,7 @@
 		// v now has a valid type but a bad address.
 		_, err = prog.Value(v)
 		if err == nil {
-			t.Error("Value of invalid location: expected error")
+			t.Error("value of invalid location: expected error")
 		}
 	}
 
@@ -300,9 +331,9 @@
 		if v, err := prog.VarByName(name); err != nil {
 			t.Errorf("VarByName(%s): %s", name, err)
 		} else if val, err := prog.Value(v); err != nil {
-			t.Errorf("value for %s: %s", name, err)
+			t.Errorf("value of %s: %s", name, err)
 		} else if err := fn(val); err != nil {
-			t.Errorf("value for %s: %s", name, err)
+			t.Errorf("value of %s: %s", name, err)
 		}
 	}
 
diff --git a/ogle/demo/tracee/main.go b/ogle/demo/tracee/main.go
index 9b44649..4fbf63b 100644
--- a/ogle/demo/tracee/main.go
+++ b/ogle/demo/tracee/main.go
@@ -70,7 +70,7 @@
 	Z_unsafe_pointer_nil  unsafe.Pointer
 )
 
-func foo() {
+func foo(x int16, y float32) {
 	fmt.Println(Z_bool_false, Z_bool_true)
 	fmt.Println(Z_int, Z_int8, Z_int16, Z_int32, Z_int64)
 	fmt.Println(Z_uint, Z_uint8, Z_uint16, Z_uint32, Z_uint64, Z_uintptr)
@@ -97,7 +97,7 @@
 }
 
 func bar() {
-	foo()
+	foo(42, 1.5)
 	fmt.Print()
 }
 
@@ -105,7 +105,7 @@
 	args := os.Args[1:]
 	expected := []string{"some", "arguments"}
 	if len(args) != 2 || args[0] != expected[0] || args[1] != expected[1] {
-		log.Fatalf("Got command-line args %v, expected %v", args, expected)
+		log.Fatalf("got command-line args %v, expected %v", args, expected)
 	}
 	for ; ; time.Sleep(2 * time.Second) {
 		bar()
diff --git a/ogle/program/program.go b/ogle/program/program.go
index 34b3961..77d8b8a 100644
--- a/ogle/program/program.go
+++ b/ogle/program/program.go
@@ -154,14 +154,25 @@
 	PC uint64
 	// SP is the hardware stack pointer.
 	SP uint64
-
 	// File and Line are the source code location of the PC.
 	File string
 	Line int
+	// Function is the name of this frame's function.
+	Function string
+	// Params contains the function's parameters.
+	Params []Param
+	// Vars contains the function's local variables.
+	Vars []LocalVar
+}
 
-	// TODO: add arguments and return values.
+// Param is a parameter of a function.
+type Param struct {
+	Name string
+	Var  Var
+}
 
-	// S is an unstructured string value used for debugging the ogle
-	// library. It is not guaranteed to be in any particular format.
-	S string
+// LocalVar is a local variable of a function.
+type LocalVar struct {
+	Name string
+	Var  Var
 }
diff --git a/ogle/program/server/dwarf.go b/ogle/program/server/dwarf.go
index ca99683..50f2220 100644
--- a/ogle/program/server/dwarf.go
+++ b/ogle/program/server/dwarf.go
@@ -5,6 +5,7 @@
 package server
 
 import (
+	"errors"
 	"regexp"
 
 	"golang.org/x/debug/dwarf"
@@ -51,22 +52,40 @@
 	return s.dwarfData.EntryForPC(pc)
 }
 
-// TODO: signedness? Return (x int64, ok bool)??
-func evalLocation(v []uint8) int64 {
+// evalLocation parses a DWARF location description encoded in v.  It works for
+// cases where the variable is stored at an offset from the Canonical Frame
+// Address.  The return value is this offset.
+// TODO: a more general location-description-parsing function.
+func evalLocation(v []uint8) (int64, error) {
+	// Some DWARF constants.
+	const (
+		opConsts       = 0x11
+		opPlus         = 0x22
+		opCallFrameCFA = 0x9C
+	)
 	if len(v) == 0 {
-		return 0
+		return 0, errors.New("empty location specifier")
 	}
-	if v[0] != 0x9C { // DW_OP_call_frame_cfa
-		return 0
+	if v[0] != opCallFrameCFA {
+		return 0, errors.New("unsupported location specifier")
 	}
 	if len(v) == 1 {
-		return 0
+		// The location description was just DW_OP_call_frame_cfa, so the location is exactly the CFA.
+		return 0, nil
 	}
-	v = v[1:]
-	if v[0] != 0x11 { // DW_OP_consts
-		return 0
+	if v[1] != opConsts {
+		return 0, errors.New("unsupported location specifier")
 	}
-	return sleb128(v[1:])
+	offset, v, err := sleb128(v[2:])
+	if err != nil {
+		return 0, err
+	}
+	if len(v) == 1 && v[0] == opPlus {
+		// The location description was DW_OP_call_frame_cfa, DW_OP_consts <offset>, DW_OP_plus.
+		// So return the offset.
+		return offset, nil
+	}
+	return 0, errors.New("unsupported location specifier")
 }
 
 func uleb128(v []uint8) (u uint64) {
@@ -81,10 +100,14 @@
 	return u
 }
 
-func sleb128(v []uint8) (s int64) {
+// sleb128 parses a signed integer encoded with sleb128 at the start of v, and
+// returns the integer and the remainder of v.
+func sleb128(v []uint8) (s int64, rest []uint8, err error) {
 	var shift uint
 	var sign int64 = -1
-	for _, x := range v {
+	var i int
+	var x uint8
+	for i, x = range v {
 		s |= (int64(x) & 0x7F) << shift
 		shift += 7
 		sign <<= 7
@@ -95,6 +118,8 @@
 			break
 		}
 	}
-	// Sign extend?
-	return s
+	if i == len(v) {
+		return 0, nil, errors.New("truncated sleb128")
+	}
+	return s, v[i+1:], nil
 }
diff --git a/ogle/program/server/server.go b/ogle/program/server/server.go
index 1a3ca3e..4afb911 100644
--- a/ogle/program/server/server.go
+++ b/ogle/program/server/server.go
@@ -587,6 +587,13 @@
 		if err != nil {
 			return err
 		}
+		frame := program.Frame{
+			PC:   pc,
+			SP:   sp,
+			File: file,
+			Line: line,
+		}
+		frame.Function, _ = entry.Val(dwarf.AttrName).(string)
 		r.Seek(entry.Offset)
 		for {
 			entry, err := r.Next()
@@ -596,38 +603,19 @@
 			if entry.Tag == 0 {
 				break
 			}
-			if entry.Tag != dwarf.TagFormalParameter {
-				continue
-			}
-			if entry.Children {
-				// TODO: handle this??
-				return fmt.Errorf("FormalParameter has children, expected none")
-			}
-
-			offset := int64(0)
-			name := "arg"
-			for _, f := range entry.Field {
-				switch f.Attr {
-				case dwarf.AttrLocation:
-					offset = evalLocation(f.Val.([]uint8))
-				case dwarf.AttrName:
-					name = f.Val.(string)
+			// TODO: report variables we couldn't parse?
+			if entry.Tag == dwarf.TagFormalParameter {
+				if v, err := s.parseParameterOrLocal(entry, fp); err == nil {
+					frame.Params = append(frame.Params, program.Param(v))
 				}
 			}
-			str, err := s.printer.SprintEntry(entry, address(fp)+address(offset))
-			fmt.Fprintf(b, "%s (%d(FP)) = %s ", name, offset, str)
-			if err != nil {
-				fmt.Fprintf(b, "(%s) ", err)
+			if entry.Tag == dwarf.TagVariable {
+				if v, err := s.parseParameterOrLocal(entry, fp); err == nil {
+					frame.Vars = append(frame.Vars, v)
+				}
 			}
 		}
-		resp.Frames = append(resp.Frames, program.Frame{
-			PC:   pc,
-			SP:   sp,
-			File: file,
-			Line: line,
-			// TODO: delete the Frame.S field.
-			S: b.String(),
-		})
+		resp.Frames = append(resp.Frames, frame)
 
 		// Walk to the caller's PC and SP.
 		if s.topOfStack(funcEntry) {
@@ -642,6 +630,29 @@
 	return nil
 }
 
+// parseParameterOrLocal parses the entry for a function parameter or local
+// variable, which are both specified the same way. fp contains the frame
+// pointer, which is used to calculate the variable location.
+func (s *Server) parseParameterOrLocal(entry *dwarf.Entry, fp uint64) (program.LocalVar, error) {
+	var v program.LocalVar
+	v.Name, _ = entry.Val(dwarf.AttrName).(string)
+	if off, err := s.dwarfData.EntryTypeOffset(entry); err != nil {
+		return v, err
+	} else {
+		v.Var.TypeID = uint64(off)
+	}
+	if i := entry.Val(dwarf.AttrLocation); i == nil {
+		return v, fmt.Errorf("missing location description")
+	} else if locationDescription, ok := i.([]uint8); !ok {
+		return v, fmt.Errorf("unsupported location description")
+	} else if offset, err := evalLocation(locationDescription); err != nil {
+		return v, err
+	} else {
+		v.Var.Address = fp + uint64(offset)
+	}
+	return v, nil
+}
+
 func (s *Server) evaluateTopOfStackAddrs() error {
 	var (
 		lookup   func(name string) (uint64, error)