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/27814Reviewed-by: Matthew Dempsky <mdempsky@google.com>

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"

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
@@ -23,6 +25,7 @@ import (
 type importer struct {
 	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