Re: lua-5.1 restorestack memory handling bug

[Marko Lindqvist <cazfi74@...>]

On 7 February 2012 11:56, Marko Lindqvist <cazfi74@gmail.com> wrote:
> On 7 February 2012 10:40, Martin Guy <martinwguy@gmail.com> wrote:
>> On 7 February 2012 08:57, Marko Lindqvist <cazfi74@gmail.com> wrote:
>>> ../../../../../src.patched/dependencies/lua-5.1/src/ldo.h:25:28: note:
>>> instantiated from:
>>> #define restorestack(L,n) ((TValue )((char )L->stack + (n)))
>>> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
>> -#define restorestack(L,n)      ((TValue *)((char *)L->stack + (n)))
>
>  I was about to say that you're missing the important part in the
> error message and write code to be replaced in your patch in a way
> that has no bug. Important thing is that compiler error message has
> simple "char" as type to cast, you have pointer "char *". But now that
> I look the original code, it's poitner there. So how does the compiler
> decide that it's not a pointer!?!

 Well, retrying compile produces saner error messages. I earlier
copied error message from gna bugtracker, and
not-so-nicely-for-C-code-with-pointers it shows "*text*" as bold
"text".

 So, yeah, if you know for sure that the pointed value is aligned as
TValue (8 byte alignment in this case) even though it's temporarily
pointed to as char (which seems like it could be in any 1 byte
alignment), there's no runtime bug here.

 But since they must be TValues, wouldn't it be safer to handle them
as ones (just as your patch does) instead of temporarily referring to
them as char arrays? That would keep you safe from accidentally
introducing "they are not really TValues any more" bugs in the future.
Is there some reason they are referred to as char arrays at the
moment?


- ML