[DTrace-devel] [PATCH 05/14] alloca: new faults

[Nick Alcock]

On 7 Mar 2022, Kris Van Hees said:

> On Wed, Mar 02, 2022 at 01:44:58PM +0000, Nick Alcock via DTrace-devel wrote:
>> We'll use DTRACEFLT_BADSIZE to distinguish in error messages between
>> out-of-bounds errors in bcopy and the like, and size-too-large errors.
>> (Without this, it's hard to tell if a, say, 0x12 in the fault value
>> is a nearly-NULL pointer or a wrong size.)
>
> Makes sense.
>
>> We'll use DTRACEFLT_INTERR to indicate can-never-happen errors where we
>> have tested the actual condition already (and emitted a proper error)
>> followed by a check to placate the verifier (which will always succeed
>> if the actual condition check failed).  The latter failure happening at
>> runtime always indicates a bug in DTrace, not in the user's code, so
>> indicate as much.
>
> Here I am confused.  If this is a fault that can never happen, why do we
> even generate code to report such a non-existant fault?  Based on your

Because without it, the verifier thinks control does not terminate in a
condition when it in fact always will (because of a conditional the
verifier does not take into consideration) and we get a spurious
verifier failure.

The idea is that we always test a bound that the verifier cannot follow
(e.g. offset + read size does not run off the end of the region, where
the read size is in a reg, so the verifier ignores it) before testing
one that satisfies the verifier (that offset + statically determined
maximum possible read size -- SIZEMAX -- does not run off the end of the
region). Of course then I went and did the tests in the opposite order,
making INTERR possible to trigger. I'll fix that!

> description this would be used in cases where the condition has already
> been tested and a proper error has emitted.  Since a probe error results
> in an immediate return from the clause function (after reporting the
> fault by means of the ERROR probe clauses), the following code will never
> execute.
>
> So, I really do not see the need for this new fault that can never happen.

It won't -- but the verifier will run through it because it thinks it
might execute, and we need at the very least to do a goto exit. The
INTERR is to make sure that if it ever *does* execute, we know, because
it's a bug.