Felix: Good finding. Could you help contribute one patch for it?
Thanks Liming > -----Original Message----- > From: Felix Poludov [mailto:[email protected]] > Sent: Saturday, May 21, 2016 4:18 AM > To: Gao, Liming <[email protected]>; Ard Biesheuvel > <[email protected]>; [email protected] > Cc: [email protected] > Subject: RE: [PATCH] MdePkg/BaseLib: do not rely on undefined behavior in > arithmetic shift > > 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
