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:58:03 PDT 2016
Sent a patch trying to fix this. http://reviews.llvm.org/rL273319
On Tue, Jun 21, 2016 at 10:52 PM Eric Liu <ioeric at google.com> wrote:
> 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/ce55964c/attachment-0001.html>
More information about the cfe-commits
mailing list