Indexing into a vector past its end is UB.

[graydon]

Even if all you're doing with the resulting reference is deriving a pointer to one-past-the-end.

Or it might be UB. Unclear.

At least: according to cplusplus.com calling operator[] on a vector with an out-of-range argument is UB.

UBSan does not report anything, however. Or rather: it reports UB if you form a reference to v[v.size()] for an empty vector v, but not for a nonempty vector v. Go figure.

Building with -stdlib=libc++ and -D_LIBCPP_DEBUG=1 produces a runtime error _LIBCPP_ASSERT '__n < size()' failed. vector[] index out of bounds for empty or nonempty vectors.

In the C++ spec section 1.3.2 [dcl.ref] it says that "A reference shall be initialized to refer to a valid object". Furthermore table 88 defines a[n] as meaning *(a.begin() + n) on vector, and 26.3.11.4 [vector.data] says that only the range [data(), data() + size()) is a valid range. But none of this explicitly says it's UB to initialize a reference to the one-past-the-end position. Just that it's not included in the ways that a reference "shall" be initialized.

So .. it is not clear to me by reading the spec whether it is UB merely to have called the operator (and thereby formed a reference to an element one-past-the-end) in order to then derive a pointer to that referenced element, but not dereferenced it. However, one can form the pointer in question without having to answer this question using the data() + size() idiom used elsewhere in this code, so that's what this PR does.

[codecov]

Codecov Report

Merging #813 (e41b18f) into master (9d9ed3b) will decrease coverage by 0.07%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #813      +/-   ##
==========================================
- Coverage   99.00%   98.92%   -0.08%     
==========================================
  Files          81       80       -1     
  Lines       12845    11834    -1011     
==========================================
- Hits        12717    11707    -1010     
+ Misses        128      127       -1     

Flag	Coverage Δ
rust	?
spectests	89.96% <100.00%> (ø)
unittests	98.92% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
lib/fizzy/execute.cpp	99.17% <100.00%> (ø)
bindings/rust/src/lib.rs

[axic]

Thank you @graydon, we'll try to get this merged soon.

[gumb0]

Nice investigation, thank you.

I have only one small cosemtic change request.

[gumb0]

Nit:

Suggested change

	assert(pc == code.instructions.data() + code.instructions.size()); // End of code must be
	// reached.
	// End of code must be reached.
	assert(pc == code.instructions.data() + code.instructions.size());