Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle assignments to global variables from the init function #253

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

k4n4ry
Copy link

@k4n4ry k4n4ry commented Jun 2, 2024

fixes #56

  • Added logic in GlobalAnalyzer's run() to retrieve the init function from the file.
  • Within getGlobalConsumers(), analyzed assignments to global variables from the init function and used it as one of the criteria for determining consumer creation.

Please provide comments if this implementation differs from nilaway's coding practices or design principles. Thank you.

@CLAassistant
Copy link

CLAassistant commented Jun 2, 2024

CLA assistant check
All committers have signed the CLA.

@sonalmahajan15
Copy link
Contributor

@k4n4ry, thank you for submitting the PR! We'll take a look soon and follow up with you.

Copy link

codecov bot commented Jun 8, 2024

Codecov Report

Attention: Patch coverage is 96.00000% with 2 lines in your changes missing coverage. Please review.

Project coverage is 87.70%. Comparing base (c91e71c) to head (9c61f48).

Files with missing lines Patch % Lines
assertion/global/analyzer.go 92.85% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #253      +/-   ##
==========================================
+ Coverage   87.60%   87.70%   +0.09%     
==========================================
  Files          63       63              
  Lines        7916     7961      +45     
==========================================
+ Hits         6935     6982      +47     
+ Misses        799      798       -1     
+ Partials      182      181       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.


// getInitFuncDecl searches for the init function declaration in the given *ast.File.
// It returns the *ast.FuncDecl representing the init function if found, or nil otherwise.
func getInitFuncDecl(file *ast.File) *ast.FuncDecl {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This only returns the first init function. I believe there can be an arbitrary number of them.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you. I have made the changes you suggested and added additional tests.

@SongShawn
Copy link

package sxn01

var (
	b *int
)

func init() {
	init_b_val()
}

func init_b_val() {
	b = new(int)
}

func use_global_var() {
	*b = 20
}

The above code triggers a false positive
image

@k4n4ry
Copy link
Author

k4n4ry commented Jun 21, 2024

@SongShawn
Thanks for the review. I've updated the function to retrieve not only the init functions but also all functions called directly or indirectly from them. dce532a

@k4n4ry
Copy link
Author

k4n4ry commented Aug 11, 2024

@sonalmahajan15
Hello, could you please rerun the CI and review this code? Thank you.

@yuxincs
Copy link
Contributor

yuxincs commented Aug 21, 2024

I'm updating your branch since we recently merged CI fixes, which are required for the CI to pass on forked repositories. I'll review the code this week :)

Copy link

Golden Test

Warning

❌ NilAway errors reported on stdlib are different 📉.

3271 errors on base branch (main, c91e71c)
2968 errors on test branch (69d920e)

Diffs
+ /opt/hostedtoolcache/go/1.22.6/x64/src/archive/tar/writer.go:536:7: Potential nil panic detected. Observed nil flow from source to dereference point: 
+ 	- fmt/print.go:170:11: literal `nil` assigned into field `buf`
+ 	- fmt/print.go:225:19: field `buf` passed as arg `p` to `Write()`
+ 	- tar/writer.go:533:26: passed as parameter `b` to `regFileWriter.Write()` (implementing `Writer.Write()`)
+ 	- tar/writer.go:536:7: function parameter `b` sliced into
+ /opt/hostedtoolcache/go/1.22.6/x64/src/archive/tar/writer.go:576:7: Potential nil panic detected. Observed nil flow from source to dereference point: 
+ 	- fmt/print.go:170:11: literal `nil` assigned into field `buf`
+ 	- fmt/print.go:225:19: field `buf` passed as arg `p` to `Write()`
+ 	- tar/writer.go:573:29: passed as parameter `b` to `sparseFileWriter.Write()` (implementing `Writer.Write()`)
+ 	- tar/writer.go:576:7: function parameter `b` sliced into
+ /opt/hostedtoolcache/go/1.22.6/x64/src/crypto/md5/md5.go:128:7: Potential nil panic detected. Observed nil flow from source to dereference point: 
+ 	- fmt/print.go:170:11: literal `nil` assigned into field `buf`
+ 	- fmt/print.go:225:19: field `buf` passed as arg `p` to `Write()`
+ 	- io/multi.go:83:23: passed as parameter `p` to `multiWriter.Write()` (implementing `Writer.Write()`)
+ 	- io/multi.go:85:20: function parameter `p` passed as arg `p` to `Write()`
+ 	- md5/md5.go:111:18: passed as parameter `p` to `digest.Write()` (implementing `Writer.Write()`)
+ 	- md5/md5.go:128:7: function parameter `p` sliced into
+ /opt/hostedtoolcache/go/1.22.6/x64/src/crypto/md5/md5.go:128:7: Potential nil panic detected. Observed nil flow from source to dereference point: 
+ 	- fmt/print.go:170:11: literal `nil` assigned into field `buf`
+ 	- fmt/print.go:225:19: field `buf` passed as arg `p` to `Write()`
+ 	- md5/md5.go:111:18: passed as parameter `p` to `digest.Write()` (implementing `Writer.Write()`)
+ 	- md5/md5.go:128:7: function parameter `p` sliced into
+ /opt/hostedtoolcache/go/1.22.6/x64/src/crypto/sha1/sha1.go:134:7: Potential nil panic detected. Observed nil flow from source to dereference point: 
+ 	- fmt/print.go:170:11: literal `nil` assigned into field `buf`
+ 	- fmt/print.go:225:19: field `buf` passed as arg `p` to `Write()`
+ 	- io/multi.go:83:23: passed as parameter `p` to `multiWriter.Write()` (implementing `Writer.Write()`)
+ 	- io/multi.go:85:20: function parameter `p` passed as arg `p` to `Write()`
+ 	- sha1/sha1.go:123:18: passed as parameter `p` to `digest.Write()` (implementing `Writer.Write()`)
+ 	- sha1/sha1.go:134:7: function parameter `p` sliced into
+ /opt/hostedtoolcache/go/1.22.6/x64/src/crypto/sha1/sha1.go:134:7: Potential nil panic detected. Observed nil flow from source to dereference point: 
+ 	- fmt/print.go:170:11: literal `nil` assigned into field `buf`
+ 	- fmt/print.go:225:19: field `buf` passed as arg `p` to `Write()`
+ 	- sha1/sha1.go:123:18: passed as parameter `p` to `digest.Write()` (implementing `Writer.Write()`)
+ 	- sha1/sha1.go:134:7: function parameter `p` sliced into
+ /opt/hostedtoolcache/go/1.22.6/x64/src/crypto/sha256/sha256.go:190:7: Potential nil panic detected. Observed nil flow from source to dereference point: 
+ 	- fmt/print.go:170:11: literal `nil` assigned into field `buf`
+ 	- fmt/print.go:225:19: field `buf` passed as arg `p` to `Write()`
+ 	- io/multi.go:83:23: passed as parameter `p` to `multiWriter.Write()` (implementing `Writer.Write()`)
+ 	- io/multi.go:85:20: function parameter `p` passed as arg `p` to `Write()`
+ 	- sha256/sha256.go:179:18: passed as parameter `p` to `digest.Write()` (implementing `Writer.Write()`)
+ 	- sha256/sha256.go:190:7: function parameter `p` sliced into
+ /opt/hostedtoolcache/go/1.22.6/x64/src/crypto/sha256/sha256.go:190:7: Potential nil panic detected. Observed nil flow from source to dereference point: 
+ 	- fmt/print.go:170:11: literal `nil` assigned into field `buf`
+ 	- fmt/print.go:225:19: field `buf` passed as arg `p` to `Write()`
+ 	- sha256/sha256.go:179:18: passed as parameter `p` to `digest.Write()` (implementing `Writer.Write()`)
+ 	- sha256/sha256.go:190:7: function parameter `p` sliced into
+ /opt/hostedtoolcache/go/1.22.6/x64/src/crypto/sha512/sha512.go:269:7: Potential nil panic detected. Observed nil flow from source to dereference point: 
+ 	- fmt/print.go:170:11: literal `nil` assigned into field `buf`
+ 	- fmt/print.go:225:19: field `buf` passed as arg `p` to `Write()`
+ 	- io/multi.go:83:23: passed as parameter `p` to `multiWriter.Write()` (implementing `Writer.Write()`)
+ 	- io/multi.go:85:20: function parameter `p` passed as arg `p` to `Write()`
+ 	- sha512/sha512.go:256:18: passed as parameter `p` to `digest.Write()` (implementing `Writer.Write()`)
+ 	- sha512/sha512.go:269:7: function parameter `p` sliced into
+ /opt/hostedtoolcache/go/1.22.6/x64/src/crypto/sha512/sha512.go:269:7: Potential nil panic detected. Observed nil flow from source to dereference point

 ...(truncated)...

l panic detected. Observed nil flow from source to dereference point: 
- 	- os/file_unix.go:108:10: literal `nil` returned from `NewFile()` in position 0
- 	- os/file.go:66:2: result 0 of `NewFile()` assigned into global variable `Stdout`
- 	- rand/example_test.go:53:27: global variable `Stdout` passed as arg `output` to `NewWriter()`
- 	- tabwriter/tabwriter.go:599:26: function parameter `output` passed as arg `output` to `Init()`
- 	- tabwriter/tabwriter.go:212:13: function parameter `output` assigned into field `output`
- 	- tabwriter/tabwriter.go:251:12: field `output` called `Write()`
- /opt/hostedtoolcache/go/1.22.6/x64/src/text/tabwriter/tabwriter.go:251:12: Potential nil panic detected. Observed nil flow from source to dereference point: 
- 	- os/file_unix.go:108:10: literal `nil` returned from `NewFile()` in position 0
- 	- os/file.go:66:2: result 0 of `NewFile()` assigned into global variable `Stdout`
- 	- tabwriter/example_test.go:17:9: global variable `Stdout` passed as arg `output` to `Init()`
- 	- tabwriter/tabwriter.go:212:13: function parameter `output` assigned into field `output`
- 	- tabwriter/tabwriter.go:251:12: field `output` called `Write()`
- /opt/hostedtoolcache/go/1.22.6/x64/src/text/tabwriter/tabwriter.go:251:12: Potential nil panic detected. Observed nil flow from source to dereference point: 
- 	- os/file_unix.go:108:10: literal `nil` returned from `NewFile()` in position 0
- 	- os/file.go:66:2: result 0 of `NewFile()` assigned into global variable `Stdout`
- 	- tabwriter/example_test.go:26:9: global variable `Stdout` passed as arg `output` to `Init()`
- 	- tabwriter/tabwriter.go:212:13: function parameter `output` assigned into field `output`
- 	- tabwriter/tabwriter.go:251:12: field `output` called `Write()`
- /opt/hostedtoolcache/go/1.22.6/x64/src/text/tabwriter/tabwriter.go:251:12: Potential nil panic detected. Observed nil flow from source to dereference point: 
- 	- os/file_unix.go:108:10: literal `nil` returned from `NewFile()` in position 0
- 	- os/file.go:66:2: result 0 of `NewFile()` assigned into global variable `Stdout`
- 	- tabwriter/example_test.go:43:27: global variable `Stdout` passed as arg `output` to `NewWriter()`
- 	- tabwriter/tabwriter.go:599:26: function parameter `output` passed as arg `output` to `Init()`
- 	- tabwriter/tabwriter.go:212:13: function parameter `output` assigned into field `output`
- 	- tabwriter/tabwriter.go:251:12: field `output` called `Write()`
- /opt/hostedtoolcache/go/1.22.6/x64/src/text/tabwriter/tabwriter.go:251:12: Potential nil panic detected. Observed nil flow from source to dereference point: 
- 	- os/file_unix.go:108:10: literal `nil` returned from `NewFile()` in position 0
- 	- os/file.go:66:2: result 0 of `NewFile()` assigned into global variable `Stdout`
- 	- tabwriter/example_test.go:61:27: global variable `Stdout` passed as arg `output` to `NewWriter()`
- 	- tabwriter/tabwriter.go:599:26: function parameter `output` passed as arg `output` to `Init()`
- 	- tabwriter/tabwriter.go:212:13: function parameter `output` assigned into field `output`
- 	- tabwriter/tabwriter.go:251:12: field `output` called `Write()`
- /opt/hostedtoolcache/go/1.22.6/x64/src/text/tabwriter/tabwriter.go:251:12: Potential nil panic detected. Observed nil flow from source to dereference point: 
- 	- os/file_unix.go:108:10: literal `nil` returned from `NewFile()` in position 0
- 	- os/file.go:66:2: result 0 of `NewFile()` assigned into global variable `Stdout`
- 	- v2/example_test.go:54:27: global variable `Stdout` passed as arg `output` to `NewWriter()`
- 	- tabwriter/tabwriter.go:599:26: function parameter `output` passed as arg `output` to `Init()`
- 	- tabwriter/tabwriter.go:212:13: function parameter `output` assigned into field `output`
- 	- tabwriter/tabwriter.go:251:12: field `output` called `Write()`
- /opt/hostedtoolcache/go/1.22.6/x64/src/text/template/funcs.go:635:2: Potential nil panic detected. Observed nil flow from source to dereference point: 
- 	- os/file_unix.go:108:10: literal `nil` returned from `NewFile()` in position 0
- 	- os/file.go:66:2: result 0 of `NewFile()` assigned into global variable `Stdout`
- 	- template/example_test.go:104:22: global variable `Stdout` passed as arg `w` to `HTMLEscape()`
- 	- template/escape.go:970:22: function parameter `w` passed as arg `w` to `HTMLEscape()`
- 	- template/funcs.go:635:2: function parameter `w` called `Write()`
- /opt/hostedtoolcache/go/1.22.6/x64/src/text/template/funcs.go:718:2: Potential nil panic detected. Observed nil flow from source to dereference point: 
- 	- os/file_unix.go:108:10: literal `nil` returned from `NewFile()` in position 0
- 	- os/file.go:66:2: result 0 of `NewFile()` assigned into global variable `Stdout`
- 	- template/example_test.go:109:20: global variable `Stdout` passed as arg `w` to `JSEscape()`
- 	- template/escape.go:986:20: function parameter `w` passed as arg `w` to `JSEscape()`
- 	- template/funcs.go:718:2: function parameter `w` called `Write()`

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement special handling for init
6 participants