On Tue, Dec 04, 2018 at 10:33:23AM +0100, Ard Biesheuvel wrote:
> >>>>> +#pragma pack(push, 1)
> >>>>
> >>>> I don't see this #pragma making any difference to the structs below,
> >>>> can it be dropped?
> >>>
> >>> The pragma pack is defensive. Without it, we rely on the compiler
> >>> packing structures by default and this may not happen on 64 bit
> >>> compiles.
> >>
> >> I understand that is what the pragma does. My comment was because all of
> >> the
> >> variables in the below structs are naturally aligned.
> >> The reason I dislike its use when effectively a no-op, is that it makes it
> >> look like it
> >> it isn't a no-op.
> >>
> >> If it covers a larger set of structs, some of which require the packed
> >> attribute I'm
> >> OK with that. But I'm not a fan of adding it "just in case" without
> >> contemplating
> >> the statement's (lack of) effect.
> >>
> >> Regards,
> >>
> >> Leif
> >>
> >
> > Makes sense. I am checking to make sure this pragma wasn't
> > included due to some observed compiler behavior on our end, since
> > this header is also used on our ARM64 work.
> > Will remove it once confirmed it is safe.
>
> I’d prefer to keep the pragma. It doesn’t only remove padding, it
> also informs the compiler that the whole struct may appear at any
> unaligned offset.
> On 32 bit ARM, this means the compiler will
> refrain from using load/store double or load/store multiple to
> access the contents when dereferencing a pointer to such a struct,
> which would result in a crash otherwise.
OK, if it is a real concern that the structs themselves may appear
unaligned in memory, please ignore this feedback.
/
Leif
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
