Re: [Xen-devel] [PATCH] trace: fix build with gcc9

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 08.03.19 at 12:58, <George.Dunlap@xxxxxxxxxx> wrote:

> 
>> On Mar 8, 2019, at 7:11 AM, Jan Beulich <jbeulich@xxxxxxxx> wrote:
>> 
>>>>> George Dunlap <george.dunlap@xxxxxxxxxx> 03/07/19 1:02 PM >>>
>>> On 3/7/19 10:34 AM, Jan Beulich wrote:
>>>> While I've not observed this myself, gcc 9 (imo validly) reportedly may
>>>> complain
>>>> 
>>>> trace.c: In function '__trace_hypercall':
>>>> trace.c:826:19: error: taking address of packed member of 'struct 
>>>> <anonymous>' 
> may result in an unaligned pointer value
>>>> [-Werror=address-of-packed-member]
>>>>  826 |     uint32_t *a = d.args;
>>> 
>>> Wait, is this saying that in this case (i.e., with a single uint32_t
>>> before args), you *do* get an unaligned pointer value, or just that if
>>> the struct changes in the future that the pointer value may become
>>> un-aligned?
>> 
>> With the __packed attribute, the compiler _could_ place the structure at
>> a misaligned slot on the stack. It has got nothing to do with struct layout
>> afaict. Note how the diagnostic says "may", not "does”.
> 
> That’s interesting — so is there no way to specify that the struct should be 
> aligned but the individual elements not?

I think there is (marking the fields __packed and the struct __aligned()),
but that wouldn't necessarily help (depending on how the checking gets
don internal to the compiler).

> If that’s the case, what about doing something like the attached instead?  
> It avoids introducing a not-very-obvious BUILD_BUG_ON(), and also I think 
> makes the algorithm a (tiny) bit easier to follow.  (And if the 
> BUILD_BUG_ON() ever triggered, we’d probably end up having to do something 
> like this anyway.)

That's an option. Yet don't forget that the compiler noticing the issue
(and spitting out the warning) likely means that it would still spot the
issue, but just have no reason to warn anymore. It spotting the issue
would mean though that on architectures where mis-aligned accesses
may fault, it may then produce pretty inefficient code for a case where
simple aligned accesses would be fine.

IOW I prefer my variant of the workaround, but you're the maintainer
of this code, so you've got to decide.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel