On Thu, Mar 26, 2020 at 12:13:11PM +0800, Michael Chang wrote: > On Wed, Mar 25, 2020 at 07:35:27PM +0100, Daniel Kiper wrote: > > On Wed, Mar 25, 2020 at 03:27:28PM +0800, Michael Chang wrote: > > > On Tue, Mar 24, 2020 at 06:54:43PM +0100, Thomas Schmitt wrote: > > [snip] > > > > >From 8666468ac1a35f0678672de5c89a3f062d1aeb39 Mon Sep 17 00:00:00 2001 > > > From: Michael Chang <mch...@suse.com> > > > Date: Wed, 25 Mar 2020 14:28:15 +0800 > > > Subject: [PATCH 2/2] zfs: Fix gcc10 error -Werror=zero-length-bounds > > > > > > We bumped into the build error while testing gcc-10 pre-release. > > > > > > In file included from ../../include/grub/file.h:22, > > > from ../../grub-core/fs/zfs/zfs.c:34: > > > ../../grub-core/fs/zfs/zfs.c: In function 'zap_leaf_lookup': > > > ../../grub-core/fs/zfs/zfs.c:2263:44: error: array subscript '<unknown>' > > > is outside the bounds of an interior zero-length array 'grub_uint16_t[0]' > > > {aka 'short unsigned int[0]'} [-Werror=zero-length-bounds] > > > 2263 | for (chunk = grub_zfs_to_cpu16 (l->l_hash[LEAF_HASH (blksft, h, > > > l)], endian); > > > ../../include/grub/types.h:241:48: note: in definition of macro > > > 'grub_le_to_cpu16' > > > 241 | # define grub_le_to_cpu16(x) ((grub_uint16_t) (x)) > > > | ^ > > > ../../grub-core/fs/zfs/zfs.c:2263:16: note: in expansion of macro > > > 'grub_zfs_to_cpu16' > > > 2263 | for (chunk = grub_zfs_to_cpu16 (l->l_hash[LEAF_HASH (blksft, h, > > > l)], endian); > > > | ^~~~~~~~~~~~~~~~~ > > > In file included from ../../grub-core/fs/zfs/zfs.c:48: > > > ../../include/grub/zfs/zap_leaf.h:72:16: note: while referencing 'l_hash' > > > 72 | grub_uint16_t l_hash[0]; > > > | ^~~~~~ > > > > > > Here I'd like to quote from the gcc document [1] which seems to be best > > > to explain what is going on here. > > > > > > "Declaring zero-length arrays in other contexts, including as interior > > > members of structure objects or as non-member objects, is discouraged. > > > Accessing elements of zero-length arrays declared in such contexts is > > > undefined and may be diagnosed." > > > > > > The l_hash[0] is apparnetly an interior member to the enclosed structure > > > while l_entries[0] is the trailing member. And the offending code tries > > > to access members in l_hash[0] array that triggers the diagnose. > > > > > > Given that the l_entries[0] is used to get proper alignment to access > > > leaf chunks, we can accomplish the same thing through the ALIGN_UP macro > > > thus eliminating l_entries[0] from the structure. In this way we can > > > pacify the warning as l_hash[0] now becomes the last member to the > > > enclosed structure. > > > > > > [1] https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html > > > > > > Signed-off-by: Michael Chang <mch...@suse.com> > > > --- > > > grub-core/fs/zfs/zfs.c | 7 ++++--- > > > include/grub/zfs/zap_leaf.h | 1 - > > > 2 files changed, 4 insertions(+), 4 deletions(-) > > > > > > diff --git a/grub-core/fs/zfs/zfs.c b/grub-core/fs/zfs/zfs.c > > > index 2f72e42bf..f38f5b102 100644 > > > --- a/grub-core/fs/zfs/zfs.c > > > +++ b/grub-core/fs/zfs/zfs.c > > > @@ -141,9 +141,10 @@ ZAP_LEAF_NUMCHUNKS (int bs) > > > static inline zap_leaf_chunk_t * > > > ZAP_LEAF_CHUNK (zap_leaf_phys_t *l, int bs, int idx) > > > { > > > - return &((zap_leaf_chunk_t *) (l->l_entries > > > - + (ZAP_LEAF_HASH_NUMENTRIES(bs) * 2) > > > - / sizeof (grub_properly_aligned_t)))[idx]; > > > + grub_properly_aligned_t *l_entries; > > > + > > > + l_entries = (grub_properly_aligned_t *) > > > ALIGN_UP((grub_addr_t)l->l_hash, sizeof (grub_properly_aligned_t)); > > > + return &((zap_leaf_chunk_t *) (l_entries + > > > ZAP_LEAF_HASH_NUMENTRIES(bs)))[idx]; > > > > Why are you skipping "* 2) / sizeof (grub_properly_aligned_t)" here?
Ah, indeed I had mistake here. It has to take "HASH_NUMENTRIES * 2) / sizeof (grub_properly_aligned_t)" for computing the entry number of l_entries from given HASH_NUMENTRIES. I will fix that and repost patches. Thanks, Michael > It is based on this comment before the function. > > "The chunks start immediately after the hash table. The end of the hash > table is at l_hash + HASH_NUMENTRIES, which we simply cast to a > chunk_t." > > I suppose the magic number "2" could be "sizeof (l->l_hash[0])", so the > computatio1n is likely to get number of entries for l->l_entries[] from > which we can take address as the chunk start. And since it is indexed by > type grub_properly_aligned_t, the alignment is automagically satisfied. > > > > > And could you add following excerpt from [1] to the commit message: > > Although the size of a zero-length array is zero, an array member of > > this kind may increase the size of the enclosing type as a result of > > tail padding. The offset of a zero-length array member from the > > beginning of the enclosing structure is the same as the offset of an > > array with one or more elements of the same type. The alignment of a > > zero-length array is the same as the alignment of its elements. > > OK. I will do so. > > > > > > } > > > > > > static inline struct zap_leaf_entry * > > > diff --git a/include/grub/zfs/zap_leaf.h b/include/grub/zfs/zap_leaf.h > > > index 95c67dcba..11447c166 100644 > > > --- a/include/grub/zfs/zap_leaf.h > > > +++ b/include/grub/zfs/zap_leaf.h > > > @@ -70,7 +70,6 @@ typedef struct zap_leaf_phys { > > > */ > > > > > > grub_uint16_t l_hash[0]; > > > - grub_properly_aligned_t l_entries[0]; > > > } zap_leaf_phys_t; > > > > > > typedef union zap_leaf_chunk { > > > -- > > > 2.16.4 > > > > > > > May I ask you to repost the patches in the proper format and CC all > > people involved in the discussion? > > OK. I will. > > Thanks, > Michael > > > > > Daniel > > > > _______________________________________________ > > Grub-devel mailing list > > Grub-devel@gnu.org > > https://lists.gnu.org/mailman/listinfo/grub-devel _______________________________________________ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel