[PATCH V2 2/3] MdePkg/Base.h: Introduce various alignment-related macros


Marvin Häuser
 

ALIGNOF: Determining the alignment requirement of data types is
crucial to ensure safe memory accesses when parsing untrusted data.

IS_POW2: Determining whether a value is a power of two is important
to verify whether untrusted values are valid alignment values.

IS_ALIGNED: In combination with ALIGNOF data offsets can be verified.
A more general version of the IS_ALIGNED macro previously defined by several modules.

ADDRESS_IS_ALIGNED: Variant of IS_ALIGNED for pointers and addresses.
Replaces module-specific definitions throughout the codebase.

ALIGN_VALUE_ADDEND: The addend to align up can be used to directly
determine the required offset for data alignment.

Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Zhiguang Liu <zhiguang.liu@intel.com>
Cc: Vitaly Cheptsov <vit9696@protonmail.com>
Signed-off-by: Marvin Häuser <mhaeuser@posteo.de>
---
MdePkg/Include/Base.h | 90 +++++++++++++++++++-
1 file changed, 89 insertions(+), 1 deletion(-)

diff --git a/MdePkg/Include/Base.h b/MdePkg/Include/Base.h
index 2da08b0c787f..32d0e512e05f 100644
--- a/MdePkg/Include/Base.h
+++ b/MdePkg/Include/Base.h
@@ -789,6 +789,35 @@ typedef UINTN *BASE_LIST;
#define OFFSET_OF(TYPE, Field) ((UINTN) &(((TYPE *)0)->Field))
#endif

+/**
+ Returns the alignment requirement of a type.
+
+ @param TYPE The name of the type to retrieve the alignment requirement of.
+
+ @return Alignment requirement, in Bytes, of TYPE.
+**/
+#if defined(__GNUC__) || defined(__clang__) || (defined(_MSC_VER) && _MSC_VER >= 1900)
+ //
+ // All supported versions of GCC and Clang, as well as MSVC 2015 and later,
+ // support the standard operator _Alignof.
+ //
+ #define ALIGNOF(TYPE) _Alignof (TYPE)
+#elif defined(_MSC_VER)
+ //
+ // Earlier versions of MSVC, at least MSVC 2008 and later, support the
+ // vendor-extension __alignof.
+ //
+ #define ALIGNOF(TYPE) __alignof (TYPE)
+#else
+ //
+ // For compilers that do not support inbuilt alignof operators, use OFFSET_OF.
+ // CHAR8 is known to have both a size and an alignment requirement of 1 Byte.
+ // As such, A must be located exactly at the offset equal to its alignment
+ // requirement.
+ //
+ #define ALIGNOF(TYPE) OFFSET_OF (struct { CHAR8 C; TYPE A; }, A)
+#endif
+
/**
Portable definition for compile time assertions.
Equivalent to C11 static_assert macro from assert.h.
@@ -824,6 +853,21 @@ STATIC_ASSERT (sizeof (CHAR16) == 2, "sizeof (CHAR16) does not meet UEFI Specif
STATIC_ASSERT (sizeof (L'A') == 2, "sizeof (L'A') does not meet UEFI Specification Data Type requirements");
STATIC_ASSERT (sizeof (L"A") == 4, "sizeof (L\"A\") does not meet UEFI Specification Data Type requirements");

+STATIC_ASSERT (ALIGNOF (BOOLEAN) == sizeof (BOOLEAN), "Alignment of BOOLEAN does not meet UEFI Specification Data Type requirements");
+STATIC_ASSERT (ALIGNOF (INT8) == sizeof (INT8), "Alignment of INT8 does not meet UEFI Specification Data Type requirements");
+STATIC_ASSERT (ALIGNOF (UINT8) == sizeof (UINT8), "Alignment of INT16 does not meet UEFI Specification Data Type requirements");
+STATIC_ASSERT (ALIGNOF (INT16) == sizeof (INT16), "Alignment of INT16 does not meet UEFI Specification Data Type requirements");
+STATIC_ASSERT (ALIGNOF (UINT16) == sizeof (UINT16), "Alignment of UINT16 does not meet UEFI Specification Data Type requirements");
+STATIC_ASSERT (ALIGNOF (INT32) == sizeof (INT32), "Alignment of INT32 does not meet UEFI Specification Data Type requirements");
+STATIC_ASSERT (ALIGNOF (UINT32) == sizeof (UINT32), "Alignment of UINT32 does not meet UEFI Specification Data Type requirements");
+STATIC_ASSERT (ALIGNOF (INT64) == sizeof (INT64), "Alignment of INT64 does not meet UEFI Specification Data Type requirements");
+STATIC_ASSERT (ALIGNOF (UINT64) == sizeof (UINT64), "Alignment of UINT64 does not meet UEFI Specification Data Type requirements");
+STATIC_ASSERT (ALIGNOF (CHAR8) == sizeof (CHAR8), "Alignment of CHAR8 does not meet UEFI Specification Data Type requirements");
+STATIC_ASSERT (ALIGNOF (CHAR16) == sizeof (CHAR16), "Alignment of CHAR16 does not meet UEFI Specification Data Type requirements");
+STATIC_ASSERT (ALIGNOF (INTN) == sizeof (INTN), "Alignment of INTN does not meet UEFI Specification Data Type requirements");
+STATIC_ASSERT (ALIGNOF (UINTN) == sizeof (UINTN), "Alignment of UINTN does not meet UEFI Specification Data Type requirements");
+STATIC_ASSERT (ALIGNOF (VOID *) == sizeof (VOID *), "Alignment of VOID * does not meet UEFI Specification Data Type requirements");
+
//
// The following three enum types are used to verify that the compiler
// configuration for enum types is compliant with Section 2.3.1 of the
@@ -847,6 +891,10 @@ STATIC_ASSERT (sizeof (__VERIFY_UINT8_ENUM_SIZE) == 4, "Size of enum does not me
STATIC_ASSERT (sizeof (__VERIFY_UINT16_ENUM_SIZE) == 4, "Size of enum does not meet UEFI Specification Data Type requirements");
STATIC_ASSERT (sizeof (__VERIFY_UINT32_ENUM_SIZE) == 4, "Size of enum does not meet UEFI Specification Data Type requirements");

+STATIC_ASSERT (ALIGNOF (__VERIFY_UINT8_ENUM_SIZE) == sizeof (__VERIFY_UINT8_ENUM_SIZE), "Alignment of enum does not meet UEFI Specification Data Type requirements");
+STATIC_ASSERT (ALIGNOF (__VERIFY_UINT16_ENUM_SIZE) == sizeof (__VERIFY_UINT16_ENUM_SIZE), "Alignment of enum does not meet UEFI Specification Data Type requirements");
+STATIC_ASSERT (ALIGNOF (__VERIFY_UINT32_ENUM_SIZE) == sizeof (__VERIFY_UINT32_ENUM_SIZE), "Alignment of enum does not meet UEFI Specification Data Type requirements");
+
/**
Macro that returns a pointer to the data structure that contains a specified field of
that data structure. This is a lightweight method to hide information by placing a
@@ -868,6 +916,46 @@ STATIC_ASSERT (sizeof (__VERIFY_UINT32_ENUM_SIZE) == 4, "Size of enum does not m
**/
#define BASE_CR(Record, TYPE, Field) ((TYPE *) ((CHAR8 *) (Record) - OFFSET_OF (TYPE, Field)))

+/**
+ Checks whether a value is a power of two.
+
+ @param Value The value to check.
+
+ @return Whether Value is a power of two.
+**/
+#define IS_POW2(Value) ((Value) != 0U && ((Value) & ((Value) - 1U)) == 0U)
+
+/**
+ Checks whether a value is aligned by a specified alignment.
+
+ @param Value The value to check.
+ @param Alignment The alignment boundary used to check against.
+
+ @return Whether Value is aligned by Alignment.
+**/
+#define IS_ALIGNED(Value, Alignment) (((Value) & ((Alignment) - 1U)) == 0U)
+
+/**
+ Checks whether a pointer or address is aligned by a specified alignment.
+
+ @param Address The pointer or address to check.
+ @param Alignment The alignment boundary used to check against.
+
+ @return Whether Address is aligned by Alignment.
+**/
+#define ADDRESS_IS_ALIGNED(Address, Alignment) IS_ALIGNED ((UINTN) (Address), Alignment)
+
+/**
+ Determines the addend to add to a value to round it up to the next boundary of
+ a specified alignment.
+
+ @param Value The value to round up.
+ @param Alignment The alignment boundary used to return the addend.
+
+ @return Addend to round Value up to alignment boundary Alignment.
+**/
+#define ALIGN_VALUE_ADDEND(Value, Alignment) (((Alignment) - (Value)) & ((Alignment) - 1U))
+
/**
Rounds a value up to the next boundary using a specified alignment.

@@ -880,7 +968,7 @@ STATIC_ASSERT (sizeof (__VERIFY_UINT32_ENUM_SIZE) == 4, "Size of enum does not m
@return A value up to the next boundary.

**/
-#define ALIGN_VALUE(Value, Alignment) ((Value) + (((Alignment) - (Value)) & ((Alignment) - 1)))
+#define ALIGN_VALUE(Value, Alignment) ((Value) + ALIGN_VALUE_ADDEND (Value, Alignment))

/**
Adjust a pointer by adding the minimum offset required for it to be aligned on
--
2.31.1


Ni, Ray
 

Marvin,
So lucky to have you in the edk2 project looking into these fundamentals!

+ #define ALIGNOF(TYPE) OFFSET_OF (struct { CHAR8 C; TYPE A; }, A)

1. Does struct{} inside a macro conform to C standard? How is the compatibility with different compilers?

+#define IS_POW2(Value) ((Value) != 0U && ((Value) & ((Value) - 1U)) ==
+0U)

2. Good to me. I learned this trick when implementing the MtrrLib.

+#define ALIGN_VALUE_ADDEND(Value, Alignment) (((Alignment) - (Value))
+& ((Alignment) - 1U))

3. Is any other open source project using the same macro for the addend?
This is actually a general question to all new macros.
I would like the macros look familiar to developers from other open source projects.


Marvin Häuser
 

Hey Ray,

On 16/08/2021 11:42, Ni, Ray wrote:
Marvin,
So lucky to have you in the edk2 project looking into these fundamentals!
Thank you. :)

+ #define ALIGNOF(TYPE) OFFSET_OF (struct { CHAR8 C; TYPE A; }, A)

1. Does struct{} inside a macro conform to C standard? How is the compatibility with different compilers?
This should work, yes. The C standard defines offsetof as such:

"The macros are [...]

        offsetof(type, member-designator)

which expands to an integer constant expression that has type size_t, the value of
which is the offset in bytes, to the structure member (designated by member-designator),
from the beginning of its structure (designated by type). The type and member designator
shall be such that given

        static type t;

then the expression &(t.member-designator) evaluates to an address constant. [...]" [1]

If we plug in t:

        static struct { CHAR8 C; TYPE A; } t;

we get a valid static storage duration variable declaration that satisfies the the last condition because:

"An address constant is [...], a pointer to an lvalue designating an object of static
storage duration, or [...]" [2]

It worked with all compilers I tinkered with at https://godbolt.org/
I sadly do not have access to any of the compilers where this may be used effectively (RVCT, EBC).

+#define IS_POW2(Value) ((Value) != 0U && ((Value) & ((Value) - 1U)) ==
+0U)

2. Good to me. I learned this trick when implementing the MtrrLib.

+#define ALIGN_VALUE_ADDEND(Value, Alignment) (((Alignment) - (Value))
+& ((Alignment) - 1U))

3. Is any other open source project using the same macro for the addend?
This is actually a general question to all new macros.
I would like the macros look familiar to developers from other open source projects.
Good question, I never really saw it. I only came up with it because for the new PE loader, we may align the PE memory within an underaligned buffer, and for that we need the addend. I initially used to align up and then subtract, but I saw this could be simplified with ALIGN_VALUE_ADDEND, which was used in ALIGN_VALUE anyway. If you have a better name, I'll change it.

Best regards,
Marvin


[1] ISO/IEC 9899:2011, 7.19, 3.

[2] ISO/IEC 9899:2011, 6.6, 9.





Ni, Ray
 

I don't have better names.

Reviewed-by: Ray Ni <ray.ni@intel.com>

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Marvin Häuser
Sent: Monday, August 16, 2021 9:10 PM
To: devel@edk2.groups.io; Ni, Ray <ray.ni@intel.com>
Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Liming Gao <gaoliming@byosoft.com.cn>; Liu, Zhiguang
<zhiguang.liu@intel.com>; Vitaly Cheptsov <vit9696@protonmail.com>
Subject: Re: [edk2-devel] [PATCH V2 2/3] MdePkg/Base.h: Introduce various alignment-related macros

Hey Ray,

On 16/08/2021 11:42, Ni, Ray wrote:
Marvin,
So lucky to have you in the edk2 project looking into these fundamentals!
Thank you. :)

+ #define ALIGNOF(TYPE) OFFSET_OF (struct { CHAR8 C; TYPE A; }, A)

1. Does struct{} inside a macro conform to C standard? How is the compatibility with different compilers?
This should work, yes. The C standard defines offsetof as such:

"The macros are [...]

        offsetof(type, member-designator)

which expands to an integer constant expression that has type size_t,
the value of
which is the offset in bytes, to the structure member (designated by
member-designator),
from the beginning of its structure (designated by type). The type and
member designator
shall be such that given

        static type t;

then the expression &(t.member-designator) evaluates to an address
constant. [...]" [1]

If we plug in t:

        static struct { CHAR8 C; TYPE A; } t;

we get a valid static storage duration variable declaration that
satisfies the the last condition because:

"An address constant is [...], a pointer to an lvalue designating an
object of static
storage duration, or [...]" [2]

It worked with all compilers I tinkered with at https://godbolt.org/
I sadly do not have access to any of the compilers where this may be
used effectively (RVCT, EBC).

+#define IS_POW2(Value) ((Value) != 0U && ((Value) & ((Value) - 1U)) ==
+0U)

2. Good to me. I learned this trick when implementing the MtrrLib.

+#define ALIGN_VALUE_ADDEND(Value, Alignment) (((Alignment) - (Value))
+& ((Alignment) - 1U))

3. Is any other open source project using the same macro for the addend?
This is actually a general question to all new macros.
I would like the macros look familiar to developers from other open source projects.
Good question, I never really saw it. I only came up with it because for
the new PE loader, we may align the PE memory within an underaligned
buffer, and for that we need the addend. I initially used to align up
and then subtract, but I saw this could be simplified with
ALIGN_VALUE_ADDEND, which was used in ALIGN_VALUE anyway. If you have a
better name, I'll change it.

Best regards,
Marvin


[1] ISO/IEC 9899:2011, 7.19, 3.

[2] ISO/IEC 9899:2011, 6.6, 9.









Marvin Häuser
 

Ping? :)

On 16.08.21 15:10, Marvin Häuser wrote:
Hey Ray,

On 16/08/2021 11:42, Ni, Ray wrote:
Marvin,
So lucky to have you in the edk2 project looking into these fundamentals!
Thank you. :)

+  #define ALIGNOF(TYPE) OFFSET_OF (struct { CHAR8 C; TYPE A; }, A)

1. Does struct{} inside a macro conform to C standard? How is the compatibility with different compilers?
This should work, yes. The C standard defines offsetof as such:

"The macros are [...]

        offsetof(type, member-designator)

which expands to an integer constant expression that has type size_t, the value of
which is the offset in bytes, to the structure member (designated by member-designator),
from the beginning of its structure (designated by type). The type and member designator
shall be such that given

        static type t;

then the expression &(t.member-designator) evaluates to an address constant. [...]" [1]

If we plug in t:

        static struct { CHAR8 C; TYPE A; } t;

we get a valid static storage duration variable declaration that satisfies the the last condition because:

"An address constant is [...], a pointer to an lvalue designating an object of static
storage duration, or [...]" [2]

It worked with all compilers I tinkered with at https://godbolt.org/
I sadly do not have access to any of the compilers where this may be used effectively (RVCT, EBC).

+#define IS_POW2(Value)  ((Value) != 0U && ((Value) & ((Value) - 1U)) ==
+0U)

2. Good to me. I learned this trick when implementing the MtrrLib.

+#define ALIGN_VALUE_ADDEND(Value, Alignment)  (((Alignment) - (Value))
+& ((Alignment) - 1U))

3. Is any other open source project using the same macro for the addend?
This is actually a general question to all new macros.
I would like the macros look familiar to developers from other open source projects.
Good question, I never really saw it. I only came up with it because for the new PE loader, we may align the PE memory within an underaligned buffer, and for that we need the addend. I initially used to align up and then subtract, but I saw this could be simplified with ALIGN_VALUE_ADDEND, which was used in ALIGN_VALUE anyway. If you have a better name, I'll change it.

Best regards,
Marvin


[1] ISO/IEC 9899:2011, 7.19, 3.

[2] ISO/IEC 9899:2011, 6.6, 9.





Marvin Häuser
 

Hey Mike,

Thanks! I agree using "offset" may make it more readable, but I haven't seen it being used much outside of memory terminology (the macro also applies to plain integers). Any feedback from the maintainers for preferences? Thanks!

Best regards,
Marvin

On 08.12.21 10:10, mjsbeaton@gmail.com wrote:
Can I tentatively suggest ALIGN_VALUE_OFFSET as a better name? (Speaking as a native English speaker, and with a relatively high level of command including technical language, addend is not at all a common word!) While I'm here additional ping and +1 for this to be merged, pls!


mjsbeaton@...
 

Can I tentatively suggest ALIGN_VALUE_OFFSET as a better name? (Speaking as a native English speaker, and with a relatively high level of command including technical language, addend is not at all a common word!)

While I'm here additional ping and +1 for this to be merged, pls!