This is an archive of the discontinued LLVM Phabricator instance.

Fix invocation of Gold plugin with LTO after r355331
ClosedPublic

Authored by nemanjai on Mar 13 2019, 9:24 AM.

Details

Summary

The above commit tries to access the parameter to the -fprofile-use option without checking whether the option has actually been specified with a parameter. When the option was not specified with a parameter, this trips an assertion.
All of our builds with PGO/LTO use -fprofile-use without a parameter so this broke all such internal builds.

To reproduce (the contents of the C++ file don't actually matter as long as it can be linked/run):

$ clang++ -O3 -fprofile-generate pgo-lto.cpp
$ ./a.out
$ clang++ -O3 -fprofile-use -flto=thin pgo-lto.cpp
clang++: /home/nemanjai/llvm/llvm-clean/include/llvm/ADT/SmallVector.h:153: const T& llvm::SmallVectorTemplateCommon<T, <template-parameter-1-2> >::operator[](llvm::SmallVectorTemplateCommon<T, <template-parameter-1-2> >::size_type) const [with T = const char*; <template-parameter-1-2> = void; llvm::SmallVectorTemplateCommon<T, <template-parameter-1-2> >::const_reference = const char* const&; llvm::SmallVectorTemplateCommon<T, <template-parameter-1-2> >::size_type = long unsigned int]: Assertion `idx < size()' failed.

This patch fixes it by handling the option the same way it is handled elsewhere.

The original patch did not include any test cases, so I've added one for the problem case. Had the patch been committed with tests, this issue would probably not have happened.

I am hoping for a quick review of this so we can commit this and not have to change our build tools to specify the default.profdata default.

Diff Detail

Repository
rL LLVM

Event Timeline

nemanjai created this revision.Mar 13 2019, 9:24 AM
This revision is now accepted and ready to land.Mar 13 2019, 9:33 AM

Thanks Teresa, I'll commit this soon.

This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptMar 13 2019, 4:54 PM