Re: regressions on HEAD

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

 



On Fri, Mar 02, 2018 at 09:06:21PM +0000, Dibyendu Majumdar wrote:
> On 2 March 2018 at 20:48, Luc Van Oostenryck
> <luc.vanoostenryck@xxxxxxxxx> wrote:
> > On Fri, Mar 02, 2018 at 06:53:49PM +0000, Dibyendu Majumdar wrote:
> >> On 2 March 2018 at 11:03, Christopher Li <sparse@xxxxxxxxxxx> wrote:
> >> >
> >> > 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,
> >
> > It make sense but only as far as you don't forget what you're doing.
> > The logic of this reasoning is:
> > - accessing an uninitialized var is undefined behaviour (UB)
> > - by definition after UB anything can happen
> > - since anything can happen the compiler can choose (to do) anything
> > - choosing 0 is as good as anything alse
> >
> > A more correct treatment is:
> > - use a specific value (UNDEF), unique for each var
> > - propagate this value as far as possible
> > - be very carefull with any simplification involving UNDEF
> > - at the end, issues an appropriate warning and/or replace it
> >   the UNDEF by some appropriate value (for example 0) if this is desired.
> >
> 
> Okay - I guess in my suggestion the size '0' signifies undefined or
> not 'register size' therefore not eligible for CSE.
> 
> >> 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?
> >
> > Indeed.
> > Anyway, once you have UB anything is '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.
> >
> > This can be done, but this opcode would be semantically be a store
> > so it doesn't help to have another op for it.
> 
> Thinking more about it - perhaps if a pseudo value of size 0 was used
> when the aggregate is bigger than register size then this would still
> be okay. I can see that when values are register size store makes
> sense.
> 
> >
> >> 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.
> >
> > Even is 'pseudo' holds for 'pseudo-register' they are used for any 'values'.
> Not sure I understood this point - I was suggesting that we only set
> pseudo value size when it makes sense to do so.

I meant that I think that it would be dangerous to special-case 
pseudos with register-size and non-register-size (we're by dangerous
I mostly mean 'add unneeded complexity').
It's a fuzzy concept anyway and target specific while there is no need
to be target specific.

> >> However, that still leaves a question in my mind:
> >>
> >> a) Is it okay to assign a value 0 to uninitialized vars?
> >
> > As explained here above, it's OK but it's just a compiler choice
> > for something UB (and certainly not worse than any other).
> >
> >> 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.
> >
> > Sure, unless you special case it in CSE.
> 
> I was wondering if the source of the bugs related to uninitialized var
> and CSE is this that the CSE doesn't know the value came from garbage.

Once we choose to give value 0 to otherwise unintialized vars, it's
no more garbage than anything else. It's something very sane to do
for a 'robust compiler'.

> >> 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?
> >
> > Currently, a size of zero is used for erroneous code, like incomplete
> > type and things like that (-1/~0 is also used for that). In such
> > situations CSE make no sense anyway, we just want to continue parsing
> > and give the best possible diagnostic for the remaining of the code.
> >
> 
> I didn't follow - size 0 is used for pseudo values?

Sorry, when I said 'size' I meant 'size of the underlying type' while
you were talking about the size given to the pseudo.
 
But my answer would be 'yes for pseudos coming from errors', 'no for
pseudo from variables we're not sure if they will be used only after
having received a value', 'why special case at this level? it would
be better to use consistently a special kind of value for erroneous
things (VOID is used for this but also used for other things which
doesn't help)'.

-- Luc
--
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