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

[Nemanja Ivanovic via Phabricator via cfe-commits]

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.


Repository:
  rC Clang

https://reviews.llvm.org/D59304

Files:
  lib/Driver/ToolChains/CommonArgs.cpp
  test/Driver/cspgo-lto.c


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 @@
       CmdArgs.push_back(
           Args.MakeArgString("-plugin-opt=cs-profile-path=default_%m.profraw"));
   } 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>