All of lore.kernel.org
 help / color / mirror / Atom feed
* Processed (with 1 error): Re: Bug#1000974: copy_move_algo.hpp:1083:10: error: ‘__fallthrough__’ was not declared in this scope; did you mean ‘fallthrough’?
       [not found]       ` <c7ccff50-c177-7f96-2d99-2077f77374ad@debian.org>
@ 2021-12-05 15:57         ` Debian Bug Tracking System
  2021-12-05 17:33         ` Thomas Goirand
  1 sibling, 0 replies; 14+ messages in thread
From: Debian Bug Tracking System @ 2021-12-05 15:57 UTC (permalink / raw)
  To: Giovanni Mascellani; +Cc: linux-xfs, team+boost

Processing commands for control@bugs.debian.org:

> reassign 1000974 xfslibs-dev
Bug #1000974 [libboost1.74-dev] copy_move_algo.hpp:1083:10: error: ‘__fallthrough__’ was not declared in this scope; did you mean ‘fallthrough’?
Bug reassigned from package 'libboost1.74-dev' to 'xfslibs-dev'.
No longer marked as found in versions boost1.74/1.74.0-13.
Ignoring request to alter fixed versions of bug #1000974 to the same values previously set
> severity 1000974 important
Bug #1000974 [xfslibs-dev] copy_move_algo.hpp:1083:10: error: ‘__fallthrough__’ was not declared in this scope; did you mean ‘fallthrough’?
Ignoring request to change severity of Bug 1000974 to the same value.
> retitle 1000974 xfs/linux.h defines common word "fallthrough" breaking
Bug #1000974 [xfslibs-dev] copy_move_algo.hpp:1083:10: error: ‘__fallthrough__’ was not declared in this scope; did you mean ‘fallthrough’?
Changed Bug title to 'xfs/linux.h defines common word "fallthrough" breaking' from 'copy_move_algo.hpp:1083:10: error: ‘__fallthrough__’ was not declared in this scope; did you mean ‘fallthrough’?'.
> unrelated headers
Unknown command or malformed arguments to command.
> thanks
Stopping processing here.

Please contact me if you need assistance.
-- 
1000974: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1000974
Debian Bug Tracking System
Contact owner@bugs.debian.org with problems

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Bug#1000974: copy_move_algo.hpp:1083:10: error: ‘__fallthrough__’ was not declared in this scope; did you mean ‘fallthrough’?
       [not found]       ` <c7ccff50-c177-7f96-2d99-2077f77374ad@debian.org>
  2021-12-05 15:57         ` Processed (with 1 error): Re: Bug#1000974: copy_move_algo.hpp:1083:10: error: ‘__fallthrough__’ was not declared in this scope; did you mean ‘fallthrough’? Debian Bug Tracking System
@ 2021-12-05 17:33         ` Thomas Goirand
  2021-12-05 17:45           ` Darrick J. Wong
  1 sibling, 1 reply; 14+ messages in thread
From: Thomas Goirand @ 2021-12-05 17:33 UTC (permalink / raw)
  To: Giovanni Mascellani, 1000974, xfslibs-dev

On 12/5/21 4:50 PM, Giovanni Mascellani wrote:
> reassign 1000974 xfslibs-dev
> severity 1000974 important
> retitle 1000974 xfs/linux.h defines common word "fallthrough" breaking
> unrelated headers
> thanks
> 
> Hi,
> 
> On 04/12/21 23:36, Thomas Goirand wrote:
>> On 12/4/21 5:11 PM, Giovanni Mascellani wrote:
>>> Could you try running that compilation command with g++ -E, so you can
>>> see what BOOST_FALLTHROUGH is actually begin replaced with?
>>
>> Oh, sorry, now I get what you meant (did a c++ --help to understand what
>> -E was doing). Here's the result in
>> CMakeFiles/seastar.dir/src/core/file.cc.o:
>>
>> __attribute__((__attribute__((__fallthrough__))));
>>
>> Probably, there's a mistake, and it should really be:
>>
>> __attribute__((__fallthrough__));
>>
>> instead, which is the source of the trouble?
> 
> It seems that the problem goes this way:
> 
>  * Boost defines in /usr/include/boost/config/compiler/gcc.hpp:
> 
> #define BOOST_FALLTHROUGH __attribute__((fallthrough))
> 
>  * But /usr/include/xfs/linux.h defines:
> 
> #define fallthrough __attribute__((__fallthrough__))
> 
> So the "fallthrough" in Boost's definition is expanded again and causes
> the problem.
> 
> I think xfs/linux.h is at fault here, because it aggressively defines a
> common word (while Boost namespaces all its definitions with a BOOST_
> prefix). Unfortunately the C/C++ preprocessor makes it easy to break
> other headers if you define stuff too liberally. This wold also, for
> example, break any program that use the name "fallthrough" for a
> variable, which is a completely reasonable name to use. A simple example
> could be:
> ---
> #include <xfs/linux.h>
> 
> int main() {
>     int fallthrough = 0;
>     return fallthrough;
> }
> ---
> which fails compilation with:
> ---
> $ LANG=C gcc test.c
> test.c: In function 'main':
> test.c:4:5: warning: 'fallthrough' attribute not followed by ';'
> [-Wattributes]
>     4 |     int fallthrough = 0;
>       |     ^~~
> test.c:4:21: error: expected identifier or '(' before '=' token
>     4 |     int fallthrough = 0;
>       |                     ^
> In file included from test.c:1:
> test.c:5:12: error: expected expression before '__attribute__'
>     5 |     return fallthrough;
>       |            ^~~~~~~~~~~
> ---
> 
> You can probably work around this problem by undefining "fallthrough"
> just after the xfs/linux.h header. In the meantime I am reassigning this
> bug to xfslibs-dev.
> 
> Giovanni.


Hi,

I can confirm that commenting away line 373 to 381 of xfs/linux.h solve
the troubles when building Ceph. Downgrading to 5.13.0-1 (using
snapshot.d.o) also solved the trouble, showing that 5.14.0 really is the
trouble here.

Thanks Giovanni for finding this out.

Cheers,

Thomas Goirand (zigo)

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Bug#1000974: copy_move_algo.hpp:1083:10: error: ‘__fallthrough__’ was not declared in this scope; did you mean ‘fallthrough’?
  2021-12-05 17:33         ` Thomas Goirand
@ 2021-12-05 17:45           ` Darrick J. Wong
  0 siblings, 0 replies; 14+ messages in thread
From: Darrick J. Wong @ 2021-12-05 17:45 UTC (permalink / raw)
  To: Thomas Goirand, 1000974; +Cc: Giovanni Mascellani, xfslibs-dev

On Sun, Dec 05, 2021 at 06:33:40PM +0100, Thomas Goirand wrote:
> On 12/5/21 4:50 PM, Giovanni Mascellani wrote:
> > reassign 1000974 xfslibs-dev
> > severity 1000974 important
> > retitle 1000974 xfs/linux.h defines common word "fallthrough" breaking
> > unrelated headers
> > thanks
> > 
> > Hi,
> > 
> > On 04/12/21 23:36, Thomas Goirand wrote:
> >> On 12/4/21 5:11 PM, Giovanni Mascellani wrote:
> >>> Could you try running that compilation command with g++ -E, so you can
> >>> see what BOOST_FALLTHROUGH is actually begin replaced with?
> >>
> >> Oh, sorry, now I get what you meant (did a c++ --help to understand what
> >> -E was doing). Here's the result in
> >> CMakeFiles/seastar.dir/src/core/file.cc.o:
> >>
> >> __attribute__((__attribute__((__fallthrough__))));
> >>
> >> Probably, there's a mistake, and it should really be:
> >>
> >> __attribute__((__fallthrough__));
> >>
> >> instead, which is the source of the trouble?
> > 
> > It seems that the problem goes this way:
> > 
> >  * Boost defines in /usr/include/boost/config/compiler/gcc.hpp:
> > 
> > #define BOOST_FALLTHROUGH __attribute__((fallthrough))
> > 
> >  * But /usr/include/xfs/linux.h defines:
> > 
> > #define fallthrough __attribute__((__fallthrough__))
> > 
> > So the "fallthrough" in Boost's definition is expanded again and causes
> > the problem.
> > 
> > I think xfs/linux.h is at fault here, because it aggressively defines a
> > common word (while Boost namespaces all its definitions with a BOOST_
> > prefix). Unfortunately the C/C++ preprocessor makes it easy to break
> > other headers if you define stuff too liberally. This wold also, for
> > example, break any program that use the name "fallthrough" for a
> > variable, which is a completely reasonable name to use. A simple example
> > could be:
> > ---
> > #include <xfs/linux.h>
> > 
> > int main() {
> >     int fallthrough = 0;
> >     return fallthrough;
> > }
> > ---
> > which fails compilation with:
> > ---
> > $ LANG=C gcc test.c
> > test.c: In function 'main':
> > test.c:4:5: warning: 'fallthrough' attribute not followed by ';'
> > [-Wattributes]
> >     4 |     int fallthrough = 0;
> >       |     ^~~
> > test.c:4:21: error: expected identifier or '(' before '=' token
> >     4 |     int fallthrough = 0;
> >       |                     ^
> > In file included from test.c:1:
> > test.c:5:12: error: expected expression before '__attribute__'
> >     5 |     return fallthrough;
> >       |            ^~~~~~~~~~~
> > ---
> > 
> > You can probably work around this problem by undefining "fallthrough"
> > just after the xfs/linux.h header. In the meantime I am reassigning this
> > bug to xfslibs-dev.

Yeah, sorry.  This is braindamage from the Linux kernel (because hey, we
share libxfs between kernel and userspace) that I mistakenly let escape
because the authors of the kernel patch didn't try to help us when they
forced this stupid mess on us.  All this BS because some compiler
writers didn't like their static checkers to have to use regular
expressions for compatibility with existing codebases, and the language
standard people decided to introduce a breaking change to "standardize"
it.

Will fix ASAP.

--D

> > 
> > Giovanni.
> 
> 
> Hi,
> 
> I can confirm that commenting away line 373 to 381 of xfs/linux.h solve
> the troubles when building Ceph. Downgrading to 5.13.0-1 (using
> snapshot.d.o) also solved the trouble, showing that 5.14.0 really is the
> trouble here.
> 
> Thanks Giovanni for finding this out.
> 
> Cheers,
> 
> Thomas Goirand (zigo)

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH xfsprogs-5.14.2 URGENT] libxfs: hide the drainbamaged fallthrough macro from xfslibs
@ 2021-12-05 17:49     ` Darrick J. Wong
  0 siblings, 0 replies; 14+ messages in thread
From: Darrick J. Wong @ 2021-12-05 17:49 UTC (permalink / raw)
  To: Eric Sandeen
  Cc: Thomas Goirand, 1000974, Giovanni Mascellani, xfslibs-dev, xfs,
	gustavoars, keescook

From: Darrick J. Wong <djwong@kernel.org>

Back in mid-2021, Kees and Gustavo rammed into the kernel a bunch of
static checker "improvements" that redefined '/* fallthrough */'
comments for switch statements as a macro that virtualizes either that
same comment, a do-while loop, or a compiler __attribute__.  This was
necessary to work around the poor decision-making of the clang, gcc, and
C language standard authors, who collectively came up with four mutually
incompatible ways to document a lack of branching in a code flow.

Having received ZERO HELP porting this to userspace, Eric and I
foolishly dumped that crap into linux.h, which was a poor decision
because we keep forgetting that linux.h is exported as a userspace
header.  This has now caused downstream regressions in Debian[1] and
will probably cause more problems in the other distros.

Move it to platform_defs.h since that's not shipped publicly and leave a
warning to anyone else who dare modify linux.h.

[1] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1000974

Fixes: df9c7d8d ("xfs: Fix fall-through warnings for Clang")
Cc: 1000974@bugs.debian.org, gustavoars@kernel.org, keescook@chromium.org
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
 include/linux.h            |   20 ++------------------
 include/platform_defs.h.in |   21 +++++++++++++++++++++
 2 files changed, 23 insertions(+), 18 deletions(-)

diff --git a/include/linux.h b/include/linux.h
index 24650228..054117aa 100644
--- a/include/linux.h
+++ b/include/linux.h
@@ -360,24 +360,8 @@ fsmap_advance(
 #endif /* HAVE_MAP_SYNC */
 
 /*
- * Add the pseudo keyword 'fallthrough' so case statement blocks
- * must end with any of these keywords:
- *   break;
- *   fallthrough;
- *   continue;
- *   goto <label>;
- *   return [expression];
- *
- *  gcc: https://gcc.gnu.org/onlinedocs/gcc/Statement-Attributes.html#Statement-Attributes
+ * Reminder: anything added to this file will be compiled into downstream
+ * userspace projects!
  */
-#if defined __has_attribute
-#  if __has_attribute(__fallthrough__)
-#    define fallthrough                    __attribute__((__fallthrough__))
-#  else
-#    define fallthrough                    do {} while (0)  /* fallthrough */
-#  endif
-#else
-#    define fallthrough                    do {} while (0)  /* fallthrough */
-#endif
 
 #endif	/* __XFS_LINUX_H__ */
diff --git a/include/platform_defs.h.in b/include/platform_defs.h.in
index 7c6b3ada..6e6f26ef 100644
--- a/include/platform_defs.h.in
+++ b/include/platform_defs.h.in
@@ -113,4 +113,25 @@ static inline size_t __ab_c_size(size_t a, size_t b, size_t c)
 		sizeof(*(p)->member) + __must_be_array((p)->member),	\
 		sizeof(*(p)))
 
+/*
+ * Add the pseudo keyword 'fallthrough' so case statement blocks
+ * must end with any of these keywords:
+ *   break;
+ *   fallthrough;
+ *   continue;
+ *   goto <label>;
+ *   return [expression];
+ *
+ *  gcc: https://gcc.gnu.org/onlinedocs/gcc/Statement-Attributes.html#Statement-Attributes
+ */
+#if defined __has_attribute
+#  if __has_attribute(__fallthrough__)
+#    define fallthrough                    __attribute__((__fallthrough__))
+#  else
+#    define fallthrough                    do {} while (0)  /* fallthrough */
+#  endif
+#else
+#    define fallthrough                    do {} while (0)  /* fallthrough */
+#endif
+
 #endif	/* __XFS_PLATFORM_DEFS_H__ */

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Bug#1000974: [PATCH xfsprogs-5.14.2 URGENT] libxfs: hide the drainbamaged fallthrough macro from xfslibs
@ 2021-12-05 17:49     ` Darrick J. Wong
  0 siblings, 0 replies; 14+ messages in thread
From: Darrick J. Wong @ 2021-12-05 17:49 UTC (permalink / raw)
  To: Eric Sandeen
  Cc: Thomas Goirand, 1000974, Giovanni Mascellani, xfslibs-dev, xfs,
	gustavoars, keescook

From: Darrick J. Wong <djwong@kernel.org>

Back in mid-2021, Kees and Gustavo rammed into the kernel a bunch of
static checker "improvements" that redefined '/* fallthrough */'
comments for switch statements as a macro that virtualizes either that
same comment, a do-while loop, or a compiler __attribute__.  This was
necessary to work around the poor decision-making of the clang, gcc, and
C language standard authors, who collectively came up with four mutually
incompatible ways to document a lack of branching in a code flow.

Having received ZERO HELP porting this to userspace, Eric and I
foolishly dumped that crap into linux.h, which was a poor decision
because we keep forgetting that linux.h is exported as a userspace
header.  This has now caused downstream regressions in Debian[1] and
will probably cause more problems in the other distros.

Move it to platform_defs.h since that's not shipped publicly and leave a
warning to anyone else who dare modify linux.h.

[1] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1000974

Fixes: df9c7d8d ("xfs: Fix fall-through warnings for Clang")
Cc: 1000974@bugs.debian.org, gustavoars@kernel.org, keescook@chromium.org
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
 include/linux.h            |   20 ++------------------
 include/platform_defs.h.in |   21 +++++++++++++++++++++
 2 files changed, 23 insertions(+), 18 deletions(-)

diff --git a/include/linux.h b/include/linux.h
index 24650228..054117aa 100644
--- a/include/linux.h
+++ b/include/linux.h
@@ -360,24 +360,8 @@ fsmap_advance(
 #endif /* HAVE_MAP_SYNC */
 
 /*
- * Add the pseudo keyword 'fallthrough' so case statement blocks
- * must end with any of these keywords:
- *   break;
- *   fallthrough;
- *   continue;
- *   goto <label>;
- *   return [expression];
- *
- *  gcc: https://gcc.gnu.org/onlinedocs/gcc/Statement-Attributes.html#Statement-Attributes
+ * Reminder: anything added to this file will be compiled into downstream
+ * userspace projects!
  */
-#if defined __has_attribute
-#  if __has_attribute(__fallthrough__)
-#    define fallthrough                    __attribute__((__fallthrough__))
-#  else
-#    define fallthrough                    do {} while (0)  /* fallthrough */
-#  endif
-#else
-#    define fallthrough                    do {} while (0)  /* fallthrough */
-#endif
 
 #endif	/* __XFS_LINUX_H__ */
diff --git a/include/platform_defs.h.in b/include/platform_defs.h.in
index 7c6b3ada..6e6f26ef 100644
--- a/include/platform_defs.h.in
+++ b/include/platform_defs.h.in
@@ -113,4 +113,25 @@ static inline size_t __ab_c_size(size_t a, size_t b, size_t c)
 		sizeof(*(p)->member) + __must_be_array((p)->member),	\
 		sizeof(*(p)))
 
+/*
+ * Add the pseudo keyword 'fallthrough' so case statement blocks
+ * must end with any of these keywords:
+ *   break;
+ *   fallthrough;
+ *   continue;
+ *   goto <label>;
+ *   return [expression];
+ *
+ *  gcc: https://gcc.gnu.org/onlinedocs/gcc/Statement-Attributes.html#Statement-Attributes
+ */
+#if defined __has_attribute
+#  if __has_attribute(__fallthrough__)
+#    define fallthrough                    __attribute__((__fallthrough__))
+#  else
+#    define fallthrough                    do {} while (0)  /* fallthrough */
+#  endif
+#else
+#    define fallthrough                    do {} while (0)  /* fallthrough */
+#endif
+
 #endif	/* __XFS_PLATFORM_DEFS_H__ */

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Bug#1000974: Info received (Bug#1000974: copy_move_algo.hpp:1083:10: error: ‘__fallthrough__’ was not declared in this scope; did you mean ‘fallthrough’?)
       [not found] ` <163839370805.58768.6385074074873965943.reportbug@zbuz.infomaniak.ch>
       [not found]   ` <9b9bda73-9554-0c75-824d-f8d1b9b98e19@debian.org>
  2021-12-05 17:49     ` Bug#1000974: " Darrick J. Wong
@ 2021-12-05 19:00   ` Thomas Goirand
  2021-12-07 12:36   ` Bug#1000974: marked as done (xfs/linux.h defines common word "fallthrough" breaking unrelated headers) Debian Bug Tracking System
  2021-12-07 12:54   ` Debian Bug Tracking System
  4 siblings, 0 replies; 14+ messages in thread
From: Thomas Goirand @ 2021-12-05 19:00 UTC (permalink / raw)
  To: 1000974

[-- Attachment #1: Type: text/plain, Size: 99 bytes --]

Please find attached to this message a debdiff fixing the problem.

Cheers,

Thomas Goirand (zigo)

[-- Attachment #2: xfsprogs_5.14.0-release-2.1.debdiff --]
[-- Type: text/plain, Size: 12482 bytes --]

diff -Nru xfsprogs-5.14.0-release/debian/changelog xfsprogs-5.14.0-release/debian/changelog
--- xfsprogs-5.14.0-release/debian/changelog	2021-11-23 21:31:02.000000000 +0100
+++ xfsprogs-5.14.0-release/debian/changelog	2021-12-05 18:47:33.000000000 +0100
@@ -1,3 +1,11 @@
+xfsprogs (5.14.0-release-2.1) unstable; urgency=medium
+
+  * Non-maintainer upload.
+  * Add 0001-Revert-xfs-Fix-fall-through-warnings-for-Clang.patch so that
+    xfslibs-dev doesn't define fallthrough globally anymore (Closes: #1000974).
+
+ -- Thomas Goirand <zigo@debian.org>  Sun, 05 Dec 2021 18:47:33 +0100
+
 xfsprogs (5.14.0-release-2) unstable; urgency=medium
 
   * Replace the Debian-only #999879 fix by upstream patch
diff -Nru xfsprogs-5.14.0-release/debian/patches/0001-Revert-xfs-Fix-fall-through-warnings-for-Clang.patch xfsprogs-5.14.0-release/debian/patches/0001-Revert-xfs-Fix-fall-through-warnings-for-Clang.patch
--- xfsprogs-5.14.0-release/debian/patches/0001-Revert-xfs-Fix-fall-through-warnings-for-Clang.patch	1970-01-01 01:00:00.000000000 +0100
+++ xfsprogs-5.14.0-release/debian/patches/0001-Revert-xfs-Fix-fall-through-warnings-for-Clang.patch	2021-12-05 18:47:33.000000000 +0100
@@ -0,0 +1,343 @@
+From 1d67907760e0b4b53b894c443c70eef474dc7cce Mon Sep 17 00:00:00 2001
+From: Thomas Goirand <zigo@debian.org>
+Date: Sun, 5 Dec 2021 18:43:04 +0100
+Subject: [PATCH] Revert "xfs: Fix fall-through warnings for Clang"
+
+This reverts commit df9c7d8d8f3ed0785ed83e7fd0c7ddc92cbfbe15.
+---
+ include/linux.h       | 21 ---------------------
+ libxfs/xfs_ag_resv.c  |  4 ++--
+ libxfs/xfs_alloc.c    |  2 +-
+ libxfs/xfs_da_btree.c |  2 +-
+ 4 files changed, 4 insertions(+), 25 deletions(-)
+
+Index: xfsprogs-5.14.0-release/include/linux.h
+===================================================================
+--- xfsprogs-5.14.0-release.orig/include/linux.h
++++ xfsprogs-5.14.0-release/include/linux.h
+@@ -359,25 +359,4 @@ fsmap_advance(
+ #include <asm-generic/mman-common.h>
+ #endif /* HAVE_MAP_SYNC */
+ 
+-/*
+- * Add the pseudo keyword 'fallthrough' so case statement blocks
+- * must end with any of these keywords:
+- *   break;
+- *   fallthrough;
+- *   continue;
+- *   goto <label>;
+- *   return [expression];
+- *
+- *  gcc: https://gcc.gnu.org/onlinedocs/gcc/Statement-Attributes.html#Statement-Attributes
+- */
+-#if defined __has_attribute
+-#  if __has_attribute(__fallthrough__)
+-#    define fallthrough                    __attribute__((__fallthrough__))
+-#  else
+-#    define fallthrough                    do {} while (0)  /* fallthrough */
+-#  endif
+-#else
+-#    define fallthrough                    do {} while (0)  /* fallthrough */
+-#endif
+-
+ #endif	/* __XFS_LINUX_H__ */
+Index: xfsprogs-5.14.0-release/libxfs/xfs_ag_resv.c
+===================================================================
+--- xfsprogs-5.14.0-release.orig/libxfs/xfs_ag_resv.c
++++ xfsprogs-5.14.0-release/libxfs/xfs_ag_resv.c
+@@ -364,7 +364,7 @@ xfs_ag_resv_alloc_extent(
+ 		break;
+ 	default:
+ 		ASSERT(0);
+-		fallthrough;
++		/* fall through */
+ 	case XFS_AG_RESV_NONE:
+ 		field = args->wasdel ? XFS_TRANS_SB_RES_FDBLOCKS :
+ 				       XFS_TRANS_SB_FDBLOCKS;
+@@ -406,7 +406,7 @@ xfs_ag_resv_free_extent(
+ 		break;
+ 	default:
+ 		ASSERT(0);
+-		fallthrough;
++		/* fall through */
+ 	case XFS_AG_RESV_NONE:
+ 		xfs_trans_mod_sb(tp, XFS_TRANS_SB_FDBLOCKS, (int64_t)len);
+ 		return;
+Index: xfsprogs-5.14.0-release/libxfs/xfs_alloc.c
+===================================================================
+--- xfsprogs-5.14.0-release.orig/libxfs/xfs_alloc.c
++++ xfsprogs-5.14.0-release/libxfs/xfs_alloc.c
+@@ -3170,7 +3170,7 @@ xfs_alloc_vextent(
+ 		}
+ 		args->agbno = XFS_FSB_TO_AGBNO(mp, args->fsbno);
+ 		args->type = XFS_ALLOCTYPE_NEAR_BNO;
+-		fallthrough;
++		/* FALLTHROUGH */
+ 	case XFS_ALLOCTYPE_FIRST_AG:
+ 		/*
+ 		 * Rotate through the allocation groups looking for a winner.
+Index: xfsprogs-5.14.0-release/libxfs/xfs_da_btree.c
+===================================================================
+--- xfsprogs-5.14.0-release.orig/libxfs/xfs_da_btree.c
++++ xfsprogs-5.14.0-release/libxfs/xfs_da_btree.c
+@@ -279,7 +279,7 @@ xfs_da3_node_read_verify(
+ 						__this_address);
+ 				break;
+ 			}
+-			fallthrough;
++			/* fall through */
+ 		case XFS_DA_NODE_MAGIC:
+ 			fa = xfs_da3_node_verify(bp);
+ 			if (fa)
+Index: xfsprogs-5.14.0-release/growfs/xfs_growfs.c
+===================================================================
+--- xfsprogs-5.14.0-release.orig/growfs/xfs_growfs.c
++++ xfsprogs-5.14.0-release/growfs/xfs_growfs.c
+@@ -78,7 +78,7 @@ main(int argc, char **argv)
+ 		switch (c) {
+ 		case 'D':
+ 			dsize = strtoll(optarg, NULL, 10);
+-			fallthrough;
++			/* fall through */
+ 		case 'd':
+ 			dflag = 1;
+ 			break;
+@@ -91,7 +91,7 @@ main(int argc, char **argv)
+ 			break;
+ 		case 'L':
+ 			lsize = strtoll(optarg, NULL, 10);
+-			fallthrough;
++			/* fall through */
+ 		case 'l':
+ 			lflag = 1;
+ 			break;
+@@ -107,7 +107,7 @@ main(int argc, char **argv)
+ 			break;
+ 		case 'R':
+ 			rsize = strtoll(optarg, NULL, 10);
+-			fallthrough;
++			/* fall through */
+ 		case 'r':
+ 			rflag = 1;
+ 			break;
+Index: xfsprogs-5.14.0-release/libxfs/xfs_attr.c
+===================================================================
+--- xfsprogs-5.14.0-release.orig/libxfs/xfs_attr.c
++++ xfsprogs-5.14.0-release/libxfs/xfs_attr.c
+@@ -483,7 +483,7 @@ xfs_attr_set_iter(
+ 		if (error)
+ 			return error;
+ 
+-		fallthrough;
++		/* fall through */
+ 	case XFS_DAS_RM_LBLK:
+ 		/* Set state in case xfs_attr_rmtval_remove returns -EAGAIN */
+ 		dac->dela_state = XFS_DAS_RM_LBLK;
+@@ -496,7 +496,7 @@ xfs_attr_set_iter(
+ 			return -EAGAIN;
+ 		}
+ 
+-		fallthrough;
++		/* fall through */
+ 	case XFS_DAS_RD_LEAF:
+ 		/*
+ 		 * This is the last step for leaf format. Read the block with
+@@ -528,7 +528,7 @@ xfs_attr_set_iter(
+ 				return error;
+ 		}
+ 
+-		fallthrough;
++		/* fall through */
+ 	case XFS_DAS_ALLOC_NODE:
+ 		/*
+ 		 * If there was an out-of-line value, allocate the blocks we
+@@ -590,7 +590,7 @@ xfs_attr_set_iter(
+ 		if (error)
+ 			return error;
+ 
+-		fallthrough;
++		/* fall through */
+ 	case XFS_DAS_RM_NBLK:
+ 		/* Set state in case xfs_attr_rmtval_remove returns -EAGAIN */
+ 		dac->dela_state = XFS_DAS_RM_NBLK;
+@@ -603,7 +603,7 @@ xfs_attr_set_iter(
+ 			return -EAGAIN;
+ 		}
+ 
+-		fallthrough;
++		/* fall through */
+ 	case XFS_DAS_CLR_FLAG:
+ 		/*
+ 		 * The last state for node format. Look up the old attr and
+@@ -1406,7 +1406,7 @@ xfs_attr_remove_iter(
+ 			state = dac->da_state;
+ 		}
+ 
+-		fallthrough;
++		/* fall through */
+ 	case XFS_DAS_RMTBLK:
+ 		dac->dela_state = XFS_DAS_RMTBLK;
+ 
+@@ -1441,7 +1441,7 @@ xfs_attr_remove_iter(
+ 			return -EAGAIN;
+ 		}
+ 
+-		fallthrough;
++		/* fall through */
+ 	case XFS_DAS_RM_NAME:
+ 		/*
+ 		 * If we came here fresh from a transaction roll, reattach all
+@@ -1469,7 +1469,7 @@ xfs_attr_remove_iter(
+ 			return -EAGAIN;
+ 		}
+ 
+-		fallthrough;
++		/* fall through */
+ 	case XFS_DAS_RM_SHRINK:
+ 		/*
+ 		 * If the result is small enough, push it all into the inode.
+Index: xfsprogs-5.14.0-release/repair/dinode.c
+===================================================================
+--- xfsprogs-5.14.0-release.orig/repair/dinode.c
++++ xfsprogs-5.14.0-release/repair/dinode.c
+@@ -531,7 +531,7 @@ _("Fatal error: inode %" PRIu64 " - blkm
+ 				do_warn(
+ _("%s fork in ino %" PRIu64 " claims free block %" PRIu64 "\n"),
+ 					forkname, ino, (uint64_t) b);
+-				fallthrough;
++				/* fall through */
+ 			case XR_E_INUSE1:	/* seen by rmap */
+ 			case XR_E_UNKNOWN:
+ 				break;
+@@ -543,7 +543,7 @@ _("%s fork in ino %" PRIu64 " claims fre
+ 			case XR_E_INO1:
+ 			case XR_E_INUSE_FS1:
+ 				do_warn(_("rmap claims metadata use!\n"));
+-				fallthrough;
++				/* fall through */
+ 			case XR_E_FS_MAP:
+ 			case XR_E_INO:
+ 			case XR_E_INUSE_FS:
+Index: xfsprogs-5.14.0-release/repair/scan.c
+===================================================================
+--- xfsprogs-5.14.0-release.orig/repair/scan.c
++++ xfsprogs-5.14.0-release/repair/scan.c
+@@ -732,7 +732,7 @@ _("%s freespace btree block claimed (sta
+ 							     XR_E_FREE);
+ 						break;
+ 					}
+-					fallthrough;
++					/* fall through */
+ 				default:
+ 					do_warn(
+ 	_("block (%d,%d-%d) multiply claimed by %s space tree, state - %d\n"),
+@@ -911,7 +911,7 @@ _("in use block (%d,%d-%d) mismatch in %
+ 		if (xfs_sb_version_hasreflink(&mp->m_sb) &&
+ 		    !XFS_RMAP_NON_INODE_OWNER(owner))
+ 			break;
+-		fallthrough;
++		/* fall through */
+ 	default:
+ 		do_warn(
+ _("unknown block (%d,%d-%d) mismatch on %s tree, state - %d,%" PRIx64 "\n"),
+Index: xfsprogs-5.14.0-release/repair/phase4.c
+===================================================================
+--- xfsprogs-5.14.0-release.orig/repair/phase4.c
++++ xfsprogs-5.14.0-release/repair/phase4.c
+@@ -317,7 +317,7 @@ phase4(xfs_mount_t *mp)
+ 				do_warn(
+ 				_("unknown block state, ag %d, blocks %u-%u\n"),
+ 					i, j, j + blen - 1);
+-				fallthrough;
++				/* fall through */
+ 			case XR_E_UNKNOWN:
+ 			case XR_E_FREE:
+ 			case XR_E_INUSE:
+@@ -349,7 +349,7 @@ phase4(xfs_mount_t *mp)
+ 			do_warn(
+ 	_("unknown rt extent state, extent %" PRIu64 "\n"),
+ 				bno);
+-			fallthrough;
++			/* fall through */
+ 		case XR_E_UNKNOWN:
+ 		case XR_E_FREE1:
+ 		case XR_E_FREE:
+Index: xfsprogs-5.14.0-release/scrub/repair.c
+===================================================================
+--- xfsprogs-5.14.0-release.orig/scrub/repair.c
++++ xfsprogs-5.14.0-release/scrub/repair.c
+@@ -133,7 +133,7 @@ action_list_find_mustfix(
+ 			alist->nr--;
+ 			list_move_tail(&aitem->list, &immediate_alist->list);
+ 			immediate_alist->nr++;
+-			fallthrough;
++			/* fall through */
+ 		case XFS_SCRUB_TYPE_BNOBT:
+ 		case XFS_SCRUB_TYPE_CNTBT:
+ 		case XFS_SCRUB_TYPE_REFCNTBT:
+Index: xfsprogs-5.14.0-release/scrub/inodes.c
+===================================================================
+--- xfsprogs-5.14.0-release.orig/scrub/inodes.c
++++ xfsprogs-5.14.0-release/scrub/inodes.c
+@@ -204,7 +204,7 @@ _("Changed too many times during scan; g
+ 			}
+ 			case ECANCELED:
+ 				error = 0;
+-				fallthrough;
++				/* fall through */
+ 			default:
+ 				goto err;
+ 			}
+Index: xfsprogs-5.14.0-release/scrub/scrub.c
+===================================================================
+--- xfsprogs-5.14.0-release.orig/scrub/scrub.c
++++ xfsprogs-5.14.0-release/scrub/scrub.c
+@@ -164,7 +164,7 @@ _("Filesystem is shut down, aborting."))
+ 		 * and the other two should be reported via sm_flags.
+ 		 */
+ 		str_liberror(ctx, error, _("Kernel bug"));
+-		fallthrough;
++		/* fall through */
+ 	default:
+ 		/* Operational error. */
+ 		str_errno(ctx, descr_render(&dsc));
+@@ -316,7 +316,7 @@ scrub_meta_type(
+ 		ret = scrub_save_repair(ctx, alist, &meta);
+ 		if (ret)
+ 			return ret;
+-		fallthrough;
++		/* fall through */
+ 	case CHECK_DONE:
+ 		return 0;
+ 	default:
+@@ -741,7 +741,7 @@ _("Filesystem is shut down, aborting."))
+ 		if (is_unoptimized(&oldm) ||
+ 		    debug_tweak_on("XFS_SCRUB_FORCE_REPAIR"))
+ 			return CHECK_DONE;
+-		fallthrough;
++		/* fall through */
+ 	case EINVAL:
+ 		/* Kernel doesn't know how to repair this? */
+ 		str_corrupt(ctx, descr_render(&dsc),
+@@ -761,7 +761,7 @@ _("Read-only filesystem; cannot make cha
+ 		/* Don't care if preen fails due to low resources. */
+ 		if (is_unoptimized(&oldm) && !needs_repair(&oldm))
+ 			return CHECK_DONE;
+-		fallthrough;
++		/* fall through */
+ 	default:
+ 		/*
+ 		 * Operational error.  If the caller doesn't want us
+Index: xfsprogs-5.14.0-release/db/type.c
+===================================================================
+--- xfsprogs-5.14.0-release.orig/db/type.c
++++ xfsprogs-5.14.0-release/db/type.c
+@@ -307,7 +307,7 @@ handle_text(
+ {
+ 	switch (action) {
+ 	case DB_FUZZ:
+-		fallthrough;
++		/* fall through */
+ 	case DB_WRITE:
+ 		dbprintf(_("text writing/fuzzing not supported.\n"));
+ 		break;
diff -Nru xfsprogs-5.14.0-release/debian/patches/series xfsprogs-5.14.0-release/debian/patches/series
--- xfsprogs-5.14.0-release/debian/patches/series	2021-11-23 21:21:38.000000000 +0100
+++ xfsprogs-5.14.0-release/debian/patches/series	2021-12-05 18:47:32.000000000 +0100
@@ -1,2 +1,3 @@
 libfrog-fix-crc32c-self-test-code-on-cross-builds.patch
 libxfs-fix-atomic64_t-poorly-for-32-bit-architectures.patch
+0001-Revert-xfs-Fix-fall-through-warnings-for-Clang.patch

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Bug#1000974: [PATCH xfsprogs-5.14.2 URGENT] libxfs: hide the drainbamaged fallthrough macro from xfslibs
@ 2021-12-05 21:10       ` Dave Chinner
  0 siblings, 0 replies; 14+ messages in thread
From: Dave Chinner @ 2021-12-05 21:10 UTC (permalink / raw)
  To: Darrick J. Wong, 1000974
  Cc: Eric Sandeen, Thomas Goirand, Giovanni Mascellani, xfslibs-dev,
	xfs, gustavoars, keescook

On Sun, Dec 05, 2021 at 09:49:51AM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> Back in mid-2021, Kees and Gustavo rammed into the kernel a bunch of
> static checker "improvements" that redefined '/* fallthrough */'
> comments for switch statements as a macro that virtualizes either that
> same comment, a do-while loop, or a compiler __attribute__.  This was
> necessary to work around the poor decision-making of the clang, gcc, and
> C language standard authors, who collectively came up with four mutually
> incompatible ways to document a lack of branching in a code flow.
> 
> Having received ZERO HELP porting this to userspace, Eric and I
> foolishly dumped that crap into linux.h, which was a poor decision
> because we keep forgetting that linux.h is exported as a userspace
> header.  This has now caused downstream regressions in Debian[1] and
> will probably cause more problems in the other distros.
> 
> Move it to platform_defs.h since that's not shipped publicly and leave a
> warning to anyone else who dare modify linux.h.
> 
> [1] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1000974
> 
> Fixes: df9c7d8d ("xfs: Fix fall-through warnings for Clang")
> Cc: 1000974@bugs.debian.org, gustavoars@kernel.org, keescook@chromium.org
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> ---
>  include/linux.h            |   20 ++------------------
>  include/platform_defs.h.in |   21 +++++++++++++++++++++
>  2 files changed, 23 insertions(+), 18 deletions(-)
> 
> diff --git a/include/linux.h b/include/linux.h
> index 24650228..054117aa 100644
> --- a/include/linux.h
> +++ b/include/linux.h
> @@ -360,24 +360,8 @@ fsmap_advance(
>  #endif /* HAVE_MAP_SYNC */
>  
>  /*
> - * Add the pseudo keyword 'fallthrough' so case statement blocks
> - * must end with any of these keywords:
> - *   break;
> - *   fallthrough;
> - *   continue;
> - *   goto <label>;
> - *   return [expression];
> - *
> - *  gcc: https://gcc.gnu.org/onlinedocs/gcc/Statement-Attributes.html#Statement-Attributes
> + * Reminder: anything added to this file will be compiled into downstream
> + * userspace projects!

This comment belongs at the top of the file before all the includes,
not at the end of it. Otherwise looks ok for a quick fix.

Reviewed-by: Dave Chinner <dchinner@redhat.com>

But I have to wonder - why is this file even exported to userspace?
It's mostly the xfsprogs source build wrapper stuff for linux -
there's no public API in it except for xfsctl()....

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Bug#1000974: [PATCH xfsprogs-5.14.2 URGENT] libxfs: hide the drainbamaged fallthrough macro from xfslibs
@ 2021-12-05 21:10       ` Dave Chinner
  0 siblings, 0 replies; 14+ messages in thread
From: Dave Chinner @ 2021-12-05 21:10 UTC (permalink / raw)
  To: Darrick J. Wong, 1000974
  Cc: Eric Sandeen, Thomas Goirand, Giovanni Mascellani, xfslibs-dev,
	xfs, gustavoars, keescook

On Sun, Dec 05, 2021 at 09:49:51AM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> Back in mid-2021, Kees and Gustavo rammed into the kernel a bunch of
> static checker "improvements" that redefined '/* fallthrough */'
> comments for switch statements as a macro that virtualizes either that
> same comment, a do-while loop, or a compiler __attribute__.  This was
> necessary to work around the poor decision-making of the clang, gcc, and
> C language standard authors, who collectively came up with four mutually
> incompatible ways to document a lack of branching in a code flow.
> 
> Having received ZERO HELP porting this to userspace, Eric and I
> foolishly dumped that crap into linux.h, which was a poor decision
> because we keep forgetting that linux.h is exported as a userspace
> header.  This has now caused downstream regressions in Debian[1] and
> will probably cause more problems in the other distros.
> 
> Move it to platform_defs.h since that's not shipped publicly and leave a
> warning to anyone else who dare modify linux.h.
> 
> [1] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1000974
> 
> Fixes: df9c7d8d ("xfs: Fix fall-through warnings for Clang")
> Cc: 1000974@bugs.debian.org, gustavoars@kernel.org, keescook@chromium.org
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> ---
>  include/linux.h            |   20 ++------------------
>  include/platform_defs.h.in |   21 +++++++++++++++++++++
>  2 files changed, 23 insertions(+), 18 deletions(-)
> 
> diff --git a/include/linux.h b/include/linux.h
> index 24650228..054117aa 100644
> --- a/include/linux.h
> +++ b/include/linux.h
> @@ -360,24 +360,8 @@ fsmap_advance(
>  #endif /* HAVE_MAP_SYNC */
>  
>  /*
> - * Add the pseudo keyword 'fallthrough' so case statement blocks
> - * must end with any of these keywords:
> - *   break;
> - *   fallthrough;
> - *   continue;
> - *   goto <label>;
> - *   return [expression];
> - *
> - *  gcc: https://gcc.gnu.org/onlinedocs/gcc/Statement-Attributes.html#Statement-Attributes
> + * Reminder: anything added to this file will be compiled into downstream
> + * userspace projects!

This comment belongs at the top of the file before all the includes,
not at the end of it. Otherwise looks ok for a quick fix.

Reviewed-by: Dave Chinner <dchinner@redhat.com>

But I have to wonder - why is this file even exported to userspace?
It's mostly the xfsprogs source build wrapper stuff for linux -
there's no public API in it except for xfsctl()....

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH xfsprogs-5.14.2 URGENT] libxfs: hide the drainbamaged fallthrough macro from xfslibs
@ 2021-12-06 14:26       ` Eric Sandeen
  0 siblings, 0 replies; 14+ messages in thread
From: Eric Sandeen @ 2021-12-06 14:26 UTC (permalink / raw)
  To: Darrick J. Wong, Eric Sandeen
  Cc: Thomas Goirand, 1000974, Giovanni Mascellani, xfslibs-dev, xfs,
	gustavoars, keescook


On 12/5/21 11:49 AM, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> Back in mid-2021, Kees and Gustavo rammed into the kernel a bunch of
> static checker "improvements" that redefined '/* fallthrough */'
> comments for switch statements as a macro that virtualizes either that
> same comment, a do-while loop, or a compiler __attribute__.  This was
> necessary to work around the poor decision-making of the clang, gcc, and
> C language standard authors, who collectively came up with four mutually
> incompatible ways to document a lack of branching in a code flow.
> 
> Having received ZERO HELP porting this to userspace, Eric and I
> foolishly dumped that crap into linux.h, which was a poor decision
> because we keep forgetting that linux.h is exported as a userspace
> header.  This has now caused downstream regressions in Debian[1] and
> will probably cause more problems in the other distros.
> 
> Move it to platform_defs.h since that's not shipped publicly and leave a
> warning to anyone else who dare modify linux.h.
> 
> [1] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1000974
> 
> Fixes: df9c7d8d ("xfs: Fix fall-through warnings for Clang")
> Cc: 1000974@bugs.debian.org, gustavoars@kernel.org, keescook@chromium.org
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>

Thanks Darrick, I'll get a 5.14.2 pushed out today. Not sure if it was you
or I who stuffed it into this header originally, but apologies for not
thinking that through before merging it in any case.

Reviewed-by: Eric Sandeen <sandeen@redhat.com>

-Eric

> ---
>   include/linux.h            |   20 ++------------------
>   include/platform_defs.h.in |   21 +++++++++++++++++++++
>   2 files changed, 23 insertions(+), 18 deletions(-)
> 
> diff --git a/include/linux.h b/include/linux.h
> index 24650228..054117aa 100644
> --- a/include/linux.h
> +++ b/include/linux.h
> @@ -360,24 +360,8 @@ fsmap_advance(
>   #endif /* HAVE_MAP_SYNC */
>   
>   /*
> - * Add the pseudo keyword 'fallthrough' so case statement blocks
> - * must end with any of these keywords:
> - *   break;
> - *   fallthrough;
> - *   continue;
> - *   goto <label>;
> - *   return [expression];
> - *
> - *  gcc: https://gcc.gnu.org/onlinedocs/gcc/Statement-Attributes.html#Statement-Attributes
> + * Reminder: anything added to this file will be compiled into downstream
> + * userspace projects!
>    */
> -#if defined __has_attribute
> -#  if __has_attribute(__fallthrough__)
> -#    define fallthrough                    __attribute__((__fallthrough__))
> -#  else
> -#    define fallthrough                    do {} while (0)  /* fallthrough */
> -#  endif
> -#else
> -#    define fallthrough                    do {} while (0)  /* fallthrough */
> -#endif
>   
>   #endif	/* __XFS_LINUX_H__ */
> diff --git a/include/platform_defs.h.in b/include/platform_defs.h.in
> index 7c6b3ada..6e6f26ef 100644
> --- a/include/platform_defs.h.in
> +++ b/include/platform_defs.h.in
> @@ -113,4 +113,25 @@ static inline size_t __ab_c_size(size_t a, size_t b, size_t c)
>   		sizeof(*(p)->member) + __must_be_array((p)->member),	\
>   		sizeof(*(p)))
>   
> +/*
> + * Add the pseudo keyword 'fallthrough' so case statement blocks
> + * must end with any of these keywords:
> + *   break;
> + *   fallthrough;
> + *   continue;
> + *   goto <label>;
> + *   return [expression];
> + *
> + *  gcc: https://gcc.gnu.org/onlinedocs/gcc/Statement-Attributes.html#Statement-Attributes
> + */
> +#if defined __has_attribute
> +#  if __has_attribute(__fallthrough__)
> +#    define fallthrough                    __attribute__((__fallthrough__))
> +#  else
> +#    define fallthrough                    do {} while (0)  /* fallthrough */
> +#  endif
> +#else
> +#    define fallthrough                    do {} while (0)  /* fallthrough */
> +#endif
> +
>   #endif	/* __XFS_PLATFORM_DEFS_H__ */
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Bug#1000974: [PATCH xfsprogs-5.14.2 URGENT] libxfs: hide the drainbamaged fallthrough macro from xfslibs
@ 2021-12-06 14:26       ` Eric Sandeen
  0 siblings, 0 replies; 14+ messages in thread
From: Eric Sandeen @ 2021-12-06 14:26 UTC (permalink / raw)
  To: Darrick J. Wong, Eric Sandeen
  Cc: Thomas Goirand, 1000974, Giovanni Mascellani, xfslibs-dev, xfs,
	gustavoars, keescook


On 12/5/21 11:49 AM, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> Back in mid-2021, Kees and Gustavo rammed into the kernel a bunch of
> static checker "improvements" that redefined '/* fallthrough */'
> comments for switch statements as a macro that virtualizes either that
> same comment, a do-while loop, or a compiler __attribute__.  This was
> necessary to work around the poor decision-making of the clang, gcc, and
> C language standard authors, who collectively came up with four mutually
> incompatible ways to document a lack of branching in a code flow.
> 
> Having received ZERO HELP porting this to userspace, Eric and I
> foolishly dumped that crap into linux.h, which was a poor decision
> because we keep forgetting that linux.h is exported as a userspace
> header.  This has now caused downstream regressions in Debian[1] and
> will probably cause more problems in the other distros.
> 
> Move it to platform_defs.h since that's not shipped publicly and leave a
> warning to anyone else who dare modify linux.h.
> 
> [1] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1000974
> 
> Fixes: df9c7d8d ("xfs: Fix fall-through warnings for Clang")
> Cc: 1000974@bugs.debian.org, gustavoars@kernel.org, keescook@chromium.org
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>

Thanks Darrick, I'll get a 5.14.2 pushed out today. Not sure if it was you
or I who stuffed it into this header originally, but apologies for not
thinking that through before merging it in any case.

Reviewed-by: Eric Sandeen <sandeen@redhat.com>

-Eric

> ---
>   include/linux.h            |   20 ++------------------
>   include/platform_defs.h.in |   21 +++++++++++++++++++++
>   2 files changed, 23 insertions(+), 18 deletions(-)
> 
> diff --git a/include/linux.h b/include/linux.h
> index 24650228..054117aa 100644
> --- a/include/linux.h
> +++ b/include/linux.h
> @@ -360,24 +360,8 @@ fsmap_advance(
>   #endif /* HAVE_MAP_SYNC */
>   
>   /*
> - * Add the pseudo keyword 'fallthrough' so case statement blocks
> - * must end with any of these keywords:
> - *   break;
> - *   fallthrough;
> - *   continue;
> - *   goto <label>;
> - *   return [expression];
> - *
> - *  gcc: https://gcc.gnu.org/onlinedocs/gcc/Statement-Attributes.html#Statement-Attributes
> + * Reminder: anything added to this file will be compiled into downstream
> + * userspace projects!
>    */
> -#if defined __has_attribute
> -#  if __has_attribute(__fallthrough__)
> -#    define fallthrough                    __attribute__((__fallthrough__))
> -#  else
> -#    define fallthrough                    do {} while (0)  /* fallthrough */
> -#  endif
> -#else
> -#    define fallthrough                    do {} while (0)  /* fallthrough */
> -#endif
>   
>   #endif	/* __XFS_LINUX_H__ */
> diff --git a/include/platform_defs.h.in b/include/platform_defs.h.in
> index 7c6b3ada..6e6f26ef 100644
> --- a/include/platform_defs.h.in
> +++ b/include/platform_defs.h.in
> @@ -113,4 +113,25 @@ static inline size_t __ab_c_size(size_t a, size_t b, size_t c)
>   		sizeof(*(p)->member) + __must_be_array((p)->member),	\
>   		sizeof(*(p)))
>   
> +/*
> + * Add the pseudo keyword 'fallthrough' so case statement blocks
> + * must end with any of these keywords:
> + *   break;
> + *   fallthrough;
> + *   continue;
> + *   goto <label>;
> + *   return [expression];
> + *
> + *  gcc: https://gcc.gnu.org/onlinedocs/gcc/Statement-Attributes.html#Statement-Attributes
> + */
> +#if defined __has_attribute
> +#  if __has_attribute(__fallthrough__)
> +#    define fallthrough                    __attribute__((__fallthrough__))
> +#  else
> +#    define fallthrough                    do {} while (0)  /* fallthrough */
> +#  endif
> +#else
> +#    define fallthrough                    do {} while (0)  /* fallthrough */
> +#endif
> +
>   #endif	/* __XFS_PLATFORM_DEFS_H__ */
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH xfsprogs-5.14.2 URGENT] libxfs: hide the drainbamaged fallthrough macro from xfslibs
@ 2021-12-06 20:15       ` Kees Cook
  0 siblings, 0 replies; 14+ messages in thread
From: Kees Cook @ 2021-12-06 20:15 UTC (permalink / raw)
  To: Darrick J. Wong
  Cc: Eric Sandeen, Thomas Goirand, 1000974, Giovanni Mascellani,
	xfslibs-dev, xfs, gustavoars

On Sun, Dec 05, 2021 at 09:49:51AM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> Back in mid-2021, Kees and Gustavo rammed into the kernel a bunch of
> static checker "improvements" that redefined '/* fallthrough */'
> comments for switch statements as a macro that virtualizes either that
> same comment, a do-while loop, or a compiler __attribute__.  This was
> necessary to work around the poor decision-making of the clang, gcc, and
> C language standard authors, who collectively came up with four mutually
> incompatible ways to document a lack of branching in a code flow.
> 
> Having received ZERO HELP porting this to userspace, Eric and I

Look, I know you don't like this feature, but claiming that you received
no help with it is just false. I explicitly offered to help with xfsprogs,
and even sent a first-attempt at a patch to do so[1], which looks very
similar to what you've got here, almost 6 months later. I even went
through and changed all the comments to an explicitly XFS-specific
macro when you made it clear you hated the statement-like "fallthrough"
macro name.

I continue to be baffled about this whole saga. We're all trying to help
make Linux better, and I went out of my way to help with xfsprogs too to
minimize the impact on you (since you said you wanted to have nothing to
do with it), yet Gustavo and I got continually flamed by yourself and
Dave, including even now in this very misleading commit log.

What is going on here?

-Kees

[1] https://lore.kernel.org/lkml/202105280915.9117D7C@keescook/

-- 
Kees Cook

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Bug#1000974: [PATCH xfsprogs-5.14.2 URGENT] libxfs: hide the drainbamaged fallthrough macro from xfslibs
@ 2021-12-06 20:15       ` Kees Cook
  0 siblings, 0 replies; 14+ messages in thread
From: Kees Cook @ 2021-12-06 20:15 UTC (permalink / raw)
  To: Darrick J. Wong
  Cc: Eric Sandeen, Thomas Goirand, 1000974, Giovanni Mascellani,
	xfslibs-dev, xfs, gustavoars

On Sun, Dec 05, 2021 at 09:49:51AM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> Back in mid-2021, Kees and Gustavo rammed into the kernel a bunch of
> static checker "improvements" that redefined '/* fallthrough */'
> comments for switch statements as a macro that virtualizes either that
> same comment, a do-while loop, or a compiler __attribute__.  This was
> necessary to work around the poor decision-making of the clang, gcc, and
> C language standard authors, who collectively came up with four mutually
> incompatible ways to document a lack of branching in a code flow.
> 
> Having received ZERO HELP porting this to userspace, Eric and I

Look, I know you don't like this feature, but claiming that you received
no help with it is just false. I explicitly offered to help with xfsprogs,
and even sent a first-attempt at a patch to do so[1], which looks very
similar to what you've got here, almost 6 months later. I even went
through and changed all the comments to an explicitly XFS-specific
macro when you made it clear you hated the statement-like "fallthrough"
macro name.

I continue to be baffled about this whole saga. We're all trying to help
make Linux better, and I went out of my way to help with xfsprogs too to
minimize the impact on you (since you said you wanted to have nothing to
do with it), yet Gustavo and I got continually flamed by yourself and
Dave, including even now in this very misleading commit log.

What is going on here?

-Kees

[1] https://lore.kernel.org/lkml/202105280915.9117D7C@keescook/

-- 
Kees Cook

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Bug#1000974: marked as done (xfs/linux.h defines common word "fallthrough" breaking unrelated headers)
       [not found] ` <163839370805.58768.6385074074873965943.reportbug@zbuz.infomaniak.ch>
                     ` (2 preceding siblings ...)
  2021-12-05 19:00   ` Bug#1000974: Info received (Bug#1000974: copy_move_algo.hpp:1083:10: error: ‘__fallthrough__’ was not declared in this scope; did you mean ‘fallthrough’?) Thomas Goirand
@ 2021-12-07 12:36   ` Debian Bug Tracking System
  2021-12-07 12:54   ` Debian Bug Tracking System
  4 siblings, 0 replies; 14+ messages in thread
From: Debian Bug Tracking System @ 2021-12-07 12:36 UTC (permalink / raw)
  To: Bastian Germann

[-- Attachment #1: Type: text/plain, Size: 865 bytes --]

Your message dated Tue, 7 Dec 2021 13:32:48 +0100
with message-id <1ae1f860-e164-4e25-0c59-d4a3bb8587c2@debian.org>
and subject line fixed upstream
has caused the Debian Bug report #1000974,
regarding xfs/linux.h defines common word "fallthrough" breaking unrelated headers
to be marked as done.

This means that you claim that the problem has been dealt with.
If this is not the case it is now your responsibility to reopen the
Bug report if necessary, and/or fix the problem forthwith.

(NB: If you are a system administrator and have no idea what this
message is talking about, this may indicate a serious mail system
misconfiguration somewhere. Please contact owner@bugs.debian.org
immediately.)


-- 
1000974: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1000974
Debian Bug Tracking System
Contact owner@bugs.debian.org with problems

[-- Attachment #2: Type: message/rfc822, Size: 3861 bytes --]

From: Thomas Goirand <zigo@debian.org>
To: Debian Bug Tracking System <submit@bugs.debian.org>
Subject: copy_move_algo.hpp:1083:10: error: ‘__fallthrough__’ was not declared in this scope; did you mean ‘fallthrough’?
Date: Wed, 01 Dec 2021 22:21:48 +0100
Message-ID: <163839370805.58768.6385074074873965943.reportbug@zbuz.infomaniak.ch>

Package: libboost1.74-dev
Version: 1.74.0-13
Severity: important

Hi there!

When building Ceph (current version in Experimental), since a few days/weeks,
I get this:

In file included from /root/ceph/ceph/src/seastar/src/core/file.cc:28:
/usr/include/boost/container/detail/copy_move_algo.hpp: In function ‘typename boost::move_detail::enable_if_c<(boost::container::dtl::is_memtransfer_copy_assignable<F, G>::value && true), void>::type boost::container::deep_swap_alloc_n(A
llocator&, F, typename boost::container::allocator_traits<Allocator>::size_type, G, typename boost::container::allocator_traits<Allocator>::size_type)’:
/usr/include/boost/container/detail/copy_move_algo.hpp:1083:10: error: ‘__fallthrough__’ was not declared in this scope; did you mean ‘fallthrough’?
 1083 |          BOOST_FALLTHROUGH;
      |          ^~~~~~~~~~~~~~~~~

To me, it looks like a bug in Boost itself, as this is where the
BOOST_FALLTHROUGH macro is defined.

Note that in Ceph, I had the same issue, until I redefined the
macro like this directly:
#define FMT_FALLTHROUGH [[gnu::fallthrough]]

removing the conditional:
#if __cplusplus == 201103L || __cplusplus == 201402L

which doesn't seem to get some matching, and then the error in
the subject of this bug report went away, at least for the Ceph
part (well, fmt which Ceph embedds...).

Is this still a problem in Ceph, or am I right that it probably
is an issue in Boost?

I can't get my head around this issue which has been blocking me
for weeks. What's annoying is that it used to build fine, and
I don't know what changed in unstable to make it fail to way.
If the issue is in Ceph's code, then I would happily recieve
advices.

Your thoughts?

Cheers,

Thomas Goirand (zigo)

[-- Attachment #3: Type: message/rfc822, Size: 2568 bytes --]

From: Bastian Germann <bage@debian.org>
To: 1000974-done@bugs.debian.org
Subject: fixed upstream
Date: Tue, 7 Dec 2021 13:32:48 +0100
Message-ID: <1ae1f860-e164-4e25-0c59-d4a3bb8587c2@debian.org>

Version: 5.14.2-1

This was fixed by commit 135ce1ed0a99 (libxfs: hide the drainbamaged fallthrough macro from xfslibs)

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Bug#1000974: marked as done (xfs/linux.h defines common word "fallthrough" breaking unrelated headers)
       [not found] ` <163839370805.58768.6385074074873965943.reportbug@zbuz.infomaniak.ch>
                     ` (3 preceding siblings ...)
  2021-12-07 12:36   ` Bug#1000974: marked as done (xfs/linux.h defines common word "fallthrough" breaking unrelated headers) Debian Bug Tracking System
@ 2021-12-07 12:54   ` Debian Bug Tracking System
  4 siblings, 0 replies; 14+ messages in thread
From: Debian Bug Tracking System @ 2021-12-07 12:54 UTC (permalink / raw)
  To: Nathan Scott

[-- Attachment #1: Type: text/plain, Size: 879 bytes --]

Your message dated Tue, 07 Dec 2021 12:50:53 +0000
with message-id <E1muZvp-000DCs-Vb@fasolo.debian.org>
and subject line Bug#1000974: fixed in xfsprogs 5.14.2-1
has caused the Debian Bug report #1000974,
regarding xfs/linux.h defines common word "fallthrough" breaking unrelated headers
to be marked as done.

This means that you claim that the problem has been dealt with.
If this is not the case it is now your responsibility to reopen the
Bug report if necessary, and/or fix the problem forthwith.

(NB: If you are a system administrator and have no idea what this
message is talking about, this may indicate a serious mail system
misconfiguration somewhere. Please contact owner@bugs.debian.org
immediately.)


-- 
1000974: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1000974
Debian Bug Tracking System
Contact owner@bugs.debian.org with problems

[-- Attachment #2: Type: message/rfc822, Size: 3861 bytes --]

From: Thomas Goirand <zigo@debian.org>
To: Debian Bug Tracking System <submit@bugs.debian.org>
Subject: copy_move_algo.hpp:1083:10: error: ‘__fallthrough__’ was not declared in this scope; did you mean ‘fallthrough’?
Date: Wed, 01 Dec 2021 22:21:48 +0100
Message-ID: <163839370805.58768.6385074074873965943.reportbug@zbuz.infomaniak.ch>

Package: libboost1.74-dev
Version: 1.74.0-13
Severity: important

Hi there!

When building Ceph (current version in Experimental), since a few days/weeks,
I get this:

In file included from /root/ceph/ceph/src/seastar/src/core/file.cc:28:
/usr/include/boost/container/detail/copy_move_algo.hpp: In function ‘typename boost::move_detail::enable_if_c<(boost::container::dtl::is_memtransfer_copy_assignable<F, G>::value && true), void>::type boost::container::deep_swap_alloc_n(A
llocator&, F, typename boost::container::allocator_traits<Allocator>::size_type, G, typename boost::container::allocator_traits<Allocator>::size_type)’:
/usr/include/boost/container/detail/copy_move_algo.hpp:1083:10: error: ‘__fallthrough__’ was not declared in this scope; did you mean ‘fallthrough’?
 1083 |          BOOST_FALLTHROUGH;
      |          ^~~~~~~~~~~~~~~~~

To me, it looks like a bug in Boost itself, as this is where the
BOOST_FALLTHROUGH macro is defined.

Note that in Ceph, I had the same issue, until I redefined the
macro like this directly:
#define FMT_FALLTHROUGH [[gnu::fallthrough]]

removing the conditional:
#if __cplusplus == 201103L || __cplusplus == 201402L

which doesn't seem to get some matching, and then the error in
the subject of this bug report went away, at least for the Ceph
part (well, fmt which Ceph embedds...).

Is this still a problem in Ceph, or am I right that it probably
is an issue in Boost?

I can't get my head around this issue which has been blocking me
for weeks. What's annoying is that it used to build fine, and
I don't know what changed in unstable to make it fail to way.
If the issue is in Ceph's code, then I would happily recieve
advices.

Your thoughts?

Cheers,

Thomas Goirand (zigo)

[-- Attachment #3: Type: message/rfc822, Size: 5238 bytes --]

From: Debian FTP Masters <ftpmaster@ftp-master.debian.org>
To: 1000974-close@bugs.debian.org
Subject: Bug#1000974: fixed in xfsprogs 5.14.2-1
Date: Tue, 07 Dec 2021 12:50:53 +0000
Message-ID: <E1muZvp-000DCs-Vb@fasolo.debian.org>

Source: xfsprogs
Source-Version: 5.14.2-1
Done: Nathan Scott <nathans@debian.org>

We believe that the bug you reported is fixed in the latest version of
xfsprogs, which is due to be installed in the Debian FTP archive.

A summary of the changes between this version and the previous one is
attached.

Thank you for reporting the bug, which will now be closed.  If you
have further comments please address them to 1000974@bugs.debian.org,
and the maintainer will reopen the bug report if appropriate.

Debian distribution maintenance software
pp.
Nathan Scott <nathans@debian.org> (supplier of updated xfsprogs package)

(This message was generated automatically at their request; if you
believe that there is a problem with it please contact the archive
administrators by mailing ftpmaster@ftp-master.debian.org)


-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

Format: 1.8
Date: Mon, 06 Dec 2021 14:26:57 -0500
Source: xfsprogs
Architecture: source
Version: 5.14.2-1
Distribution: unstable
Urgency: medium
Maintainer: XFS Development Team <linux-xfs@vger.kernel.org>
Changed-By: Nathan Scott <nathans@debian.org>
Closes: 1000974
Changes:
 xfsprogs (5.14.2-1) unstable; urgency=medium
 .
   * New upstream release
     - Move rogue fallthrough macro out of linux.h (Closes: #1000974)
Checksums-Sha1:
 7d58d8887618f090bfc2b5c0d9c309da788ba960 2017 xfsprogs_5.14.2-1.dsc
 035e552cf4a08d5dbe1330ec1e3e6ceeb21b6bc9 1308912 xfsprogs_5.14.2.orig.tar.xz
 ef951cda194275eee5c3975cfd46544cf0f70a67 14316 xfsprogs_5.14.2-1.debian.tar.xz
 2d71d75e23c654180f1c794e4d5c8c569f62873f 6141 xfsprogs_5.14.2-1_source.buildinfo
Checksums-Sha256:
 6fcd3408e1c1edc0d776f54323424dd36eeaffc148191911c464d5a02b2bad6b 2017 xfsprogs_5.14.2-1.dsc
 01ccd3ef9df2837753a5d876b8da84ea957d13d7a461b8c46e8afa4eb09aabc8 1308912 xfsprogs_5.14.2.orig.tar.xz
 9581848bd59128cdd38ccaa4c92d82d5ddd66c74baa9dc2fc76fb5146497af4a 14316 xfsprogs_5.14.2-1.debian.tar.xz
 4d20924cfc79905a769dbff0aca10eab696536aab5f591e36e9e35001f63a5f6 6141 xfsprogs_5.14.2-1_source.buildinfo
Files:
 14d5f362b8d4ea4bee80edb78a36ad9e 2017 admin optional xfsprogs_5.14.2-1.dsc
 597e7067b8aa24d440bc87c29e987377 1308912 admin optional xfsprogs_5.14.2.orig.tar.xz
 89ea7c8a994352dcecd267a0ec13136c 14316 admin optional xfsprogs_5.14.2-1.debian.tar.xz
 4e41a97053eeb8ec0c6547e293683776 6141 admin optional xfsprogs_5.14.2-1_source.buildinfo

-----BEGIN PGP SIGNATURE-----

iQGzBAEBCgAdFiEEQGIgyLhVKAI3jM5BH1x6i0VWQxQFAmGvU4sACgkQH1x6i0VW
QxS0egv9HwfcbxyRn43bj+CxNKMCdKtgIPfV0SZxejKwarbdDg3d8f4/1i7gNVpJ
+isPdnKvVULyxEhQxdoYfsjWaTCCX3YMlyrd6i5+2nrn6J9cwXrrJJdjPWvMgXR2
o2u+brvm9cPYllAbbEVUYe6dtHY+/cH1c16W1SYK3yln7MWWWCTvAT0a7YrnrZQj
7FVdd0qcbM+PU/m2qwpUxz2KmtVjJ8RfiF+a/X7P1zeAxAoODFx3GRRY9cTyvgnV
tDwAdlSu4W6UgiKwgTpDFUT6EPRigEEqjTk3k0BgvYeFEidX57TsknQXwWKn3dI/
Dxfo5wxGVlDIywj4iN9EugsYCBwW8ggm60St/BiJhuNpgup339yUBGtszm/x97RM
XLKTOgb642bqTVu1Otee+Bdd5VSMwlmkyUnDDvuh52EPF3uFNf55sgiYsv02ATrB
jb8rcN1zeqUZ9r+9m6w65pp2wQX6HAWqZSUEl2HToPITse3OPwwCPUKZja+BYpT5
bGdytpiC
=ORx6
-----END PGP SIGNATURE-----

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2021-12-07 12:54 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <1ae1f860-e164-4e25-0c59-d4a3bb8587c2@debian.org>
     [not found] ` <163839370805.58768.6385074074873965943.reportbug@zbuz.infomaniak.ch>
     [not found]   ` <9b9bda73-9554-0c75-824d-f8d1b9b98e19@debian.org>
     [not found]     ` <7686ac7e-0df1-a98c-27ce-51dc5e46c55e@debian.org>
     [not found]       ` <c7ccff50-c177-7f96-2d99-2077f77374ad@debian.org>
2021-12-05 15:57         ` Processed (with 1 error): Re: Bug#1000974: copy_move_algo.hpp:1083:10: error: ‘__fallthrough__’ was not declared in this scope; did you mean ‘fallthrough’? Debian Bug Tracking System
2021-12-05 17:33         ` Thomas Goirand
2021-12-05 17:45           ` Darrick J. Wong
2021-12-05 17:49   ` [PATCH xfsprogs-5.14.2 URGENT] libxfs: hide the drainbamaged fallthrough macro from xfslibs Darrick J. Wong
2021-12-05 17:49     ` Bug#1000974: " Darrick J. Wong
2021-12-05 21:10     ` Dave Chinner
2021-12-05 21:10       ` Dave Chinner
2021-12-06 14:26     ` Eric Sandeen
2021-12-06 14:26       ` Bug#1000974: " Eric Sandeen
2021-12-06 20:15     ` Kees Cook
2021-12-06 20:15       ` Bug#1000974: " Kees Cook
2021-12-05 19:00   ` Bug#1000974: Info received (Bug#1000974: copy_move_algo.hpp:1083:10: error: ‘__fallthrough__’ was not declared in this scope; did you mean ‘fallthrough’?) Thomas Goirand
2021-12-07 12:36   ` Bug#1000974: marked as done (xfs/linux.h defines common word "fallthrough" breaking unrelated headers) Debian Bug Tracking System
2021-12-07 12:54   ` Debian Bug Tracking System

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.