From d9f37288c249ddc2b530bf3b82aa6aa9cafc4ec5 Mon Sep 17 00:00:00 2001 From: Alex Useche Date: Wed, 24 Jan 2024 12:44:49 -0800 Subject: [PATCH 1/4] reduced FPs for invalid-usage-of-modified-variable and missing-runlock-on-*mutex --- go/invalid-usage-of-modified-variable.go | 57 +++++++++++++++++----- go/invalid-usage-of-modified-variable.yaml | 19 ++++++++ go/missing-runlock-on-rwmutex.go | 11 +++++ go/missing-runlock-on-rwmutex.yaml | 6 +++ go/missing-unlock-before-return.go | 11 +++++ go/missing-unlock-before-return.yaml | 6 +++ 6 files changed, 97 insertions(+), 13 deletions(-) diff --git a/go/invalid-usage-of-modified-variable.go b/go/invalid-usage-of-modified-variable.go index 6615d47..6e2c598 100644 --- a/go/invalid-usage-of-modified-variable.go +++ b/go/invalid-usage-of-modified-variable.go @@ -13,7 +13,7 @@ type Engineer struct { } func (e *Engineer) String() string { - return e.FName + " " + e.LName + return e.FName + " " + e.LName } type Address struct { @@ -45,7 +45,7 @@ func main() { } var eng12 *Engineer - // ruleid: invalid-usage-of-modified-variable + // ruleid: invalid-usage-of-modified-variable eng12, err = getEngineerAtIndex(engineers, 5) if err != nil { fmt.Printf("Unable to obtain engineer with FName %s\n", eng12.FName) @@ -113,7 +113,6 @@ func main() { fmt.Printf("Something") } - // FALSE POSITIVES eng3 := Engineer{4, "Lee", "Renaldo", 50, nil} // ok: invalid-usage-of-modified-variable @@ -170,7 +169,6 @@ func main() { fmt.Printf("Something") } - var eng8 *Engineer // ok: invalid-usage-of-modified-variable eng8, err = getEngineerAtIndex(engineers, 7) @@ -210,13 +208,26 @@ func main() { fmt.Printf("Something") } - // ok: invalid-usage-of-modified-variable + // ok: invalid-usage-of-modified-variable eng93, err := getEngineerAtIndex(engineers, 8) - if err != nil { - eng93 = &Engineer{0, "N/A", "N/A", 0, nil} - eng93 = &Engineer{eng93.Id, "N/A", "N/A", 0, nil} - } + if err != nil { + eng93 = &Engineer{0, "N/A", "N/A", 0, nil} + eng93 = &Engineer{eng93.Id, "N/A", "N/A", 0, nil} + } + // ok: invalid-usage-of-modified-variable + eng94, err := getEngineerAtOrDefault(engineers, 8) + if err != nil { + if eng94 != nil { + fmt.Printf("Got engineer %s %s\n", eng94.FName, eng94.LName) + } + } + + // ok: invalid-usage-of-modified-variable + eng95, err := getEngineerAtOrDefault(engineers, 8) + if err != nil && eng95.FName == "Doe" { + fmt.Println("Got a generic engineer") + } // TRUE POSITIVES @@ -226,11 +237,11 @@ func main() { eng11.Address = nil } - // ruleid: invalid-usage-of-modified-variable + // ruleid: invalid-usage-of-modified-variable eng12, err = getEngineerAtIndex(engineers, 8) - if err != nil { - eng12 = &Engineer{eng12.Id, "N/A", "N/A", 0, nil} - } + if err != nil { + eng12 = &Engineer{eng12.Id, "N/A", "N/A", 0, nil} + } // The following test case should match, but I was unable to find a way to // match it without causing some of the false positives to match. This is @@ -285,6 +296,26 @@ func getEngineerAndAddressByIndex(slice []Engineer, idx int) (*Engineer, *Addres return &slice[idx], slice[idx].Address, nil } +func getEngineerAtOrDefault(slice []Engineer, idx int) (*Engineer, error) { + if idx >= len(slice) { + return nil, fmt.Errorf("invalid index") + } + return &slice[idx], nil +} + +func defaultEngineer() Engineer { + return Engineer{ + 1, + "Jane", + "Doe", + 24, + &Address{ + "San Marcos", + "California", + }, + } +} + func getEngineers() []Engineer { return []Engineer{ { diff --git a/go/invalid-usage-of-modified-variable.yaml b/go/invalid-usage-of-modified-variable.yaml index 6db447f..b8a8b3d 100644 --- a/go/invalid-usage-of-modified-variable.yaml +++ b/go/invalid-usage-of-modified-variable.yaml @@ -41,3 +41,22 @@ rules: ... <... $X.$Y ...> } + - pattern-not: | + ..., $X, ..., $ERR = ... + if $ERR != nil { + ... + if $X != nil { + <... $X.$Y ...> + } + ... + } + - pattern-not: | + ..., $X, ..., $ERR := ... + if $ERR != nil { + ... + if $X != nil && <... $X.$Y ...> { + ... + } + ... + } + diff --git a/go/missing-runlock-on-rwmutex.go b/go/missing-runlock-on-rwmutex.go index ad04f1c..09971fb 100644 --- a/go/missing-runlock-on-rwmutex.go +++ b/go/missing-runlock-on-rwmutex.go @@ -7,6 +7,8 @@ import ( "sync" ) +type RUnlocker func() + type RWContainer struct { mu sync.RWMutex counters map[string]int @@ -76,3 +78,12 @@ func (c *RWContainer) inc_FP(name string) error { c.mu.RUnlock() return nil } + +func (c *RWContainer) inc4FP(name string) RUnlocker { + c.mu.Lock() + c.counters[name]++ + return func() { + // ok: missing-unlock-before-return + c.mu.Unlock() + } +} diff --git a/go/missing-runlock-on-rwmutex.yaml b/go/missing-runlock-on-rwmutex.yaml index d076278..616ab1c 100644 --- a/go/missing-runlock-on-rwmutex.yaml +++ b/go/missing-runlock-on-rwmutex.yaml @@ -34,3 +34,9 @@ rules: $FOO(..., ..., func(...) { ... }) + - pattern-not-inside: | + return func(...) { + ... + $T.RUnlock() + ... + } diff --git a/go/missing-unlock-before-return.go b/go/missing-unlock-before-return.go index fc400cc..13624ad 100644 --- a/go/missing-unlock-before-return.go +++ b/go/missing-unlock-before-return.go @@ -7,6 +7,8 @@ import ( "sync" ) +type Unlocker func() + type Container struct { mu sync.Mutex counters map[string]int @@ -102,3 +104,12 @@ func (c *Container) inc3(name string) error { // ruleid: missing-unlock-before-return return nil } + +func (c *Container) inc4FP(name string) Unlocker { + c.mu.Lock() + c.counters[name]++ + return func() { + // ok: missing-unlock-before-return + c.mu.Unlock() + } +} diff --git a/go/missing-unlock-before-return.yaml b/go/missing-unlock-before-return.yaml index 4d45b6a..cd640ea 100644 --- a/go/missing-unlock-before-return.yaml +++ b/go/missing-unlock-before-return.yaml @@ -35,3 +35,9 @@ rules: $FOO(..., ..., func(...) { ... }) + - pattern-not-inside: | + return func(...) { + ... + $T.Unlock() + ... + } From 88c7b3099845d85f5b76b8da22d9e266bbaacf13 Mon Sep 17 00:00:00 2001 From: Alex Useche Date: Wed, 24 Jan 2024 12:55:30 -0800 Subject: [PATCH 2/4] ran prettier on invalid-usage-of-modified-variable.yaml --- go/invalid-usage-of-modified-variable.yaml | 1 - 1 file changed, 1 deletion(-) diff --git a/go/invalid-usage-of-modified-variable.yaml b/go/invalid-usage-of-modified-variable.yaml index b8a8b3d..3a6cbea 100644 --- a/go/invalid-usage-of-modified-variable.yaml +++ b/go/invalid-usage-of-modified-variable.yaml @@ -59,4 +59,3 @@ rules: } ... } - From f0948f2b9bd0268a060cd4a0ffafba725dddc169 Mon Sep 17 00:00:00 2001 From: Alex Useche <1915998+hex0punk@users.noreply.github.com> Date: Thu, 8 Feb 2024 15:06:23 -0800 Subject: [PATCH 3/4] Update missing-runlock-on-rwmutex.go --- go/missing-runlock-on-rwmutex.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go/missing-runlock-on-rwmutex.go b/go/missing-runlock-on-rwmutex.go index 09971fb..1d57a46 100644 --- a/go/missing-runlock-on-rwmutex.go +++ b/go/missing-runlock-on-rwmutex.go @@ -83,7 +83,7 @@ func (c *RWContainer) inc4FP(name string) RUnlocker { c.mu.Lock() c.counters[name]++ return func() { - // ok: missing-unlock-before-return + // ok: missing-runlock-before-return c.mu.Unlock() } } From d3a44e597a7885c0c1804d73aeacefbc7ea51620 Mon Sep 17 00:00:00 2001 From: Alex Useche <1915998+hex0punk@users.noreply.github.com> Date: Thu, 8 Feb 2024 15:10:58 -0800 Subject: [PATCH 4/4] Update missing-runlock-on-rwmutex.go --- go/missing-runlock-on-rwmutex.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go/missing-runlock-on-rwmutex.go b/go/missing-runlock-on-rwmutex.go index 1d57a46..ac2621b 100644 --- a/go/missing-runlock-on-rwmutex.go +++ b/go/missing-runlock-on-rwmutex.go @@ -83,7 +83,7 @@ func (c *RWContainer) inc4FP(name string) RUnlocker { c.mu.Lock() c.counters[name]++ return func() { - // ok: missing-runlock-before-return + // ok: missing-runlock-on-rwmutex c.mu.Unlock() } }