internal/gcimporter: remove end-of-section marker from size

Remove the bytes for the end-of-section marker "\n$$\n" from
the end of the exportdata by descreasing the returned size
appropriately. This marker is not a part of the export data
so it should be removed. Updates the unified IR reader to not
expect the marker to be present.

Updates golang/go#70651

Change-Id: Id3324ff21162d80f8e3d1c9792e9157b01ae1a3f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/633296
Reviewed-by: Robert Findley <[email protected]>
Commit-Queue: Tim King <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>

Author: timothy-king

internal/gcimporter/exportdata.go

@@ -42,7 +42,7 @@ func readGopackHeader(r *bufio.Reader) (name string, size int64, err error) {
 // export data section of an underlying cmd/compile created archive
 // file by reading from it. The reader must be positioned at the
 // start of the file before calling this function.
-// The size result is the length of the export data in bytes.
+// This returns the length of the export data in bytes.
 //
 // This function is needed by [gcexportdata.Read], which must
 // accept inputs produced by the last two releases of cmd/compile,
@@ -62,11 +62,12 @@ func FindExportData(r *bufio.Reader) (size int64, err error) {
 	}
 
 	// Archive file. Scan to __.PKGDEF.
+	var arsize int64
 	var name string
-	if name, size, err = readGopackHeader(r); err != nil {
+	if name, arsize, err = readGopackHeader(r); err != nil {
 		return
 	}
-	arsize := size
+	size = arsize
 
 	// First entry should be __.PKGDEF.
 	if name != "__.PKGDEF" {
@@ -105,7 +106,21 @@ func FindExportData(r *bufio.Reader) (size int64, err error) {
 		err = fmt.Errorf("unknown export data header: %q", hdr)
 		return
 	}
-	// TODO(taking): Remove end-of-section marker "\n$$\n" from size.
+
+	// For files with a binary export data header "$$B\n",
+	// these are always terminated by an end-of-section marker "\n$$\n".
+	// So the last bytes must always be this constant.
+	//
+	// The end-of-section marker is not a part of the export data itself.
+	// Do not include these in size.
+	//
+	// It would be nice to have sanity check that the final bytes after
+	// the export data are indeed the end-of-section marker. The split
+	// of gcexportdata.NewReader and gcexportdata.Read make checking this
+	// ugly so gcimporter gives up enforcing this. The compiler and go/types
+	// importer do enforce this, which seems good enough.
+	const endofsection = "\n$$\n"
+	size -= int64(len(endofsection))
 
 	if size < 0 {
 		err = fmt.Errorf("invalid size (%d) in the archive file: %d bytes remain without section headers (recompile package)", arsize, size)

internal/gcimporter/gcimporter_test.go

@@ -327,6 +327,7 @@ func TestVersionHandling(t *testing.T) {
 			t.Fatal(err)
 		}
 		// 2) find export data
+		// Index is an incorrect but 'good enough for tests' way to find the end of the export data.
 		i := bytes.Index(data, []byte("\n$$B\n")) + 5
 		j := bytes.Index(data[i:], []byte("\n$$\n")) + i
 		if i < 0 || j < 0 || i > j {

internal/gcimporter/ureader_yes.go

@@ -11,7 +11,6 @@ import (
 	"go/token"
 	"go/types"
 	"sort"
-	"strings"
 
 	"golang.org/x/tools/internal/aliases"
 	"golang.org/x/tools/internal/pkgbits"
@@ -71,7 +70,6 @@ func UImportData(fset *token.FileSet, imports map[string]*types.Package, data []
 	}
 
 	s := string(data)
-	s = s[:strings.LastIndex(s, "\n$$\n")]
 	input := pkgbits.NewPkgDecoder(path, s)
 	pkg = readUnifiedPackage(fset, nil, imports, input)
 	return