On 4/25/25 11:11, Jan Beulich wrote:
On 24.04.2025 16:04, Andrew Cooper wrote:
I have a sneaking suspicion that this is sufficient:
diff --git a/xen/arch/x86/x86_emulate/private.h
b/xen/arch/x86/x86_emulate/private.h
index 30be59547032..9f3d6f0e5357 100644
--- a/xen/arch/x86/x86_emulate/private.h
+++ b/xen/arch/x86/x86_emulate/private.h
@@ -385,9 +385,9 @@ struct x87_env16 {
union {
struct {
uint16_t fip_lo;
- uint16_t fop:11, :1, fip_hi:4;
+ uint32_t fop:11, :1, fip_hi:4;
uint16_t fdp_lo;
- uint16_t :12, fdp_hi:4;
+ uint32_t :12, fdp_hi:4;
} real;
struct {
uint16_t fip;
The problem is that a uint16_t bitfield promotes into int. A base type
of uint32_t should cause the bitfield to promote into unsigned int directly.
I fear that's not gcc's way of thinking. In fact, the other involved structure
already uses bitfields with uint32_t base type, and the issue was reported
there nevertheless. Having known only compilers which respect declared type of
bitfields, I was rather surprised by gcc not doing so when I first started
using it on not exactly trivial code. Just to learn that they are free to do
so. Looks like I never dared to submit a patch I've been carrying to optionally
alter that behavior.
So I took some time to play around with this and you're definitely right
about GCC not respecting the declared type. Even in the struct
`x87_env32`, where the types are already declared as `uint32_t`, GCC
will just pick `short unsigned int` as the type for a 16-bit wide
bit-field. As such, in the offending left-shift expression the bit-field
will be promoted to an `int`, thereby causing the observed UB.
I could not find a way to make GCC actually pick a correctly sized
unsigned type in this context, without also modifying the width of the
bit-field. So I'm relatively sure the only way to fix this is to
properly adjust the usages of these bit-fields (as was suggested
previously).
For completeness sake I also checked how Clang deals with this issue and
the UB manifests in the same manner.
What's worse is that I didn't find any proper documentation on this
behavior for neither GCC or Clang. If you have anything I would be very
interested to know.
Best,
Manuel
