]> Cypherpunks.ru repositories - gostls13.git/commitdiff
cmd/compile/internal/inline: fix buglet in panic path scoring
authorThan McIntosh <thanm@google.com>
Tue, 14 Nov 2023 15:34:59 +0000 (10:34 -0500)
committerThan McIntosh <thanm@google.com>
Wed, 15 Nov 2023 14:23:20 +0000 (14:23 +0000)
Fix a bug in scoring of calls appearing on panic paths. For this code
snippet:

  if x < 101 {
     foo()
     panic("bad")
  }

the function flags analyzer was correctly capturing the status of the
block corresponding to the true arm of the "if" statement, but wasn't
marking "foo()" as being on a panic path.

Change-Id: Iee13782828a1399028e2b560fed5f946850eb253
Reviewed-on: https://go-review.googlesource.com/c/go/+/542216
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>

src/cmd/compile/internal/inline/inlheur/analyze_func_flags.go
src/cmd/compile/internal/inline/inlheur/testdata/props/calls.go

index 8211c452d554d2900d613a4ebb3da9062d30b60d..588d2f4f598da746371bced9e863b11f1fb503e9 100644 (file)
@@ -148,14 +148,28 @@ func branchCombine(p1, p2 pstate) pstate {
 // as updating disposition of intermediate nodes.
 func (ffa *funcFlagsAnalyzer) stateForList(list ir.Nodes) pstate {
        st := psTop
-       for i := range list {
+       // Walk the list backwards so that we can update the state for
+       // earlier list elements based on what we find out about their
+       // successors. Example:
+       //
+       //        if ... {
+       //  L10:    foo()
+       //  L11:    <stmt>
+       //  L12:    panic(...)
+       //        }
+       //
+       // After combining the dispositions for line 11 and 12, we want to
+       // update the state for the call at line 10 based on that combined
+       // disposition (if L11 has no path to "return", then the call at
+       // line 10 will be on a panic path).
+       for i := len(list) - 1; i >= 0; i-- {
                n := list[i]
                psi := ffa.getstate(n)
                if debugTrace&debugTraceFuncFlags != 0 {
                        fmt.Fprintf(os.Stderr, "=-= %v: stateForList n=%s ps=%s\n",
                                ir.Line(n), n.Op().String(), psi.String())
                }
-               st = blockCombine(st, psi)
+               st = blockCombine(psi, st)
                ffa.updatestate(n, st)
        }
        if st == psTop {
index 1d35a1ad47ea195441689317cdbcb29c9953dce0..f9cc023da3120758276729be503a9e7b6b185ab6 100644 (file)
@@ -59,8 +59,8 @@ func T_calls_in_pseudo_loop(x int, q []string) {
 // calls.go T_calls_on_panic_paths 67 0 1
 // <endpropsdump>
 // {"Flags":0,"ParamFlags":[0,0],"ResultFlags":[]}
-// callsite: calls.go:69:9|0 flagstr "" flagval 0 score 2 mask 0 maskstr ""
-// callsite: calls.go:73:9|1 flagstr "" flagval 0 score 2 mask 0 maskstr ""
+// callsite: calls.go:69:9|0 flagstr "CallSiteOnPanicPath" flagval 2 score 42 mask 1 maskstr "panicPathAdj"
+// callsite: calls.go:73:9|1 flagstr "CallSiteOnPanicPath" flagval 2 score 42 mask 1 maskstr "panicPathAdj"
 // callsite: calls.go:77:12|2 flagstr "CallSiteOnPanicPath" flagval 2 score 102 mask 1 maskstr "panicPathAdj"
 // <endcallsites>
 // <endfuncpreamble>
@@ -87,7 +87,7 @@ func T_calls_on_panic_paths(x int, q []string) {
 // callsite: calls.go:103:9|0 flagstr "" flagval 0 score 2 mask 0 maskstr ""
 // callsite: calls.go:112:9|1 flagstr "" flagval 0 score 2 mask 0 maskstr ""
 // callsite: calls.go:115:9|2 flagstr "" flagval 0 score 2 mask 0 maskstr ""
-// callsite: calls.go:119:12|3 flagstr "" flagval 0 score 62 mask 0 maskstr ""
+// callsite: calls.go:119:12|3 flagstr "CallSiteOnPanicPath" flagval 2 score 102 mask 1 maskstr "panicPathAdj"
 // <endcallsites>
 // <endfuncpreamble>
 func T_calls_not_on_panic_paths(x int, q []string) {