]> Cypherpunks repositories - gostls13.git/commitdiff
[dev.regabi] go/types: refuse excessively long constants
authorRob Findley <rfindley@google.com>
Wed, 10 Feb 2021 17:04:31 +0000 (12:04 -0500)
committerRobert Findley <rfindley@google.com>
Sat, 13 Feb 2021 00:39:48 +0000 (00:39 +0000)
This is a port of CL 289049 to go/types. In that CL, tests were written
using the ability of tests/run.go to generate test packages dynamically.
For this CL, similar functionality is added to the go/types errmap
tests: tests are refactored to decouple the loading of source code from
the filesystem, so that tests for long constants may be generated
dynamically rather than checked-in as a large testdata file.

Change-Id: I92c7cb61a8d42c6593570ef7ae0af86b501fa34e
Reviewed-on: https://go-review.googlesource.com/c/go/+/290949
Trust: Robert Findley <rfindley@google.com>
Trust: Robert Griesemer <gri@golang.org>
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
src/go/types/check_test.go
src/go/types/expr.go

index 47d749b3a331bd1e55dd472ab33d86aa40ddf980..7292f7bcb2c3fdd5dbaf597524090018dadd5b2f 100644 (file)
@@ -68,11 +68,11 @@ func splitError(err error) (pos, msg string) {
        return
 }
 
-func parseFiles(t *testing.T, filenames []string) ([]*ast.File, []error) {
+func parseFiles(t *testing.T, filenames []string, srcs [][]byte) ([]*ast.File, []error) {
        var files []*ast.File
        var errlist []error
-       for _, filename := range filenames {
-               file, err := parser.ParseFile(fset, filename, nil, parser.AllErrors)
+       for i, filename := range filenames {
+               file, err := parser.ParseFile(fset, filename, srcs[i], parser.AllErrors)
                if file == nil {
                        t.Fatalf("%s: %s", filename, err)
                }
@@ -101,19 +101,17 @@ var errRx = regexp.MustCompile(`^ *ERROR *(HERE)? *"?([^"]*)"?`)
 // errMap collects the regular expressions of ERROR comments found
 // in files and returns them as a map of error positions to error messages.
 //
-func errMap(t *testing.T, testname string, files []*ast.File) map[string][]string {
+// srcs must be a slice of the same length as files, containing the original
+// source for the parsed AST.
+func errMap(t *testing.T, files []*ast.File, srcs [][]byte) map[string][]string {
        // map of position strings to lists of error message patterns
        errmap := make(map[string][]string)
 
-       for _, file := range files {
-               filename := fset.Position(file.Package).Filename
-               src, err := os.ReadFile(filename)
-               if err != nil {
-                       t.Fatalf("%s: could not read %s", testname, filename)
-               }
-
+       for i, file := range files {
+               tok := fset.File(file.Package)
+               src := srcs[i]
                var s scanner.Scanner
-               s.Init(fset.AddFile(filename, -1, len(src)), src, nil, scanner.ScanComments)
+               s.Init(tok, src, nil, scanner.ScanComments)
                var prev token.Pos // position of last non-comment, non-semicolon token
                var here token.Pos // position immediately after the token at position prev
 
@@ -190,13 +188,13 @@ func eliminate(t *testing.T, errmap map[string][]string, errlist []error) {
        }
 }
 
-func checkFiles(t *testing.T, sources []string) {
-       if len(sources) == 0 {
+func checkFiles(t *testing.T, filenames []string, srcs [][]byte) {
+       if len(filenames) == 0 {
                t.Fatal("no source files")
        }
 
        // parse files and collect parser errors
-       files, errlist := parseFiles(t, sources)
+       files, errlist := parseFiles(t, filenames, srcs)
 
        pkgName := "<no package>"
        if len(files) > 0 {
@@ -214,11 +212,12 @@ func checkFiles(t *testing.T, sources []string) {
        var conf Config
 
        // special case for importC.src
-       if len(sources) == 1 && strings.HasSuffix(sources[0], "importC.src") {
-               conf.FakeImportC = true
+       if len(filenames) == 1 {
+               if strings.HasSuffix(filenames[0], "importC.src") {
+                       conf.FakeImportC = true
+               }
        }
-       // TODO(rFindley) we may need to use the source importer when adding generics
-       // tests.
+
        conf.Importer = importer.Default()
        conf.Error = func(err error) {
                if *haltOnError {
@@ -253,7 +252,7 @@ func checkFiles(t *testing.T, sources []string) {
 
        // match and eliminate errors;
        // we are expecting the following errors
-       errmap := errMap(t, pkgName, files)
+       errmap := errMap(t, files, srcs)
        eliminate(t, errmap, errlist)
 
        // there should be no expected errors left
@@ -274,7 +273,13 @@ func TestCheck(t *testing.T) {
        }
        testenv.MustHaveGoBuild(t)
        DefPredeclaredTestFuncs()
-       checkFiles(t, strings.Split(*testFiles, " "))
+       testPkg(t, strings.Split(*testFiles, " "))
+}
+
+func TestLongConstants(t *testing.T) {
+       format := "package longconst\n\nconst _ = %s\nconst _ = %s // ERROR excessively long constant"
+       src := fmt.Sprintf(format, strings.Repeat("1", 9999), strings.Repeat("1", 10001))
+       checkFiles(t, []string{"longconst.go"}, [][]byte{[]byte(src)})
 }
 
 func TestTestdata(t *testing.T)  { DefPredeclaredTestFuncs(); testDir(t, "testdata") }
@@ -293,26 +298,33 @@ func testDir(t *testing.T, dir string) {
                path := filepath.Join(dir, fi.Name())
 
                // if fi is a directory, its files make up a single package
-               var files []string
+               var filenames []string
                if fi.IsDir() {
                        fis, err := ioutil.ReadDir(path)
                        if err != nil {
                                t.Error(err)
                                continue
                        }
-                       files = make([]string, len(fis))
-                       for i, fi := range fis {
-                               // if fi is a directory, checkFiles below will complain
-                               files[i] = filepath.Join(path, fi.Name())
-                               if testing.Verbose() {
-                                       fmt.Printf("\t%s\n", files[i])
-                               }
+                       for _, fi := range fis {
+                               filenames = append(filenames, filepath.Join(path, fi.Name()))
                        }
                } else {
-                       files = []string{path}
+                       filenames = []string{path}
                }
                t.Run(filepath.Base(path), func(t *testing.T) {
-                       checkFiles(t, files)
+                       testPkg(t, filenames)
                })
        }
 }
+
+func testPkg(t *testing.T, filenames []string) {
+       srcs := make([][]byte, len(filenames))
+       for i, filename := range filenames {
+               src, err := os.ReadFile(filename)
+               if err != nil {
+                       t.Fatalf("could not read %s: %v", filename, err)
+               }
+               srcs[i] = src
+       }
+       checkFiles(t, filenames, srcs)
+}
index 5e1fe28a43aae9fc442dfe4cfc0b63963a1eae95..1a3c486af7ee857f31210cc33b36fe38e0f61f33 100644 (file)
@@ -1140,6 +1140,23 @@ func (check *Checker) exprInternal(x *operand, e ast.Expr, hint Type) exprKind {
                goto Error
 
        case *ast.BasicLit:
+               switch e.Kind {
+               case token.INT, token.FLOAT, token.IMAG:
+                       // The max. mantissa precision for untyped numeric values
+                       // is 512 bits, or 4048 bits for each of the two integer
+                       // parts of a fraction for floating-point numbers that are
+                       // represented accurately in the go/constant package.
+                       // Constant literals that are longer than this many bits
+                       // are not meaningful; and excessively long constants may
+                       // consume a lot of space and time for a useless conversion.
+                       // Cap constant length with a generous upper limit that also
+                       // allows for separators between all digits.
+                       const limit = 10000
+                       if len(e.Value) > limit {
+                               check.errorf(e, _InvalidConstVal, "excessively long constant: %s... (%d chars)", e.Value[:10], len(e.Value))
+                               goto Error
+                       }
+               }
                x.setConst(e.Kind, e.Value)
                if x.mode == invalid {
                        // The parser already establishes syntactic correctness.