[PATCH] multiboot2: enable quirk-modules-after-kernel
In multiboot2, currently there is no way to control where to load the
modules. In case of user wants to reserve low address for specific
usage, this quirk is useful.
Signed-off-by: Zide Chen
---
grub-core/loader/i386/multiboot_mbi.c | 1 -
grub-core/loader/multiboot.c | 23 ---
grub-core/loader/multiboot_elfxx.c| 7 ---
grub-core/loader/multiboot_mbi2.c | 2 ++
include/grub/multiboot.h | 1 +
include/grub/multiboot2.h | 8
6 files changed, 27 insertions(+), 15 deletions(-)
diff --git a/grub-core/loader/i386/multiboot_mbi.c
b/grub-core/loader/i386/multiboot_mbi.c
index ad3cc292fd18..18aaac847011 100644
--- a/grub-core/loader/i386/multiboot_mbi.c
+++ b/grub-core/loader/i386/multiboot_mbi.c
@@ -63,7 +63,6 @@ static int bootdev_set;
static grub_size_t elf_sec_num, elf_sec_entsize;
static unsigned elf_sec_shstrndx;
static void *elf_sections;
-grub_multiboot_quirks_t grub_multiboot_quirks;
static grub_err_t
load_kernel (grub_file_t file, const char *filename,
diff --git a/grub-core/loader/multiboot.c b/grub-core/loader/multiboot.c
index 4a98d7082598..f9f66378dfb2 100644
--- a/grub-core/loader/multiboot.c
+++ b/grub-core/loader/multiboot.c
@@ -32,6 +32,8 @@
#include
#define GRUB_MULTIBOOT_CONSOLE_FRAMEBUFFER GRUB_MULTIBOOT2_CONSOLE_FRAMEBUFFER
#define GRUB_MULTIBOOT_CONSOLE_EGA_TEXT GRUB_MULTIBOOT2_CONSOLE_EGA_TEXT
+#define GRUB_MULTIBOOT_QUIRKS_NONE GRUB_MULTIBOOT2_QUIRKS_NONE
+#define GRUB_MULTIBOOT_QUIRK_MODULES_AFTER_KERNEL
GRUB_MULTIBOOT2_QUIRK_MODULES_AFTER_KERNEL
#define GRUB_MULTIBOOT(x) grub_multiboot2_ ## x
#else
#include
@@ -210,7 +212,8 @@ grub_multiboot_unload (void)
return GRUB_ERR_NONE;
}
-static grub_uint64_t highest_load;
+GRUB_MULTIBOOT (quirks_t) GRUB_MULTIBOOT (quirks);
+grub_uint64_t GRUB_MULTIBOOT (highest_load);
#define MULTIBOOT_LOAD_ELF64
#include "multiboot_elfxx.c"
@@ -291,32 +294,32 @@ grub_cmd_multiboot (grub_command_t cmd __attribute__
((unused)),
grub_loader_unset ();
- highest_load = 0;
+ GRUB_MULTIBOOT (highest_load) = 0;
-#ifndef GRUB_USE_MULTIBOOT2
- grub_multiboot_quirks = GRUB_MULTIBOOT_QUIRKS_NONE;
+ GRUB_MULTIBOOT (quirks) = GRUB_MULTIBOOT_QUIRKS_NONE;
int option_found = 0;
do
{
option_found = 0;
+#ifndef GRUB_USE_MULTIBOOT2
if (argc != 0 && grub_strcmp (argv[0], "--quirk-bad-kludge") == 0)
{
argc--;
argv++;
option_found = 1;
- grub_multiboot_quirks |= GRUB_MULTIBOOT_QUIRK_BAD_KLUDGE;
+ GRUB_MULTIBOOT (quirks) |= GRUB_MULTIBOOT_QUIRK_BAD_KLUDGE;
}
+#endif
if (argc != 0 && grub_strcmp (argv[0], "--quirk-modules-after-kernel")
== 0)
{
argc--;
argv++;
option_found = 1;
- grub_multiboot_quirks |= GRUB_MULTIBOOT_QUIRK_MODULES_AFTER_KERNEL;
+ GRUB_MULTIBOOT (quirks) |= GRUB_MULTIBOOT_QUIRK_MODULES_AFTER_KERNEL;
}
} while (option_found);
-#endif
if (argc == 0)
return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("filename expected"));
@@ -392,11 +395,9 @@ grub_cmd_module (grub_command_t cmd __attribute__
((unused)),
if (! file)
return grub_errno;
-#ifndef GRUB_USE_MULTIBOOT2
lowest_addr = 0x10;
- if (grub_multiboot_quirks & GRUB_MULTIBOOT_QUIRK_MODULES_AFTER_KERNEL)
-lowest_addr = ALIGN_UP (highest_load + 1048576, 4096);
-#endif
+ if (GRUB_MULTIBOOT (quirks) & GRUB_MULTIBOOT_QUIRK_MODULES_AFTER_KERNEL)
+lowest_addr = ALIGN_UP (GRUB_MULTIBOOT (highest_load) + 1048576, 4096);
size = grub_file_size (file);
if (size)
diff --git a/grub-core/loader/multiboot_elfxx.c
b/grub-core/loader/multiboot_elfxx.c
index 70cd1db513e6..8adce7b3489b 100644
--- a/grub-core/loader/multiboot_elfxx.c
+++ b/grub-core/loader/multiboot_elfxx.c
@@ -89,17 +89,18 @@ CONCAT(grub_multiboot_load_elf, XX) (mbi_load_data_t *mld)
if (phdr(i)->p_type == PT_LOAD)
{
mld->link_base_addr = grub_min (mld->link_base_addr, phdr(i)->p_paddr);
- highest_load = grub_max (highest_load, phdr(i)->p_paddr +
phdr(i)->p_memsz);
+ GRUB_MULTIBOOT (highest_load) = grub_max (GRUB_MULTIBOOT (highest_load),
+ phdr(i)->p_paddr + phdr(i)->p_memsz);
}
#ifdef MULTIBOOT_LOAD_ELF64
- if (highest_load >= 0x1)
+ if (GRUB_MULTIBOOT(highest_load) >= 0x1)
return grub_error (GRUB_ERR_BAD_OS, "segment crosses 4 GiB border");
#endif
if (mld->relocatable)
{
- load_size = highest_load - mld->link_base_addr;
+ load_size = GRUB_MULTIBOOT(highest_load) - mld->link_base_addr;
grub_dprintf ("multiboot_loader", "align=0x%lx, preference=0x%x, "
"load_size=0x%x, avoid_efi_boot_services=%d\n",
diff --git a/grub-core/loader/multiboot_mbi2.c
b/grub-core/loader/multiboot_mbi2.c
index 18e766c7b31c..3883732df531 100644
--- a/grub-core/loader/multiboot_mbi2.c
+++ b/grub-core/loader/multiboot_mbi2.c
@@ -315,8
Re: [PATCH v1] fix kernel cmdline corruption
CC-ing Vladimir... On Thu, Mar 19, 2020 at 03:48:28PM +0800, Michael Chang wrote: > Hi Olaf, > > The patch rang a bell to me and eventually I figured out that I had > similar patch posted. > > https://lists.gnu.org/archive/html/grub-devel/2018-04/msg00038.html Yeah, it looks that it fell through the cracks... > In short, the issue is not only in the fix itself, but also to take care > and provide a measure to avoid incompatible grub.cfg before and after > the patch. > > To recap, there were two options provided to deal with the compatibilty > issue. > > 1. Introduce a shell variable to disable the change, whenever anything > goes wrong. > > 2. Use readonly feature variable similar to $feature_menuentry_id that > can be used to detect capability and apply settings accordingly, thus > still fulfill the requirement of one grub.cfg can work for all versions > IMHO. > > I am in favour of #2, but still they didn't seem to have upstream's > consent yet. Michael, please prepare a patch and CC Olaf and Vladimir on it. Daniel ___ 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 Wed, Mar 25, 2020 at 08:30:38PM +0100, Olaf Hering wrote: > 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. May I ask you to do the change for 404 and test it too? If it works please post a new patch. Daniel ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH 2/2] zfs: Fix gcc10 error -Werror=zero-length-bounds
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 '' 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
Reviewed-by: Daniel Kiper
Daniel
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
Re: fs/ntfscomp.c:82:11: error: ‘flg’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
On 3/26/20 1:39 AM, Paul Menzel wrote: > I am unable to reproduce this with 1st sanity re-check: I _am_ able to reproduce this consistently, with same error. I've tested now on multiple machines; not identical, but all similarly opensuse + GCC10 dev envs ... > Here is the code in question: snip > Why is it complaining complaining in line 82 and not 78, where `flg` is > already accessed? On 3/26/20 3:39 AM, Michael Chang wrote: > Looking into build log, the build option seems to have been overridden > with CFLAGS settings like this. > > CFLAGS="-O3 -Wall -fstack-protector-strong -funwind-tables > -fasynchronous-unwind-tables -fmessage-length=0 -grecord-gcc-switches > -march=native -mtune=native" > > I'm not sure if -O3 is considered as supported since that will result > in larger binaries we are striving to reduce all the time. Also the > optimization it brings would require careful review if we don't enable > it by default. > > In addition, -fstack-protector-strong breaks the build even harder with > a lot of __stack_chk_fail undefined symbol in the modules. > > If going with default build option, I also don't have this compliation > error. indeed. building with unset CC CPP + unset LD CFLAGS CPPFLAGS CXXFLAGS ./bootstrap ./autogen.sh ./configure \ --prefix=/usr/local/grub-build-test make V=1 completes without error, and installs, /usr/local/grub-build-test/bin/grub-mkrescue --version /usr/local/grub-build-test/bin/grub-mkrescue (GRUB) 2.05 which I can certainly manage easily enough for local build. > If going with default build option _is_ a 'clear' env expected/recommended/required for a grub2 build? if so, does this need to be handled at config time? in either case, this > Why is it complaining complaining in line 82 and not 78, where `flg` is > already accessed? doesn't make sense to me; code looks straightforward enough. the -03 optimization sounds a good 1st guess. also, Michael, fyi, I _think_ this^ started for me with linked GCC10 builds of grub2 on opensuse OBS ... where, iirc, the builds 1st failed, and I started testing locally. ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: fs/ntfscomp.c:82:11: error: ‘flg’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
On Thu, Mar 26, 2020 at 09:39:16AM +0100, Paul Menzel wrote: > Dear PGNet Dev, > > > Thank you for your report. > > > Am 26.03.20 um 04:50 schrieb PGNet Dev: > > 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 > > I am unable to reproduce this with > > $ gcc --version > gcc (Debian 10-20200324-1) 10.0.1 20200324 (experimental) [master > revision 596c90d3559:023579257f5:906b3eb9df6c577d3f6e9c3ea5c9d7e4d1e90536] Looking into build log, the build option seems to have been overridden with CFLAGS settings like this. CFLAGS="-O3 -Wall -fstack-protector-strong -funwind-tables -fasynchronous-unwind-tables -fmessage-length=0 -grecord-gcc-switches -march=native -mtune=native" I'm not sure if -O3 is considered as supported since that will result in larger binaries we are striving to reduce all the time. Also the optimization it brings would require careful review if we don't enable it by default. In addition, -fstack-protector-strong breaks the build even harder with a lot of __stack_chk_fail undefined symbol in the modules. If going with
Re: fs/ntfscomp.c:82:11: error: ‘flg’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
Dear PGNet Dev,
Thank you for your report.
Am 26.03.20 um 04:50 schrieb PGNet Dev:
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
I am unable to reproduce this with
$ gcc --version
gcc (Debian 10-20200324-1) 10.0.1 20200324 (experimental) [master
revision 596c90d3559:023579257f5:906b3eb9df6c577d3f6e9c3ea5c9d7e4d1e90536]
Here is the code in question:
``
static grub_err_t
decomp_get16 (struct grub_ntfs_comp *cc, grub_uint16_t * res)
{
grub_uint8_t c1 = 0, c2 = 0;
if ((decomp_getch (cc, )) || (decomp_getch (cc, )))
return grub_errno;
*res = ((grub_uint16_t) c2) * 256 + ((grub_uint16_t) c1);
return 0;
}
[…]
grub_uint16_t flg, cnt;
if (decomp_get16 (cc, ))
return grub_errno;
cnt = (flg & 0xFFF) + 1;
if (dest)
{
if (flg & 0x8000)
[…]
```
In the code, the `if (decomp_get16 (cc, ))` ensures, that `flg` is
initialized, doesn’t it?
Why is it complaining complaining in line 82 and not 78, where `flg` is
already accessed?
Kind regards,
Paul
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[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 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
---
grub-core/fs/zfs/zfs.c | 5 -
include/grub/zfs/zap_leaf.h | 1 -
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/grub-core/fs/zfs/zfs.c b/grub-core/fs/zfs/zfs.c
index 2f72e42bf..b5e10fd0b 100644
--- a/grub-core/fs/zfs/zfs.c
+++ b/grub-core/fs/zfs/zfs.c
@@ -141,7 +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
+ 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) * 2)
/ sizeof (grub_properly_aligned_t)))[idx];
}
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
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[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
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
