[llvm] r253497 - [llvm-profdata] Add SaturatingAdd/SaturatingMultiply Helper Functions (2nd try)
Nathan Slingerland via llvm-commits
llvm-commits at lists.llvm.org
Thu Nov 19 15:01:48 PST 2015
I am able to reproduce the failure using -fsanitizer=undefined (I tried on
Ubuntu 15.10 x86_64, clang 3.6).
Test case: http://pastebin.com/Eejy5rNB
$ clang++ -O1 -std=c++11 -fsanitize=undefined test.cpp -o test
$ ./test
main.cpp:25:11: runtime error: signed integer overflow: 65535 * 65535
cannot be represented in type 'int'
Interestingly, the failure only occurs with uint16_t.
The other unsigned types (uint8_t/uint32_t/uint64_t) pass if I comment out
the uint16_t case.
It's not clear to me why this is triggering the signed multiply check with
uint16_t.
I have put a fix (work-around, really) up for review as D14845.
On Wed, Nov 18, 2015 at 9:48 PM, Xinliang David Li via llvm-commits <
llvm-commits at lists.llvm.org> wrote:
> probably
>
> cmake -DLLVM_USE_SANITIZER=Undefined ...
>
> David
>
> On Wed, Nov 18, 2015 at 9:37 PM, Sean Silva via llvm-commits <
> llvm-commits at lists.llvm.org> wrote:
>
>> What is the configure line for this bot?
>>
>> On Wed, Nov 18, 2015 at 4:28 PM, Bruno Cardoso Lopes via llvm-commits <
>> llvm-commits at lists.llvm.org> wrote:
>>
>>> Hi Nathan,
>>>
>>> Ubsan is detecting some undefined behavior after your commit:
>>> http://lab.llvm.org:8080/green/job/clang-stage2-cmake-RgSan_check/746/
>>>
>>> ******************** TEST 'LLVM-Unit ::
>>> Support/SupportTests/MathExtras.SaturatingMultiply' FAILED
>>> ********************
>>> Note: Google Test filter = MathExtras.SaturatingMultiply
>>> [==========] Running 1 test from 1 test case.
>>> [----------] Global test environment set-up.
>>> [----------] 1 test from MathExtras
>>> [ RUN ] MathExtras.SaturatingMultiply
>>> /Users/buildslave/jenkins/sharedspace/clang-stage2-cmake-RgSan at 2
>>> /llvm/include/llvm/Support/MathExtras.h:675:11:
>>> runtime error: signed integer overflow: 65535 * 65535 cannot be
>>> represented in type 'int'
>>> SUMMARY: AddressSanitizer: undefined-behavior
>>> /Users/buildslave/jenkins/sharedspace/clang-stage2-cmake-RgSan at 2
>>> /llvm/include/llvm/Support/MathExtras.h:675:11
>>> in
>>> 0 SupportTests 0x0000000105b92545
>>> llvm::sys::PrintStackTrace(llvm::raw_ostream&) + 277
>>> 1 SupportTests 0x0000000105b90400
>>> llvm::sys::RunSignalHandlers() + 448
>>> 2 SupportTests 0x0000000105b937d3
>>> SignalHandler(int) + 611
>>> 3 libsystem_platform.dylib 0x00007fff8d239f1a _sigtramp + 26
>>> 4 libsystem_platform.dylib 0x0000000000000003 _sigtramp +
>>> 1927045379
>>> 5 libsystem_c.dylib 0x00007fff88edd9b3 abort + 129
>>> 6 libclang_rt.asan_osx_dynamic.dylib 0x00000001060c0f76
>>> __sanitizer::Abort() + 6
>>>
>>>
>>>
>>> On Wed, Nov 18, 2015 at 12:40 PM, Nathan Slingerland via llvm-commits
>>> <llvm-commits at lists.llvm.org> wrote:
>>> > Author: slingn
>>> > Date: Wed Nov 18 14:40:41 2015
>>> > New Revision: 253497
>>> >
>>> > URL: http://llvm.org/viewvc/llvm-project?rev=253497&view=rev
>>> > Log:
>>> > [llvm-profdata] Add SaturatingAdd/SaturatingMultiply Helper Functions
>>> (2nd try)
>>> >
>>> > Summary:
>>> > This change adds MathExtras helper functions for handling unsigned,
>>> saturating addition and multiplication. It also updates the instrumentation
>>> and sample profile merge implementations to use them.
>>> >
>>> > Reviewers: dnovillo, bogner, davidxl
>>> >
>>> > Subscribers: davidxl, llvm-commits
>>> >
>>> > Differential Revision: http://reviews.llvm.org/D14720
>>> >
>>> > Modified:
>>> > llvm/trunk/include/llvm/ProfileData/InstrProf.h
>>> > llvm/trunk/include/llvm/ProfileData/SampleProf.h
>>> > llvm/trunk/include/llvm/Support/MathExtras.h
>>> > llvm/trunk/unittests/ProfileData/InstrProfTest.cpp
>>> > llvm/trunk/unittests/Support/MathExtrasTest.cpp
>>> >
>>> > Modified: llvm/trunk/include/llvm/ProfileData/InstrProf.h
>>> > URL:
>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ProfileData/InstrProf.h?rev=253497&r1=253496&r2=253497&view=diff
>>> >
>>> ==============================================================================
>>> > --- llvm/trunk/include/llvm/ProfileData/InstrProf.h (original)
>>> > +++ llvm/trunk/include/llvm/ProfileData/InstrProf.h Wed Nov 18
>>> 14:40:41 2015
>>> > @@ -226,7 +226,7 @@ struct InstrProfValueSiteRecord {
>>> > while (I != IE && I->Value < J->Value)
>>> > ++I;
>>> > if (I != IE && I->Value == J->Value) {
>>> > - I->Count += J->Count;
>>> > + I->Count = SaturatingAdd(I->Count, J->Count);
>>> > ++I;
>>> > continue;
>>> > }
>>> >
>>> > Modified: llvm/trunk/include/llvm/ProfileData/SampleProf.h
>>> > URL:
>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ProfileData/SampleProf.h?rev=253497&r1=253496&r2=253497&view=diff
>>> >
>>> ==============================================================================
>>> > --- llvm/trunk/include/llvm/ProfileData/SampleProf.h (original)
>>> > +++ llvm/trunk/include/llvm/ProfileData/SampleProf.h Wed Nov 18
>>> 14:40:41 2015
>>> > @@ -173,10 +173,7 @@ public:
>>> > /// Sample counts accumulate using saturating arithmetic, to avoid
>>> wrapping
>>> > /// around unsigned integers.
>>> > void addSamples(uint64_t S) {
>>> > - if (NumSamples <= std::numeric_limits<uint64_t>::max() - S)
>>> > - NumSamples += S;
>>> > - else
>>> > - NumSamples = std::numeric_limits<uint64_t>::max();
>>> > + NumSamples = SaturatingAdd(NumSamples, S);
>>> > }
>>> >
>>> > /// Add called function \p F with samples \p S.
>>> > @@ -185,10 +182,7 @@ public:
>>> > /// around unsigned integers.
>>> > void addCalledTarget(StringRef F, uint64_t S) {
>>> > uint64_t &TargetSamples = CallTargets[F];
>>> > - if (TargetSamples <= std::numeric_limits<uint64_t>::max() - S)
>>> > - TargetSamples += S;
>>> > - else
>>> > - TargetSamples = std::numeric_limits<uint64_t>::max();
>>> > + TargetSamples = SaturatingAdd(TargetSamples, S);
>>> > }
>>> >
>>> > /// Return true if this sample record contains function calls.
>>> >
>>> > Modified: llvm/trunk/include/llvm/Support/MathExtras.h
>>> > URL:
>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Support/MathExtras.h?rev=253497&r1=253496&r2=253497&view=diff
>>> >
>>> ==============================================================================
>>> > --- llvm/trunk/include/llvm/Support/MathExtras.h (original)
>>> > +++ llvm/trunk/include/llvm/Support/MathExtras.h Wed Nov 18 14:40:41
>>> 2015
>>> > @@ -653,6 +653,32 @@ inline int64_t SignExtend64(uint64_t X,
>>> > return int64_t(X << (64 - B)) >> (64 - B);
>>> > }
>>> >
>>> > +/// \brief Add two unsigned integers, X and Y, of type T.
>>> > +/// Clamp the result to the maximum representable value of T on
>>> overflow.
>>> > +template <typename T>
>>> > +typename std::enable_if<std::is_unsigned<T>::value, T>::type
>>> > +SaturatingAdd(T X, T Y) {
>>> > + // Hacker's Delight, p. 29
>>> > + T Z = X + Y;
>>> > + if (Z < X || Z < Y)
>>> > + return std::numeric_limits<T>::max();
>>> > + else
>>> > + return Z;
>>> > +}
>>> > +
>>> > +/// \brief Multiply two unsigned integers, X and Y, of type T.
>>> > +/// Clamp the result to the maximum representable value of T on
>>> overflow.
>>> > +template <typename T>
>>> > +typename std::enable_if<std::is_unsigned<T>::value, T>::type
>>> > +SaturatingMultiply(T X, T Y) {
>>> > + // Hacker's Delight, p. 30
>>> > + T Z = X * Y;
>>> > + if (Y != 0 && Z / Y != X)
>>> > + return std::numeric_limits<T>::max();
>>> > + else
>>> > + return Z;
>>> > +}
>>> > +
>>> > extern const float huge_valf;
>>> > } // End llvm namespace
>>> >
>>> >
>>> > Modified: llvm/trunk/unittests/ProfileData/InstrProfTest.cpp
>>> > URL:
>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ProfileData/InstrProfTest.cpp?rev=253497&r1=253496&r2=253497&view=diff
>>> >
>>> ==============================================================================
>>> > --- llvm/trunk/unittests/ProfileData/InstrProfTest.cpp (original)
>>> > +++ llvm/trunk/unittests/ProfileData/InstrProfTest.cpp Wed Nov 18
>>> 14:40:41 2015
>>> > @@ -350,6 +350,38 @@ TEST_F(InstrProfTest, get_icall_data_mer
>>> > ASSERT_EQ(2U, VD_4[2].Count);
>>> > }
>>> >
>>> > +TEST_F(InstrProfTest, get_icall_data_merge1_saturation) {
>>> > + const uint64_t Max = std::numeric_limits<uint64_t>::max();
>>> > +
>>> > + InstrProfRecord Record1("caller", 0x1234, {1});
>>> > + InstrProfRecord Record2("caller", 0x1234, {1});
>>> > + InstrProfRecord Record3("callee1", 0x1235, {3, 4});
>>> > +
>>> > + Record1.reserveSites(IPVK_IndirectCallTarget, 1);
>>> > + InstrProfValueData VD1[] = {{(uint64_t) "callee1", 1}};
>>> > + Record1.addValueData(IPVK_IndirectCallTarget, 0, VD1, 1, nullptr);
>>> > +
>>> > + Record2.reserveSites(IPVK_IndirectCallTarget, 1);
>>> > + InstrProfValueData VD2[] = {{(uint64_t) "callee1", Max}};
>>> > + Record2.addValueData(IPVK_IndirectCallTarget, 0, VD2, 1, nullptr);
>>> > +
>>> > + Writer.addRecord(std::move(Record1));
>>> > + Writer.addRecord(std::move(Record2));
>>> > + Writer.addRecord(std::move(Record3));
>>> > +
>>> > + auto Profile = Writer.writeBuffer();
>>> > + readProfile(std::move(Profile));
>>> > +
>>> > + // Verify saturation of counts.
>>> > + ErrorOr<InstrProfRecord> R = Reader->getInstrProfRecord("caller",
>>> 0x1234);
>>> > + ASSERT_TRUE(NoError(R.getError()));
>>> > + ASSERT_EQ(1U, R.get().getNumValueSites(IPVK_IndirectCallTarget));
>>> > + std::unique_ptr<InstrProfValueData[]> VD =
>>> > + R.get().getValueForSite(IPVK_IndirectCallTarget, 0);
>>> > + ASSERT_EQ(StringRef("callee1"), StringRef((const char
>>> *)VD[0].Value, 7));
>>> > + ASSERT_EQ(Max, VD[0].Count);
>>> > +}
>>> > +
>>> > TEST_F(InstrProfTest, get_max_function_count) {
>>> > InstrProfRecord Record1("foo", 0x1234, {1ULL << 31, 2});
>>> > InstrProfRecord Record2("bar", 0, {1ULL << 63});
>>> >
>>> > Modified: llvm/trunk/unittests/Support/MathExtrasTest.cpp
>>> > URL:
>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Support/MathExtrasTest.cpp?rev=253497&r1=253496&r2=253497&view=diff
>>> >
>>> ==============================================================================
>>> > --- llvm/trunk/unittests/Support/MathExtrasTest.cpp (original)
>>> > +++ llvm/trunk/unittests/Support/MathExtrasTest.cpp Wed Nov 18
>>> 14:40:41 2015
>>> > @@ -190,4 +190,40 @@ TEST(MathExtras, RoundUpToAlignment) {
>>> > EXPECT_EQ(552u, RoundUpToAlignment(321, 255, 42));
>>> > }
>>> >
>>> > +template<typename T>
>>> > +void SaturatingAddTestHelper()
>>> > +{
>>> > + const T Max = std::numeric_limits<T>::max();
>>> > + EXPECT_EQ(T(3), SaturatingAdd(T(1), T(2)));
>>> > + EXPECT_EQ(Max, SaturatingAdd(Max, T(1)));
>>> > + EXPECT_EQ(Max, SaturatingAdd(T(1), Max));
>>> > + EXPECT_EQ(Max, SaturatingAdd(Max, Max));
>>> > +}
>>> > +
>>> > +TEST(MathExtras, SaturatingAdd) {
>>> > + SaturatingAddTestHelper<uint8_t>();
>>> > + SaturatingAddTestHelper<uint16_t>();
>>> > + SaturatingAddTestHelper<uint32_t>();
>>> > + SaturatingAddTestHelper<uint64_t>();
>>> > +}
>>> > +
>>> > +template<typename T>
>>> > +void SaturatingMultiplyTestHelper()
>>> > +{
>>> > + const T Max = std::numeric_limits<T>::max();
>>> > + EXPECT_EQ(T(0), SaturatingMultiply(T(1), T(0)));
>>> > + EXPECT_EQ(T(0), SaturatingMultiply(T(0), T(1)));
>>> > + EXPECT_EQ(T(6), SaturatingMultiply(T(2), T(3)));
>>> > + EXPECT_EQ(Max, SaturatingMultiply(Max, T(2)));
>>> > + EXPECT_EQ(Max, SaturatingMultiply(T(2),Max));
>>> > + EXPECT_EQ(Max, SaturatingMultiply(Max, Max));
>>> > +}
>>> > +
>>> > +TEST(MathExtras, SaturatingMultiply) {
>>> > + SaturatingMultiplyTestHelper<uint8_t>();
>>> > + SaturatingMultiplyTestHelper<uint16_t>();
>>> > + SaturatingMultiplyTestHelper<uint32_t>();
>>> > + SaturatingMultiplyTestHelper<uint64_t>();
>>> > +}
>>> > +
>>> > }
>>> >
>>> >
>>> > _______________________________________________
>>> > llvm-commits mailing list
>>> > llvm-commits at lists.llvm.org
>>> > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>>
>>>
>>>
>>> --
>>> Bruno Cardoso Lopes
>>> http://www.brunocardoso.cc
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>>
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>
>>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20151119/7446ef2d/attachment.html>
More information about the llvm-commits
mailing list