Hi Eric,
The patch is better, but it is not quite right. It is now allowing the
full range of values for each integer type. You can refer to the Two's
Compliment Wiki article for it additional information
http://en.wikipedia.org/wiki/Two%27s_complement.
The following code shows that the full range of integers are not being
allowed by the patch:
numeric varid = MY_EFI_VARSTORE_DATA.Nothing,
prompt = STRING_TOKEN(1),
help = STRING_TOKEN(2),
flags = DISPLAY_INT_DEC,
minimum = -128,
maximum = 127,
step = 1,
endnumeric;
A signed integer should be able to store from -128 to 127. The above
example results in an error because the patch is checking against 0x7F for
both negative and non negative numbers.
if (IntDecStyle) {
if (MinU1 > 0x7F) {
_PCATCH (VFR_RETURN_INVALID_PARAMETER, I->getLine(), "INT8
type minimum can small than -0x7F");
}
}
If you modify the matches to follow the below pattern, than it will allow
for the full range of negative to positive numbers.
if (IntDecStyle) {
if (MaxNegative == TRUE && MaxU1 > 0x80) {
_PCATCH (VFR_RETURN_INVALID_PARAMETER, A->getLine(), "INT8
type cannot be smaller than -128 (-0x80)");
} else if (MaxNegative == FALSE && MaxU1 > 0x7F) {
_PCATCH (VFR_RETURN_INVALID_PARAMETER, A->getLine(), "INT8
type cannot be bigger than 127 (0x7F)");
}
}
You can check the Limits.h in your compiler's include folder to see the
range of values for each integer type
Best Regards,
Aaron
From: "Dong, Eric" <eric.d...@intel.com>
To: "edk2-devel@lists.sourceforge.net"
<edk2-devel@lists.sourceforge.net>, "aaron....@congatec.com"
<aaron....@congatec.com>, "Gao, Liming" <liming....@intel.com>,
Date: 05/03/2015 10:59 PM
Subject: RE: [edk2] [Patch 1/2] BaseTools: Enhance the check for
numeric opcode with EFI_IFR_DISPLAY_INT_DEC attribute.
Aaron,
Thanks for your comments. I check the code you provide, I found the input
maximum value(200) is bigger than the allowed maximum value(0x7F). So
vfrcompiler assume the maximum value is (INT (200) = -38) which is small
than minimum value(0), and the error raised.
Now I add code to do bound check first. Please review this new patch which
add bound check.
Thanks,
Eric
From: aaron....@congatec.com [mailto:aaron....@congatec.com]
Sent: Friday, May 01, 2015 1:33 AM
To: edk2-devel@lists.sourceforge.net
Subject: Re: [edk2] [Patch 1/2] BaseTools: Enhance the check for numeric
opcode with EFI_IFR_DISPLAY_INT_DEC attribute.
Hi Eric,
I think this patch still needs some work because it isn't enforcing the
min/max of signed integers. In the below example, the following error is
generated:
typedef struct {
UINT8 Nothing[1];
} MY_EFI_VARSTORE_DATA;
numeric varid = MY_EFI_VARSTORE_DATA.Nothing,
prompt = STRING_TOKEN(1),
help = STRING_TOKEN(2),
flags = DISPLAY_INT_DEC,
minimum = 0,
maximum = 200,
step = 1,
endnumeric;
ERROR 12288: Maximum can't be less than Minimum
: invalid parameter
The patch appears to not be bounds checking the values against the maximum
value that can be stored in the type. From limits.h:
#define _I8_MIN (-127i8 - 1) /* minimum signed 8 bit value */
#define _I8_MAX 127i8 /* maximum signed 8 bit value */
numeric varid = MY_EFI_VARSTORE_DATA.Nothing,
prompt = STRING_TOKEN(1),
help = STRING_TOKEN(2),
flags = DISPLAY_INT_DEC,
minimum = -100,
maximum = 200,
step = 1,
endnumeric;
And this is allowed to compile successfully even though 200 is above the
maximum 127 that can be stored in a signed integer.
Best Regards,
Aaron
From: Eric Dong <eric.d...@intel.com>
To: edk2-devel@lists.sourceforge.net, liming....@intel.com,
Date: 04/29/2015 10:57 PM
Subject: [edk2] [Patch 1/2] BaseTools: Enhance the check for
numeric opcode with EFI_IFR_DISPLAY_INT_DEC attribute.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Eric Dong <eric.d...@intel.com>
---
BaseTools/Source/C/VfrCompile/VfrFormPkg.h | 8 ++
BaseTools/Source/C/VfrCompile/VfrSyntax.g | 170
+++++++++++++++++++++++++----
2 files changed, 156 insertions(+), 22 deletions(-)
diff --git a/BaseTools/Source/C/VfrCompile/VfrFormPkg.h
b/BaseTools/Source/C/VfrCompile/VfrFormPkg.h
index 3d2def8..e64ef1b 100644
--- a/BaseTools/Source/C/VfrCompile/VfrFormPkg.h
+++ b/BaseTools/Source/C/VfrCompile/VfrFormPkg.h
@@ -308,10 +308,14 @@ public:
}
VOID UpdateHeader (IN EFI_IFR_OP_HEADER *Header) {
mHeader = Header;
}
+
+ UINT8 GetOpCode () {
+ return mHeader->OpCode;
+ }
};
extern UINT8 gScopeCount;
/*
@@ -1356,10 +1360,14 @@ public:
} else {
mNumeric->Flags = LFlags;
}
return VFR_RETURN_SUCCESS;
}
+
+ UINT8 GetNumericFlags () {
+ return mNumeric->Flags;
+ }
};
class CIfrOneOf : public CIfrObj, public CIfrOpHeader, public
CIfrQuestionHeader, public CIfrMinMaxStepData {
private:
EFI_IFR_ONE_OF *mOneOf;
diff --git a/BaseTools/Source/C/VfrCompile/VfrSyntax.g
b/BaseTools/Source/C/VfrCompile/VfrSyntax.g
index 2255d6f..d014aff 100644
--- a/BaseTools/Source/C/VfrCompile/VfrSyntax.g
+++ b/BaseTools/Source/C/VfrCompile/VfrSyntax.g
@@ -1394,24 +1394,40 @@ vfrQuestionDataFieldName [EFI_QUESTION_ID &QId,
UINT32 &Mask, CHAR8 *&VarIdStr,
;
vfrConstantValueField[UINT8 Type] > [EFI_IFR_TYPE_VALUE Value] :
<<
EFI_GUID Guid;
+ BOOLEAN Negative = FALSE;
>>
+ {
+ "\-" << Negative = TRUE;
>>
+ }
N1:Number <<
switch ($Type) {
case
EFI_IFR_TYPE_NUM_SIZE_8 :
- $Value.u8 =
_STOU8(N1->getText(), N1->getLine());
+ $Value.u8 =
_STOU8(N1->getText(), N1->getLine());
+ if (Negative) {
+ $Value.u8 =
~$Value.u8 + 1;
+ }
break;
case
EFI_IFR_TYPE_NUM_SIZE_16 :
- $Value.u16 =
_STOU16(N1->getText(), N1->getLine());
+ $Value.u16 =
_STOU16(N1->getText(), N1->getLine());
+ if (Negative) {
+ $Value.u16 =
~$Value.u16 + 1;
+ }
break;
case
EFI_IFR_TYPE_NUM_SIZE_32 :
$Value.u32 =
_STOU32(N1->getText(), N1->getLine());
+ if (Negative) {
+ $Value.u32 =
~$Value.u32 + 1;
+ }
break;
case
EFI_IFR_TYPE_NUM_SIZE_64 :
$Value.u64 =
_STOU64(N1->getText(), N1->getLine());
+ if (Negative) {
+ $Value.u64 =
~$Value.u64 + 1;
+ }
break;
case
EFI_IFR_TYPE_BOOLEAN :
$Value.b =
_STOU8(N1->getText(), N1->getLine());
break;
case
EFI_IFR_TYPE_STRING :
@@ -1531,20 +1547,53 @@ vfrStatementDefault :
EFI_DEFAULT_ID DefaultId = EFI_HII_DEFAULT_CLASS_STANDARD;
CHAR8 *VarStoreName = NULL;
EFI_VFR_VARSTORE_TYPE VarStoreType = EFI_VFR_VARSTORE_INVALID;
UINT32 Size = 0;
EFI_GUID *VarGuid = NULL;
+ CIfrNumeric *NumericQst = NULL;
>>
D:Default
(
(
"=" vfrConstantValueField[_GET_CURRQEST_DATATYPE()] > [Val] ","
<<
if
(gCurrentMinMaxData != NULL && gCurrentMinMaxData->IsNumericOpcode()) {
//check default
value is valid for Numeric Opcode
- if (Val.u64 <
gCurrentMinMaxData->GetMinData(_GET_CURRQEST_DATATYPE()) || Val.u64 >
gCurrentMinMaxData->GetMaxData(_GET_CURRQEST_DATATYPE())) {
- _PCATCH
(VFR_RETURN_INVALID_PARAMETER, D->getLine(), "Numeric default value must
be between MinValue and MaxValue.");
+ NumericQst =
(CIfrNumeric *) gCurrentQuestion;
+ if
((NumericQst->GetNumericFlags() & EFI_IFR_DISPLAY) == 0) {
+ switch
(_GET_CURRQEST_DATATYPE()) {
+ case
EFI_IFR_TYPE_NUM_SIZE_8:
+ if (((INT8)
Val.u8 < (INT8) gCurrentMinMaxData->GetMinData(_GET_CURRQEST_DATATYPE()))
||
+ ((INT8)
Val.u8 > (INT8) gCurrentMinMaxData->GetMaxData(_GET_CURRQEST_DATATYPE())))
{
+ _PCATCH
(VFR_RETURN_INVALID_PARAMETER, D->getLine(), "Numeric default value must
be between MinValue and MaxValue.");
+ }
+ break;
+ case
EFI_IFR_TYPE_NUM_SIZE_16:
+ if
(((INT16) Val.u16 < (INT16)
gCurrentMinMaxData->GetMinData(_GET_CURRQEST_DATATYPE())) ||
+ ((INT16) Val.u16 > (INT16)
gCurrentMinMaxData->GetMaxData(_GET_CURRQEST_DATATYPE()))) {
+ _PCATCH
(VFR_RETURN_INVALID_PARAMETER, D->getLine(), "Numeric default value must
be between MinValue and MaxValue.");
+ }
+ break;
+ case
EFI_IFR_TYPE_NUM_SIZE_32:
+ if
(((INT32) Val.u32 < (INT32)
gCurrentMinMaxData->GetMinData(_GET_CURRQEST_DATATYPE())) ||
+ ((INT32) Val.u32 > (INT32)
gCurrentMinMaxData->GetMaxData(_GET_CURRQEST_DATATYPE()))) {
+ _PCATCH
(VFR_RETURN_INVALID_PARAMETER, D->getLine(), "Numeric default value must
be between MinValue and MaxValue.");
+ }
+ break;
+ case
EFI_IFR_TYPE_NUM_SIZE_64:
+ if
(((INT64) Val.u64 < (INT64)
gCurrentMinMaxData->GetMinData(_GET_CURRQEST_DATATYPE())) ||
+ ((INT64) Val.u64 > (INT64)
gCurrentMinMaxData->GetMaxData(_GET_CURRQEST_DATATYPE()))) {
+ _PCATCH
(VFR_RETURN_INVALID_PARAMETER, D->getLine(), "Numeric default value must
be between MinValue and MaxValue.");
+ }
+ break;
+ default:
+ break;
+ }
+ } else {
+ if (Val.u64 <
gCurrentMinMaxData->GetMinData(_GET_CURRQEST_DATATYPE()) || Val.u64 >
gCurrentMinMaxData->GetMaxData(_GET_CURRQEST_DATATYPE())) {
+ _PCATCH
(VFR_RETURN_INVALID_PARAMETER, D->getLine(), "Numeric default value must
be between MinValue and MaxValue.");
+ }
}
}
if
(_GET_CURRQEST_DATATYPE() == EFI_IFR_TYPE_OTHER) {
_PCATCH
(VFR_RETURN_FATAL_ERROR, D->getLine(), "Default data type error.");
Size = sizeof
(EFI_IFR_TYPE_VALUE);
@@ -2218,45 +2267,122 @@ vfrSetMinMaxStep[CIfrMinMaxStepData & MMSDObj] :
<<
UINT64 MaxU8 = 0, MinU8 = 0, StepU8 = 0;
UINT32 MaxU4 = 0, MinU4 = 0, StepU4 = 0;
UINT16 MaxU2 = 0, MinU2 = 0, StepU2 = 0;
UINT8 MaxU1 = 0, MinU1 = 0, StepU1 = 0;
+ BOOLEAN IntDecStyle = FALSE;
+ CIfrNumeric *NObj = (CIfrNumeric *) (&MMSDObj);
+ if ((NObj->GetOpCode() == EFI_IFR_NUMERIC_OP) &&
((NObj->GetNumericFlags() & EFI_IFR_DISPLAY) == 0)) {
+ IntDecStyle = TRUE;
+ }
+ BOOLEAN MinNegative = FALSE;
+ BOOLEAN MaxNegative = FALSE;
>>
- Minimum "=" I:Number ","
- <<
+ Minimum "="
+ {
+ "\-" << MinNegative =
TRUE; >>
+ }
+ I:Number "," <<
+ if
(!IntDecStyle && MinNegative) {
+ _PCATCH
(VFR_RETURN_INVALID_PARAMETER, I->getLine(), "\"-\" can't be used when not
in int decimal type. ");
+ }
switch
(_GET_CURRQEST_DATATYPE()) {
- case
EFI_IFR_TYPE_NUM_SIZE_64 : MinU8 = _STOU64(I->getText(), I->getLine());
break;
- case
EFI_IFR_TYPE_NUM_SIZE_32 : MinU4 = _STOU32(I->getText(), I->getLine());
break;
- case
EFI_IFR_TYPE_NUM_SIZE_16 : MinU2 = _STOU16(I->getText(), I->getLine());
break;
- case
EFI_IFR_TYPE_NUM_SIZE_8 : MinU1 = _STOU8(I->getText(), I->getLine());
break;
+ case
EFI_IFR_TYPE_NUM_SIZE_64 :
+ MinU8 =
_STOU64(I->getText(), I->getLine());
+ if
(MinNegative) {
+ MinU8 =
~MinU8 + 1;
+ }
+ break;
+ case
EFI_IFR_TYPE_NUM_SIZE_32 :
+ MinU4 =
_STOU32(I->getText(), I->getLine());
+ if
(MinNegative) {
+ MinU4 =
~MinU4 + 1;
+ }
+ break;
+ case
EFI_IFR_TYPE_NUM_SIZE_16 :
+ MinU2 =
_STOU16(I->getText(), I->getLine());
+ if
(MinNegative) {
+ MinU2 =
~MinU2 + 1;
+ }
+ break;
+ case
EFI_IFR_TYPE_NUM_SIZE_8 :
+ MinU1 =
_STOU8(I->getText(), I->getLine());
+ if
(MinNegative) {
+ MinU1 =
~MinU1 + 1;
+ }
+ break;
}
>>
- Maximum "=" A:Number ","
- <<
+ Maximum "="
+ {
+ "\-" << MaxNegative =
TRUE; >>
+ }
+ A:Number "," <<
+ if
(!IntDecStyle && MaxNegative) {
+ _PCATCH
(VFR_RETURN_INVALID_PARAMETER, A->getLine(), "\"-\" can't be used when not
in int decimal type. ");
+ }
+
switch
(_GET_CURRQEST_DATATYPE()) {
case
EFI_IFR_TYPE_NUM_SIZE_64 :
MaxU8 =
_STOU64(A->getText(), A->getLine());
- if (MaxU8 <
MinU8) {
- _PCATCH
(VFR_RETURN_INVALID_PARAMETER, A->getLine(), "Maximum can't be less than
Minimum");
+ if
(MaxNegative) {
+ MaxU8 =
~MaxU8 + 1;
+ }
+ if
(IntDecStyle) {
+ if ((INT64)
MaxU8 < (INT64) MinU8) {
+ _PCATCH
(VFR_RETURN_INVALID_PARAMETER, A->getLine(), "Maximum can't be less than
Minimum");
+ }
+ } else {
+ if (MaxU8 <
MinU8) {
+ _PCATCH
(VFR_RETURN_INVALID_PARAMETER, A->getLine(), "Maximum can't be less than
Minimum");
+ }
}
break;
case
EFI_IFR_TYPE_NUM_SIZE_32 :
- MaxU4 =
_STOU32(A->getText(), A->getLine());
- if (MaxU4 <
MinU4) {
- _PCATCH
(VFR_RETURN_INVALID_PARAMETER, A->getLine(), "Maximum can't be less than
Minimum");
+ MaxU4 =
_STOU32(A->getText(), A->getLine());
+ if
(MaxNegative) {
+ MaxU4 =
~MaxU4 + 1;
+ }
+ if
(IntDecStyle) {
+ if ((INT32)
MaxU4 < (INT32) MinU4) {
+ _PCATCH
(VFR_RETURN_INVALID_PARAMETER, A->getLine(), "Maximum can't be less than
Minimum");
+ }
+ } else {
+ if (MaxU4 <
MinU4) {
+ _PCATCH
(VFR_RETURN_INVALID_PARAMETER, A->getLine(), "Maximum can't be less than
Minimum");
+ }
}
break;
case
EFI_IFR_TYPE_NUM_SIZE_16 :
MaxU2 =
_STOU16(A->getText(), A->getLine());
- if (MaxU2 <
MinU2) {
- _PCATCH
(VFR_RETURN_INVALID_PARAMETER, A->getLine(), "Maximum can't be less than
Minimum");
+ if
(MaxNegative) {
+ MaxU2 =
~MaxU2 + 1;
+ }
+
+ if
(IntDecStyle) {
+ if ((INT16)
MaxU2 < (INT16) MinU2) {
+ _PCATCH
(VFR_RETURN_INVALID_PARAMETER, A->getLine(), "Maximum can't be less than
Minimum");
+ }
+ } else {
+ if (MaxU2 <
MinU2) {
+ _PCATCH
(VFR_RETURN_INVALID_PARAMETER, A->getLine(), "Maximum can't be less than
Minimum");
+ }
}
break;
case
EFI_IFR_TYPE_NUM_SIZE_8 :
- MaxU1 =
_STOU8(A->getText(), A->getLine());
- if (MaxU1 <
MinU1) {
- _PCATCH
(VFR_RETURN_INVALID_PARAMETER, A->getLine(), "Maximum can't be less than
Minimum");
+ MaxU1 =
_STOU8(A->getText(), A->getLine());
+ if
(MaxNegative) {
+ MaxU1 =
~MaxU1 + 1;
+ }
+ if
(IntDecStyle) {
+ if ((INT8)
MaxU1 < (INT8) MinU1) {
+ _PCATCH
(VFR_RETURN_INVALID_PARAMETER, A->getLine(), "Maximum can't be less than
Minimum");
+ }
+ } else {
+ if (MaxU1 <
MinU1) {
+ _PCATCH
(VFR_RETURN_INVALID_PARAMETER, A->getLine(), "Maximum can't be less than
Minimum");
+ }
}
break;
}
>>
{
--
1.9.5.msysgit.1
------------------------------------------------------------------------------
One dashboard for servers and applications across Physical-Virtual-Cloud
Widest out-of-the-box monitoring support with 50+ applications
Performance metrics, stats and reports that give you Actionable Insights
Deep dive visibility with transaction tracing using APM Insight.
http://ad.doubleclick.net/ddm/clk/290420510;117567292;y
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel[attachment
"0001-BaseTools-Enhance-the-check-for-numeric-opcode-with-.patch" deleted
by Aaron Pop/cus/congatec] [attachment
"0003-BaseTools-Add-boudary-check-for-minimum-maximum-defa.patch" deleted
by Aaron Pop/cus/congatec]
------------------------------------------------------------------------------
One dashboard for servers and applications across Physical-Virtual-Cloud
Widest out-of-the-box monitoring support with 50+ applications
Performance metrics, stats and reports that give you Actionable Insights
Deep dive visibility with transaction tracing using APM Insight.
http://ad.doubleclick.net/ddm/clk/290420510;117567292;y
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel
