Re: gcc9.1 and f2fs
Hello John,
Except for lacking commit message and SOB, the fix is LGTM.
You could add my Reviewed-by: Michael Chang if you
resend the patch to mailing list with those missing message fixed.
Thanks,
Michael
On Mon, May 06, 2019 at 07:59:42PM +0200, John Paul Adrian Glaubitz wrote:
> On 5/6/19 7:15 PM, Neil MacLeod wrote:
> > Does anyone have a patch?
>
> Try the attached patch.
>
> Adrian
>
> --
> .''`. John Paul Adrian Glaubitz
> : :' : Debian Developer - glaub...@debian.org
> `. `' Freie Universitaet Berlin - glaub...@physik.fu-berlin.de
> `-GPG: 62FF 8A75 84E0 2956 9546 0006 7426 3B37 F5B5 F913
> >From b0d03dd5605220d0d23d72e662f10ce7b02a54a6 Mon Sep 17 00:00:00 2001
> From: John Paul Adrian Glaubitz
> Date: Mon, 6 May 2019 19:55:01 +0200
> Subject: [PATCH] f2fs: Disable gcc9 -Waddress-of-packed-member
>
> ---
> grub-core/fs/f2fs.c | 10 ++
> 1 file changed, 10 insertions(+)
>
> diff --git a/grub-core/fs/f2fs.c b/grub-core/fs/f2fs.c
> index 1cad2615f..0dd09bc23 100644
> --- a/grub-core/fs/f2fs.c
> +++ b/grub-core/fs/f2fs.c
> @@ -1235,6 +1235,12 @@ grub_f2fs_utf16_to_utf8 (grub_uint16_t *in_buf_le)
>return out_buf;
> }
>
> +
> +#if __GNUC__ >= 9
> +#pragma GCC diagnostic push
> +#pragma GCC diagnostic ignored "-Waddress-of-packed-member"
> +#endif
> +
> static grub_err_t
> grub_f2fs_label (grub_device_t device, char **label)
> {
> @@ -1255,6 +1261,10 @@ grub_f2fs_label (grub_device_t device, char **label)
>return grub_errno;
> }
>
> +#if __GNUC__ >= 9
> +#pragma GCC diagnostic pop
> +#endif
> +
> static grub_err_t
> grub_f2fs_uuid (grub_device_t device, char **uuid)
> {
> --
> 2.20.1
>
> ___
> 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: gcc9.1 and f2fs
On Wed, May 08, 2019 at 01:51:20AM +0100, Neil MacLeod wrote: > Adrian > > I used the attached patch and grub is now building for me with gcc-9.1. > > I've no idea if this is the right solution, but it does at least work > (ie. builds) which is all I require for now as I'm not personally > using grub in the distribution I'm building (LibreELEC) and can't > actually say if it works at run-time. Hi Neil, The gcc diagnose here is right and addressed a real problem IMHO, so we can't suppress it but have to fix the real problem it has pointed out. I will try to come up with a patch and post it to mailing list. Thanks, Michael > > Many thanks > Neil > > > On Mon, 6 May 2019 at 20:58, Neil MacLeod wrote: > > > > Hi Adrian > > > > With your patch, grub now fails to build as follows: > > > > fs/f2fs.c: In function 'grub_f2fs_get_block': > > fs/f2fs.c:864:43: error: 'offset' may be used uninitialized in this > > function [-Werror=maybe-uninitialized] > > 864 | return grub_le_to_cpu32 (inode->i_addr[offset[0]]); > > | ^ > > cc1: all warnings being treated as errors > > make[4]: *** [Makefile:33773: fs/f2fs_module-f2fs.o] Error 1 > > > > This is the last 1000 lines of the build log with your patch applied: > > http://ix.io/1IfE > > > > And this is the f2fs.c used for the compilation (it includes your > > patch): http://ix.io/1IfC > > > > Many thanks > > Neil > > > > On Mon, 6 May 2019 at 19:19, Neil MacLeod wrote: > > > > > > Thanks Adrian - testing with your patch now, will be another hour or > > > so until the build is complete (cross-compiling environment with own > > > toolchain etc.) > > > > > > Just a correction, I'm testing with > > > 4dd4ceec023111a4ccf69f8de6fa0885c6847a35 (current HEAD) - looks like I > > > cut & pasted a rev from a different repo in my original post! > > > > > > Regards > > > Neil > > > > > > On Mon, 6 May 2019 at 18:59, John Paul Adrian Glaubitz > > > wrote: > > > > > > > > On 5/6/19 7:15 PM, Neil MacLeod wrote: > > > > > Does anyone have a patch? > > > > > > > > Try the attached patch. > > > > > > > > Adrian > > > > > > > > -- > > > > .''`. John Paul Adrian Glaubitz > > > > : :' : Debian Developer - glaub...@debian.org > > > > `. `' Freie Universitaet Berlin - glaub...@physik.fu-berlin.de > > > > `-GPG: 62FF 8A75 84E0 2956 9546 0006 7426 3B37 F5B5 F913 > ___ > 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
[PATCH] f2fs: Fix gcc9 error -Werror=maybe-uninitialized
The function grub_get_node_path could return uninitialized offset with
level == 0 if the block is greater than direct_index + 2*direct_blks +
2*indirect_blks + dindirect_blks. The uninitialized offset is then used
by function grub_f2fs_get_block because level == 0 is valid and
meaningful return to be processed.
The fix is to set level = -1 as return value by grub_get_node_path to
signify an error that the input block cannot be handled. Any caller
should therefore check level is negative or not before processing the
output.
Reported-by: Neil MacLeod
Signed-off-by: Michael Chang
---
grub-core/fs/f2fs.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/grub-core/fs/f2fs.c b/grub-core/fs/f2fs.c
index 644653dbe..bb28b291b 100644
--- a/grub-core/fs/f2fs.c
+++ b/grub-core/fs/f2fs.c
@@ -702,7 +702,7 @@ grub_get_node_path (struct grub_f2fs_inode *inode,
grub_uint32_t block,
grub_uint32_t dindirect_blks = indirect_blks * NIDS_PER_BLOCK;
grub_uint32_t direct_index = DEF_ADDRS_PER_INODE;
int n = 0;
- int level = 0;
+ int level = -1;
if (inode->i_inline & F2FS_INLINE_XATTR)
direct_index -= F2FS_INLINE_XATTR_ADDRS;
@@ -712,6 +712,7 @@ grub_get_node_path (struct grub_f2fs_inode *inode,
grub_uint32_t block,
if (block < direct_index)
{
offset[n] = block;
+ level = 0;
goto got;
}
@@ -860,6 +861,10 @@ grub_f2fs_get_block (grub_fshelp_node_t node,
grub_disk_addr_t block_ofs)
int level, i;
level = grub_get_node_path (inode, block_ofs, offset, noffset);
+
+ if (level < 0)
+return -1;
+
if (level == 0)
return grub_le_to_cpu32 (inode->i_addr[offset[0]]);
--
2.16.4
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
Re: gcc9.1 and f2fs
Hi Michael! On 5/17/19 9:13 AM, Michael Chang wrote: > Except for lacking commit message and SOB, the fix is LGTM. > > You could add my Reviewed-by: Michael Chang if you > resend the patch to mailing list with those missing message fixed. I can do that later today. I'm currently busy with work (I'm also @suse.com ;)). Adrian -- .''`. John Paul Adrian Glaubitz : :' : Debian Developer - glaub...@debian.org `. `' Freie Universitaet Berlin - glaub...@physik.fu-berlin.de `-GPG: 62FF 8A75 84E0 2956 9546 0006 7426 3B37 F5B5 F913 ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] f2fs: Fix gcc9 error -Werror=maybe-uninitialized
CC-ing n...@nmacleod.com
Michael, thank you for posting the patch.
Neil, does it solve your problem?
Daniel
On Fri, May 17, 2019 at 05:00:19PM +0800, Michael Chang wrote:
> The function grub_get_node_path could return uninitialized offset with
> level == 0 if the block is greater than direct_index + 2*direct_blks +
> 2*indirect_blks + dindirect_blks. The uninitialized offset is then used
> by function grub_f2fs_get_block because level == 0 is valid and
> meaningful return to be processed.
>
> The fix is to set level = -1 as return value by grub_get_node_path to
> signify an error that the input block cannot be handled. Any caller
> should therefore check level is negative or not before processing the
> output.
>
> Reported-by: Neil MacLeod
> Signed-off-by: Michael Chang
> ---
> grub-core/fs/f2fs.c | 7 ++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/grub-core/fs/f2fs.c b/grub-core/fs/f2fs.c
> index 644653dbe..bb28b291b 100644
> --- a/grub-core/fs/f2fs.c
> +++ b/grub-core/fs/f2fs.c
> @@ -702,7 +702,7 @@ grub_get_node_path (struct grub_f2fs_inode *inode,
> grub_uint32_t block,
>grub_uint32_t dindirect_blks = indirect_blks * NIDS_PER_BLOCK;
>grub_uint32_t direct_index = DEF_ADDRS_PER_INODE;
>int n = 0;
> - int level = 0;
> + int level = -1;
>
>if (inode->i_inline & F2FS_INLINE_XATTR)
> direct_index -= F2FS_INLINE_XATTR_ADDRS;
> @@ -712,6 +712,7 @@ grub_get_node_path (struct grub_f2fs_inode *inode,
> grub_uint32_t block,
>if (block < direct_index)
> {
>offset[n] = block;
> + level = 0;
>goto got;
> }
>
> @@ -860,6 +861,10 @@ grub_f2fs_get_block (grub_fshelp_node_t node,
> grub_disk_addr_t block_ofs)
>int level, i;
>
>level = grub_get_node_path (inode, block_ofs, offset, noffset);
> +
> + if (level < 0)
> +return -1;
> +
>if (level == 0)
> return grub_le_to_cpu32 (inode->i_addr[offset[0]]);
>
> --
> 2.16.4
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] f2fs: Fix gcc9 error -Werror=maybe-uninitialized
Hi Daniel & Michael
I've tested the new patch (in place of my attempt) and grub is
building successfully so it looks good here!
Many thanks!
Neil
On Fri, 17 May 2019 at 13:26, Daniel Kiper wrote:
>
> CC-ing n...@nmacleod.com
>
> Michael, thank you for posting the patch.
>
> Neil, does it solve your problem?
>
> Daniel
>
> On Fri, May 17, 2019 at 05:00:19PM +0800, Michael Chang wrote:
> > The function grub_get_node_path could return uninitialized offset with
> > level == 0 if the block is greater than direct_index + 2*direct_blks +
> > 2*indirect_blks + dindirect_blks. The uninitialized offset is then used
> > by function grub_f2fs_get_block because level == 0 is valid and
> > meaningful return to be processed.
> >
> > The fix is to set level = -1 as return value by grub_get_node_path to
> > signify an error that the input block cannot be handled. Any caller
> > should therefore check level is negative or not before processing the
> > output.
> >
> > Reported-by: Neil MacLeod
> > Signed-off-by: Michael Chang
> > ---
> > grub-core/fs/f2fs.c | 7 ++-
> > 1 file changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/grub-core/fs/f2fs.c b/grub-core/fs/f2fs.c
> > index 644653dbe..bb28b291b 100644
> > --- a/grub-core/fs/f2fs.c
> > +++ b/grub-core/fs/f2fs.c
> > @@ -702,7 +702,7 @@ grub_get_node_path (struct grub_f2fs_inode *inode,
> > grub_uint32_t block,
> >grub_uint32_t dindirect_blks = indirect_blks * NIDS_PER_BLOCK;
> >grub_uint32_t direct_index = DEF_ADDRS_PER_INODE;
> >int n = 0;
> > - int level = 0;
> > + int level = -1;
> >
> >if (inode->i_inline & F2FS_INLINE_XATTR)
> > direct_index -= F2FS_INLINE_XATTR_ADDRS;
> > @@ -712,6 +712,7 @@ grub_get_node_path (struct grub_f2fs_inode *inode,
> > grub_uint32_t block,
> >if (block < direct_index)
> > {
> >offset[n] = block;
> > + level = 0;
> >goto got;
> > }
> >
> > @@ -860,6 +861,10 @@ grub_f2fs_get_block (grub_fshelp_node_t node,
> > grub_disk_addr_t block_ofs)
> >int level, i;
> >
> >level = grub_get_node_path (inode, block_ofs, offset, noffset);
> > +
> > + if (level < 0)
> > +return -1;
> > +
> >if (level == 0)
> > return grub_le_to_cpu32 (inode->i_addr[offset[0]]);
> >
> > --
> > 2.16.4
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
Re: gcc9.1 and f2fs
On 5/17/19 2:13 AM, Michael Chang wrote:
Hello John,
Except for lacking commit message and SOB, the fix is LGTM.
[snip]
---
grub-core/fs/f2fs.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/grub-core/fs/f2fs.c b/grub-core/fs/f2fs.c
index 1cad2615f..0dd09bc23 100644
--- a/grub-core/fs/f2fs.c
+++ b/grub-core/fs/f2fs.c
@@ -1235,6 +1235,12 @@ grub_f2fs_utf16_to_utf8 (grub_uint16_t *in_buf_le)
return out_buf;
}
+
+#if __GNUC__ >= 9
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Waddress-of-packed-member"
+#endif
+
static grub_err_t
grub_f2fs_label (grub_device_t device, char **label)
{
@@ -1255,6 +1261,10 @@ grub_f2fs_label (grub_device_t device, char **label)
return grub_errno;
}
+#if __GNUC__ >= 9
+#pragma GCC diagnostic pop
+#endif
+
static grub_err_t
grub_f2fs_uuid (grub_device_t device, char **uuid)
{
I would like to point out that it is not a good idea to make a stable
release using -Werror as a default. It is fine for development, but
grub has traditionally taken a long time between stable releases
(version 2.00 was in 2012 and version 2.02 was in 2017). After
release, a new version of a compiler has a high probability of producing
more warnings, which will break the build if using -Werrror.
At linuxfromscratch we add --disable-werror, but it really shouldn't be
needed.
-- Bruce
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
