Re: [PATCH 1/2] V2 Give the constant pseudo value a size

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 3 March 2018 at 21:42, Dibyendu Majumdar <mobile@xxxxxxxxxxxxxxx> wrote:
> On 3 March 2018 at 21:26, Dibyendu Majumdar <mobile@xxxxxxxxxxxxxxx> wrote:
>> On 13 November 2017 at 15:28, Christopher Li <sparse@xxxxxxxxxxx> wrote:
>>> Currently value pseudo does not have size.
>>> This create a problem pointed out by Dibyendu.
>>> When using LLVM, calling varidic function with
>>> constant value, there is no where to find the
>>> size.
>>>
>>> Linus give out two suggestions. One is give pseudo
>>> a size. The other one is the push instruction.
>>> This is the implementation of the first suggestion.
>>>
>>> The model is actual very simple. The pseudo is
>>> exactly as before if you are not looking at the size.
>>> There is a size at create time, which tag alone with
>>> it.
>>>
>>
>> Chris, I think the discussion of this topic should be moved to this thread.
>>
>> I found a problem with this as I mentioned in the other thread. Last
>> year a change was introduced to initialize aggregate structures in
>> Sparse using a store instruction that uses a PSEUDO_VAL. However when
>> this creates a pseudo the size is set to the size of the aggregate
>> types - and this size (> 64 bits) is kind of nonsensical for a
>> PSEUDO_VAL.
>>
>> I suggested that when creating a PSEUDO_VAL we should only create
>> distinct values when size is register size - i.e. one of 8, 16, 32, or
>> 64. Else we should set size to 0.
>>
>> Couple of more points:
>>
>> a) We should also set size to 0 when we create a PSEUDO_VAL in CSE for
>> uninitialized vars.
>> b) We should ideally avoid CSE on PSEUDO_VAL with size = 0 because I
>> don't think it makes sense to do so - i.e. they either represent
>> uninitialized vars or aggregates > 64 bits.
>>
>> The above aggregate initialization change is one of the changes I did
>> not merge into my project as I wasn't happy with it. I still think
>> this change is not correct and needs to be removed or redone. I am not
>> 100% sure but presumably creating PSEUDO_VAL where the instruction
>> size is > 64-bits means that these PSEUDO_VAL values might be
>> considered 'same as' any other PSEUDO_VAL with value 0.
>>
>
> Forgot to add a simple example from my test pack:
>
> struct foo {
>     long long int i,j;
> };
>
> extern void dosomething(struct foo *foo);
>
> int main(void)
> {
>     struct foo bar = { 99 };
>     dosomething(&bar);
>     return 0;
> }
>
> Here you will see a PESUDO_VAL with value 0 being created with size 16 bytes.
>

In case you are wondering where this is happening - look at:

static pseudo_t linearize_one_symbol(struct entrypoint *ep, struct symbol *sym)
{
struct access_data ad = { NULL, };
pseudo_t value;

if (!sym || !sym->initializer || sym->initialized)
return VOID;

/* We need to output these puppies some day too.. */
if (sym->ctype.modifiers & (MOD_STATIC | MOD_TOPLEVEL))
return VOID;

sym->initialized = 1;
ad.address = symbol_pseudo(ep, sym);

if (sym->initializer && !is_scalar_type(sym)) {
// default zero initialization [6.7.9.21]
// FIXME: this init the whole aggregate while
// only the existing fields need to be initialized.
// FIXME: this init the whole aggregate even if
// all fields arelater  explicitely initialized.
struct expression *expr = sym->initializer;
ad.pos = expr->pos;
ad.result_type = sym;
ad.source_type = base_type(sym);
ad.address = symbol_pseudo(ep, sym);
linearize_store_gen(ep, value_pseudo(sym, 0), &ad);
}

value = linearize_initializer(ep, sym->initializer, &ad);
finish_address_gen(ep, &ad);
return value;
}
--
To unsubscribe from this list: send the line "unsubscribe linux-sparse" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Newbies FAQ]     [LKML]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Trinity Fuzzer Tool]

  Powered by Linux