Re: lparser.c:singlevar() asserts potentially uninitialised value.

[Duane Leslie <parakleta@...>]

On 4 May 2016, at 22:58, Roberto Ierusalimschy <roberto@inf.puc-rio.br> wrote:

>> In lparser.c function `singlevar()` it checks that the `ls->envn`
>> named variable (i.e. _ENV) is initialised to a VLOCAL or VUPVAL, but
>> the `singlevaraux()` function may return VVOID without ever
>> initialising the `var->k` value.  The assert should be instead that
>> `singlevaraux()` doesn't return VVOID.
> 
> We cannot put the call inside the assert, so we need to assign
> the return to a variable and assert on that variable. But without
> assertions, we get a "unused variable" warning (and a weird code).  Then
> we add a "(void)var" to avoid the warning, and get weirder code.  All in
> all, the current assertion documents what we want and keeps the regular
> code simple. (After all, if the assertion holds, its code is correct ;-)

You're claiming the converse of the assertion, not the contrapositive.  The actual statement should be that if the code is correct the assertion holds.   The problem is that assertions only state that if the assertion fails the code is invalid, they cannot assert correctness, especially in the case of undefined behaviour (in this case an uninitialised variable)

Avoiding a correct assertion for the sake of also avoiding "weird" code is a mistake in my opinion.  What is wrong a construction like:

```c
if (VVOID == singlevaraux()) {
 lua_assert(0 && "Error Message");
}
```

The problem is that your current assertion is misleading in what it documents because it implies that the `var->k` value is valid after the call to `singlevaraux`.

The assertion is particularly dangerous since it is actually quite likely to pass invalid code given the uninitialised value you are testing is on the stack.  If the code leads to the same call stack (highly likely) then you will exactly map your uninitialised value over the valid value used in a previous call stack.

Regards,

Duane