From 59be2261078ebf98907317d3a9a2507eba5d015c Mon Sep 17 00:00:00 2001 From: Robert Griesemer Date: Tue, 12 Jun 2018 17:32:50 -0700 Subject: [PATCH] go/importer: better error message when importer is out of date Separated out panic handling for bimporter and importer so that the handler can consider the current version and report a better error. Added new export data test for export data version 999 (created by changing the compiler temporarily) and verifying expected error message. Fixes #25856. Change-Id: Iaafec07b79499154ef7c007341783fa07c57f24d Reviewed-on: https://go-review.googlesource.com/118496 Run-TryBot: Robert Griesemer TryBot-Result: Gobot Gobot Reviewed-by: Matthew Dempsky --- src/go/internal/gcimporter/bimport.go | 27 +++++++++--------- src/go/internal/gcimporter/gcimporter.go | 21 ++++++++++---- src/go/internal/gcimporter/gcimporter_test.go | 12 ++++++++ src/go/internal/gcimporter/iimport.go | 19 ++++++++++-- .../gcimporter/testdata/versions/test.go | 5 +++- .../testdata/versions/test_go1.11_0i.a | Bin 0 -> 2420 bytes .../testdata/versions/test_go1.11_6b.a | Bin 0 -> 2426 bytes .../testdata/versions/test_go1.11_999b.a | Bin 0 -> 2600 bytes .../testdata/versions/test_go1.11_999i.a | Bin 0 -> 2420 bytes 9 files changed, 62 insertions(+), 22 deletions(-) create mode 100644 src/go/internal/gcimporter/testdata/versions/test_go1.11_0i.a create mode 100644 src/go/internal/gcimporter/testdata/versions/test_go1.11_6b.a create mode 100644 src/go/internal/gcimporter/testdata/versions/test_go1.11_999b.a create mode 100644 src/go/internal/gcimporter/testdata/versions/test_go1.11_999i.a diff --git a/src/go/internal/gcimporter/bimport.go b/src/go/internal/gcimporter/bimport.go index 73ce465eab..503845e31c 100644 --- a/src/go/internal/gcimporter/bimport.go +++ b/src/go/internal/gcimporter/bimport.go @@ -50,24 +50,24 @@ type importer struct { // compromised, an error is returned. func BImportData(fset *token.FileSet, imports map[string]*types.Package, data []byte, path string) (_ int, pkg *types.Package, err error) { // catch panics and return them as errors + const currentVersion = 6 + version := -1 // unknown version 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). // Return a (possibly nil or incomplete) package unchanged (see #16088). - err = fmt.Errorf("cannot import, possibly version skew (%v) - reinstall package", e) + if version > currentVersion { + err = fmt.Errorf("cannot import %q (%v), export data is newer version - update tool", path, e) + } else { + err = fmt.Errorf("cannot import %q (%v), possibly version skew - reinstall package", path, e) + } } }() - if len(data) > 0 && data[0] == 'i' { - return iImportData(fset, imports, data[1:], path) - } - p := importer{ imports: imports, data: data, importpath: path, - version: -1, // unknown version + version: version, strList: []string{""}, // empty string is mapped to 0 pathList: []string{""}, // empty string is mapped to 0 fake: fakeFileSet{ @@ -92,7 +92,7 @@ func BImportData(fset *token.FileSet, imports map[string]*types.Package, data [] p.posInfoFormat = p.int() != 0 versionstr = p.string() if versionstr == "v1" { - p.version = 0 + version = 0 } } else { // Go1.8 extensible encoding @@ -100,24 +100,25 @@ func BImportData(fset *token.FileSet, imports map[string]*types.Package, data [] 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 + version = v } } } + p.version = version // read version specific flags - extend as necessary switch p.version { - // case 7: + // case currentVersion: // ... // fallthrough - case 6, 5, 4, 3, 2, 1: + case currentVersion, 5, 4, 3, 2, 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) + errorf("unknown bexport format version %d (%q)", p.version, versionstr) } // --- generic export data --- diff --git a/src/go/internal/gcimporter/gcimporter.go b/src/go/internal/gcimporter/gcimporter.go index cf89fcd1b4..d117f6fe4d 100644 --- a/src/go/internal/gcimporter/gcimporter.go +++ b/src/go/internal/gcimporter/gcimporter.go @@ -144,16 +144,27 @@ func Import(packages map[string]*types.Package, path, srcDir string, lookup func switch hdr { case "$$\n": 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) - if err == nil { - // TODO(gri): allow clients of go/importer to provide a FileSet. - // Or, define a new standard go/types/gcexportdata package. - fset := token.NewFileSet() + if err != nil { + break + } + + // TODO(gri): allow clients of go/importer to provide a FileSet. + // Or, define a new standard go/types/gcexportdata package. + fset := token.NewFileSet() + + // The indexed export format starts with an 'i'; the older + // binary export format starts with a 'c', 'd', or 'v' + // (from "version"). Select appropriate importer. + if len(data) > 0 && data[0] == 'i' { + _, pkg, err = iImportData(fset, packages, data[1:], id) + } else { _, pkg, err = BImportData(fset, packages, data, id) - return } + default: err = fmt.Errorf("unknown export data header: %q", hdr) } diff --git a/src/go/internal/gcimporter/gcimporter_test.go b/src/go/internal/gcimporter/gcimporter_test.go index 308f93e8bd..d496f2e57d 100644 --- a/src/go/internal/gcimporter/gcimporter_test.go +++ b/src/go/internal/gcimporter/gcimporter_test.go @@ -141,9 +141,21 @@ func TestVersionHandling(t *testing.T) { } pkgpath := "./" + name[:len(name)-2] + if testing.Verbose() { + t.Logf("importing %s", name) + } + // test that export data can be imported _, err := Import(make(map[string]*types.Package), pkgpath, dir, nil) if err != nil { + // ok to fail if it fails with a newer version error for select files + if strings.Contains(err.Error(), "newer version") { + switch name { + case "test_go1.11_999b.a", "test_go1.11_999i.a": + continue + } + // fall through + } t.Errorf("import %q failed: %v", pkgpath, err) continue } diff --git a/src/go/internal/gcimporter/iimport.go b/src/go/internal/gcimporter/iimport.go index 1d13449ef6..a333f98f3a 100644 --- a/src/go/internal/gcimporter/iimport.go +++ b/src/go/internal/gcimporter/iimport.go @@ -10,6 +10,7 @@ package gcimporter import ( "bytes" "encoding/binary" + "fmt" "go/constant" "go/token" "go/types" @@ -60,13 +61,25 @@ const ( // If the export data version is not recognized or the format is otherwise // compromised, an error is returned. func iImportData(fset *token.FileSet, imports map[string]*types.Package, data []byte, path string) (_ int, pkg *types.Package, err error) { + const currentVersion = 0 + version := -1 + defer func() { + if e := recover(); e != nil { + if version > currentVersion { + err = fmt.Errorf("cannot import %q (%v), export data is newer version - update tool", path, e) + } else { + err = fmt.Errorf("cannot import %q (%v), possibly version skew - reinstall package", path, e) + } + } + }() + r := &intReader{bytes.NewReader(data), path} - version := r.uint64() + version = int(r.uint64()) switch version { - case 0: + case currentVersion: default: - errorf("cannot import %q: unknown iexport format version %d", path, version) + errorf("unknown iexport format version %d", version) } sLen := int64(r.uint64()) diff --git a/src/go/internal/gcimporter/testdata/versions/test.go b/src/go/internal/gcimporter/testdata/versions/test.go index ac9c968c2d..227fc09251 100644 --- a/src/go/internal/gcimporter/testdata/versions/test.go +++ b/src/go/internal/gcimporter/testdata/versions/test.go @@ -11,7 +11,10 @@ // // go build -o test_go1.$X_$Y.a test.go // -// with $X = Go version and $Y = export format version. +// with $X = Go version and $Y = export format version +// (add 'b' or 'i' to distinguish between binary and +// indexed format starting with 1.11 as long as both +// formats are supported). // // Make sure this source is extended such that it exercises // whatever export format change has taken place. diff --git a/src/go/internal/gcimporter/testdata/versions/test_go1.11_0i.a b/src/go/internal/gcimporter/testdata/versions/test_go1.11_0i.a new file mode 100644 index 0000000000000000000000000000000000000000..b00fefed0462172f5f5370f9769ed19342fc0c16 GIT binary patch literal 2420 zcmd5--)q}e6h69muBw_KLdZ-RK{Y0XwvlDomSrzBA-GAqvNg0Rgub}4e66UFB}bN1 zGRnHwvDZCpu!ju#KkT)%u;;z(|JYw}yK^PWah)GyZ@WnM+1DqefAUVHk@lW)sAU!R=Z-ziAz*d$3R7Pll9 zcIA!Y&f_PKAMce$p6nHiH}FUCis}R)bTZs2 zZF!b|xb0G}ckQ0=MuV0B_x9eXpn!016NLK`U_uExPS8<$Mh9x4pq?oLj0}OT#_rqmo&#X{*fU1qhfxSD`(3C5+jDF$ zuq|)&{&(G~RkWJbrCP0AH}tATjdIsCbd8o9mRYWS5?ra+J!)g4U219-z0oiCdtFNF z{hC>?+FfAy*!r%Am2Oz$C~&AxLo1@a7O=a?4(QRj`j3Du#?EdKc%S6!>snnm>iurN zVOUzZZc?*eGi`%bsAf~$1lDo_6}O@$7Vyes0pbClCe#!zDr7zrrnKiC z)Ep?mkht`l%9?7!(MVim;V9&%Z~u} z&`C3<^pF0^eSS?8e@W;6X4CzqHjQ6P2bWkLA7fG=w}b~Xfskcb1`VgV488`UoRM-$ zM#>1;tSDrq9NyWuK~`LlLFOp3h`=S1gWMT#SHf54q z+T#odoZ&U^e&Pgo*>!b7pd|!ECs+{S7h$A2oh&h85CxPB_vkWqRYuG#M&X~SNmgMM zZB`t@P{fp{!UpC9b61q0`2t5RMmF&rp{ityyoT3k+e8ErZ43Su@LJ@R8XUD5qxKP+ q<vTbv3ss< z^@i{M(5+HID_KpdmGWgnFKX1tH%&v=Xue{Z`O*i*g@WCpHa6O%rdH4^?R>k{q_o^F zndPG01cq;^-R)tdo7O0B9Xg=C70}ieu#M;h^z>T%r@$6NWz%(gANXtQT3I*B?Pj}T zSX#bpQnOq#ZG#r5W>eh+w&l1g9z~7FZ%WTLT670(_YRYd*kn3^%dz4ze>=G2TLXvQ zL099b;V|e2cNkUCXcGAKtZc@vZ%bPyHi)vu6LB@L>x^L{u7-8p#GItDJsQ~4Z>ku_ za+7j=R;%Ys!?ONp5On)gb?jz07^+$x*mguUz^cQqy1E`-@yTbc0#jlbQd+)my#}@t zqNcv`<=Ada4{R+Wj|z%s1>uzemYwq1cmhlg_waVW#XGT@%E*cRpK2@e{*Sel%zLaX z*b}o@WcUR8pSUf&CrQAbNCJctzKp1E`2Jw33BU0TVcx?bRLyrTXZ&so(JmxrgiG3M zFKUVtu9KeLMmnaSX1KD(2;}M4A&&L2{JofNgD4tTV7E*^Yvf)G@qdtW{BF*1b6EGe zi0giCQulLl-Ot5!AJ5Q;J;wCU|4w~!PZWQR*Z(Es^Tsl^UyCQVNNyiuVj!3Jy8&PV zA!$g1hRaj}KLb%tNGT;DC4^*B6p~U3pJdn|DbC9va}-%b;1WqeY648li6jCu9GJ!j zftgF-!z)1P)5x>9Ox_6aMo1N90;DFBG&vTlu-Fq#P<{|_%>ELN?EXrGm|aCF0h89o z4C~}vfO8?GA;Z$POj1jA&d}iuXMFgX6Kt^i>X<-_2#8KFAi_IgqO4 E0@X}F`Tzg` literal 0 HcmV?d00001 diff --git a/src/go/internal/gcimporter/testdata/versions/test_go1.11_999b.a b/src/go/internal/gcimporter/testdata/versions/test_go1.11_999b.a new file mode 100644 index 0000000000000000000000000000000000000000..c35d22dce691e67127e04ea74ddd8c97a57e22ff GIT binary patch literal 2600 zcmd5-QE%c#5MFNy=W|eEYCj zofNe{t4~#>{-OSb&aBx$f#kU>+p|0K%{Q~N-rZLxPV5d&m2OwR`tYv({kfJlV9tK4GN1>%$9hg#UXWp1K*PW)>y)|`=NRW*-&W<%|Dk5#Om*JxNw>r9#U!*r~b zP0ci$Mx$A+X{J%OwbuqT?WWm}oe_(|V4Sd6=}m*7rv;u?yl%MV_s08v?Wf6Q6m@>S zx_JAs)U_|7*t!_KtKHs)x1H;^ei#;&H_#MC+23y|pIJN!!m-w9G!%tU=~==i2_YNG z@A|fKfBo+A$IHvk#w2#X^22Q&a_kIW_--(Y!Wa_RM!e@F&i0IHqM%0i!xycgGydf~ zU_+05O{vr+l#orjNhAW1m98Il^>AK5QK?63X_$73{$JMel|qOgmK#G)BLDpLr;&V# zn63|;$v`g_^@UP^kwyqP;Ej138~ZV%zGZv9{g znV&e*B=p&s#ZJP!Q?wnYb7FUEW^{*u9S-P);czK(-!$r`Rqyxu4a+ggb(`7sn(bPw z!VH(0Ho{N+P=|@>X^{1WI|EEG?uSo;jsdh=}V&d^_VEymMv z5{#JcyS*Tp=tdb=^(-YE!-<85gnq;ihxvBU5u^!MAa>%|`GU%Q?zyr%_5^ko2BC*( zkn~83oP=*VG!&Pjci*5^9g-`C$oe6cpk{Da~(`lDJVy;zog4 zbVW=-{{e#MzIi6gvuXw^X8{AKl@uyEI2~bJx6@$%1PEUGwmQYvIB5<$qk2Z$*50 zk75>_FIQD5p>37s>5beZJ44wf8d^aX7v%GO;k|hPJq4YWL!pz878ng++&xoiZnuMs z17y5kpgjiH=els+Gz1+rDI`qsK{i@_81CZGq8iaMXf;*%R8ePV0gI{7COseryqA?D z63LJRsHGFggVXp7#Hd+yYxGLd9RSj+|6dE7!@>Xn literal 0 HcmV?d00001 diff --git a/src/go/internal/gcimporter/testdata/versions/test_go1.11_999i.a b/src/go/internal/gcimporter/testdata/versions/test_go1.11_999i.a new file mode 100644 index 0000000000000000000000000000000000000000..99401d7c37ca48fbca32f0448b71606b7f853ce4 GIT binary patch literal 2420 zcmd5-&uimG6n+|9N2{11LU7Geg4UQ2(nOZ!Uy7R;W4zgI15L=fG&ip-j}>(l%aP@5 z1EJe%>9vOzdPrgahhCRm=((5v4ZZd+nDmWgC0^&p(pv{<-psuBeecaDjr3^C^sS@1 z)aj`EyH8q=ca*4U6U$9Bj`~We6ngYZWEJJPWP6I&{f1hB(lh;+&OkAbd+6@bQ|c<8 zmWzf_EK|}RnwHiTHiO1>Ckfq0rjQs*l~M` z(^Im$U%veM#jyKq|CxPI9T!}uqv`px)3g4;aP;{oXzzd9(!9g7QZ*}WV3Q;%mupJS z-*fx&R_@8(!QNgwKk}`-?d3!2f$8RL%Q+r;J|@ZsbQJW=z|5agf8=dW>OKs7V9D=%9a!78T+cGy z(c9m3vs~25W|wN^LbaloG+HTijf$?(!lr2y%I^gii&l?X*l3p;T2bHZ7y7*}rPY4f zsFtiQFnnr#$Hhvw%yHn^bU=MGpuIY<^=Jq5^7VDCs#mK0Zhy04 zYK5vnjcVDjDzr#7i|Pijy6vgB6*VHiO`ZluID@`-Psn;4GArP6?6}gei7UQ2u<1Q? zEsh$Eg5%(xpc)2U7k*QftvK~vvSs3cNHs3RbYV9cLm{Tix*1|z%h(?etl1}3ifg&e zIoYe}+Ox2%KOO|mF;#7=>jWcJD*$UmR0I6p@T0G;@pC@)TC2d67>1Ol@0(}9av^Hw zEnkk~CiK8oBl742Gc<$n$NpzUBRcnI`(tQxc$`tI5;b)!vS0ntc=WXBNKY;)8AHY3y z;*2rW<9?>Dw_{AxV7!}9nL69c&=JeUcDq#+F&PE!ed4MaI1rIdt} z5RyqzNJ=TZlVO9TxFCbfQDhN;OC$xUYhYSVBoUb5z%1wjXNQEe%B)CVHu`4lRBryvAOf|9sD`=DA z5QZYAyb!i9Cm8#p1hwZlsx#8SbA+mrCOL<5v@Ifnh_(*@2zV{>N-G@I2vLU!O>$^+ nD3dor{X)Qn<{JT5T7U9>hmi$T8}%kKFB9#