Skip to content

Commit c01eead

Browse files
timothy-kingGo LUCI
timothy-king
authored and
Go LUCI
committed
internal/gcimporter: require binary export data header
FindExportData now returns an error if the header indicating the beginning of the export data is not "$$B\n". (Historically "$$\n" was also used for non-binary export data.) The signature of FindExportData no longer returns hdr as it is redundant. This also simplifies internal/gcimporter.Import using this new assumption. Updates golang/go#70651 Change-Id: I0d136d6c474ae6a7bfc2baeb33c22fd412711452 Reviewed-on: https://go-review.googlesource.com/c/tools/+/633279 Reviewed-by: Robert Griesemer <[email protected]> LUCI-TryBot-Result: Go LUCI <[email protected]> Commit-Queue: Tim King <[email protected]> Reviewed-by: Robert Findley <[email protected]>
1 parent 9a04136 commit c01eead

File tree

3 files changed

+50
-51
lines changed

3 files changed

+50
-51
lines changed

‎go/gcexportdata/gcexportdata.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ func Find(importPath, srcDir string) (filename, path string) {
106106
// additional trailing data beyond the end of the export data.
107107
func NewReader(r io.Reader) (io.Reader, error) {
108108
buf := bufio.NewReader(r)
109-
_, size, err := gcimporter.FindExportData(buf)
109+
size, err := gcimporter.FindExportData(buf)
110110
if err != nil {
111111
return nil, err
112112
}

‎internal/gcimporter/exportdata.go‎

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -41,14 +41,13 @@ func readGopackHeader(r *bufio.Reader) (name string, size int64, err error) {
4141
// FindExportData positions the reader r at the beginning of the
4242
// export data section of an underlying cmd/compile created archive
4343
// file by reading from it. The reader must be positioned at the
44-
// start of the file before calling this function. The hdr result
45-
// is the string before the export data, either "$$" or "$$B".
44+
// start of the file before calling this function.
4645
// The size result is the length of the export data in bytes.
4746
//
4847
// This function is needed by [gcexportdata.Read], which must
4948
// accept inputs produced by the last two releases of cmd/compile,
5049
// plus tip.
51-
func FindExportData(r *bufio.Reader) (hdr string, size int64, err error) {
50+
func FindExportData(r *bufio.Reader) (size int64, err error) {
5251
// Read first line to make sure this is an object file.
5352
line, err := r.ReadSlice('\n')
5453
if err != nil {
@@ -90,21 +89,27 @@ func FindExportData(r *bufio.Reader) (hdr string, size int64, err error) {
9089
return
9190
}
9291

93-
// Skip over object header to export data.
94-
// Begins after first line starting with $$.
92+
// Skip over object headers to get to the export data section header "$$B\n".
93+
// Object headers are lines that do not start with '$'.
9594
for line[0] != '$' {
9695
if line, err = r.ReadSlice('\n'); err != nil {
9796
err = fmt.Errorf("can't find export data (%v)", err)
9897
return
9998
}
10099
size -= int64(len(line))
101100
}
102-
hdr = string(line)
103-
// TODO(taking): Return an error when hdr != "$$B\n".
101+
102+
// Check for the binary export data section header "$$B\n".
103+
hdr := string(line)
104+
if hdr != "$$B\n" {
105+
err = fmt.Errorf("unknown export data header: %q", hdr)
106+
return
107+
}
104108
// TODO(taking): Remove end-of-section marker "\n$$\n" from size.
105109

106110
if size < 0 {
107111
err = fmt.Errorf("invalid size (%d) in the archive file: %d bytes remain without section headers (recompile package)", arsize, size)
112+
return
108113
}
109114

110115
return

‎internal/gcimporter/gcimporter.go‎

Lines changed: 37 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,8 @@ func FindPkg(path, srcDir string) (filename, id string) {
161161
// Import imports a gc-generated package given its import path and srcDir, adds
162162
// the corresponding package object to the packages map, and returns the object.
163163
// The packages map must contain all packages already imported.
164+
//
165+
// TODO(taking): Import is only used in tests. Move to gcimporter_test.
164166
func Import(packages map[string]*types.Package, path, srcDir string, lookup func(path string) (io.ReadCloser, error)) (pkg *types.Package, err error) {
165167
var rc io.ReadCloser
166168
var filename, id string
@@ -210,58 +212,50 @@ func Import(packages map[string]*types.Package, path, srcDir string, lookup func
210212
}
211213
defer rc.Close()
212214

213-
var hdr string
214215
var size int64
215216
buf := bufio.NewReader(rc)
216-
if hdr, size, err = FindExportData(buf); err != nil {
217+
if size, err = FindExportData(buf); err != nil {
217218
return
218219
}
219220

220-
switch hdr {
221-
case "$$B\n":
222-
var data []byte
223-
data, err = io.ReadAll(buf)
224-
if err != nil {
225-
break
226-
}
221+
var data []byte
222+
data, err = io.ReadAll(buf)
223+
if err != nil {
224+
return
225+
}
226+
if len(data) == 0 {
227+
return nil, fmt.Errorf("no data to load a package from for path %s", id)
228+
}
227229

228-
// TODO(gri): allow clients of go/importer to provide a FileSet.
229-
// Or, define a new standard go/types/gcexportdata package.
230-
fset := token.NewFileSet()
231-
232-
// Select appropriate importer.
233-
if len(data) > 0 {
234-
switch data[0] {
235-
case 'v', 'c', 'd':
236-
// binary: emitted by cmd/compile till go1.10; obsolete.
237-
return nil, fmt.Errorf("binary (%c) import format is no longer supported", data[0])
238-
239-
case 'i':
240-
// indexed: emitted by cmd/compile till go1.19;
241-
// now used only for serializing go/types.
242-
// See https://github.com/golang/go/issues/69491.
243-
_, pkg, err := IImportData(fset, packages, data[1:], id)
244-
return pkg, err
245-
246-
case 'u':
247-
// unified: emitted by cmd/compile since go1.20.
248-
_, pkg, err := UImportData(fset, packages, data[1:size], id)
249-
return pkg, err
250-
251-
default:
252-
l := len(data)
253-
if l > 10 {
254-
l = 10
255-
}
256-
return nil, fmt.Errorf("unexpected export data with prefix %q for path %s", string(data[:l]), id)
257-
}
258-
}
230+
// TODO(gri): allow clients of go/importer to provide a FileSet.
231+
// Or, define a new standard go/types/gcexportdata package.
232+
fset := token.NewFileSet()
233+
234+
// Select appropriate importer.
235+
switch data[0] {
236+
case 'v', 'c', 'd':
237+
// binary: emitted by cmd/compile till go1.10; obsolete.
238+
return nil, fmt.Errorf("binary (%c) import format is no longer supported", data[0])
239+
240+
case 'i':
241+
// indexed: emitted by cmd/compile till go1.19;
242+
// now used only for serializing go/types.
243+
// See https://github.com/golang/go/issues/69491.
244+
_, pkg, err := IImportData(fset, packages, data[1:], id)
245+
return pkg, err
246+
247+
case 'u':
248+
// unified: emitted by cmd/compile since go1.20.
249+
_, pkg, err := UImportData(fset, packages, data[1:size], id)
250+
return pkg, err
259251

260252
default:
261-
err = fmt.Errorf("unknown export data header: %q", hdr)
253+
l := len(data)
254+
if l > 10 {
255+
l = 10
256+
}
257+
return nil, fmt.Errorf("unexpected export data with prefix %q for path %s", string(data[:l]), id)
262258
}
263-
264-
return
265259
}
266260

267261
type byPath []*types.Package

0 commit comments

Comments
 (0)