From 5a6f973565ccb54c77e03cbb4844fd0ea392d3fe Mon Sep 17 00:00:00 2001 From: Robert Griesemer Date: Thu, 25 Aug 2016 16:53:10 -0700 Subject: [PATCH] cmd/compile: fail gracefully on export format skew Import errors due to unexpected format are virtually always due to version skew. Don't panic but report a good error message (incl. hint that the imported package needs to be reinstalled) if not in debugFormat mode. Recognize export data format version and store it so it can be used to automatically handle minor version differences. We did this before, but not very well. No export data format changes. Manually tested with corrupted export data. For #16881. Change-Id: I53ba98ef747b1c81033a914bb61ee52991f35a90 Reviewed-on: https://go-review.googlesource.com/27814 Reviewed-by: Matthew Dempsky --- src/cmd/compile/internal/gc/bexport.go | 10 ++- src/cmd/compile/internal/gc/bimport.go | 106 ++++++++++++++++--------- 2 files changed, 73 insertions(+), 43 deletions(-) diff --git a/src/cmd/compile/internal/gc/bexport.go b/src/cmd/compile/internal/gc/bexport.go index a43158d14b..f3204656dc 100644 --- a/src/cmd/compile/internal/gc/bexport.go +++ b/src/cmd/compile/internal/gc/bexport.go @@ -157,9 +157,8 @@ const debugFormat = false // default: false // TODO(gri) disable and remove once there is only one export format again const forceObjFileStability = true -// Current export format version. -// Must not start with 'c' or 'd' (initials of prior format). -const exportVersion = "version 1" +// Current export format version. Increase with each format change. +const exportVersion = 1 // exportInlined enables the export of inlined function bodies and related // dependencies. The compiler should work w/o any loss of functionality with @@ -217,7 +216,10 @@ func export(out *bufio.Writer, trace bool) int { } // write version info - p.rawStringln(exportVersion) + // The version string must start with "version %d" where %d is the version + // number. Additional debugging information may follow after a blank; that + // text is ignored by the importer. + p.rawStringln(fmt.Sprintf("version %d", exportVersion)) var debug string if debugFormat { debug = "debug" diff --git a/src/cmd/compile/internal/gc/bimport.go b/src/cmd/compile/internal/gc/bimport.go index be196ceee5..39fa844457 100644 --- a/src/cmd/compile/internal/gc/bimport.go +++ b/src/cmd/compile/internal/gc/bimport.go @@ -13,6 +13,8 @@ import ( "cmd/compile/internal/big" "encoding/binary" "fmt" + "strconv" + "strings" ) // The overall structure of Import is symmetric to Export: For each @@ -21,8 +23,9 @@ import ( // changes to bimport.go and bexport.go. type importer struct { - in *bufio.Reader - buf []byte // reused for reading strings + in *bufio.Reader + buf []byte // reused for reading strings + version int // export format version // object lists, in order of deserialization strList []string @@ -48,10 +51,12 @@ type importer struct { func Import(in *bufio.Reader) { p := importer{ in: in, + 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). @@ -64,18 +69,34 @@ func Import(in *bufio.Reader) { } p.trackAllTypes = p.rawByte() == 'a' p.posInfoFormat = p.bool() - const go17version = "v1" - if s := p.string(); s != go17version { - Fatalf("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 - if s := p.rawStringln(b); s != exportVersion { - Fatalf("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.bool() p.posInfoFormat = p.bool() + case 0: + // Go1.7 encoding format - nothing to do here + default: + formatErrorf("unknown export format version %d (%q)", p.version, versionstr) } // --- generic export data --- @@ -106,7 +127,7 @@ func Import(in *bufio.Reader) { // self-verification if count := p.int(); count != objcount { - Fatalf("importer: got %d objects; want %d", objcount, count) + formatErrorf("got %d objects; want %d", objcount, count) } // --- compiler-specific export data --- @@ -126,12 +147,12 @@ func Import(in *bufio.Reader) { // self-verification if count := p.int(); count != objcount { - Fatalf("importer: got %d objects; want %d", objcount, count) + formatErrorf("got %d objects; want %d", objcount, count) } // read inlineable functions bodies if dclcontext != PEXTERN { - Fatalf("importer: unexpected context %d", dclcontext) + formatErrorf("unexpected context %d", dclcontext) } objcount = 0 @@ -143,12 +164,12 @@ func Import(in *bufio.Reader) { // don't process the same function twice if i <= i0 { - Fatalf("importer: index not increasing: %d <= %d", i, i0) + formatErrorf("index not increasing: %d <= %d", i, i0) } i0 = i if Funcdepth != 0 { - Fatalf("importer: unexpected Funcdepth %d", Funcdepth) + formatErrorf("unexpected Funcdepth %d", Funcdepth) } // Note: In the original code, funchdr and funcbody are called for @@ -182,11 +203,11 @@ func Import(in *bufio.Reader) { // self-verification if count := p.int(); count != objcount { - Fatalf("importer: got %d functions; want %d", objcount, count) + formatErrorf("got %d functions; want %d", objcount, count) } if dclcontext != PEXTERN { - Fatalf("importer: unexpected context %d", dclcontext) + formatErrorf("unexpected context %d", dclcontext) } p.verifyTypes() @@ -199,15 +220,22 @@ func Import(in *bufio.Reader) { testdclstack() // debugging only } +func formatErrorf(format string, args ...interface{}) { + if debugFormat { + Fatalf(format, args...) + } + + Yyerror("cannot import %q due to version skew - reinstall package (%s)", + importpkg.Path, fmt.Sprintf(format, args...)) + errorexit() +} + func (p *importer) verifyTypes() { for _, pair := range p.cmpList { pt := pair.pt t := pair.t if !Eqtype(pt.Orig, t) { - // TODO(gri) Is this a possible regular error (stale files) - // or can this only happen if export/import is flawed? - // (if the latter, change to Fatalf here) - Yyerror("inconsistent definition for type %v during import\n\t%v (in %q)\n\t%v (in %q)", pt.Sym, Tconv(pt, FmtLong), pt.Sym.Importdef.Path, Tconv(t, FmtLong), importpkg.Path) + formatErrorf("inconsistent definition for type %v during import\n\t%v (in %q)\n\t%v (in %q)", pt.Sym, Tconv(pt, FmtLong), pt.Sym.Importdef.Path, Tconv(t, FmtLong), importpkg.Path) } } } @@ -227,7 +255,7 @@ func (p *importer) pkg() *Pkg { // otherwise, i is the package tag (< 0) if i != packageTag { - Fatalf("importer: expected package tag, found tag = %d", i) + formatErrorf("expected package tag, found tag = %d", i) } // read package data @@ -236,18 +264,18 @@ func (p *importer) pkg() *Pkg { // we should never see an empty package name if name == "" { - Fatalf("importer: empty package name for path %q", path) + formatErrorf("empty package name for path %q", path) } // we should never see a bad import path if isbadimport(path) { - Fatalf("importer: bad package path %q for package %s", path, name) + formatErrorf("bad package path %q for package %s", path, name) } // an empty path denotes the package we are currently importing; // it must be the first package we see if (path == "") != (len(p.pkgList) == 0) { - Fatalf("importer: package path %q for pkg index %d", path, len(p.pkgList)) + formatErrorf("package path %q for pkg index %d", path, len(p.pkgList)) } // add package to pkgList @@ -259,7 +287,7 @@ func (p *importer) pkg() *Pkg { pkg.Name = name numImport[name]++ } else if pkg.Name != name { - Yyerror("importer: conflicting package names %s and %s for path %q", pkg.Name, name, path) + Yyerror("conflicting package names %s and %s for path %q", pkg.Name, name, path) } if incannedimport == 0 && myimportpath != "" && path == myimportpath { Yyerror("import %q: package depends on %q (import cycle)", importpkg.Path, path) @@ -307,7 +335,7 @@ func (p *importer) obj(tag int) { if sym.Def != nil && sym.Def.Op == ONAME { // function was imported before (via another import) if !Eqtype(sig, sym.Def.Type) { - Fatalf("importer: inconsistent definition for func %v during import\n\t%v\n\t%v", sym, sym.Def.Type, sig) + formatErrorf("inconsistent definition for func %v during import\n\t%v\n\t%v", sym, sym.Def.Type, sig) } p.funcList = append(p.funcList, nil) break @@ -327,7 +355,7 @@ func (p *importer) obj(tag int) { } default: - Fatalf("importer: unexpected object (tag = %d)", tag) + formatErrorf("unexpected object (tag = %d)", tag) } } @@ -500,7 +528,7 @@ func (p *importer) typ() *Type { case interfaceTag: t = p.newtyp(TINTER) if p.int() != 0 { - Fatalf("importer: unexpected embedded interface") + formatErrorf("unexpected embedded interface") } tointerface0(t, p.methodList()) @@ -517,11 +545,11 @@ func (p *importer) typ() *Type { ct.Elem = p.typ() default: - Fatalf("importer: unexpected type (tag = %d)", i) + formatErrorf("unexpected type (tag = %d)", i) } if t == nil { - Fatalf("importer: nil type (type tag = %d)", i) + formatErrorf("nil type (type tag = %d)", i) } return t @@ -642,7 +670,7 @@ func (p *importer) param(named bool) *Node { if named { name := p.string() if name == "" { - Fatalf("importer: expected named parameter") + formatErrorf("expected named parameter") } // TODO(gri) Supply function/method package rather than // encoding the package for each parameter repeatedly. @@ -696,18 +724,18 @@ func (p *importer) value(typ *Type) (x Val) { x.U = p.string() case unknownTag: - Fatalf("importer: unknown constant (importing package with errors)") + formatErrorf("unknown constant (importing package with errors)") case nilTag: x.U = new(NilVal) default: - Fatalf("importer: unexpected value tag %d", tag) + formatErrorf("unexpected value tag %d", tag) } // verify ideal type if typ.IsUntyped() && untype(x.Ctype()) != typ { - Fatalf("importer: value %v and type %v don't match", x, typ) + formatErrorf("value %v and type %v don't match", x, typ) } return @@ -1156,7 +1184,7 @@ func (p *importer) tagOrIndex() int { func (p *importer) int() int { x := p.int64() if int64(int(x)) != x { - Fatalf("importer: exported integer too large") + formatErrorf("exported integer too large") } return int(x) } @@ -1195,12 +1223,12 @@ func (p *importer) string() string { func (p *importer) marker(want byte) { if got := p.rawByte(); got != want { - Fatalf("importer: incorrect marker: got %c; want %c (pos = %d)", got, want, p.read) + formatErrorf("incorrect marker: got %c; want %c (pos = %d)", got, want, p.read) } pos := p.read if n := int(p.rawInt64()); n != pos { - Fatalf("importer: incorrect position: got %d; want %d", n, pos) + formatErrorf("incorrect position: got %d; want %d", n, pos) } } @@ -1208,7 +1236,7 @@ func (p *importer) marker(want byte) { func (p *importer) rawInt64() int64 { i, err := binary.ReadVarint(p) if err != nil { - Fatalf("importer: read error: %v", err) + formatErrorf("read error: %v", err) } return i } @@ -1235,13 +1263,13 @@ func (p *importer) rawByte() byte { c, err := p.in.ReadByte() p.read++ if err != nil { - Fatalf("importer: read error: %v", err) + formatErrorf("read error: %v", err) } if c == '|' { c, err = p.in.ReadByte() p.read++ if err != nil { - Fatalf("importer: read error: %v", err) + formatErrorf("read error: %v", err) } switch c { case 'S': @@ -1249,7 +1277,7 @@ func (p *importer) rawByte() byte { case '|': // nothing to do default: - Fatalf("importer: unexpected escape sequence in export data") + formatErrorf("unexpected escape sequence in export data") } } return c -- 2.48.1