Fix crash in "gn gen --ide=xcode" when project is malformed
As found while investigating https://crbug.com/1056925, if the
project was malformed, "gn gen --ide=xcode" could crash due to
some undefined behaviour (likely dereferencing an uninitialised
pointer).
This was due to FindApplicationTargetByName() using DCHECK to
enforce a property of the project when said property is not
under the control of the programmer but under the control of
the user writing the project's BUILD.gn files.
Instead change the function implementation to return an error
if the expected property is not verified. That error is then
propagated back to the "gn gen" implementation and displayed
to the user.
All other uses of DCHECK/NOTREACHED in xcode_writer.cc have
been inspected to verify they correspond to assertion about
the state of the code and do not depend on the properties of
the project's BUILD.gn files.
Bug: 150
Change-Id: Ibe711f2d380ef0ed5dca637799563223c248ec7d
Reviewed-on: https://gn-review.googlesource.com/c/gn/+/7560
Commit-Queue: Brett Wilson <brettw@chromium.org>
Reviewed-by: Brett Wilson <brettw@chromium.org>
diff --git a/src/gn/xcode_writer.cc b/src/gn/xcode_writer.cc
index 6ffcc58..849aca7 100644
--- a/src/gn/xcode_writer.cc
+++ b/src/gn/xcode_writer.cc
@@ -152,15 +152,22 @@
}
const Target* FindApplicationTargetByName(
+ const ParseNode* node,
const std::string& target_name,
- const std::vector<const Target*>& targets) {
+ const std::vector<const Target*>& targets,
+ Err* err) {
for (const Target* target : targets) {
if (target->label().name() == target_name) {
- DCHECK(IsApplicationTarget(target));
+ if (!IsApplicationTarget(target)) {
+ *err = Err(node, "host application target \"" + target_name +
+ "\" not an application bundle");
+ return nullptr;
+ }
return target;
}
}
- NOTREACHED();
+ *err =
+ Err(node, "cannot find host application bundle \"" + target_name + "\"");
return nullptr;
}
@@ -179,16 +186,21 @@
// Adds the corresponding test application target as dependency of xctest or
// xcuitest module target in the generated Xcode project.
-void AddDependencyTargetForTestModuleTargets(
+bool AddDependencyTargetForTestModuleTargets(
const std::vector<const Target*>& targets,
const TargetToPBXTarget& bundle_target_to_pbxtarget,
- const PBXProject* project) {
+ const PBXProject* project,
+ Err* err) {
for (const Target* target : targets) {
if (!IsXCTestModuleTarget(target) && !IsXCUITestModuleTarget(target))
continue;
const Target* test_application_target = FindApplicationTargetByName(
- target->bundle_data().xcode_test_application_name(), targets);
+ target->defined_from(),
+ target->bundle_data().xcode_test_application_name(), targets, err);
+ if (!test_application_target)
+ return false;
+
const PBXTarget* test_application_pbxtarget =
bundle_target_to_pbxtarget.at(test_application_target);
PBXTarget* module_pbxtarget = bundle_target_to_pbxtarget.at(target);
@@ -198,6 +210,8 @@
AddPBXTargetDependency(test_application_pbxtarget, module_pbxtarget,
project);
}
+
+ return true;
}
// Searches the list of xctest files recursively under |target|.
@@ -405,10 +419,12 @@
}
XcodeWriter workspace(workspace_name);
- workspace.CreateProductsProject(targets, all_targets, attributes, source_path,
- config_name, root_target_name,
- ninja_executable, ninja_extra_args,
- build_settings, target_os);
+ if (!workspace.CreateProductsProject(
+ targets, all_targets, attributes, source_path, config_name,
+ root_target_name, ninja_executable, ninja_extra_args, build_settings,
+ target_os, err)) {
+ return false;
+ }
return workspace.WriteFiles(build_settings, err);
}
@@ -474,7 +490,7 @@
return true;
}
-void XcodeWriter::CreateProductsProject(
+bool XcodeWriter::CreateProductsProject(
const std::vector<const Target*>& targets,
const std::vector<const Target*>& all_targets,
const PBXAttributes& attributes,
@@ -484,7 +500,8 @@
const std::string& ninja_executable,
const std::string& ninja_extra_args,
const BuildSettings* build_settings,
- TargetOsType target_os) {
+ TargetOsType target_os,
+ Err* err) {
std::unique_ptr<PBXProject> main_project(
new PBXProject("products", config_name, source_path, attributes));
@@ -563,11 +580,14 @@
const Target* target_with_xctest_files = nullptr;
if (IsXCTestModuleTarget(target)) {
target_with_xctest_files = FindApplicationTargetByName(
- target->bundle_data().xcode_test_application_name(), targets);
- } else if (IsXCUITestModuleTarget(target)) {
- target_with_xctest_files = target;
+ target->defined_from(),
+ target->bundle_data().xcode_test_application_name(), targets,
+ err);
+ if (!target_with_xctest_files)
+ return false;
} else {
- NOTREACHED();
+ DCHECK(IsXCUITestModuleTarget(target));
+ target_with_xctest_files = target;
}
SearchXCTestFilesForTarget(target_with_xctest_files,
@@ -592,10 +612,14 @@
// Adding the corresponding test application target as a dependency of xctest
// or xcuitest module target in the generated Xcode project so that the
// application target is re-compiled when compiling the test module target.
- AddDependencyTargetForTestModuleTargets(
- bundle_targets, bundle_target_to_pbxtarget, main_project.get());
+ if (!AddDependencyTargetForTestModuleTargets(bundle_targets,
+ bundle_target_to_pbxtarget,
+ main_project.get(), err)) {
+ return false;
+ }
projects_.push_back(std::move(main_project));
+ return true;
}
bool XcodeWriter::WriteFiles(const BuildSettings* build_settings, Err* err) {
diff --git a/src/gn/xcode_writer.h b/src/gn/xcode_writer.h
index e391c56..b50b394 100644
--- a/src/gn/xcode_writer.h
+++ b/src/gn/xcode_writer.h
@@ -65,7 +65,7 @@
// Generate the "products.xcodeproj" project that reference all products
// (i.e. targets that have a build artefact usable from Xcode, mostly
// application bundles).
- void CreateProductsProject(const std::vector<const Target*>& targets,
+ bool CreateProductsProject(const std::vector<const Target*>& targets,
const std::vector<const Target*>& all_targets,
const PBXAttributes& attributes,
const std::string& source_path,
@@ -74,7 +74,8 @@
const std::string& ninja_executable,
const std::string& ninja_extra_args,
const BuildSettings* build_settings,
- TargetOsType target_os);
+ TargetOsType target_os,
+ Err* err);
bool WriteFiles(const BuildSettings* build_settings, Err* err);
bool WriteProjectFile(const BuildSettings* build_settings,
