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

False positive when wrapping errors #276

Open
johannaschwarzcheck24 opened this issue Sep 4, 2024 · 3 comments
Open

False positive when wrapping errors #276

johannaschwarzcheck24 opened this issue Sep 4, 2024 · 3 comments
Labels
false positive Requires more analysis and support

Comments

@johannaschwarzcheck24
Copy link

A minimal example:

The wrap function returns nil if the error passed to it is nil.
The test function will only call wrap and pass err if err is not nil.

func wrap(msg string, err error) error {
	if err == nil {
		return nil
	}
	return errors.New(msg)
}

func test() (*string, error) {
	var err error
	if err != nil {
		return nil, wrap("failed to read file", err)
	}
	s := "qwe"
	return &s, nil
}

func main() {
	b, err := test()
	if err != nil {
		log.Fatal(err)
	}
	_ = *b
}

nilaway reports a potential nil panic:

error: Potential nil panic detected. Observed nil flow from source to dereference point: 
        - nilaway/main.go:18:10: literal `nil` returned from `test()` in position 0 when the error return in position 1 is not guaranteed to be non-nil through all paths
        - nilaway/main.go:29:7: result 0 of `test()` dereferenced via the assignment(s):
                - `test()` to `b` at nilaway/main.go:25:2
@sonalmahajan15 sonalmahajan15 added the false positive Requires more analysis and support label Sep 4, 2024
@sonalmahajan15
Copy link
Contributor

Hmm, we had added context sensitivity via contract support to track such error propagation and not report a false positive. Perhaps some corner case that we missed. @yuxincs, can you take a look?

@johannaschwarzcheck24, thanks for reporting!

@danilobuerger
Copy link

@sonalmahajan15 @yuxincs any chance you can take a look at this?

@sonalmahajan15
Copy link
Contributor

@danilobuerger, thanks for reaching out. We would likely address this in December.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
false positive Requires more analysis and support
Projects
None yet
Development

No branches or pull requests

3 participants