Re: regressions on HEAD

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

 



On 2 March 2018 at 11:03, Christopher Li <sparse@xxxxxxxxxxx> wrote:
> On Thu, Mar 1, 2018 at 1:52 PM, Dibyendu Majumdar
> <mobile@xxxxxxxxxxxxxxx> wrote:
>> On 1 March 2018 at 20:57, Dibyendu Majumdar <mobile@xxxxxxxxxxxxxxx> wrote:
>>> On 27 February 2018 at 01:12, Luc Van Oostenryck
>>> <luc.vanoostenryck@xxxxxxxxx> wrote:
>>>> On Mon, Feb 26, 2018 at 03:36:38PM -0800, Christopher Li wrote:
>>>>>
>>>>> You said you did not found crash in your first round testing.
>>>>> However there is some regression of the IR. Can you share
>>>>> the regression test case you found?
>>>>
>>>> I didn't spend much time at it but a typical small example
>>>> of what I've seen is:
>>>>         void a(void) {
>>>>           long b;
>>>>           unsigned c = 0;
>>>>           for (;;)
>>>>             if (c)
>>>>               c = b;
>>>>         }
>>>>
>>>
>>> Now where is the value pseudo of size 64 coming from as it is not in
>>> the code above?
>>>
>>
>> Okay it seems to come from flow.c in find_dominating_stores() - in the
>> call to convert_load_instruction(). Any explanation of what this is
>> doing?
>
> That is just the memory operation convert to pseudo.
> Sparse find out "b" has no dominating store (uninitialized), so
> it just replace it with 0 value.
>

Does that make sense? I guess that if a var is uninitialized then the
compiler is free to do what it likes, but in the previous version
without size, this pseudo value will be considered 'equivalent' to any
other pseudo value of 0 right? Is that correct?

> The 64 bit one is b, the long type.
> The 32 bit one is c, the int type.
>
> I already find out the place cause it:
> The fix is here:

Okay cool. However I was looking at a recent change that was done
(last year) I think - to use a store to initialize an aggregate using
value_pseudo() - see linearize_one_symbol() in linearize.c. Does that
mean in the pre-sized code, such values would be considered same as
everything else?

In my view the aggregate initialization should be done using a
dedicated OP code and not using store.

But overall, I feel that we don't really care about sizes that are not
'register' size - do we? In reality we probably only care about 8, 16,
32, 64 bits. So one option is to only create distinct pesudo values
when the size is one of these - otherwise create pseudo values of 0
size.

However, that still leaves a question in my mind:

a) Is it okay to assign a value 0 to uninitialized vars?
b) If size is not present or is 0 - does that mean that the CSE logic
will consider all these equivalent? That doesn't sound right to me.

I don't really understand how the CSE handles these but maybe - if you
agree with above then CSE should ignore any pseudo value that is size
0?

Thanks and Regards
Dibyendu
--
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