core: Improve the natural alignment calculation

We need to take more than just arrays into account when figuring out the
natural alignment of struct members, looking recursively at types till
we get to basic types and pointers.

Before this patch the 'new' struct field in the 'v' union was considered
__packed__, when in fact it is not, as the natural alignment for the
'state_id' typedef is 4, so it can start at offset 36 (or 4 considering
just its container struct), see below:

  $ pahole -IC nfsd4_lock /home/acme/git/build/v5.1-rc4+/fs/nfsd/nfs4xdr.o
  /* Used at: /home/acme/git/linux/fs/nfsd/nfs4xdr.c */
  /* <1717a> /home/acme/git/linux/fs/nfsd/xdr4.h:156 */
  struct nfsd4_lock {
          u32                        lk_type;              /*     0     4 */
          u32                        lk_reclaim;           /*     4     4 */
          u64                        lk_offset;            /*     8     8 */
          u64                        lk_length;            /*    16     8 */
          u32                        lk_is_new;            /*    24     4 */

          /* XXX 4 bytes hole, try to pack */

          union {
                  struct {
                          u32        open_seqid;           /*    32     4 */
                          stateid_t  open_stateid;         /*    36    16 */
                          u32        lock_seqid;           /*    52     4 */
                          clientid_t clientid;             /*    56     8 */
                          /* --- cacheline 1 boundary (64 bytes) --- */
                          struct xdr_netobj owner;         /*    64    16 */
                  } __attribute__((__packed__)) new;       /*    32    48 */
                  struct {
                          stateid_t  lock_stateid;         /*    32    16 */
                          u32        lock_seqid;           /*    48     4 */
                  } __attribute__((__packed__)) old;       /*    32    20 */
          } v;                                             /*    32    48 */
          /* --- cacheline 1 boundary (64 bytes) was 16 bytes ago --- */
          union {
                  struct {
                          stateid_t  stateid;              /*    80    16 */
                  } ok;                                    /*    80    16 */
                  struct nfsd4_lock_denied denied;         /*    80    48 */
          } u;                                             /*    80    48 */

          /* size: 128, cachelines: 2, members: 7 */
          /* sum members: 124, holes: 1, sum holes: 4 */
  };
  $

Asking for -rEIC, i.e. relative offsets, expand types we can see that
stateid_t opaque type:

                struct {
                        /* typedef u32 -> __u32 */ unsigned int open_seqid;              /*     0     4 */
                        /* typedef stateid_t */ struct {
                                /* typedef u32 -> __u32 */ unsigned int si_generation;   /*     0     4 */
                                /* typedef stateid_opaque_t */ struct {
                                        /* typedef clientid_t */ struct {
                                                /* typedef u32 -> __u32 */ unsigned int   cl_boot; /*     0     4 */
                                                /* typedef u32 -> __u32 */ unsigned int   cl_id; /*     4     4 */
                                        } so_clid; /*     0     8 */
                                        /* typedef u32 -> __u32 */ unsigned int so_id;   /*     8     4 */
                                } si_opaque; /*     4    12 */
                        } open_stateid; /*     4    16 */

With the algorithm implemented in this patch we get it correctly as not
packed:

  $ pahole -IC nfsd4_lock /home/acme/git/build/v5.1-rc4+/fs/nfsd/nfs4xdr.o
  /* Used at: /home/acme/git/linux/fs/nfsd/nfs4xdr.c */
  /* <1717a> /home/acme/git/linux/fs/nfsd/xdr4.h:156 */
  struct nfsd4_lock {
          u32                        lk_type;              /*     0     4 */
          u32                        lk_reclaim;           /*     4     4 */
          u64                        lk_offset;            /*     8     8 */
          u64                        lk_length;            /*    16     8 */
          u32                        lk_is_new;            /*    24     4 */

          /* XXX 4 bytes hole, try to pack */

          union {
                  struct {
                          u32        open_seqid;           /*    32     4 */
                          stateid_t  open_stateid;         /*    36    16 */
                          u32        lock_seqid;           /*    52     4 */
                          clientid_t clientid;             /*    56     8 */
                          /* --- cacheline 1 boundary (64 bytes) --- */
                          struct xdr_netobj owner;         /*    64    16 */
                  } new;                                   /*    32    48 */
                  struct {
                          stateid_t  lock_stateid;         /*    32    16 */
                          u32        lock_seqid;           /*    48     4 */
                  } old;                                   /*    32    20 */
          } v;                                             /*    32    48 */
          /* --- cacheline 1 boundary (64 bytes) was 16 bytes ago --- */
          union {
                  struct {
                          stateid_t  stateid;              /*    80    16 */
                  } ok;                                    /*    80    16 */
                  struct nfsd4_lock_denied denied;         /*    80    48 */
          } u;                                             /*    80    48 */

          /* size: 128, cachelines: 2, members: 7 */
          /* sum members: 124, holes: 1, sum holes: 4 */
  };

Fixes: f2641ce169 ("core: Take arrays into account when inferring if a struct is packed")
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
This commit is contained in:
Arnaldo Carvalho de Melo 2019-04-11 16:36:49 -03:00
parent ac32e5e908
commit dc3d441961
2 changed files with 44 additions and 17 deletions

View File

@ -1291,17 +1291,49 @@ void class__find_holes(struct class *class)
class->holes_searched = true;
}
static size_t class_member__byte_size_for_alignment(struct class_member *member, const struct cu *cu)
static size_t type__natural_alignment(struct type *type, const struct cu *cu);
static size_t tag__natural_alignment(struct tag *tag, const struct cu *cu)
{
struct tag *type = tag__strip_typedefs_and_modifiers(&member->tag, cu);
size_t natural_alignment = 1;
if (type->tag == DW_TAG_array_type) {
type = tag__strip_typedefs_and_modifiers(type, cu);
return tag__size(type, cu);
if (tag__is_pointer(tag)) {
natural_alignment = cu->addr_size;
} else if (tag->tag == DW_TAG_base_type) {
natural_alignment = base_type__size(tag);
} else if (tag__is_enumeration(tag)) {
natural_alignment = tag__type(tag)->size / 8;
} else if (tag__is_struct(tag) || tag__is_union(tag)) {
natural_alignment = type__natural_alignment(tag__type(tag), cu);
} else if (tag->tag == DW_TAG_array_type) {
tag = tag__strip_typedefs_and_modifiers(tag, cu);
natural_alignment = tag__natural_alignment(tag, cu);
}
return member->byte_size;
return natural_alignment;
}
static size_t type__natural_alignment(struct type *type, const struct cu *cu)
{
struct class_member *member;
if (type->natural_alignment != 0)
return type->natural_alignment;
type__for_each_member(type, member) {
/* XXX for now just skip these */
if (member->tag.tag == DW_TAG_inheritance &&
member->virtuality == DW_VIRTUALITY_virtual)
continue;
struct tag *member_type = tag__strip_typedefs_and_modifiers(&member->tag, cu);
size_t member_natural_alignment = tag__natural_alignment(member_type, cu);
if (type->natural_alignment < member_natural_alignment)
type->natural_alignment = member_natural_alignment;
}
return type->natural_alignment;
}
bool class__infer_packed_attributes(struct class *cls, const struct cu *cu)
@ -1332,20 +1364,13 @@ bool class__infer_packed_attributes(struct class *cls, const struct cu *cu)
if (pos->is_static)
continue;
size_t byte_size = class_member__byte_size_for_alignment(pos, cu);
struct tag *member_type = tag__strip_typedefs_and_modifiers(&pos->tag, cu);
size_t natural_alignment = tag__natural_alignment(member_type, cu);
/* Always aligned: */
if (byte_size == sizeof(char))
if (natural_alignment == sizeof(char))
continue;
if (byte_size >= cu->addr_size) {
max_natural_alignment = cu->addr_size;
if ((pos->byte_offset % cu->addr_size) == 0)
continue;
}
uint16_t natural_alignment = __roundup_pow_of_two(byte_size);
if (max_natural_alignment < natural_alignment)
max_natural_alignment = natural_alignment;

View File

@ -902,6 +902,7 @@ static __pure inline int tag__is_class_member(const struct tag *tag)
* @nr_static_members: number of static DW_TAG_member entries
* @nr_tags: number of tags
* @alignment: DW_AT_alignement, zero if not present, gcc emits since circa 7.3.1
* @natural_alignment: For inferring __packed__, normally the widest scalar in it, recursively
*/
struct type {
struct namespace namespace;
@ -911,6 +912,7 @@ struct type {
uint16_t nr_static_members;
uint16_t nr_members;
uint32_t alignment;
uint16_t natural_alignment;
uint8_t declaration; /* only one bit used */
uint8_t definition_emitted:1;
uint8_t fwd_decl_emitted:1;