r273290 - Added calculateRangesAfterReplaments() to calculate affacted ranges in the new code.
Eric Liu via cfe-commits
cfe-commits at lists.llvm.org
Tue Jun 21 13:52:23 PDT 2016
Thanks Mike! I am working on a fix.
On Tue, Jun 21, 2016 at 10:44 PM Mike Aizatsky <aizatsky at google.com> wrote:
> I think this change has buffer overflow. It breaks asan build:
>
> http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-fast/builds/13946
>
> FAIL: Clang-Unit :: Tooling/ToolingTests/Range.RangesWithNonOverlappingReplacements (9361 of 9474)
> ******************** TEST 'Clang-Unit :: Tooling/ToolingTests/Range.RangesWithNonOverlappingReplacements' FAILED ********************
> Note: Google Test filter = Range.RangesWithNonOverlappingReplacements
> [==========] Running 1 test from 1 test case.
> [----------] Global test environment set-up.
> [----------] 1 test from Range
> [ RUN ] Range.RangesWithNonOverlappingReplacements
> =================================================================
> ==32475==ERROR: AddressSanitizer: global-buffer-overflow on address 0x0000078a4b02 at pc 0x000000540095 bp 0x7ffcd9799a10 sp 0x7ffcd97991c0
> READ of size 5 at 0x0000078a4b02 thread T0
> #0 0x540094 in __asan_memcpy /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/projects/compiler-rt/lib/asan/asan_interceptors.cc:393
> #1 0x7f6e902e6dc1 in copy /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/projects/libcxx/include/string:655:50
> #2 0x7f6e902e6dc1 in std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >::__init(char const*, unsigned long) /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/projects/libcxx/include/string:2054
> #3 0x30b45e3 in basic_string /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/libcxx_build_asan/include/c++/v1/string:2086:5
> #4 0x30b45e3 in clang::tooling::calculateRangesAfterReplacements(std::__1::set<clang::tooling::Replacement, std::__1::less<clang::tooling::Replacement>, std::__1::allocator<clang::tooling::Replacement> > const&, std::__1::vector<clang::tooling::Range, std::__1::allocator<clang::tooling::Range> > const&) /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/tools/clang/lib/Tooling/Core/Replacement.cpp:325
> #5 0xaa1a6c in clang::tooling::Range_RangesWithNonOverlappingReplacements_Test::TestBody() /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/tools/clang/unittests/Tooling/RefactoringTest.cpp:503:3
> #6 0x241400a in HandleExceptionsInMethodIfSupported<testing::Test, void> /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/utils/unittest/googletest/src/gtest.cc:2145:12
> #7 0x241400a in testing::Test::Run() /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/utils/unittest/googletest/src/gtest.cc:2161
> #8 0x2415426 in testing::TestInfo::Run() /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/utils/unittest/googletest/src/gtest.cc:2309:5
> #9 0x241636a in testing::TestCase::Run() /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/utils/unittest/googletest/src/gtest.cc:2416:5
> #10 0x242c6fc in testing::internal::UnitTestImpl::RunAllTests() /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/utils/unittest/googletest/src/gtest.cc:4207:11
> #11 0x242b808 in HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool> /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/utils/unittest/googletest/src/gtest.cc:2145:12
> #12 0x242b808 in testing::UnitTest::Run() /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/utils/unittest/googletest/src/gtest.cc:3841
> #13 0x24000dd in main /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/utils/unittest/UnitTestMain/TestMain.cpp:48:10
> #14 0x7f6e8fcdcf44 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21f44)
> #15 0x4aeb02 in _start (/mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm_build_asan/tools/clang/unittests/Tooling/ToolingTests+0x4aeb02)
>
> 0x0000078a4b02 is located 62 bytes to the left of global variable '<string literal>' defined in '/mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/include/llvm/ADT/SmallVector.h:150:5' (0x78a4b40) of size 13
> '<string literal>' is ascii string 'idx < size()'
> 0x0000078a4b02 is located 0 bytes to the right of global variable '<string literal>' defined in '/mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/tools/clang/lib/Tooling/Core/Replacement.cpp:325:49' (0x78a4b00) of size 2
> '<string literal>' is ascii string ' '
> SUMMARY: AddressSanitizer: global-buffer-overflow /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/projects/compiler-rt/lib/asan/asan_interceptors.cc:393 in __asan_memcpy
> Shadow bytes around the buggy address:
> 0x000080f0c910: 00 00 00 00 00 00 00 00 00 07 f9 f9 f9 f9 f9 f9
> 0x000080f0c920: 00 00 00 00 00 00 00 00 01 f9 f9 f9 f9 f9 f9 f9
> 0x000080f0c930: 03 f9 f9 f9 f9 f9 f9 f9 03 f9 f9 f9 f9 f9 f9 f9
> 0x000080f0c940: 03 f9 f9 f9 f9 f9 f9 f9 02 f9 f9 f9 f9 f9 f9 f9
> 0x000080f0c950: 00 f9 f9 f9 f9 f9 f9 f9 01 f9 f9 f9 f9 f9 f9 f9
> =>0x000080f0c960:[02]f9 f9 f9 f9 f9 f9 f9 00 05 f9 f9 f9 f9 f9 f9
> 0x000080f0c970: 00 00 00 00 00 00 00 00 00 00 00 00 01 f9 f9 f9
> 0x000080f0c980: f9 f9 f9 f9 00 00 00 00 00 00 00 00 00 00 00 00
> 0x000080f0c990: 00 00 00 00 00 04 f9 f9 f9 f9 f9 f9 00 00 00 00 0x000080f0c9a0: 00 02 f9 f9 f9 f9 f9 f9 00 00 00 00 00 00 00 00
> 0x000080f0c9b0: 00 00 00 00 00 00 02 f9 f9 f9 f9 f9 00 00 00 00
> Shadow byte legend (one shadow byte represents 8 application bytes):
> Addressable: 00
> Partially addressable: 01 02 03 04 05 06 07
> Heap left redzone: fa
> Heap right redzone: fb
> Freed heap region: fd
> Stack left redzone: f1
> Stack mid redzone: f2
> Stack right redzone: f3
> Stack partial redzone: f4
> Stack after return: f5
> Stack use after scope: f8
> Global redzone: f9
> Global init order: f6
> Poisoned by user: f7
> Container overflow: fc
> Array cookie: ac
> Intra object redzone: bb
> ASan internal: fe
> Left alloca redzone: ca
> Right alloca redzone: cb
> ==32475==ABORTING
>
>
>
>
> On Tue, Jun 21, 2016 at 11:03 AM Eric Liu via cfe-commits <
> cfe-commits at lists.llvm.org> wrote:
>
>> Author: ioeric
>> Date: Tue Jun 21 12:56:31 2016
>> New Revision: 273290
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=273290&view=rev
>> Log:
>> Added calculateRangesAfterReplaments() to calculate affacted ranges in
>> the new code.
>>
>> Summary:
>> Added calculateRangesAfterReplaments() to calculate original ranges as
>> well as
>> newly affacted ranges in the new code.
>>
>> Reviewers: klimek, djasper
>>
>> Subscribers: cfe-commits, klimek
>>
>> Differential Revision: http://reviews.llvm.org/D21547
>>
>> Modified:
>> cfe/trunk/include/clang/Tooling/Core/Replacement.h
>> cfe/trunk/lib/Tooling/Core/Replacement.cpp
>> cfe/trunk/unittests/Tooling/RefactoringTest.cpp
>>
>> Modified: cfe/trunk/include/clang/Tooling/Core/Replacement.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Tooling/Core/Replacement.h?rev=273290&r1=273289&r2=273290&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/include/clang/Tooling/Core/Replacement.h (original)
>> +++ cfe/trunk/include/clang/Tooling/Core/Replacement.h Tue Jun 21
>> 12:56:31 2016
>> @@ -57,6 +57,11 @@ public:
>> return RHS.Offset >= Offset &&
>> (RHS.Offset + RHS.Length) <= (Offset + Length);
>> }
>> +
>> + /// \brief Whether this range equals to \p RHS or not.
>> + bool operator==(const Range &RHS) const {
>> + return Offset == RHS.getOffset() && Length == RHS.getLength();
>> + }
>> /// @}
>>
>> private:
>> @@ -222,11 +227,25 @@ bool applyAllReplacements(const std::vec
>> std::string applyAllReplacements(StringRef Code, const Replacements
>> &Replaces);
>>
>> /// \brief Calculates the ranges in a single file that are affected by
>> the
>> -/// Replacements.
>> +/// Replacements. Overlapping ranges will be merged.
>> ///
>> /// \pre Replacements must be for the same file.
>> +///
>> +/// \returns a non-overlapping and sorted ranges.
>> std::vector<Range> calculateChangedRanges(const Replacements &Replaces);
>>
>> +/// \brief Calculates the new ranges after \p Replaces are applied. These
>> +/// include both the original \p Ranges and the affected ranges of \p
>> Replaces
>> +/// in the new code.
>> +///
>> +/// \pre Replacements must be for the same file.
>> +///
>> +/// \return The new ranges after \p Replaces are applied. The new ranges
>> will be
>> +/// sorted and non-overlapping.
>> +std::vector<Range>
>> +calculateRangesAfterReplacements(const Replacements &Replaces,
>> + const std::vector<Range> &Ranges);
>> +
>> /// \brief Groups a random set of replacements by file path. Replacements
>> /// related to the same file entry are put into the same vector.
>> std::map<std::string, Replacements>
>>
>> Modified: cfe/trunk/lib/Tooling/Core/Replacement.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Tooling/Core/Replacement.cpp?rev=273290&r1=273289&r2=273290&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/lib/Tooling/Core/Replacement.cpp (original)
>> +++ cfe/trunk/lib/Tooling/Core/Replacement.cpp Tue Jun 21 12:56:31 2016
>> @@ -278,6 +278,30 @@ std::string applyAllReplacements(StringR
>> return Result;
>> }
>>
>> +// Merge and sort overlapping ranges in \p Ranges.
>> +static std::vector<Range> mergeAndSortRanges(std::vector<Range> Ranges) {
>> + std::sort(Ranges.begin(), Ranges.end(),
>> + [](const Range &LHS, const Range &RHS) {
>> + if (LHS.getOffset() != RHS.getOffset())
>> + return LHS.getOffset() < RHS.getOffset();
>> + return LHS.getLength() < RHS.getLength();
>> + });
>> + std::vector<Range> Result;
>> + for (const auto &R : Ranges) {
>> + if (Result.empty() ||
>> + Result.back().getOffset() + Result.back().getLength() <
>> R.getOffset()) {
>> + Result.push_back(R);
>> + } else {
>> + unsigned NewEnd =
>> + std::max(Result.back().getOffset() + Result.back().getLength(),
>> + R.getOffset() + R.getLength());
>> + Result[Result.size() - 1] =
>> + Range(Result.back().getOffset(), NewEnd -
>> Result.back().getOffset());
>> + }
>> + }
>> + return Result;
>> +}
>> +
>> std::vector<Range> calculateChangedRanges(const Replacements &Replaces) {
>> std::vector<Range> ChangedRanges;
>> int Shift = 0;
>> @@ -287,7 +311,20 @@ std::vector<Range> calculateChangedRange
>> Shift += Length - R.getLength();
>> ChangedRanges.push_back(Range(Offset, Length));
>> }
>> - return ChangedRanges;
>> + return mergeAndSortRanges(ChangedRanges);
>> +}
>> +
>> +std::vector<Range>
>> +calculateRangesAfterReplacements(const Replacements &Replaces,
>> + const std::vector<Range> &Ranges) {
>> + auto MergedRanges = mergeAndSortRanges(Ranges);
>> + tooling::Replacements FakeReplaces;
>> + for (const auto &R : MergedRanges)
>> + FakeReplaces.insert(Replacement(Replaces.begin()->getFilePath(),
>> + R.getOffset(), R.getLength(),
>> + std::string(" ", R.getLength())));
>> + tooling::Replacements NewReplaces = mergeReplacements(FakeReplaces,
>> Replaces);
>> + return calculateChangedRanges(NewReplaces);
>> }
>>
>> namespace {
>> @@ -434,4 +471,3 @@ Replacements mergeReplacements(const Rep
>>
>> } // end namespace tooling
>> } // end namespace clang
>> -
>>
>> Modified: cfe/trunk/unittests/Tooling/RefactoringTest.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Tooling/RefactoringTest.cpp?rev=273290&r1=273289&r2=273290&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/unittests/Tooling/RefactoringTest.cpp (original)
>> +++ cfe/trunk/unittests/Tooling/RefactoringTest.cpp Tue Jun 21 12:56:31
>> 2016
>> @@ -462,13 +462,96 @@ TEST(Range, CalculateRangesOfReplacement
>>
>> std::vector<Range> Ranges = calculateChangedRanges(Replaces);
>>
>> - EXPECT_EQ(3ul, Ranges.size());
>> + EXPECT_EQ(2ul, Ranges.size());
>> EXPECT_TRUE(Ranges[0].getOffset() == 0);
>> EXPECT_TRUE(Ranges[0].getLength() == 0);
>> EXPECT_TRUE(Ranges[1].getOffset() == 6);
>> - EXPECT_TRUE(Ranges[1].getLength() == 6);
>> - EXPECT_TRUE(Ranges[2].getOffset() == 12);
>> - EXPECT_TRUE(Ranges[2].getLength() == 16);
>> + EXPECT_TRUE(Ranges[1].getLength() == 22);
>> +}
>> +
>> +TEST(Range, RangesAfterReplacements) {
>> + std::vector<Range> Ranges = {Range(5, 2), Range(10, 5)};
>> + Replacements Replaces = {Replacement("foo", 0, 2, "1234")};
>> + std::vector<Range> Expected = {Range(0, 4), Range(7, 2), Range(12, 5)};
>> + EXPECT_EQ(Expected, calculateRangesAfterReplacements(Replaces,
>> Ranges));
>> +}
>> +
>> +TEST(Range, RangesBeforeReplacements) {
>> + std::vector<Range> Ranges = {Range(5, 2), Range(10, 5)};
>> + Replacements Replaces = {Replacement("foo", 20, 2, "1234")};
>> + std::vector<Range> Expected = {Range(5, 2), Range(10, 5), Range(20,
>> 4)};
>> + EXPECT_EQ(Expected, calculateRangesAfterReplacements(Replaces,
>> Ranges));
>> +}
>> +
>> +TEST(Range, NotAffectedByReplacements) {
>> + std::vector<Range> Ranges = {Range(0, 2), Range(5, 2), Range(10, 5)};
>> + Replacements Replaces = {Replacement("foo", 3, 2, "12"),
>> + Replacement("foo", 12, 2, "12"),
>> + Replacement("foo", 20, 5, "")};
>> + std::vector<Range> Expected = {Range(0, 2), Range(3, 4), Range(10, 5),
>> + Range(20, 0)};
>> + EXPECT_EQ(Expected, calculateRangesAfterReplacements(Replaces,
>> Ranges));
>> +}
>> +
>> +TEST(Range, RangesWithNonOverlappingReplacements) {
>> + std::vector<Range> Ranges = {Range(0, 2), Range(5, 2), Range(10, 5)};
>> + Replacements Replaces = {Replacement("foo", 3, 1, ""),
>> + Replacement("foo", 6, 1, "123"),
>> + Replacement("foo", 20, 2, "12345")};
>> + std::vector<Range> Expected = {Range(0, 2), Range(3, 0), Range(4, 4),
>> + Range(11, 5), Range(21, 5)};
>> + EXPECT_EQ(Expected, calculateRangesAfterReplacements(Replaces,
>> Ranges));
>> +}
>> +
>> +TEST(Range, RangesWithOverlappingReplacements) {
>> + std::vector<Range> Ranges = {Range(0, 2), Range(5, 2), Range(15, 5),
>> + Range(30, 5)};
>> + Replacements Replaces = {
>> + Replacement("foo", 1, 3, ""), Replacement("foo", 6, 1, "123"),
>> + Replacement("foo", 13, 3, "1"), Replacement("foo", 25, 15, "")};
>> + std::vector<Range> Expected = {Range(0, 1), Range(2, 4), Range(12, 5),
>> + Range(22, 0)};
>> + EXPECT_EQ(Expected, calculateRangesAfterReplacements(Replaces,
>> Ranges));
>> +}
>> +
>> +TEST(Range, MergeIntoOneRange) {
>> + std::vector<Range> Ranges = {Range(0, 2), Range(5, 2), Range(15, 5)};
>> + Replacements Replaces = {Replacement("foo", 1, 15, "1234567890")};
>> + std::vector<Range> Expected = {Range(0, 15)};
>> + EXPECT_EQ(Expected, calculateRangesAfterReplacements(Replaces,
>> Ranges));
>> +}
>> +
>> +TEST(Range, ReplacementsStartingAtRangeOffsets) {
>> + std::vector<Range> Ranges = {Range(0, 2), Range(5, 5), Range(15, 5)};
>> + Replacements Replaces = {
>> + Replacement("foo", 0, 2, "12"), Replacement("foo", 5, 1, "123"),
>> + Replacement("foo", 7, 4, "12345"), Replacement("foo", 15, 10,
>> "12")};
>> + std::vector<Range> Expected = {Range(0, 2), Range(5, 9), Range(18, 2)};
>> + EXPECT_EQ(Expected, calculateRangesAfterReplacements(Replaces,
>> Ranges));
>> +}
>> +
>> +TEST(Range, ReplacementsEndingAtRangeEnds) {
>> + std::vector<Range> Ranges = {Range(0, 2), Range(5, 2), Range(15, 5)};
>> + Replacements Replaces = {Replacement("foo", 6, 1, "123"),
>> + Replacement("foo", 17, 3, "12")};
>> + std::vector<Range> Expected = {Range(0, 2), Range(5, 4), Range(17, 4)};
>> + EXPECT_EQ(Expected, calculateRangesAfterReplacements(Replaces,
>> Ranges));
>> +}
>> +
>> +TEST(Range, AjacentReplacements) {
>> + std::vector<Range> Ranges = {Range(0, 0), Range(15, 5)};
>> + Replacements Replaces = {Replacement("foo", 1, 2, "123"),
>> + Replacement("foo", 12, 3, "1234")};
>> + std::vector<Range> Expected = {Range(0, 0), Range(1, 3), Range(13, 9)};
>> + EXPECT_EQ(Expected, calculateRangesAfterReplacements(Replaces,
>> Ranges));
>> +}
>> +
>> +TEST(Range, MergeRangesAfterReplacements) {
>> + std::vector<Range> Ranges = {Range(8, 0), Range(5, 2), Range(9, 0),
>> Range(0, 1)};
>> + Replacements Replaces = {Replacement("foo", 1, 3, ""),
>> + Replacement("foo", 7, 0, "12"),
>> Replacement("foo", 9, 2, "")};
>> + std::vector<Range> Expected = {Range(0, 1), Range(2, 4), Range(7, 0),
>> Range(8, 0)};
>> + EXPECT_EQ(Expected, calculateRangesAfterReplacements(Replaces,
>> Ranges));
>> }
>>
>> TEST(DeduplicateTest, removesDuplicates) {
>>
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>
> --
> Mike
> Sent from phone
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160621/14422d5c/attachment-0001.html>
More information about the cfe-commits
mailing list