On 17 December 2015 at 10:29, Gao, Liming <[email protected]> wrote: > Ard: > Have you found other similar cases in EDKII project? Or, it this the only > one? >
I found this one as well: .../PcAtChipsetPkg/Library/SerialIoLib/SerialPortLib.c:488:32: error: shifting a negative signed value is undefined [-Werror,-Wshift-negative-value] OutputData = (UINT8) ((~DLAB << 7) | (gBreakSet << 6) | (LcrParity << 3) | (LcrStop << 2) | LcrData); Changing the definition of DLAB to 0x1U fixes the problem here. I will add a patch for this instance as well. Note that these problems are new in Clang 3.7, they were not noticed in 3.5 or 3.6 > 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
