<div dir="ltr">I think this change has buffer overflow. It breaks asan build:<div><br></div><div><a href="http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-fast/builds/13946">http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-fast/builds/13946</a><br></div><div><br></div><div><pre style="font-family:'courier new',courier,monotype,monospace;font-size:medium;line-height:normal"><span class="inbox-inbox-stdout">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 </span><span class="inbox-inbox-stdout">00 04 f9 f9 f9 f9 f9 f9 00 00 00 00
</span><span class="inbox-inbox-stdout"> 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
</span></pre><br class="inbox-inbox-Apple-interchange-newline"></div><div><br></div></div><br><div class="gmail_quote"><div dir="ltr">On Tue, Jun 21, 2016 at 11:03 AM Eric Liu via cfe-commits <<a href="mailto:cfe-commits@lists.llvm.org">cfe-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: ioeric<br>
Date: Tue Jun 21 12:56:31 2016<br>
New Revision: 273290<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=273290&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=273290&view=rev</a><br>
Log:<br>
Added calculateRangesAfterReplaments() to calculate affacted ranges in the new code.<br>
<br>
Summary:<br>
Added calculateRangesAfterReplaments() to calculate original ranges as well as<br>
newly affacted ranges in the new code.<br>
<br>
Reviewers: klimek, djasper<br>
<br>
Subscribers: cfe-commits, klimek<br>
<br>
Differential Revision: <a href="http://reviews.llvm.org/D21547" rel="noreferrer" target="_blank">http://reviews.llvm.org/D21547</a><br>
<br>
Modified:<br>
cfe/trunk/include/clang/Tooling/Core/Replacement.h<br>
cfe/trunk/lib/Tooling/Core/Replacement.cpp<br>
cfe/trunk/unittests/Tooling/RefactoringTest.cpp<br>
<br>
Modified: cfe/trunk/include/clang/Tooling/Core/Replacement.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Tooling/Core/Replacement.h?rev=273290&r1=273289&r2=273290&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Tooling/Core/Replacement.h?rev=273290&r1=273289&r2=273290&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/include/clang/Tooling/Core/Replacement.h (original)<br>
+++ cfe/trunk/include/clang/Tooling/Core/Replacement.h Tue Jun 21 12:56:31 2016<br>
@@ -57,6 +57,11 @@ public:<br>
return RHS.Offset >= Offset &&<br>
(RHS.Offset + RHS.Length) <= (Offset + Length);<br>
}<br>
+<br>
+ /// \brief Whether this range equals to \p RHS or not.<br>
+ bool operator==(const Range &RHS) const {<br>
+ return Offset == RHS.getOffset() && Length == RHS.getLength();<br>
+ }<br>
/// @}<br>
<br>
private:<br>
@@ -222,11 +227,25 @@ bool applyAllReplacements(const std::vec<br>
std::string applyAllReplacements(StringRef Code, const Replacements &Replaces);<br>
<br>
/// \brief Calculates the ranges in a single file that are affected by the<br>
-/// Replacements.<br>
+/// Replacements. Overlapping ranges will be merged.<br>
///<br>
/// \pre Replacements must be for the same file.<br>
+///<br>
+/// \returns a non-overlapping and sorted ranges.<br>
std::vector<Range> calculateChangedRanges(const Replacements &Replaces);<br>
<br>
+/// \brief Calculates the new ranges after \p Replaces are applied. These<br>
+/// include both the original \p Ranges and the affected ranges of \p Replaces<br>
+/// in the new code.<br>
+///<br>
+/// \pre Replacements must be for the same file.<br>
+///<br>
+/// \return The new ranges after \p Replaces are applied. The new ranges will be<br>
+/// sorted and non-overlapping.<br>
+std::vector<Range><br>
+calculateRangesAfterReplacements(const Replacements &Replaces,<br>
+ const std::vector<Range> &Ranges);<br>
+<br>
/// \brief Groups a random set of replacements by file path. Replacements<br>
/// related to the same file entry are put into the same vector.<br>
std::map<std::string, Replacements><br>
<br>
Modified: cfe/trunk/lib/Tooling/Core/Replacement.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Tooling/Core/Replacement.cpp?rev=273290&r1=273289&r2=273290&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Tooling/Core/Replacement.cpp?rev=273290&r1=273289&r2=273290&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Tooling/Core/Replacement.cpp (original)<br>
+++ cfe/trunk/lib/Tooling/Core/Replacement.cpp Tue Jun 21 12:56:31 2016<br>
@@ -278,6 +278,30 @@ std::string applyAllReplacements(StringR<br>
return Result;<br>
}<br>
<br>
+// Merge and sort overlapping ranges in \p Ranges.<br>
+static std::vector<Range> mergeAndSortRanges(std::vector<Range> Ranges) {<br>
+ std::sort(Ranges.begin(), Ranges.end(),<br>
+ [](const Range &LHS, const Range &RHS) {<br>
+ if (LHS.getOffset() != RHS.getOffset())<br>
+ return LHS.getOffset() < RHS.getOffset();<br>
+ return LHS.getLength() < RHS.getLength();<br>
+ });<br>
+ std::vector<Range> Result;<br>
+ for (const auto &R : Ranges) {<br>
+ if (Result.empty() ||<br>
+ Result.back().getOffset() + Result.back().getLength() < R.getOffset()) {<br>
+ Result.push_back(R);<br>
+ } else {<br>
+ unsigned NewEnd =<br>
+ std::max(Result.back().getOffset() + Result.back().getLength(),<br>
+ R.getOffset() + R.getLength());<br>
+ Result[Result.size() - 1] =<br>
+ Range(Result.back().getOffset(), NewEnd - Result.back().getOffset());<br>
+ }<br>
+ }<br>
+ return Result;<br>
+}<br>
+<br>
std::vector<Range> calculateChangedRanges(const Replacements &Replaces) {<br>
std::vector<Range> ChangedRanges;<br>
int Shift = 0;<br>
@@ -287,7 +311,20 @@ std::vector<Range> calculateChangedRange<br>
Shift += Length - R.getLength();<br>
ChangedRanges.push_back(Range(Offset, Length));<br>
}<br>
- return ChangedRanges;<br>
+ return mergeAndSortRanges(ChangedRanges);<br>
+}<br>
+<br>
+std::vector<Range><br>
+calculateRangesAfterReplacements(const Replacements &Replaces,<br>
+ const std::vector<Range> &Ranges) {<br>
+ auto MergedRanges = mergeAndSortRanges(Ranges);<br>
+ tooling::Replacements FakeReplaces;<br>
+ for (const auto &R : MergedRanges)<br>
+ FakeReplaces.insert(Replacement(Replaces.begin()->getFilePath(),<br>
+ R.getOffset(), R.getLength(),<br>
+ std::string(" ", R.getLength())));<br>
+ tooling::Replacements NewReplaces = mergeReplacements(FakeReplaces, Replaces);<br>
+ return calculateChangedRanges(NewReplaces);<br>
}<br>
<br>
namespace {<br>
@@ -434,4 +471,3 @@ Replacements mergeReplacements(const Rep<br>
<br>
} // end namespace tooling<br>
} // end namespace clang<br>
-<br>
<br>
Modified: cfe/trunk/unittests/Tooling/RefactoringTest.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Tooling/RefactoringTest.cpp?rev=273290&r1=273289&r2=273290&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Tooling/RefactoringTest.cpp?rev=273290&r1=273289&r2=273290&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/unittests/Tooling/RefactoringTest.cpp (original)<br>
+++ cfe/trunk/unittests/Tooling/RefactoringTest.cpp Tue Jun 21 12:56:31 2016<br>
@@ -462,13 +462,96 @@ TEST(Range, CalculateRangesOfReplacement<br>
<br>
std::vector<Range> Ranges = calculateChangedRanges(Replaces);<br>
<br>
- EXPECT_EQ(3ul, Ranges.size());<br>
+ EXPECT_EQ(2ul, Ranges.size());<br>
EXPECT_TRUE(Ranges[0].getOffset() == 0);<br>
EXPECT_TRUE(Ranges[0].getLength() == 0);<br>
EXPECT_TRUE(Ranges[1].getOffset() == 6);<br>
- EXPECT_TRUE(Ranges[1].getLength() == 6);<br>
- EXPECT_TRUE(Ranges[2].getOffset() == 12);<br>
- EXPECT_TRUE(Ranges[2].getLength() == 16);<br>
+ EXPECT_TRUE(Ranges[1].getLength() == 22);<br>
+}<br>
+<br>
+TEST(Range, RangesAfterReplacements) {<br>
+ std::vector<Range> Ranges = {Range(5, 2), Range(10, 5)};<br>
+ Replacements Replaces = {Replacement("foo", 0, 2, "1234")};<br>
+ std::vector<Range> Expected = {Range(0, 4), Range(7, 2), Range(12, 5)};<br>
+ EXPECT_EQ(Expected, calculateRangesAfterReplacements(Replaces, Ranges));<br>
+}<br>
+<br>
+TEST(Range, RangesBeforeReplacements) {<br>
+ std::vector<Range> Ranges = {Range(5, 2), Range(10, 5)};<br>
+ Replacements Replaces = {Replacement("foo", 20, 2, "1234")};<br>
+ std::vector<Range> Expected = {Range(5, 2), Range(10, 5), Range(20, 4)};<br>
+ EXPECT_EQ(Expected, calculateRangesAfterReplacements(Replaces, Ranges));<br>
+}<br>
+<br>
+TEST(Range, NotAffectedByReplacements) {<br>
+ std::vector<Range> Ranges = {Range(0, 2), Range(5, 2), Range(10, 5)};<br>
+ Replacements Replaces = {Replacement("foo", 3, 2, "12"),<br>
+ Replacement("foo", 12, 2, "12"),<br>
+ Replacement("foo", 20, 5, "")};<br>
+ std::vector<Range> Expected = {Range(0, 2), Range(3, 4), Range(10, 5),<br>
+ Range(20, 0)};<br>
+ EXPECT_EQ(Expected, calculateRangesAfterReplacements(Replaces, Ranges));<br>
+}<br>
+<br>
+TEST(Range, RangesWithNonOverlappingReplacements) {<br>
+ std::vector<Range> Ranges = {Range(0, 2), Range(5, 2), Range(10, 5)};<br>
+ Replacements Replaces = {Replacement("foo", 3, 1, ""),<br>
+ Replacement("foo", 6, 1, "123"),<br>
+ Replacement("foo", 20, 2, "12345")};<br>
+ std::vector<Range> Expected = {Range(0, 2), Range(3, 0), Range(4, 4),<br>
+ Range(11, 5), Range(21, 5)};<br>
+ EXPECT_EQ(Expected, calculateRangesAfterReplacements(Replaces, Ranges));<br>
+}<br>
+<br>
+TEST(Range, RangesWithOverlappingReplacements) {<br>
+ std::vector<Range> Ranges = {Range(0, 2), Range(5, 2), Range(15, 5),<br>
+ Range(30, 5)};<br>
+ Replacements Replaces = {<br>
+ Replacement("foo", 1, 3, ""), Replacement("foo", 6, 1, "123"),<br>
+ Replacement("foo", 13, 3, "1"), Replacement("foo", 25, 15, "")};<br>
+ std::vector<Range> Expected = {Range(0, 1), Range(2, 4), Range(12, 5),<br>
+ Range(22, 0)};<br>
+ EXPECT_EQ(Expected, calculateRangesAfterReplacements(Replaces, Ranges));<br>
+}<br>
+<br>
+TEST(Range, MergeIntoOneRange) {<br>
+ std::vector<Range> Ranges = {Range(0, 2), Range(5, 2), Range(15, 5)};<br>
+ Replacements Replaces = {Replacement("foo", 1, 15, "1234567890")};<br>
+ std::vector<Range> Expected = {Range(0, 15)};<br>
+ EXPECT_EQ(Expected, calculateRangesAfterReplacements(Replaces, Ranges));<br>
+}<br>
+<br>
+TEST(Range, ReplacementsStartingAtRangeOffsets) {<br>
+ std::vector<Range> Ranges = {Range(0, 2), Range(5, 5), Range(15, 5)};<br>
+ Replacements Replaces = {<br>
+ Replacement("foo", 0, 2, "12"), Replacement("foo", 5, 1, "123"),<br>
+ Replacement("foo", 7, 4, "12345"), Replacement("foo", 15, 10, "12")};<br>
+ std::vector<Range> Expected = {Range(0, 2), Range(5, 9), Range(18, 2)};<br>
+ EXPECT_EQ(Expected, calculateRangesAfterReplacements(Replaces, Ranges));<br>
+}<br>
+<br>
+TEST(Range, ReplacementsEndingAtRangeEnds) {<br>
+ std::vector<Range> Ranges = {Range(0, 2), Range(5, 2), Range(15, 5)};<br>
+ Replacements Replaces = {Replacement("foo", 6, 1, "123"),<br>
+ Replacement("foo", 17, 3, "12")};<br>
+ std::vector<Range> Expected = {Range(0, 2), Range(5, 4), Range(17, 4)};<br>
+ EXPECT_EQ(Expected, calculateRangesAfterReplacements(Replaces, Ranges));<br>
+}<br>
+<br>
+TEST(Range, AjacentReplacements) {<br>
+ std::vector<Range> Ranges = {Range(0, 0), Range(15, 5)};<br>
+ Replacements Replaces = {Replacement("foo", 1, 2, "123"),<br>
+ Replacement("foo", 12, 3, "1234")};<br>
+ std::vector<Range> Expected = {Range(0, 0), Range(1, 3), Range(13, 9)};<br>
+ EXPECT_EQ(Expected, calculateRangesAfterReplacements(Replaces, Ranges));<br>
+}<br>
+<br>
+TEST(Range, MergeRangesAfterReplacements) {<br>
+ std::vector<Range> Ranges = {Range(8, 0), Range(5, 2), Range(9, 0), Range(0, 1)};<br>
+ Replacements Replaces = {Replacement("foo", 1, 3, ""),<br>
+ Replacement("foo", 7, 0, "12"), Replacement("foo", 9, 2, "")};<br>
+ std::vector<Range> Expected = {Range(0, 1), Range(2, 4), Range(7, 0), Range(8, 0)};<br>
+ EXPECT_EQ(Expected, calculateRangesAfterReplacements(Replaces, Ranges));<br>
}<br>
<br>
TEST(DeduplicateTest, removesDuplicates) {<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><br>
</blockquote></div><div dir="ltr">-- <br></div><div data-smartmail="gmail_signature">Mike<br>Sent from phone</div>