Re: [edk2-devel] [PATCH v2 1/2] MdePkg/BaseLib: Fix unaligned API prototypes

["Marvin Häuser" <mhaeuser@posteo.de>]

On 10/08/2021 19:36, Andrew Fish via groups.io wrote:
>
>
>> On Aug 10, 2021, at 1:53 AM, Marvin Häuser <mhaeuser@posteo.de 
>> <mailto:mhaeuser@posteo.de>> wrote:
>>
>> On 09/08/2021 23:32, Andrew Fish viagroups.io <http://groups.io/>wrote:
>>>
>>>
>>>> On Aug 9, 2021, at 9:15 AM, Michael D Kinney 
>>>> <michael.d.kinney@intel.com 
>>>> <mailto:michael.d.kinney@intel.com><mailto:michael.d.kinney@intel.com 
>>>> <mailto:michael.d.kinney@intel.com>>> wrote:
>>>>
>>>> Hi Marvin,
>>>>
>>>> Can you provide an example of which C compiler is flagging this as
>>>> an error and what error message is generated.
>>>>
>>>> Please enter a BZ with this background information and add link to the
>>>> BZ in the commit message.
>>>>
>>>> This is a change to the BaseLib class, so we need to make sure there
>>>> are no impacts to any existing code.  I looks like a safe change
>>>> because changing from a pointer to a fixed size type to VOID *
>>>> should be compatible.  Please add that analysis to the background
>>>> in the BZ as well.
>>>>
>>>
>>> MIke,
>>>
>>> I want to say we had a discussion about this years ago? I don’t 
>>> remember the outcome.
>>>
>>> Dereferencing a misaligned pointer is UB (Undefined Behavior) in C 
>>> [1], but historically x86 compilers have let it slide.
>>>
>>> I think the situation we are in is the BaseLib functions don’t 
>>> contain UB, but it is UB for the caller to use the returned pointer 
>>> directly.
>>
>> They do contain UB, inherently from the combination of their 
>> prototype and their usage.
>> Please refer to the new BZ added for 
>> V2:https://bugzilla.tianocore.org/show_bug.cgi?id=3542 
>> <https://bugzilla.tianocore.org/show_bug.cgi?id=3542>
>>
>
> I think we are saying the same thing.

OK, sorry, misunderstood then.

> My point is it is possible to use the library API correctly, but 
> likely no one does. Changing the API to a VOID also does not fix every 
> issue, actually it is a drop in to existing code so it does not really 
> force any fixes...

Yes, but at least it does not *force* the caller to do UB casts.
I actually have fixed everything I found here: 
https://github.com/mhaeuser/edk2/commit/7266c31f04634bf3e1861e84f9be9983a6084f3c
Just I thought I could not possibly defend such a huge patch for an 
issue nobody encounters and likely never will either. If you want the 
patch, you totally can have it, just I did not want to increase the 
burden for no reason. I'm fine if future code does it right, but also to 
polish and submit above patch.

>
>> I could only speculate why UBsan does not check cast safety...
>> To be fair, I don't know a real-world platform that has issues with 
>> this UB, but the fix is simple enough in my opinion.
>>
>
> Cast is compile time UB, not runtime. UBSan is doing runtime checks. 
> You can catch compile time UB with warnings. I've have had way too 
> many talks with the clang folks about UB and firmware :).

Right, that would have been a guess. One could make a point though that 
it is RT UB, because UB is only invoked if the address is actually not 
aligned, the compiler not having a proof is not enough. Thanks for the 
insight!

>
>> Actually, enabling "-Wcast-align" some day would be great either way. :)
>>
>
> In my example this gets you past the -Wcast-align. We have the same 
> type of casts for the size of pointers and casting through UINTN.
> UINT16 *pointer = (UINT16 *)(void *)(buffer+1);

Yes, but this looks much more ominous than without the void cast. I 
sometimes experiment with code style a bit in my own private code, and 
omitting void pointers as far as possible (exceptions for cases as 
above) has been one of those experiments. I don't think one can nail C 
code style perfectly, it's always a game of trade-offs.

>
> Actually I’m starting to remember why we chose not to fix this before. 
> It does not really help…. Notice if we change the API to use `VOID *` 
> vs. `UINT16 *` nothing really changed [1] ?  It is still up to the 
> caller to do the right thing.

Yes-ish, this is not quite a functional fix, it's about guarantees. If 
the functions take UINT16 pointers, they request a guarantee from every 
caller, with no docs required, that the pointers must be aligned for 
this type. It's a guarantee for the functional implementation, but more 
importantly a guarantee for the compiler. If we change it to a VOID 
pointer, it will basically be no-op for the binary, as no compiler 
really makes such assumptions at cast-time (to my knowledge), but the 
guarantees were shifted. Now, if the function wants an alignment 
guarantee from the caller, it has to define it in the docs. No such 
exists, so the caller knows that the input pointer does not impose any 
alignment constraints, and the compiler knows it cannot assume any such.

> The real issue is the functions are not really useful given strict UB 
> in C….
>
> A better solution is to just tell the compiler this pointer is not 
> aligned, and at that point you DON'T need the lib functions….
>
> This code passes -Wcast-align and -fsanitize=undefined runtime checks:
> typedef UINT16 UUINT16 __attribute__ ((aligned (1)));
>
> int main()
> {
> UINT8 *buffer = malloc(64);
> UUINT16 *pointer = (UUINT16 *)(buffer+1);
> *pointer = 42;
> return *pointer;
> }
>
> So a better solution maybe to defined unaligned basic types? I’m not 
> sure if __declspec(align(1)) does the right thing for VC++, 
> __attribute__ ((aligned (1))); does the right thing for GCC and clang. 
> Assuming this works we could defined unaligned version of our basic 
> types, and over time obsolete the current alignment lib functions. We 
> could start with a warning comment to use the unaligned types vs. 
> current library functions.
>
> I’m thinking for the types in ProcessorBind.h[2] and Base.h[3] we 
> could have a byte aligned versions. For example given UINT16 we add 
> UUINT16 to Base.h. I guess we could also  look into a #define 
> abstraction for ` __attribute__ ((aligned (1)))` so new could could 
> use that in a portable way?
>
> Just to be clear if we define UUINT16 then the compiler will do the 
> the work of WriteUnaligned16() in the code generation, if it is needed 
> by the CPU arch. Given that it is much harder for the coder to “mess 
> it up”.

Well, the caller code can cast to aligned types over unaligned types 
just as well with this concept. But I like it just for the sake of 
leveraging compiler code over providing own implementations. At the very 
least for packed structures they sound great. Clang added 
"-Waddress-of-packed-member" at some point, and unaligned base types 
allow safe (and warning-free) address retrieval.

>
> Actually I changed my mind I think we will want to keep the Unaligned 
> library functions, but update them to use the unaligned pointers. The 
> reason we need the function is the compiler is generating code that 
> follow the rules of the CPU, so we need the library functions to talk 
> to hardware devices that have stricter alignment rules than the CPU. 
> For example on x86 UUINT16 will end up being a word operation in 
> assembler. This is the reason the current lib functions use volatile. 
> Sorry figuring this out as I type. So looks like we need to add 
> unaligned types and update the libs to use those types? For memory 
> operations you can use the new unaligned types, and for hardware 
> access use the lib functions.

In this case I would probably rebrand them (docs are enough) to MMIO 
functions, and encourage new code to use the unaligned pointer types 
directly? This may actually provide optimisation advantages due to lack 
of volatile.

Thanks for the detailed notes!

Best regards,
Marvin

>
> [1] ~/work/Compiler>cat ub.c
> #include <stdlib.h>
>
> #define EFIAPI
> #define IN
> #define OUT
> #define VOID void
>
> typedef unsigned char UINT8;
> typedef unsigned short UINT16;
>
> UINT16
> EFIAPI
> WriteUnaligned16 (
>   OUT VOID                      *Buffer,
>   IN  UINT16                    Value
>   )
> {
>   // ASSERT (Buffer != NULL);
>
>   ((volatile UINT8*)Buffer)[0] = (UINT8)Value;
>   ((volatile UINT8*)Buffer)[1] = (UINT8)(Value >> 8);
>
>   return Value;
> }
>
>
> int main()
> {
> UINT8 *buffer = malloc(64);
> UINT16 *pointer = (UINT16 *)(void *)(buffer+1);
>
>
> WriteUnaligned16 (pointer, 42);
>
>
> // *pointer = 42; // Error: misaligned integer pointer assignment
> return *pointer;
> }
> ~/work/Compiler>clang -fsanitize=undefined -Wcast-align  ub.c
> ~/work/Compiler>./a.out
> *ub.c:35:9:**runtime error: **load of misaligned address 
> 0x7fdd9d405aa1 for type 'UINT16' (aka 'unsigned short'), which 
> requires 2 byte alignment*
> *0x7fdd9d405aa1: note: *pointer points here
>  00 00 00  64 2a 00 79 6d 28 52 54  4c 44 5f 44 45 46 41 55  4c 54 2c 
> 20 73 77 69 66  74 5f 64 65 6d
> *              ^ *
> SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior ub.c:35:9 in
> ~/work/Compiler>
>
>
> [2] 
> https://github.com/tianocore/edk2/blob/master/MdePkg/Include/X64/ProcessorBind.h#L127 
> <https://github.com/tianocore/edk2/blob/master/MdePkg/Include/X64/ProcessorBind.h#L127> 
>
> [3] 
> https://github.com/tianocore/edk2/blob/master/MdePkg/Include/Base.h 
> <https://github.com/tianocore/edk2/blob/master/MdePkg/Include/Base.h>
>
> Thanks,
>
> Andrew Fish
>
>> Best regards,
>> Marvin
>>
>>>
>>> Here is a simple example with clang UndefinedBehaviorSanitizer (UBSan) .
>>>
>>> ~/work/Compiler>cat ub.c
>>> #include <stdlib.h>
>>>
>>> #define EFIAPI
>>> #define IN
>>> #define OUT
>>>
>>> typedef unsigned char UINT8;
>>> typedef unsigned short UINT16;
>>>
>>> UINT16
>>> EFIAPI
>>> WriteUnaligned16 (
>>>   OUT UINT16                    *Buffer,
>>>   IN  UINT16                    Value
>>>   )
>>> {
>>>   // ASSERT (Buffer != NULL);
>>>
>>>   ((volatile UINT8*)Buffer)[0] = (UINT8)Value;
>>>   ((volatile UINT8*)Buffer)[1] = (UINT8)(Value >> 8);
>>>
>>>   return Value;
>>> }
>>>
>>>
>>> int main()
>>> {
>>> UINT8 *buffer = malloc(64);
>>> UINT16 *pointer = (UINT16 *)(buffer + 1);
>>>
>>>
>>> WriteUnaligned16 (pointer, 42);
>>>
>>>
>>> // *pointer = 42; // Error: misaligned integer pointer assignment
>>> return *pointer;
>>> }
>>> ~/work/Compiler>clang -fsanitize=undefined  ub.c
>>> ~/work/Compiler>./a.out
>>> *ub.c:34:9:**runtime error: **load of misaligned address 
>>> 0x7feac6405aa1 for type 'UINT16' (aka 'unsigned short'), which 
>>> requires 2 byte alignment*
>>> *0x7feac6405aa1: note: *pointer points here
>>>  00 00 00  64 2a 00 79 6d 28 52 54  4c 44 5f 44 45 46 41 55  4c 54 
>>> 2c 20 73 77 69 66  74 5f 64 65 6d
>>> *              ^ *
>>> SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior ub.c:34:9 in
>>>
>>> FYI line 39 is `return *pointer`and 42 is 0x2A. So reading an 
>>> writing to *pointer is UB.
>>>
>>>
>>> As you can see in [1] the general advice is to take code that looks 
>>> like:
>>> int8_t *buffer = malloc(64);int32_t *pointer = (int32_t *)(buffer + 
>>> 1);*pointer = 42; // Error: misaligned integer pointer assignment
>>> And replace it with;
>>> int8_t *buffer = malloc(64);int32_t value = 42;memcpy(buffer + 1, 
>>> &value, sizeof(int32_t)); // Correct
>>>
>>> But in these cases the result is in a byte aligned buffer….
>>>
>>> [1]https://developer.apple.com/documentation/xcode/misaligned-pointer 
>>> <https://developer.apple.com/documentation/xcode/misaligned-pointer><https://developer.apple.com/documentation/xcode/misaligned-pointer 
>>> <https://developer.apple.com/documentation/xcode/misaligned-pointer>>
>>>
>>> Thanks,
>>>
>>> Andrew Fish
>>>
>>>> Thanks,
>>>>
>>>> Mike
>>>>
>>>>
>>>>> -----Original Message-----
>>>>> From: Marvin Häuser <mhaeuser@posteo.de 
>>>>> <mailto:mhaeuser@posteo.de><mailto:mhaeuser@posteo.de 
>>>>> <mailto:mhaeuser@posteo.de>>>
>>>>> Sent: Monday, August 9, 2021 2:51 AM
>>>>> To:devel@edk2.groups.io 
>>>>> <mailto:devel@edk2.groups.io><mailto:devel@edk2.groups.io 
>>>>> <mailto:devel@edk2.groups.io>>
>>>>> Cc: Kinney, Michael D <michael.d.kinney@intel.com 
>>>>> <mailto:michael.d.kinney@intel.com><mailto:michael.d.kinney@intel.com 
>>>>> <mailto:michael.d.kinney@intel.com>>>; Liming Gao 
>>>>> <gaoliming@byosoft.com.cn 
>>>>> <mailto:gaoliming@byosoft.com.cn><mailto:gaoliming@byosoft.com.cn 
>>>>> <mailto:gaoliming@byosoft.com.cn>>>; Liu, Zhiguang
>>>>> <zhiguang.liu@intel.com 
>>>>> <mailto:zhiguang.liu@intel.com><mailto:zhiguang.liu@intel.com 
>>>>> <mailto:zhiguang.liu@intel.com>>>; Vitaly Cheptsov 
>>>>> <vit9696@protonmail.com 
>>>>> <mailto:vit9696@protonmail.com><mailto:vit9696@protonmail.com 
>>>>> <mailto:vit9696@protonmail.com>>>
>>>>> Subject: [PATCH v2 1/2] MdePkg/BaseLib: Fix unaligned API prototypes
>>>>>
>>>>> C prohibits not only dereferencing but also casting to unaligned
>>>>> pointers. Thus, the current set of unaligned APIs cannot be called
>>>>> safely. Update their prototypes to take VOID * pointers, which must
>>>>> be able to represent any valid pointer.
>>>>>
>>>>> Cc: Michael D Kinney <michael.d.kinney@intel.com 
>>>>> <mailto:michael.d.kinney@intel.com><mailto:michael.d.kinney@intel.com 
>>>>> <mailto:michael.d.kinney@intel.com>>>
>>>>> Cc: Liming Gao <gaoliming@byosoft.com.cn 
>>>>> <mailto:gaoliming@byosoft.com.cn><mailto:gaoliming@byosoft.com.cn 
>>>>> <mailto:gaoliming@byosoft.com.cn>>>
>>>>> Cc: Zhiguang Liu <zhiguang.liu@intel.com 
>>>>> <mailto:zhiguang.liu@intel.com><mailto:zhiguang.liu@intel.com 
>>>>> <mailto:zhiguang.liu@intel.com>>>
>>>>> Cc: Vitaly Cheptsov <vit9696@protonmail.com 
>>>>> <mailto:vit9696@protonmail.com><mailto:vit9696@protonmail.com 
>>>>> <mailto:vit9696@protonmail.com>>>
>>>>> Signed-off-by: Marvin Häuser <mhaeuser@posteo.de 
>>>>> <mailto:mhaeuser@posteo.de><mailto:mhaeuser@posteo.de 
>>>>> <mailto:mhaeuser@posteo.de>>>
>>>>> ---
>>>>> MdePkg/Library/BaseLib/Arm/Unaligned.c | 14 ++++-----
>>>>> MdePkg/Library/BaseLib/Unaligned.c     | 32 ++++++++++----------
>>>>> MdePkg/Include/Library/BaseLib.h       | 16 +++++-----
>>>>> 3 files changed, 31 insertions(+), 31 deletions(-)
>>>>>
>>>>> diff --git a/MdePkg/Library/BaseLib/Arm/Unaligned.c 
>>>>> b/MdePkg/Library/BaseLib/Arm/Unaligned.c
>>>>> index e9934e7003cb..57f19fc44e0b 100644
>>>>> --- a/MdePkg/Library/BaseLib/Arm/Unaligned.c
>>>>> +++ b/MdePkg/Library/BaseLib/Arm/Unaligned.c
>>>>> @@ -59,7 +59,7 @@ ReadUnaligned16 (
>>>>> UINT16
>>>>>
>>>>> EFIAPI
>>>>>
>>>>> WriteUnaligned16 (
>>>>>
>>>>> -  OUT UINT16                    *Buffer,
>>>>>
>>>>> +  OUT VOID                      *Buffer,
>>>>>
>>>>>   IN  UINT16                    Value
>>>>>
>>>>>   )
>>>>>
>>>>> {
>>>>>
>>>>> @@ -87,7 +87,7 @@ WriteUnaligned16 (
>>>>> UINT32
>>>>>
>>>>> EFIAPI
>>>>>
>>>>> ReadUnaligned24 (
>>>>>
>>>>> -  IN CONST UINT32              *Buffer
>>>>>
>>>>> +  IN CONST VOID                *Buffer
>>>>>
>>>>>   )
>>>>>
>>>>> {
>>>>>
>>>>>   ASSERT (Buffer != NULL);
>>>>>
>>>>> @@ -116,7 +116,7 @@ ReadUnaligned24 (
>>>>> UINT32
>>>>>
>>>>> EFIAPI
>>>>>
>>>>> WriteUnaligned24 (
>>>>>
>>>>> -  OUT UINT32                    *Buffer,
>>>>>
>>>>> +  OUT VOID                      *Buffer,
>>>>>
>>>>>   IN  UINT32                    Value
>>>>>
>>>>>   )
>>>>>
>>>>> {
>>>>>
>>>>> @@ -143,7 +143,7 @@ WriteUnaligned24 (
>>>>> UINT32
>>>>>
>>>>> EFIAPI
>>>>>
>>>>> ReadUnaligned32 (
>>>>>
>>>>> -  IN CONST UINT32              *Buffer
>>>>>
>>>>> +  IN CONST VOID                *Buffer
>>>>>
>>>>>   )
>>>>>
>>>>> {
>>>>>
>>>>>   UINT16  LowerBytes;
>>>>>
>>>>> @@ -175,7 +175,7 @@ ReadUnaligned32 (
>>>>> UINT32
>>>>>
>>>>> EFIAPI
>>>>>
>>>>> WriteUnaligned32 (
>>>>>
>>>>> -  OUT UINT32                    *Buffer,
>>>>>
>>>>> +  OUT VOID                      *Buffer,
>>>>>
>>>>>   IN  UINT32                    Value
>>>>>
>>>>>   )
>>>>>
>>>>> {
>>>>>
>>>>> @@ -202,7 +202,7 @@ WriteUnaligned32 (
>>>>> UINT64
>>>>>
>>>>> EFIAPI
>>>>>
>>>>> ReadUnaligned64 (
>>>>>
>>>>> -  IN CONST UINT64              *Buffer
>>>>>
>>>>> +  IN CONST VOID                *Buffer
>>>>>
>>>>>   )
>>>>>
>>>>> {
>>>>>
>>>>>   UINT32  LowerBytes;
>>>>>
>>>>> @@ -234,7 +234,7 @@ ReadUnaligned64 (
>>>>> UINT64
>>>>>
>>>>> EFIAPI
>>>>>
>>>>> WriteUnaligned64 (
>>>>>
>>>>> -  OUT UINT64                    *Buffer,
>>>>>
>>>>> +  OUT VOID                      *Buffer,
>>>>>
>>>>>   IN  UINT64                    Value
>>>>>
>>>>>   )
>>>>>
>>>>> {
>>>>>
>>>>> diff --git a/MdePkg/Library/BaseLib/Unaligned.c 
>>>>> b/MdePkg/Library/BaseLib/Unaligned.c
>>>>> index a419cb85e53c..3041adcde606 100644
>>>>> --- a/MdePkg/Library/BaseLib/Unaligned.c
>>>>> +++ b/MdePkg/Library/BaseLib/Unaligned.c
>>>>> @@ -26,12 +26,12 @@
>>>>> UINT16
>>>>>
>>>>> EFIAPI
>>>>>
>>>>> ReadUnaligned16 (
>>>>>
>>>>> -  IN CONST UINT16              *Buffer
>>>>>
>>>>> +  IN CONST VOID                *Buffer
>>>>>
>>>>>   )
>>>>>
>>>>> {
>>>>>
>>>>>   ASSERT (Buffer != NULL);
>>>>>
>>>>>
>>>>>
>>>>> -  return *Buffer;
>>>>>
>>>>> +  return *(CONST UINT16 *) Buffer;
>>>>>
>>>>> }
>>>>>
>>>>>
>>>>>
>>>>> /**
>>>>>
>>>>> @@ -52,13 +52,13 @@ ReadUnaligned16 (
>>>>> UINT16
>>>>>
>>>>> EFIAPI
>>>>>
>>>>> WriteUnaligned16 (
>>>>>
>>>>> -  OUT UINT16                    *Buffer,
>>>>>
>>>>> +  OUT VOID                      *Buffer,
>>>>>
>>>>>   IN  UINT16                    Value
>>>>>
>>>>>   )
>>>>>
>>>>> {
>>>>>
>>>>>   ASSERT (Buffer != NULL);
>>>>>
>>>>>
>>>>>
>>>>> -  return *Buffer = Value;
>>>>>
>>>>> +  return *(UINT16 *) Buffer = Value;
>>>>>
>>>>> }
>>>>>
>>>>>
>>>>>
>>>>> /**
>>>>>
>>>>> @@ -77,12 +77,12 @@ WriteUnaligned16 (
>>>>> UINT32
>>>>>
>>>>> EFIAPI
>>>>>
>>>>> ReadUnaligned24 (
>>>>>
>>>>> -  IN CONST UINT32              *Buffer
>>>>>
>>>>> +  IN CONST VOID                *Buffer
>>>>>
>>>>>   )
>>>>>
>>>>> {
>>>>>
>>>>>   ASSERT (Buffer != NULL);
>>>>>
>>>>>
>>>>>
>>>>> -  return *Buffer & 0xffffff;
>>>>>
>>>>> +  return *(CONST UINT32 *) Buffer & 0xffffff;
>>>>>
>>>>> }
>>>>>
>>>>>
>>>>>
>>>>> /**
>>>>>
>>>>> @@ -103,13 +103,13 @@ ReadUnaligned24 (
>>>>> UINT32
>>>>>
>>>>> EFIAPI
>>>>>
>>>>> WriteUnaligned24 (
>>>>>
>>>>> -  OUT UINT32                    *Buffer,
>>>>>
>>>>> +  OUT VOID                      *Buffer,
>>>>>
>>>>>   IN  UINT32                    Value
>>>>>
>>>>>   )
>>>>>
>>>>> {
>>>>>
>>>>>   ASSERT (Buffer != NULL);
>>>>>
>>>>>
>>>>>
>>>>> -  *Buffer = BitFieldWrite32 (*Buffer, 0, 23, Value);
>>>>>
>>>>> +  *(UINT32 *) Buffer = BitFieldWrite32 (*(CONST UINT32 *) Buffer, 
>>>>> 0, 23, Value);
>>>>>
>>>>>   return Value;
>>>>>
>>>>> }
>>>>>
>>>>>
>>>>>
>>>>> @@ -129,12 +129,12 @@ WriteUnaligned24 (
>>>>> UINT32
>>>>>
>>>>> EFIAPI
>>>>>
>>>>> ReadUnaligned32 (
>>>>>
>>>>> -  IN CONST UINT32              *Buffer
>>>>>
>>>>> +  IN CONST VOID                *Buffer
>>>>>
>>>>>   )
>>>>>
>>>>> {
>>>>>
>>>>>   ASSERT (Buffer != NULL);
>>>>>
>>>>>
>>>>>
>>>>> -  return *Buffer;
>>>>>
>>>>> +  return *(CONST UINT32 *) Buffer;
>>>>>
>>>>> }
>>>>>
>>>>>
>>>>>
>>>>> /**
>>>>>
>>>>> @@ -155,13 +155,13 @@ ReadUnaligned32 (
>>>>> UINT32
>>>>>
>>>>> EFIAPI
>>>>>
>>>>> WriteUnaligned32 (
>>>>>
>>>>> -  OUT UINT32                    *Buffer,
>>>>>
>>>>> +  OUT VOID                      *Buffer,
>>>>>
>>>>>   IN  UINT32                    Value
>>>>>
>>>>>   )
>>>>>
>>>>> {
>>>>>
>>>>>   ASSERT (Buffer != NULL);
>>>>>
>>>>>
>>>>>
>>>>> -  return *Buffer = Value;
>>>>>
>>>>> +  return *(UINT32 *) Buffer = Value;
>>>>>
>>>>> }
>>>>>
>>>>>
>>>>>
>>>>> /**
>>>>>
>>>>> @@ -180,12 +180,12 @@ WriteUnaligned32 (
>>>>> UINT64
>>>>>
>>>>> EFIAPI
>>>>>
>>>>> ReadUnaligned64 (
>>>>>
>>>>> -  IN CONST UINT64              *Buffer
>>>>>
>>>>> +  IN CONST VOID                *Buffer
>>>>>
>>>>>   )
>>>>>
>>>>> {
>>>>>
>>>>>   ASSERT (Buffer != NULL);
>>>>>
>>>>>
>>>>>
>>>>> -  return *Buffer;
>>>>>
>>>>> +  return *(CONST UINT64 *) Buffer;
>>>>>
>>>>> }
>>>>>
>>>>>
>>>>>
>>>>> /**
>>>>>
>>>>> @@ -206,11 +206,11 @@ ReadUnaligned64 (
>>>>> UINT64
>>>>>
>>>>> EFIAPI
>>>>>
>>>>> WriteUnaligned64 (
>>>>>
>>>>> -  OUT UINT64                    *Buffer,
>>>>>
>>>>> +  OUT VOID                      *Buffer,
>>>>>
>>>>>   IN  UINT64                    Value
>>>>>
>>>>>   )
>>>>>
>>>>> {
>>>>>
>>>>>   ASSERT (Buffer != NULL);
>>>>>
>>>>>
>>>>>
>>>>> -  return *Buffer = Value;
>>>>>
>>>>> +  return *(UINT64 *) Buffer = Value;
>>>>>
>>>>> }
>>>>>
>>>>> diff --git a/MdePkg/Include/Library/BaseLib.h 
>>>>> b/MdePkg/Include/Library/BaseLib.h
>>>>> index 2452c1d92e51..4d30f0539c6b 100644
>>>>> --- a/MdePkg/Include/Library/BaseLib.h
>>>>> +++ b/MdePkg/Include/Library/BaseLib.h
>>>>> @@ -3420,7 +3420,7 @@ DivS64x64Remainder (
>>>>> UINT16
>>>>>
>>>>> EFIAPI
>>>>>
>>>>> ReadUnaligned16 (
>>>>>
>>>>> -  IN CONST UINT16              *Buffer
>>>>>
>>>>> +  IN CONST VOID                *Buffer
>>>>>
>>>>>   );
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> @@ -3442,7 +3442,7 @@ ReadUnaligned16 (
>>>>> UINT16
>>>>>
>>>>> EFIAPI
>>>>>
>>>>> WriteUnaligned16 (
>>>>>
>>>>> -  OUT UINT16                    *Buffer,
>>>>>
>>>>> +  OUT VOID                      *Buffer,
>>>>>
>>>>>   IN  UINT16                    Value
>>>>>
>>>>>   );
>>>>>
>>>>>
>>>>>
>>>>> @@ -3463,7 +3463,7 @@ WriteUnaligned16 (
>>>>> UINT32
>>>>>
>>>>> EFIAPI
>>>>>
>>>>> ReadUnaligned24 (
>>>>>
>>>>> -  IN CONST UINT32              *Buffer
>>>>>
>>>>> +  IN CONST VOID                *Buffer
>>>>>
>>>>>   );
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> @@ -3485,7 +3485,7 @@ ReadUnaligned24 (
>>>>> UINT32
>>>>>
>>>>> EFIAPI
>>>>>
>>>>> WriteUnaligned24 (
>>>>>
>>>>> -  OUT UINT32                    *Buffer,
>>>>>
>>>>> +  OUT VOID                      *Buffer,
>>>>>
>>>>>   IN  UINT32                    Value
>>>>>
>>>>>   );
>>>>>
>>>>>
>>>>>
>>>>> @@ -3506,7 +3506,7 @@ WriteUnaligned24 (
>>>>> UINT32
>>>>>
>>>>> EFIAPI
>>>>>
>>>>> ReadUnaligned32 (
>>>>>
>>>>> -  IN CONST UINT32              *Buffer
>>>>>
>>>>> +  IN CONST VOID                *Buffer
>>>>>
>>>>>   );
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> @@ -3528,7 +3528,7 @@ ReadUnaligned32 (
>>>>> UINT32
>>>>>
>>>>> EFIAPI
>>>>>
>>>>> WriteUnaligned32 (
>>>>>
>>>>> -  OUT UINT32                    *Buffer,
>>>>>
>>>>> +  OUT VOID                      *Buffer,
>>>>>
>>>>>   IN  UINT32                    Value
>>>>>
>>>>>   );
>>>>>
>>>>>
>>>>>
>>>>> @@ -3549,7 +3549,7 @@ WriteUnaligned32 (
>>>>> UINT64
>>>>>
>>>>> EFIAPI
>>>>>
>>>>> ReadUnaligned64 (
>>>>>
>>>>> -  IN CONST UINT64              *Buffer
>>>>>
>>>>> +  IN CONST VOID                *Buffer
>>>>>
>>>>>   );
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> @@ -3571,7 +3571,7 @@ ReadUnaligned64 (
>>>>> UINT64
>>>>>
>>>>> EFIAPI
>>>>>
>>>>> WriteUnaligned64 (
>>>>>
>>>>> -  OUT UINT64                    *Buffer,
>>>>>
>>>>> +  OUT VOID                      *Buffer,
>>>>>
>>>>>   IN  UINT64                    Value
>>>>>
>>>>>   );
>>>>>
>>>>>
>>>>>
>>>>> --
>>>>> 2.31.1
>>>>
>>>>
>>>>
>
>