From 1fc40931913dc99f58244eef1458e4ad5ef3e7de Mon Sep 17 00:00:00 2001 From: Stephen Williams Date: Wed, 15 Jan 2014 14:26:53 -0800 Subject: [PATCH] Fix possible overflow in calculation of canonical indices. --- elab_expr.cc | 2 +- elab_lval.cc | 26 ++++++++++++++++++++++- elab_net.cc | 2 +- netmisc.cc | 58 +++++++++++++++++++++++++++++++++++++++++++++------- netmisc.h | 8 ++++++++ 5 files changed, 86 insertions(+), 10 deletions(-) diff --git a/elab_expr.cc b/elab_expr.cc index dc0d8de69..d2af0b178 100644 --- a/elab_expr.cc +++ b/elab_expr.cc @@ -3964,7 +3964,7 @@ NetExpr* PEIdent::elaborate_expr_net_word_(Design*des, NetScope*scope, indices_flags idx_flags; indices_to_expressions(des, scope, this, name_tail.index, net->unpacked_dimensions(), - need_const, + need_const, net->unpacked_count(), idx_flags, unpacked_indices, unpacked_indices_const); diff --git a/elab_lval.cc b/elab_lval.cc index 13edb26c0..5ccda9a9e 100644 --- a/elab_lval.cc +++ b/elab_lval.cc @@ -201,6 +201,11 @@ NetAssign_* PEIdent::elaborate_lval(Design*des, NetEvent* eve = 0; perm_string method_name; + if (debug_elaborate) { + cerr << get_fileline() << ": PEIdent::elaborate_lval: " + << "Elaborate l-value ident expression: " << *this << endl; + } + /* Try to detect the special case that we are in a method and the identifier is a member of the class. */ if (NetAssign_*tmp = elaborate_lval_method_class_member_(des, scope)) @@ -259,6 +264,13 @@ NetAssign_* PEIdent::elaborate_lval(Design*des, } ivl_assert(*this, reg); + + if (debug_elaborate) { + cerr << get_fileline() << ": PEIdent::elaborate_lval: " + << "Found l-value as reg." + << " unpacked_dimensions()=" << reg->unpacked_dimensions() << endl; + } + // We are processing the tail of a string of names. For // example, the verilog may be "a.b.c", so we are processing // "c" at this point. (Note that if method_name is not nil, @@ -440,6 +452,11 @@ NetAssign_* PEIdent::elaborate_lval_net_word_(Design*des, const name_component_t&name_tail = path_.back(); ivl_assert(*this, !name_tail.index.empty()); + if (debug_elaborate) { + cerr << get_fileline() << ": PEIdent::elaborate_lval_net_word_: " + << "Handle as n-dimensional array." << endl; + } + if (name_tail.index.size() < reg->unpacked_dimensions()) { cerr << get_fileline() << ": error: Array " << reg->name() << " needs " << reg->unpacked_dimensions() << " indices," @@ -448,6 +465,8 @@ NetAssign_* PEIdent::elaborate_lval_net_word_(Design*des, return 0; } + unsigned array_need_words = reg->unpacked_count(); + // Make sure there are enough indices to address an array element. const index_component_t&index_head = name_tail.index.front(); if (index_head.sel == index_component_t::SEL_PART) { @@ -465,7 +484,7 @@ NetAssign_* PEIdent::elaborate_lval_net_word_(Design*des, indices_flags flags; indices_to_expressions(des, scope, this, name_tail.index, reg->unpacked_dimensions(), - false, + false, array_need_words, flags, unpacked_indices, unpacked_indices_const); @@ -507,6 +526,11 @@ NetAssign_* PEIdent::elaborate_lval_net_word_(Design*des, canon_index = new NetEConst(verinum(verinum::Vx)); canon_index->set_line(*this); + if (debug_elaborate) { + cerr << get_fileline() << ": PEIdent::elaborate_lval_net_word_: " + << "canon_index=" << *canon_index << endl; + } + NetAssign_*lv = new NetAssign_(reg); lv->set_word(canon_index); diff --git a/elab_net.cc b/elab_net.cc index 69f0a857e..335735578 100644 --- a/elab_net.cc +++ b/elab_net.cc @@ -607,7 +607,7 @@ NetNet* PEIdent::elaborate_lnet_common_(Design*des, NetScope*scope, indices_flags flags; indices_to_expressions(des, scope, this, path_tail.index, sig->unpacked_dimensions(), - true, + true, sig->unpacked_count(), flags, unpacked_indices, unpacked_indices_const); diff --git a/netmisc.cc b/netmisc.cc index 351e32f24..8adb4c507 100644 --- a/netmisc.cc +++ b/netmisc.cc @@ -468,12 +468,21 @@ void indices_to_expressions(Design*des, NetScope*scope, const list&src, unsigned count, // True if the expression MUST be constant. bool need_const, + // Total words in target array + unsigned need_addr, // These are the outputs. indices_flags&flags, list&indices, list&indices_const) { ivl_assert(*loc, count <= src.size()); + int need_wid = ceil(log2(need_addr)) + 1; + if (debug_elaborate) { + cerr << loc->get_fileline() << ": indices_to_expressions: " + << "To address " << need_addr << " words, " + << "we need " << need_wid << " bits." << endl; + } + flags.invalid = false; flags.variable = false; flags.undefined = false; @@ -488,7 +497,7 @@ void indices_to_expressions(Design*des, NetScope*scope, } ivl_assert(*loc, cur->msb); - NetExpr*word_index = elab_and_eval(des, scope, cur->msb, -1, need_const); + NetExpr*word_index = elab_and_eval_min_width(des, scope, cur->msb, need_wid, need_const); if (word_index == 0) flags.invalid = true; @@ -730,13 +739,16 @@ NetExpr* condition_reduce(NetExpr*expr) return cmp; } -NetExpr* elab_and_eval(Design*des, NetScope*scope, PExpr*pe, - int context_width, bool need_const, bool annotatable, - ivl_variable_type_t cast_type) +static NetExpr* do_elab_and_eval(Design*des, NetScope*scope, PExpr*pe, + int context_width, bool need_const, bool annotatable, + bool width_is_min, + ivl_variable_type_t cast_type) { PExpr::width_mode_t mode = PExpr::SIZED; if ((context_width == -2) && !gn_strict_expr_width_flag) mode = PExpr::EXPAND; + if (width_is_min) + mode = PExpr::EXPAND; pe->test_width(des, scope, mode); @@ -755,8 +767,10 @@ NetExpr* elab_and_eval(Design*des, NetScope*scope, PExpr*pe, << *pe << endl; cerr << pe->get_fileline() << ": : " << "returns type=" << pe->expr_type() - << ", width=" << expr_width + << ", context_width=" << context_width << ", signed=" << pe->has_sign() + << ", width_is_min=" << width_is_min + << ", expr_width=" << expr_width << ", mode=" << PExpr::width_mode_name(mode) << endl; cerr << pe->get_fileline() << ": : " << "cast_type=" << cast_type << endl; @@ -764,7 +778,8 @@ NetExpr* elab_and_eval(Design*des, NetScope*scope, PExpr*pe, // If we can get the same result using a smaller expression // width, do so. - if ((context_width > 0) && (pe->expr_type() != IVL_VT_REAL) + if ((context_width > 0) && (!width_is_min) + && (pe->expr_type() != IVL_VT_REAL) && (expr_width > (unsigned)context_width)) { expr_width = max(pe->min_width(), (unsigned)context_width); @@ -780,6 +795,11 @@ NetExpr* elab_and_eval(Design*des, NetScope*scope, PExpr*pe, if (annotatable) flags |= PExpr::ANNOTATABLE; + if (debug_elaborate) { + cerr << pe->get_fileline() << ": elab_and_eval: " + << "Calculated width is " << expr_width << "." << endl; + } + NetExpr*tmp = pe->elaborate_expr(des, scope, expr_width, flags); if (tmp == 0) return 0; @@ -799,6 +819,13 @@ NetExpr* elab_and_eval(Design*des, NetScope*scope, PExpr*pe, } } + // If the context_width sent is is actually the minimim width, + // then raise the context_width to be big enough for the + // lossless expression. + if (width_is_min && context_width > 0) { + context_width = max(context_width, (int)expr_width); + } + eval_expr(tmp, context_width); if (NetEConst*ce = dynamic_cast(tmp)) { @@ -809,6 +836,23 @@ NetExpr* elab_and_eval(Design*des, NetScope*scope, PExpr*pe, return tmp; } +NetExpr* elab_and_eval(Design*des, NetScope*scope, PExpr*pe, + int context_width, bool need_const, bool annotatable, + ivl_variable_type_t cast_type) +{ + return do_elab_and_eval(des, scope, pe, context_width, + need_const, annotatable, false, cast_type); +} + +NetExpr* elab_and_eval_min_width(Design*des, NetScope*scope, PExpr*pe, + int context_width, bool need_const, bool annotatable, + ivl_variable_type_t cast_type) +{ + ivl_assert(*pe, context_width > 0); + return do_elab_and_eval(des, scope, pe, context_width, + need_const, annotatable, true, cast_type); +} + NetExpr* elab_and_eval(Design*des, NetScope*scope, PExpr*pe, ivl_type_t lv_net_type, bool need_const) { @@ -1262,7 +1306,7 @@ NetExpr*collapse_array_exprs(Design*des, NetScope*scope, indices_flags flags; indices_to_expressions(des, scope, loc, indices, net->packed_dimensions(), - false, flags, exprs, exprs_const); + false, net->unpacked_count(), flags, exprs, exprs_const); ivl_assert(*loc, exprs.size() == net->packed_dimensions()); // Special Case: there is only 1 packed dimension, so the diff --git a/netmisc.h b/netmisc.h index 90d73cb9c..024445248 100644 --- a/netmisc.h +++ b/netmisc.h @@ -179,6 +179,8 @@ extern void indices_to_expressions(Design*des, NetScope*scope, const list&src, unsigned count, // True if the expression MUST be constant. bool need_const, + // Total array size, for sizing expressions + unsigned need_addr, // These are the outputs. indices_flags&flags, list&indices,list&indices_const); @@ -244,6 +246,12 @@ extern NetExpr* elab_and_eval(Design*des, NetScope*scope, bool annotatable =false, ivl_variable_type_t cast_type =IVL_VT_NO_TYPE); +extern NetExpr* elab_and_eval_min_width(Design*des, NetScope*scope, + PExpr*pe, int context_width, + bool need_const =false, + bool annotatable =false, + ivl_variable_type_t cast_type =IVL_VT_NO_TYPE); + /* * This form of elab_and_eval uses the ivl_type_t to carry type * information instead of the piecemeal form. We should transition to