This is an archive of the discontinued LLVM Phabricator instance.

[ADT] Fix asan-detected stack-buffer-overflow in StringSetTest.cpp
ClosedPublic

Authored by mmpozulp on Jun 7 2019, 2:07 AM.

Download Raw Diff

Details

Reviewers

aaron.ballman
dblaikie

Diff Detail

Repository

rG LLVM Github Monorepo

Build Status

Buildable 33051
Build 33050: arc lint + arc unit

Event Timeline

mmpozulp created this revision.Jun 7 2019, 2:07 AM

Herald added a project: Restricted Project. · View Herald TranscriptJun 7 2019, 2:07 AM

Herald added subscribers: llvm-commits, dexonsmith. · View Herald Transcript

Harbormaster completed remote builds in B33051: Diff 203516.Jun 7 2019, 2:08 AM

This fixes the build error discovered by the asan buildbot

FAIL: LLVM-Unit :: ADT/./ADTTests/StringSetTest.InsertAndCountStringMapEntry (1017 of 31700)
******************** TEST 'LLVM-Unit :: ADT/./ADTTests/StringSetTest.InsertAndCountStringMapEntry' FAILED ********************
Note: Google Test filter = StringSetTest.InsertAndCountStringMapEntry
[==========] Running 1 test from 1 test case.
[----------] Global test environment set-up.
[----------] 1 test from StringSetTest
[ RUN      ] StringSetTest.InsertAndCountStringMapEntry
=================================================================
==10147==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7fc65d1d0738 at pc 0x0000013db225 bp 0x7ffd223d3200 sp 0x7ffd223d31f8
READ of size 1 at 0x7fc65d1d0738 thread T0
    #0 0x13db224 in djbHash /b/sanitizer-x86_64-linux-fast/build/llvm/include/llvm/Support/DJB.h:22:24
    #1 0x13db224 in llvm::StringMapImpl::LookupBucketFor(llvm::StringRef) /b/sanitizer-x86_64-linux-fast/build/llvm/lib/Support/StringMap.cpp:83
    #2 0x11e5d81 in std::__1::pair<llvm::StringMapIterator<char>, bool> llvm::StringMap<char, llvm::MallocAllocator>::try_emplace<char>(llvm::StringRef, char&&) /b/sanitizer-x86_64-linux-fast/build/llvm/include/llvm/ADT/StringMap.h:400:25
    #3 0x11e6360 in insert /b/sanitizer-x86_64-linux-fast/build/llvm/include/llvm/ADT/StringMap.h:391:12
    #4 0x11e6360 in insert /b/sanitizer-x86_64-linux-fast/build/llvm/include/llvm/ADT/StringSet.h:40
    #5 0x11e6360 in insert<llvm::StringRef> /b/sanitizer-x86_64-linux-fast/build/llvm/include/llvm/ADT/StringSet.h:52
    #6 0x11e6360 in (anonymous namespace)::StringSetTest_InsertAndCountStringMapEntry_Test::TestBody() /b/sanitizer-x86_64-linux-fast/build/llvm/unittests/ADT/StringSetTest.cpp:37
    #7 0x1470290 in HandleExceptionsInMethodIfSupported<testing::Test, void> /b/sanitizer-x86_64-linux-fast/build/llvm/utils/unittest/googletest/src/gtest.cc
    #8 0x1470290 in testing::Test::Run() /b/sanitizer-x86_64-linux-fast/build/llvm/utils/unittest/googletest/src/gtest.cc:2474
    #9 0x1472845 in testing::TestInfo::Run() /b/sanitizer-x86_64-linux-fast/build/llvm/utils/unittest/googletest/src/gtest.cc:2656:11
    #10 0x1473cc0 in testing::TestCase::Run() /b/sanitizer-x86_64-linux-fast/build/llvm/utils/unittest/googletest/src/gtest.cc:2774:28
    #11 0x14927ad in testing::internal::UnitTestImpl::RunAllTests() /b/sanitizer-x86_64-linux-fast/build/llvm/utils/unittest/googletest/src/gtest.cc:4649:43
    #12 0x1491960 in HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool> /b/sanitizer-x86_64-linux-fast/build/llvm/utils/unittest/googletest/src/gtest.cc
    #13 0x1491960 in testing::UnitTest::Run() /b/sanitizer-x86_64-linux-fast/build/llvm/utils/unittest/googletest/src/gtest.cc:4257
    #14 0x1454700 in RUN_ALL_TESTS /b/sanitizer-x86_64-linux-fast/build/llvm/utils/unittest/googletest/include/gtest/gtest.h:2233:46
    #15 0x1454700 in main /b/sanitizer-x86_64-linux-fast/build/llvm/utils/unittest/UnitTestMain/TestMain.cpp:50
    #16 0x7fc66053c2e0 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x202e0)
    #17 0x5daf89 in _start (/b/sanitizer-x86_64-linux-fast/build/llvm_build_asan/unittests/ADT/ADTTests+0x5daf89)

Address 0x7fc65d1d0738 is located in stack of thread T0 at offset 312 in frame
    #0 0x11e614f in (anonymous namespace)::StringSetTest_InsertAndCountStringMapEntry_Test::TestBody() /b/sanitizer-x86_64-linux-fast/build/llvm/unittests/ADT/StringSetTest.cpp:32

  This frame has 10 object(s):
    [32, 56) 'ref.tmp.i'
    [96, 120) 'ref.tmp1.i'
    [160, 184) 'agg.tmp3.i.i'
    [224, 256) 'Set' (line 35)
    [288, 312) 'Element' (line 36) <== Memory access at offset 312 overflows this variable
    [352, 360) 'Count' (line 38)
    [384, 392) 'Expected' (line 39)
    [416, 432) 'gtest_ar' (line 40)
    [448, 456) 'ref.tmp' (line 40)
    [480, 488) 'ref.tmp4' (line 40)
HINT: this may be a false positive if your program uses some custom stack unwind mechanism, swapcontext or vfork
      (longjmp and C++ exceptions *are* supported)
SUMMARY: AddressSanitizer: stack-buffer-overflow /b/sanitizer-x86_64-linux-fast/build/llvm/include/llvm/Support/DJB.h:22:24 in djbHash
Shadow bytes around the buggy address:
  0x0ff94ba32090: f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5
  0x0ff94ba320a0: f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5
  0x0ff94ba320b0: f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5
  0x0ff94ba320c0: f1 f1 f1 f1 f8 f8 f8 f2 f2 f2 f2 f2 f8 f8 f8 f2
  0x0ff94ba320d0: f2 f2 f2 f2 00 00 00 f2 f2 f2 f2 f2 00 00 00 00
=>0x0ff94ba320e0: f2 f2 f2 f2 00 00 00[f2]f2 f2 f2 f2 f8 f2 f2 f2
  0x0ff94ba320f0: f8 f2 f2 f2 f8 f8 f2 f2 f8 f2 f2 f2 f8 f3 f3 f3
  0x0ff94ba32100: f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5
  0x0ff94ba32110: f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5
  0x0ff94ba32120: f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5
  0x0ff94ba32130: f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5
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
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  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
  Shadow gap:              cc

I'm still not convinced that I'm using the StringMapEntry API correctly, but this revision at least gets rid of the stack-buffer-overflow that I created in r362766 when I wrote this test.

Sounds good - thanks

This revision is now accepted and ready to land.Jun 7 2019, 11:25 AM

I applied this revision on top of D62369.

Revision Contents

Path

Size

llvm/

unittests/

ADT/

StringSetTest.cpp

7 lines

Diff 203516

llvm/unittests/ADT/StringSetTest.cpp

Show All 27 Lines	TEST_F(StringSetTest, IterSetKeys) {
SmallVector<StringRef, 4> Expected = {"A", "B", "C", "D"};		SmallVector<StringRef, 4> Expected = {"A", "B", "C", "D"};
EXPECT_EQ(Expected, Keys);		EXPECT_EQ(Expected, Keys);
}		}

TEST_F(StringSetTest, InsertAndCountStringMapEntry) {		TEST_F(StringSetTest, InsertAndCountStringMapEntry) {
// Test insert(StringMapEntry) and count(StringMapEntry)		// Test insert(StringMapEntry) and count(StringMapEntry)
// which are required for set_difference(StringSet, StringSet).		// which are required for set_difference(StringSet, StringSet).
StringSet<> Set;		StringSet<> Set;
StringMapEntry<StringRef> Element(1, "A");		StringMapEntry<StringRef> *Element = StringMapEntry<StringRef>::Create("A");
Set.insert(Element);		Set.insert(*Element);
size_t Count = Set.count(Element);		size_t Count = Set.count(*Element);
size_t Expected = 1;		size_t Expected = 1;
EXPECT_EQ(Expected, Count);		EXPECT_EQ(Expected, Count);
		Element->Destroy();
}		}

} // end anonymous namespace		} // end anonymous namespace