<optional>: Should optional::operator* trigger a debug assert if there is no value?

[matthew-dews]

Looking at the implementations of operator* in std::optional I wondered if it would be helpful if they had debug asserts which check that has_value() is true to catch optionals being dereferenced when they shouldn't be. Would this make any sense to add or are there reasons not to have something like that?

[CaseyCarter]

This sounds perfectly reasonable to me. We should change operator-> similarly. @BillyONeal, would it make sense to guard these with _CONTAINER_DEBUG_LEVEL?

[BillyONeal]

Hmmm, that's a good question. I've been trying to draw a strong distinction between settings that affect the containers and settings that affect debug asserts in general, particularly since we did the effort to make iterator asserts work nicely in release builds (e.g. NDEBUG _ITERATOR_DEBUG_LEVEL=2 should work and do something reasonable). However, the majority of the motivation for that distinction is that _ITERATOR_DEBUG_LEVEL affects layout, and _CONTAINER_DEBUG_LEVEL does not (the intent being that we expect folks who value security over perf to increase _CONTAINER_DEBUG_LEVEL in release builds).

Is this has_value check something you might expect those security folks to care about? If so, do we want to consider changing the name _CONTAINER_DEBUG_LEVEL since that's new-ish and we can probably get away with doing so?

[matthew-dews]

It looks like the default constructor of optional does not initialize the value, so dereferencing an empty optional results in reading uninitialized memory, which seems like a security concern.

[StephanTLavavej]

Note that if you want to change the name of a macro, you can still support the old name for backwards compatibility; we did this when unifying _SECURE_SCL and _HAS_ITERATOR_DEBUGGING into _ITERATOR_DEBUG_LEVEL and that was much more complicated.

Perhaps _STL_SECURITY_LEVEL would be a better name for an STL-wide feature that enables "cheap" security checks.

[barcharcraz]

I think it's probably fine to put this one in particular under _CONTAINER_DEBUG_LEVEL. Optional is sorta a container :D.

[panikc]

Has this been fixed with #1362?

[sylveon]

Seems like it

[CaseyCarter]

Fixed by #1362.

(Good catch, @panikc - I didn't recall the duplicate.)