`<utility>`: `unreachable` does not abort in debug builds

[K-ballo]

Describe the bug

Calling std::unreachable is not diagnosed in debug builds.

Expected behavior

In debug builds, I would expect calling std::unreachable to abort execution.

STL version

e2115ba

Additional context

This was discovered after std::terminate failed to capture undefined behavior due to a logic error. Aborting execution is compatible with undefined behavior, and it helps catch bugs. In that regard, the old assert(false)-based implementations are preferable to std::unreachable().

The original implementation (#2526) attempted to call abort in debug builds:

STL/stl/inc/utility

Line 757 in b6b9778

_CSTD abort(); // likely to be called in debug mode, but can't be relied upon - already entered the UB territory

This was later removed in (#5560), with no obvious explanation for the rationale.

e2115ba#diff-b40b803e74da64226183307c996e495cce0bf23a9c83d9822fea849bb99b7144L962

[AlexGuteniev]

The explanation is in PR description:

🚨 Drops abort() from <utility> without any replacement🚨! Core headers should not call any functions.

I'm not sure how much we should pursue the Core Headers idea though.

Maybe we should add that abort() back.

[K-ballo]

Core headers should not call any functions.

why?

[AlexGuteniev]

Core headers are supposed to have "no dependencies". I'd defer further comments to maintainers.

[AlexGuteniev]

FWIW. I initially implemented with just abort() and no UB in debug, but was persuaded to change that:
#2526 (comment)

I still think that maybe we should not have UB in debug.

[StephanTLavavej]

We still don't really have a clear idea if/how core headers are useful (beyond keeping the really wacky stuff out of the import lib). However, with the introduction of fastfail-as-doom, I could be persuaded to make unreachable() call the doom function. By default it would fastfail/builtin_verbose_trap, which are intrinsics to emit lethal instruction sequences and don't actually call any functions.

[AlexGuteniev]

However, with the introduction of fastfail-as-doom, I could be persuaded to make unreachable() call the doom function

There's a layering problem. Doom function is not a core headers mechanism.

[AlexGuteniev]

Can we demote <utility> to non-core to get this done?

[StephanTLavavej]

We've never taken a header out of the core set before, to my knowledge.

[AlexGuteniev]

Time to do this for the first time then, I guess. The standard asks for non-core feature in a header that previously was thought as core. It happened, and may happen again.

I really don't see a better fix for this bug.

[AlexGuteniev]

Other solutions that seem worse are:

• Restoring abort()
• Upgrading the doom function to core
• wontfix this

[frederick-vs-ja]

<utility> is so fundamental that I don't think it's worthwhile making it non-core.

Note that <cstdlib> is also a core header, and we're actually calling functions provided by UCRT in it, so I guess it's possibly fine to call abort in <utility>.

STL/stl/inc/cstdlib

Lines 23 to 33 in 26b7ca5

	_NODISCARD _Check_return_ inline double abs(_In_ double _Xx) noexcept /* strengthened */ {
	return _CSTD fabs(_Xx);
	}
	_NODISCARD _Check_return_ inline float abs(_In_ float _Xx) noexcept /* strengthened */ {
	return _CSTD fabsf(_Xx);
	}
	_NODISCARD _Check_return_ inline long double abs(_In_ long double _Xx) noexcept /* strengthened */ {
	return _CSTD fabsl(_Xx);
	}