Skip to content

fix rowserrcheck reports false positive #1670

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

Merged
merged 3 commits into from
Jan 30, 2021

Conversation

jingyugao
Copy link
Contributor

@jingyugao jingyugao commented Jan 29, 2021

Fixes #943

This PR fix that rowserrcheck reports false positive when handling phi instruction.

For example

func badcase() {
	var db *sql.DB
	var rows *sql.Rows
	if rand.Float64() < 0.5 {
		rows, _ = db.Query("select 1")
	}
	if err := rows.Err(); err != nil {
		panic(err)
	}
}

which ssa is as below:

func badcase():
0:                                                                entry P:0 S:2
	t0 = math/rand.Float64()                                        float64
	t1 = t0 < 0.5:float64                                              bool
	if t1 goto 1 else 2
1:                                                              if.then P:1 S:1
	t2 = (*database/sql.DB).Query(nil:*database/sql.DB, "select 1":string, nil:[]interface{}...) (*database/sql.Rows, error)
	t3 = extract t2 #0                                   *database/sql.Rows
	t4 = extract t2 #1                                                error
	jump 2
2:                                                              if.done P:2 S:2
	// t5'Err is checked,but t5 is not t3's referrer. t5 is phi(...,t3)'referrer.
        t5 = phi [0: nil:*database/sql.Rows, 1: t3] #rows    *database/sql.Rows
	t6 = (*database/sql.Rows).Err(t5)                                 error
	t7 = t6 != nil:error                                               bool
	if t7 goto 3 else 4
3:                                                              if.then P:1 S:0
	t8 = change interface interface{} <- error (t6)             interface{}
	panic t8
4:                                                              if.done P:1 S:0
	return

if we move the assignment statement out of the if block.

func passcase() {
	var db *sql.DB
	var rows *sql.Rows
	if rand.Float64() < 0.5 {
		rows, _ = db.Query("select 1")
	}
	if err := rows.Err(); err != nil {
		panic(err)
	}
}

ths ssa is below

0:                                                                entry P:0 S:2
	t0 = math/rand.Float64()                                        float64
	t1 = t0 < 0.5:float64                                              bool
	t2 = (*database/sql.DB).Query(nil:*database/sql.DB, "select 1":string, nil:[]interface{}...) (*database/sql.Rows, error)
	t3 = extract t2 #0                                   *database/sql.Rows
	t4 = extract t2 #1                                                error
        // here t3 is t2'referrer's referrer
	t5 = (*database/sql.Rows).Err(t3)                                 error
	t6 = t5 != nil:error                                               bool
	if t6 goto 1 else 2
1:                                                              if.then P:1 S:0
	t7 = change interface interface{} <- error (t5)             interface{}
	panic t7
2:                                                              if.done P:1 S:0
	return

@CLAassistant
Copy link

CLAassistant commented Jan 29, 2021

CLA assistant check
All committers have signed the CLA.

@ldez
Copy link
Member

ldez commented Jan 29, 2021

@jingyugao there is an issue with the authorship of your commits, could you recreate your commits with a valid author?

also could you merge jingyugao/rowserrcheck#13

@ldez ldez self-requested a review January 29, 2021 22:51
@jingyugao jingyugao force-pushed the fix_-rowserrcheck branch 5 times, most recently from 3aae547 to 8e1b13f Compare January 30, 2021 01:33
@ldez ldez merged commit 839dd74 into golangci:master Jan 30, 2021
@golangci-automator
Copy link

Hey, @jingyugao — we just merged your PR to golangci-lint! 🔥🚀

golangci-lint is built by awesome people like you. Let us say “thanks”: we just invited you to join the GolangCI organization on GitHub.
This will add you to our team of maintainers. Accept the invite by visiting this link.

By joining the team, you’ll be able to label issues, review pull requests, and merge approved pull requests.
More information about contributing is here.

Thanks again!

@jingyugao jingyugao deleted the fix_-rowserrcheck branch January 30, 2021 16:18
ashanbrown pushed a commit to ashanbrown/golangci-lint that referenced this pull request Feb 20, 2021
@ldez ldez added this to the v1.37 milestone Mar 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
linter: update version Update version of linter
Projects
None yet
Development

Successfully merging this pull request may close these issues.

How can I report some issues about rowserrcheck linter?
3 participants