Handled the case where int and float are of correct format, but large. The GLSL spec is not very clear on how integers should be interpreted for expressions. C99 says the expression is of type intmax_t. I am parsing all integers as int except those in expressions, which are being parsed as unsigned int.
Review URL: https://codereview.appspot.com/6351051
git-svn-id: https://angleproject.googlecode.com/svn/trunk@1179 736b8ea6-26fd-11df-bfd4-992fa37f6226
diff --git a/src/compiler/preprocessor/new/Diagnostics.cpp b/src/compiler/preprocessor/new/Diagnostics.cpp
index 6360a18..0171edc 100644
--- a/src/compiler/preprocessor/new/Diagnostics.cpp
+++ b/src/compiler/preprocessor/new/Diagnostics.cpp
@@ -48,6 +48,12 @@
return "invalid character";
case INVALID_NUMBER:
return "invalid number";
+ case INTEGER_OVERFLOW:
+ return "integer overflow";
+ case FLOAT_OVERFLOW:
+ return "float overflow";
+ case IDENTIFIER_OVERFLOW:
+ return "identifier buffer overflow";
case INVALID_EXPRESSION:
return "invalid expression";
case DIVISION_BY_ZERO:
diff --git a/src/compiler/preprocessor/new/Diagnostics.h b/src/compiler/preprocessor/new/Diagnostics.h
index 4b982a1..d5a960d 100644
--- a/src/compiler/preprocessor/new/Diagnostics.h
+++ b/src/compiler/preprocessor/new/Diagnostics.h
@@ -31,6 +31,9 @@
OUT_OF_MEMORY,
INVALID_CHARACTER,
INVALID_NUMBER,
+ INTEGER_OVERFLOW,
+ FLOAT_OVERFLOW,
+ IDENTIFIER_OVERFLOW,
INVALID_EXPRESSION,
DIVISION_BY_ZERO,
EOF_IN_COMMENT,
diff --git a/src/compiler/preprocessor/new/DirectiveParser.cpp b/src/compiler/preprocessor/new/DirectiveParser.cpp
index 9d12319..d066aab 100644
--- a/src/compiler/preprocessor/new/DirectiveParser.cpp
+++ b/src/compiler/preprocessor/new/DirectiveParser.cpp
@@ -550,7 +550,7 @@
{
assert(getDirective(token) == DIRECTIVE_ERROR);
- std::stringstream stream;
+ std::ostringstream stream;
mTokenizer->lex(token);
while ((token->type != '\n') && (token->type != Token::LAST))
{
@@ -709,7 +709,12 @@
token->location, token->text);
valid = false;
}
- if (valid) version = atoi(token->text.c_str());
+ if (valid && !token->iValue(&version))
+ {
+ mDiagnostics->report(Diagnostics::INTEGER_OVERFLOW,
+ token->location, token->text);
+ valid = false;
+ }
break;
default:
if (valid)
@@ -759,7 +764,12 @@
token->location, token->text);
valid = false;
}
- if (valid) line = atoi(token->text.c_str());
+ if (valid && !token->iValue(&line))
+ {
+ mDiagnostics->report(Diagnostics::INTEGER_OVERFLOW,
+ token->location, token->text);
+ valid = false;
+ }
break;
case FILE_NUMBER:
if (valid && (token->type != Token::CONST_INT))
@@ -768,7 +778,12 @@
token->location, token->text);
valid = false;
}
- if (valid) file = atoi(token->text.c_str());
+ if (valid && !token->iValue(&file))
+ {
+ mDiagnostics->report(Diagnostics::INTEGER_OVERFLOW,
+ token->location, token->text);
+ valid = false;
+ }
break;
default:
if (valid)
diff --git a/src/compiler/preprocessor/new/ExpressionParser.cpp b/src/compiler/preprocessor/new/ExpressionParser.cpp
index 34994c8..a6a17e1 100644
--- a/src/compiler/preprocessor/new/ExpressionParser.cpp
+++ b/src/compiler/preprocessor/new/ExpressionParser.cpp
@@ -93,7 +93,6 @@
#include "ExpressionParser.h"
#include <cassert>
-#include <cstdlib>
#include <sstream>
#include "Diagnostics.h"
@@ -102,7 +101,6 @@
#if defined(_MSC_VER)
typedef __int64 YYSTYPE;
-#define strtoll _strtoi64
#else
#include <stdint.h>
typedef intmax_t YYSTYPE;
@@ -469,9 +467,9 @@
/* YYRLINE[YYN] -- source line where rule number YYN was defined. */
static const yytype_uint8 yyrline[] =
{
- 0, 87, 87, 94, 95, 98, 101, 104, 107, 110,
- 113, 116, 119, 122, 125, 128, 131, 134, 137, 140,
- 153, 166, 169, 172, 175, 178, 181
+ 0, 85, 85, 92, 93, 96, 99, 102, 105, 108,
+ 111, 114, 117, 120, 123, 126, 129, 132, 135, 138,
+ 151, 164, 167, 170, 173, 176, 179
};
#endif
@@ -1552,7 +1550,7 @@
{
if ((yyvsp[(3) - (3)]) == 0) {
- std::stringstream stream;
+ std::ostringstream stream;
stream << (yyvsp[(1) - (3)]) << " % " << (yyvsp[(3) - (3)]);
std::string text = stream.str();
context->diagnostics->report(pp::Diagnostics::DIVISION_BY_ZERO,
@@ -1569,7 +1567,7 @@
{
if ((yyvsp[(3) - (3)]) == 0) {
- std::stringstream stream;
+ std::ostringstream stream;
stream << (yyvsp[(1) - (3)]) << " / " << (yyvsp[(3) - (3)]);
std::string text = stream.str();
context->diagnostics->report(pp::Diagnostics::DIVISION_BY_ZERO,
@@ -1846,10 +1844,17 @@
switch (token->type)
{
case pp::Token::CONST_INT:
- *lvalp = strtoll(token->text.c_str(), NULL, 0);
+ {
+ unsigned int val = 0;
+ if (!token->uValue(&val))
+ {
+ context->diagnostics->report(pp::Diagnostics::INTEGER_OVERFLOW,
+ token->location, token->text);
+ }
+ *lvalp = static_cast<YYSTYPE>(val);
type = CONST_INT;
break;
-
+ }
case pp::Token::OP_OR: type = OP_OR; break;
case pp::Token::OP_AND: type = OP_AND; break;
case pp::Token::OP_NE: type = OP_NE; break;
diff --git a/src/compiler/preprocessor/new/ExpressionParser.y b/src/compiler/preprocessor/new/ExpressionParser.y
index c28d2e7..68823b3 100644
--- a/src/compiler/preprocessor/new/ExpressionParser.y
+++ b/src/compiler/preprocessor/new/ExpressionParser.y
@@ -30,7 +30,6 @@
#include "ExpressionParser.h"
#include <cassert>
-#include <cstdlib>
#include <sstream>
#include "Diagnostics.h"
@@ -39,7 +38,6 @@
#if defined(_MSC_VER)
typedef __int64 YYSTYPE;
-#define strtoll _strtoi64
#else
#include <stdint.h>
typedef intmax_t YYSTYPE;
@@ -139,7 +137,7 @@
}
| expression '%' expression {
if ($3 == 0) {
- std::stringstream stream;
+ std::ostringstream stream;
stream << $1 << " % " << $3;
std::string text = stream.str();
context->diagnostics->report(pp::Diagnostics::DIVISION_BY_ZERO,
@@ -152,7 +150,7 @@
}
| expression '/' expression {
if ($3 == 0) {
- std::stringstream stream;
+ std::ostringstream stream;
stream << $1 << " / " << $3;
std::string text = stream.str();
context->diagnostics->report(pp::Diagnostics::DIVISION_BY_ZERO,
@@ -193,10 +191,17 @@
switch (token->type)
{
case pp::Token::CONST_INT:
- *lvalp = strtoll(token->text.c_str(), NULL, 0);
+ {
+ unsigned int val = 0;
+ if (!token->uValue(&val))
+ {
+ context->diagnostics->report(pp::Diagnostics::INTEGER_OVERFLOW,
+ token->location, token->text);
+ }
+ *lvalp = static_cast<YYSTYPE>(val);
type = CONST_INT;
break;
-
+ }
case pp::Token::OP_OR: type = OP_OR; break;
case pp::Token::OP_AND: type = OP_AND; break;
case pp::Token::OP_NE: type = OP_NE; break;
diff --git a/src/compiler/preprocessor/new/Preprocessor.cpp b/src/compiler/preprocessor/new/Preprocessor.cpp
index d3251f4..8f7dfc5 100644
--- a/src/compiler/preprocessor/new/Preprocessor.cpp
+++ b/src/compiler/preprocessor/new/Preprocessor.cpp
@@ -95,6 +95,48 @@
case Token::PP_HASH:
assert(false);
break;
+ case Token::CONST_INT:
+ {
+ int val = 0;
+ if (!token->iValue(&val))
+ {
+ // Do not mark the token as invalid.
+ // Just emit the diagnostic and reset value to 0.
+ mImpl->diagnostics->report(Diagnostics::INTEGER_OVERFLOW,
+ token->location, token->text);
+ token->text.assign("0");
+ }
+ validToken = true;
+ break;
+ }
+ case Token::CONST_FLOAT:
+ {
+ float val = 0;
+ if (!token->fValue(&val))
+ {
+ // Do not mark the token as invalid.
+ // Just emit the diagnostic and reset value to 0.0.
+ mImpl->diagnostics->report(Diagnostics::FLOAT_OVERFLOW,
+ token->location, token->text);
+ token->text.assign("0.0");
+ }
+ validToken = true;
+ break;
+ }
+ case Token::IDENTIFIER:
+ {
+ static const int kMaxIdentifierLength = 256;
+ if (token->text.size() > kMaxIdentifierLength)
+ {
+ // Do not mark the token as invalid.
+ // Just emit the diagnostic and clamp string length.
+ mImpl->diagnostics->report(Diagnostics::IDENTIFIER_OVERFLOW,
+ token->location, token->text);
+ token->text.erase(kMaxIdentifierLength);
+ }
+ validToken = true;
+ break;
+ }
case Token::PP_NUMBER:
mImpl->diagnostics->report(Diagnostics::INVALID_NUMBER,
token->location, token->text);
diff --git a/src/compiler/preprocessor/new/Token.cpp b/src/compiler/preprocessor/new/Token.cpp
index e525f38..e909150 100644
--- a/src/compiler/preprocessor/new/Token.cpp
+++ b/src/compiler/preprocessor/new/Token.cpp
@@ -6,6 +6,28 @@
#include "Token.h"
+#include <cassert>
+#include <sstream>
+
+template<typename IntType>
+static bool atoi_t(const std::string& str, IntType* value)
+{
+ std::ios::fmtflags base = std::ios::dec;
+ if ((str.size() >= 2) && (str[0] == '0') && (tolower(str[1]) == 'x'))
+ {
+ base = std::ios::hex;
+ }
+ else if ((str.size() >= 1) && (str[0] == '0'))
+ {
+ base = std::ios::oct;
+ }
+
+ std::istringstream stream(str);
+ stream.setf(base, std::ios::basefield);
+ stream >> (*value);
+ return !stream.fail();
+}
+
namespace pp
{
@@ -49,6 +71,28 @@
flags &= ~EXPANSION_DISABLED;
}
+bool Token::iValue(int* value) const
+{
+ assert(type == CONST_INT);
+ return atoi_t(text, value);
+}
+
+bool Token::uValue(unsigned int* value) const
+{
+ assert(type == CONST_INT);
+ return atoi_t(text, value);
+}
+
+bool Token::fValue(float* value) const
+{
+ assert(type == CONST_FLOAT);
+
+ std::istringstream stream(text);
+ stream.imbue(std::locale("C"));
+ stream >> (*value);
+ return !stream.fail();
+}
+
std::ostream& operator<<(std::ostream& out, const Token& token)
{
if (token.hasLeadingSpace())
diff --git a/src/compiler/preprocessor/new/Token.h b/src/compiler/preprocessor/new/Token.h
index 0f8dce7..8b553ae 100644
--- a/src/compiler/preprocessor/new/Token.h
+++ b/src/compiler/preprocessor/new/Token.h
@@ -78,6 +78,12 @@
bool expansionDisabled() const { return (flags & EXPANSION_DISABLED) != 0; }
void setExpansionDisabled(bool disable);
+ // Converts text into numeric value for CONST_INT and CONST_FLOAT token.
+ // Returns false if the parsed value cannot fit into an int or float.
+ bool iValue(int* value) const;
+ bool uValue(unsigned int* value) const;
+ bool fValue(float* value) const;
+
int type;
unsigned int flags;
SourceLocation location;
diff --git a/tests/preprocessor_tests/if_test.cpp b/tests/preprocessor_tests/if_test.cpp
index 5696ebd..7a7548f 100644
--- a/tests/preprocessor_tests/if_test.cpp
+++ b/tests/preprocessor_tests/if_test.cpp
@@ -648,6 +648,48 @@
mPreprocessor.lex(&token);
}
+TEST_F(IfTest, DecIntegerOverflow)
+{
+ const char* str = "#if 4294967296\n"
+ "#endif\n";
+ ASSERT_TRUE(mPreprocessor.init(1, &str, 0));
+
+ EXPECT_CALL(mDiagnostics,
+ print(pp::Diagnostics::INTEGER_OVERFLOW,
+ pp::SourceLocation(0, 1), "4294967296"));
+
+ pp::Token token;
+ mPreprocessor.lex(&token);
+}
+
+TEST_F(IfTest, OctIntegerOverflow)
+{
+ const char* str = "#if 077777777777\n"
+ "#endif\n";
+ ASSERT_TRUE(mPreprocessor.init(1, &str, 0));
+
+ EXPECT_CALL(mDiagnostics,
+ print(pp::Diagnostics::INTEGER_OVERFLOW,
+ pp::SourceLocation(0, 1), "077777777777"));
+
+ pp::Token token;
+ mPreprocessor.lex(&token);
+}
+
+TEST_F(IfTest, HexIntegerOverflow)
+{
+ const char* str = "#if 0xfffffffff\n"
+ "#endif\n";
+ ASSERT_TRUE(mPreprocessor.init(1, &str, 0));
+
+ EXPECT_CALL(mDiagnostics,
+ print(pp::Diagnostics::INTEGER_OVERFLOW,
+ pp::SourceLocation(0, 1), "0xfffffffff"));
+
+ pp::Token token;
+ mPreprocessor.lex(&token);
+}
+
TEST_F(IfTest, UndefinedMacro)
{
const char* str = "#if UNDEFINED\n"
diff --git a/tests/preprocessor_tests/location_test.cpp b/tests/preprocessor_tests/location_test.cpp
index 5343304..b615f99 100644
--- a/tests/preprocessor_tests/location_test.cpp
+++ b/tests/preprocessor_tests/location_test.cpp
@@ -295,7 +295,9 @@
{"#line\n", pp::Diagnostics::INVALID_LINE_DIRECTIVE},
{"#line foo\n", pp::Diagnostics::INVALID_LINE_NUMBER},
{"#line 10 foo\n", pp::Diagnostics::INVALID_FILE_NUMBER},
- {"#line 10 20 foo\n", pp::Diagnostics::UNEXPECTED_TOKEN}
+ {"#line 10 20 foo\n", pp::Diagnostics::UNEXPECTED_TOKEN},
+ {"#line 0xffffffff\n", pp::Diagnostics::INTEGER_OVERFLOW},
+ {"#line 10 0xffffffff\n", pp::Diagnostics::INTEGER_OVERFLOW}
};
INSTANTIATE_TEST_CASE_P(All, InvalidLineTest, testing::ValuesIn(kParams));
diff --git a/tests/preprocessor_tests/version_test.cpp b/tests/preprocessor_tests/version_test.cpp
index d2d314c..283b01f 100644
--- a/tests/preprocessor_tests/version_test.cpp
+++ b/tests/preprocessor_tests/version_test.cpp
@@ -90,7 +90,8 @@
static const VersionTestParam kParams[] = {
{"#version\n", pp::Diagnostics::INVALID_VERSION_DIRECTIVE},
{"#version foo\n", pp::Diagnostics::INVALID_VERSION_NUMBER},
- {"#version 100 foo\n", pp::Diagnostics::UNEXPECTED_TOKEN}
+ {"#version 100 foo\n", pp::Diagnostics::UNEXPECTED_TOKEN},
+ {"#version 0xffffffff\n", pp::Diagnostics::INTEGER_OVERFLOW}
};
INSTANTIATE_TEST_CASE_P(All, InvalidVersionTest, testing::ValuesIn(kParams));
