[patch] MdeModulePkg/DriverSampleDxe: Make bit fields aligned in C structure

[patch] MdeModulePkg/DriverSampleDxe: Make bit fields aligned in C structure

[Dandan Bi]

For a structure with a series of bit fields and used as a storage
in vfr file, and if the bit fields do not add up to the size of
the defined type.In the C code use sizeof() to get size of the
structure, the results may vary form the compiler(VS,GCC...).
But the size of the storage calculated by VfrCompiler is fixed
(calculate with alignment).To avoid the issue cased by above case,
we need to make the total width of the bit fields in the structure
aligned with the size of the defined type for these bit fields.

Cc: Eric Dong <eric.dong@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Dandan Bi <dandan.bi@intel.com>
---
 MdeModulePkg/Universal/DriverSampleDxe/NVDataStruc.h | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/MdeModulePkg/Universal/DriverSampleDxe/NVDataStruc.h b/MdeModulePkg/Universal/DriverSampleDxe/NVDataStruc.h
index 40fb3d0..af3d4bc 100644
--- a/MdeModulePkg/Universal/DriverSampleDxe/NVDataStruc.h
+++ b/MdeModulePkg/Universal/DriverSampleDxe/NVDataStruc.h
@@ -33,10 +33,18 @@ Revision History:
 #define CONFIGURATION_VARSTORE_ID    0x1234
 #define BITS_VARSTORE_ID             0x2345
 
 #pragma pack(1)
 
+//
+// !!! For a structure with a series of bit fields and used as a storage in vfr file, and if the bit fields do not add up to the size of the defined type.
+// In the C code use sizeof() to get the size the strucure, the results may vary form the compiler(VS,GCC...).
+// But the size of the storage calculated by VfrCompiler is fixed (calculate with alignment).
+// To avoid above case, we need to make the total bit width in the structure aligned with the size of the defined type for these bit fields. We can:
+// 1. Add bit field (with/without name) with remianing with for padding.
+// 2. Add unnamed bit field with 0 for padding, the amount of padding is determined by the alignment characteristics of the members of the structure.
+//
 typedef struct {
   UINT16   NestByteField;
   UINT8                    : 1;  // unamed field can be used for padding
   UINT8    NestBitCheckbox : 1;
   UINT8    NestBitOneof    : 2;
@@ -82,11 +90,13 @@ typedef struct {
   EFI_HII_TIME  Time;
   UINT8   RefreshGuidCount;
   UINT8   Match2;
   UINT8   GetDefaultValueFromCallBackForOrderedList[3];
   UINT8   BitCheckbox : 1;
+  UINT8   ReservedBits: 7;  // Reserved bit fields for padding.
   UINT16  BitOneof    : 6;
+  UINT16              : 0;  // Width 0 used to force alignment.
   UINT16  BitNumeric  : 12;
   MY_BITS_DATA  MyBitData;
   MY_EFI_UNION_DATA MyUnionData;
 } DRIVER_SAMPLE_CONFIGURATION;
 
@@ -107,10 +117,11 @@ typedef struct {
   MY_BITS_DATA  BitsData;
   UINT32   EfiBitGrayoutTest : 5;
   UINT32   EfiBitNumeric     : 4;
   UINT32   EfiBitOneof       : 10;
   UINT32   EfiBitCheckbox    : 1;
+  UINT32                     : 0;  // Width 0 used to force alignment.
 } MY_EFI_BITS_VARSTORE_DATA;
 
 //
 // Labels definition
 //
-- 
1.9.5.msysgit.1

Re: [patch] MdeModulePkg/DriverSampleDxe: Make bit fields aligned in C structure

[Gao, Liming]

Reviewed-by: Liming Gao <liming.gao@intel.com>

>-----Original Message-----
>From: Bi, Dandan
>Sent: Thursday, February 08, 2018 10:29 PM
>To: edk2-devel@lists.01.org
>Cc: Dong, Eric <eric.dong@intel.com>; Gao, Liming <liming.gao@intel.com>
>Subject: [patch] MdeModulePkg/DriverSampleDxe: Make bit fields aligned in
>C structure
>
>For a structure with a series of bit fields and used as a storage
>in vfr file, and if the bit fields do not add up to the size of
>the defined type.In the C code use sizeof() to get size of the
>structure, the results may vary form the compiler(VS,GCC...).
>But the size of the storage calculated by VfrCompiler is fixed
>(calculate with alignment).To avoid the issue cased by above case,
>we need to make the total width of the bit fields in the structure
>aligned with the size of the defined type for these bit fields.
>
>Cc: Eric Dong <eric.dong@intel.com>
>Cc: Liming Gao <liming.gao@intel.com>
>Contributed-under: TianoCore Contribution Agreement 1.1
>Signed-off-by: Dandan Bi <dandan.bi@intel.com>
>---
> MdeModulePkg/Universal/DriverSampleDxe/NVDataStruc.h | 11
>+++++++++++
> 1 file changed, 11 insertions(+)
>
>diff --git a/MdeModulePkg/Universal/DriverSampleDxe/NVDataStruc.h
>b/MdeModulePkg/Universal/DriverSampleDxe/NVDataStruc.h
>index 40fb3d0..af3d4bc 100644
>--- a/MdeModulePkg/Universal/DriverSampleDxe/NVDataStruc.h
>+++ b/MdeModulePkg/Universal/DriverSampleDxe/NVDataStruc.h
>@@ -33,10 +33,18 @@ Revision History:
> #define CONFIGURATION_VARSTORE_ID    0x1234
> #define BITS_VARSTORE_ID             0x2345
>
> #pragma pack(1)
>
>+//
>+// !!! For a structure with a series of bit fields and used as a storage in vfr file,
>and if the bit fields do not add up to the size of the defined type.
>+// In the C code use sizeof() to get the size the strucure, the results may vary
>form the compiler(VS,GCC...).
>+// But the size of the storage calculated by VfrCompiler is fixed (calculate
>with alignment).
>+// To avoid above case, we need to make the total bit width in the structure
>aligned with the size of the defined type for these bit fields. We can:
>+// 1. Add bit field (with/without name) with remianing with for padding.
>+// 2. Add unnamed bit field with 0 for padding, the amount of padding is
>determined by the alignment characteristics of the members of the structure.
>+//
> typedef struct {
>   UINT16   NestByteField;
>   UINT8                    : 1;  // unamed field can be used for padding
>   UINT8    NestBitCheckbox : 1;
>   UINT8    NestBitOneof    : 2;
>@@ -82,11 +90,13 @@ typedef struct {
>   EFI_HII_TIME  Time;
>   UINT8   RefreshGuidCount;
>   UINT8   Match2;
>   UINT8   GetDefaultValueFromCallBackForOrderedList[3];
>   UINT8   BitCheckbox : 1;
>+  UINT8   ReservedBits: 7;  // Reserved bit fields for padding.
>   UINT16  BitOneof    : 6;
>+  UINT16              : 0;  // Width 0 used to force alignment.
>   UINT16  BitNumeric  : 12;
>   MY_BITS_DATA  MyBitData;
>   MY_EFI_UNION_DATA MyUnionData;
> } DRIVER_SAMPLE_CONFIGURATION;
>
>@@ -107,10 +117,11 @@ typedef struct {
>   MY_BITS_DATA  BitsData;
>   UINT32   EfiBitGrayoutTest : 5;
>   UINT32   EfiBitNumeric     : 4;
>   UINT32   EfiBitOneof       : 10;
>   UINT32   EfiBitCheckbox    : 1;
>+  UINT32                     : 0;  // Width 0 used to force alignment.
> } MY_EFI_BITS_VARSTORE_DATA;
>
> //
> // Labels definition
> //
>--
>1.9.5.msysgit.1