{"action":"created","issue":{"url":"https://api.github.com/repos/dominikh/go-tools/issues/258","repository_url":"https://api.github.com/repos/dominikh/go-tools","labels_url":"https://api.github.com/repos/dominikh/go-tools/issues/258/labels{/name}","comments_url":"https://api.github.com/repos/dominikh/go-tools/issues/258/comments","events_url":"https://api.github.com/repos/dominikh/go-tools/issues/258/events","html_url":"https://github.com/dominikh/go-tools/issues/258","id":293593999,"number":258,"title":"staticcheck: flag ambiguous evaluation order","user":{"login":"tamird","id":1535036,"avatar_url":"https://avatars0.githubusercontent.com/u/1535036?v=4","gravatar_id":"","url":"https://api.github.com/users/tamird","html_url":"https://github.com/tamird","followers_url":"https://api.github.com/users/tamird/followers","following_url":"https://api.github.com/users/tamird/following{/other_user}","gists_url":"https://api.github.com/users/tamird/gists{/gist_id}","starred_url":"https://api.github.com/users/tamird/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/tamird/subscriptions","organizations_url":"https://api.github.com/users/tamird/orgs","repos_url":"https://api.github.com/users/tamird/repos","events_url":"https://api.github.com/users/tamird/events{/privacy}","received_events_url":"https://api.github.com/users/tamird/received_events","type":"User","site_admin":false},"labels":[{"id":693024929,"url":"https://api.github.com/repos/dominikh/go-tools/labels/new-check","name":"new-check","color":"84b6eb","default":false},{"id":690512109,"url":"https://api.github.com/repos/dominikh/go-tools/labels/research","name":"research","color":"d4c5f9","default":false}],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":1,"created_at":"2018-02-01T16:24:24Z","updated_at":"2018-03-10T00:32:47Z","closed_at":null,"author_association":"NONE","body":"See https://github.com/golang/go/issues/23188.\r\n\r\nIn short, evaluating multiple expressions in a single statement works differently in cmd/compile than it does in gccgo. This can lead to subtle bugs in code such as:\r\n```golang\r\npackage main\r\n\r\nimport \"fmt\"\r\n\r\ntype foo struct{ i int }\r\n\r\nfunc (f *foo) inc() int { f.i++; return f.i }\r\n\r\nfunc main() {\r\n\tvar f1 foo\r\n\tf2, i := f1, f1.inc()\r\n\tfmt.Println(f1, f2, i)\r\n}\r\n```\r\nhttps://play.golang.org/p/r6DksPtox1T\r\n\r\nin cmd/compile, this prints `{1} {1} 1`; in gccgo, it prints `{1} {0} 1`. It would be amazingly helpful to have staticcheck flag these ambiguities."},"comment":{"url":"https://api.github.com/repos/dominikh/go-tools/issues/comments/371983985","html_url":"https://github.com/dominikh/go-tools/issues/258#issuecomment-371983985","issue_url":"https://api.github.com/repos/dominikh/go-tools/issues/258","id":371983985,"user":{"login":"mdempsky","id":38349,"avatar_url":"https://avatars0.githubusercontent.com/u/38349?v=4","gravatar_id":"","url":"https://api.github.com/users/mdempsky","html_url":"https://github.com/mdempsky","followers_url":"https://api.github.com/users/mdempsky/followers","following_url":"https://api.github.com/users/mdempsky/following{/other_user}","gists_url":"https://api.github.com/users/mdempsky/gists{/gist_id}","starred_url":"https://api.github.com/users/mdempsky/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/mdempsky/subscriptions","organizations_url":"https://api.github.com/users/mdempsky/orgs","repos_url":"https://api.github.com/users/mdempsky/repos","events_url":"https://api.github.com/users/mdempsky/events{/privacy}","received_events_url":"https://api.github.com/users/mdempsky/received_events","type":"User","site_admin":false},"created_at":"2018-03-10T00:32:47Z","updated_at":"2018-03-10T00:32:47Z","author_association":"NONE","body":"@dominikh Just curious, how were you thinking of statically detecting this? It seems quite difficult to statically detect without false positives.\r\n\r\nI was thinking of a compiler instrumentation pass that eagerly evaluates all side-effect-free subexpressions as early as possible, and saves those values. Later, when the values are computed normally (which is typically as late as possible), the values can be compared against the older values, and raise an error if they're different.\r\n\r\nTo reduce overhead, I think we can safely ignore expressions that 1) contain only constants and local variables whose address is never taken, and 2) don't contain any dereference operations (i.e., pointer indirection or slice/map indexing).\r\n\r\nFor example, if `x` is a local variable whose address is never taken, then `x + 2` will always evaluate to the same thing whether it's evaluated eagerly or lazily.\r\n\r\nThis is imperfect because it's vulnerable to the ABA problem, but I expect it should handle other cases well.\r\n\r\nAnother possibility might be to extend the race/msan detector with \"read locks\". Then we could just mark the memory ranges as \"locked\" from the time they're first valid to evaluate until when they're actually loaded, and any writes that happen during this time are errors. This seems more complex, but could avoid the ABA problem."}}
{"actor":{"display_login":"mdempsky"}}