[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
Re: CVS commit: src/sys/kern
On Thu, Nov 07, 2019 at 09:58:06 -0600, David Young wrote:
> On Thu, Nov 07, 2019 at 04:26:51PM +0100, Martin Husemann wrote:
> > On Thu, Nov 07, 2019 at 02:53:08PM +0100, Kamil Rytarowski wrote:
> > > On 07.11.2019 14:25, Valery Ushakov wrote:
> > > > If the sanitizer does complain about other uses, there is little point
> > > > in fixing one instance and not the others.
> > >
> > > We already agreed with Christos that this is appeasing of GCC. If you
> > > want to scan the whole kernel (or whole C) file for more occurrences of
> > > violations - please go for it.
> > No. The commit needs to be reverted, and then
> > a) either the root cause for the unaligned address be fixed or
> > b) some other means be found to make the sanitizer shut up
> > As uwe said: papering over a tiny detail that *never* hits in the real
> > world but potentialy hiding a real issue is not the way to go.
> > Martin
> > P.S.: Independend of this I would still like an official C standard
> > clarification; in my reading a simple address calculation is not
> > accessing an object through a pointer (which would be the undefined
> > behaviour). If the C standard is not clear on this, it needs to be
> > improved.
> I think the problem is that if you have the series of statements,
> element_t *e = &s->element;
> if (s == NULL)
> then the comparison with NULL implies that in this scope, s could
> be NULL. NULL does not necessarily have any "arithmetic" relationship
> to any other pointer---by that rationale, you probably cannot assign
> any alignment to it---so there is no sensible value that you can
> give to e.
This is not what the changed code does. The code in question has
struct disklabel *dlp = ...;
apparently gcc complains about
but later the code uses e.g. dlp->d_partitions (right after the
check_label_magic call) and other memebers. So it's very suspicious
that one usage is flagged and others are not.
Until very recently the magic check was also explicitly comparing
dlp->d_magic != DISKMAGIC, etc. So may be we should stop pretending
and rewrite check_label_magic() to use that instead of memcmp. (And
then fix all dlp->foo in one swoop).
If my I interpretation is wrong, I would be glad to be corrected.
> There is probably an argument to be made that in a
> segmented/tagged/capability architecture that has run C programs
> (8086; Burroughs Large Systems) or may run them in the future (CHERI),
> &(NULL)->element cannot sensibly be computed.
Amen :). I actually did encounter problems like that when compiling
software on Xenix 286 ages ago (e.g. 0 instead of NULL passed as the
last argument to exec). While that is a fascinating excercise, it's
not what happens here.
Main Index |
Thread Index |