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,