Here is another case of shifting a negative value:
It's CoreRestoreTpl (MdeModulePkg\Core\Dxe\Event\Tpl.c):
while (((-2 << NewTpl) & gEventPending) != 0) {
....
}-----Original Message----- From: edk2-devel [mailto:[email protected]] On Behalf Of Gao, Liming Sent: Thursday, December 17, 2015 4:29 AM To: Ard Biesheuvel; [email protected] Cc: [email protected] Subject: Re: [edk2] [PATCH] MdePkg/BaseLib: do not rely on undefined behavior in arithmetic shift Ard: Have you found other similar cases in EDKII project? Or, it this the only one? Thanks Liming -----Original Message----- From: Ard Biesheuvel [mailto:[email protected]] Sent: Wednesday, December 16, 2015 6:49 PM To: [email protected]; Gao, Liming Cc: [email protected]; Ard Biesheuvel Subject: [PATCH] MdePkg/BaseLib: do not rely on undefined behavior in arithmetic shift The runtime test whether the compiler supports arithmetic shift of negative signed numbers currently relies on undefined behavior in C, which means that all bets are off regarding whether the condition that follows passes or fails, regardless of whether the compiler in fact supports arithmetic shift or not. Relevant quote from ISO C99 (6.5.7/4) The result of E1 << E2 is E1 left-shifted E2 bit positions; vacated bits are filled with zeros. If E1 has an unsigned type, the value of the result is E1 × 2^E2, reduced modulo one more than the maximum value representable in the result type. If E1 has a signed type and nonnegative value, and E1 × 2^E2 is representable in the result type, then that is the resulting value; otherwise, the behavior is undefined. For historic purposes, let's keep the test in place (although it is doubtful we actually need it) but rewrite it in a way that prevents compilers from this century from doing whacky things with it. Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Ard Biesheuvel <[email protected]> --- Starting with version 3.7, Clang warns about the use of a negative left hand operand when performing arithmetic shift left, leading to build failures under -Werror. MdePkg/Library/BaseLib/Math64.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/MdePkg/Library/BaseLib/Math64.c b/MdePkg/Library/BaseLib/Math64.c index 83d76847213e..9624cf90029f 100644 --- a/MdePkg/Library/BaseLib/Math64.c +++ b/MdePkg/Library/BaseLib/Math64.c @@ -86,7 +86,7 @@ InternalMathARShiftU64 ( // // Test if this compiler supports arithmetic shift // - TestValue = (((-1) << (sizeof (-1) * 8 - 1)) >> (sizeof (-1) * 8 - 1)); + TestValue = (INTN)((INT64)(1ULL << 63) >> 63); if (TestValue == -1) { // // Arithmetic shift is supported -- 2.5.0 _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel The information contained in this message may be confidential and proprietary to American Megatrends, Inc. This communication is intended to be read only by the individual or entity to whom it is addressed or by their designee. If the reader of this message is not the intended recipient, you are on notice that any distribution of this message, in any form, is strictly prohibited. Please promptly notify the sender by reply e-mail or by telephone at 770-246-8600, and then delete or destroy all copies of the transmission. _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
