binpac: Improve storage type used for case-type index

The type used to store the index for a case-type now tracks the
type of the index expression rather than always using an "int".

The case fields also now have some checking done at code-gen-time to
ensure the constants used for cases does not exceed the numeric limit
of the type used in the case's index expression.  Then, assuming, it
looks safe, the C++ case labels are generated with casts to the type
of the Binpac case's index expression to ensure compilers accept it
(since all Binpac numbers use "int" for storage/printing internally).
This commit is contained in:
Jon Siwek 2019-05-16 08:44:42 -07:00 committed by Tim Wojtulewicz
parent b4b229acf7
commit 21cf20fc6f
7 changed files with 137 additions and 21 deletions

View file

@ -1,6 +1,9 @@
#ifndef binpac_exception_h #ifndef binpac_exception_h
#define binpac_exception_h #define binpac_exception_h
#include <stdint.h>
#include <inttypes.h>
namespace binpac { namespace binpac {
class Exception class Exception
@ -45,19 +48,19 @@ class ExceptionInvalidCase : public Exception
{ {
public: public:
ExceptionInvalidCase(const char* location, ExceptionInvalidCase(const char* location,
int index, int64_t index,
const char *expected) const char *expected)
: location_(location), : location_(location),
index_(index), index_(index),
expected_(expected) expected_(expected)
{ {
append(binpac_fmt("invalid case: %s: %d (%s)", append(binpac_fmt("invalid case: %s: %" PRIi64 " (%s)",
location, index, expected)); location, index, expected));
} }
protected: protected:
const char* location_; const char* location_;
int index_; int64_t index_;
string expected_; string expected_;
}; };
@ -65,17 +68,17 @@ class ExceptionInvalidCaseIndex : public Exception
{ {
public: public:
ExceptionInvalidCaseIndex(const char* location, ExceptionInvalidCaseIndex(const char* location,
int index) int64_t index)
: location_(location), : location_(location),
index_(index) index_(index)
{ {
append(binpac_fmt("invalid index for case: %s: %d", append(binpac_fmt("invalid index for case: %s: %" PRIi64,
location, index)); location, index));
} }
protected: protected:
const char* location_; const char* location_;
int index_; int64_t index_;
}; };
class ExceptionInvalidOffset : public Exception class ExceptionInvalidOffset : public Exception

View file

@ -5,9 +5,13 @@
#include "pac_output.h" #include "pac_output.h"
#include "pac_typedecl.h" #include "pac_typedecl.h"
#include "pac_utils.h" #include "pac_utils.h"
#include "pac_btype.h"
#include "pac_case.h" #include "pac_case.h"
#include <limits>
#include <cstdint>
CaseType::CaseType(Expr* index_expr, CaseFieldList* cases) CaseType::CaseType(Expr* index_expr, CaseFieldList* cases)
: Type(CASE), index_expr_(index_expr), cases_(cases) : Type(CASE), index_expr_(index_expr), cases_(cases)
{ {
@ -64,7 +68,9 @@ void CaseType::Prepare(Env* env, int flags)
ASSERT(flags & TO_BE_PARSED); ASSERT(flags & TO_BE_PARSED);
index_var_ = new ID(strfmt("%s_case_index", value_var()->Name())); index_var_ = new ID(strfmt("%s_case_index", value_var()->Name()));
env->AddID(index_var_, MEMBER_VAR, extern_type_int); // Unable to get the type for index_var_ at this moment, but we'll
// generate the right type based on index_expr_ later.
env->AddID(index_var_, MEMBER_VAR, 0);
// Sort the cases_ to put the default case at the end of the list // Sort the cases_ to put the default case at the end of the list
CaseFieldList::iterator default_case_it = CaseFieldList::iterator default_case_it =
@ -100,13 +106,25 @@ void CaseType::Prepare(Env* env, int flags)
void CaseType::GenPrivDecls(Output* out_h, Env* env) void CaseType::GenPrivDecls(Output* out_h, Env* env)
{ {
out_h->println("int %s;", env->LValue(index_var_)); Type* t = index_expr_->DataType(env);
if ( t->tot() != Type::BUILTIN )
// It's a Type::EXTERN with a C++ type of "int", "bool", or "enum",
// any of which will convert consistently using an int as storage type.
t = extern_type_int;
out_h->println("%s %s;", t->DataTypeStr().c_str(), env->LValue(index_var_));
Type::GenPrivDecls(out_h, env); Type::GenPrivDecls(out_h, env);
} }
void CaseType::GenPubDecls(Output* out_h, Env* env) void CaseType::GenPubDecls(Output* out_h, Env* env)
{ {
out_h->println("int %s const { return %s; }", Type* t = index_expr_->DataType(env);
if ( t->tot() != Type::BUILTIN )
t = extern_type_int;
out_h->println("%s %s const { return %s; }", t->DataTypeStr().c_str(),
env->RValue(index_var_), env->LValue(index_var_)); env->RValue(index_var_), env->LValue(index_var_));
Type::GenPubDecls(out_h, env); Type::GenPubDecls(out_h, env);
} }
@ -168,7 +186,7 @@ void CaseType::DoGenParseCode(Output* out_cc, Env* env,
{ {
out_cc->println("default:"); out_cc->println("default:");
out_cc->inc_indent(); out_cc->inc_indent();
out_cc->println("throw binpac::ExceptionInvalidCaseIndex(\"%s\", %s);", out_cc->println("throw binpac::ExceptionInvalidCaseIndex(\"%s\", (int64)%s);",
decl_id()->Name(), env->RValue(index_var_)); decl_id()->Name(), env->RValue(index_var_));
out_cc->println("break;"); out_cc->println("break;");
out_cc->dec_indent(); out_cc->dec_indent();
@ -248,18 +266,80 @@ CaseField::~CaseField()
delete_list(ExprList, index_); delete_list(ExprList, index_);
} }
void GenCaseStr(ExprList *index_list, Output *out_cc, Env *env) void GenCaseStr(ExprList *index_list, Output *out_cc, Env *env, Type* switch_type)
{ {
if ( index_list ) if ( index_list )
{ {
foreach(i, ExprList, index_list) foreach(i, ExprList, index_list)
{ {
Expr *index_expr = *i; Expr *index_expr = *i;
Type* case_type = index_expr->DataType(env);
if ( case_type->tot() == Type::BUILTIN && case_type->StaticSize(env) > 4 )
throw ExceptionInvalidCaseSizeExpr(index_expr);
int index_const; int index_const;
if ( ! index_expr->ConstFold(env, &index_const) ) if ( ! index_expr->ConstFold(env, &index_const) )
throw ExceptionNonConstExpr(index_expr); throw ExceptionNonConstExpr(index_expr);
out_cc->println("case %d:", index_const);
// External C++ types like "int", "bool", "enum"
// all use "int" type internally by default.
int case_type_width = 4;
int switch_type_width = 4;
if ( switch_type->tot() == Type::BUILTIN )
switch_type_width = switch_type->StaticSize(env);
if ( case_type->tot() == Type::BUILTIN )
case_type_width = case_type->StaticSize(env);
if ( case_type_width > switch_type_width )
{
BuiltInType* st = (BuiltInType*)switch_type;
if ( switch_type_width == 1 )
{
if ( st->bit_type() == BuiltInType::INT8 )
{
if ( index_const < std::numeric_limits<int8_t>::min() )
throw ExceptionInvalidCaseLimitExpr(index_expr);
if ( index_const > std::numeric_limits<int8_t>::max() )
throw ExceptionInvalidCaseLimitExpr(index_expr);
}
else
{
if ( index_const < std::numeric_limits<uint8_t>::min() )
throw ExceptionInvalidCaseLimitExpr(index_expr);
if ( index_const > std::numeric_limits<uint8_t>::max() )
throw ExceptionInvalidCaseLimitExpr(index_expr);
}
}
else if ( switch_type_width == 2 )
{
if ( st->bit_type() == BuiltInType::INT16 )
{
if ( index_const < std::numeric_limits<int16_t>::min() )
throw ExceptionInvalidCaseLimitExpr(index_expr);
if ( index_const > std::numeric_limits<int16_t>::max() )
throw ExceptionInvalidCaseLimitExpr(index_expr);
}
else
{
if ( index_const < std::numeric_limits<uint16_t>::min() )
throw ExceptionInvalidCaseLimitExpr(index_expr);
if ( index_const > std::numeric_limits<uint16_t>::max() )
throw ExceptionInvalidCaseLimitExpr(index_expr);
}
}
}
// We're always using "int" for storage, so ok to just
// cast into the type used by the switch statement since
// some unsafe stuff is already checked above.
out_cc->println("case ((%s) %d):",
switch_type->DataTypeStr().c_str(), index_const);
} }
} }
else else
@ -296,7 +376,7 @@ void CaseField::GenPubDecls(Output* out_h, Env* env)
out_h->println("switch ( %s )", env->RValue(index_var_)); out_h->println("switch ( %s )", env->RValue(index_var_));
out_h->inc_indent(); out_h->inc_indent();
out_h->println("{"); out_h->println("{");
GenCaseStr(index_, out_h, env); GenCaseStr(index_, out_h, env, case_type()->IndexExpr()->DataType(env));
out_h->inc_indent(); out_h->inc_indent();
out_h->println("break; // OK"); out_h->println("break; // OK");
out_h->dec_indent(); out_h->dec_indent();
@ -304,7 +384,7 @@ void CaseField::GenPubDecls(Output* out_h, Env* env)
out_h->println("default:"); out_h->println("default:");
out_h->inc_indent(); out_h->inc_indent();
out_h->println( out_h->println(
"throw binpac::ExceptionInvalidCase(\"%s\", %s, \"%s\");", "throw binpac::ExceptionInvalidCase(\"%s\", (int64)%s, \"%s\");",
id_->LocName(), id_->LocName(),
env->RValue(index_var_), env->RValue(index_var_),
OrigExprList(index_).c_str()); OrigExprList(index_).c_str());
@ -335,7 +415,7 @@ void CaseField::GenInitCode(Output* out_cc, Env* env)
void CaseField::GenCleanUpCode(Output* out_cc, Env* env) void CaseField::GenCleanUpCode(Output* out_cc, Env* env)
{ {
GenCaseStr(index_, out_cc, env); GenCaseStr(index_, out_cc, env, case_type()->IndexExpr()->DataType(env));
out_cc->inc_indent(); out_cc->inc_indent();
out_cc->println("// Clean up \"%s\"", id_->Name()); out_cc->println("// Clean up \"%s\"", id_->Name());
out_cc->println("{"); out_cc->println("{");
@ -349,7 +429,7 @@ void CaseField::GenCleanUpCode(Output* out_cc, Env* env)
void CaseField::GenParseCode(Output* out_cc, Env* env, void CaseField::GenParseCode(Output* out_cc, Env* env,
const DataPtr& data, const ID* size_var) const DataPtr& data, const ID* size_var)
{ {
GenCaseStr(index_, out_cc, env); GenCaseStr(index_, out_cc, env, case_type()->IndexExpr()->DataType(env));
out_cc->inc_indent(); out_cc->inc_indent();
out_cc->println("// Parse \"%s\"", id_->Name()); out_cc->println("// Parse \"%s\"", id_->Name());
out_cc->println("{"); out_cc->println("{");

View file

@ -32,6 +32,8 @@ public:
Type *ValueType() const; Type *ValueType() const;
Expr* IndexExpr() const { return index_expr_; }
bool IsPointerType() const { return ValueType()->IsPointerType(); } bool IsPointerType() const { return ValueType()->IsPointerType(); }
protected: protected:
@ -94,6 +96,6 @@ protected:
// Generate a list of "case X:" lines from index_list. Each index // Generate a list of "case X:" lines from index_list. Each index
// expression must be constant foldable. // expression must be constant foldable.
void GenCaseStr(ExprList *index_list, Output *out_cc, Env *env); void GenCaseStr(ExprList *index_list, Output *out_cc, Env *env, Type* switch_type);
#endif // pac_case_h #endif // pac_case_h

View file

@ -69,3 +69,15 @@ ExceptionNonConstExpr::ExceptionNonConstExpr(const Expr* expr)
{ {
append(strfmt("Expression `%s' is not constant", expr->orig())); append(strfmt("Expression `%s' is not constant", expr->orig()));
} }
ExceptionInvalidCaseSizeExpr::ExceptionInvalidCaseSizeExpr(const Expr* expr)
: Exception(expr), expr(expr)
{
append(strfmt("Expression `%s' is greater than the 32-bit limit for use as a case index", expr->orig()));
}
ExceptionInvalidCaseLimitExpr::ExceptionInvalidCaseLimitExpr(const Expr* expr)
: Exception(expr), expr(expr)
{
append(strfmt("Expression `%s' as a case index is outside the numeric limit of the type used for the switch expression", expr->orig()));
}

View file

@ -92,4 +92,22 @@ private:
const Expr *expr; const Expr *expr;
}; };
class ExceptionInvalidCaseSizeExpr : public Exception
{
public:
ExceptionInvalidCaseSizeExpr(const Expr* expr);
private:
const Expr *expr;
};
class ExceptionInvalidCaseLimitExpr : public Exception
{
public:
ExceptionInvalidCaseLimitExpr(const Expr* expr);
private:
const Expr *expr;
};
#endif /* pac_exception_h */ #endif /* pac_exception_h */

View file

@ -257,6 +257,7 @@ void Expr::GenCaseEval(Output *out_cc, Env *env)
(*i)->value()->ForceIDEval(out_cc, env); (*i)->value()->ForceIDEval(out_cc, env);
out_cc->println("switch ( %s )", operand_[0]->EvalExpr(out_cc, env)); out_cc->println("switch ( %s )", operand_[0]->EvalExpr(out_cc, env));
Type* switch_type = operand_[0]->DataType(env);
out_cc->inc_indent(); out_cc->inc_indent();
out_cc->println("{"); out_cc->println("{");
@ -274,7 +275,7 @@ void Expr::GenCaseEval(Output *out_cc, Env *env)
} }
else else
{ {
GenCaseStr(index, out_cc, env); GenCaseStr(index, out_cc, env, switch_type);
out_cc->inc_indent(); out_cc->inc_indent();
out_cc->println("%s = %s;", out_cc->println("%s = %s;",
env->LValue(val_var), env->LValue(val_var),
@ -285,7 +286,7 @@ void Expr::GenCaseEval(Output *out_cc, Env *env)
} }
// Generate the default case after all other cases // Generate the default case after all other cases
GenCaseStr(0, out_cc, env); GenCaseStr(0, out_cc, env, switch_type);
out_cc->inc_indent(); out_cc->inc_indent();
if ( default_case ) if ( default_case )
{ {
@ -295,7 +296,7 @@ void Expr::GenCaseEval(Output *out_cc, Env *env)
} }
else else
{ {
out_cc->println("throw binpac::ExceptionInvalidCaseIndex(\"%s\", %s);", out_cc->println("throw binpac::ExceptionInvalidCaseIndex(\"%s\", (int64)%s);",
Location(), operand_[0]->EvalExpr(out_cc, env)); Location(), operand_[0]->EvalExpr(out_cc, env));
} }
out_cc->println("break;"); out_cc->println("break;");

View file

@ -126,7 +126,7 @@ LetDecl::LetDecl(ID *id, Type *type, Expr *expr)
Env *env = global_env(); Env *env = global_env();
int c; int c;
if ( expr_ && expr_->ConstFold(env, &c) ) if ( expr_ && expr_->ConstFold(env, &c) )
env->AddConstID(id_, c); env->AddConstID(id_, c, type);
else else
env->AddID(id_, GLOBAL_VAR, type_); env->AddID(id_, GLOBAL_VAR, type_);
} }