Jeff Fan [mailto:jeff....@intel.com] wrote:
]Sent: Thursday, June 25, 2015 02:59 AM ]To: edk2-devel@lists.sourceforge.net ]Subject: [edk2] [Patch] SourceLevelDebugPkg/DebugAgent: Add typecast to fix sign extension ] ]OffsetHigh is 16bit value and its type is UINT32 and defined in structure. ]If its high bit is 1, it will sign extension when do 16-bit left-shift ]operation. Need to typecast if after left-shift operation on GCC. The patch is working but I think the description is not exactly right. Shift related sign extension happens with right shift, not with left shift. Removing the shift does not remove the gcc sign extension. The problem is related to how the C language evaluates integers of size smaller than int. The C99 spec says: "If an int can represent all values of the original type, the value is converted to an int; otherwise, it is converted to an unsigned int" This means that a UINT16 is converted to int when evaluation an expression. Both gcc and Microsoft compilers do this. I think the gcc/Microsoft difference has to do with the bit fields: struct { UINT32 OffsetLow:16; ///< Offset bits 15..0. . . . UINT32 OffsetHigh:16; ///< Offset bits 31..16. } Bits; Gcc takes the view that "an int can represent all values of the original type" for OffsetLow and OffsetHigh. Microsoft compilers treat these two fields as not representable by signed int. Who is right? Well, the C99 spec is not real clear about this particular case. However, C11 is: "If an int can represent all values of the original type (as restricted by the width, for a bit-field), the value --------------------------------------------- is converted to an int; otherwise, it is converted to an unsigned int" So it looks like gcc is using the interpretation that has now become standard. Thanks, Scott ]Contributed-under: TianoCore Contribution Agreement 1.0 ]Signed-off-by: Jeff Fan <jeff....@intel.com> ]CC: Ruiyu Ni <ruiyu...@intel.com> ]--- ] SourceLevelDebugPkg/Library/DebugAgent/DxeDebugAgent/DxeDebugAgentLib.c | 2 +- ] .../Library/DebugAgent/SecPeiDebugAgent/SecPeiDebugAgentLib.c | 2 +- ] SourceLevelDebugPkg/Library/DebugAgent/SmmDebugAgent/SmmDebugAgentLib.c | 2 +- ] 3 files changed, 3 insertions(+), 3 deletions(-) ] ]diff --git a/SourceLevelDebugPkg/Library/DebugAgent/DxeDebugAgent/DxeDebugAgentLib.c ]b/SourceLevelDebugPkg/Library/DebugAgent/DxeDebugAgent/DxeDebugAgentLib.c ]index f7fbb3c..35fb5e6 100644 ]--- a/SourceLevelDebugPkg/Library/DebugAgent/DxeDebugAgent/DxeDebugAgentLib.c ]+++ b/SourceLevelDebugPkg/Library/DebugAgent/DxeDebugAgent/DxeDebugAgentLib.c ]@@ -512,7 +512,7 @@ InitializeDebugAgent ( ] Ia32Idtr = (IA32_DESCRIPTOR *) Context; ] Ia32IdtEntry = (IA32_IDT_ENTRY *)(Ia32Idtr->Base); ] MailboxLocation = (UINT64 *) (UINTN) (Ia32IdtEntry[DEBUG_MAILBOX_VECTOR].Bits.OffsetLow + ]- (Ia32IdtEntry[DEBUG_MAILBOX_VECTOR].Bits.OffsetHigh << 16)); ]+ (UINT32) (Ia32IdtEntry[DEBUG_MAILBOX_VECTOR].Bits.OffsetHigh << 16)); ] Mailbox = (DEBUG_AGENT_MAILBOX *)(UINTN)(*MailboxLocation); ] VerifyMailboxChecksum (Mailbox); ] } ]diff --git a/SourceLevelDebugPkg/Library/DebugAgent/SecPeiDebugAgent/SecPeiDebugAgentLib.c ]b/SourceLevelDebugPkg/Library/DebugAgent/SecPeiDebugAgent/SecPeiDebugAgentLib.c ]index e77ff72..fcc7a4b 100644 ]--- a/SourceLevelDebugPkg/Library/DebugAgent/SecPeiDebugAgent/SecPeiDebugAgentLib.c ]+++ b/SourceLevelDebugPkg/Library/DebugAgent/SecPeiDebugAgent/SecPeiDebugAgentLib.c ]@@ -579,7 +579,7 @@ InitializeDebugAgent ( ] Ia32Idtr = (IA32_DESCRIPTOR *) Context; ] Ia32IdtEntry = (IA32_IDT_ENTRY *)(Ia32Idtr->Base); ] MailboxLocationPointer = (UINT64 *) (UINTN) (Ia32IdtEntry[DEBUG_MAILBOX_VECTOR].Bits.OffsetLow + ]- (Ia32IdtEntry[DEBUG_MAILBOX_VECTOR].Bits.OffsetHigh << ]16)); ]+ (UINT32) (Ia32IdtEntry[DEBUG_MAILBOX_VECTOR].Bits.OffsetHigh << ]16)); ] Mailbox = (DEBUG_AGENT_MAILBOX *) (UINTN)(*MailboxLocationPointer); ] // ] // Mailbox should valid and setup before executing thunk code ]diff --git a/SourceLevelDebugPkg/Library/DebugAgent/SmmDebugAgent/SmmDebugAgentLib.c ]b/SourceLevelDebugPkg/Library/DebugAgent/SmmDebugAgent/SmmDebugAgentLib.c ]index 6ac5f88..3a759f9 100644 ]--- a/SourceLevelDebugPkg/Library/DebugAgent/SmmDebugAgent/SmmDebugAgentLib.c ]+++ b/SourceLevelDebugPkg/Library/DebugAgent/SmmDebugAgent/SmmDebugAgentLib.c ]@@ -328,7 +328,7 @@ InitializeDebugAgent ( ] Ia32Idtr = (IA32_DESCRIPTOR *) Context; ] Ia32IdtEntry = (IA32_IDT_ENTRY *)(Ia32Idtr->Base); ] MailboxLocation = (UINT64 *) (UINTN) (Ia32IdtEntry[DEBUG_MAILBOX_VECTOR].Bits.OffsetLow + ]- (Ia32IdtEntry[DEBUG_MAILBOX_VECTOR].Bits.OffsetHigh << 16)); ]+ (UINT32) (Ia32IdtEntry[DEBUG_MAILBOX_VECTOR].Bits.OffsetHigh << 16)); ] mMailboxPointer = (DEBUG_AGENT_MAILBOX *)(UINTN)(*MailboxLocation); ] VerifyMailboxChecksum (mMailboxPointer); ] // ]-- ]1.9.5.msysgit.0 ------------------------------------------------------------------------------ Monitor 25 network devices or servers for free with OpManager! OpManager is web-based network management software that monitors network devices and physical & virtual servers, alerts via email & sms for fault. Monitor 25 devices for free with no restriction. Download now http://ad.doubleclick.net/ddm/clk/292181274;119417398;o _______________________________________________ edk2-devel mailing list edk2-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/edk2-devel