]> Cypherpunks repositories - gostls13.git/commitdiff
cmd/go/internal/mmap: reslice to file size on Windows
authorMichael Matloob <matloob@golang.org>
Fri, 3 Jan 2025 20:06:42 +0000 (15:06 -0500)
committerMichael Matloob <matloob@golang.org>
Thu, 9 Jan 2025 19:44:05 +0000 (11:44 -0800)
The Mmap function returns a Data struct containing a slice with the
mapped contents of the file. Before this change, on Windows, the slice
contained the contents of all the pages of the mapping, including past
the end of the file. Re-slice the slice to the length of the file (if
if the slice is longer) so that the slice contains only the data in the
file.

For #71059

Change-Id: I389b752505b6fa1252b5c6d836a37bc7e662a45d
Reviewed-on: https://go-review.googlesource.com/c/go/+/640155
Reviewed-by: Russ Cox <rsc@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>

src/cmd/go/internal/mmap/mmap_test.go [new file with mode: 0644]
src/cmd/go/internal/mmap/mmap_windows.go
src/cmd/go/internal/mmap/testdata/small_file.txt [new file with mode: 0644]

diff --git a/src/cmd/go/internal/mmap/mmap_test.go b/src/cmd/go/internal/mmap/mmap_test.go
new file mode 100644 (file)
index 0000000..3f4b63c
--- /dev/null
@@ -0,0 +1,32 @@
+// Copyright 2025 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package mmap
+
+import (
+       "bytes"
+       "testing"
+)
+
+// TestMmap does a round trip to make sure the slice returned by
+// mmap contains the same data as was written to the file. It's
+// a test on one of the issues in #71059: on Windows we were
+// returning a slice containing all the data in the mmapped pages,
+// which could be longer than the file.
+func TestMmap(t *testing.T) {
+       // Use an already existing file as our test data. Avoid creating
+       // a temporary file so that we don't have to close the mapping on
+       // Windows before deleting the file during test cleanup.
+       f := "testdata/small_file.txt"
+
+       want := []byte("This file is shorter than 4096 bytes.\n")
+
+       data, _, err := Mmap(f)
+       if err != nil {
+               t.Fatalf("calling Mmap: %v", err)
+       }
+       if !bytes.Equal(data.Data, want) {
+               t.Fatalf("mmapped data slice: got %q; want %q", data.Data, want)
+       }
+}
index d00bef71e5cfc5fbbbd5208b39baa56951e456a6..4163484b1a351026231cb378ef0c83b2b3a5133d 100644 (file)
@@ -37,5 +37,11 @@ func mmapFile(f *os.File) (Data, error) {
                return Data{}, fmt.Errorf("VirtualQuery %s: %w", f.Name(), err)
        }
        data := unsafe.Slice((*byte)(unsafe.Pointer(addr)), int(info.RegionSize))
-       return Data{f, data}, nil
+       if len(data) < int(size) {
+               // In some cases, especially on 386, we may not receive a in incomplete mapping:
+               // one that is shorter than the file itself. Return an error in those cases because
+               // incomplete mappings are not useful.
+               return Data{}, fmt.Errorf("mmapFile: received incomplete mapping of file")
+       }
+       return Data{f, data[:int(size)]}, nil
 }
diff --git a/src/cmd/go/internal/mmap/testdata/small_file.txt b/src/cmd/go/internal/mmap/testdata/small_file.txt
new file mode 100644 (file)
index 0000000..10bb609
--- /dev/null
@@ -0,0 +1 @@
+This file is shorter than 4096 bytes.