Re: [Tinycc-devel] Zeroing stack variables CValue

[Michael Matz]

Hi,

On Wed, 26 Mar 2014, Domingo Alvarez Duarte wrote:

> On my commit

It would be easier if you wrote your reasons for doing things in mails, 
not only in commit messages, it makes quoting much harder.  Anyway, in the 
commit message you wrote:

>    I found the problem it was because CValue stack variables have rubish 
> as it inital values
>     and assigning to a member that is smaller than the big union item 
> and trying to
>     recover it later as a different member gives bak garbage.
>
>     ST_FUNC void vset(TCCState* tcc_state, CType *type, int r, int v)
>     {
>         CValue cval;
>         memset(&cval, 0, sizeof(CValue));
>
>         cval.i = v; //,<<<<<<<<<<< here is the main bug that mix with garbage
>         vsetc(tcc_state, type, r, &cval);
>     }
...

vset only initializes the .i member, that's an invariant.  If you want to 
read out something else later you have to use other functions, or set 
vtop->correctmember after vset yourself ...

>     static void init_putv(TCCState* tcc_state, CType *type, Section *sec, 
> unsigned long c,
>                           int v, int expr_type)
>     {
>     ...
>             case VT_PTR:
>                 if (tcc_state->tccgen_vtop->r & VT_SYM) {
>                     greloc(tcc_state, sec, tcc_state->tccgen_vtop->sym, c, 
> R_DATA_PTR);
>                 }
>
>     //<<< on the next line is where we try to get the assigned value to 
> cvalue.i as cvalue.ull
>
>                 *(addr_t *)ptr |= (tcc_state->tccgen_vtop->c.ull & bit_mask) << 
> bit_pos;

... like here.  You either need to find out which vset() it was that 
created the vtop used above and fix that (to initialize ->c.ull), or 
access only ->c.i here and extend it to unsigned long long yourself, 
depending on what's the right thing.  (That will generally require looking 
at the type of vtop and access the right member according to that).

The thing you did (simply zeroing all local CValues) doesn't fix the bug 
(it papers over it, and on big-endian platforms doesn't even do that), and 
generates needless code.  Please revert and fix it correctly.  Or post a 
testcase here that breaks without that zeroing, in which case I'll promise 
to take a look.


Ciao,
Michael.