Skip to content

Commit

Permalink
update
Browse files Browse the repository at this point in the history
  • Loading branch information
yuxincs committed Feb 1, 2024
1 parent a82d5b4 commit d6d74bb
Show file tree
Hide file tree
Showing 7 changed files with 37 additions and 89 deletions.
39 changes: 0 additions & 39 deletions annotation/consume_trigger.go
Original file line number Diff line number Diff line change
Expand Up @@ -1615,45 +1615,6 @@ func (g GlobalVarAssignDeepPrestring) String() string {
return sb.String()
}

// ChanAccess is when a channel is accessed for sending, and thus must be non-nil
type ChanAccess struct {
*ConsumeTriggerTautology
}

// equals returns true if the passed ConsumingAnnotationTrigger is equal to this one
func (c *ChanAccess) equals(other ConsumingAnnotationTrigger) bool {
if other, ok := other.(*ChanAccess); ok {
return c.ConsumeTriggerTautology.equals(other.ConsumeTriggerTautology)
}
return false
}

// Copy returns a deep copy of this ConsumingAnnotationTrigger
func (c *ChanAccess) Copy() ConsumingAnnotationTrigger {
copyConsumer := *c
copyConsumer.ConsumeTriggerTautology = c.ConsumeTriggerTautology.Copy().(*ConsumeTriggerTautology)
return &copyConsumer
}

// Prestring returns this MapWrittenTo as a Prestring
func (c *ChanAccess) Prestring() Prestring {
return ChanAccessPrestring{
AssignmentStr: c.assignmentFlow.String(),
}
}

// ChanAccessPrestring is a Prestring storing the needed information to compactly encode a ChanAccess
type ChanAccessPrestring struct {
AssignmentStr string
}

func (c ChanAccessPrestring) String() string {
var sb strings.Builder
sb.WriteString("uninitialized; nil channel accessed")
sb.WriteString(c.AssignmentStr)
return sb.String()
}

// LocalVarAssignDeep is when a value flows to a point where it is assigned deeply into a local variable of deeply nonnil type
type LocalVarAssignDeep struct {
*TriggerIfDeepNonNil
Expand Down
1 change: 0 additions & 1 deletion annotation/consume_trigger_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@ var initStructsConsumingAnnotationTrigger = []any{
&VariadicParamAssignDeep{TriggerIfNonNil: &TriggerIfNonNil{Ann: newMockKey()}},
&FieldAssignDeep{TriggerIfDeepNonNil: &TriggerIfDeepNonNil{Ann: newMockKey()}},
&GlobalVarAssignDeep{TriggerIfDeepNonNil: &TriggerIfDeepNonNil{Ann: newMockKey()}},
&ChanAccess{ConsumeTriggerTautology: &ConsumeTriggerTautology{}},
&LocalVarAssignDeep{TriggerIfDeepNonNil: &TriggerIfDeepNonNil{Ann: newMockKey()}},
&ChanSend{TriggerIfDeepNonNil: &TriggerIfDeepNonNil{Ann: newMockKey()}},
&FldEscape{TriggerIfNonNil: &TriggerIfNonNil{Ann: newMockKey()}},
Expand Down
14 changes: 7 additions & 7 deletions assertion/function/assertiontree/backprop.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,13 +116,13 @@ func backpropAcrossNode(rootNode *RootAssertionNode, node ast.Node) error {
// backpropAcrossSend handles backpropagation for send statements. It is designed to be called from
// backpropAcrossNode as a special handler.
func backpropAcrossSend(rootNode *RootAssertionNode, node *ast.SendStmt) error {
// Added this consumer since sending over a nil channel can cause panic
rootNode.AddConsumption(&annotation.ConsumeTrigger{
Annotation: &annotation.ChanAccess{ConsumeTriggerTautology: &annotation.ConsumeTriggerTautology{}},
Expr: node.Chan,
Guards: util.NoGuards(),
})

// Note that for channel sends, we have:
// (1) A send to a nil channel blocks forever;
// (2) A send to a closed channel panics.
// (1) falls out of scope for NilAway and hence we do not create a consumer here for the
// channel variable. For (2), since we do not track the state of the channels, we currently
// can not support it.
// TODO: rethink our strategy of handling channels (#192).
consumer, err := exprAsAssignmentConsumer(rootNode, node, nil)
if err != nil {
return err
Expand Down
17 changes: 8 additions & 9 deletions assertion/function/assertiontree/root_assertion_node.go
Original file line number Diff line number Diff line change
Expand Up @@ -807,15 +807,14 @@ func (r *RootAssertionNode) AddComputation(expr ast.Expr) {
// doesn't need to be non-nil, but really should be
r.AddComputation(expr.X)
case *ast.UnaryExpr:
// channel receive case
if expr.Op == token.ARROW {
// added this consumer since receiving over a nil channel can cause panic
r.AddConsumption(&annotation.ConsumeTrigger{
Annotation: &annotation.ChanAccess{ConsumeTriggerTautology: &annotation.ConsumeTriggerTautology{}},
Expr: expr.X,
Guards: util.NoGuards(),
})
}
// Note if expr.Op == token.ARROW it represents a channel receive (<-X), and we have:
// (1) A receive from a nil channel blocks forever;
// (2) A receive from a closed channel returns the zero value immediately.
// (1) falls out of scope of NilAway, and we have a lot of valid Go code that receives
// from nil channels (e.g., select statements with nilable channels). So we do not create
// consumer for the channel variable here. For (2), since we currently do not track the
// state of channels, we currently can not support it either.
// TODO: rethink our strategy of handling channels (#192).
r.AddComputation(expr.X)
case *ast.FuncLit:
// TODO: analyze the bodies of anonymous functions
Expand Down
1 change: 0 additions & 1 deletion inference/engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -505,7 +505,6 @@ func GobRegister() {
gob.RegisterName(nextStr(), annotation.MapAccessPrestring{})
gob.RegisterName(nextStr(), annotation.MapWrittenToPrestring{})
gob.RegisterName(nextStr(), annotation.SliceAccessPrestring{})
gob.RegisterName(nextStr(), annotation.ChanAccessPrestring{})
gob.RegisterName(nextStr(), annotation.FldAccessPrestring{})
gob.RegisterName(nextStr(), annotation.UseAsErrorResultPrestring{})
gob.RegisterName(nextStr(), annotation.FldAssignPrestring{})
Expand Down
32 changes: 14 additions & 18 deletions testdata/src/go.uber.org/channels/channels.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,10 @@
// See the License for the specific language governing permissions and
// limitations under the License.

/*
These tests aim to ensure that nilaway properly handles channels
<nilaway no inference>
*/
// Package channels tests that nilaway properly handles channels.
// <nilaway no inference>
package channels

// BELOW TESTS CHECK DEEP NILABILITY OF CHANNELS

// nilable(<-nilableChan)
var nilableChan = make(chan *int)
Expand Down Expand Up @@ -528,7 +524,7 @@ func testRangeOverChans(a, b, c, d chan *int) *int {
func takesNonnil(interface{}) {}

func singleKeysEstablishNonnil(ch chan *int) {
v, ok := <-ch //want "uninitialized"
v, ok := <-ch

// here, ch and v should be nilable
takesNonnil(v) //want "passed"
Expand Down Expand Up @@ -581,7 +577,7 @@ func plainReflCheck(ch chan any) any {
return ch //want "returned"
}

_, ok := <-ch //want "uninitialized"
_, ok := <-ch

if ok {
return ch
Expand All @@ -595,39 +591,39 @@ var nilChanGlobal chan string
var nonnilChanGlobal = make(chan string)

func testSendToGlobalChan() {
nilChanGlobal <- "xyz" //want "uninitialized"
nilChanGlobal <- "xyz"
nonnilChanGlobal <- "xyz"
}

// nonnil(nonnilChanParam)
func testSendToParamChan(nilChanParam chan string, nonnilChanParam chan string) {
nilChanParam <- "xyz" //want "uninitialized"
nilChanParam <- "xyz"
nonnilChanParam <- "xyz"
}

func testSendToLocalChan() {
var nilChanLocal chan string
nilChanLocal <- "xyz" //want "uninitialized"
nilChanLocal <- "xyz"

var nonnilChanLocal = make(chan string)
nonnilChanLocal <- "xyz"
}

func testRecvFromGlobalChan() (string, string) {
return <-nilChanGlobal, <-nonnilChanGlobal //want "uninitialized"
return <-nilChanGlobal, <-nonnilChanGlobal
}

// nonnil(nonnilChanParam)
func testRecvFromParamChan(nilChanParam chan string, nonnilChanParam chan string) {
v1 := <-nilChanParam //want "uninitialized"
v1 := <-nilChanParam
v2 := <-nonnilChanParam
func(...any) {}(v1, v2)
}

func testRecvFromLocalChan() {
var nilChanLocal chan string
nilChanLocal <- "xyz" //want "uninitialized"
v1 := <-nilChanLocal //want "uninitialized"
nilChanLocal <- "xyz"
v1 := <-nilChanLocal

var nonnilChanLocal = make(chan string)
nonnilChanLocal <- "xyz"
Expand All @@ -648,14 +644,14 @@ func retNonNilChan() chan string {

func testSendRecvFuncRet() {
nilChanLocal := retNilChan()
nilChanLocal <- "xyz" //want "uninitialized"
v1 := <-nilChanLocal //want "uninitialized"
nilChanLocal <- "xyz"
v1 := <-nilChanLocal

nonnilChanLocal := retNonNilChan()
nonnilChanLocal <- "xyz"
v2 := <-nonnilChanLocal

nilChanLocal <- <-nonnilChanGlobal //want "uninitialized"
nilChanLocal <- <-nonnilChanGlobal
nonnilChanLocal <- <-nonnilChanGlobal

func(...any) {}(v1, v2)
Expand Down
22 changes: 8 additions & 14 deletions testdata/src/go.uber.org/errormessage/errormessage.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,28 +101,22 @@ func test9(m map[int]*int) {
print(*y) //want "`m\\[0\\]` to `x`"
}

func test10(ch chan *int) {
x := <-ch //want "nil channel accessed"
y := x
print(*y)
}

func callTest10() {
var ch chan *int
test10(ch)
// nilable(nilableChan) nonnil(nonnilDeeplyNonnilChan, <-nonnilDeeplyNonnilChan)
func test10(nilableChan chan *int, nonnilDeeplyNonnilChan chan *int) {
x := 1
nilableChan <- &x
// Sending nilable values to nonnil and deeply nonnil channels is not OK.
var y *int
nonnilDeeplyNonnilChan <- y //want "`y` assigned deeply into parameter arg `nonnilDeeplyNonnilChan`"
}

// nilable(s)
func test11(s []*int) {
x := s[0] //want "`s` sliced into"
y := x
print(*y)
}

func callTest11() {
var s []*int
test11(s)
}

func test12(mp map[int]S, i int) {
x := mp[i] // unrelated assignment, should not be printed in the error message
_ = x
Expand Down

0 comments on commit d6d74bb

Please sign in to comment.