From 3172e88af544ca53a54930ccf3f580b15141f01c Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Sat, 20 Jul 2024 22:46:09 +0200 Subject: [PATCH] Make abort() call sites log first --- src/libcmd/repl.cc | 2 +- src/libexpr/attr-path.cc | 2 +- src/libexpr/eval.cc | 8 +++---- src/libexpr/nixexpr.cc | 4 ++-- src/libexpr/primops.cc | 2 +- src/libexpr/print-ambiguous.cc | 2 +- src/libexpr/print.cc | 4 ++-- src/libexpr/symbol-table.hh | 3 ++- src/libexpr/value.hh | 2 +- src/libfetchers/attrs.cc | 4 ++-- src/libmain/loggers.cc | 2 +- .../build/drv-output-substitution-goal.hh | 2 +- src/libstore/build/goal.hh | 4 ++-- src/libstore/build/substitution-goal.hh | 2 +- src/libstore/build/worker.cc | 4 ++-- src/libstore/daemon.cc | 2 +- src/libstore/gc.cc | 2 +- src/libstore/globals.cc | 2 +- src/libstore/nar-info-disk-cache.cc | 4 ++-- .../unix/build/local-derivation-goal.cc | 2 +- src/libstore/unix/pathlocks.cc | 2 +- src/libutil/chunked-vector.hh | 4 +++- src/libutil/error.cc | 12 ++++++++++ src/libutil/error.hh | 24 +++++++++++++++++++ src/libutil/file-content-address.cc | 2 +- src/libutil/fs-sink.cc | 2 +- src/libutil/git.cc | 2 +- src/libutil/hash.cc | 2 +- src/libutil/logging.cc | 2 +- src/libutil/serialise.cc | 8 +++---- src/libutil/sync.hh | 4 +++- src/libutil/unix/monitor-fd.hh | 4 +++- src/libutil/unix/processes.cc | 2 +- src/nix-env/nix-env.cc | 2 +- src/nix-store/nix-store.cc | 2 +- 35 files changed, 88 insertions(+), 45 deletions(-) diff --git a/src/libcmd/repl.cc b/src/libcmd/repl.cc index b5d0816dd..bf0d820c2 100644 --- a/src/libcmd/repl.cc +++ b/src/libcmd/repl.cc @@ -217,7 +217,7 @@ ReplExitStatus NixRepl::mainLoop() case ProcessLineResult::PromptAgain: break; default: - abort(); + unreachable(); } } catch (ParseError & e) { if (e.msg().find("unexpected end of file") != std::string::npos) { diff --git a/src/libexpr/attr-path.cc b/src/libexpr/attr-path.cc index d61d93630..2f67260c5 100644 --- a/src/libexpr/attr-path.cc +++ b/src/libexpr/attr-path.cc @@ -134,7 +134,7 @@ std::pair findPackageFilename(EvalState & state, Value & v return {SourcePath{path.accessor, CanonPath(fn.substr(0, colon))}, lineno}; } catch (std::invalid_argument & e) { fail(); - abort(); + unreachable(); } } diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index b192f9b4b..32bc68e6d 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -149,7 +149,7 @@ std::string_view showType(ValueType type, bool withArticle) case nFloat: return WA("a", "float"); case nThunk: return WA("a", "thunk"); } - abort(); + unreachable(); } @@ -771,7 +771,7 @@ void EvalState::runDebugRepl(const Error * error, const Env & env, const Expr & case ReplExitStatus::Continue: break; default: - abort(); + unreachable(); } } } @@ -1140,7 +1140,7 @@ inline void EvalState::evalAttrs(Env & env, Expr * e, Value & v, const PosIdx po void Expr::eval(EvalState & state, Env & env, Value & v) { - abort(); + unreachable(); } @@ -1573,7 +1573,7 @@ void EvalState::callFunction(Value & fun, size_t nrArgs, Value * * args, Value & .withFrame(*fun.payload.lambda.env, lambda) .debugThrow(); } - abort(); // can't happen + unreachable(); } } diff --git a/src/libexpr/nixexpr.cc b/src/libexpr/nixexpr.cc index 6c6769cfd..dbc74faf9 100644 --- a/src/libexpr/nixexpr.cc +++ b/src/libexpr/nixexpr.cc @@ -25,7 +25,7 @@ std::ostream & operator <<(std::ostream & str, const SymbolStr & symbol) void Expr::show(const SymbolTable & symbols, std::ostream & str) const { - abort(); + unreachable(); } void ExprInt::show(const SymbolTable & symbols, std::ostream & str) const @@ -271,7 +271,7 @@ std::string showAttrPath(const SymbolTable & symbols, const AttrPath & attrPath) void Expr::bindVars(EvalState & es, const std::shared_ptr & env) { - abort(); + unreachable(); } void ExprInt::bindVars(EvalState & es, const std::shared_ptr & env) diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index 5a373a43b..0b3b19b57 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -426,7 +426,7 @@ static void prim_typeOf(EvalState & state, const PosIdx pos, Value * * args, Val t = args[0]->external()->typeOf(); break; case nFloat: t = "float"; break; - case nThunk: abort(); + case nThunk: unreachable(); } v.mkString(t); } diff --git a/src/libexpr/print-ambiguous.cc b/src/libexpr/print-ambiguous.cc index 5d55b45da..a40c98643 100644 --- a/src/libexpr/print-ambiguous.cc +++ b/src/libexpr/print-ambiguous.cc @@ -94,7 +94,7 @@ void printAmbiguous( break; default: printError("Nix evaluator internal error: printAmbiguous: invalid value type"); - abort(); + unreachable(); } } diff --git a/src/libexpr/print.cc b/src/libexpr/print.cc index bc17d6bfe..4d1a6868c 100644 --- a/src/libexpr/print.cc +++ b/src/libexpr/print.cc @@ -475,7 +475,7 @@ private: else output << "primop"; } else { - abort(); + unreachable(); } output << "ยป"; @@ -504,7 +504,7 @@ private: if (options.ansiColors) output << ANSI_NORMAL; } else { - abort(); + unreachable(); } } diff --git a/src/libexpr/symbol-table.hh b/src/libexpr/symbol-table.hh index c7a3563b0..dee7369e8 100644 --- a/src/libexpr/symbol-table.hh +++ b/src/libexpr/symbol-table.hh @@ -7,6 +7,7 @@ #include "types.hh" #include "chunked-vector.hh" +#include "error.hh" namespace nix { @@ -113,7 +114,7 @@ public: SymbolStr operator[](Symbol s) const { if (s.id == 0 || s.id > store.size()) - abort(); + unreachable(); return SymbolStr(store[s.id - 1]); } diff --git a/src/libexpr/value.hh b/src/libexpr/value.hh index 1f4d72d39..fdc6c84c4 100644 --- a/src/libexpr/value.hh +++ b/src/libexpr/value.hh @@ -285,7 +285,7 @@ public: if (invalidIsThunk) return nThunk; else - abort(); + unreachable(); } inline void finishValue(InternalType newType, Payload newPayload) diff --git a/src/libfetchers/attrs.cc b/src/libfetchers/attrs.cc index b788c5948..25d04cdc9 100644 --- a/src/libfetchers/attrs.cc +++ b/src/libfetchers/attrs.cc @@ -33,7 +33,7 @@ nlohmann::json attrsToJSON(const Attrs & attrs) json[attr.first] = *v; } else if (auto v = std::get_if>(&attr.second)) { json[attr.first] = v->t; - } else abort(); + } else unreachable(); } return json; } @@ -99,7 +99,7 @@ std::map attrsToQuery(const Attrs & attrs) query.insert_or_assign(attr.first, *v); } else if (auto v = std::get_if>(&attr.second)) { query.insert_or_assign(attr.first, v->t ? "1" : "0"); - } else abort(); + } else unreachable(); } return query; } diff --git a/src/libmain/loggers.cc b/src/libmain/loggers.cc index 9829859de..a4e0530c8 100644 --- a/src/libmain/loggers.cc +++ b/src/libmain/loggers.cc @@ -36,7 +36,7 @@ Logger * makeDefaultLogger() { return logger; } default: - abort(); + unreachable(); } } diff --git a/src/libstore/build/drv-output-substitution-goal.hh b/src/libstore/build/drv-output-substitution-goal.hh index 807054926..8c60d0198 100644 --- a/src/libstore/build/drv-output-substitution-goal.hh +++ b/src/libstore/build/drv-output-substitution-goal.hh @@ -36,7 +36,7 @@ public: Co init() override; Co realisationFetched(std::shared_ptr outputInfo, nix::ref sub); - void timedOut(Error && ex) override { abort(); }; + void timedOut(Error && ex) override { unreachable(); }; std::string key() override; diff --git a/src/libstore/build/goal.hh b/src/libstore/build/goal.hh index 162c392d0..9c6a40c84 100644 --- a/src/libstore/build/goal.hh +++ b/src/libstore/build/goal.hh @@ -400,12 +400,12 @@ public: virtual void handleChildOutput(Descriptor fd, std::string_view data) { - abort(); + unreachable(); } virtual void handleEOF(Descriptor fd) { - abort(); + unreachable(); } void trace(std::string_view s); diff --git a/src/libstore/build/substitution-goal.hh b/src/libstore/build/substitution-goal.hh index 86e4f5423..c1de45379 100644 --- a/src/libstore/build/substitution-goal.hh +++ b/src/libstore/build/substitution-goal.hh @@ -50,7 +50,7 @@ public: PathSubstitutionGoal(const StorePath & storePath, Worker & worker, RepairFlag repair = NoRepair, std::optional ca = std::nullopt); ~PathSubstitutionGoal(); - void timedOut(Error && ex) override { abort(); }; + void timedOut(Error && ex) override { unreachable(); }; /** * We prepend "a$" to the key name to ensure substitution goals diff --git a/src/libstore/build/worker.cc b/src/libstore/build/worker.cc index 7fc41b121..ab0ba67b5 100644 --- a/src/libstore/build/worker.cc +++ b/src/libstore/build/worker.cc @@ -216,7 +216,7 @@ void Worker::childStarted(GoalPtr goal, const std::set std::string BaseSetting::to_string() const if (value == smEnabled) return "true"; else if (value == smRelaxed) return "relaxed"; else if (value == smDisabled) return "false"; - else abort(); + else unreachable(); } template<> void BaseSetting::convertToArg(Args & args, const std::string & category) diff --git a/src/libstore/nar-info-disk-cache.cc b/src/libstore/nar-info-disk-cache.cc index 288f618d5..83e63794e 100644 --- a/src/libstore/nar-info-disk-cache.cc +++ b/src/libstore/nar-info-disk-cache.cc @@ -164,7 +164,7 @@ public: Cache & getCache(State & state, const std::string & uri) { auto i = state.caches.find(uri); - if (i == state.caches.end()) abort(); + if (i == state.caches.end()) unreachable(); return i->second; } @@ -211,7 +211,7 @@ public: { auto r(state->insertCache.use()(uri)(time(0))(storeDir)(wantMassQuery)(priority)); - if (!r.next()) { abort(); } + if (!r.next()) { unreachable(); } ret.id = (int) r.getInt(0); } diff --git a/src/libstore/unix/build/local-derivation-goal.cc b/src/libstore/unix/build/local-derivation-goal.cc index 0dd102200..ac5b4dd0b 100644 --- a/src/libstore/unix/build/local-derivation-goal.cc +++ b/src/libstore/unix/build/local-derivation-goal.cc @@ -165,7 +165,7 @@ void LocalDerivationGoal::killSandbox(bool getStats) buildResult.cpuSystem = stats.cpuSystem; } #else - abort(); + unreachable(); #endif } diff --git a/src/libstore/unix/pathlocks.cc b/src/libstore/unix/pathlocks.cc index af21319a7..1ec4579ec 100644 --- a/src/libstore/unix/pathlocks.cc +++ b/src/libstore/unix/pathlocks.cc @@ -45,7 +45,7 @@ bool lockFile(Descriptor desc, LockType lockType, bool wait) if (lockType == ltRead) type = LOCK_SH; else if (lockType == ltWrite) type = LOCK_EX; else if (lockType == ltNone) type = LOCK_UN; - else abort(); + else unreachable(); if (wait) { while (flock(desc, type) != 0) { diff --git a/src/libutil/chunked-vector.hh b/src/libutil/chunked-vector.hh index d914e2542..4709679a6 100644 --- a/src/libutil/chunked-vector.hh +++ b/src/libutil/chunked-vector.hh @@ -6,6 +6,8 @@ #include #include +#include "error.hh" + namespace nix { /** @@ -30,7 +32,7 @@ private: auto & addChunk() { if (size_ >= std::numeric_limits::max() - ChunkSize) - abort(); + unreachable(); chunks.emplace_back(); chunks.back().reserve(ChunkSize); return chunks.back(); diff --git a/src/libutil/error.cc b/src/libutil/error.cc index 33c391963..b1858911a 100644 --- a/src/libutil/error.cc +++ b/src/libutil/error.cc @@ -430,4 +430,16 @@ std::ostream & showErrorInfo(std::ostream & out, const ErrorInfo & einfo, bool s return out; } +void panic(std::string_view msg) +{ + printError(msg); + printError("This was a fatal error, aborting."); + abort(); +} + +void panic(const char * file, int line, const char * func) +{ + panic(std::string("Unexpected condition in ") + func + " at " + file + ":" + std::to_string(line)); +} + } diff --git a/src/libutil/error.hh b/src/libutil/error.hh index d7fe902d6..572a1baf7 100644 --- a/src/libutil/error.hh +++ b/src/libutil/error.hh @@ -273,4 +273,28 @@ using NativeSysError = */ void throwExceptionSelfCheck(); +/** + * Print a message and abort(). + * + * @note: This assumes that the logger is operational + */ +[[noreturn]] +void panic(std::string_view msg); + +/** + * Print a basic error message with source position and abort(). + * Use the unreachable macro to call this. + * + * @note: This assumes that the logger is operational + */ +[[noreturn]] +void panic(const char * file, int line, const char * func); + +/** + * Print a basic error message with source position and abort(). + * + * @note: This assumes that the logger is operational + */ +#define unreachable() (panic(__FILE__, __LINE__, __func__)) + } diff --git a/src/libutil/file-content-address.cc b/src/libutil/file-content-address.cc index 438dac7da..86378dd67 100644 --- a/src/libutil/file-content-address.cc +++ b/src/libutil/file-content-address.cc @@ -63,7 +63,7 @@ std::string_view renderFileIngestionMethod(FileIngestionMethod method) case FileIngestionMethod::Git: return "git"; default: - abort(); + unreachable(); } } diff --git a/src/libutil/fs-sink.cc b/src/libutil/fs-sink.cc index 3246e0902..f15324d0a 100644 --- a/src/libutil/fs-sink.cc +++ b/src/libutil/fs-sink.cc @@ -53,7 +53,7 @@ void copyRecursive( throw Error("file '%1%' has an unsupported type", from); default: - abort(); + unreachable(); } } diff --git a/src/libutil/git.cc b/src/libutil/git.cc index a6968a43e..af91fa643 100644 --- a/src/libutil/git.cc +++ b/src/libutil/git.cc @@ -201,7 +201,7 @@ std::optional convertMode(SourceAccessor::Type type) case SourceAccessor::tRegular: return Mode::Regular; case SourceAccessor::tDirectory: return Mode::Directory; case SourceAccessor::tMisc: return std::nullopt; - default: abort(); + default: unreachable(); } } diff --git a/src/libutil/hash.cc b/src/libutil/hash.cc index 35b913e42..ab2a8695d 100644 --- a/src/libutil/hash.cc +++ b/src/libutil/hash.cc @@ -25,7 +25,7 @@ static size_t regularHashSize(HashAlgorithm type) { case HashAlgorithm::SHA256: return sha256HashSize; case HashAlgorithm::SHA512: return sha512HashSize; } - abort(); + unreachable(); } diff --git a/src/libutil/logging.cc b/src/libutil/logging.cc index 55751b4cf..29427f2f6 100644 --- a/src/libutil/logging.cc +++ b/src/libutil/logging.cc @@ -189,7 +189,7 @@ struct JSONLogger : Logger { else if (f.type == Logger::Field::tString) arr.push_back(f.s); else - abort(); + unreachable(); } void write(const nlohmann::json & json) diff --git a/src/libutil/serialise.cc b/src/libutil/serialise.cc index 36b99905a..ee4351545 100644 --- a/src/libutil/serialise.cc +++ b/src/libutil/serialise.cc @@ -260,7 +260,7 @@ std::unique_ptr sourceToSink(std::function fun) }); } - if (!*coro) { abort(); } + if (!*coro) { unreachable(); } if (!cur.empty()) { CoroutineContext ctx; @@ -271,12 +271,12 @@ std::unique_ptr sourceToSink(std::function fun) void finish() override { if (!coro) return; - if (!*coro) abort(); + if (!*coro) unreachable(); { CoroutineContext ctx; (*coro)(true); } - if (*coro) abort(); + if (*coro) unreachable(); } }; @@ -316,7 +316,7 @@ std::unique_ptr sinkToSource( }); } - if (!*coro) { eof(); abort(); } + if (!*coro) { eof(); unreachable(); } if (pos == cur.size()) { if (!cur.empty()) { diff --git a/src/libutil/sync.hh b/src/libutil/sync.hh index 20dd6ee52..c1b699ffc 100644 --- a/src/libutil/sync.hh +++ b/src/libutil/sync.hh @@ -7,6 +7,8 @@ #include #include +#include "error.hh" + namespace nix { /** @@ -47,7 +49,7 @@ public: friend SyncBase; Lock(SyncBase * s) : s(s), lk(s->mutex) { } public: - Lock(Lock && l) : s(l.s) { abort(); } + Lock(Lock && l) : s(l.s) { unreachable(); } Lock(const Lock & l) = delete; ~Lock() { } diff --git a/src/libutil/unix/monitor-fd.hh b/src/libutil/unix/monitor-fd.hh index 103894de9..b6610feff 100644 --- a/src/libutil/unix/monitor-fd.hh +++ b/src/libutil/unix/monitor-fd.hh @@ -40,7 +40,9 @@ public: #endif ; auto count = poll(fds, 1, -1); - if (count == -1) abort(); // can't happen + if (count == -1) + unreachable(); + /* This shouldn't happen, but can on macOS due to a bug. See rdar://37550628. diff --git a/src/libutil/unix/processes.cc b/src/libutil/unix/processes.cc index 1af559a21..c5ce74acc 100644 --- a/src/libutil/unix/processes.cc +++ b/src/libutil/unix/processes.cc @@ -182,7 +182,7 @@ static pid_t doFork(bool allowVfork, ChildWrapperFunction & fun) #endif if (pid != 0) return pid; fun(); - abort(); + unreachable(); } diff --git a/src/nix-env/nix-env.cc b/src/nix-env/nix-env.cc index 5e170c99d..40c200542 100644 --- a/src/nix-env/nix-env.cc +++ b/src/nix-env/nix-env.cc @@ -1159,7 +1159,7 @@ static void opQuery(Globals & globals, Strings opFlags, Strings opArgs) case cvEqual: ch = '='; break; case cvGreater: ch = '<'; break; case cvUnavail: ch = '-'; break; - default: abort(); + default: unreachable(); } if (xmlOutput) { diff --git a/src/nix-store/nix-store.cc b/src/nix-store/nix-store.cc index f073074e8..b4de42ba1 100644 --- a/src/nix-store/nix-store.cc +++ b/src/nix-store/nix-store.cc @@ -480,7 +480,7 @@ static void opQuery(Strings opFlags, Strings opArgs) } default: - abort(); + unreachable(); } }