From 502fcededca91d10faca860c8467f4ff12ac28b2 Mon Sep 17 00:00:00 2001 From: komem3 Date: Tue, 23 Mar 2021 23:03:57 +0900 Subject: [PATCH 1/5] go/analysis/passes/nilness: fixed slice not being considered as non-nil The slice produced by `s := []T{}` or `s := make([]T, 0)` will always be non-nil. But, nilness could not detect it, so I fixed it. Fixes golang/go#45177 --- go/analysis/passes/nilness/nilness.go | 3 ++- go/analysis/passes/nilness/testdata/src/a/a.go | 4 ++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/go/analysis/passes/nilness/nilness.go b/go/analysis/passes/nilness/nilness.go index f0d2c7edfec..86f59ca42e4 100644 --- a/go/analysis/passes/nilness/nilness.go +++ b/go/analysis/passes/nilness/nilness.go @@ -273,7 +273,8 @@ func nilnessOf(stack []fact, v ssa.Value) nilness { *ssa.MakeClosure, *ssa.MakeInterface, *ssa.MakeMap, - *ssa.MakeSlice: + *ssa.MakeSlice, + *ssa.Slice: return isnonnil case *ssa.Const: if v.IsNil() { diff --git a/go/analysis/passes/nilness/testdata/src/a/a.go b/go/analysis/passes/nilness/testdata/src/a/a.go index b9426f444d9..b1aaa2556bc 100644 --- a/go/analysis/passes/nilness/testdata/src/a/a.go +++ b/go/analysis/passes/nilness/testdata/src/a/a.go @@ -71,6 +71,10 @@ func f3() error { if ch != nil { // want "tautological condition: non-nil != nil" print(0) } + slice := make([]string, 0) + if slice == nil { // want "impossible condition: non-nil == nil" + print(0) + } return nil } From 872709e688818697d7bb398cd8b5cdc9c7c86ee4 Mon Sep 17 00:00:00 2001 From: komem3 Date: Wed, 24 Mar 2021 19:24:49 +0900 Subject: [PATCH 2/5] fixed nil slice to be considered nil --- go/analysis/passes/nilness/nilness.go | 8 +++++-- .../passes/nilness/testdata/src/a/a.go | 21 ++++++++++++++----- 2 files changed, 22 insertions(+), 7 deletions(-) diff --git a/go/analysis/passes/nilness/nilness.go b/go/analysis/passes/nilness/nilness.go index 86f59ca42e4..4527a4b5562 100644 --- a/go/analysis/passes/nilness/nilness.go +++ b/go/analysis/passes/nilness/nilness.go @@ -273,8 +273,12 @@ func nilnessOf(stack []fact, v ssa.Value) nilness { *ssa.MakeClosure, *ssa.MakeInterface, *ssa.MakeMap, - *ssa.MakeSlice, - *ssa.Slice: + *ssa.MakeSlice: + return isnonnil + case *ssa.Slice: + if cons, ok := v.X.(*ssa.Const); ok && cons.IsNil() { + return isnil + } return isnonnil case *ssa.Const: if v.IsNil() { diff --git a/go/analysis/passes/nilness/testdata/src/a/a.go b/go/analysis/passes/nilness/testdata/src/a/a.go index b1aaa2556bc..abe26967d3c 100644 --- a/go/analysis/passes/nilness/testdata/src/a/a.go +++ b/go/analysis/passes/nilness/testdata/src/a/a.go @@ -71,10 +71,6 @@ func f3() error { if ch != nil { // want "tautological condition: non-nil != nil" print(0) } - slice := make([]string, 0) - if slice == nil { // want "impossible condition: non-nil == nil" - print(0) - } return nil } @@ -134,7 +130,6 @@ func f9(x interface { b() c() }) { - x.b() // we don't catch this panic because we don't have any facts yet xx := interface { a() @@ -159,6 +154,22 @@ func f9(x interface { } } +func f10() { + s0 := make([]string, 0) + if s0 == nil { // want "impossible condition: non-nil == nil" + print(0) + } + + var s1 []string + if s1 == nil { // want "tautological condition: nil == nil" + print(0) + } + s2 := s1[:] + if s2 == nil { // want "tautological condition: nil == nil" + print(0) + } +} + func unknown() bool { return false } From 3dd1004a157ee50689c9aa724f553220f3d61aa1 Mon Sep 17 00:00:00 2001 From: komem3 Date: Thu, 25 Mar 2021 20:16:13 +0900 Subject: [PATCH 3/5] Changed to perform null check recursively. --- go/analysis/passes/nilness/nilness.go | 7 ++++--- go/analysis/passes/nilness/testdata/src/a/a.go | 2 +- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/go/analysis/passes/nilness/nilness.go b/go/analysis/passes/nilness/nilness.go index 4527a4b5562..c99955cc824 100644 --- a/go/analysis/passes/nilness/nilness.go +++ b/go/analysis/passes/nilness/nilness.go @@ -276,10 +276,11 @@ func nilnessOf(stack []fact, v ssa.Value) nilness { *ssa.MakeSlice: return isnonnil case *ssa.Slice: - if cons, ok := v.X.(*ssa.Const); ok && cons.IsNil() { - return isnil + // unwrap Slice values recursively, to detect if underlying + // values have any facts recorded or are otherwise known with regard to nilness. + if underlying := nilnessOf(stack, v.X); underlying != unknown { + return underlying } - return isnonnil case *ssa.Const: if v.IsNil() { return isnil diff --git a/go/analysis/passes/nilness/testdata/src/a/a.go b/go/analysis/passes/nilness/testdata/src/a/a.go index abe26967d3c..37b7516b847 100644 --- a/go/analysis/passes/nilness/testdata/src/a/a.go +++ b/go/analysis/passes/nilness/testdata/src/a/a.go @@ -164,7 +164,7 @@ func f10() { if s1 == nil { // want "tautological condition: nil == nil" print(0) } - s2 := s1[:] + s2 := s1[:][:] if s2 == nil { // want "tautological condition: nil == nil" print(0) } From f4c4daa9954c663165755e22c445312231562a2c Mon Sep 17 00:00:00 2001 From: komem3 Date: Fri, 26 Mar 2021 21:07:44 +0900 Subject: [PATCH 4/5] Changed the location of the case closer to similar case --- go/analysis/passes/nilness/nilness.go | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/go/analysis/passes/nilness/nilness.go b/go/analysis/passes/nilness/nilness.go index c99955cc824..58571c7d1a4 100644 --- a/go/analysis/passes/nilness/nilness.go +++ b/go/analysis/passes/nilness/nilness.go @@ -245,7 +245,7 @@ func (n nilness) String() string { return nilnessStrings[n+1] } // or unknown given the dominating stack of facts. func nilnessOf(stack []fact, v ssa.Value) nilness { switch v := v.(type) { - // unwrap ChangeInterface values recursively, to detect if underlying + // unwrap ChangeInterface and Slice values recursively, to detect if underlying // values have any facts recorded or are otherwise known with regard to nilness. // // This work must be in addition to expanding facts about @@ -259,6 +259,10 @@ func nilnessOf(stack []fact, v ssa.Value) nilness { if underlying := nilnessOf(stack, v.X); underlying != unknown { return underlying } + case *ssa.Slice: + if underlying := nilnessOf(stack, v.X); underlying != unknown { + return underlying + } } // Is value intrinsically nil or non-nil? @@ -275,12 +279,6 @@ func nilnessOf(stack []fact, v ssa.Value) nilness { *ssa.MakeMap, *ssa.MakeSlice: return isnonnil - case *ssa.Slice: - // unwrap Slice values recursively, to detect if underlying - // values have any facts recorded or are otherwise known with regard to nilness. - if underlying := nilnessOf(stack, v.X); underlying != unknown { - return underlying - } case *ssa.Const: if v.IsNil() { return isnil From f3a532e568e43ae57080bdf8046ac46d93b4cc3f Mon Sep 17 00:00:00 2001 From: komem3 Date: Tue, 17 May 2022 11:10:35 +0900 Subject: [PATCH 5/5] fix the test by adding more function term numbers --- go/analysis/passes/nilness/testdata/src/a/a.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/go/analysis/passes/nilness/testdata/src/a/a.go b/go/analysis/passes/nilness/testdata/src/a/a.go index bec66930864..aa7f9a8f859 100644 --- a/go/analysis/passes/nilness/testdata/src/a/a.go +++ b/go/analysis/passes/nilness/testdata/src/a/a.go @@ -174,7 +174,7 @@ func unknown() bool { return false } -func f10(a interface{}) { +func f11(a interface{}) { switch a.(type) { case nil: return @@ -185,7 +185,7 @@ func f10(a interface{}) { } } -func f11(a interface{}) { +func f12(a interface{}) { switch a { case nil: return @@ -205,7 +205,7 @@ type innerY struct { value int } -func f12() { +func f13() { var d *Y print(d.value) // want "nil dereference in field selection" }