From: Robert Griesemer Date: Fri, 26 Aug 2016 00:17:50 +0000 (-0700) Subject: go/internal/gcimporter: fail gracefully on export format skew X-Git-Tag: go1.8beta1~1622 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=fba8f4deba81b8c5d903ec2f52dcb151f13a147b;p=gostls13.git go/internal/gcimporter: fail gracefully on export format skew Port of changes made to compiler in https://go-review.googlesource.com/27814. Correctly handle export format version 0 (we only do this in x/tools/gcimporter15 at the moment - this is a backport of that code for struct fields). Added tests for version handling and detection of corrupted export data. Fixes #16881. Change-Id: I246553c689c89ef5c7fedd1e43717504c2838804 Reviewed-on: https://go-review.googlesource.com/27816 Reviewed-by: Matthew Dempsky --- diff --git a/src/go/internal/gcimporter/bimport.go b/src/go/internal/gcimporter/bimport.go index b657cc79ba..f155b8fe75 100644 --- a/src/go/internal/gcimporter/bimport.go +++ b/src/go/internal/gcimporter/bimport.go @@ -11,6 +11,7 @@ import ( "go/token" "go/types" "sort" + "strconv" "strings" "unicode" "unicode/utf8" @@ -21,6 +22,7 @@ type importer struct { data []byte path string buf []byte // for reading strings + version int // export format version // object lists strList []string // in order of appearance @@ -40,17 +42,28 @@ type importer struct { // BImportData imports a package from the serialized package data // and returns the number of bytes consumed and a reference to the package. -// If data is obviously malformed, an error is returned but in -// general it is not recommended to call BImportData on untrusted data. -func BImportData(imports map[string]*types.Package, data []byte, path string) (int, *types.Package, error) { +// If the export data version is not recognized or the format is otherwise +// compromised, an error is returned. +func BImportData(imports map[string]*types.Package, data []byte, path string) (_ int, _ *types.Package, err error) { + // catch panics and return them as errors + defer func() { + if e := recover(); e != nil { + // The package (filename) causing the problem is added to this + // error by a wrapper in the caller (Import in gcimporter.go). + err = fmt.Errorf("cannot import, possibly version skew (%v) - reinstall package", e) + } + }() + p := importer{ imports: imports, data: data, path: path, + version: -1, // unknown version strList: []string{""}, // empty string is mapped to 0 } // read version info + var versionstr string if b := p.rawByte(); b == 'c' || b == 'd' { // Go1.7 encoding; first byte encodes low-level // encoding format (compact vs debug). @@ -63,19 +76,34 @@ func BImportData(imports map[string]*types.Package, data []byte, path string) (i } p.trackAllTypes = p.rawByte() == 'a' p.posInfoFormat = p.int() != 0 - const go17version = "v1" - if s := p.string(); s != go17version { - return p.read, nil, fmt.Errorf("importer: unknown export data format: %s (imported package compiled with old compiler?)", s) + versionstr = p.string() + if versionstr == "v1" { + p.version = 0 } } else { // Go1.8 extensible encoding - const exportVersion = "version 1" - if s := p.rawStringln(b); s != exportVersion { - return p.read, nil, fmt.Errorf("importer: unknown export data format: %s (imported package compiled with old compiler?)", s) + // read version string and extract version number (ignore anything after the version number) + versionstr = p.rawStringln(b) + if s := strings.SplitN(versionstr, " ", 3); len(s) >= 2 && s[0] == "version" { + if v, err := strconv.Atoi(s[1]); err == nil && v > 0 { + p.version = v + } } + } + + // read version specific flags - extend as necessary + switch p.version { + // case 2: + // ... + // fallthrough + case 1: p.debugFormat = p.rawStringln(p.rawByte()) == "debug" p.trackAllTypes = p.int() != 0 p.posInfoFormat = p.int() != 0 + case 0: + // Go1.7 encoding format - nothing to do here + default: + errorf("unknown export format version %d (%q)", p.version, versionstr) } // --- generic export data --- @@ -99,7 +127,7 @@ func BImportData(imports map[string]*types.Package, data []byte, path string) (i // self-verification if count := p.int(); count != objcount { - panic(fmt.Sprintf("got %d objects; want %d", objcount, count)) + errorf("got %d objects; want %d", objcount, count) } // ignore compiler-specific import data @@ -127,6 +155,10 @@ func BImportData(imports map[string]*types.Package, data []byte, path string) (i return p.read, pkg, nil } +func errorf(format string, args ...interface{}) { + panic(fmt.Sprintf(format, args...)) +} + func (p *importer) pkg() *types.Package { // if the package was seen before, i is its index (>= 0) i := p.tagOrIndex() @@ -136,7 +168,7 @@ func (p *importer) pkg() *types.Package { // otherwise, i is the package tag (< 0) if i != packageTag { - panic(fmt.Sprintf("unexpected package tag %d", i)) + errorf("unexpected package tag %d", i) } // read package data @@ -145,13 +177,13 @@ func (p *importer) pkg() *types.Package { // we should never see an empty package name if name == "" { - panic("empty package name in import") + errorf("empty package name in import") } // an empty path denotes the package we are currently importing; // it must be the first package we see if (path == "") != (len(p.pkgList) == 0) { - panic(fmt.Sprintf("package path %q for pkg index %d", path, len(p.pkgList))) + errorf("package path %q for pkg index %d", path, len(p.pkgList)) } // if the package was imported before, use that one; otherwise create a new one @@ -163,7 +195,7 @@ func (p *importer) pkg() *types.Package { pkg = types.NewPackage(path, name) p.imports[path] = pkg } else if pkg.Name() != name { - panic(fmt.Sprintf("conflicting names %s and %s for package %q", pkg.Name(), name, path)) + errorf("conflicting names %s and %s for package %q", pkg.Name(), name, path) } p.pkgList = append(p.pkgList, pkg) @@ -180,7 +212,7 @@ func (p *importer) declare(obj types.Object) { // imported. // (See also the comment in cmd/compile/internal/gc/bimport.go importer.obj, // switch case importing functions). - panic(fmt.Sprintf("inconsistent import:\n\t%v\npreviously imported as:\n\t%v\n", alt, obj)) + errorf("inconsistent import:\n\t%v\npreviously imported as:\n\t%v\n", alt, obj) } } @@ -211,7 +243,7 @@ func (p *importer) obj(tag int) { p.declare(types.NewFunc(token.NoPos, pkg, name, sig)) default: - panic(fmt.Sprintf("unexpected object tag %d", tag)) + errorf("unexpected object tag %d", tag) } } @@ -283,7 +315,7 @@ func (p *importer) typ(parent *types.Package) types.Type { } if _, ok := obj.(*types.TypeName); !ok { - panic(fmt.Sprintf("pkg = %s, name = %s => %s", parent, name, obj)) + errorf("pkg = %s, name = %s => %s", parent, name, obj) } // associate new named type with obj if it doesn't exist yet @@ -390,7 +422,7 @@ func (p *importer) typ(parent *types.Package) types.Type { // no embedded interfaces with gc compiler if p.int() != 0 { - panic("unexpected embedded interface") + errorf("unexpected embedded interface") } t := types.NewInterface(p.methodList(parent), nil) @@ -426,14 +458,15 @@ func (p *importer) typ(parent *types.Package) types.Type { case 3 /* Cboth */ : dir = types.SendRecv default: - panic(fmt.Sprintf("unexpected channel dir %d", d)) + errorf("unexpected channel dir %d", d) } val := p.typ(parent) *t = *types.NewChan(dir, val) return t default: - panic(fmt.Sprintf("unexpected type tag %d", i)) + errorf("unexpected type tag %d", i) + panic("unreachable") } } @@ -464,7 +497,7 @@ func (p *importer) field(parent *types.Package) *types.Var { case *types.Named: name = typ.Obj().Name() default: - panic("anonymous field expected") + errorf("anonymous field expected") } anonymous = true } @@ -498,6 +531,10 @@ func (p *importer) fieldName(parent *types.Package) (*types.Package, string) { // use the imported package instead pkg = p.pkgList[0] } + if p.version == 0 && name == "_" { + // version 0 didn't export a package for _ fields + return pkg, name + } if name != "" && !exported(name) { if name == "?" { name = "" @@ -539,7 +576,7 @@ func (p *importer) param(named bool) (*types.Var, bool) { if named { name = p.string() if name == "" { - panic("expected named parameter") + errorf("expected named parameter") } if name != "_" { pkg = p.pkg() @@ -577,7 +614,8 @@ func (p *importer) value() constant.Value { case stringTag: return constant.MakeString(p.string()) default: - panic(fmt.Sprintf("unexpected value tag %d", tag)) + errorf("unexpected value tag %d", tag) + panic("unreachable") } } @@ -640,7 +678,7 @@ func (p *importer) tagOrIndex() int { func (p *importer) int() int { x := p.int64() if int64(int(x)) != x { - panic("exported integer too large") + errorf("exported integer too large") } return int(x) } @@ -679,12 +717,12 @@ func (p *importer) string() string { func (p *importer) marker(want byte) { if got := p.rawByte(); got != want { - panic(fmt.Sprintf("incorrect marker: got %c; want %c (pos = %d)", got, want, p.read)) + errorf("incorrect marker: got %c; want %c (pos = %d)", got, want, p.read) } pos := p.read if n := int(p.rawInt64()); n != pos { - panic(fmt.Sprintf("incorrect position: got %d; want %d", n, pos)) + errorf("incorrect position: got %d; want %d", n, pos) } } @@ -692,7 +730,7 @@ func (p *importer) marker(want byte) { func (p *importer) rawInt64() int64 { i, err := binary.ReadVarint(p) if err != nil { - panic(fmt.Sprintf("read error: %v", err)) + errorf("read error: %v", err) } return i } @@ -727,7 +765,7 @@ func (p *importer) rawByte() byte { case '|': // nothing to do default: - panic("unexpected escape sequence in export data") + errorf("unexpected escape sequence in export data") } } p.data = p.data[r:] diff --git a/src/go/internal/gcimporter/gcimporter.go b/src/go/internal/gcimporter/gcimporter.go index 23520e6e63..1e1102a451 100644 --- a/src/go/internal/gcimporter/gcimporter.go +++ b/src/go/internal/gcimporter/gcimporter.go @@ -106,7 +106,7 @@ func Import(packages map[string]*types.Package, path, srcDir string) (pkg *types f.Close() if err != nil { // add file name to error - err = fmt.Errorf("reading export data: %s: %v", filename, err) + err = fmt.Errorf("%s: %v", filename, err) } }() @@ -118,7 +118,7 @@ func Import(packages map[string]*types.Package, path, srcDir string) (pkg *types switch hdr { case "$$\n": - err = fmt.Errorf("cannot import %s: old export format no longer supported (recompile library)", path) + err = fmt.Errorf("import %q: old export format no longer supported (recompile library)", path) case "$$B\n": var data []byte data, err = ioutil.ReadAll(buf) diff --git a/src/go/internal/gcimporter/gcimporter_test.go b/src/go/internal/gcimporter/gcimporter_test.go index 8301937e6f..03658f5208 100644 --- a/src/go/internal/gcimporter/gcimporter_test.go +++ b/src/go/internal/gcimporter/gcimporter_test.go @@ -5,6 +5,7 @@ package gcimporter import ( + "bytes" "fmt" "internal/testenv" "io/ioutil" @@ -118,6 +119,70 @@ func TestImportTestdata(t *testing.T) { } } +func TestVersionHandling(t *testing.T) { + skipSpecialPlatforms(t) // we really only need to exclude nacl platforms, but this is fine + + // This package only handles gc export data. + if runtime.Compiler != "gc" { + t.Skipf("gc-built packages not available (compiler = %s)", runtime.Compiler) + return + } + + const dir = "./testdata/versions" + list, err := ioutil.ReadDir(dir) + if err != nil { + t.Fatal(err) + } + + for _, f := range list { + name := f.Name() + if !strings.HasSuffix(name, ".a") { + continue // not a package file + } + if strings.Contains(name, "corrupted") { + continue // don't process a leftover corrupted file + } + pkgpath := "./" + name[:len(name)-2] + + // test that export data can be imported + _, err := Import(make(map[string]*types.Package), pkgpath, dir) + if err != nil { + t.Errorf("import %q failed: %v", pkgpath, err) + continue + } + + // create file with corrupted export data + // 1) read file + data, err := ioutil.ReadFile(filepath.Join(dir, name)) + if err != nil { + t.Fatal(err) + } + // 2) find export data + i := bytes.Index(data, []byte("\n$$B\n")) + 5 + j := bytes.Index(data[i:], []byte("\n$$\n")) + i + if i < 0 || j < 0 || i > j { + t.Fatalf("export data section not found (i = %d, j = %d)", i, j) + } + // 3) corrupt the data (increment every 7th byte) + for k := j - 13; k >= i; k -= 7 { + data[k]++ + } + // 4) write the file + pkgpath += "_corrupted" + filename := filepath.Join(dir, pkgpath) + ".a" + ioutil.WriteFile(filename, data, 0666) + defer os.Remove(filename) + + // test that importing the corrupted file results in an error + _, err = Import(make(map[string]*types.Package), pkgpath, dir) + if err == nil { + t.Errorf("import corrupted %q succeeded", pkgpath) + } else if msg := err.Error(); !strings.Contains(msg, "version skew") { + t.Errorf("import %q error incorrect (%s)", pkgpath, msg) + } + } +} + func TestImportStdLib(t *testing.T) { skipSpecialPlatforms(t) diff --git a/src/go/internal/gcimporter/testdata/versions/test.go b/src/go/internal/gcimporter/testdata/versions/test.go new file mode 100644 index 0000000000..ac9c968c2d --- /dev/null +++ b/src/go/internal/gcimporter/testdata/versions/test.go @@ -0,0 +1,25 @@ +// Copyright 2016 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. + +// To create a test case for a new export format version, +// build this package with the latest compiler and store +// the resulting .a file appropriately named in the versions +// directory. The VersionHandling test will pick it up. +// +// In the testdata/versions: +// +// go build -o test_go1.$X_$Y.a test.go +// +// with $X = Go version and $Y = export format version. +// +// Make sure this source is extended such that it exercises +// whatever export format change has taken place. + +package test + +// Any release before and including Go 1.7 didn't encode +// the package for a blank struct field. +type BlankField struct { + _ int +} diff --git a/src/go/internal/gcimporter/testdata/versions/test_go1.7_0.a b/src/go/internal/gcimporter/testdata/versions/test_go1.7_0.a new file mode 100644 index 0000000000..edb6c3f25a Binary files /dev/null and b/src/go/internal/gcimporter/testdata/versions/test_go1.7_0.a differ diff --git a/src/go/internal/gcimporter/testdata/versions/test_go1.7_1.a b/src/go/internal/gcimporter/testdata/versions/test_go1.7_1.a new file mode 100644 index 0000000000..554d04a72a Binary files /dev/null and b/src/go/internal/gcimporter/testdata/versions/test_go1.7_1.a differ