On Wed, 24 Feb 2021, Stefan Seyfried wrote:
Hi Richard,
On 24.02.21 13:14, Richard Biener wrote:
On Wed, 24 Feb 2021, Stefan Seyfried wrote:
Hi all,
I just noticed that a build for SLES12 threw the following warning:
gcc -DHAVE_CONFIG_H -I. -I.. -DCONFDIR='"/etc"' -fmessage-length=0 -grecord-gcc-switches -fstack-protector -O2 -Wall -D_FORTIFY_SOURCE=2 -funwind-tables -fasynchronous-unwind-tables -g -c -o pdnsd-list.o `test -f 'list.c' || echo './'`list.c list.c: In function 'dlist_grow': list.c:113:3: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing] *((size_t *)&a->data[a->last])=a->lastsz; ^
While when build for SLES15, with similar CFLAGS="-fmessage-length=0 -grecord-gcc-switches -O2 -Wall -D_FORTIFY_SOURCE=2 -fstack-protector-strong -funwind-tables -fasynchronous-unwind-tables -fstack-clash-protection -g" it does not warn.
The code is like:
109 size_t sz=0, allocsz=0, szincr, newsz; 110 if(a) { 111 sz=a->last+a->lastsz; 112 allocsz = (sz+DLISTCHUNKSIZEMASK)&(~DLISTCHUNKSIZEMASK); 113 *((size_t *)&a->data[a->last])=a->lastsz; 114 }
I was just starting to patch this, then I noticed that I could not even get the warning to show on newer distributions. (My OBS instance does not have factory available easily, so I just tested OBS build on SLES12 and SLES15, but the warning does not trigger on current Tumbleweed, too, even with "-Wstrict-aliasing" added to the CFLAGS mix.
What has changed?
The compiler, likely. To tell whether the *((size_t *)&... has an issue one would ned to see the aggregate type used for 'a'. In case a->data is not of type size_t * or size_t[] there is a possible strict-aliasing issue. The compiler change might be to no longer warn when a->data is for example an array of chars.
Yes, it is: ------ struct _dynamic_list_head { size_t last,lastsz; char data[0]; }; typedef struct _dynamic_list_head *dlist; -----
the function is dlist dlist_grow(dlist a, size_t len) {}
(and yes, I know the "char data[0];" is no longer state of the art and one should really do "char *data;" :-)
So the compiler has become more smart and does know that this is unproblematic (my patch would have been something like
- *((size_t *)&a->data[a->last])=a->lastsz; + char *dataptr = a->data + a->last; + *(size_t *)dataptr = a->lastsz;
this, nothing really complicated, but if I can avoid it, I'll not do that just to silence a no longer existing warning :-).
It indeed is not problematic. Note that a slightly more elaborated
#define N large enough
struct _dynamic_list_head {
int last,lastsz; // NOT size_t
char data[N];
} *ptr, *ptr2;
there's the possibility of breakage if you do
*(size_t *)ptr->data = ...;
and then
memset (ptr2, 0, sizeof (*ptr2));
*ptr2 = *ptr;
(an aggregate copy of the "container"). Here the aggregate copy
would _not_ alias with the size_t store and thus a load like
... = *(size_t)ptr2->data;
could yield zero (from the memset). For your type, since data has
zero size you'd not copy anything anyway that way. The
"modern" way would be to use char data[]; which makes the type
incomplete and thus an attempt to aggregate copy it would be
rejected.
So the warning was removed because it fired more often in a
OK context than in a problematical case (where the problem doesn't
lie in the warned stmt but elsewhere).
Sorry if I added confusion here ;)
Richard.
--
Richard Biener