[PATCH] D44607: Recompute invalidated iterator in insertTargetAndModeArgs
marcan created this revision.
marcan added reviewers: sepavloff, echristo.
Herald added a subscriber: cfe-commits.
Doing an `.insert()` can potentially invalidate iterators by reallocating the
vector's storage. When all the stars align just right, this causes segfaults or
glibc aborts. Fix this by recomputing the position before each insert.
Gentoo Linux bug (crashes while building Chromium):
https://bugs.gentoo.org/650082
Repository:
rC Clang
https://reviews.llvm.org/D44607
Files:
tools/driver/driver.cpp
Index: tools/driver/driver.cpp
===
--- tools/driver/driver.cpp
+++ tools/driver/driver.cpp
@@ -209,20 +209,24 @@
static void insertTargetAndModeArgs(const ParsedClangName &NameParts,
SmallVectorImpl &ArgVector,
std::set &SavedStrings) {
- // Put target and mode arguments at the start of argument list so that
- // arguments specified in command line could override them. Avoid putting
- // them at index 0, as an option like '-cc1' must remain the first.
- auto InsertionPoint = ArgVector.begin();
- if (InsertionPoint != ArgVector.end())
-++InsertionPoint;
-
if (NameParts.DriverMode) {
+// Put target and mode arguments at the start of argument list so that
+// arguments specified in command line could override them. Avoid putting
+// them at index 0, as an option like '-cc1' must remain the first.
+auto InsertionPoint = ArgVector.begin();
+if (InsertionPoint != ArgVector.end())
+++InsertionPoint;
+
// Add the mode flag to the arguments.
ArgVector.insert(InsertionPoint,
GetStableCStr(SavedStrings, NameParts.DriverMode));
}
if (NameParts.TargetIsValid) {
+auto InsertionPoint = ArgVector.begin();
+if (InsertionPoint != ArgVector.end())
+++InsertionPoint;
+
const char *arr[] = {"-target", GetStableCStr(SavedStrings,
NameParts.TargetPrefix)};
ArgVector.insert(InsertionPoint, std::begin(arr), std::end(arr));
Index: tools/driver/driver.cpp
===
--- tools/driver/driver.cpp
+++ tools/driver/driver.cpp
@@ -209,20 +209,24 @@
static void insertTargetAndModeArgs(const ParsedClangName &NameParts,
SmallVectorImpl &ArgVector,
std::set &SavedStrings) {
- // Put target and mode arguments at the start of argument list so that
- // arguments specified in command line could override them. Avoid putting
- // them at index 0, as an option like '-cc1' must remain the first.
- auto InsertionPoint = ArgVector.begin();
- if (InsertionPoint != ArgVector.end())
-++InsertionPoint;
-
if (NameParts.DriverMode) {
+// Put target and mode arguments at the start of argument list so that
+// arguments specified in command line could override them. Avoid putting
+// them at index 0, as an option like '-cc1' must remain the first.
+auto InsertionPoint = ArgVector.begin();
+if (InsertionPoint != ArgVector.end())
+++InsertionPoint;
+
// Add the mode flag to the arguments.
ArgVector.insert(InsertionPoint,
GetStableCStr(SavedStrings, NameParts.DriverMode));
}
if (NameParts.TargetIsValid) {
+auto InsertionPoint = ArgVector.begin();
+if (InsertionPoint != ArgVector.end())
+++InsertionPoint;
+
const char *arr[] = {"-target", GetStableCStr(SavedStrings,
NameParts.TargetPrefix)};
ArgVector.insert(InsertionPoint, std::begin(arr), std::end(arr));
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44607: Recompute invalidated iterator in insertTargetAndModeArgs
marcan added a comment. I'm not sure how to test this. Getting the bug to manifest involves all of having a lot of command line arguments, getting unlucky with the way SmallVectorImpl decides to allocate, and getting unlucky with the glibc allocator deciding to move the data block on realloc (or running under valgrind). The easiest reproducer I have is `valgrind x86_64-pc-linux-gnu-clang++ $(seq 1 512)` with a debug build, but I'm not sure that that guarantees an abort in every case. 511 arguments is not sufficient to trigger it on my system, but 512 is. Repository: rC Clang https://reviews.llvm.org/D44607 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44607: Recompute invalidated iterator in insertTargetAndModeArgs
marcan updated this revision to Diff 13.
Repository:
rC Clang
https://reviews.llvm.org/D44607
Files:
tools/driver/driver.cpp
Index: tools/driver/driver.cpp
===
--- tools/driver/driver.cpp
+++ tools/driver/driver.cpp
@@ -212,20 +212,21 @@
// Put target and mode arguments at the start of argument list so that
// arguments specified in command line could override them. Avoid putting
// them at index 0, as an option like '-cc1' must remain the first.
- auto InsertionPoint = ArgVector.begin();
- if (InsertionPoint != ArgVector.end())
+ int InsertionPoint = 0;
+ if (ArgVector.size() > 0)
++InsertionPoint;
if (NameParts.DriverMode) {
// Add the mode flag to the arguments.
-ArgVector.insert(InsertionPoint,
+ArgVector.insert(ArgVector.begin() + InsertionPoint,
GetStableCStr(SavedStrings, NameParts.DriverMode));
}
if (NameParts.TargetIsValid) {
const char *arr[] = {"-target", GetStableCStr(SavedStrings,
NameParts.TargetPrefix)};
-ArgVector.insert(InsertionPoint, std::begin(arr), std::end(arr));
+ArgVector.insert(ArgVector.begin() + InsertionPoint,
+ std::begin(arr), std::end(arr));
}
}
Index: tools/driver/driver.cpp
===
--- tools/driver/driver.cpp
+++ tools/driver/driver.cpp
@@ -212,20 +212,21 @@
// Put target and mode arguments at the start of argument list so that
// arguments specified in command line could override them. Avoid putting
// them at index 0, as an option like '-cc1' must remain the first.
- auto InsertionPoint = ArgVector.begin();
- if (InsertionPoint != ArgVector.end())
+ int InsertionPoint = 0;
+ if (ArgVector.size() > 0)
++InsertionPoint;
if (NameParts.DriverMode) {
// Add the mode flag to the arguments.
-ArgVector.insert(InsertionPoint,
+ArgVector.insert(ArgVector.begin() + InsertionPoint,
GetStableCStr(SavedStrings, NameParts.DriverMode));
}
if (NameParts.TargetIsValid) {
const char *arr[] = {"-target", GetStableCStr(SavedStrings,
NameParts.TargetPrefix)};
-ArgVector.insert(InsertionPoint, std::begin(arr), std::end(arr));
+ArgVector.insert(ArgVector.begin() + InsertionPoint,
+ std::begin(arr), std::end(arr));
}
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44607: Recompute invalidated iterator in insertTargetAndModeArgs
marcan added a comment. Note that I don't have commit access, so someone else will have to commit it for me :) Repository: rC Clang https://reviews.llvm.org/D44607 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
