Constructing a protocol_view from a valueless protocol is undiagnosed, and calls through it hit a null function pointer
- Dominant language
- C++
- Stars
- 11
- Forks
- 4
- Avg merge
- 16h 39m
- Merged PRs (30d)
- 131
Description
A moved-from `protocol` has `object_ == nullptr` and `vtable_ == &null_vtable`,
whose member entries are null function pointers. Calling a member function on
it directly is caught in debug builds: `method_thunk::operator()` asserts
`!valueless_after_move()` before dispatching.
Since #244 and #272 a `protocol_view` or `protocol_view` constructs
implicitly from a `protocol&`, copying `object_` and `vtable_`. Nothing
checks the source, so a valueless protocol converts silently, and the thunk
assert is `if constexpr (is_protocol_v)`, so it is compiled out
for views. A member call through such a view calls a null function pointer with
no diagnostic, even in a debug build.
```c++
protocol a{Circle{}};
protocol b = std::move(a); // a is valueless
protocol_view v = a; // accepted; comment says "Precondition: p is not valueless"
v.area(); // release: null call; debug: also null call
```
The from-protocol constructors document the precondition in a comment
(`protocol.hh`, both `protocol_view` specializations) but do not enforce it.
Suggested fix, small enough for one PR:
- `assert(!p.valueless_after_move())` in the two from-protocol constructors.
- Drop the `is_protocol_v` gate in `method_thunk` and assert `object_ != nullptr`
instead, which covers protocol and view alike. A view's `object_` is otherwise
never null.
- A death test for each path.
#208 settled the direct-call case as a precondition violation diagnosed by an
assert in debug builds; this extends the same contract to views. Raised in the
review of #272, where it was agreed to be out of scope there but worth fixing.
Contributor guide
Assessment
This issue has not been assessed yet.