Skip to content

Commit 21afbf4

Browse files
committed
all: merge master (5b5d57c) into gopls-release-branch.0.17
Merge List: + 2024-11-14 5b5d57c gopls/codeaction: fix panic when removing unused parameters with syntax errors. + 2024-11-13 025b812 gopls/internal/server: don't call window/showDocument if unsupported + 2024-11-13 288b9cb gopls/internal/golang: add missing imports in foo_test.go + 2024-11-13 87ac91f gopls/internal/server: revert the gopls.run_govulncheck command + 2024-11-12 c531f1b gopls/internal/golang: avoid crash in hover on field of non-struct + 2024-11-12 a37eeb4 README: mention the git repo + 2024-11-12 8dd84a4 go/ssa/interp: assign phi nodes in parallel For golang/go#70301 Change-Id: I9b584de0bcbd622a6bcf5f06f34232993ac2f9a4
2 parents 3296d74 + 5b5d57c commit 21afbf4

File tree

23 files changed

+971
-185
lines changed

23 files changed

+971
-185
lines changed

README.md

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -76,10 +76,12 @@ golang.org/x/tools/txtar
7676
## Contributing
7777

7878
This repository uses Gerrit for code changes.
79-
To learn how to submit changes, see https://golang.org/doc/contribute.html.
79+
To learn how to submit changes, see https://go.dev/doc/contribute.
80+
81+
The git repository is https://go.googlesource.com/tools.
8082

8183
The main issue tracker for the tools repository is located at
82-
https://github.com/golang/go/issues. Prefix your issue with "x/tools/(your
84+
https://go.dev/issues. Prefix your issue with "x/tools/(your
8385
subdir):" in the subject line, so it is easy to find.
8486

8587
### JavaScript and CSS Formatting

go/ssa/interp/interp.go

Lines changed: 44 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -48,9 +48,11 @@ import (
4848
"fmt"
4949
"go/token"
5050
"go/types"
51+
"log"
5152
"os"
5253
"reflect"
5354
"runtime"
55+
"slices"
5456
"sync/atomic"
5557
_ "unsafe"
5658

@@ -108,6 +110,7 @@ type frame struct {
108110
result value
109111
panicking bool
110112
panic interface{}
113+
phitemps []value // temporaries for parallel phi assignment
111114
}
112115

113116
func (fr *frame) get(key ssa.Value) value {
@@ -379,12 +382,7 @@ func visitInstr(fr *frame, instr ssa.Instruction) continuation {
379382
fr.env[instr] = &closure{instr.Fn.(*ssa.Function), bindings}
380383

381384
case *ssa.Phi:
382-
for i, pred := range instr.Block().Preds {
383-
if fr.prevBlock == pred {
384-
fr.env[instr] = fr.get(instr.Edges[i])
385-
break
386-
}
387-
}
385+
log.Fatal("unreachable") // phis are processed at block entry
388386

389387
case *ssa.Select:
390388
var cases []reflect.SelectCase
@@ -589,25 +587,57 @@ func runFrame(fr *frame) {
589587
if fr.i.mode&EnableTracing != 0 {
590588
fmt.Fprintf(os.Stderr, ".%s:\n", fr.block)
591589
}
592-
block:
593-
for _, instr := range fr.block.Instrs {
590+
591+
nonPhis := executePhis(fr)
592+
for _, instr := range nonPhis {
594593
if fr.i.mode&EnableTracing != 0 {
595594
if v, ok := instr.(ssa.Value); ok {
596595
fmt.Fprintln(os.Stderr, "\t", v.Name(), "=", instr)
597596
} else {
598597
fmt.Fprintln(os.Stderr, "\t", instr)
599598
}
600599
}
601-
switch visitInstr(fr, instr) {
602-
case kReturn:
600+
if visitInstr(fr, instr) == kReturn {
603601
return
604-
case kNext:
605-
// no-op
606-
case kJump:
607-
break block
608602
}
603+
// Inv: kNext (continue) or kJump (last instr)
604+
}
605+
}
606+
}
607+
608+
// executePhis executes the phi-nodes at the start of the current
609+
// block and returns the non-phi instructions.
610+
func executePhis(fr *frame) []ssa.Instruction {
611+
firstNonPhi := -1
612+
for i, instr := range fr.block.Instrs {
613+
if _, ok := instr.(*ssa.Phi); !ok {
614+
firstNonPhi = i
615+
break
616+
}
617+
}
618+
// Inv: 0 <= firstNonPhi; every block contains a non-phi.
619+
620+
nonPhis := fr.block.Instrs[firstNonPhi:]
621+
if firstNonPhi > 0 {
622+
phis := fr.block.Instrs[:firstNonPhi]
623+
// Execute parallel assignment of phis.
624+
//
625+
// See "the swap problem" in Briggs et al's "Practical Improvements
626+
// to the Construction and Destruction of SSA Form" for discussion.
627+
predIndex := slices.Index(fr.block.Preds, fr.prevBlock)
628+
fr.phitemps = fr.phitemps[:0]
629+
for _, phi := range phis {
630+
phi := phi.(*ssa.Phi)
631+
if fr.i.mode&EnableTracing != 0 {
632+
fmt.Fprintln(os.Stderr, "\t", phi.Name(), "=", phi)
633+
}
634+
fr.phitemps = append(fr.phitemps, fr.get(phi.Edges[predIndex]))
635+
}
636+
for i, phi := range phis {
637+
fr.env[phi.(*ssa.Phi)] = fr.phitemps[i]
609638
}
610639
}
640+
return nonPhis
611641
}
612642

613643
// doRecover implements the recover() built-in.

go/ssa/interp/interp_test.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,7 @@ var testdataTests = []string{
136136
"fixedbugs/issue52835.go",
137137
"fixedbugs/issue55086.go",
138138
"fixedbugs/issue66783.go",
139+
"fixedbugs/issue69929.go",
139140
"typeassert.go",
140141
"zeros.go",
141142
"slice2array.go",
Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
package main
2+
3+
// This is a regression test for a bug (#69929) in
4+
// the SSA interpreter in which it would not execute phis in parallel.
5+
//
6+
// The insert function below has interdependent phi nodes:
7+
//
8+
// entry:
9+
// t0 = *root // t0 is x or y before loop
10+
// jump test
11+
// body:
12+
// print(t5) // t5 is x at loop entry
13+
// t3 = t5.Child // t3 is x after loop
14+
// jump test
15+
// test:
16+
// t5 = phi(t0, t3) // t5 is x at loop entry
17+
// t6 = phi(t0, t5) // t6 is y at loop entry
18+
// if t5 != nil goto body else done
19+
// done:
20+
// print(t6)
21+
// return
22+
//
23+
// The two phis:
24+
//
25+
// t5 = phi(t0, t3)
26+
// t6 = phi(t0, t5)
27+
//
28+
// must be executed in parallel as if they were written in Go
29+
// as:
30+
//
31+
// t5, t6 = phi(t0, t3), phi(t0, t5)
32+
//
33+
// with the second phi node observing the original, not
34+
// updated, value of t5. (In more complex examples, the phi
35+
// nodes may be mutually recursive, breaking partial solutions
36+
// based on simple reordering of the phi instructions. See the
37+
// Briggs paper for detail.)
38+
//
39+
// The correct behavior is print(1, root); print(2, root); print(3, root).
40+
// The previous incorrect behavior had print(2, nil).
41+
42+
func main() {
43+
insert()
44+
print(3, root)
45+
}
46+
47+
var root = new(node)
48+
49+
type node struct{ child *node }
50+
51+
func insert() {
52+
x := root
53+
y := x
54+
for x != nil {
55+
y = x
56+
print(1, y)
57+
x = x.child
58+
}
59+
print(2, y)
60+
}
61+
62+
func print(order int, ptr *node) {
63+
println(order, ptr)
64+
if ptr != root {
65+
panic(ptr)
66+
}
67+
}

gopls/doc/codelenses.md

Lines changed: 22 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -97,16 +97,15 @@ Default: off
9797

9898
File type: Go
9999

100-
## `run_govulncheck`: Run govulncheck
100+
## `run_govulncheck`: Run govulncheck (legacy)
101101

102102

103-
This codelens source annotates the `module` directive in a
104-
go.mod file with a command to run Govulncheck.
103+
This codelens source annotates the `module` directive in a go.mod file
104+
with a command to run Govulncheck asynchronously.
105105

106-
[Govulncheck](https://go.dev/blog/vuln) is a static
107-
analysis tool that computes the set of functions reachable
108-
within your application, including dependencies;
109-
queries a database of known security vulnerabilities; and
106+
[Govulncheck](https://go.dev/blog/vuln) is a static analysis tool that
107+
computes the set of functions reachable within your application, including
108+
dependencies; queries a database of known security vulnerabilities; and
110109
reports any potential problems it finds.
111110

112111

@@ -157,4 +156,20 @@ Default: on
157156

158157
File type: go.mod
159158

159+
## `vulncheck`: Run govulncheck
160+
161+
162+
This codelens source annotates the `module` directive in a go.mod file
163+
with a command to run govulncheck synchronously.
164+
165+
[Govulncheck](https://go.dev/blog/vuln) is a static analysis tool that
166+
computes the set of functions reachable within your application, including
167+
dependencies; queries a database of known security vulnerabilities; and
168+
reports any potential problems it finds.
169+
170+
171+
Default: off
172+
173+
File type: go.mod
174+
160175
<!-- END Lenses: DO NOT MANUALLY EDIT THIS SECTION -->

gopls/go.mod

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,3 +26,5 @@ require (
2626
golang.org/x/exp/typeparams v0.0.0-20231108232855-2478ac86f678 // indirect
2727
gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15 // indirect
2828
)
29+
30+
replace golang.org/x/tools => ../

gopls/internal/doc/api.json

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -811,7 +811,7 @@
811811
},
812812
{
813813
"Name": "\"run_govulncheck\"",
814-
"Doc": "`\"run_govulncheck\"`: Run govulncheck\n\nThis codelens source annotates the `module` directive in a\ngo.mod file with a command to run Govulncheck.\n\n[Govulncheck](https://go.dev/blog/vuln) is a static\nanalysis tool that computes the set of functions reachable\nwithin your application, including dependencies;\nqueries a database of known security vulnerabilities; and\nreports any potential problems it finds.\n",
814+
"Doc": "`\"run_govulncheck\"`: Run govulncheck (legacy)\n\nThis codelens source annotates the `module` directive in a go.mod file\nwith a command to run Govulncheck asynchronously.\n\n[Govulncheck](https://go.dev/blog/vuln) is a static analysis tool that\ncomputes the set of functions reachable within your application, including\ndependencies; queries a database of known security vulnerabilities; and\nreports any potential problems it finds.\n",
815815
"Default": "false"
816816
},
817817
{
@@ -833,6 +833,11 @@
833833
"Name": "\"vendor\"",
834834
"Doc": "`\"vendor\"`: Update vendor directory\n\nThis codelens source annotates the `module` directive in a\ngo.mod file with a command to run [`go mod\nvendor`](https://go.dev/ref/mod#go-mod-vendor), which\ncreates or updates the directory named `vendor` in the\nmodule root so that it contains an up-to-date copy of all\nnecessary package dependencies.\n",
835835
"Default": "true"
836+
},
837+
{
838+
"Name": "\"vulncheck\"",
839+
"Doc": "`\"vulncheck\"`: Run govulncheck\n\nThis codelens source annotates the `module` directive in a go.mod file\nwith a command to run govulncheck synchronously.\n\n[Govulncheck](https://go.dev/blog/vuln) is a static analysis tool that\ncomputes the set of functions reachable within your application, including\ndependencies; queries a database of known security vulnerabilities; and\nreports any potential problems it finds.\n",
840+
"Default": "false"
836841
}
837842
]
838843
},
@@ -953,8 +958,8 @@
953958
{
954959
"FileType": "go.mod",
955960
"Lens": "run_govulncheck",
956-
"Title": "Run govulncheck",
957-
"Doc": "\nThis codelens source annotates the `module` directive in a\ngo.mod file with a command to run Govulncheck.\n\n[Govulncheck](https://go.dev/blog/vuln) is a static\nanalysis tool that computes the set of functions reachable\nwithin your application, including dependencies;\nqueries a database of known security vulnerabilities; and\nreports any potential problems it finds.\n",
961+
"Title": "Run govulncheck (legacy)",
962+
"Doc": "\nThis codelens source annotates the `module` directive in a go.mod file\nwith a command to run Govulncheck asynchronously.\n\n[Govulncheck](https://go.dev/blog/vuln) is a static analysis tool that\ncomputes the set of functions reachable within your application, including\ndependencies; queries a database of known security vulnerabilities; and\nreports any potential problems it finds.\n",
958963
"Default": false
959964
},
960965
{
@@ -977,6 +982,13 @@
977982
"Title": "Update vendor directory",
978983
"Doc": "\nThis codelens source annotates the `module` directive in a\ngo.mod file with a command to run [`go mod\nvendor`](https://go.dev/ref/mod#go-mod-vendor), which\ncreates or updates the directory named `vendor` in the\nmodule root so that it contains an up-to-date copy of all\nnecessary package dependencies.\n",
979984
"Default": true
985+
},
986+
{
987+
"FileType": "go.mod",
988+
"Lens": "vulncheck",
989+
"Title": "Run govulncheck",
990+
"Doc": "\nThis codelens source annotates the `module` directive in a go.mod file\nwith a command to run govulncheck synchronously.\n\n[Govulncheck](https://go.dev/blog/vuln) is a static analysis tool that\ncomputes the set of functions reachable within your application, including\ndependencies; queries a database of known security vulnerabilities; and\nreports any potential problems it finds.\n",
991+
"Default": false
980992
}
981993
],
982994
"Analyzers": [

0 commit comments

Comments
 (0)