Incorporate excellent feedback from @grimar and @jhenderson. Thanks!

Committed as r362838.

I applied this revision on top of D62369.

Committed as r362835.

Fix asan-detected stack-buffer-overflow in StringSetTest.cpp

@dblaikie approved the stack-buffer-overflow fix (see https://reviews.llvm.org/D63000) which I have applied here, so I'm going to re-land this patch :)

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)

Committed as r362771.

Incorporate feedback from @jhenderson to change 'headers' to 'files' in test description.

Committed as r362768. Thanks!

Incorporate feedback from @MaskRay to delete redundant if-statement.

I committed as r362766. Thanks!

Please see https://reviews.llvm.org/D62369 which is the revision that I meant to update. Instead I created this new revision by accident.

Delete unused header in StringSetTest.cpp.

I meant to run arc diff --update D62369 but I ran arc diff instead, oops! Please ignore this revision.

Good news! Apparently I can commit from git (https://llvm.org/docs/GettingStarted.html#for-developers-to-commit-changes-from-git). Thank you, @smeenai!!! I should be able to land this commit soon. Good-bye svn :)

Will do. Sorry for the delay, I'm fighting with svn. See my comment in https://reviews.llvm.org/D62369.

I'm fighting with svn. See my comment in https://reviews.llvm.org/D62369.

I'm fighting with svn. I ran svn co https://user@llvm.org/svn/llvm-project/llvm/trunk llvm which took about 90 minutes to checkout the repo. If I run svn up it hangs for ~10 minutes on a poll() call (which I discovered by running strace) then emits an error

Incorporate feedback from @grimar to simplify an if-statement.

In D62461#1517931, @grimar wrote:

This LGTM. But please hold this on until anyone else take a look too.

In D62369#1522112, @dblaikie wrote:

Can you commit this yourself - or would you like me to do it for you?

Change a const char * to a StringRef

Incorporate feedback from @dblaikie to call insert(StringMapEntry) and count(StringMapEntry) instead of set_difference(StringSet, StringSet) in StringSetTest.cpp, which better adheres to the spirit of unit-testing.

Incorporate feedback from @jhenderson to remove unneeded errs().flush().

Incorporated feedback from @dblaikie to change test name from SetOperationsTest.cpp to StringSetTest.cpp and moved an existing StringSet test from StringMapTest.cpp to new file StringSetTest.cpp.

Incorporate feedback from @jhenderson, including moving ADT changes (which must land before this patch) to a separate patch https://reviews.llvm.org/D62369

This patch must land before https://reviews.llvm.org/D62275 in which I use set_difference() on StringSet

In D61117#1513247, @jhenderson wrote:

LGTM. Do you need help committing this?

I've taken a bit of a heavy-hammer approach here. In order to use set_difference() on StringSet I had to extend the StringMap and StringSet APIs. Are the downsides of having a bigger API warranted by the ability to use set_difference() on StringSet? If so, I can break it out into a different patch. An alternative would be to write this patch without calling set_difference(), but that imposes some code duplication.

Use yaml2obj %s instead of yaml2obj < %s in test, thanks @rupprecht.

Update test to make sure that we print 'U' for a unique symbol without a section. Thanks, @jhenderson, for suggesting this.

Move the 'u' check before if (SecI != Obj.section_end()), thanks @MaskRay.

Change a CHECK to CHECK-NEXT, thanks @jhenderson.

In D62054#1508331, @jhenderson wrote:

LGTM, with one minor comment outstanding.

Do you need somebody to commit it for you?

In D61117#1507698, @MaskRay wrote:

BTW, I think STB_GNU_UNIQUE is a misfeature (no clear specification, only (complex) implementation is in glibc, interaction with STB_WEAK/STB_GLOBAL is unclear..).. this feature will highlight the problem :)

Incorporate feedback from @jhenderson.

Squish tests together into one file and use more descriptive section/symbol names. Thanks @jhenderson :)

Incorporate feedback by @jhenderson.

Rebase and fix yaml spaces. Thanks @MaskRay and @jhenderson :)

Incorporate feedback by @grimar

Inline the check for gnu unique symbol as per suggestion by @grimar. Use yaml2obj instead of llvm-mc in unique.test.

Delete unnecessary modification to ELFTypes.h

Incorporate feedback from @rupprecht

Protect dyn_cast with isELF() check