[PATCH 0/5] Code refinements in MdeModulePkg/EbcDebugger

[PATCH 0/5] Code refinements in MdeModulePkg/EbcDebugger

[Hao Wu]

Cc: Jiewen Yao <jiewen.yao@intel.com>

Hao Wu (5):
  MdeModulePkg/EbcDebugger: Operands of same size for bitwise operation
  MdeModulePkg/EbcDebugger: Add check for invalid 'CommandArg'
  MdeModulePkg/EbcDebugger: Add missing check for symbol not found
  MdeModulePkg/EbcDebugger: Add ASSERT to ensure FieldBuffer is not NULL
  MdeModulePkg/EbcDebugger: Compare ASCII char with '\0'

 MdeModulePkg/Universal/EbcDxe/EbcDebugger/Edb.c            |  6 +++---
 .../Universal/EbcDxe/EbcDebugger/EdbCmdBreakpoint.c        | 14 +++++++++++++-
 MdeModulePkg/Universal/EbcDxe/EbcDebugger/EdbCmdSymbol.c   |  4 ++--
 .../Universal/EbcDxe/EbcDebugger/EdbSupportString.c        |  4 ++--
 MdeModulePkg/Universal/EbcDxe/EbcDebugger/EdbSymbol.c      |  3 ++-
 5 files changed, 22 insertions(+), 9 deletions(-)

-- 
1.9.5.msysgit.0

[PATCH 1/5] MdeModulePkg/EbcDebugger: Operands of same size for bitwise operation

[Hao Wu]

Operands in a bitwise operation should have the same size to eliminate
unexpected results.

Cc: Jiewen Yao <jiewen.yao@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
---
 MdeModulePkg/Universal/EbcDxe/EbcDebugger/Edb.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/MdeModulePkg/Universal/EbcDxe/EbcDebugger/Edb.c b/MdeModulePkg/Universal/EbcDxe/EbcDebugger/Edb.c
index e0b85c7..9e44026 100644
--- a/MdeModulePkg/Universal/EbcDxe/EbcDebugger/Edb.c
+++ b/MdeModulePkg/Universal/EbcDxe/EbcDebugger/Edb.c
@@ -1,6 +1,6 @@
 /*++
 
-Copyright (c) 2007, Intel Corporation
+Copyright (c) 2007 - 2016, Intel Corporation
 All rights reserved. This program and the accompanying materials
 are licensed and made available under the terms and conditions of the BSD License
 which accompanies this distribution.  The full text of the license may be found at
@@ -360,8 +360,8 @@ Returns:
   //
   // clear STEP flag in any condition.
   //
-  if (SystemContext.SystemContextEbc->Flags & VMFLAGS_STEP) {
-    SystemContext.SystemContextEbc->Flags &= ~VMFLAGS_STEP;
+  if (SystemContext.SystemContextEbc->Flags & ((UINT64) VMFLAGS_STEP)) {
+    SystemContext.SystemContextEbc->Flags &= ~((UINT64) VMFLAGS_STEP);
   }
 
   if (!Initialized) {
-- 
1.9.5.msysgit.0

[PATCH 2/5] MdeModulePkg/EbcDebugger: Add check for invalid 'CommandArg'

[Hao Wu]

Add checks for the return value of function Atoi() in EdbCmdBreakpoint.c.
If the input parameter 'CommandArg' contains non-digit character, print
corresponding error message.

Cc: Jiewen Yao <jiewen.yao@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
---
 .../Universal/EbcDxe/EbcDebugger/EdbCmdBreakpoint.c        | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/Universal/EbcDxe/EbcDebugger/EdbCmdBreakpoint.c b/MdeModulePkg/Universal/EbcDxe/EbcDebugger/EdbCmdBreakpoint.c
index 4be6dd4..f06963f 100644
--- a/MdeModulePkg/Universal/EbcDxe/EbcDebugger/EdbCmdBreakpoint.c
+++ b/MdeModulePkg/Universal/EbcDxe/EbcDebugger/EdbCmdBreakpoint.c
@@ -1,6 +1,6 @@
 /*++
 
-Copyright (c) 2007, Intel Corporation
+Copyright (c) 2007 - 2016, Intel Corporation
 All rights reserved. This program and the accompanying materials
 are licensed and made available under the terms and conditions of the BSD License
 which accompanies this distribution.  The full text of the license may be found at
@@ -461,6 +461,10 @@ Returns:
   // Get breakpoint index
   //
   Index = Atoi(CommandArg);
+  if (Index == (UINTN) -1) {
+    EDBPrint (L"BreakpointClear Argument error!\n");
+    return EFI_DEBUG_CONTINUE;
+  }
 
   if ((Index >= EFI_DEBUGGER_BREAKPOINT_MAX) ||
       (Index >= DebuggerPrivate->DebuggerBreakpointCount)) {
@@ -534,6 +538,10 @@ Returns:
   // Get breakpoint index
   //
   Index = Atoi(CommandArg);
+  if (Index == (UINTN) -1) {
+    EDBPrint (L"BreakpointDisable Argument error!\n");
+    return EFI_DEBUG_CONTINUE;
+  }
 
   //
   // Disable breakpoint
@@ -598,6 +606,10 @@ Returns:
   // Get breakpoint index
   //
   Index = Atoi(CommandArg);
+  if (Index == (UINTN) -1) {
+    EDBPrint (L"BreakpointEnable Argument error!\n");
+    return EFI_DEBUG_CONTINUE;
+  }
 
   //
   // Enable breakpoint
-- 
1.9.5.msysgit.0

[PATCH 3/5] MdeModulePkg/EbcDebugger: Add missing check for symbol not found

[Hao Wu]

In function DebuggerDisplaySymbolAccrodingToAddress(), when variable
'CandidateAddress' (returned by EbdFindSymbolAddress function) equals
(UINTN) -1, it also indicates that the symbol is not found at the given
address.

This commit adds this missing check.

Cc: Jiewen Yao <jiewen.yao@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
---
 MdeModulePkg/Universal/EbcDxe/EbcDebugger/EdbCmdSymbol.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/MdeModulePkg/Universal/EbcDxe/EbcDebugger/EdbCmdSymbol.c b/MdeModulePkg/Universal/EbcDxe/EbcDebugger/EdbCmdSymbol.c
index 329a642..8a418d5 100644
--- a/MdeModulePkg/Universal/EbcDxe/EbcDebugger/EdbCmdSymbol.c
+++ b/MdeModulePkg/Universal/EbcDxe/EbcDebugger/EdbCmdSymbol.c
@@ -1,6 +1,6 @@
 /*++
 
-Copyright (c) 2007, Intel Corporation
+Copyright (c) 2007 - 2016, Intel Corporation
 All rights reserved. This program and the accompanying materials
 are licensed and made available under the terms and conditions of the BSD License
 which accompanies this distribution.  The full text of the license may be found at
@@ -185,7 +185,7 @@ Returns:
   // Find the nearest symbol address
   //
   CandidateAddress = EbdFindSymbolAddress (Address, EdbMatchSymbolTypeNearestAddress, &Object, &Entry);
-  if (CandidateAddress == 0) {
+  if (CandidateAddress == 0 || CandidateAddress == (UINTN) -1) {
     EDBPrint (L"Symbole at Address not found!\n");
     return EFI_DEBUG_CONTINUE;
   } else if (Address != CandidateAddress) {
-- 
1.9.5.msysgit.0

[PATCH 4/5] MdeModulePkg/EbcDebugger: Add ASSERT to ensure FieldBuffer is not NULL

[Hao Wu]

In function EdbLoadCodBySymbolByIec(), AsciiStrGetNewTokenField() at line
1589 will return NULL if the first character in 'LineBuffer' is '\0'. But
the previous if statement at line 1576 ensures the above case will not
happen.

This commit adds ASSERT as warnings for the case that will not happen.

Cc: Jiewen Yao <jiewen.yao@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
---
 MdeModulePkg/Universal/EbcDxe/EbcDebugger/EdbSymbol.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/Universal/EbcDxe/EbcDebugger/EdbSymbol.c b/MdeModulePkg/Universal/EbcDxe/EbcDebugger/EdbSymbol.c
index 9573c43..22a827d 100644
--- a/MdeModulePkg/Universal/EbcDxe/EbcDebugger/EdbSymbol.c
+++ b/MdeModulePkg/Universal/EbcDxe/EbcDebugger/EdbSymbol.c
@@ -1,6 +1,6 @@
 /*++
 
-Copyright (c) 2007, Intel Corporation
+Copyright (c) 2007 - 2016, Intel Corporation
 All rights reserved. This program and the accompanying materials
 are licensed and made available under the terms and conditions of the BSD License
 which accompanies this distribution.  The full text of the license may be found at
@@ -1587,6 +1587,7 @@ Returns:
       // get function name, function name is followed by char 0x09.
       //
       FieldBuffer = AsciiStrGetNewTokenField (LineBuffer, Char);
+      ASSERT (FieldBuffer != NULL);
       if (AsciiStriCmp (FieldBuffer, Name) == 0) {
         BufferStart = FieldBuffer;
         CodParseState = EdbEbcCodParseStateSymbolStart;
-- 
1.9.5.msysgit.0

[PATCH 5/5] MdeModulePkg/EbcDebugger: Compare ASCII char with '\0'

[Hao Wu]

Current code is using L'\0' to compare with a ASCII char.

Cc: Jiewen Yao <jiewen.yao@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
---
 MdeModulePkg/Universal/EbcDxe/EbcDebugger/EdbSupportString.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/MdeModulePkg/Universal/EbcDxe/EbcDebugger/EdbSupportString.c b/MdeModulePkg/Universal/EbcDxe/EbcDebugger/EdbSupportString.c
index 558fe91..555ee96 100644
--- a/MdeModulePkg/Universal/EbcDxe/EbcDebugger/EdbSupportString.c
+++ b/MdeModulePkg/Universal/EbcDxe/EbcDebugger/EdbSupportString.c
@@ -1,6 +1,6 @@
 /*++
 
-Copyright (c) 2007, Intel Corporation
+Copyright (c) 2007 - 2016, Intel Corporation
 All rights reserved. This program and the accompanying materials
 are licensed and made available under the terms and conditions of the BSD License
 which accompanies this distribution.  The full text of the license may be found at
@@ -882,7 +882,7 @@ Routine Description:
     return NULL;
   }
 
-  if (*Begin == L'\0') {
+  if (*Begin == '\0') {
     mAsciiFieldBuffer = NULL;
     return NULL;
   }
-- 
1.9.5.msysgit.0

Re: [PATCH 0/5] Code refinements in MdeModulePkg/EbcDebugger

[Yao, Jiewen]

All reviewed-by: Jiewen.yao@intel.com

> -----Original Message-----
> From: Wu, Hao A
> Sent: Thursday, November 24, 2016 10:57 AM
> To: edk2-devel@lists.01.org
> Cc: Wu, Hao A <hao.a.wu@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>
> Subject: [PATCH 0/5] Code refinements in MdeModulePkg/EbcDebugger
> 
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> 
> Hao Wu (5):
>   MdeModulePkg/EbcDebugger: Operands of same size for bitwise
> operation
>   MdeModulePkg/EbcDebugger: Add check for invalid 'CommandArg'
>   MdeModulePkg/EbcDebugger: Add missing check for symbol not found
>   MdeModulePkg/EbcDebugger: Add ASSERT to ensure FieldBuffer is not
> NULL
>   MdeModulePkg/EbcDebugger: Compare ASCII char with '\0'
> 
>  MdeModulePkg/Universal/EbcDxe/EbcDebugger/Edb.c            |  6
> +++---
>  .../Universal/EbcDxe/EbcDebugger/EdbCmdBreakpoint.c        | 14
> +++++++++++++-
>  MdeModulePkg/Universal/EbcDxe/EbcDebugger/EdbCmdSymbol.c   |  4
> ++--
>  .../Universal/EbcDxe/EbcDebugger/EdbSupportString.c        |  4 ++--
>  MdeModulePkg/Universal/EbcDxe/EbcDebugger/EdbSymbol.c      |  3
> ++-
>  5 files changed, 22 insertions(+), 9 deletions(-)
> 
> --
> 1.9.5.msysgit.0