Re: [PATCH 2/2] MdeModulePkg/BaseSerialPortLib16550: Prevent truncating constant values.

[Laszlo Ersek <lersek@redhat.com>]

On 02/27/18 19:55, Marvin Häuser wrote:
> Hey Laszlo,
>
> Thanks for your... detailed explanation. :)
> I actually submitted another patch to prevent what you explained -
> "[edk2] [PATCH 1/2] MdePkg/Base.h: Ensure safe bitwise operations.",
> which marks all BIT defines (and more) as unsigned.
> Most definitely I should have mentioned it in the commit message or
> held it back till that patch will be accepted (or denied?), seems like
> I forgot about that.
> Would you still prefer your suggestion even when the Base.h patch is
> merged? After all, int might happen to be even larger than INT32, if
> I'm not mistaken.
>
> I'm quite sure VS2015x86 issued the warning despite that commit being
> applied locally. It seems to always warn when a constant is truncated,
> explicitely or implicitely, to give you the change to increase its
> size:
> https://msdn.microsoft.com/en-us/library/sz5z1byt.aspx

With the other (pre-requisite) change applied, such that

#define  BIT0     0x00000001u
#define  BIT1     0x00000002u

it's even less comprehensible why VS2015x86 warns about the original
code, namely:

  (UINT16)~(BIT0 | BIT1)

In this case we apply bit-or to two "unsigned int" operands, and then
apply bit-neg to the "unsigned int" result. Finally we explicitly cast
the expression to another unsigned integer type. All well-defined.

The MSDN reference writes, "The type conversion causes a constant to
exceed the space allocated for it." I don't understand how that does
*not* apply to the post-patch code, namely

  (UINT16)~(UINT16)(BIT0 | BIT1)

Again, in this variant (with the BIT macros being unsigned), we
basically have

  (UINT16)(-4)

How on earth does the constant "minus 4" (-4) fit in the UINT16 "space"?
It totally doesn't.

Sorry, VS2015x86 boggles my mind. I think it's a compiler bug, plain and
simple. Please just do whatever you have to do to shut it up. I give up.

Thanks,
Laszlo