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@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@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
--
Mike
Sent from phone
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
