[PATCH] D59304: Fix invocation of Gold plugin with LTO after r355331

Nemanja Ivanovic via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Mar 13 09:26:46 PDT 2019

nemanjai created this revision.
nemanjai added reviewers: xur, tejohnson, davidxl.
Herald added subscribers: dexonsmith, steven_wu, inglorion, mehdi_amini.
Herald added a project: clang.

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.

  rC Clang



Index: test/Driver/cspgo-lto.c
--- test/Driver/cspgo-lto.c
+++ test/Driver/cspgo-lto.c
@@ -0,0 +1,6 @@
+// RUN: touch %t.o
+// RUN: %clang -target x86_64-unknown-linux -### %t.o -flto=thin \
+// RUN:   -fprofile-use 2>&1 | FileCheck %s
+// CHECK: -plugin-opt=cs-profile-path=default.profdata
Index: lib/Driver/ToolChains/CommonArgs.cpp
--- lib/Driver/ToolChains/CommonArgs.cpp
+++ lib/Driver/ToolChains/CommonArgs.cpp
@@ -464,8 +464,12 @@
   } else if (ProfileUseArg) {
+    SmallString<128> Path(
+        ProfileUseArg->getNumValues() == 0 ? "" : ProfileUseArg->getValue());
+    if (Path.empty() || llvm::sys::fs::is_directory(Path))
+      llvm::sys::path::append(Path, "default.profdata");
     CmdArgs.push_back(Args.MakeArgString(Twine("-plugin-opt=cs-profile-path=") +
-                                         ProfileUseArg->getValue()));
+                                         Path));
   // Need this flag to turn on new pass manager via Gold plugin.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D59304.190429.patch
Type: text/x-patch
Size: 1221 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20190313/6440c8df/attachment.bin>

More information about the cfe-commits mailing list