diff --git a/NEWS b/NEWS index 6dbfb75eb..417bcafe2 100644 --- a/NEWS +++ b/NEWS @@ -1,6 +1,9 @@ New in spot 2.1.1a (not yet released) - Nothing yet. + Bug fixes: + + - Fix spurious uninitialized read reported by valgrind when + is_Kleene_star() is compiled by clang++. New in spot 2.1.1 (2016-09-20) diff --git a/spot/tl/formula.cc b/spot/tl/formula.cc index 09460762d..8eb6cc3cd 100644 --- a/spot/tl/formula.cc +++ b/spot/tl/formula.cc @@ -1025,16 +1025,15 @@ namespace spot void fnode::setup_props(op o) { - op_ = o; id_ = next_id_++; // If the counter of formulae ever loops, we want to skip the // first three values, because they are permanently associated // to constants, and it is convenient to have constants // smaller than all other formulas. - if (next_id_ == 0) + if (SPOT_UNLIKELY(next_id_ == 0)) next_id_ = 3; - switch (op_) + switch (o) { case op::ff: case op::tt: diff --git a/spot/tl/formula.hh b/spot/tl/formula.hh index f96f45237..398fbb6ca 100644 --- a/spot/tl/formula.hh +++ b/spot/tl/formula.hh @@ -491,17 +491,32 @@ namespace spot template - fnode(op o, iter begin, iter end) - { - size_t s = std::distance(begin, end); - if (s > (size_t) UINT16_MAX) - throw std::runtime_error("too many children for formula"); - size_ = s; - auto pos = children; - for (auto i = begin; i != end; ++i) - *pos++ = *i; - setup_props(o); - } + fnode(op o, iter begin, iter end) + // Clang has some optimization where is it able to combine the + // 4 movb initializing op_,min_,max_,saturated_ into a single + // movl. Also it can optimize the three byte-comparisons of + // is_Kleene_star() into a single masked 32-bit comparison. + // The latter optimization triggers warnings from valgrind if + // min_ and max_ are not initialized. So to benefit from the + // initialization optimization and the is_Kleene_star() + // optimization in Clang, we always initialize min_ and max_ + // with this compiler. Do not do it the rest of the time, + // since the optimization is not done. + : op_(o), +#if __llvm__ + min_(0), max_(0), +#endif + saturated_(0) + { + size_t s = std::distance(begin, end); + if (s > (size_t) UINT16_MAX) + throw std::runtime_error("too many children for formula"); + size_ = s; + auto pos = children; + for (auto i = begin; i != end; ++i) + *pos++ = *i; + setup_props(o); + } fnode(op o, std::initializer_list l) : fnode(o, l.begin(), l.end()) @@ -509,11 +524,9 @@ namespace spot } fnode(op o, const fnode* f, uint8_t min, uint8_t max) + : op_(o), min_(min), max_(max), saturated_(0), size_(1) { - size_ = 1; children[0] = f; - min_ = min; - max_ = max; setup_props(o); } @@ -525,7 +538,7 @@ namespace spot op op_; // operator uint8_t min_; // range minimum (for star-like operators) uint8_t max_; // range maximum; - mutable uint8_t saturated_ = 0; + mutable uint8_t saturated_; uint16_t size_; // number of children mutable uint16_t refs_ = 0; // reference count - 1; size_t id_; // Also used as hash.