Re: Build regressions/improvements in v5.18-rc1

[Arnd Bergmann]

On Mon, Apr 4, 2022 at 12:19 PM Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> wrote:

> >
> > /kisskb/src/fs/xfs/./xfs_trace.h:432:2: note: in expansion of macro 'TP_printk'
> >   TP_printk("dev %d:%d daddr 0x%llx bbcount 0x%x hold %d pincount %d "
> >   ^
> > /kisskb/src/fs/xfs/./xfs_trace.h:440:5: note: in expansion of macro '__print_flags'
> >      __print_flags(__entry->flags, "|", XFS_BUF_FLAGS),
> >      ^
> > /kisskb/src/fs/xfs/xfs_buf.h:67:4: note: in expansion of macro 'XBF_UNMAPPED'
> >   { XBF_UNMAPPED,  "UNMAPPED" }
> >     ^
> > /kisskb/src/fs/xfs/./xfs_trace.h:440:40: note: in expansion of macro 'XFS_BUF_FLAGS'
> >      __print_flags(__entry->flags, "|", XFS_BUF_FLAGS),
> >                                         ^
> > /kisskb/src/fs/xfs/./xfs_trace.h: In function 'trace_raw_output_xfs_buf_flags_class':
> > /kisskb/src/fs/xfs/xfs_buf.h:46:23: error: initializer element is not constant
> >  #define XBF_UNMAPPED  (1 << 31)/* do not map the buffer */
> >
> > This doesn't make a whole lotta sense to me. It's blown up in a
> > tracepoint macro in XFS that was not changed at all in 5.18-rc1, nor
> > was any of the surrounding XFS code or contexts.  Perhaps something
> > outside XFS changed to cause this on these platforms?
>
> Upon closer look, all builds showing this issue are using gcc-5...
>
> > Can you bisect this, please?
>
> Fortunately I still have gcc-5 installed on an older machine,
> and I could reproduce the issue on amd64 with
> "make allmodconfig fs/xfs/xfs_trace.o".
>
> Bisection points to commit e8c07082a810fbb9 ("Kbuild: move to
> -std=gnu11").
>
> [1] gcc version 5.5.0 20171010 (Ubuntu 5.5.0-12ubuntu1

Thanks for the report. I've produced it and can see that the problem
is assigning
the value of "(1 << 31)" to an 'unsigned long' struct member. Since this is
a signed integer overflow, the result is technically undefined behavior,
which gcc-5 does not accept as an integer constant.

The patch below fixes it for me, but I have not checked if there are any
other instances. This could also be done using the 'BIT()' macro if the
XFS maintainers prefer:

diff --git a/fs/xfs/xfs_buf.h b/fs/xfs/xfs_buf.h
index edcb6254fa6a..762348973e8c 100644
--- a/fs/xfs/xfs_buf.h
+++ b/fs/xfs/xfs_buf.h
@@ -22,28 +22,28 @@ struct xfs_buf;

 #define XFS_BUF_DADDR_NULL     ((xfs_daddr_t) (-1LL))

-#define XBF_READ        (1 << 0) /* buffer intended for reading from device */
-#define XBF_WRITE       (1 << 1) /* buffer intended for writing to device */
-#define XBF_READ_AHEAD  (1 << 2) /* asynchronous read-ahead */
-#define XBF_NO_IOACCT   (1 << 3) /* bypass I/O accounting (non-LRU bufs) */
-#define XBF_ASYNC       (1 << 4) /* initiator will not wait for completion */
-#define XBF_DONE        (1 << 5) /* all pages in the buffer uptodate */
-#define XBF_STALE       (1 << 6) /* buffer has been staled, do not find it */
-#define XBF_WRITE_FAIL  (1 << 7) /* async writes have failed on this buffer */
+#define XBF_READ        (1ul << 0) /* buffer intended for reading
from device */
+#define XBF_WRITE       (1ul << 1) /* buffer intended for writing to device */
+#define XBF_READ_AHEAD  (1ul << 2) /* asynchronous read-ahead */
+#define XBF_NO_IOACCT   (1ul << 3) /* bypass I/O accounting (non-LRU bufs) */
+#define XBF_ASYNC       (1ul << 4) /* initiator will not wait for completion */
+#define XBF_DONE        (1ul << 5) /* all pages in the buffer uptodate */
+#define XBF_STALE       (1ul << 6) /* buffer has been staled, do not find it */
+#define XBF_WRITE_FAIL  (1ul << 7) /* async writes have failed on
this buffer */

 /* buffer type flags for write callbacks */
-#define _XBF_INODES     (1 << 16)/* inode buffer */
-#define _XBF_DQUOTS     (1 << 17)/* dquot buffer */
-#define _XBF_LOGRECOVERY        (1 << 18)/* log recovery buffer */
+#define _XBF_INODES     (1ul << 16)/* inode buffer */
+#define _XBF_DQUOTS     (1ul << 17)/* dquot buffer */
+#define _XBF_LOGRECOVERY        (1ul << 18)/* log recovery buffer */

 /* flags used only internally */
-#define _XBF_PAGES      (1 << 20)/* backed by refcounted pages */
-#define _XBF_KMEM       (1 << 21)/* backed by heap memory */
-#define _XBF_DELWRI_Q   (1 << 22)/* buffer on a delwri queue */
+#define _XBF_PAGES      (1ul << 20)/* backed by refcounted pages */
+#define _XBF_KMEM       (1ul << 21)/* backed by heap memory */
+#define _XBF_DELWRI_Q   (1ul << 22)/* buffer on a delwri queue */

 /* flags used only as arguments to access routines */
-#define XBF_TRYLOCK     (1 << 30)/* lock requested, but do not wait */
-#define XBF_UNMAPPED    (1 << 31)/* do not map the buffer */
+#define XBF_TRYLOCK     (1ul << 30)/* lock requested, but do not wait */
+#define XBF_UNMAPPED    (1ul << 31)/* do not map the buffer */

 typedef unsigned int xfs_buf_flags_t;