Re: disk/mdraid1x_linux.c:181:15: warning: array subscript ...
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
> > > 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 ''
> > > 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
> > > ---
> > > 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
Re: disk/mdraid1x_linux.c:181:15: warning: array subscript ...
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
> > 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 '' 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
> > ---
> > 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?
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 {
> > */
>
Re: disk/mdraid1x_linux.c:181:15: warning: array subscript ...
On 3/25/20 8:29 PM, Michael Chang wrote: >> with your patches, no immediate mdraid1x or zfs build errors > > Thanks a lot for your test and validation. > >> one does surface, now, for ntfscomp ... > > I am also using openSUSE, somehow I didn't have the ntfscomp build error > on my gcc-10 build test. :( > > Would you mind to start a new thread for it ? > > Thanks, > Michael done --> https://lists.gnu.org/archive/html/grub-devel/2020-03/msg00240.html o/ ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
fs/ntfscomp.c:82:11: error: ‘flg’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
building cd grub git log | head -n5 commit 552c9fd08122a3036c724ce96dfe68aa2f75705f Author: Patrick Steinhardt Date: Sat Mar 7 17:29:09 2020 +0100 gnulib: Fix build of base64 when compiling with memory debugging with gcc --version gcc (SUSE Linux) 10.0.1 20200324 (experimental) [revision 75c24a08d697d6442fe6c26142f0559f803af977] patched patch -p1 < /tmp/grub_patches/grub_patches/0001-mdraid1x_linux-Fix-gcc10-error-Werror-array-bounds.patch patching file grub-core/disk/mdraid1x_linux.c patch -p1 < /tmp/grub_patches/grub_patches/0002-zfs-Fix-gcc10-error-Werror-zero-length-bounds.patch patching file grub-core/fs/zfs/zfs.c patching file include/grub/zfs/zap_leaf.h from https://lists.gnu.org/archive/html/grub-devel/2020-03/msg00222.html config's ok unset CC CPP ./bootstrap ./autogen.sh ./configure build FAILs make V=1 gcc -DHAVE_CONFIG_H -I. -Wall -W -DGRUB_UTIL=1 -D_FILE_OFFSET_BITS=64 -I./include -DGRUB_FILE=\"grub-core/fs/ntfscomp.c\" -I. -I. -I. -I. -I./include -I./include -I./grub-core/lib/libgcrypt-grub/src/ -I./grub-core/lib/minilzo -I./grub-core/lib/xzembed -I./grub-core/lib/zstd -DMINILZO_HAVE_CONFIG_H -O3 -Wall -fstack-protector-strong -funwind-tables -fasynchronous-unwind-tables -fmessage-length=0 -grecord-gcc-switches -march=native -mtune=native -D_FORTIFY_SOURCE=2 -D_FILE_OFFSET_BITS=64 -Wall -W -Wshadow -Wpointer-arith -Wundef -Wchar-subscripts -Wcomment -Wdeprecated-declarations -Wdisabled-optimization -Wdiv-by-zero -Wfloat-equal -Wformat-extra-args -Wformat-security -Wformat-y2k -Wimplicit -Wimplicit-function-declaration -Wimplicit-int -Wmain -Wmissing-braces -Wmissing-format-attribute -Wmultichar -Wparentheses -Wreturn-type -Wsequence-point -Wshadow -Wsign-compare -Wswitch -Wtrigraphs -Wunknown-pragmas -Wunused -Wunused-function -Wunused-label -Wunused-parameter -Wunused-value -Wunused-variable -Wwrite-strings -Wnested-externs -Wstrict-prototypes -Wcast-align -Wextra -Wattributes -Wendif-labels -Winit-self -Wint-to-pointer-cast -Winvalid-pch -Wmissing-field-initializers -Wnonnull -Woverflow -Wvla -Wpointer-to-int-cast -Wstrict-aliasing -Wvariadic-macros -Wvolatile-register-var -Wpointer-sign -Wmissing-include-dirs -Wmissing-prototypes -Wmissing-declarations -Wformat=2 -Werror -fno-builtin -Wno-undef -O3 -Wall -fstack-protector-strong -funwind-tables -fasynchronous-unwind-tables -fmessage-length=0 -grecord-gcc-switches -march=native -mtune=native -MT grub-core/fs/libgrubmods_a-ntfscomp.o -MD -MP -MF grub-core/fs/.deps-util/libgrubmods_a-ntfscomp.Tpo -c -o grub-core/fs/libgrubmods_a-ntfscomp.o `test -f 'grub-core/fs/ntfscomp.c' || echo './'`grub-core/fs/ntfscomp.c grub-core/fs/ntfscomp.c: In function ‘read_block’: grub-core/fs/ntfscomp.c:82:11: error: ‘flg’ may be used uninitialized in this function [-Werror=maybe-uninitialized] 82 | if (flg & 0x8000) | ^~~ grub-core/fs/ntfscomp.c:74:17: note: ‘flg’ was declared here 74 | grub_uint16_t flg, cnt; | ^~~ cc1: all warnings being treated as errors make[2]: *** [Makefile:7647: grub-core/fs/libgrubmods_a-ntfscomp.o] Error 1 make[2]: Leaving directory '/usr/local/src/grub' make[1]: *** [Makefile:11920: all-recursive] Error 1 make[1]: Leaving directory '/usr/local/src/grub' make: *** [Makefile:3772: all] Error 2 ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: disk/mdraid1x_linux.c:181:15: warning: array subscript ...
On Wed, Mar 25, 2020 at 08:35:58AM -0700, PGNet Dev wrote:
> On 3/25/20 12:27 AM, Michael Chang wrote:
> > It would be great if you can help to test patch to solve the build
> > problem for gcc-10 in your system or not.
>
> with your patches, no immediate mdraid1x or zfs build errors
Thanks a lot for your test and validation.
> one does surface, now, for ntfscomp ...
I am also using openSUSE, somehow I didn't have the ntfscomp build error
on my gcc-10 build test. :(
Would you mind to start a new thread for it ?
Thanks,
Michael
>
> gcc --version
> gcc (SUSE Linux) 10.0.1 20200320 (experimental) [revision
> 7d4549b2cd209eb621453ce13be7ffd84ffa720a]
>
> cd grub
> git log -n1
> 1 commit 552c9fd08122a3036c724ce96dfe68aa2f75705f (HEAD -> master,
> origin/master, origin/HEAD)
> 2 Author: Patrick Steinhardt
> 3 Date: Sat Mar 7 17:29:09 2020 +0100
>
> patch -p1 <
> /tmp/grub_patches/0001-mdraid1x_linux-Fix-gcc10-error-Werror-array-bounds.patch
> patching file grub-core/disk/mdraid1x_linux.c
> patch -p1 <
> /tmp/grub_patches/0002-zfs-Fix-gcc10-error-Werror-zero-length-bounds.patch
> patching file grub-core/fs/zfs/zfs.c
> patching file include/grub/zfs/zap_leaf.h
>
> unset CC CPP
> ./bootstrap
> ./autogen.sh
> ./configure
> make V=1 -j${CORES}
> ...
> make[2]: Entering directory '/usr/local/src/grub'
> gcc -DHAVE_CONFIG_H -I. -Wall -W -DGRUB_UTIL=1 -D_FILE_OFFSET_BITS=64
> -I./include -DGRUB_FILE=\"grub-core/fs/ntfscomp.c\" -I. -I. -I. -I.
> -I./include -I./include -I./grub-core/lib/libgcrypt-grub/src/
> -I./grub-core/lib/minilzo -I./grub-core/lib/xzembed -I./grub-core/lib/zstd
> -DMINILZO_HAVE_CONFIG_H -O3 -Wall -fstack-protector-strong -funwind-tables
> -fasynchronous-unwind-tables -fmessage-length=0 -grecord-gcc-switches
> -march=native -mtune=native -D_FORTIFY_SOURCE=2 -D_FILE_OFFSET_BITS=64 -Wall
> -W -Wshadow -Wpointer-arith -Wundef -Wchar-subscripts -Wcomment
> -Wdeprecated-declarations -Wdisabled-optimization -Wdiv-by-zero -Wfloat-equal
> -Wformat-extra-args -Wformat-security -Wformat-y2k -Wimplicit
> -Wimplicit-function-declaration -Wimplicit-int -Wmain -Wmissing-braces
> -Wmissing-format-attribute -Wmultichar -Wparentheses -Wreturn-type
> -Wsequence-point -Wshadow -Wsign-compare -Wswitch -Wtrigraphs
> -Wunknown-pragmas -Wunused -Wunused-function -Wunused-label
> -Wunused-parameter -Wunused-value -Wunused-variable -Wwrite-strings
> -Wnested-externs -Wstrict-prototypes -Wcast-align -Wextra -Wattributes
> -Wendif-labels -Winit-self -Wint-to-pointer-cast -Winvalid-pch
> -Wmissing-field-initializers -Wnonnull -Woverflow -Wvla -Wpointer-to-int-cast
> -Wstrict-aliasing -Wvariadic-macros -Wvolatile-register-var -Wpointer-sign
> -Wmissing-include-dirs -Wmissing-prototypes -Wmissing-declarations -Wformat=2
> -Werror -fno-builtin -Wno-undef -O3 -Wall -fstack-protector-strong
> -funwind-tables -fasynchronous-unwind-tables -fmessage-length=0
> -grecord-gcc-switches -march=native -mtune=native -MT
> grub-core/fs/libgrubmods_a-ntfscomp.o -MD -MP -MF
> grub-core/fs/.deps-util/libgrubmods_a-ntfscomp.Tpo -c -o
> grub-core/fs/libgrubmods_a-ntfscomp.o `test -f 'grub-core/fs/ntfscomp.c' ||
> echo './'`grub-core/fs/ntfscomp.c
> grub-core/fs/ntfscomp.c: In function ‘read_block’:
> grub-core/fs/ntfscomp.c:82:11: error: ‘flg’ may be used uninitialized
> in this function [-Werror=maybe-uninitialized]
> 82 | if (flg & 0x8000)
> | ^~~
> grub-core/fs/ntfscomp.c:74:17: note: ‘flg’ was declared here
> 74 | grub_uint16_t flg, cnt;
> | ^~~
> cc1: all warnings being treated as errors
> make[2]: *** [Makefile:7647: grub-core/fs/libgrubmods_a-ntfscomp.o]
> Error 1
> make[2]: Leaving directory '/usr/local/src/grub'
> make[1]: *** [Makefile:11920: all-recursive] Error 1
> make[1]: Leaving directory '/usr/local/src/grub'
> make: *** [Makefile:3772: all] Error 2
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
Re: disk/mdraid1x_linux.c:181:15: warning: array subscript ...
On Wed, Mar 25, 2020 at 12:13:49PM +0100, Thomas Schmitt wrote:
> Hi,
>
> Michael Chang wrote in patch 2/2:
> > ../../grub-core/fs/zfs/zfs.c:2263:44: error: array subscript '' is
> > outside the bounds of an interior zero-length array 'grub_uint16_t[0]' {aka
> > 'short unsigned int[0]'} [-Werror=zero-length-bounds]
> > [...]
> > The l_hash[0] is apparnetly an interior member to the enclosed structure
> > [...] the l_entries[0] is used to get proper alignment to access leaf chunks
>
> That's what i call a dirty hack.
>
> I wonder what gcc would say to a union of [0]-sized arrays as last
> member of the struct type:
>
> struct typedef struct zap_leaf_phys {
> ...
> union {
> grub_uint16_t l_hash[0];
> grub_properly_aligned_t l_entries[0];
> } l_;
> } zap_leaf_phys_t;
As far as I know the union will store different types in same memory
location, thus the allocation is determined by the largest member in the
union, so does the padding has to meet the alignment requirement by the
largest member.
So in this case l_hash[0] (2 bytes) will be aligned to
grub_properly_aligned_t (8 bytes) and for the result is desired or not
really depends on what you want to achieve.
>
> Mine likes such a gesture, but it is way behind in respect to modern bitrot.
Admittedly that would produce much more readable code, but it is just
not work for the extraordinary case here because of specific requirment
to the alignment.
>
>
> So in the end, your patch looks like the solid and unambiguous way to
> implement what is desired.
>
>
> > It would be great if you can help to test patch to solve the build
> > problem for gcc-10 in your system or not.
>
> Due to lack of modernity i can only contribute statements that the
> concepts and motivations of your two patches look good to me.
>
> (I also lack the occasion to test the two use cases.
> Just lurking here for grub-mkrescue issues, where i provide the last stage
> of packing up the ISO image.)
The grub-mkrescue is a gem, it never disappoints me whenever I need it.
:)
Thanks a lot for your feedback.
Michael
>
>
> Have a nice day :)
>
> Thomas
>
>
> ___
> 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
Re: [PATCH] grub.d: Use linuxefi and initrdefi commands if platform is efi
I got it, Thanks for review. Tianjia On 2020/3/26 1:38, Daniel Kiper wrote: On Mon, Mar 23, 2020 at 07:53:15PM +0800, Tianjia Zhang wrote: When the platform is EFI platform, use 'linuxefi' and 'initrdefi' commands instead of 'linux' and 'initrd'. Signed-off-by: Jia Zhang Signed-off-by: Tianjia Zhang Sorry, NAK! We do not want more "linuxefi" kinda commands in the GRUB upstream. 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
Re: [PATCH v1] http: return error on unhandled HTTP error responses
Am Wed, 25 Mar 2020 19:55:47 +0100 schrieb Daniel Kiper : > Should not we do the same for 404, file not found, a few lines above? Maybe. For some reason a 404 returns quickly, while a 400 will request the file 4 times. With this patch there is still some delay, but the request is sent just once. I wonder what the author had in mind, where the error/state is actually supposed to be checked. Olaf pgpvo_NftlTNt.pgp Description: Digitale Signatur von OpenPGP ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH 0/7] ZFS/other CoW FS save_env support
Not a problem, I suspected as much. We don't have any strong need to get this work integrated ASAP, since we are going to have to build a custom version of GRUB for internal use until this makes its way into a release that lands in an Ubuntu LTS. We just want to make sure that we get some good reviews on the changes from people with experience in the codebase, and that one day we will be able to stop maintaining our fork. On Wed, Mar 25, 2020 at 10:06 AM Daniel Kiper wrote: > > Hi Paul, > > On Wed, Mar 11, 2020 at 10:37:08AM -0700, Paul Dagnelie wrote: > > Hey all, I previously discussed my concept for this patch in my email > > https://lists.gnu.org/archive/html/grub-devel/2020-01/msg4.html . > > I'm pleased to announce that I've gotten it into a working state and > > it is ready to review. There are a number of changes here, which I > > will break down below. > > > > First, the concept of "special files" is introduced into grub. These > > are files that are manipulated using different functions from the > > remainder of the filesystem. A filesystem advertises its support for > > these files by filling in new entries in the grub_fs struct. > > > > Second, the loadenv command was modified to use the special file > > functions of the root filesystem if they are detected. If that > > happens, these functions are used to open, read, and write to the > > loadenv file instead of the normal grub file functions. A variable > > was also added that allows the user to force a file to be used instead > > of the special files, or vice versa. > > > > Third, the grub-editenv command was modified to teach it to probe the > > root filesystem and then check in an array of structures that contain > > functions that will read and modify the filesystem's special file in > > userland. These functions are very similar to normal read and write > > functions, and so don't result in a very different code flow. > > > > Finally, the GRUB ZFS logic was modified to have new special_file > > functions. These functions manipulate the pad2 area of the ZFS label, > > which was previously unused. It now stores a version number, the raw > > contents of the grubenv file, and an embedded checksum for integrity > > purposes. GRUB was taught to read and verify these areas, and also to > > modify them, computing the embeddded checksum appropriately. In > > addition, if GRUB is build with libzfs support, an entry is added to > > the grub-editenv table that points GRUB to the appropriate libzfs > > functions, and init and fini functions are built into grub-editenv > > itself. > > > > Future work: > > * In the ZFS code could store a packed nvlist instead of a raw file, > > but this would involve further changes to the grub environment file > > code and was deferred for when it would be more useful and important. > > * For the special files code, there is only one special file allowed > > per filesystem, but a specification interface (using either an enum or > > a set of names) could be added in the future. > > * Other filesystem support was not built into this change, but > > extensibility was a primary goal, so adding support for btrfs or other > > filesystems should be relatively straightforward. > > * I did not implement the proposed UEFI variable support, but I did > > develop this with that future in mind, so adding it should be a > > relatively simple extension of these changes. > > > > This patchset has been tested on systems with and without ZFS as the boot > > filesystem, and built with and without ZFS libraries installed. It > > worked in each case I tried, including with manual corruption applied > > to the ZFS labels to force GRUB to read from the other label. This > > was tested in conjunction with > > https://github.com/zfsonlinux/zfs/pull/10009 , the ZoL patch that > > enables ZFS to read from and write to the padding areas we reserved > > for the data. > > First of all I would like to thank you for doing this work. This is very > interesting feature. I went quickly through the code and it looks quite > nice. However, I realized that it touches a lot of critical places in the > GRUB code. ...and we are in freeze right now. So, at this point I think > it is dangerous to merge that code. Simply we can make a lot of fallout > which we may not be able to catch and clear before the release. Hence, > I would like to suggest to defer this work until the release. Sorry about > that but I do not want to risk GRUB code breakage at this point. I hope > this is not a problem for you. > > Daniel -- Paul Dagnelie ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH v1] http: return error on unhandled HTTP error responses
On Tue, Mar 17, 2020 at 07:56:14PM +0100, Olaf Hering wrote:
> A http transfer will hang if an unhandled error is returned.
> The error branch returns the value zero, which is not expected by the caller.
>
> Signed-off-by: Olaf Hering
> ---
> grub-core/net/http.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/grub-core/net/http.c b/grub-core/net/http.c
> index b616cf40b..9d92a4905 100644
> --- a/grub-core/net/http.c
> +++ b/grub-core/net/http.c
> @@ -125,7 +125,7 @@ parse_line (grub_file_t file, http_data_t data, char
> *ptr, grub_size_t len)
>valid answers like 403 will trigger this very generic message. */
> data->errmsg = grub_xasprintf (_("unsupported HTTP error %d: %s"),
>code, ptr);
> - return GRUB_ERR_NONE;
> + return GRUB_ERR_FILE_READ_ERROR;
Should not we do the same for 404, file not found, a few lines above?
Daniel
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
Re: disk/mdraid1x_linux.c:181:15: warning: array subscript ...
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:
> > Hi,
> >
> > i wrote:
> > > >(char *) _roles - (char *) sb
> > > >+ grub_le_to_cpu32 (sb.dev_number) *
> > > > sizeof(grub_uint16_t)
> >
> > PGNet Dev wrote:
> > > grub-core/disk/mdraid1x_linux.c:183:6: error: cannot convert to a
> > > pointer type
> >
> > My fault. I forgot the "&" before "sb".
> >
> > (char *) _roles - (char *)
> >
> > I invested time in examining the C riddle, not in testing my proposal
> > by at least some dummy.
> >
> > Now this compiles for me without complaint by gcc -Wall
> >
> > struct {
> >char a_array[10];
> >uint16_t dev_roles[0];
> > } sb;
> >
> > printf("%u\n", (unsigned int) (((char *) _roles - (char *) )
> > + 2 * sizeof(uint16_t)));
> >
> > Running this program yields 14 as result. The same as with the equivalent
> > of the old expression
> >
> > printf("%u\n", (unsigned int) ((char *) _roles[2] - (char *) ));
> >
> >
> > > not sure I'm reading your intent from your post,
> >
> > My observation is that not "dev_roles[0]" is to blame for the warning, but
> > rather the computation which involves taking the address of an array
> > element while not a single one is allocated.
> > The resulting number is used as offset in a file, not in the sparsely
> > allocated "struct grub_raid_super_1x sb".
> >
> > My proposal is to avoid "[...]" in the course of the computation.
> > This should be valid for both ways to express an open ended struct:
> > "dev_roles[0]" and "dev_roles[]".
>
> I am also investigating the GCC 10 build problems, and my conclusion
> mostly coincided with yours. There we can replce the offset computation
> with pointer arithmetic like this.
>
> (char *) (sb.dev_roles + grub_le_to_cpu32 (sb.dev_number)) - (char *) ,
>
> Besides, there is also build error in zfs that I managed to come up with
> a patch as well. I attached both my patch here for your refernce.
>
> It would be great if you can help to test patch to solve the build
> problem for gcc-10 in your system or not.
>
> Thanks,
> Michael
>
> >
> >
> > Have a nice day :)
> >
> > Thomas
> >
> >
> > ___
> > Grub-devel mailing list
> > Grub-devel@gnu.org
> > https://lists.gnu.org/mailman/listinfo/grub-devel
> >From 19f55c05ea592b1339ee0d503c86be349447553f Mon Sep 17 00:00:00 2001
> From: Michael Chang
> Date: Wed, 25 Mar 2020 13:52:51 +0800
> Subject: [PATCH 1/2] mdraid1x_linux: Fix gcc10 error -Werror=array-bounds
>
> We bumped into the build error while testing gcc-10 pre-release.
>
> ../../grub-core/disk/mdraid1x_linux.c: In function 'grub_mdraid_detect':
> ../../grub-core/disk/mdraid1x_linux.c:181:15: error: array subscript
> is outside array bounds of 'grub_uint16_t[0]' {aka 'short unsigned
> int[0]'} [-Werror=array-bounds]
> 181 | (char *) _roles[grub_le_to_cpu32 (sb.dev_number)]
> | ^~~
> ../../grub-core/disk/mdraid1x_linux.c:98:17: note: while referencing
> 'dev_roles'
>98 | grub_uint16_t dev_roles[0]; /* Role in array, or 0x for a
> spare, or 0xfffe for faulty. */
> | ^
> ../../grub-core/disk/mdraid1x_linux.c:127:33: note: defined here 'sb'
> 127 | struct grub_raid_super_1x sb;
> | ^~
> cc1: all warnings being treated as errors
>
> Apparently gcc issues the warning when trying to access sb.dev_roles
> array's member, since it is a zero length array as the last element of
> struct grub_raid_super_1x that is allocated sparsely without extra
> chunks for the trailing bits, so the warning looks legitimate in this
> regard.
>
> As the whole thing here is doing offset computation, it is undue to use
> syntax that would imply array member access then take address from it
> later. Instead we could accomplish the same thing through basic array
> pointer arithmetic to pacify the warning.
>
> Signed-off-by: Michael Chang
Reviewed-by: Daniel Kiper
> ---
> grub-core/disk/mdraid1x_linux.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/grub-core/disk/mdraid1x_linux.c b/grub-core/disk/mdraid1x_linux.c
> index 7cc80d3df..c980feba4 100644
> --- a/grub-core/disk/mdraid1x_linux.c
> +++ b/grub-core/disk/mdraid1x_linux.c
> @@ -178,7 +178,7 @@ grub_mdraid_detect (grub_disk_t disk,
> return NULL;
>
>if (grub_disk_read (disk, sector,
> - (char *) _roles[grub_le_to_cpu32
> (sb.dev_number)]
> + (char *) (sb.dev_roles + grub_le_to_cpu32
> (sb.dev_number))
> - (char *) ,
> sizeof (role), ))
> return NULL;
> --
> 2.16.4
>
> >From 8666468ac1a35f0678672de5c89a3f062d1aeb39 Mon Sep 17 00:00:00 2001
> From: Michael Chang
> Date: Wed, 25 Mar 2020 14:28:15 +0800
> Subject: [PATCH
Re: [PATCH] grub.d: Use linuxefi and initrdefi commands if platform is efi
On Mon, Mar 23, 2020 at 07:53:15PM +0800, Tianjia Zhang wrote: > When the platform is EFI platform, use 'linuxefi' and 'initrdefi' > commands instead of 'linux' and 'initrd'. > > Signed-off-by: Jia Zhang > Signed-off-by: Tianjia Zhang Sorry, NAK! We do not want more "linuxefi" kinda commands in the GRUB upstream. Daniel ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] efi/tpm: Fix memory leak in grub_tpm1/2_log_event()
On Mon, Mar 23, 2020 at 07:22:43PM +0100, Javier Martinez Canillas wrote: > Hello Tianjia, > > On 3/23/20 12:52 PM, Tianjia Zhang wrote: > > The memory requested for the event is not released here, > > causing memory leaks. This patch fixes this problem. > > > > Signed-off-by: Jia Zhang > > Signed-off-by: Tianjia Zhang > > --- > > Reviewed-by: Javier Martinez Canillas Reviewed-by: Daniel Kiper Daniel ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] Fix GRUB i386-pc build with Ubuntu gcc
On Tue, Mar 24, 2020 at 01:29:12PM +, Simon Hardy wrote: > With recent versions of gcc on Ubuntu a very large lzma_decompress.img file is > output. (e.g. 134479600 bytes instead of 2864.) This causes grub-mkimage to > fail with: "error: Decompressor is too big." > > This seems to be caused by a section .note.gnu.property that is placed at an > offset such that objcopy needs to pad the img file with zeros. > > This issue is present on: > Ubuntu 19.10 with gcc (Ubuntu 8.3.0-26ubuntu1~19.10) 8.3.0 > Ubuntu 19.10 with gcc (Ubuntu 9.2.1-9ubuntu2) 9.2.1 20191008 > > This issue is not present on: > Ubuntu 19.10 with gcc (Ubuntu 7.5.0-3ubuntu1~19.10) 7.5.0 > RHEL 8.0 with gcc 8.3.1 20190507 (Red Hat 8.3.1-4) > > The issue can be fixed by removing the section using objcopy as shown in this > patch: > > Signed-off-by: Simon Hardy Reviewed-by: Daniel Kiper Daniel ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH v3 0/5] Support Argon2 KDF in LUKS2
Hi Patrick, On Tue, Mar 10, 2020 at 07:58:27PM +0100, Patrick Steinhardt wrote: > Hi, > > this is the third version of my patchset to support the Argon2 KDF in > LUKS2. The following things have changed in comparison to v2: > > - Improved the GRUB_UINT_C macros to not use `elif 1` and fixed > indentation. > > - Dropped the upstreamed patch to fix a missing newline. > > - Reworked how we allocate memory on EFI. Previously, we always > targeted to acquire 1/4 of available memory. Now we're always > trying to allocate MAX_HEAP_SIZE (1.6GB) but clamp it to at most > 1/2 of available memory and at least MIN_HEAP_SIZE (100MB). > > So especially the last part is the interesting one. I _think_ that it's > roughly what Leif had in mind, but please do correct me if I'm wrong. So, according to our earlier discussion I am not treating this patchset as a release material. However, I would like to merge fixed version of it after the release. In the meantime I am investigating the legal matter related to this patchset. Daniel ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH 0/7] ZFS/other CoW FS save_env support
Hi Paul, On Wed, Mar 11, 2020 at 10:37:08AM -0700, Paul Dagnelie wrote: > Hey all, I previously discussed my concept for this patch in my email > https://lists.gnu.org/archive/html/grub-devel/2020-01/msg4.html . > I'm pleased to announce that I've gotten it into a working state and > it is ready to review. There are a number of changes here, which I > will break down below. > > First, the concept of "special files" is introduced into grub. These > are files that are manipulated using different functions from the > remainder of the filesystem. A filesystem advertises its support for > these files by filling in new entries in the grub_fs struct. > > Second, the loadenv command was modified to use the special file > functions of the root filesystem if they are detected. If that > happens, these functions are used to open, read, and write to the > loadenv file instead of the normal grub file functions. A variable > was also added that allows the user to force a file to be used instead > of the special files, or vice versa. > > Third, the grub-editenv command was modified to teach it to probe the > root filesystem and then check in an array of structures that contain > functions that will read and modify the filesystem's special file in > userland. These functions are very similar to normal read and write > functions, and so don't result in a very different code flow. > > Finally, the GRUB ZFS logic was modified to have new special_file > functions. These functions manipulate the pad2 area of the ZFS label, > which was previously unused. It now stores a version number, the raw > contents of the grubenv file, and an embedded checksum for integrity > purposes. GRUB was taught to read and verify these areas, and also to > modify them, computing the embeddded checksum appropriately. In > addition, if GRUB is build with libzfs support, an entry is added to > the grub-editenv table that points GRUB to the appropriate libzfs > functions, and init and fini functions are built into grub-editenv > itself. > > Future work: > * In the ZFS code could store a packed nvlist instead of a raw file, > but this would involve further changes to the grub environment file > code and was deferred for when it would be more useful and important. > * For the special files code, there is only one special file allowed > per filesystem, but a specification interface (using either an enum or > a set of names) could be added in the future. > * Other filesystem support was not built into this change, but > extensibility was a primary goal, so adding support for btrfs or other > filesystems should be relatively straightforward. > * I did not implement the proposed UEFI variable support, but I did > develop this with that future in mind, so adding it should be a > relatively simple extension of these changes. > > This patchset has been tested on systems with and without ZFS as the boot > filesystem, and built with and without ZFS libraries installed. It > worked in each case I tried, including with manual corruption applied > to the ZFS labels to force GRUB to read from the other label. This > was tested in conjunction with > https://github.com/zfsonlinux/zfs/pull/10009 , the ZoL patch that > enables ZFS to read from and write to the padding areas we reserved > for the data. First of all I would like to thank you for doing this work. This is very interesting feature. I went quickly through the code and it looks quite nice. However, I realized that it touches a lot of critical places in the GRUB code. ...and we are in freeze right now. So, at this point I think it is dangerous to merge that code. Simply we can make a lot of fallout which we may not be able to catch and clear before the release. Hence, I would like to suggest to defer this work until the release. Sorry about that but I do not want to risk GRUB code breakage at this point. I hope this is not a problem for you. Daniel ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: disk/mdraid1x_linux.c:181:15: warning: array subscript ...
Dear PGNet Dev, Am 25.03.20 um 16:54 schrieb PGNet Dev: On 3/25/20 8:52 AM, Paul Menzel wrote: Thanks, but please follow the mailing list netiquette I was responding to a question asked by a developer in THIS thread Yes, the question was, if the reported(?) build issues were fixed, and you replied to them. But, you new build error, which I quoted, should be discussed and fixed in a separate thread with the correct descriptive subject line. Kind regards, Paul ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: disk/mdraid1x_linux.c:181:15: warning: array subscript ...
On 3/25/20 8:52 AM, Paul Menzel wrote: > Thanks, but please follow the mailing list netiquette I was responding to a question asked by a developer in THIS thread ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: disk/mdraid1x_linux.c:181:15: warning: array subscript ...
Dear PGNet Dev, Am 25.03.20 um 16:35 schrieb PGNet Dev: […] with your patches, no immediate mdraid1x or zfs build errors one does surface, now, for ntfscomp ... […] Thanks, but please follow the mailing list netiquette, and start a new discussion thread with a descriptive subject line. Kind regards, Paul ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: disk/mdraid1x_linux.c:181:15: warning: array subscript ...
On 3/25/20 12:27 AM, Michael Chang wrote:
> It would be great if you can help to test patch to solve the build
> problem for gcc-10 in your system or not.
with your patches, no immediate mdraid1x or zfs build errors
one does surface, now, for ntfscomp ...
gcc --version
gcc (SUSE Linux) 10.0.1 20200320 (experimental) [revision
7d4549b2cd209eb621453ce13be7ffd84ffa720a]
cd grub
git log -n1
1 commit 552c9fd08122a3036c724ce96dfe68aa2f75705f (HEAD -> master,
origin/master, origin/HEAD)
2 Author: Patrick Steinhardt
3 Date: Sat Mar 7 17:29:09 2020 +0100
patch -p1 <
/tmp/grub_patches/0001-mdraid1x_linux-Fix-gcc10-error-Werror-array-bounds.patch
patching file grub-core/disk/mdraid1x_linux.c
patch -p1 <
/tmp/grub_patches/0002-zfs-Fix-gcc10-error-Werror-zero-length-bounds.patch
patching file grub-core/fs/zfs/zfs.c
patching file include/grub/zfs/zap_leaf.h
unset CC CPP
./bootstrap
./autogen.sh
./configure
make V=1 -j${CORES}
...
make[2]: Entering directory '/usr/local/src/grub'
gcc -DHAVE_CONFIG_H -I. -Wall -W -DGRUB_UTIL=1 -D_FILE_OFFSET_BITS=64
-I./include -DGRUB_FILE=\"grub-core/fs/ntfscomp.c\" -I. -I. -I. -I. -I./include
-I./include -I./grub-core/lib/libgcrypt-grub/src/ -I./grub-core/lib/minilzo
-I./grub-core/lib/xzembed -I./grub-core/lib/zstd -DMINILZO_HAVE_CONFIG_H -O3
-Wall -fstack-protector-strong -funwind-tables -fasynchronous-unwind-tables
-fmessage-length=0 -grecord-gcc-switches -march=native -mtune=native
-D_FORTIFY_SOURCE=2 -D_FILE_OFFSET_BITS=64 -Wall -W -Wshadow -Wpointer-arith
-Wundef -Wchar-subscripts -Wcomment -Wdeprecated-declarations
-Wdisabled-optimization -Wdiv-by-zero -Wfloat-equal -Wformat-extra-args
-Wformat-security -Wformat-y2k -Wimplicit -Wimplicit-function-declaration
-Wimplicit-int -Wmain -Wmissing-braces -Wmissing-format-attribute -Wmultichar
-Wparentheses -Wreturn-type -Wsequence-point -Wshadow -Wsign-compare -Wswitch
-Wtrigraphs -Wunknown-pragmas -Wunused -Wunused-function -Wunused-label
-Wunused-parameter -Wunused-value -Wunused-variable -Wwrite-strings
-Wnested-externs -Wstrict-prototypes -Wcast-align -Wextra -Wattributes
-Wendif-labels -Winit-self -Wint-to-pointer-cast -Winvalid-pch
-Wmissing-field-initializers -Wnonnull -Woverflow -Wvla -Wpointer-to-int-cast
-Wstrict-aliasing -Wvariadic-macros -Wvolatile-register-var -Wpointer-sign
-Wmissing-include-dirs -Wmissing-prototypes -Wmissing-declarations -Wformat=2
-Werror -fno-builtin -Wno-undef -O3 -Wall -fstack-protector-strong
-funwind-tables -fasynchronous-unwind-tables -fmessage-length=0
-grecord-gcc-switches -march=native -mtune=native -MT
grub-core/fs/libgrubmods_a-ntfscomp.o -MD -MP -MF
grub-core/fs/.deps-util/libgrubmods_a-ntfscomp.Tpo -c -o
grub-core/fs/libgrubmods_a-ntfscomp.o `test -f 'grub-core/fs/ntfscomp.c' ||
echo './'`grub-core/fs/ntfscomp.c
grub-core/fs/ntfscomp.c: In function ‘read_block’:
grub-core/fs/ntfscomp.c:82:11: error: ‘flg’ may be used uninitialized
in this function [-Werror=maybe-uninitialized]
82 | if (flg & 0x8000)
| ^~~
grub-core/fs/ntfscomp.c:74:17: note: ‘flg’ was declared here
74 | grub_uint16_t flg, cnt;
| ^~~
cc1: all warnings being treated as errors
make[2]: *** [Makefile:7647: grub-core/fs/libgrubmods_a-ntfscomp.o]
Error 1
make[2]: Leaving directory '/usr/local/src/grub'
make[1]: *** [Makefile:11920: all-recursive] Error 1
make[1]: Leaving directory '/usr/local/src/grub'
make: *** [Makefile:3772: all] Error 2
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
Re: disk/mdraid1x_linux.c:181:15: warning: array subscript ...
Hi,
Michael Chang wrote in patch 2/2:
> ../../grub-core/fs/zfs/zfs.c:2263:44: error: array subscript '' is
> outside the bounds of an interior zero-length array 'grub_uint16_t[0]' {aka
> 'short unsigned int[0]'} [-Werror=zero-length-bounds]
> [...]
> The l_hash[0] is apparnetly an interior member to the enclosed structure
> [...] the l_entries[0] is used to get proper alignment to access leaf chunks
That's what i call a dirty hack.
I wonder what gcc would say to a union of [0]-sized arrays as last
member of the struct type:
struct typedef struct zap_leaf_phys {
...
union {
grub_uint16_t l_hash[0];
grub_properly_aligned_t l_entries[0];
} l_;
} zap_leaf_phys_t;
Mine likes such a gesture, but it is way behind in respect to modern bitrot.
So in the end, your patch looks like the solid and unambiguous way to
implement what is desired.
> It would be great if you can help to test patch to solve the build
> problem for gcc-10 in your system or not.
Due to lack of modernity i can only contribute statements that the
concepts and motivations of your two patches look good to me.
(I also lack the occasion to test the two use cases.
Just lurking here for grub-mkrescue issues, where i provide the last stage
of packing up the ISO image.)
Have a nice day :)
Thomas
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
Re: disk/mdraid1x_linux.c:181:15: warning: array subscript ...
On Tue, Mar 24, 2020 at 06:54:43PM +0100, Thomas Schmitt wrote:
> Hi,
>
> i wrote:
> > >(char *) _roles - (char *) sb
> > >+ grub_le_to_cpu32 (sb.dev_number) * sizeof(grub_uint16_t)
>
> PGNet Dev wrote:
> > grub-core/disk/mdraid1x_linux.c:183:6: error: cannot convert to a
> > pointer type
>
> My fault. I forgot the "&" before "sb".
>
> (char *) _roles - (char *)
>
> I invested time in examining the C riddle, not in testing my proposal
> by at least some dummy.
>
> Now this compiles for me without complaint by gcc -Wall
>
> struct {
>char a_array[10];
>uint16_t dev_roles[0];
> } sb;
>
> printf("%u\n", (unsigned int) (((char *) _roles - (char *) )
> + 2 * sizeof(uint16_t)));
>
> Running this program yields 14 as result. The same as with the equivalent
> of the old expression
>
> printf("%u\n", (unsigned int) ((char *) _roles[2] - (char *) ));
>
>
> > not sure I'm reading your intent from your post,
>
> My observation is that not "dev_roles[0]" is to blame for the warning, but
> rather the computation which involves taking the address of an array
> element while not a single one is allocated.
> The resulting number is used as offset in a file, not in the sparsely
> allocated "struct grub_raid_super_1x sb".
>
> My proposal is to avoid "[...]" in the course of the computation.
> This should be valid for both ways to express an open ended struct:
> "dev_roles[0]" and "dev_roles[]".
I am also investigating the GCC 10 build problems, and my conclusion
mostly coincided with yours. There we can replce the offset computation
with pointer arithmetic like this.
(char *) (sb.dev_roles + grub_le_to_cpu32 (sb.dev_number)) - (char *) ,
Besides, there is also build error in zfs that I managed to come up with
a patch as well. I attached both my patch here for your refernce.
It would be great if you can help to test patch to solve the build
problem for gcc-10 in your system or not.
Thanks,
Michael
>
>
> Have a nice day :)
>
> Thomas
>
>
> ___
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
>From 19f55c05ea592b1339ee0d503c86be349447553f Mon Sep 17 00:00:00 2001
From: Michael Chang
Date: Wed, 25 Mar 2020 13:52:51 +0800
Subject: [PATCH 1/2] mdraid1x_linux: Fix gcc10 error -Werror=array-bounds
We bumped into the build error while testing gcc-10 pre-release.
../../grub-core/disk/mdraid1x_linux.c: In function 'grub_mdraid_detect':
../../grub-core/disk/mdraid1x_linux.c:181:15: error: array subscript is outside array bounds of 'grub_uint16_t[0]' {aka 'short unsigned int[0]'} [-Werror=array-bounds]
181 | (char *) _roles[grub_le_to_cpu32 (sb.dev_number)]
| ^~~
../../grub-core/disk/mdraid1x_linux.c:98:17: note: while referencing 'dev_roles'
98 | grub_uint16_t dev_roles[0]; /* Role in array, or 0x for a spare, or 0xfffe for faulty. */
| ^
../../grub-core/disk/mdraid1x_linux.c:127:33: note: defined here 'sb'
127 | struct grub_raid_super_1x sb;
| ^~
cc1: all warnings being treated as errors
Apparently gcc issues the warning when trying to access sb.dev_roles
array's member, since it is a zero length array as the last element of
struct grub_raid_super_1x that is allocated sparsely without extra
chunks for the trailing bits, so the warning looks legitimate in this
regard.
As the whole thing here is doing offset computation, it is undue to use
syntax that would imply array member access then take address from it
later. Instead we could accomplish the same thing through basic array
pointer arithmetic to pacify the warning.
Signed-off-by: Michael Chang
---
grub-core/disk/mdraid1x_linux.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/grub-core/disk/mdraid1x_linux.c b/grub-core/disk/mdraid1x_linux.c
index 7cc80d3df..c980feba4 100644
--- a/grub-core/disk/mdraid1x_linux.c
+++ b/grub-core/disk/mdraid1x_linux.c
@@ -178,7 +178,7 @@ grub_mdraid_detect (grub_disk_t disk,
return NULL;
if (grub_disk_read (disk, sector,
- (char *) _roles[grub_le_to_cpu32 (sb.dev_number)]
+ (char *) (sb.dev_roles + grub_le_to_cpu32 (sb.dev_number))
- (char *) ,
sizeof (role), ))
return NULL;
--
2.16.4
>From 8666468ac1a35f0678672de5c89a3f062d1aeb39 Mon Sep 17 00:00:00 2001
From: Michael Chang
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 '' is outside the bounds of an interior zero-length array 'grub_uint16_t[0]' {aka
