Identify sources from panicky ssa.TypeAssert

[vinayakankugoyal]

If a ssa.TypeAssert instruction does not have CommaOk and the AssertedType is a source then, create a Source.
Fixes #190

Why is this change needed?

Today go-flow-levee correctly identifies that a source has reached a sink for the following code:

func TestSourcePointerAssertedFromParameterEfaceCommaOk(e interface{}) {
	s,ok := e.(*core.Source)
        core.Sink(s) 
}

The SSA for the code above is:

func TestSourcePointerAssertedFromParameterEfaceCommaOk(e interface{})
0: entry
	 0(*ssa.TypeAssert     ): t0 = typeassert,ok e.(*example.com/core.Source)
	 1(*ssa.Extract        ): t1 = extract t0 #0
	 2(*ssa.Extract        ): t2 = extract t0 #1
	 3(*ssa.Alloc          ): t3 = new [1]interface{} (varargs)
	 4(*ssa.IndexAddr      ): t4 = &t3[0:int]
	 5(*ssa.MakeInterface  ): t5 = make interface{} <- *example.com/core.Source (t1)
	 6(*ssa.Store          ): *t4 = t5
	 7(*ssa.Slice          ): t6 = slice t3[:]
	 8(*ssa.Call           ): t7 = example.com/core.Sink(t6...)
	 9(*ssa.Alloc          ): t8 = new [1]interface{} (varargs)
	10(*ssa.IndexAddr      ): t9 = &t8[0:int]
	11(*ssa.MakeInterface  ): t10 = make interface{} <- bool (t2)
	12(*ssa.Store          ): *t9 = t10
	13(*ssa.Slice          ): t11 = slice t8[:]
	14(*ssa.Call           ): t12 = example.com/core.Sink(t11...)
	15(*ssa.Return         ): return

Note that the ssa.TypeAssert is followed by the ssa.Extract. This works because we handle ssa.Extract correctly.

However, go-flow-levee does not report that a source has reached a sink for the function below:

func TestSourcePointerAssertedFromParameterEface(e interface{}) {
	s := e.(*core.Source)
        core.Sink(s) 
}

The SSA for this as follow:

func TestSourcePointerAssertedFromParameterEface(e interface{})
0: entry
	0(*ssa.TypeAssert     ): t0 = typeassert e.(*example.com/core.Source)
	1(*ssa.Alloc          ): t1 = new [1]interface{} (varargs)
	2(*ssa.IndexAddr      ): t2 = &t1[0:int]
	3(*ssa.MakeInterface  ): t3 = make interface{} <- *example.com/core.Source (t0)
	4(*ssa.Store          ): *t2 = t3
	5(*ssa.Slice          ): t4 = slice t1[:]
	6(*ssa.Call           ): t5 = example.com/core.Sink(t4...)
	7(*ssa.Return         ): return

Since we were not checking if the type assertion worked as in s, ok := e.(*core.Source) case and just panic there is no ssa.Extract instruction, we need to handle the case ssa.TypeAssert with ssa.TypeAssert.CommaOk being false case explicitly and create a source if ssa.TypeAssert.AssertedType is a source type.

Thanks to @mlevesquedion for helping me understand this problem and its solution.

• Tests pass
• Running against a large codebase such as Kubernetes does not error out. (See DEVELOPING.md for instructions on how to do that.)
• Appropriate changes to README are included in PR

[mlevesquedion]

Good change!

A couple of things to look into before I can approve:

• Could you shorten the title so that it fits entirely in the PR's title field? (e.g. "Identify sources created by TypeAssert instructions") The PR title will eventually become part of the auto-generated commit message. Also, it is often useful to be able to view the entire title in e.g. the pull requests page.
• Could you explain why we need this change? (I understand why, but for documentation purposes it is helpful to have a bit more detail in the description)
• While the passing test for Leverage type information provided by type assertions #210 is a welcome surprise, I don't think this "fixes" Leverage type information provided by type assertions #210. We'll still need to decide what to do when the TypeAssert is CommaOk. Please remove "Fixes Leverage type information provided by type assertions #210" from the description so that the issue does not get closed.

[PurelyApplied]

+1 to all of Michael's comments.

Could you shorten the title so that it fits entirely in the PR's title field? (e.g. "Identify sources created by TypeAssert instructions") The PR title will eventually become part of the auto-generated commit message. Also, it is often useful to be able to view the entire title in e.g. the pull requests page.

Personally, I'd include the panicky adjective in the commit summary, e.g., Identify sources from panicky ssa.TypeAssert. As Michael pointed out, we still have several #210 cases that won't be covered by this PR, so it could be good do distinguish in the summary the CommaOK vs panic.