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

Reply via email to