A few recent godebugs are missing IncNonDefault uses.
Test for that, so that people remember to do it.
Filed bugs for the missing ones.
For #66215.
For #66216.
For #66217.
Change-Id: Ia3fd10fd108e1b003bb30a8bc2f83995c768fab6
Reviewed-on: https://go-review.googlesource.com/c/go/+/570275
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Damien Neil <dneil@google.com>
func init() { initEnv() }
var (
- goCacheVerify = godebug.New("gocacheverify")
- goDebugHash = godebug.New("gocachehash")
- goCacheTest = godebug.New("gocachetest")
+ gocacheverify = godebug.New("gocacheverify")
+ gocachehash = godebug.New("gocachehash")
+ gocachetest = godebug.New("gocachetest")
)
func initEnv() {
- if goCacheVerify.Value() == "1" {
- goCacheVerify.IncNonDefault()
+ if gocacheverify.Value() == "1" {
+ gocacheverify.IncNonDefault()
verify = true
}
- if goDebugHash.Value() == "1" {
- goDebugHash.IncNonDefault()
+ if gocachehash.Value() == "1" {
+ gocachehash.IncNonDefault()
debugHash = true
}
- if goCacheTest.Value() == "1" {
- goCacheTest.IncNonDefault()
+ if gocachetest.Value() == "1" {
+ gocachetest.IncNonDefault()
DebugTest = true
}
}
return nil
}
-var usePoliciesField = godebug.New("x509usepolicies")
+var x509usepolicies = godebug.New("x509usepolicies")
func buildCertExtensions(template *Certificate, subjectIsEmpty bool, authorityKeyId []byte, subjectKeyId []byte) (ret []pkix.Extension, err error) {
ret = make([]pkix.Extension, 10 /* maximum number of elements. */)
n++
}
- usePolicies := usePoliciesField.Value() == "1"
+ usePolicies := x509usepolicies.Value() == "1"
if ((!usePolicies && len(template.PolicyIdentifiers) > 0) || (usePolicies && len(template.Policies) > 0)) &&
!oidInExtensions(oidExtensionCertificatePolicies, template.ExtraExtensions) {
ret[n], err = marshalCertificatePolicies(template.Policies, template.PolicyIdentifiers)
b := cryptobyte.NewBuilder(make([]byte, 0, 128))
b.AddASN1(cryptobyte_asn1.SEQUENCE, func(child *cryptobyte.Builder) {
- if usePoliciesField.Value() == "1" {
- usePoliciesField.IncNonDefault()
+ if x509usepolicies.Value() == "1" {
+ x509usepolicies.IncNonDefault()
for _, v := range policies {
child.AddASN1(cryptobyte_asn1.SEQUENCE, func(child *cryptobyte.Builder) {
child.AddASN1(cryptobyte_asn1.OBJECT_IDENTIFIER, func(child *cryptobyte.Builder) {
// }
//
// Each time a non-default setting causes a change in program behavior,
-// code should call [Setting.IncNonDefault] to increment a counter that can
-// be reported by [runtime/metrics.Read].
+// code must call [Setting.IncNonDefault] to increment a counter that can
+// be reported by [runtime/metrics.Read]. The call must only happen when
+// the program executes a non-default behavior, not just when the setting
+// is set to a non-default value. This is occasionally (but very rarely)
+// infeasible, in which case the internal/godebugs table entry must set
+// Opaque: true, and the documentation in doc/godebug.md should
+// mention that metrics are unavailable.
+//
+// Conventionally, the global variable representing a godebug is named
+// for the godebug itself, with no case changes:
+//
+// var gotypesalias = godebug.New("gotypesalias") // this
+// var goTypesAlias = godebug.New("gotypesalias") // NOT THIS
+//
+// The test in internal/godebugs that checks for use of IncNonDefault
+// requires the use of this convention.
+//
// Note that counters used with IncNonDefault must be added to
// various tables in other packages. See the [Setting.IncNonDefault]
// documentation for details.
// To disable that panic for access to an undocumented setting,
// prefix the name with a #, as in godebug.New("#gofsystrace").
// The # is a signal to New but not part of the key used in $GODEBUG.
+//
+// Note that almost all settings should arrange to call [IncNonDefault] precisely
+// when program behavior is changing from the default due to the setting
+// (not just when the setting is different, but when program behavior changes).
+// See the [internal/godebug] package comment for more.
func New(name string) *Setting {
return &Setting{name: name}
}
"internal/godebugs"
"internal/testenv"
"os"
+ "os/exec"
+ "path/filepath"
+ "regexp"
"runtime"
"strings"
"testing"
)
func TestAll(t *testing.T) {
+ testenv.MustHaveGoBuild(t)
+
data, err := os.ReadFile("../../../doc/godebug.md")
if err != nil {
if os.IsNotExist(err) && (testenv.Builder() == "" || runtime.GOOS != "linux") {
}
doc := string(data)
+ incs := incNonDefaults(t)
+
last := ""
for _, info := range godebugs.All {
if info.Name <= last {
if !strings.Contains(doc, "`"+info.Name+"`") {
t.Errorf("Name=%s not documented in doc/godebug.md", info.Name)
}
+ if !info.Opaque && !incs[info.Name] {
+ t.Errorf("Name=%s missing IncNonDefault calls; see 'go doc internal/godebug'", info.Name)
+ }
+ }
+}
+
+var incNonDefaultRE = regexp.MustCompile(`([\pL\p{Nd}_]+)\.IncNonDefault\(\)`)
+
+func incNonDefaults(t *testing.T) map[string]bool {
+ // Build list of all files importing internal/godebug.
+ // Tried a more sophisticated search in go list looking for
+ // imports containing "internal/godebug", but that turned
+ // up a bug in go list instead. #66218
+ out, err := exec.Command("go", "list", "-f={{.Dir}}", "std", "cmd").CombinedOutput()
+ if err != nil {
+ t.Fatalf("go list: %v\n%s", err, out)
+ }
+
+ seen := map[string]bool{}
+ for _, dir := range strings.Split(string(out), "\n") {
+ if dir == "" {
+ continue
+ }
+ files, err := os.ReadDir(dir)
+ if err != nil {
+ t.Fatal(err)
+ }
+ for _, file := range files {
+ name := file.Name()
+ if !strings.HasSuffix(name, ".go") || strings.HasSuffix(name, "_test.go") {
+ continue
+ }
+ data, err := os.ReadFile(filepath.Join(dir, name))
+ if err != nil {
+ t.Fatal(err)
+ }
+ for _, m := range incNonDefaultRE.FindAllSubmatch(data, -1) {
+ seen[string(m[1])] = true
+ }
+ }
}
+ return seen
}
{Name: "gocachehash", Package: "cmd/go"},
{Name: "gocachetest", Package: "cmd/go"},
{Name: "gocacheverify", Package: "cmd/go"},
- {Name: "gotypesalias", Package: "go/types"},
+ {Name: "gotypesalias", Package: "go/types", Opaque: true}, // bug #66216: remove Opaque
{Name: "http2client", Package: "net/http"},
{Name: "http2debug", Package: "net/http", Opaque: true},
{Name: "http2server", Package: "net/http"},
{Name: "httplaxcontentlength", Package: "net/http", Changed: 22, Old: "1"},
{Name: "httpmuxgo121", Package: "net/http", Changed: 22, Old: "1"},
{Name: "installgoroot", Package: "go/build"},
- {Name: "jstmpllitinterp", Package: "html/template"},
+ {Name: "jstmpllitinterp", Package: "html/template", Opaque: true}, // bug #66217: remove Opaque
//{Name: "multipartfiles", Package: "mime/multipart"},
{Name: "multipartmaxheaders", Package: "mime/multipart"},
{Name: "multipartmaxparts", Package: "mime/multipart"},
{Name: "tlsmaxrsasize", Package: "crypto/tls"},
{Name: "tlsrsakex", Package: "crypto/tls", Changed: 22, Old: "1"},
{Name: "tlsunsafeekm", Package: "crypto/tls", Changed: 22, Old: "1"},
- {Name: "winreadlinkvolume", Package: "os", Changed: 22, Old: "0"},
- {Name: "winsymlink", Package: "os", Changed: 22, Old: "0"},
+ {Name: "winreadlinkvolume", Package: "os", Changed: 22, Old: "0", Opaque: true}, // bug #66215: remove Opaque
+ {Name: "winsymlink", Package: "os", Changed: 22, Old: "0", Opaque: true}, // bug #66215: remove Opaque
{Name: "x509sha1", Package: "crypto/x509"},
{Name: "x509usefallbackroots", Package: "crypto/x509"},
{Name: "x509usepolicies", Package: "crypto/x509"},
}
var (
- multipartFiles = godebug.New("#multipartfiles") // TODO: document and remove #
- multipartMaxParts = godebug.New("multipartmaxparts")
+ multipartfiles = godebug.New("#multipartfiles") // TODO: document and remove #
+ multipartmaxparts = godebug.New("multipartmaxparts")
)
func (r *Reader) readForm(maxMemory int64) (_ *Form, err error) {
)
numDiskFiles := 0
combineFiles := true
- if multipartFiles.Value() == "distinct" {
+ if multipartfiles.Value() == "distinct" {
combineFiles = false
- // multipartFiles.IncNonDefault() // TODO: uncomment after documenting
+ // multipartfiles.IncNonDefault() // TODO: uncomment after documenting
}
maxParts := 1000
- if s := multipartMaxParts.Value(); s != "" {
+ if s := multipartmaxparts.Value(); s != "" {
if v, err := strconv.Atoi(s); err == nil && v >= 0 {
maxParts = v
- multipartMaxParts.IncNonDefault()
+ multipartmaxparts.IncNonDefault()
}
}
maxHeaders := maxMIMEHeaders()
// including header keys, values, and map overhead.
const maxMIMEHeaderSize = 10 << 20
-// multipartMaxHeaders is the maximum number of header entries NextPart will return,
+// multipartmaxheaders is the maximum number of header entries NextPart will return,
// as well as the maximum combined total of header entries Reader.ReadForm will return
// in FileHeaders.
-var multipartMaxHeaders = godebug.New("multipartmaxheaders")
+var multipartmaxheaders = godebug.New("multipartmaxheaders")
func maxMIMEHeaders() int64 {
- if s := multipartMaxHeaders.Value(); s != "" {
+ if s := multipartmaxheaders.Value(); s != "" {
if v, err := strconv.ParseInt(s, 10, 64); err == nil && v >= 0 {
- multipartMaxHeaders.IncNonDefault()
+ multipartmaxheaders.IncNonDefault()
return v
}
}
return bl.b.readLocked(p)
}
-var laxContentLength = godebug.New("httplaxcontentlength")
+var httplaxcontentlength = godebug.New("httplaxcontentlength")
// parseContentLength checks that the header is valid and then trims
// whitespace. It returns -1 if no value is set otherwise the value
// The Content-Length must be a valid numeric value.
// See: https://datatracker.ietf.org/doc/html/rfc2616/#section-14.13
if cl == "" {
- if laxContentLength.Value() == "1" {
- laxContentLength.IncNonDefault()
+ if httplaxcontentlength.Value() == "1" {
+ httplaxcontentlength.IncNonDefault()
return -1, nil
}
return 0, badStringError("invalid empty Content-Length", cl)
The number of non-default behaviors executed by the cmd/go
package due to a non-default GODEBUG=gocacheverify=... setting.
- /godebug/non-default-behavior/gotypesalias:events
- The number of non-default behaviors executed by the go/types
- package due to a non-default GODEBUG=gotypesalias=... setting.
-
/godebug/non-default-behavior/http2client:events
The number of non-default behaviors executed by the net/http
package due to a non-default GODEBUG=http2client=... setting.
The number of non-default behaviors executed by the go/build
package due to a non-default GODEBUG=installgoroot=... setting.
- /godebug/non-default-behavior/jstmpllitinterp:events
- The number of non-default behaviors executed by
- the html/template package due to a non-default
- GODEBUG=jstmpllitinterp=... setting.
-
/godebug/non-default-behavior/multipartmaxheaders:events
The number of non-default behaviors executed by
the mime/multipart package due to a non-default
The number of non-default behaviors executed by the crypto/tls
package due to a non-default GODEBUG=tlsunsafeekm=... setting.
- /godebug/non-default-behavior/winreadlinkvolume:events
- The number of non-default behaviors executed by the os package
- due to a non-default GODEBUG=winreadlinkvolume=... setting.
-
- /godebug/non-default-behavior/winsymlink:events
- The number of non-default behaviors executed by the os package
- due to a non-default GODEBUG=winsymlink=... setting.
-
/godebug/non-default-behavior/x509sha1:events
The number of non-default behaviors executed by the crypto/x509
package due to a non-default GODEBUG=x509sha1=... setting.