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 <mdempsky@google.com>

diff --git a/src/cmd/compile/internal/gc/bexport.go b/src/cmd/compile/internal/gc/bexport.go
index a43158d14ba10159ade4ecb17ae06c7e3873ad0c..f3204656dc213acf21845482647c70592706bb31 100644 (file)
--- 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 be196ceee53682e0123493c454a29526a1259854..39fa844457825846adb9287ef5cb5190a21c3c78 100644 (file)
--- 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