r273290 - Added calculateRangesAfterReplaments() to calculate affacted ranges in the new code.

Mike Aizatsky via cfe-commits cfe-commits at lists.llvm.org
Tue Jun 21 13:44:19 PDT 2016


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/2677f096/attachment-0001.html>


More information about the cfe-commits mailing list