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

Add support for handling abnormal control flow #70

Closed
sonalmahajan15 opened this issue Oct 5, 2023 · 5 comments
Closed

Add support for handling abnormal control flow #70

sonalmahajan15 opened this issue Oct 5, 2023 · 5 comments
Labels
enhancement New feature or request

Comments

@sonalmahajan15
Copy link
Contributor

sonalmahajan15 commented Oct 5, 2023

Go exposes "abnormal" control flow primitives, such as the following

  • defer
  • panic
  • recover
  • os.Exit
  • log.Fatal

Currently, NilAway reports false positives in these cases. We must ensure that NilAway's models account for the result of using these primitives.

@FiloSottile
Copy link

Another case is (*testing.T).Fatal.

For example, I am getting a lot of false positives for

foo, err := Foo()
fatalIfErr(t, err)
foo.Bar
func fatalIfErr(t testing.TB, err error) {
	t.Helper()
	if err != nil {
		t.Fatal(err)
	}
}

@ksurent
Copy link

ksurent commented Nov 20, 2023

One thing to consider here is that many production systems wrap built–in panic calls to have errors reported to some error tracking system. Case in point: Uber's own zap (Panic, Panicf, etc).

It would be cool if nilaway could recognise such wrappers automatically but even being able to supply a list of known terminal functions would be good enough (more than just stdlib's log.Fatal).

@dolmen
Copy link

dolmen commented Nov 22, 2023

Also: runtime.Goexit which testing.T.FailNow (called by testing.T.Fatal) and testing.T.SkipNow use.

@sonalmahajan15 sonalmahajan15 added the enhancement New feature or request label Dec 5, 2023
@yuxincs
Copy link
Contributor

yuxincs commented Jun 5, 2024

The support for terminal function calls should already be supported since ctrlflow analyzer correctly constructs the CFG if there is a call to such functions (include the wrappers) in the block. On top of that NilAway should be able to automatically recognize those functions.

Users using OSS drivers should be automatically covered. For bazel/nogo users, please wait for merge of golang/tools#497 upstream (or apply that patch locally). Please let us know if NilAway is still not treating those functions correctly. 😄

This issue will be kept open, but only for tracking support for defer/recover now.

@sonalmahajan15
Copy link
Contributor Author

Addressed in PR #252 .

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

No branches or pull requests

5 participants