From 59a057980d9b6570ee5652ba19190ced0f78a28a Mon Sep 17 00:00:00 2001
From: Jamie Liu <jamieliu@google.com>
Date: Thu, 8 Feb 2024 23:07:46 -0800
Subject: [PATCH] Minor FPU save/restore fixes.

- Use the correct instruction in safecopy.checkXstate(). Before this CL:

```
TEXT pkg/sentry/arch/fpu/fpu.initX86FPState.abi0(SB)
  ...
  fpu_amd64.s:78        0x7661b2                480fae2f                XRSTOR64 0(DI)

TEXT pkg/safecopy/safecopy.checkXstate.abi0(SB)
  ...
  xrstor_amd64.s:54     0x7648a0                0fae2f                  XRSTOR 0(DI)
```

I'm not sure what the actual difference between XRSTOR and XRSTOR64 is, but
Linux is careful to use XRSTOR64 (arch/x86/kernel/fpu/xstate.h:XRSTOR,
REX_PREFIX) so it probably matters.

- When an AfterLoad callback fails, log the error message before the failing
  object, since the latter can be huge and prevent the error message from being
  logged.

- Include additional information in the error message emitted by
  fpu.State.AfterLoad().

PiperOrigin-RevId: 605534648
---
 pkg/safecopy/xrstor_amd64.s      | 2 +-
 pkg/sentry/arch/fpu/fpu_amd64.go | 7 ++++++-
 pkg/state/decode.go              | 2 +-
 3 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/pkg/safecopy/xrstor_amd64.s b/pkg/safecopy/xrstor_amd64.s
index 94da2b705..6c81d1b9a 100644
--- a/pkg/safecopy/xrstor_amd64.s
+++ b/pkg/safecopy/xrstor_amd64.s
@@ -44,7 +44,7 @@ TEXT ·checkXstate(SB),NOSPLIT|NOFRAME,$0-26
 	MOVQ addr+0(FP), DI
 	MOVL $0xffffffff, AX
 	MOVL $0xffffffff, DX
-	XRSTOR (DI)
+	XRSTOR64 (DI)
 
 	// Restore MXCSR and the x87 control word.
 	LDMXCSR	mxcsr+20(FP)
diff --git a/pkg/sentry/arch/fpu/fpu_amd64.go b/pkg/sentry/arch/fpu/fpu_amd64.go
index a7e4bf3c3..7c6a10f51 100644
--- a/pkg/sentry/arch/fpu/fpu_amd64.go
+++ b/pkg/sentry/arch/fpu/fpu_amd64.go
@@ -199,6 +199,7 @@ const (
 	// xstateBVOffset is the offset in bytes of the XSTATE_BV field in an x86
 	// XSAVE area.
 	xstateBVOffset = 512
+	xcompBVOffset  = 520
 
 	// xsaveHeaderZeroedOffset and xsaveHeaderZeroedBytes indicate parts of the
 	// XSAVE header that we coerce to zero: "Bytes 15:8 of the XSAVE header is
@@ -381,7 +382,11 @@ func (s *State) AfterLoad() {
 	}
 	if hostFeatureSet.UseXsave() {
 		if err := safecopy.CheckXstate(s.BytePointer()); err != nil {
-			panic(fmt.Sprintf("incompatible state: %s (%#v)", err, *s))
+			xcompBV := uint64(0)
+			if len(old) >= xcompBVOffset+8 {
+				xcompBV = hostarch.ByteOrder.Uint64(old[xcompBVOffset:])
+			}
+			panic(fmt.Sprintf("incompatible state: %s\nlen(old)=%d len(new)=%d supportedBV=%#x XSTATE_BV=%#x XCOMP_BV=%#x", err, len(old), len(*s), supportedBV, savedBV, xcompBV))
 		}
 	}
 }
diff --git a/pkg/state/decode.go b/pkg/state/decode.go
index ea08923f4..eb826e60a 100644
--- a/pkg/state/decode.go
+++ b/pkg/state/decode.go
@@ -691,7 +691,7 @@ func (ds *decodeState) Load(obj reflect.Value) {
 			}
 		}
 	}); err != nil {
-		Failf("error executing callbacks for %#v: %w", ods.obj.Interface(), err)
+		Failf("error executing callbacks: %w\nfor object %#v", err, ods.obj.Interface())
 	}
 
 	// Check if we have any remaining dependency cycles. If there are any