From 91336b480a9f2e65e9126c9739f41f8baf8ddc66 Mon Sep 17 00:00:00 2001
From: Davis Goodin <dagood@microsoft.com>
Date: Fri, 12 Jan 2024 15:11:05 -0800
Subject: [PATCH] fileinfo: internally fix FileBasicInfo memory alignment

Signed-off-by: Davis Goodin <dagood@microsoft.com>
---
 fileinfo.go      | 22 ++++++++++++++++++----
 fileinfo_test.go | 47 +++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 65 insertions(+), 4 deletions(-)

diff --git a/fileinfo.go b/fileinfo.go
index 702950e7..c860eb99 100644
--- a/fileinfo.go
+++ b/fileinfo.go
@@ -18,9 +18,18 @@ type FileBasicInfo struct {
 	_                                                       uint32 // padding
 }
 
+// alignedFileBasicInfo is a FileBasicInfo, but aligned to uint64 by containing
+// uint64 rather than windows.Filetime. Filetime contains two uint32s. uint64
+// alignment is necessary to pass this as FILE_BASIC_INFO.
+type alignedFileBasicInfo struct {
+	CreationTime, LastAccessTime, LastWriteTime, ChangeTime uint64
+	FileAttributes                                          uint32
+	_                                                       uint32 // padding
+}
+
 // GetFileBasicInfo retrieves times and attributes for a file.
 func GetFileBasicInfo(f *os.File) (*FileBasicInfo, error) {
-	bi := &FileBasicInfo{}
+	bi := &alignedFileBasicInfo{}
 	if err := windows.GetFileInformationByHandleEx(
 		windows.Handle(f.Fd()),
 		windows.FileBasicInfo,
@@ -30,16 +39,21 @@ func GetFileBasicInfo(f *os.File) (*FileBasicInfo, error) {
 		return nil, &os.PathError{Op: "GetFileInformationByHandleEx", Path: f.Name(), Err: err}
 	}
 	runtime.KeepAlive(f)
-	return bi, nil
+	// Reinterpret the alignedFileBasicInfo as a FileBasicInfo so it matches the
+	// public API of this module. The data may be unnecessarily aligned.
+	return (*FileBasicInfo)(unsafe.Pointer(bi)), nil
 }
 
 // SetFileBasicInfo sets times and attributes for a file.
 func SetFileBasicInfo(f *os.File, bi *FileBasicInfo) error {
+	// Create an alignedFileBasicInfo based on a FileBasicInfo. The copy is
+	// suitable to pass to GetFileInformationByHandleEx.
+	biAligned := *(*alignedFileBasicInfo)(unsafe.Pointer(bi))
 	if err := windows.SetFileInformationByHandle(
 		windows.Handle(f.Fd()),
 		windows.FileBasicInfo,
-		(*byte)(unsafe.Pointer(bi)),
-		uint32(unsafe.Sizeof(*bi)),
+		(*byte)(unsafe.Pointer(&biAligned)),
+		uint32(unsafe.Sizeof(biAligned)),
 	); err != nil {
 		return &os.PathError{Op: "SetFileInformationByHandle", Path: f.Name(), Err: err}
 	}
diff --git a/fileinfo_test.go b/fileinfo_test.go
index 2e2e81bc..7dfca6eb 100644
--- a/fileinfo_test.go
+++ b/fileinfo_test.go
@@ -6,6 +6,7 @@ package winio
 import (
 	"os"
 	"testing"
+	"unsafe"
 
 	"golang.org/x/sys/windows"
 )
@@ -131,3 +132,49 @@ func TestGetFileStandardInfo_Directory(t *testing.T) {
 	}
 	checkFileStandardInfo(t, info, expectedFileInfo)
 }
+
+// TestFileInfoStructAlignment checks that the alignment of Go fileinfo structs
+// match what is expected by the Windows API.
+func TestFileInfoStructAlignment(t *testing.T) {
+	//nolint:revive // SNAKE_CASE is not idiomatic in Go, but aligned with Win32 API.
+	const (
+		exampleLARGE_INTEGER uint64 = 0
+		exampleULONGLONG     uint64 = 0
+	)
+	tests := []struct {
+		name              string
+		actualAlign       uintptr
+		actualSize        uintptr
+		winName           string
+		expectedAlignment uintptr
+	}{
+		{
+			// alignedFileBasicInfo is passed to the Windows API rather than FileBasicInfo.
+			"alignedFileBasicInfo", unsafe.Alignof(alignedFileBasicInfo{}), unsafe.Sizeof(alignedFileBasicInfo{}),
+			// https://learn.microsoft.com/en-us/windows/win32/api/winbase/ns-winbase-file_basic_info
+			"FILE_BASIC_INFO", unsafe.Alignof(exampleLARGE_INTEGER),
+		},
+		{
+			"FileStandardInfo", unsafe.Alignof(FileStandardInfo{}), unsafe.Sizeof(FileStandardInfo{}),
+			// https://learn.microsoft.com/en-us/windows/win32/api/winbase/ns-winbase-file_standard_info
+			"FILE_STANDARD_INFO", unsafe.Alignof(exampleLARGE_INTEGER),
+		},
+		{
+			"FileIDInfo", unsafe.Alignof(FileIDInfo{}), unsafe.Sizeof(FileIDInfo{}),
+			// https://learn.microsoft.com/en-us/windows/win32/api/winbase/ns-winbase-file_id_info
+			"FILE_ID_INFO", unsafe.Alignof(exampleULONGLONG),
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if tt.actualAlign != tt.expectedAlignment {
+				t.Errorf("alignment mismatch: actual %d, expected %d", tt.actualAlign, tt.expectedAlignment)
+			}
+			if r := tt.actualSize % tt.expectedAlignment; r != 0 {
+				t.Errorf(
+					"size is not a multiple of alignment: size %% alignment (%d %% %d) is %d, expected 0",
+					tt.actualSize, tt.expectedAlignment, r)
+			}
+		})
+	}
+}