Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions nix-meson-build-support/common/clang-tidy/.clang-tidy
Original file line number Diff line number Diff line change
Expand Up @@ -55,10 +55,6 @@ Checks:
- -bugprone-nondeterministic-pointer-iteration-order
# 9 warnings - intentional std::bit_cast/memcpy on Value* arrays (evaluator hot path)
- -bugprone-bitwise-pointer-cast
# 1 warning (header) - value.hh mkFailed: GC alloc in noexcept. Boehm's
# gc_cleanup::operator new isn't marked noexcept but aborts on OOM rather
# than throws; std::terminate here is the intended behavior anyway.
- -bugprone-unhandled-exception-at-new
# 1 warning (header) - fmt.hh Magenta<T>::operator<<: generic colorizer
# template; fires when T=unsigned char but that instantiation is correct.
- -bugprone-unintended-char-ostream-output
Expand Down
37 changes: 36 additions & 1 deletion src/libexpr/include/nix/expr/value.hh
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
#include <bit>
#include <cassert>
#include <cstddef>
#include <cstdio>
#include <cstdlib>
#include <cstring>
#include <memory>
#include <memory_resource>
Expand Down Expand Up @@ -1427,7 +1429,40 @@ public:

inline void mkFailed(std::exception_ptr e, Value * recovery) noexcept
{
setStorage(new Value::Failed(e, recovery));
try {
setStorage(new Value::Failed(e, recovery));
} catch (...) {
/* Most likely a std::bad_alloc from the GC heap.
This is called while handling an evaluation error,
and recording that failure is itself what just failed,
so there is no way to recover.
Report and bail out, using only static strings to avoid touching the heap again. */
std::fputs("error: failed to record an evaluation error", stderr);
try {
throw;
} catch (const std::exception & allocEx) {
std::fputs(": ", stderr);
std::fputs(allocEx.what(), stderr);
} catch (...) {
}
std::fputc('\n', stderr);

/* Best effort: show the evaluation error that was being recorded.
Formatting it (e.g. BaseError::what) may itself allocate and terminate on failure,
hence printing the message above first. */
try {
std::rethrow_exception(e);
} catch (const std::exception & evalEx) {
std::fputs("the evaluation error was: ", stderr);
std::fputs(evalEx.what(), stderr);
std::fputc('\n', stderr);
} catch (...) {
std::fputs("the evaluation error was of an unknown type\n", stderr);
}

// Abort not exception but function.
std::abort();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seeems... very excessively bad. Can't we just call nix::panic? It doesn't try to allocate memory do it should be safe from the perspective.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm newbie in nix repo.
I don't know nix:panic.
I will research it.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I use nix:panic.

}
}

bool isList() const noexcept
Expand Down
Loading