Repository: google/go-flow-levee
Pull request #201 (2020-11-23 22:23:17)

Unify traversal through Field and FieldAddr instructions
This PR is part of #188.

This PR uniformizes the way propagation works for `Field` and `FieldAddr` instructions. While the dynamic semantics of these two instructions are a little different, for the purposes of our traversal they should be the handled the same way.

I know we've recently been discussing (#195, #228) how we might need to rethink the way we handle fields, and I think unifying the way these instructions are handled is a necessary step towards that. 

- [x] Running against a large codebase such as [Kubernetes](https://github.com/kubernetes/kubernetes) does not error out.


Patch:
diff --git a/DEVELOPING.md b/DEVELOPING.md
index 435c442..de51af0 100644
--- a/DEVELOPING.md
+++ b/DEVELOPING.md
@@ -4,25 +4,14 @@
 
 When creating a pull request, you must verify that your changes are safe by running them on a large codebase such as [https://github.com/kubernetes/kubernetes](https://github.com/kubernetes/kubernetes). This is not intended as a replacement for proper automated testing. Running without error on a large codebase provides an additional level of confidence, since a large codebase is likely to contain edge cases that you may not have considered. Indeed, such edge cases have caused failures in the past ([#74](https://github.com/google/go-flow-levee/pull/74), [#143](https://github.com/google/go-flow-levee/pull/143)).
 
-Here's an example script showing how you can test your changes against [Kubernetes](https://github.com/kubernetes/kubernetes):
+In addition we recommend running master and feature branch versions of levee against k8s and diffing the results to see if your changes have led to some new findings. To make this easier we have created a script that will run both master and your feature branch version of levee against k8s and print the diff.
 
-```bash
-#!/bin/bash
-
-set -eux
-
-# set these to appropriate values for your setup
-# GO_FLOW_LEVEE_PATH=path to the root of the go-flow-levee repository on your machine
-# K8S_PATH="$GOPATH"/src/k8s.io/kubernetes
+Note: The script makes the following assumptions
+1. The kubernetes repository on your machine is at `$(GOPATH)/src/k8s.io/kuberenetes`.
+2. You are running the script from within your go-flow-levee directory and have your feature branch checked out.
 
-cd "$GO_FLOW_LEVEE_PATH"/cmd/levee
-go install
-
-cd "$K8S_PATH"
-git checkout master
-git pull
-CFG_PATH="$(realpath ./hack/testdata/levee/levee-config.yaml)"
-make vet WHAT="-vettool=$(which levee) -config=$CFG_PATH"
+```bash
+./hack/verify-kubernetes.sh
 ```
 
 ## Recommended Reading
diff --git a/hack/verify-kubernetes.sh b/hack/verify-kubernetes.sh
new file mode 100755
index 0000000..97c3f27
--- /dev/null
+++ b/hack/verify-kubernetes.sh
@@ -0,0 +1,35 @@
+#!/bin/bash
+
+set -ex
+
+install_and_run_levee_against_k8s_from_branch() {
+    local branch_name=$1
+    git checkout "$branch_name"
+    local levee_output
+    levee_output="$(mktemp -d)/levee"
+    go build -o "${levee_output}" "${LEVEE_DIR}/cmd/levee"
+    local cfg_path
+    cfg_path="$(realpath "${K8S_SRC_PATH}"/hack/testdata/levee/levee-config.yaml)"
+    make -C "${K8S_SRC_PATH}" clean
+    FINDINGS=$(mktemp)
+    set +e
+    make -C "${K8S_SRC_PATH}" vet WHAT="-vettool=$levee_output -config=$cfg_path" 2>"${FINDINGS}"
+    set -e
+}
+
+LEVEE_DIR="$(dirname "$(dirname "$(realpath "$0")")")"
+K8S_SRC_PATH="$(go env GOPATH)/src/k8s.io/kubernetes"
+FEATURE_BRANCH=$(git branch --show-current)
+
+if [[ "${FEATURE_BRANCH}" == "master" ]]; then
+    echo  "Please run from feature branch"
+    exit 1
+fi
+
+install_and_run_levee_against_k8s_from_branch "master"
+MASTER_FINDINGS=$FINDINGS
+
+install_and_run_levee_against_k8s_from_branch "$FEATURE_BRANCH" 
+FEATURE_BRANCH_FINDINGS=$FINDINGS
+
+diff <(sort "${FEATURE_BRANCH_FINDINGS}") <(sort "${MASTER_FINDINGS}")
diff --git a/internal/pkg/config/config.go b/internal/pkg/config/config.go
index e564ba5..f203f5e 100644
--- a/internal/pkg/config/config.go
+++ b/internal/pkg/config/config.go
@@ -44,11 +44,12 @@ func init() {
 
 // Config contains matchers and analysis scope information.
 type Config struct {
-	Sources    []sourceMatcher
-	Sinks      []funcMatcher
-	Sanitizers []funcMatcher
-	FieldTags  []fieldTagMatcher
-	Exclude    []funcMatcher
+	Sources                   []sourceMatcher
+	Sinks                     []funcMatcher
+	Sanitizers                []funcMatcher
+	FieldTags                 []fieldTagMatcher
+	Exclude                   []funcMatcher
+	AllowPanicOnTaintedValues bool
 }
 
 // IsSourceFieldTag determines whether a field tag made up of a key and value
diff --git a/internal/pkg/fieldpropagator/analyzer.go b/internal/pkg/fieldpropagator/analyzer.go
index 1dcf5a5..e93d75a 100644
--- a/internal/pkg/fieldpropagator/analyzer.go
+++ b/internal/pkg/fieldpropagator/analyzer.go
@@ -112,10 +112,8 @@ func analyzeResults(pass *analysis.Pass, conf *config.Config, tf fieldtags.Resul
 			continue
 		}
 
-		deref := utils.Dereference(fa.X.Type())
-		path, typeName := utils.DecomposeType(deref)
-		fieldName := utils.FieldName(fa)
-		if conf.IsSourceField(path, typeName, fieldName) || tf.IsSourceFieldAddr(fa) {
+		xt, field := fa.X.Type(), fa.Field
+		if conf.IsSourceField(utils.DecomposeField(xt, field)) || tf.IsSourceField(xt, field) {
 			pass.ExportObjectFact(meth.Object(), &isFieldPropagator{})
 		}
 	}
diff --git a/internal/pkg/fieldtags/analyzer.go b/internal/pkg/fieldtags/analyzer.go
index 1282b9c..5036e5b 100644
--- a/internal/pkg/fieldtags/analyzer.go
+++ b/internal/pkg/fieldtags/analyzer.go
@@ -22,10 +22,10 @@ import (
 	"reflect"
 
 	"github.com/google/go-flow-levee/internal/pkg/config"
+	"github.com/google/go-flow-levee/internal/pkg/utils"
 	"golang.org/x/tools/go/analysis"
 	"golang.org/x/tools/go/analysis/passes/inspect"
 	"golang.org/x/tools/go/ast/inspector"
-	"golang.org/x/tools/go/ssa"
 )
 
 // ResultType is a map from types.Object to bool.
@@ -89,11 +89,13 @@ func run(pass *analysis.Pass) (interface{}, error) {
 	return ResultType(result), nil
 }
 
-// IsSourceFieldAddr determines whether a ssa.FieldAddr is a source, that is whether it refers to a field previously identified as a source.
-func (r ResultType) IsSourceFieldAddr(fa *ssa.FieldAddr) bool {
+// IsSourceField determines whether a field on a type is a source field,
+// using the type of the struct holding the field as well as the index
+// of the field.
+func (r ResultType) IsSourceField(t types.Type, field int) bool {
 	// incantation plundered from the docstring for ssa.FieldAddr.Field
-	field := fa.X.Type().Underlying().(*types.Pointer).Elem().Underlying().(*types.Struct).Field(fa.Field)
-	return r.IsSource(field)
+	fieldVar := utils.Dereference(t).Underlying().(*types.Struct).Field(field)
+	return r.IsSource(fieldVar)
 }
 
 // IsSource determines whether a types.Var is a source, that is whether it refers to a field previously identified as a source.
diff --git a/internal/pkg/levee/levee.go b/internal/pkg/levee/levee.go
index 08ce537..66d85f3 100644
--- a/internal/pkg/levee/levee.go
+++ b/internal/pkg/levee/levee.go
@@ -19,7 +19,6 @@ import (
 	"strings"
 
 	"github.com/google/go-flow-levee/internal/pkg/config"
-	"github.com/google/go-flow-levee/internal/pkg/fieldpropagator"
 	"github.com/google/go-flow-levee/internal/pkg/fieldtags"
 	"github.com/google/go-flow-levee/internal/pkg/levee/propagation"
 	"github.com/google/go-flow-levee/internal/pkg/utils"
@@ -34,7 +33,7 @@ var Analyzer = &analysis.Analyzer{
 	Run:      run,
 	Flags:    config.FlagSet,
 	Doc:      "reports attempts to source data to sinks",
-	Requires: []*analysis.Analyzer{source.Analyzer, fieldpropagator.Analyzer, fieldtags.Analyzer},
+	Requires: []*analysis.Analyzer{source.Analyzer, fieldtags.Analyzer},
 }
 
 func run(pass *analysis.Pass) (interface{}, error) {
@@ -43,7 +42,6 @@ func run(pass *analysis.Pass) (interface{}, error) {
 		return nil, err
 	}
 	sourcesMap := pass.ResultOf[source.Analyzer].(source.ResultType)
-	fieldPropagators := pass.ResultOf[fieldpropagator.Analyzer].(fieldpropagator.ResultType)
 	taggedFields := pass.ResultOf[fieldtags.Analyzer].(fieldtags.ResultType)
 
 	propagations := map[ssa.Node]propagation.Propagation{}
@@ -57,24 +55,18 @@ func run(pass *analysis.Pass) (interface{}, error) {
 	for fn, sources := range sourcesMap {
 		for _, b := range fn.Blocks {
 			for _, instr := range b.Instrs {
-				v, ok := instr.(*ssa.Call)
-				if !ok {
-					continue
-				}
+				switch v := instr.(type) {
+
+				case *ssa.Call:
+					if callee := v.Call.StaticCallee(); callee != nil && conf.IsSink(utils.DecomposeFunction(callee)) {
+						reportSourcesReachingSink(pass, sources, instr, propagations)
+					}
 
-				callee := v.Call.StaticCallee()
-				switch {
-				case fieldPropagators.IsFieldPropagator(v):
-					propagations[v] = propagation.Dfs(v, conf, taggedFields)
-					sources = append(sources, source.New(v))
-				case callee != nil && conf.IsSink(utils.DecomposeFunction(callee)):
-					for _, s := range sources {
-						prop := propagations[s.Node]
-						if prop.HasPathTo(instr.(ssa.Node)) && !prop.IsSanitizedAt(v) {
-							report(pass, s, v)
-							break
-						}
+				case *ssa.Panic:
+					if conf.AllowPanicOnTaintedValues {
+						continue
 					}
+					reportSourcesReachingSink(pass, sources, instr, propagations)
 				}
 			}
 		}
@@ -83,6 +75,16 @@ func run(pass *analysis.Pass) (interface{}, error) {
 	return nil, nil
 }
 
+func reportSourcesReachingSink(pass *analysis.Pass, sources []*source.Source, instr ssa.Instruction, propagations map[ssa.Node]propagation.Propagation) {
+	for _, s := range sources {
+		prop := propagations[s.Node]
+		if prop.HasPathTo(instr.(ssa.Node)) && !prop.IsSanitizedAt(instr) {
+			report(pass, s, instr.(ssa.Node))
+			break
+		}
+	}
+}
+
 func report(pass *analysis.Pass, source *source.Source, sink ssa.Node) {
 	var b strings.Builder
 	b.WriteString("a source has reached a sink")
diff --git a/internal/pkg/levee/propagation/propagation.go b/internal/pkg/levee/propagation/propagation.go
index 1671bfe..656c461 100644
--- a/internal/pkg/levee/propagation/propagation.go
+++ b/internal/pkg/levee/propagation/propagation.go
@@ -145,6 +145,11 @@ func (prop *Propagation) visit(n ssa.Node, maxInstrReached map[*ssa.BasicBlock]i
 		}
 
 	case *ssa.Call:
+		// The builtin delete(m map[Type]Type1, key Type) func does not propagate a taint.
+		if builtin, ok := t.Call.Value.(*ssa.Builtin); ok && builtin.Name() == "delete" {
+			return
+		}
+
 		if callee := t.Call.StaticCallee(); callee != nil && prop.config.IsSanitizer(utils.DecomposeFunction(callee)) {
 			prop.sanitizers = append(prop.sanitizers, &sanitizer.Sanitizer{Call: t})
 		}
@@ -162,15 +167,11 @@ func (prop *Propagation) visit(n ssa.Node, maxInstrReached map[*ssa.BasicBlock]i
 			}
 		}
 
+	case *ssa.Field:
+		prop.visitField(n, maxInstrReached, lastBlockVisited, t.X.Type(), t.Field)
+
 	case *ssa.FieldAddr:
-		deref := utils.Dereference(t.X.Type())
-		typPath, typName := utils.DecomposeType(deref)
-		fieldName := utils.FieldName(t)
-		if !prop.config.IsSourceField(typPath, typName, fieldName) && !prop.taggedFields.IsSourceFieldAddr(t) {
-			return
-		}
-		prop.visitReferrers(n, maxInstrReached, lastBlockVisited)
-		prop.visitOperands(n, maxInstrReached, lastBlockVisited)
+		prop.visitField(n, maxInstrReached, lastBlockVisited, t.X.Type(), t.Field)
 
 	// Everything but the actual integer Index should be visited.
 	case *ssa.Index:
@@ -213,7 +214,7 @@ func (prop *Propagation) visit(n ssa.Node, maxInstrReached map[*ssa.BasicBlock]i
 		prop.visitOperands(n, maxInstrReached, lastBlockVisited)
 
 	// These nodes are both Instructions and Values, and currently have no special restrictions.
-	case *ssa.Field, *ssa.MakeInterface, *ssa.Select, *ssa.Slice, *ssa.TypeAssert, *ssa.UnOp:
+	case *ssa.MakeInterface, *ssa.Select, *ssa.Slice, *ssa.TypeAssert, *ssa.UnOp:
 		prop.visitReferrers(n, maxInstrReached, lastBlockVisited)
 		prop.visitOperands(n, maxInstrReached, lastBlockVisited)
 
@@ -225,6 +226,14 @@ func (prop *Propagation) visit(n ssa.Node, maxInstrReached map[*ssa.BasicBlock]i
 	}
 }
 
+func (prop *Propagation) visitField(n ssa.Node, maxInstrReached map[*ssa.BasicBlock]int, lastBlockVisited *ssa.BasicBlock, t types.Type, field int) {
+	if !prop.config.IsSourceField(utils.DecomposeField(t, field)) && !prop.taggedFields.IsSourceField(t, field) {
+		return
+	}
+	prop.visitReferrers(n, maxInstrReached, lastBlockVisited)
+	prop.visitOperands(n, maxInstrReached, lastBlockVisited)
+}
+
 func (prop *Propagation) visitReferrers(n ssa.Node, maxInstrReached map[*ssa.BasicBlock]int, lastBlockVisited *ssa.BasicBlock) {
 	if n.Referrers() == nil {
 		return
diff --git a/internal/pkg/source/analyzer.go b/internal/pkg/source/analyzer.go
index 465c7b1..b0d0362 100644
--- a/internal/pkg/source/analyzer.go
+++ b/internal/pkg/source/analyzer.go
@@ -19,6 +19,7 @@ import (
 	"reflect"
 
 	"github.com/google/go-flow-levee/internal/pkg/config"
+	"github.com/google/go-flow-levee/internal/pkg/fieldpropagator"
 	"github.com/google/go-flow-levee/internal/pkg/fieldtags"
 	"golang.org/x/tools/go/analysis"
 	"golang.org/x/tools/go/analysis/passes/buildssa"
@@ -32,20 +33,21 @@ var Analyzer = &analysis.Analyzer{
 	Doc:        "This analyzer identifies ssa.Values that are sources.",
 	Flags:      config.FlagSet,
 	Run:        run,
-	Requires:   []*analysis.Analyzer{buildssa.Analyzer, fieldtags.Analyzer},
+	Requires:   []*analysis.Analyzer{buildssa.Analyzer, fieldtags.Analyzer, fieldpropagator.Analyzer},
 	ResultType: reflect.TypeOf(new(ResultType)).Elem(),
 }
 
 func run(pass *analysis.Pass) (interface{}, error) {
 	ssaInput := pass.ResultOf[buildssa.Analyzer].(*buildssa.SSA)
 	taggedFields := pass.ResultOf[fieldtags.Analyzer].(fieldtags.ResultType)
+	fieldPropagators := pass.ResultOf[fieldpropagator.Analyzer].(fieldpropagator.ResultType)
 
 	conf, err := config.ReadConfig()
 	if err != nil {
 		return nil, err
 	}
 
-	sourceMap := identify(conf, ssaInput, taggedFields)
+	sourceMap := identify(conf, ssaInput, taggedFields, fieldPropagators)
 
 	for _, srcs := range sourceMap {
 		for _, s := range srcs {
diff --git a/internal/pkg/source/source.go b/internal/pkg/source/source.go
index 4a52b85..c8595f0 100644
--- a/internal/pkg/source/source.go
+++ b/internal/pkg/source/source.go
@@ -21,6 +21,7 @@ import (
 	"go/types"
 
 	"github.com/google/go-flow-levee/internal/pkg/config"
+	"github.com/google/go-flow-levee/internal/pkg/fieldpropagator"
 	"github.com/google/go-flow-levee/internal/pkg/fieldtags"
 	"github.com/google/go-flow-levee/internal/pkg/utils"
 	"golang.org/x/tools/go/analysis/passes/buildssa"
@@ -63,7 +64,7 @@ func New(in ssa.Node) *Source {
 // identify individually examines each Function in the SSA code looking for Sources.
 // It produces a map relating a Function to the Sources it contains.
 // If a Function contains no Sources, it does not appear in the map.
-func identify(conf *config.Config, ssaInput *buildssa.SSA, taggedFields fieldtags.ResultType) map[*ssa.Function][]*Source {
+func identify(conf *config.Config, ssaInput *buildssa.SSA, taggedFields fieldtags.ResultType, propagators fieldpropagator.ResultType) map[*ssa.Function][]*Source {
 	sourceMap := make(map[*ssa.Function][]*Source)
 
 	for _, fn := range ssaInput.SrcFuncs {
@@ -76,7 +77,7 @@ func identify(conf *config.Config, ssaInput *buildssa.SSA, taggedFields fieldtag
 		var sources []*Source
 		sources = append(sources, sourcesFromParams(fn, conf, taggedFields)...)
 		sources = append(sources, sourcesFromClosures(fn, conf, taggedFields)...)
-		sources = append(sources, sourcesFromBlocks(fn, conf, taggedFields)...)
+		sources = append(sources, sourcesFromBlocks(fn, conf, taggedFields, propagators)...)
 
 		if len(sources) > 0 {
 			sourceMap[fn] = sources
@@ -114,7 +115,7 @@ func sourcesFromClosures(fn *ssa.Function, conf *config.Config, taggedFields fie
 }
 
 // sourcesFromBlocks finds Source values created by instructions within a function's body.
-func sourcesFromBlocks(fn *ssa.Function, conf *config.Config, taggedFields fieldtags.ResultType) []*Source {
+func sourcesFromBlocks(fn *ssa.Function, conf *config.Config, taggedFields fieldtags.ResultType, propagators fieldpropagator.ResultType) []*Source {
 	var sources []*Source
 	for _, b := range fn.Blocks {
 		for _, instr := range b.Instrs {
@@ -126,9 +127,21 @@ func sourcesFromBlocks(fn *ssa.Function, conf *config.Config, taggedFields field
 			default:
 				continue
 
-			// local variable or value returned from a call
-			case *ssa.Alloc, *ssa.Call:
-				if isProducedBySanitizer(v.(ssa.Value), conf) {
+			// Values produced by sanitizers are not sources.
+			case *ssa.Alloc:
+				if isProducedBySanitizer(v, conf) {
+					continue
+				}
+
+			// Values produced by sanitizers are not sources.
+			// Values produced by field propagators are.
+			case *ssa.Call:
+				if isProducedBySanitizer(v, conf) {
+					continue
+				}
+
+				if propagators.IsFieldPropagator(v) {
+					sources = append(sources, New(v))
 					continue
 				}
 
diff --git a/internal/pkg/utils/testdata/src/fieldname/embedded/test.go b/internal/pkg/utils/testdata/src/fields/embedded/test.go
similarity index 100%
rename from internal/pkg/utils/testdata/src/fieldname/embedded/test.go
rename to internal/pkg/utils/testdata/src/fields/embedded/test.go
diff --git a/internal/pkg/utils/testdata/src/fieldname/regular/test.go b/internal/pkg/utils/testdata/src/fields/regular/test.go
similarity index 100%
rename from internal/pkg/utils/testdata/src/fieldname/regular/test.go
rename to internal/pkg/utils/testdata/src/fields/regular/test.go
diff --git a/internal/pkg/utils/utils.go b/internal/pkg/utils/utils.go
index 955b29a..21cc3f8 100644
--- a/internal/pkg/utils/utils.go
+++ b/internal/pkg/utils/utils.go
@@ -34,19 +34,6 @@ func Dereference(t types.Type) types.Type {
 	}
 }
 
-// FieldName returns the name of the field identified by the FieldAddr.
-// It is the responsibility of the caller to ensure that the returned value is a non-empty string.
-func FieldName(fa *ssa.FieldAddr) string {
-	// fa.Type() refers to the accessed field's type.
-	// fa.X.Type() refers to the surrounding struct's type.
-	d := Dereference(fa.X.Type())
-	st, ok := d.Underlying().(*types.Struct)
-	if !ok {
-		return ""
-	}
-	return st.Field(fa.Field).Name()
-}
-
 // DecomposeType returns the path and name of a Named type
 // Returns empty strings if the type is not *types.Named
 func DecomposeType(t types.Type) (path, name string) {
@@ -62,6 +49,17 @@ func DecomposeType(t types.Type) (path, name string) {
 	return path, n.Obj().Name()
 }
 
+// DecomposeField returns the decomposed type of the
+// struct containing the field, as well as the field's name.
+// If the referenced struct's type is not a named type,
+// the type path and name will both be empty strings.
+func DecomposeField(t types.Type, field int) (typePath, typeName, fieldName string) {
+	deref := Dereference(t)
+	typePath, typeName = DecomposeType(deref)
+	fieldName = deref.Underlying().(*types.Struct).Field(field).Name()
+	return
+}
+
 func unqualifiedName(v *types.Var) string {
 	packageQualifiedName := v.Type().String()
 	dotPos := strings.LastIndexByte(packageQualifiedName, '.')


Test patch:
diff --git a/internal/pkg/levee/levee_test.go b/internal/pkg/levee/levee_test.go
index 1c53106..7828ae5 100644
--- a/internal/pkg/levee/levee_test.go
+++ b/internal/pkg/levee/levee_test.go
@@ -34,3 +34,14 @@ func TestLevee(t *testing.T) {
 	}
 	analysistest.Run(t, dataDir, Analyzer, "./...")
 }
+
+func TestLeveeDoesNotCreateReportsForPanicIfPanicingOnTaintedValuesIsAllowed(t *testing.T) {
+	dataDir := analysistest.TestData()
+	if err := Analyzer.Flags.Set("config", dataDir+"/allowpanicontaintedvalues-config.yaml"); err != nil {
+		t.Error(err)
+	}
+	if *debugging {
+		Analyzer.Requires = append(Analyzer.Requires, debug.Analyzer)
+	}
+	analysistest.Run(t, dataDir, Analyzer, "./src/nopanic.com/...")
+}
diff --git a/internal/pkg/levee/testdata/allowpanicontaintedvalues-config.yaml b/internal/pkg/levee/testdata/allowpanicontaintedvalues-config.yaml
new file mode 100644
index 0000000..7d16e60
--- /dev/null
+++ b/internal/pkg/levee/testdata/allowpanicontaintedvalues-config.yaml
@@ -0,0 +1,19 @@
+# Copyright 2020 Google LLC
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# https://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+---
+AllowPanicOnTaintedValues: true
+Sources:
+- Package: "nopanic.com/core"
+  Type: "Source"
+  Field: "Data"
diff --git a/internal/pkg/levee/testdata/src/example.com/tests/collections/maps.go b/internal/pkg/levee/testdata/src/example.com/tests/collections/maps.go
index a417556..b99d612 100644
--- a/internal/pkg/levee/testdata/src/example.com/tests/collections/maps.go
+++ b/internal/pkg/levee/testdata/src/example.com/tests/collections/maps.go
@@ -57,8 +57,7 @@ func TestDeletingFromTaintedMapDoesNotTaintKey(key *string, sources map[*string]
 	// The key needs to be a pointer parameter, because we don't traverse to non-pointer
 	// arguments of a call, and we don't traverse to Allocs.
 	delete(sources, key)
-	// TODO(#186): no report should be produced here
-	core.Sink(key) // want "a source has reached a sink"
+	core.Sink(key)
 }
 
 func TestMapUpdateWithTaintedValueDoesNotTaintTheKey(key string, value core.Source, sources map[string]core.Source) {
diff --git a/internal/pkg/levee/testdata/src/example.com/tests/fields/tests.go b/internal/pkg/levee/testdata/src/example.com/tests/fields/tests.go
index bce4007..5ac8545 100644
--- a/internal/pkg/levee/testdata/src/example.com/tests/fields/tests.go
+++ b/internal/pkg/levee/testdata/src/example.com/tests/fields/tests.go
@@ -34,6 +34,14 @@ func TestDirectFieldAccess(c *core.Source) {
 	core.Sinkf("ID: %v", c.ID)
 }
 
+func TestInlinedDirectFieldAccess() {
+	// This pattern is unlikely to occur in real code.
+	// The intent is to get Field instructions in the SSA
+	// so that we can validate that those are handled correctly.
+	core.Sinkf("Data: %v", core.Source{Data: "password1234", ID: 1234}.Data) // want "a source has reached a sink"
+	core.Sinkf("ID: %v", core.Source{Data: "password1234", ID: 1234}.ID)
+}
+
 func TestProtoStyleFieldAccessorSanitizedPII(c *core.Source) {
 	core.Sinkf("Source data: %v", core.Sanitize(c.GetData()))
 }
diff --git a/internal/pkg/levee/testdata/src/example.com/tests/panic/tests.go b/internal/pkg/levee/testdata/src/example.com/tests/panic/tests.go
new file mode 100644
index 0000000..8d4ad22
--- /dev/null
+++ b/internal/pkg/levee/testdata/src/example.com/tests/panic/tests.go
@@ -0,0 +1,41 @@
+// Copyright 2020 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// https://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package panic
+
+import (
+	"example.com/core"
+)
+
+func TestPanicIsASink(source core.Source) {
+	panic(source) // want "a source has reached a sink"
+}
+
+func TestPanicIsASinkSanitized(source core.Source) {
+	sanitized := core.Sanitize(source)[0]
+	panic(sanitized)
+}
+
+func TestGoPanicIsASink(source core.Source) {
+	go panic(source) // TODO(#231): want "a source has reached a sink"
+}
+
+func TestDeferPanicIsASink(source core.Source) {
+	defer panic(source) // TODO(#231): want "a source has reached a sink"
+}
+
+func TestPanicOnNonSourceDoesNotProduceReport(source core.Source) {
+	y := 42
+	panic(y)
+}
diff --git a/internal/pkg/levee/testdata/src/nopanic.com/core/core.go b/internal/pkg/levee/testdata/src/nopanic.com/core/core.go
new file mode 100644
index 0000000..74e4c23
--- /dev/null
+++ b/internal/pkg/levee/testdata/src/nopanic.com/core/core.go
@@ -0,0 +1,25 @@
+// Copyright 2020 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// https://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package core
+
+type Source struct {
+	Data string
+	ID   int
+}
+
+func TestPanickingOnTaintedValueIsAllowed() {
+	s := Source{Data: "password", ID: 1337}
+	panic(s)
+}
diff --git a/internal/pkg/utils/utils_test.go b/internal/pkg/utils/utils_test.go
index c9ba9fa..510928b 100644
--- a/internal/pkg/utils/utils_test.go
+++ b/internal/pkg/utils/utils_test.go
@@ -99,38 +99,55 @@ func TestDereference(t *testing.T) {
 	}
 }
 
-func TestFieldName(t *testing.T) {
+func TestDecomposeField(t *testing.T) {
 	dir := analysistest.TestData()
 
 	testCases := []struct {
-		pattern string
-		want    string
+		pattern   string
+		typePath  string
+		typeName  string
+		fieldName string
 	}{
 		{
-			pattern: "regular",
-			want:    "name",
+			pattern:   "regular",
+			typePath:  "fields/regular",
+			typeName:  "foo",
+			fieldName: "name",
 		},
 		{
-			pattern: "embedded",
-			want:    "foo",
+			pattern:   "embedded",
+			typePath:  "fields/embedded",
+			typeName:  "bar",
+			fieldName: "foo",
 		},
 	}
 
 	for _, tt := range testCases {
 		t.Run(tt.pattern, func(t *testing.T) {
-			r := analysistest.Run(t, dir, testAnalyzer, fmt.Sprintf("fieldname/%s", tt.pattern))
+			r := analysistest.Run(t, dir, testAnalyzer, fmt.Sprintf("fields/%s", tt.pattern))
+
 			if len(r) != 1 {
 				t.Fatalf("Got len(result) == %d, want 1", len(r))
 			}
 
-			a, ok := r[0].Result.(testAnalyzerResult)
-			if !ok {
-				t.Fatalf("Got result of type %T, wanted testAnalyzerResult", a)
+			if r[0].Err != nil {
+				t.Fatalf("Got unexpected error: %s", r[0].Err)
 			}
 
-			got := FieldName(a.fieldAddr[0])
-			if got != tt.want {
-				t.Fatalf("Got %s, want %s", got, tt.want)
+			res := r[0].Result.(testAnalyzerResult)
+
+			fa := res.fieldAddr[0]
+			fmt.Println(fa.Pos())
+
+			typePath, typeName, fieldName := DecomposeField(fa.X.Type(), fa.Field)
+			if typePath != tt.typePath {
+				t.Fatalf("Got typePath %s, want %s", typePath, tt.typePath)
+			}
+			if typeName != tt.typeName {
+				t.Fatalf("Got typeName %s, want %s", typeName, tt.typeName)
+			}
+			if fieldName != tt.fieldName {
+				t.Fatalf("Got fieldName %s, want %s", fieldName, tt.fieldName)
 			}
 		})
 	}