[mlir] Make single value ValueRanges memory safer
#121996
Merged
+66
−16
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
A very common mistake users (and yours truly) make when using `ValueRange`s is assigning a temporary `Value` to it. Example: ```cpp ValueRange values = op.getOperand(); apiThatUsesValueRange(values); ``` The issue is caused by the implicit `const Value&` constructor: As per C++ rules a const reference can be constructed from a temporary and the address of it taken. After the statement, the temporary goes out of scope and `stack-use-after-free` error occurs. This PR fixes that issue by making `ValueRange` capable of owning a single `Value` instance for that case specifically. While technically a departure from the other owner types that are non-owning, I'd argue that this behavior is more intuitive for the majority of users that usually don't need to care about the lifetime of `Value` instances. `TypeRange` has similarly been adopted to accept a single `Type` instance to implement `getTypes`.
|
@llvm/pr-subscribers-mlir-core Author: Markus Böck (zero9178) ChangesA very common mistake users (and yours truly) make when using ValueRange values = op.getOperand();
apiThatUsesValueRange(values);The issue is caused by the implicit This PR fixes that issue by making
Full diff: https://github.com/llvm/llvm-project/pull/121996.diff 5 Files Affected:
diff --git a/mlir/include/mlir/IR/TypeRange.h b/mlir/include/mlir/IR/TypeRange.h
index 99fabab334f922..3a255583e28583 100644
--- a/mlir/include/mlir/IR/TypeRange.h
+++ b/mlir/include/mlir/IR/TypeRange.h
@@ -31,9 +31,9 @@ namespace mlir {
/// parameter.
class TypeRange : public llvm::detail::indexed_accessor_range_base<
TypeRange,
- llvm::PointerUnion<const Value *, const Type *,
- OpOperand *, detail::OpResultImpl *>,
- Type, Type, Type> {
+ llvm::PointerUnion<const Value *, const Type *, OpOperand *,
+ detail::OpResultImpl *, Type>,
+ Type, Type, Type> {
public:
using RangeBaseT::RangeBaseT;
TypeRange(ArrayRef<Type> types = std::nullopt);
@@ -44,8 +44,11 @@ class TypeRange : public llvm::detail::indexed_accessor_range_base<
TypeRange(ValueTypeRange<ValueRangeT> values)
: TypeRange(ValueRange(ValueRangeT(values.begin().getCurrent(),
values.end().getCurrent()))) {}
- template <typename Arg, typename = std::enable_if_t<std::is_constructible<
- ArrayRef<Type>, Arg>::value>>
+
+ TypeRange(Type type) : TypeRange(type, /*count=*/1) {}
+ template <typename Arg, typename = std::enable_if_t<
+ std::is_constructible_v<ArrayRef<Type>, Arg> &&
+ !std::is_constructible_v<Type, Arg>>>
TypeRange(Arg &&arg) : TypeRange(ArrayRef<Type>(std::forward<Arg>(arg))) {}
TypeRange(std::initializer_list<Type> types)
: TypeRange(ArrayRef<Type>(types)) {}
@@ -56,8 +59,9 @@ class TypeRange : public llvm::detail::indexed_accessor_range_base<
/// * A pointer to the first element of an array of types.
/// * A pointer to the first element of an array of operands.
/// * A pointer to the first element of an array of results.
+ /// * A single 'Type' instance.
using OwnerT = llvm::PointerUnion<const Value *, const Type *, OpOperand *,
- detail::OpResultImpl *>;
+ detail::OpResultImpl *, Type>;
/// See `llvm::detail::indexed_accessor_range_base` for details.
static OwnerT offset_base(OwnerT object, ptrdiff_t index);
diff --git a/mlir/include/mlir/IR/ValueRange.h b/mlir/include/mlir/IR/ValueRange.h
index 4b421c08d8418e..f878abd63de35f 100644
--- a/mlir/include/mlir/IR/ValueRange.h
+++ b/mlir/include/mlir/IR/ValueRange.h
@@ -377,13 +377,14 @@ class ResultRange::UseIterator final
class ValueRange final
: public llvm::detail::indexed_accessor_range_base<
ValueRange,
- PointerUnion<const Value *, OpOperand *, detail::OpResultImpl *>,
- Value, Value, Value> {
+ PointerUnion<const Value *, OpOperand *,
+ detail::OpResultImpl *, Value>,
+ Value, Value, Value> {
public:
/// The type representing the owner of a ValueRange. This is either a list of
- /// values, operands, or results.
+ /// values, operands, or results or a single value.
using OwnerT =
- PointerUnion<const Value *, OpOperand *, detail::OpResultImpl *>;
+ PointerUnion<const Value *, OpOperand *, detail::OpResultImpl *, Value>;
using RangeBaseT::RangeBaseT;
@@ -392,7 +393,7 @@ class ValueRange final
std::is_constructible<ArrayRef<Value>, Arg>::value &&
!std::is_convertible<Arg, Value>::value>>
ValueRange(Arg &&arg) : ValueRange(ArrayRef<Value>(std::forward<Arg>(arg))) {}
- ValueRange(const Value &value) : ValueRange(&value, /*count=*/1) {}
+ ValueRange(Value value) : ValueRange(value, /*count=*/1) {}
ValueRange(const std::initializer_list<Value> &values)
: ValueRange(ArrayRef<Value>(values)) {}
ValueRange(iterator_range<OperandRange::iterator> values)
diff --git a/mlir/lib/IR/OperationSupport.cpp b/mlir/lib/IR/OperationSupport.cpp
index 957195202d78d2..803fcd8d18fbd5 100644
--- a/mlir/lib/IR/OperationSupport.cpp
+++ b/mlir/lib/IR/OperationSupport.cpp
@@ -653,6 +653,15 @@ ValueRange::ValueRange(ResultRange values)
/// See `llvm::detail::indexed_accessor_range_base` for details.
ValueRange::OwnerT ValueRange::offset_base(const OwnerT &owner,
ptrdiff_t index) {
+ if (llvm::isa_and_nonnull<Value>(owner)) {
+ // Prevent out-of-bounds indexing for single values.
+ // Note that we do allow an index of 1 as is required by 'slice'ing that
+ // returns an empty range. This also matches the usual rules of C++ of being
+ // allowed to index past the last element of an array.
+ assert(index <= 1 && "out-of-bound offset into single-value 'ValueRange'");
+ // Return nullptr to quickly cause segmentation faults on misuse.
+ return index == 0 ? owner : nullptr;
+ }
if (const auto *value = llvm::dyn_cast_if_present<const Value *>(owner))
return {value + index};
if (auto *operand = llvm::dyn_cast_if_present<OpOperand *>(owner))
@@ -661,6 +670,10 @@ ValueRange::OwnerT ValueRange::offset_base(const OwnerT &owner,
}
/// See `llvm::detail::indexed_accessor_range_base` for details.
Value ValueRange::dereference_iterator(const OwnerT &owner, ptrdiff_t index) {
+ if (auto value = llvm::dyn_cast_if_present<Value>(owner)) {
+ assert(index == 0 && "cannot offset into single-value 'ValueRange'");
+ return value;
+ }
if (const auto *value = llvm::dyn_cast_if_present<const Value *>(owner))
return value[index];
if (auto *operand = llvm::dyn_cast_if_present<OpOperand *>(owner))
diff --git a/mlir/lib/IR/TypeRange.cpp b/mlir/lib/IR/TypeRange.cpp
index f8878303727d4f..7e5f99c884512e 100644
--- a/mlir/lib/IR/TypeRange.cpp
+++ b/mlir/lib/IR/TypeRange.cpp
@@ -31,12 +31,23 @@ TypeRange::TypeRange(ValueRange values) : TypeRange(OwnerT(), values.size()) {
this->base = result;
else if (auto *operand = llvm::dyn_cast_if_present<OpOperand *>(owner))
this->base = operand;
+ else if (auto value = llvm::dyn_cast_if_present<Value>(owner))
+ this->base = value.getType();
else
this->base = cast<const Value *>(owner);
}
/// See `llvm::detail::indexed_accessor_range_base` for details.
TypeRange::OwnerT TypeRange::offset_base(OwnerT object, ptrdiff_t index) {
+ if (llvm::isa_and_nonnull<Type>(object)) {
+ // Prevent out-of-bounds indexing for single values.
+ // Note that we do allow an index of 1 as is required by 'slice'ing that
+ // returns an empty range. This also matches the usual rules of C++ of being
+ // allowed to index past the last element of an array.
+ assert(index <= 1 && "out-of-bound offset into single-value 'ValueRange'");
+ // Return nullptr to quickly cause segmentation faults on misuse.
+ return index == 0 ? object : nullptr;
+ }
if (const auto *value = llvm::dyn_cast_if_present<const Value *>(object))
return {value + index};
if (auto *operand = llvm::dyn_cast_if_present<OpOperand *>(object))
@@ -48,6 +59,10 @@ TypeRange::OwnerT TypeRange::offset_base(OwnerT object, ptrdiff_t index) {
/// See `llvm::detail::indexed_accessor_range_base` for details.
Type TypeRange::dereference_iterator(OwnerT object, ptrdiff_t index) {
+ if (auto type = llvm::dyn_cast_if_present<Type>(object)) {
+ assert(index == 0 && "cannot offset into single-value 'TypeRange'");
+ return type;
+ }
if (const auto *value = llvm::dyn_cast_if_present<const Value *>(object))
return (value + index)->getType();
if (auto *operand = llvm::dyn_cast_if_present<OpOperand *>(object))
diff --git a/mlir/unittests/IR/OperationSupportTest.cpp b/mlir/unittests/IR/OperationSupportTest.cpp
index f94dc784458077..2a1b8d2ef7f55b 100644
--- a/mlir/unittests/IR/OperationSupportTest.cpp
+++ b/mlir/unittests/IR/OperationSupportTest.cpp
@@ -313,4 +313,21 @@ TEST(OperationEquivalenceTest, HashWorksWithFlags) {
op2->destroy();
}
+TEST(ValueRangeTest, ValueConstructable) {
+ MLIRContext context;
+ Builder builder(&context);
+
+ Operation *useOp =
+ createOp(&context, /*operands=*/std::nullopt, builder.getIntegerType(16));
+ // Valid construction despite a temporary 'OpResult'.
+ ValueRange operands = useOp->getResult(0);
+
+ useOp->setOperands(operands);
+ EXPECT_EQ(useOp->getNumOperands(), 1u);
+ EXPECT_EQ(useOp->getOperand(0), useOp->getResult(0));
+
+ useOp->dropAllUses();
+ useOp->destroy();
+}
+
} // namespace
|
|
@llvm/pr-subscribers-mlir Author: Markus Böck (zero9178) ChangesA very common mistake users (and yours truly) make when using ValueRange values = op.getOperand();
apiThatUsesValueRange(values);The issue is caused by the implicit This PR fixes that issue by making
Full diff: https://github.com/llvm/llvm-project/pull/121996.diff 5 Files Affected:
diff --git a/mlir/include/mlir/IR/TypeRange.h b/mlir/include/mlir/IR/TypeRange.h
index 99fabab334f922..3a255583e28583 100644
--- a/mlir/include/mlir/IR/TypeRange.h
+++ b/mlir/include/mlir/IR/TypeRange.h
@@ -31,9 +31,9 @@ namespace mlir {
/// parameter.
class TypeRange : public llvm::detail::indexed_accessor_range_base<
TypeRange,
- llvm::PointerUnion<const Value *, const Type *,
- OpOperand *, detail::OpResultImpl *>,
- Type, Type, Type> {
+ llvm::PointerUnion<const Value *, const Type *, OpOperand *,
+ detail::OpResultImpl *, Type>,
+ Type, Type, Type> {
public:
using RangeBaseT::RangeBaseT;
TypeRange(ArrayRef<Type> types = std::nullopt);
@@ -44,8 +44,11 @@ class TypeRange : public llvm::detail::indexed_accessor_range_base<
TypeRange(ValueTypeRange<ValueRangeT> values)
: TypeRange(ValueRange(ValueRangeT(values.begin().getCurrent(),
values.end().getCurrent()))) {}
- template <typename Arg, typename = std::enable_if_t<std::is_constructible<
- ArrayRef<Type>, Arg>::value>>
+
+ TypeRange(Type type) : TypeRange(type, /*count=*/1) {}
+ template <typename Arg, typename = std::enable_if_t<
+ std::is_constructible_v<ArrayRef<Type>, Arg> &&
+ !std::is_constructible_v<Type, Arg>>>
TypeRange(Arg &&arg) : TypeRange(ArrayRef<Type>(std::forward<Arg>(arg))) {}
TypeRange(std::initializer_list<Type> types)
: TypeRange(ArrayRef<Type>(types)) {}
@@ -56,8 +59,9 @@ class TypeRange : public llvm::detail::indexed_accessor_range_base<
/// * A pointer to the first element of an array of types.
/// * A pointer to the first element of an array of operands.
/// * A pointer to the first element of an array of results.
+ /// * A single 'Type' instance.
using OwnerT = llvm::PointerUnion<const Value *, const Type *, OpOperand *,
- detail::OpResultImpl *>;
+ detail::OpResultImpl *, Type>;
/// See `llvm::detail::indexed_accessor_range_base` for details.
static OwnerT offset_base(OwnerT object, ptrdiff_t index);
diff --git a/mlir/include/mlir/IR/ValueRange.h b/mlir/include/mlir/IR/ValueRange.h
index 4b421c08d8418e..f878abd63de35f 100644
--- a/mlir/include/mlir/IR/ValueRange.h
+++ b/mlir/include/mlir/IR/ValueRange.h
@@ -377,13 +377,14 @@ class ResultRange::UseIterator final
class ValueRange final
: public llvm::detail::indexed_accessor_range_base<
ValueRange,
- PointerUnion<const Value *, OpOperand *, detail::OpResultImpl *>,
- Value, Value, Value> {
+ PointerUnion<const Value *, OpOperand *,
+ detail::OpResultImpl *, Value>,
+ Value, Value, Value> {
public:
/// The type representing the owner of a ValueRange. This is either a list of
- /// values, operands, or results.
+ /// values, operands, or results or a single value.
using OwnerT =
- PointerUnion<const Value *, OpOperand *, detail::OpResultImpl *>;
+ PointerUnion<const Value *, OpOperand *, detail::OpResultImpl *, Value>;
using RangeBaseT::RangeBaseT;
@@ -392,7 +393,7 @@ class ValueRange final
std::is_constructible<ArrayRef<Value>, Arg>::value &&
!std::is_convertible<Arg, Value>::value>>
ValueRange(Arg &&arg) : ValueRange(ArrayRef<Value>(std::forward<Arg>(arg))) {}
- ValueRange(const Value &value) : ValueRange(&value, /*count=*/1) {}
+ ValueRange(Value value) : ValueRange(value, /*count=*/1) {}
ValueRange(const std::initializer_list<Value> &values)
: ValueRange(ArrayRef<Value>(values)) {}
ValueRange(iterator_range<OperandRange::iterator> values)
diff --git a/mlir/lib/IR/OperationSupport.cpp b/mlir/lib/IR/OperationSupport.cpp
index 957195202d78d2..803fcd8d18fbd5 100644
--- a/mlir/lib/IR/OperationSupport.cpp
+++ b/mlir/lib/IR/OperationSupport.cpp
@@ -653,6 +653,15 @@ ValueRange::ValueRange(ResultRange values)
/// See `llvm::detail::indexed_accessor_range_base` for details.
ValueRange::OwnerT ValueRange::offset_base(const OwnerT &owner,
ptrdiff_t index) {
+ if (llvm::isa_and_nonnull<Value>(owner)) {
+ // Prevent out-of-bounds indexing for single values.
+ // Note that we do allow an index of 1 as is required by 'slice'ing that
+ // returns an empty range. This also matches the usual rules of C++ of being
+ // allowed to index past the last element of an array.
+ assert(index <= 1 && "out-of-bound offset into single-value 'ValueRange'");
+ // Return nullptr to quickly cause segmentation faults on misuse.
+ return index == 0 ? owner : nullptr;
+ }
if (const auto *value = llvm::dyn_cast_if_present<const Value *>(owner))
return {value + index};
if (auto *operand = llvm::dyn_cast_if_present<OpOperand *>(owner))
@@ -661,6 +670,10 @@ ValueRange::OwnerT ValueRange::offset_base(const OwnerT &owner,
}
/// See `llvm::detail::indexed_accessor_range_base` for details.
Value ValueRange::dereference_iterator(const OwnerT &owner, ptrdiff_t index) {
+ if (auto value = llvm::dyn_cast_if_present<Value>(owner)) {
+ assert(index == 0 && "cannot offset into single-value 'ValueRange'");
+ return value;
+ }
if (const auto *value = llvm::dyn_cast_if_present<const Value *>(owner))
return value[index];
if (auto *operand = llvm::dyn_cast_if_present<OpOperand *>(owner))
diff --git a/mlir/lib/IR/TypeRange.cpp b/mlir/lib/IR/TypeRange.cpp
index f8878303727d4f..7e5f99c884512e 100644
--- a/mlir/lib/IR/TypeRange.cpp
+++ b/mlir/lib/IR/TypeRange.cpp
@@ -31,12 +31,23 @@ TypeRange::TypeRange(ValueRange values) : TypeRange(OwnerT(), values.size()) {
this->base = result;
else if (auto *operand = llvm::dyn_cast_if_present<OpOperand *>(owner))
this->base = operand;
+ else if (auto value = llvm::dyn_cast_if_present<Value>(owner))
+ this->base = value.getType();
else
this->base = cast<const Value *>(owner);
}
/// See `llvm::detail::indexed_accessor_range_base` for details.
TypeRange::OwnerT TypeRange::offset_base(OwnerT object, ptrdiff_t index) {
+ if (llvm::isa_and_nonnull<Type>(object)) {
+ // Prevent out-of-bounds indexing for single values.
+ // Note that we do allow an index of 1 as is required by 'slice'ing that
+ // returns an empty range. This also matches the usual rules of C++ of being
+ // allowed to index past the last element of an array.
+ assert(index <= 1 && "out-of-bound offset into single-value 'ValueRange'");
+ // Return nullptr to quickly cause segmentation faults on misuse.
+ return index == 0 ? object : nullptr;
+ }
if (const auto *value = llvm::dyn_cast_if_present<const Value *>(object))
return {value + index};
if (auto *operand = llvm::dyn_cast_if_present<OpOperand *>(object))
@@ -48,6 +59,10 @@ TypeRange::OwnerT TypeRange::offset_base(OwnerT object, ptrdiff_t index) {
/// See `llvm::detail::indexed_accessor_range_base` for details.
Type TypeRange::dereference_iterator(OwnerT object, ptrdiff_t index) {
+ if (auto type = llvm::dyn_cast_if_present<Type>(object)) {
+ assert(index == 0 && "cannot offset into single-value 'TypeRange'");
+ return type;
+ }
if (const auto *value = llvm::dyn_cast_if_present<const Value *>(object))
return (value + index)->getType();
if (auto *operand = llvm::dyn_cast_if_present<OpOperand *>(object))
diff --git a/mlir/unittests/IR/OperationSupportTest.cpp b/mlir/unittests/IR/OperationSupportTest.cpp
index f94dc784458077..2a1b8d2ef7f55b 100644
--- a/mlir/unittests/IR/OperationSupportTest.cpp
+++ b/mlir/unittests/IR/OperationSupportTest.cpp
@@ -313,4 +313,21 @@ TEST(OperationEquivalenceTest, HashWorksWithFlags) {
op2->destroy();
}
+TEST(ValueRangeTest, ValueConstructable) {
+ MLIRContext context;
+ Builder builder(&context);
+
+ Operation *useOp =
+ createOp(&context, /*operands=*/std::nullopt, builder.getIntegerType(16));
+ // Valid construction despite a temporary 'OpResult'.
+ ValueRange operands = useOp->getResult(0);
+
+ useOp->setOperands(operands);
+ EXPECT_EQ(useOp->getNumOperands(), 1u);
+ EXPECT_EQ(useOp->getOperand(0), useOp->getResult(0));
+
+ useOp->dropAllUses();
+ useOp->destroy();
+}
+
} // namespace
|
Mogball
reviewed
Jan 7, 2025
I like to see the world not for what it appears to be, but for what it could be 🙂 Jokes aside, I think there is an argument to be made that the underlying thing the From that POV, the |
If we can guard against that too somehow, I'd say let's have that as well! I have been bitten by this before. Maybe a clang-tidy check? It's even worse because some functions intentionally return ArrayRef or ValueRange pointing to an object already owned by the caller, so we can't just disable copy-initialization for ValueRange or document that one should not create variables of this type. |
ftynse
approved these changes
Jan 10, 2025
Mogball
approved these changes
Jan 15, 2025
River707
approved these changes
Jan 15, 2025
|
The change seems to cause some assert failed. |
Thanks for the notification! Could you share more details about your C++ compiler and the environment you're compiling for? I thought the change would be safe for 32-bit due to Currently AFK but feel free to revert in the meantime if this blocks you in any way |
|
I am seeing the same error as @yijia1212 . The build error is through an emscripten toolchain. The key part of the invocation is: I suspect this target triple is not covered by CI? |
Will revert now. |
cota
added a commit
that referenced
this pull request
Jan 16, 2025
Reverts #121996 because it broke an emscripten build with `--target=wasm32-unknown-emscripten`: ``` llvm/llvm-project/llvm/include/llvm/ADT/PointerIntPair.h:172:17: error: static assertion failed due to requirement '3U <= PointerUnionUIntTraits<const mlir::Value *, const mlir::Type *, mlir::OpOperand *, mlir::detail::OpResultImpl *, mlir::Type>::NumLowBitsAvailable': PointerIntPair with integer size too large for pointer 172 | static_assert(IntBits <= PtrTraits::NumLowBitsAvailable, | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ llvm/llvm-project/llvm/include/llvm/ADT/PointerIntPair.h:111:13: note: in instantiation of template class 'llvm::PointerIntPairInfo<void *, 3, llvm::pointer_union_detail::PointerUnionUIntTraits<const mlir::Value *, const mlir::Type *, mlir::OpOperand *, mlir::detail::OpResultImpl *, mlir::Type>>' requested here 111 | Value = Info::updateInt(Info::updatePointer(0, PtrVal), | ^ llvm/llvm-project/llvm/include/llvm/ADT/PointerIntPair.h:89:5: note: in instantiation of member function 'llvm::PointerIntPair<void *, 3, int, llvm::pointer_union_detail::PointerUnionUIntTraits<const mlir::Value *, const mlir::Type *, mlir::OpOperand *, mlir::detail::OpResultImpl *, mlir::Type>>::setPointerAndInt' requested here 89 | setPointerAndInt(PtrVal, IntVal); | ^ llvm/llvm-project/llvm/include/llvm/ADT/PointerUnion.h:77:16: note: in instantiation of member function 'llvm::PointerIntPair<void *, 3, int, llvm::pointer_union_detail::PointerUnionUIntTraits<const mlir::Value *, const mlir::Type *, mlir::OpOperand *, mlir::detail::OpResultImpl *, mlir::Type>>::PointerIntPair' requested here 77 | : Base(ValTy(const_cast<void *>( | ^ llvm/llvm-project/mlir/include/mlir/IR/TypeRange.h:49:36: note: in instantiation of member function 'llvm::pointer_union_detail::PointerUnionMembers<llvm::PointerUnion<const mlir::Value *, const mlir::Type *, mlir::OpOperand *, mlir::detail::OpResultImpl *, mlir::Type>, llvm::PointerIntPair<void *, 3, int, llvm::pointer_union_detail::PointerUnionUIntTraits<const mlir::Value *, const mlir::Type *, mlir::OpOperand *, mlir::detail::OpResultImpl *, mlir::Type>>, 4, mlir::Type>::PointerUnionMembers' requested here 49 | TypeRange(Type type) : TypeRange(type, /*count=*/1) {} | ^ llvm/llvm-project/llvm/include/llvm/ADT/PointerIntPair.h:172:25: note: expression evaluates to '3 <= 2' 172 | static_assert(IntBits <= PtrTraits::NumLowBitsAvailable, | ~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 1 error generated. ```
|
Revert just landed. |
github-actions bot
pushed a commit
to arm/arm-toolchain
that referenced
this pull request
Jan 16, 2025
…r" (#123187) Reverts llvm/llvm-project#121996 because it broke an emscripten build with `--target=wasm32-unknown-emscripten`: ``` llvm/llvm-project/llvm/include/llvm/ADT/PointerIntPair.h:172:17: error: static assertion failed due to requirement '3U <= PointerUnionUIntTraits<const mlir::Value *, const mlir::Type *, mlir::OpOperand *, mlir::detail::OpResultImpl *, mlir::Type>::NumLowBitsAvailable': PointerIntPair with integer size too large for pointer 172 | static_assert(IntBits <= PtrTraits::NumLowBitsAvailable, | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ llvm/llvm-project/llvm/include/llvm/ADT/PointerIntPair.h:111:13: note: in instantiation of template class 'llvm::PointerIntPairInfo<void *, 3, llvm::pointer_union_detail::PointerUnionUIntTraits<const mlir::Value *, const mlir::Type *, mlir::OpOperand *, mlir::detail::OpResultImpl *, mlir::Type>>' requested here 111 | Value = Info::updateInt(Info::updatePointer(0, PtrVal), | ^ llvm/llvm-project/llvm/include/llvm/ADT/PointerIntPair.h:89:5: note: in instantiation of member function 'llvm::PointerIntPair<void *, 3, int, llvm::pointer_union_detail::PointerUnionUIntTraits<const mlir::Value *, const mlir::Type *, mlir::OpOperand *, mlir::detail::OpResultImpl *, mlir::Type>>::setPointerAndInt' requested here 89 | setPointerAndInt(PtrVal, IntVal); | ^ llvm/llvm-project/llvm/include/llvm/ADT/PointerUnion.h:77:16: note: in instantiation of member function 'llvm::PointerIntPair<void *, 3, int, llvm::pointer_union_detail::PointerUnionUIntTraits<const mlir::Value *, const mlir::Type *, mlir::OpOperand *, mlir::detail::OpResultImpl *, mlir::Type>>::PointerIntPair' requested here 77 | : Base(ValTy(const_cast<void *>( | ^ llvm/llvm-project/mlir/include/mlir/IR/TypeRange.h:49:36: note: in instantiation of member function 'llvm::pointer_union_detail::PointerUnionMembers<llvm::PointerUnion<const mlir::Value *, const mlir::Type *, mlir::OpOperand *, mlir::detail::OpResultImpl *, mlir::Type>, llvm::PointerIntPair<void *, 3, int, llvm::pointer_union_detail::PointerUnionUIntTraits<const mlir::Value *, const mlir::Type *, mlir::OpOperand *, mlir::detail::OpResultImpl *, mlir::Type>>, 4, mlir::Type>::PointerUnionMembers' requested here 49 | TypeRange(Type type) : TypeRange(type, /*count=*/1) {} | ^ llvm/llvm-project/llvm/include/llvm/ADT/PointerIntPair.h:172:25: note: expression evaluates to '3 <= 2' 172 | static_assert(IntBits <= PtrTraits::NumLowBitsAvailable, | ~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 1 error generated. ```
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
A very common mistake users (and yours truly) make when using
ValueRanges is assigning a temporaryValueto it. Example:ValueRange values = op.getOperand(); apiThatUsesValueRange(values);The issue is caused by the implicit
const Value&constructor: As per C++ rules a const reference can be constructed from a temporary and the address of it taken. After the statement, the temporary goes out of scope andstack-use-after-freeerror occurs.This PR fixes that issue by making
ValueRangecapable of owning a singleValueinstance for that case specifically. While technically a departure from the other owner types that are non-owning, I'd argue that this behavior is more intuitive for the majority of users that usually don't need to care about the lifetime ofValueinstances.TypeRangehas similarly been adopted to accept a singleTypeinstance to implementgetTypes.