Skip to content

Commit ccfee53

Browse files
committed
[MLIR] Fix use-after-move for DEBUG builds
Fix use-after-move issues that occur only in DEBUG builds. In one, constructor arguments are moved in the initializer list to initialize member variables, but the function body mistakenly accesses the moved-from arguments instead of the corresponding class members with identical names. While this is a UB, here it actually renders the affected asserts ineffective, since the comparisons operate on two hollowed-out objects, by implementation. In a second fix, a variable is moved from within an assert, introducing a side effect that alters its subsequent use and causes divergence between DEBUG and RELEASE builds. Testing: ninja check-llvm ninja check-llvm-unit
1 parent df970d5 commit ccfee53

File tree

2 files changed

+5
-7
lines changed

2 files changed

+5
-7
lines changed

mlir/include/mlir/Dialect/SparseTensor/IR/SparseTensor.h

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -158,16 +158,14 @@ namespace sparse_tensor {
158158
/// Convenience method to abbreviate casting `getType()`.
159159
template <typename T>
160160
inline RankedTensorType getRankedTensorType(T &&t) {
161-
assert(static_cast<bool>(std::forward<T>(t)) &&
162-
"getRankedTensorType got null argument");
161+
assert(static_cast<bool>(t) && "getRankedTensorType got null argument");
163162
return dyn_cast<RankedTensorType>(std::forward<T>(t).getType());
164163
}
165164

166165
/// Convenience method to abbreviate casting `getType()`.
167166
template <typename T>
168167
inline MemRefType getMemRefType(T &&t) {
169-
assert(static_cast<bool>(std::forward<T>(t)) &&
170-
"getMemRefType got null argument");
168+
assert(static_cast<bool>(t) && "getMemRefType got null argument");
171169
return cast<MemRefType>(std::forward<T>(t).getType());
172170
}
173171

mlir/lib/Dialect/SparseTensor/Transforms/Utils/IterationGraphSorter.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -159,12 +159,12 @@ IterationGraphSorter::IterationGraphSorter(
159159
loop2OutLvl(loop2OutLvl), iterTypes(std::move(iterTypes)),
160160
strategy(strategy) {
161161
// One map per tensor.
162-
assert(loop2InsLvl.size() == ins.size());
162+
assert(this->loop2InsLvl.size() == this->ins.size());
163163
// All the affine maps have the same number of dimensions (loops).
164164
assert(llvm::all_equal(llvm::map_range(
165-
loop2InsLvl, [](AffineMap m) { return m.getNumDims(); })));
165+
this->loop2InsLvl, [](AffineMap m) { return m.getNumDims(); })));
166166
// The number of results of the map should match the rank of the tensor.
167-
assert(llvm::all_of(llvm::zip(loop2InsLvl, ins), [](auto mvPair) {
167+
assert(llvm::all_of(llvm::zip(this->loop2InsLvl, this->ins), [](auto mvPair) {
168168
auto [m, v] = mvPair;
169169
return m.getNumResults() == cast<ShapedType>(v.getType()).getRank();
170170
}));

0 commit comments

Comments
 (0)