Re: [patch] BaseTools/VfrCompile: report error for Integer overflow

["Gao, Liming" <liming.gao@intel.com>]

Reviewed-by: Liming Gao <liming.gao@intel.com>

> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Dandan Bi
> Sent: Friday, December 28, 2018 4:54 PM
> To: edk2-devel@lists.01.org
> Cc: Gao, Liming <liming.gao@intel.com>
> Subject: [edk2] [patch] BaseTools/VfrCompile: report error for Integer overflow
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1415
> 
> When an integer constant specified in the .vfr file is
> too large for the varstore field it is being used with,
> the VFR compiler reports an overflow warning like this:
> Test.vfr(693): WARNING: Overflow: Value 1024 is too large to
>          store in a UINT8
>     : String to UINT* Overflow
> Since Warning does not break the build process,
> and it is easy to miss it.
> This patch is to update the code to report error and break
> the build if meet this kind of issue.
> 
> Cc: Bob Feng <bob.c.feng@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Dandan Bi <dandan.bi@intel.com>
> ---
>  BaseTools/Source/C/VfrCompile/VfrError.cpp |  2 +-
>  BaseTools/Source/C/VfrCompile/VfrError.h   |  2 +-
>  BaseTools/Source/C/VfrCompile/VfrSyntax.g  | 10 +++++-----
>  3 files changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/BaseTools/Source/C/VfrCompile/VfrError.cpp b/BaseTools/Source/C/VfrCompile/VfrError.cpp
> index bbf738c217..c105d73bea 100644
> --- a/BaseTools/Source/C/VfrCompile/VfrError.cpp
> +++ b/BaseTools/Source/C/VfrCompile/VfrError.cpp
> @@ -45,16 +45,16 @@ static SVFR_ERROR_HANDLE VFR_ERROR_HANDLE_TABLE [] = {
>    { VFR_RETURN_DATA_STRING_ERROR, ": data field string error or not support"},
>    { VFR_RETURN_DEFAULT_VALUE_REDEFINED, ": default value re-defined with different value"},
>    { VFR_RETURN_CONSTANT_ONLY, ": only constant is allowed in the expression"},
>    { VFR_RETURN_VARSTORE_NAME_REDEFINED_ERROR, ": Varstore name is defined by more than one varstores, it can't be referred
> as varstore, only varstore strucure name could be used."},
>    { VFR_RETURN_BIT_WIDTH_ERROR, ": bit width must be <= sizeof (type) * 8 and the max width can not > 32" },
> +  { VFR_RETURN_STRING_TO_UINT_OVERFLOW, ": String to UINT* Overflow"},
>    { VFR_RETURN_CODEUNDEFINED, ": undefined Error Code" }
>  };
> 
>  static SVFR_WARNING_HANDLE VFR_WARNING_HANDLE_TABLE [] = {
>    { VFR_WARNING_DEFAULT_VALUE_REDEFINED, ": default value re-defined with different value"},
> -  { VFR_WARNING_STRING_TO_UINT_OVERFLOW, ": String to UINT* Overflow"},
>    { VFR_WARNING_ACTION_WITH_TEXT_TWO, ": Action opcode should not have TextTwo part"},
>    { VFR_WARNING_OBSOLETED_FRAMEWORK_OPCODE, ": Not recommend to use obsoleted framework opcode"},
>    { VFR_WARNING_CODEUNDEFINED, ": undefined Warning Code" }
>  };
> 
> diff --git a/BaseTools/Source/C/VfrCompile/VfrError.h b/BaseTools/Source/C/VfrCompile/VfrError.h
> index e83ae900fa..e795f1631d 100644
> --- a/BaseTools/Source/C/VfrCompile/VfrError.h
> +++ b/BaseTools/Source/C/VfrCompile/VfrError.h
> @@ -43,16 +43,16 @@ typedef enum {
>    VFR_RETURN_DATA_STRING_ERROR,
>    VFR_RETURN_DEFAULT_VALUE_REDEFINED,
>    VFR_RETURN_CONSTANT_ONLY,
>    VFR_RETURN_VARSTORE_NAME_REDEFINED_ERROR,
>    VFR_RETURN_BIT_WIDTH_ERROR,
> +  VFR_RETURN_STRING_TO_UINT_OVERFLOW,
>    VFR_RETURN_CODEUNDEFINED
>  } EFI_VFR_RETURN_CODE;
> 
>  typedef enum {
>    VFR_WARNING_DEFAULT_VALUE_REDEFINED = 0,
> -  VFR_WARNING_STRING_TO_UINT_OVERFLOW,
>    VFR_WARNING_ACTION_WITH_TEXT_TWO,
>    VFR_WARNING_OBSOLETED_FRAMEWORK_OPCODE,
>    VFR_WARNING_CODEUNDEFINED
>  } EFI_VFR_WARNING_CODE;
> 
> diff --git a/BaseTools/Source/C/VfrCompile/VfrSyntax.g b/BaseTools/Source/C/VfrCompile/VfrSyntax.g
> index 84dd2c3ed3..7d8088a889 100644
> --- a/BaseTools/Source/C/VfrCompile/VfrSyntax.g
> +++ b/BaseTools/Source/C/VfrCompile/VfrSyntax.g
> @@ -1,9 +1,9 @@
>  /*++ @file
>  Vfr Syntax
> 
> -Copyright (c) 2004 - 2017, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2004 - 2018, Intel Corporation. All rights reserved.<BR>
>  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
>  http://opensource.org/licenses/bsd-license.php
> 
> @@ -5320,11 +5320,11 @@ EfiVfrParser::_STOU8 (
>      if (c >= '0' && c <= '9') {
>        Value += (c - '0');
>      }
>      if((IsHex && ((Value/16) != PreviousValue)) || (!IsHex && ((Value/10) != PreviousValue))) {
>        sprintf(ErrorMsg, "Overflow: Value %s is too large to store in a UINT8", OrigString);
> -      gCVfrErrorHandle.HandleWarning (VFR_WARNING_STRING_TO_UINT_OVERFLOW, LineNum, ErrorMsg);
> +      mParserStatus = mParserStatus + gCVfrErrorHandle.HandleError (VFR_RETURN_STRING_TO_UINT_OVERFLOW, LineNum,
> ErrorMsg);
>      }
>    }
> 
>    return Value;
>  }
> @@ -5357,11 +5357,11 @@ EfiVfrParser::_STOU16 (
>      if (c >= '0' && c <= '9') {
>        Value += (c - '0');
>      }
>      if((IsHex && ((Value/16) != PreviousValue)) || (!IsHex && ((Value/10) != PreviousValue))) {
>        sprintf(ErrorMsg, "Overflow: Value %s is too large to store in a UINT16", OrigString);
> -      gCVfrErrorHandle.HandleWarning (VFR_WARNING_STRING_TO_UINT_OVERFLOW, LineNum, ErrorMsg);
> +      mParserStatus = mParserStatus + gCVfrErrorHandle.HandleError (VFR_RETURN_STRING_TO_UINT_OVERFLOW, LineNum,
> ErrorMsg);
>      }
>    }
> 
>    return Value;
>  }
> @@ -5394,11 +5394,11 @@ EfiVfrParser::_STOU32 (
>      if (c >= '0' && c <= '9') {
>        Value += (c - '0');
>      }
>      if((IsHex && ((Value/16) != PreviousValue)) || (!IsHex && ((Value/10) != PreviousValue ))) {
>        sprintf(ErrorMsg, "Overflow: Value %s is too large to store in a UINT32", OrigString);
> -      gCVfrErrorHandle.HandleWarning (VFR_WARNING_STRING_TO_UINT_OVERFLOW, LineNum, ErrorMsg);
> +      mParserStatus = mParserStatus + gCVfrErrorHandle.HandleError (VFR_RETURN_STRING_TO_UINT_OVERFLOW, LineNum,
> ErrorMsg);
>      }
>    }
> 
>    return Value;
>  }
> @@ -5430,11 +5430,11 @@ EfiVfrParser::_STOU64 (
>      if (c >= '0' && c <= '9') {
>        Value += (c - '0');
>      }
>      if((IsHex && ((Value/16) != PreviousValue)) || ((!IsHex && (Value/10) != PreviousValue))) {
>        sprintf(ErrorMsg, "Overflow: Value %s is too large to store in a UINT64", OrigString);
> -      gCVfrErrorHandle.HandleWarning (VFR_WARNING_STRING_TO_UINT_OVERFLOW, LineNum, ErrorMsg);
> +      mParserStatus = mParserStatus + gCVfrErrorHandle.HandleError (VFR_RETURN_STRING_TO_UINT_OVERFLOW, LineNum,
> ErrorMsg);
>      }
>    }
> 
>    return Value;
>  }
> --
> 2.18.0.windows.1
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel