Re: [PATCH 2/2] zfs: Fix gcc10 error -Werror=zero-length-bounds

[Daniel Kiper]

On Thu, Mar 26, 2020 at 02:35:35PM +0800, Michael Chang wrote:
> 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 best to
> explain what is going on here.
>
> "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.
>
> 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 <address@hidden>

Reviewed-by: Daniel Kiper <address@hidden>

Daniel