Incorporate feedback from @jhenderson and @rupprecht

In D65372#1664237, @MaskRay wrote:

Ping

In D62462#1627011, @jhenderson wrote:

@mmpozulp, can I confirm that this is now in? It looks like Phabricator hasn't auto-closed this issue.

In D65991#1624878, @jhenderson wrote:

In D65991#1624866, @MaskRay wrote:

Copied from llvm/tools/llvm-objcopy/StripOpts.td

Phabricator is smart?

I think it's to do with how the patch is written. I'm not sure though :/

Rebaseline to get the new llvm-strip.rst docs page.
Incorporate feedback from @jhenderson to

1. Click "Done" boxes in phabricator. Thanks :)
2. Add @FILE check to help-message.test
3. Update llvm-strip.rst and llvm-objcopy.rst docs with @FILE option.
4. Remove the header re-ordering. I like it the way it was before.
5. Change response-file.test to a) use llvm-readobj to check sections,

and b) run cmp to show the output files are identical.
TODO: Try refactoring tools which call cl::ExpandResponseFiles to
avoid copy-pasting code from lib/Support/CommandLine.cpp like I did here.

Delete CommonAliases.td by incorporating feedback from @jhenderson.

Capitalize variable newArgv -> NewArgv

Incorporate feedback from @jhenderson. I still need to try refactoring tools which call cl::ExpandResponseFiles to avoid code duplication. If successful then we won't have to continue the code duplication trend here.

Rebase on master to get commit from @wolfgangp which added --strip-sections
to llvm-strip. Move it into CommonOpts.td since llvm-objcopy supports it to.

Also, I moved the tablegen changes to https://reviews.llvm.org/D65991

Incorporate feedback from @jhenderson.
Add new switch --strip-sections. Thanks, James.

Thanks, @grimar and @jhenderson. I feel ready for another re-commit. Think so too?

Use {{.*}}2.o rather than {{.+}}

Incorporate feedback from @jhenderson.

Ok thanks for the explanations @jhenderson and @dyung. I abandoned my attempt to use [[FILE]] in the tests. See https://reviews.llvm.org/D62462.

Abandon [[FILE]] use in tests.
@grimar please try this on your windows box.

@grimar I reverted the commit, re-opened the original revision, and updated it with the '[[FILE]]' change. I'd love if you could test it on windows for me! Please use https://reviews.llvm.org/D62462.

Add single quotes so [[FILE]] is now '[[FILE]]'.
@grimar can you see if this fixes the windows failures?

Reverted in r367816 because the modified tests failed on windows http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast/builds/27368

@dyung @jhenderson do either of you know how I can test this on windows without having my own windows machine? It passes on my linux box but that's not where the failure was. Thanks :)

Replacing svn diff with git diff.

Reverted after the buildbots informed me that I caused a test to fail
http://lab.llvm.org:8011/builders/clang-ppc64be-linux-lnt/builds/29488

In D65384#1604328, @jhenderson wrote:

Thanks for working on this! You're missing a number of aliases from the list. Please make sure they are included in the doc.

In D65374#1604381, @jhenderson wrote:

Hi @mmpozulp. Thanks for looking at this. One of my colleagues (@Sunil_Srivastava) has been working on the same fix, but we forgot to update the LLVM bug. His approach also solves a bug in llvm-readelf, so would you mind holding off on this patch until he's got a chance to finish off his version ready for review?

Should we match gnu nm exactly or is this enough?
If we check all 16 possible bit-patterns of the 4-bit-wide symbol type (as in format-sysv-type.test) we can see that gnu nm prints the symbol_section and symbol_common symbols differently than we do. See (4) below for the differences.

Run git-clang-format

Thanks James. I was able to induce an error by modifying SymbolizableObjectFile::create to unconditionally return object_error::unexpected_eof which produced

./bin/llvm-objdump: warning: failed to parse debug information for tmp2.o: The end of the file was unexpectedly encountered

Use error message from Symbolizer->symbolizeCode() in "failed to parse debug information" warning

@jhenderson, can you think of an input that would cause Symbolizer->symbolizeCode(*Obj, Address) to return an Error?

@jhenderson thanks for clarifying. I'll take a look and see what I can find.

@jhenderson thanks so much for the feedback. Anything else?

Use original wording of "failed to parse..."

Emit object file name in warning message. Duh. Thanks, @jhenderson!

Incorporate feedback from @jhenderson

Change global variable names, update warning message, delete extra newline.

Rebase on master

Taking off on a camping trip for the week. If anyone wants these features upstream'd before I get back please feel free to pick up where I left off, otherwise I'll be back at next week. :)

Rebase on master

Hey @jhenderson, thanks for asking! I will have some questions for you once I look at the code again. I return from vacation on Wednesday. :)

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.