From fbc25f3dfff942a7513fd328793373bd3677377e Mon Sep 17 00:00:00 2001 From: Ryan Macnak Date: Tue, 30 Mar 2021 17:06:16 +0000 Subject: [PATCH] [vm, compiler] Avoid undefined behavior when tracking induction variables wraps around. TEST=ubsan Bug: https://github.com/dart-lang/sdk/issues/45511 Change-Id: Iaa5733dc048a811c87f479fa54fdb89bf64a0373 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/193443 Reviewed-by: Alexander Markov Commit-Queue: Ryan Macnak --- runtime/platform/utils.h | 7 +++++++ runtime/vm/compiler/backend/loops.cc | 21 ++++++++++++++------- 2 files changed, 21 insertions(+), 7 deletions(-) diff --git a/runtime/platform/utils.h b/runtime/platform/utils.h index 788dd2a8912..a38150566b4 100644 --- a/runtime/platform/utils.h +++ b/runtime/platform/utils.h @@ -292,6 +292,13 @@ class Utils { return static_cast(static_cast(a) * static_cast(b)); } + template + static inline T NegWithWrapAround(T a) { + // Avoid undefined behavior by doing arithmetic in the unsigned type. + using Unsigned = typename std::make_unsigned::type; + return static_cast(-static_cast(a)); + } + // Shifts int64_t value left. Supports any non-negative number of bits and // silently discards shifted out bits. static inline int64_t ShiftLeftWithTruncation(int64_t a, int64_t b) { diff --git a/runtime/vm/compiler/backend/loops.cc b/runtime/vm/compiler/backend/loops.cc index 4ff2dc8805c..f28ae39440c 100644 --- a/runtime/vm/compiler/backend/loops.cc +++ b/runtime/vm/compiler/backend/loops.cc @@ -723,13 +723,16 @@ InductionVar* InductionVarAnalysis::Add(InductionVar* x, InductionVar* y) { // Invariant + Invariant : only for same or just one instruction. if (x->def_ == y->def_) { return new (zone_) - InductionVar(x->offset_ + y->offset_, x->mult_ + y->mult_, x->def_); + InductionVar(Utils::AddWithWrapAround(x->offset_, y->offset_), + Utils::AddWithWrapAround(x->mult_, y->mult_), x->def_); } else if (y->mult_ == 0) { return new (zone_) - InductionVar(x->offset_ + y->offset_, x->mult_, x->def_); + InductionVar(Utils::AddWithWrapAround(x->offset_, y->offset_), + x->mult_, x->def_); } else if (x->mult_ == 0) { return new (zone_) - InductionVar(x->offset_ + y->offset_, y->mult_, y->def_); + InductionVar(Utils::AddWithWrapAround(x->offset_, y->offset_), + y->mult_, y->def_); } } else if (y != nullptr) { // Invariant + Induction. @@ -768,13 +771,16 @@ InductionVar* InductionVarAnalysis::Sub(InductionVar* x, InductionVar* y) { // Invariant + Invariant : only for same or just one instruction. if (x->def_ == y->def_) { return new (zone_) - InductionVar(x->offset_ - y->offset_, x->mult_ - y->mult_, x->def_); + InductionVar(Utils::SubWithWrapAround(x->offset_, y->offset_), + Utils::SubWithWrapAround(x->mult_, y->mult_), x->def_); } else if (y->mult_ == 0) { return new (zone_) - InductionVar(x->offset_ - y->offset_, x->mult_, x->def_); + InductionVar(Utils::SubWithWrapAround(x->offset_, y->offset_), + x->mult_, x->def_); } else if (x->mult_ == 0) { return new (zone_) - InductionVar(x->offset_ - y->offset_, -y->mult_, y->def_); + InductionVar(Utils::SubWithWrapAround(x->offset_, y->offset_), + Utils::NegWithWrapAround(y->mult_), y->def_); } } else if (y != nullptr) { // Invariant - Induction. @@ -823,7 +829,8 @@ InductionVar* InductionVarAnalysis::Mul(InductionVar* x, InductionVar* y) { if (InductionVar::IsConstant(x) && y != nullptr) { if (y->kind_ == InductionVar::kInvariant) { return new (zone_) - InductionVar(x->offset_ * y->offset_, x->offset_ * y->mult_, y->def_); + InductionVar(Utils::MulWithWrapAround(x->offset_, y->offset_), + Utils::MulWithWrapAround(x->offset_, y->mult_), y->def_); } return new (zone_) InductionVar(y->kind_, Mul(x, y->initial_), Mul(x, y->next_));