From fba8f4deba81b8c5d903ec2f52dcb151f13a147b Mon Sep 17 00:00:00 2001 From: Robert Griesemer Date: Thu, 25 Aug 2016 17:17:50 -0700 Subject: [PATCH] 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 --- src/go/internal/gcimporter/bimport.go | 94 ++++++++++++------ src/go/internal/gcimporter/gcimporter.go | 4 +- src/go/internal/gcimporter/gcimporter_test.go | 65 ++++++++++++ .../gcimporter/testdata/versions/test.go | 25 +++++ .../testdata/versions/test_go1.7_0.a | Bin 0 -> 1862 bytes .../testdata/versions/test_go1.7_1.a | Bin 0 -> 2316 bytes 6 files changed, 158 insertions(+), 30 deletions(-) create mode 100644 src/go/internal/gcimporter/testdata/versions/test.go create mode 100644 src/go/internal/gcimporter/testdata/versions/test_go1.7_0.a create mode 100644 src/go/internal/gcimporter/testdata/versions/test_go1.7_1.a 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 0000000000000000000000000000000000000000..edb6c3f25a159d65fd097bcf0b5bc6a2add86eee GIT binary patch literal 1862 zcmcgsUyIvD5FhP1j?lS5s8`p7a45zc=D3sfXHS+(DIr|bKwk3DLf)K}w9=`OB}bN1 z^3s>&rSuczE%^q0%WFuW&wc6l=qK3gNV1LV2CkISWwo<2znR(DnOWWbk`=+lSITs1 zJbduY`PcXKW|%8lk!IhlE~PbMkKL8VV{4ha}V4EOeHssAj^ZGh&GQ1o9aT0V zE7|B7>L+=&s3k_6(@*cFEPHY<;b{n;kv(Zj0QL#&g9Lc0GL7@8k*`v;$zj8PET0xV z^BVO3%BON$0l*rK(@+9ome1$hFfQ2Q!sz#nt;>lH@;??fh+eI;#5~Q*0)&k6i-ycW z9M$0Qj_tQ`kmdnP7w_MizUT7DnM6*+L(e&jEb6nfh=$|9wIhc9x0JC3LLT7YKKD$= zoW5pom1AG}q=&cQ38gfS>1SbHk^94bS&&D_~y4t#C474yO~F zx{e=?EsI(n1K~A}t9(YwEZB8V*XMo=G#Zel@vd3FSY~B1<3=3#Nx3jg3&fp9JHtm- zb*gb(_3Tyw4G{X;3|+FKV9!AeYO9@U(KxTqPLu43v0KW+wUgD^AcoD!&10FfvhExZ z{hffO7=c>Luya7T4gB{`d;c~9VK?Y*K`QL>muo}l$gTT7pAXdgKb;SHAILu7Z0*CZ z$Vnea8*sMT_y(I%(LMYsy53j(RYQ_M-OZ!CGFsC&v>SmeyI1*Y@NSULW-JR&F}M7b z74dS$vvPr9P1g?BjvoBp`TV*pzgqKG(e2A_>khY>f3O#XkS=t=MBR2vZ?$E$)mEU@ zX}2+{H&g(k335O>(Ans8dupq*(Ye{^Xev=b1)<#_J?J65xuUl!IjJTQ+Tn&d5SmVO z&|4L9OU30-X#F;7q3wVi!4c99%waB@DtRp(-9eT9Ls@~rW84jeb__De1W?iu+DW^* ztk4W%1O8PslOTw62!BX8HS_^`<>I7v@u;$@{1|hUypx8nB@`I%B$SxHRjVyDx``D7 NcQ?$zihvOW!*Feif1)VE2;9JR0_2dO%^`=P@{(NAY$Q^q zq@)3YqSvA?&;mKQK%XFo=67nQwk}c6N1r4`=@I z4QVjY?%la{@Y$hS41G>HDZY!Vw@d!y__I<77%Q$(zN)K#c>n z721~V^(@nOdp7a9*e2NS6U%b@y`XRQecUyDrXek9hYjgpSxW8pP3aNICUKgmh9m(J zE>Fp13M=mcJbwDct-GJ!y?ehi$^1@~cJd4pJnTe%JQ}AN%eTYP0iNQ{+{+}FXi;i^ zaC3;0hlep42Jj)eBn$untfEyAfQ6M{5TyexofmUa?SenA@`l@+AI|@+^3rt)AWDti zFc5(1(gGoT`q9~Wc{bR$=8Rc!A^{xJ3N zaPs=4?zs*Lt$t{QByg>6XxJX^hIY{N9W%tt{}R};{DAmu5RbUJW%ffO3_L=5VV5l6*ZhCesyzp>E=A;JYzhJs%EWh zmQ3Rj(IVfArxQ&#*ggwa33~Y@Drfz+^W(fHroT2598#a z_Et?wC-c}qH;c%{Wgg>c-mpMhm4pT?138zmh6U;r>bKA1^~(^bS!k>6z;$OoihIxy zYqx(mXv??1KWMMNVb1}ZwN?0)al#u^2R3VU_C^$g(@X5fDY__b0bN>!l`#m;?xy<~7Z;B9V zLKAd`t=H6AU6gBe32Kdcoi)N0nVpA1(Hd$%V}WU|%eBS=bG5)IGLk_Cs$4ycBR~f-YN8|AfotChY*l~BSE^i{gInmi0M7+UXW+Gc88z0vVt#QqU%$7~6Y}XlzC8MDY*kH5)V;GCf`>CKa_pp6Wlt4RT zUnW&-5J5!1Bn_%KbzU{9xX1+dk}JE2DH%244}ndEXSl^(IJt`l+=}~g$1-{)n5P00 dxUU2z+5L^LdsMlx$dx6&zhKtR&Wb;+KLH~_A*28R literal 0 HcmV?d00001 -- 2.48.1