text/template: avoid a global map to help the linker's deadcode elimination

Fixes #36021
Updates #2559
Updates #26775

Change-Id: I2e6708691311035b63866f25d5b4b3977a118290
Reviewed-on: https://go-review.googlesource.com/c/go/+/210284
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Rob Pike <r@golang.org>

diff --git a/src/text/template/funcs.go b/src/text/template/funcs.go
index 6a6843dfa08f99ee78879557b12d9699d3b78b95..fb56bc3fc6220a91c69d64bfca1422801161a394 100644 (file)
--- a/src/text/template/funcs.go
+++ b/src/text/template/funcs.go
@@ -12,6 +12,7 @@ import (
        "net/url"
        "reflect"
        "strings"
+       "sync"
        "unicode"
        "unicode/utf8"
 )
@@ -29,31 +30,49 @@ import (
 // type can return interface{} or reflect.Value.
 type FuncMap map[string]interface{}
 
-var builtins = FuncMap{
-       "and":      and,
-       "call":     call,
-       "html":     HTMLEscaper,
-       "index":    index,
-       "slice":    slice,
-       "js":       JSEscaper,
-       "len":      length,
-       "not":      not,
-       "or":       or,
-       "print":    fmt.Sprint,
-       "printf":   fmt.Sprintf,
-       "println":  fmt.Sprintln,
-       "urlquery": URLQueryEscaper,
-
-       // Comparisons
-       "eq": eq, // ==
-       "ge": ge, // >=
-       "gt": gt, // >
-       "le": le, // <=
-       "lt": lt, // <
-       "ne": ne, // !=
-}
-
-var builtinFuncs = createValueFuncs(builtins)
+// builtins returns the FuncMap.
+// It is not a global variable so the linker can dead code eliminate
+// more when this isn't called. See golang.org/issue/36021.
+// TODO: revert this back to a global map once golang.org/issue/2559 is fixed.
+func builtins() FuncMap {
+       return FuncMap{
+               "and":      and,
+               "call":     call,
+               "html":     HTMLEscaper,
+               "index":    index,
+               "slice":    slice,
+               "js":       JSEscaper,
+               "len":      length,
+               "not":      not,
+               "or":       or,
+               "print":    fmt.Sprint,
+               "printf":   fmt.Sprintf,
+               "println":  fmt.Sprintln,
+               "urlquery": URLQueryEscaper,
+
+               // Comparisons
+               "eq": eq, // ==
+               "ge": ge, // >=
+               "gt": gt, // >
+               "le": le, // <=
+               "lt": lt, // <
+               "ne": ne, // !=
+       }
+}
+
+var builtinFuncsOnce struct {
+       sync.Once
+       v map[string]reflect.Value
+}
+
+// builtinFuncsOnce lazily computes & caches the builtinFuncs map.
+// TODO: revert this back to a global map once golang.org/issue/2559 is fixed.
+func builtinFuncs() map[string]reflect.Value {
+       builtinFuncsOnce.Do(func() {
+               builtinFuncsOnce.v = createValueFuncs(builtins())
+       })
+       return builtinFuncsOnce.v
+}
 
 // createValueFuncs turns a FuncMap into a map[string]reflect.Value
 func createValueFuncs(funcMap FuncMap) map[string]reflect.Value {
@@ -125,7 +144,7 @@ func findFunction(name string, tmpl *Template) (reflect.Value, bool) {
                        return fn, true
                }
        }
-       if fn := builtinFuncs[name]; fn.IsValid() {
+       if fn := builtinFuncs()[name]; fn.IsValid() {
                return fn, true
        }
        return reflect.Value{}, false

diff --git a/src/text/template/link_test.go b/src/text/template/link_test.go
new file mode 100644 (file)
index 0000000..b7415d2
--- /dev/null
+++ b/src/text/template/link_test.go
@@ -0,0 +1,64 @@
+// Copyright 2019 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package template_test
+
+import (
+       "bytes"
+       "internal/testenv"
+       "io/ioutil"
+       "os"
+       "os/exec"
+       "path/filepath"
+       "testing"
+)
+
+// Issue 36021: verify that text/template doesn't prevent the linker from removing
+// unused methods.
+func TestLinkerGC(t *testing.T) {
+       if testing.Short() {
+               t.Skip("skipping in short mode")
+       }
+       testenv.MustHaveGoBuild(t)
+       const prog = `package main
+
+import (
+       _ "text/template"
+)
+
+type T struct{}
+
+func (t *T) Unused() { println("THIS SHOULD BE ELIMINATED") }
+func (t *T) Used() {}
+
+var sink *T
+
+func main() {
+       var t T
+       sink = &t
+       t.Used()
+}
+`
+       td, err := ioutil.TempDir("", "text_template_TestDeadCodeElimination")
+       if err != nil {
+               t.Fatal(err)
+       }
+       defer os.RemoveAll(td)
+
+       if err := ioutil.WriteFile(filepath.Join(td, "x.go"), []byte(prog), 0644); err != nil {
+               t.Fatal(err)
+       }
+       cmd := exec.Command("go", "build", "-o", "x.exe", "x.go")
+       cmd.Dir = td
+       if out, err := cmd.CombinedOutput(); err != nil {
+               t.Fatalf("go build: %v, %s", err, out)
+       }
+       slurp, err := ioutil.ReadFile(filepath.Join(td, "x.exe"))
+       if err != nil {
+               t.Fatal(err)
+       }
+       if bytes.Contains(slurp, []byte("THIS SHOULD BE ELIMINATED")) {
+               t.Error("binary contains code that should be deadcode eliminated")
+       }
+}

diff --git a/src/text/template/multi_test.go b/src/text/template/multi_test.go
index 5769470ff909e133b815da6fae2d00df83a46398..bf1f1b2701df2dff1fd60b9f77e964a687772381 100644 (file)
--- a/src/text/template/multi_test.go
+++ b/src/text/template/multi_test.go
@@ -242,7 +242,7 @@ func TestAddParseTree(t *testing.T) {
                t.Fatal(err)
        }
        // Add a new parse tree.
-       tree, err := parse.Parse("cloneText3", cloneText3, "", "", nil, builtins)
+       tree, err := parse.Parse("cloneText3", cloneText3, "", "", nil, builtins())
        if err != nil {
                t.Fatal(err)
        }

diff --git a/src/text/template/template.go b/src/text/template/template.go
index e0c096207c79cb287d84e1ca0a7629b90e5c5392..ec26eb4c50a7a4a17d2450b18cbdfebd31af01f1 100644 (file)
--- a/src/text/template/template.go
+++ b/src/text/template/template.go
@@ -198,7 +198,7 @@ func (t *Template) Lookup(name string) *Template {
 func (t *Template) Parse(text string) (*Template, error) {
        t.init()
        t.muFuncs.RLock()
-       trees, err := parse.Parse(t.name, text, t.leftDelim, t.rightDelim, t.parseFuncs, builtins)
+       trees, err := parse.Parse(t.name, text, t.leftDelim, t.rightDelim, t.parseFuncs, builtins())
        t.muFuncs.RUnlock()
        if err != nil {
                return nil, err