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.
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.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
