Skip to content

Commit

Permalink
[APInt] provide a safe API for zext value and sext value.
Browse files Browse the repository at this point in the history
Currently, APInt::getSExtValue and getZExtValue crashes on values with more than 64 bits.
Users may accidently crash the compiler with this setting when the integer may be i128.
As shown in #59316

In this patch we provide a trySExtValue and tryZExtValue to return an Optional, the user
needs to explictly unwrap it and condsier the possibility where there my no value in it.

Reviewed By: RKSimon

Differential Revision: https://reviews.llvm.org/D139683
  • Loading branch information
DataCorrupted committed Dec 15, 2022
1 parent 80f2f1e commit 5596810
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 0 deletions.
20 changes: 20 additions & 0 deletions llvm/include/llvm/ADT/APInt.h
Expand Up @@ -1490,6 +1490,16 @@ class [[nodiscard]] APInt {
return U.pVal[0];
}

/// Get zero extended value if possible
///
/// This method attempts to return the value of this APInt as a zero extended
/// uint64_t. The bitwidth must be <= 64 or the value must fit within a
/// uint64_t. Otherwise no value is returned.
std::optional<uint64_t> tryZExtValue() const {
return (getActiveBits() <= 64) ? std::optional<uint64_t>(getZExtValue())
: std::nullopt;
};

/// Get sign extended value
///
/// This method attempts to return the value of this APInt as a sign extended
Expand All @@ -1502,6 +1512,16 @@ class [[nodiscard]] APInt {
return int64_t(U.pVal[0]);
}

/// Get sign extended value if possible
///
/// This method attempts to return the value of this APInt as a sign extended
/// int64_t. The bitwidth must be <= 64 or the value must fit within an
/// int64_t. Otherwise no value is returned.
std::optional<int64_t> trySExtValue() const {
return (getSignificantBits() <= 64) ? std::optional<int64_t>(getSExtValue())
: std::nullopt;
};

/// Get bits required for string value.
///
/// This method determines how many bits are required to hold the APInt
Expand Down
21 changes: 21 additions & 0 deletions llvm/unittests/ADT/APIntTest.cpp
Expand Up @@ -3134,4 +3134,25 @@ TEST(APIntTest, DenseMap) {
Map.find(ZeroWidthInt);
}

TEST(APIntTest, TryExt) {
APInt small(32, 42);
APInt large(128, {0xffff, 0xffff});
ASSERT_TRUE(small.tryZExtValue().has_value());
ASSERT_TRUE(small.trySExtValue().has_value());
ASSERT_FALSE(large.tryZExtValue().has_value());
ASSERT_FALSE(large.trySExtValue().has_value());
ASSERT_EQ(small.trySExtValue().value_or(41), 42);
ASSERT_EQ(large.trySExtValue().value_or(41), 41);

APInt negOne32(32, 0);
negOne32.setAllBits();
ASSERT_EQ(negOne32.trySExtValue().value_or(42), -1);
APInt negOne64(64, 0);
negOne64.setAllBits();
ASSERT_EQ(negOne64.trySExtValue().value_or(42), -1);
APInt negOne128(128, 0);
negOne128.setAllBits();
ASSERT_EQ(negOne128.trySExtValue().value_or(42), 42);
}

} // end anonymous namespace

4 comments on commit 5596810

@kazutakahirata
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This patch seems to cause:

unittests/ADT/./ADTTests --gtest_filter=APIntTest.TryExt
--
llvm-project/llvm/unittests/ADT/APIntTest.cpp:3155: Failure
Expected equality of these values:
  negOne128.trySExtValue().value_or(42)
    Which is: -1
  42

Would you mind taking a look? Thanks in advance!

@DataCorrupted
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes I am fixing it. It's a bug in unit testing. Sorry for the problem.

@DataCorrupted
Copy link
Member Author

@DataCorrupted DataCorrupted commented on 5596810 Dec 15, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be fixed by a3b4fef

@kazutakahirata
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot!

Please sign in to comment.