diff --git a/database_writer.cc b/database_writer.cc index a2efa890..4a3b8667 100644 --- a/database_writer.cc +++ b/database_writer.cc @@ -881,6 +881,15 @@ absl::Status DatabaseReader::Read(CallGraph& call_graph1, .Into(&similarity_) .Into(&confidence_); ABSL_RETURN_IF_ERROR(statement.Execute()); + if (primary_filename_ == secondary_filename_) { + // Both sides would be read from the same .BinExport next to the results + // file; one of them has necessarily been overwritten by the other. + return absl::FailedPreconditionError(absl::StrCat( + "primary and secondary BinExport files both resolve to \"", + JoinPath(path_, primary_filename_ + ".BinExport"), + "\". Results for same-named binaries need to be re-diffed and saved " + "again")); + } // Function matches ABSL_ASSIGN_OR_RETURN( diff --git a/ida/main_plugin.cc b/ida/main_plugin.cc index 2834c805..de9ebbca 100644 --- a/ida/main_plugin.cc +++ b/ida/main_plugin.cc @@ -223,6 +223,17 @@ absl::StatusOr ExportIdbs() { "same name in the same directory. Please rename or move one of the " "files."); } + // IDA unpacks an open database next to it. The secondary export runs in a + // separate IDA process, which cannot open such a database and exits without + // exporting. Check now instead of failing after the primary export. + if (const std::string unpacked = + ReplaceFileExtension(secondary_idb_path, ".id0"); + FileExists(unpacked)) { + return absl::FailedPreconditionError(absl::StrCat( + "The secondary database is open in another IDA instance or was not " + "closed properly (found \"", + unpacked, "\").\nPlease close it there and try again.")); + } LOG(INFO) << "Diffing " << Basename(primary_idb_path) << " vs " << Basename(secondary_idb_path); @@ -236,6 +247,20 @@ absl::StatusOr ExportIdbs() { RemoveAll(secondary_temp_dir).IgnoreError(); ABSL_RETURN_IF_ERROR(CreateDirectories(secondary_temp_dir)); + // The export names end up as the "filename" column of the result file and + // as the names of the .BinExport files saved next to it. Same-named + // databases (e.g. two versions of the same binary in different directories) + // would overwrite each other there, so disambiguate both sides the same way + // the BinDiff UI does. + std::string primary_name = + ReplaceFileExtension(Basename(primary_idb_path), ""); + const std::string secondary_name = + ReplaceFileExtension(Basename(secondary_idb_path), ""); + const bool same_name = primary_name == secondary_name; + if (same_name) { + primary_name += "_primary"; + } + { const auto& config = config::Proto(); auto options = @@ -254,9 +279,8 @@ absl::StatusOr ExportIdbs() { std::thread export_thread( [&status, &exporter]() { status = exporter.Export(); }); - const std::string primary_binexport = JoinPath( - primary_temp_dir, - ReplaceFileExtension(Basename(primary_idb_path), ".BinExport")); + const std::string primary_binexport = + JoinPath(primary_temp_dir, absl::StrCat(primary_name, ".BinExport")); idc_value_t arg = primary_binexport.c_str(); if (qstring errbuf; !call_idc_func( /*result=*/nullptr, "BinExportBinary", &arg, @@ -272,6 +296,22 @@ absl::StatusOr ExportIdbs() { "Export of the secondary database failed: ", status.message())); } } + if (same_name) { + // IdbExporter always names its output after the database. It ignores IDA's + // exit code, so a failed secondary export (e.g. the database is open in + // another IDA instance) only shows as a missing file, which the caller + // reports. + const std::string exported = JoinPath( + secondary_temp_dir, absl::StrCat(secondary_name, ".BinExport")); + const std::string renamed = + JoinPath(secondary_temp_dir, + absl::StrCat(secondary_name, "_secondary.BinExport")); + if (FileExists(exported) && + std::rename(exported.c_str(), renamed.c_str()) != 0) { + return absl::UnknownError(absl::StrCat( + "Failed to rename secondary export to \"", renamed, "\"")); + } + } return true; }