Bug 25267 - kernel 5.3: error: this statement may fall through [-Werror=implicit-fallthrough=]
Summary: kernel 5.3: error: this statement may fall through [-Werror=implicit-fallthro...
Status: RESOLVED OBSOLETE
Alias: None
Product: systemtap
Classification: Unclassified
Component: runtime (show other bugs)
Version: unspecified
: P2 normal
Target Milestone: ---
Assignee: Unassigned
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-12-10 06:23 UTC by Craig Ringer
Modified: 2020-07-13 06:51 UTC (History)
1 user (show)

See Also:
Host:
Target:
Build:
Last reconfirmed:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Craig Ringer 2019-12-10 06:23:12 UTC
When building on kernel 5.3.14 with stap 4.2/0.177 (git describe = release-4.2-6-g0c5c0f434) stap modules fail to build due to a runtime configure bug detailed in #25265 . When this is worked around locally the build then fails due to -Werror complaints from gcc 9.2.1 :

~~~
In file included from /usr/local/share/systemtap/runtime/unwind.c:16,
                 from /usr/local/share/systemtap/runtime/linux/runtime.h:255,
                 from /usr/local/share/systemtap/runtime/runtime.h:26,
                 from /tmp/stapsiNon3/stap_767845_src.c:27:
/usr/local/share/systemtap/runtime/unwind/unwind.h: In function ‘read_ptr_sect’:
/usr/local/share/systemtap/runtime/unwind/unwind.h:146:20: error: this statement may fall through [-Werror=implicit-fallthrough=]
  146 |   if (!compat_task || (compat_task && (tableSize == 4 || tableSize == 0)))
      |       ~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/usr/local/share/systemtap/runtime/unwind/unwind.h:157:2: note: here
  157 |  case DW_EH_PE_data8:
      |  ^~~~
In file included from ./include/asm-generic/bug.h:5,
                 from ./arch/x86/include/asm/bug.h:83,
                 from ./include/linux/bug.h:5,
                 from ./include/linux/mmdebug.h:5,
                 from ./include/linux/gfp.h:5,
                 from /usr/local/share/systemtap/runtime/linux/runtime_defines.h:20,
                 from /usr/local/share/systemtap/runtime/runtime_defines.h:8,
                 from /tmp/stapsiNon3/stap_767845_src.c:11:
./include/linux/compiler.h:328:5: error: this statement may fall through [-Werror=implicit-fallthrough=]
  328 |  do {        \
      |     ^
./include/linux/compiler.h:338:2: note: in expansion of macro ‘__compiletime_assert’
  338 |  __compiletime_assert(condition, msg, prefix, suffix)
      |  ^~~~~~~~~~~~~~~~~~~~
./include/linux/compiler.h:350:2: note: in expansion of macro ‘_compiletime_assert’
  350 |  _compiletime_assert(condition, msg, __compiletime_assert_, __LINE__)
      |  ^~~~~~~~~~~~~~~~~~~
./include/linux/build_bug.h:39:37: note: in expansion of macro ‘compiletime_assert’
   39 | #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
      |                                     ^~~~~~~~~~~~~~~~~~
./include/linux/build_bug.h:50:2: note: in expansion of macro ‘BUILD_BUG_ON_MSG’
   50 |  BUILD_BUG_ON_MSG(condition, "BUILD_BUG_ON failed: " #condition)
      |  ^~~~~~~~~~~~~~~~
/usr/local/share/systemtap/runtime/unwind/unwind.h:158:3: note: in expansion of macro ‘BUILD_BUG_ON’
  158 |   BUILD_BUG_ON(sizeof(u64) != sizeof(value));
      |   ^~~~~~~~~~~~
In file included from /usr/local/share/systemtap/runtime/unwind.c:16,
                 from /usr/local/share/systemtap/runtime/linux/runtime.h:255,
                 from /usr/local/share/systemtap/runtime/runtime.h:26,
                 from /tmp/stapsiNon3/stap_767845_src.c:27:
/usr/local/share/systemtap/runtime/unwind/unwind.h:163:2: note: here
  163 |  case DW_EH_PE_absptr:
      |  ^~~~
In file included from /usr/local/share/systemtap/runtime/linux/runtime.h:255,
                 from /usr/local/share/systemtap/runtime/runtime.h:26,
                 from /tmp/stapsiNon3/stap_767845_src.c:27:
/usr/local/share/systemtap/runtime/unwind.c: In function ‘processCFI’:
/usr/local/share/systemtap/runtime/unwind.c:519:8: error: this statement may fall through [-Werror=implicit-fallthrough=]
  519 |     if (compat_task) {
      |        ^
/usr/local/share/systemtap/runtime/unwind.c:531:4: note: here
  531 |    case DW_CFA_def_cfa_offset:
      |    ^~~~
/usr/local/share/systemtap/runtime/unwind.c:543:8: error: this statement may fall through [-Werror=implicit-fallthrough=]
  543 |     if (compat_task) {
      |        ^
/usr/local/share/systemtap/runtime/unwind.c:553:4: note: here
  553 |    case DW_CFA_def_cfa_offset_sf:
      |    ^~~~
cc1: all warnings being treated as errors
~~~

Editing the generated Makefile to remove -Werror i.e.


- EXTRA_CFLAGS += -Wno-unused $(call cc-option,-Wno-tautological-compare) -Werror
+ EXTRA_CFLAGS += -Wno-unused $(call cc-option,-Wno-tautological-compare)


permits the trace module to compile. However, buildrun.cxx says

~~~
  // A bit of obfuscation for Gentoo's sake.
  // We *need* -Werror for stapconf to work correctly.
  // https://bugs.gentoo.org/show_bug.cgi?id=522908
  #define WERROR ("-W" "error")
~~~

... so I presume some of the runtime configure tests are relying on things like implicit function declaration warnings, and anyway it's not desirable to just ignore warnings.

It's unfortunate that staprun offers no way to disable -Werror or selectively suppress some -Werror sub-categories though, as it makes stap very fragile in the face of compiler and kernel changes.
Comment 1 Craig Ringer 2019-12-10 06:35:53 UTC
I wrote this up earlier as https://stackoverflow.com/questions/58443706/systemtap-stap-probes-fail-with-this-statement-may-fall-through-werror-impl 

Cross linked there.

Workaround:

```
diff --git a/buildrun.cxx b/buildrun.cxx
index 505902bc5..b29eeb797 100644
--- a/buildrun.cxx
+++ b/buildrun.cxx
@@ -235,6 +235,7 @@ compile_dyninst (systemtap_session& s)
       "gcc", "--std=gnu99", s.translated_source, "-o", module,
       "-fvisibility=hidden", "-O2", "-I" + s.runtime_path, "-D__DYNINST__",
       "-Wall", WERROR, "-Wno-unused", "-Wno-strict-aliasing",
+      "-Wno-error=implicit-fallthrough", "-Wno-error=strict-prototypes",
       "-pthread", "-lrt", "-fPIC", "-shared",
     };
 
```
Comment 2 Craig Ringer 2019-12-10 08:02:32 UTC
Correction, workaround is

```
commit 495ed9143004e62284f959f0a7630bdf229c6e62 (HEAD -> master)
Author: Craig Ringer <craig@2ndquadrant.com>
Date:   Tue Dec 10 15:24:15 2019 +0800

    Work around bug #25267 on Linux 5.3

diff --git a/buildrun.cxx b/buildrun.cxx
index 505902bc5..5f9a7208a 100644
--- a/buildrun.cxx
+++ b/buildrun.cxx
@@ -235,6 +235,7 @@ compile_dyninst (systemtap_session& s)
       "gcc", "--std=gnu99", s.translated_source, "-o", module,
       "-fvisibility=hidden", "-O2", "-I" + s.runtime_path, "-D__DYNINST__",
       "-Wall", WERROR, "-Wno-unused", "-Wno-strict-aliasing",
+      "-Wno-error=implicit-fallthrough",
       "-pthread", "-lrt", "-fPIC", "-shared",
     };
 
@@ -539,6 +540,7 @@ compile_pass (systemtap_session& s)
 
   // Assumes linux 2.6 kbuild
   o << "EXTRA_CFLAGS += -Wno-unused $(call cc-option,-Wno-tautological-compare) " << WERROR
+    << "-Wno-error=implicit-fallthrough "
     << endl;
   #if CHECK_POINTER_ARITH_PR5947
   o << "EXTRA_CFLAGS += -Wpointer-arith" << endl;
```
Comment 3 Frank Ch. Eigler 2019-12-10 17:29:42 UTC
Craig, we have handled several such diagnostics in the past by adding in explicit /* fallthru */ type comments between the cases.  Any reason to think that won't do here in the case of these unwind.[ch]?
Comment 4 Craig Ringer 2019-12-13 05:59:02 UTC
Sounds fine to me. I wasn't aware that the gcc's -Wimplicit-fallthrough saw and respected such comments.
Comment 5 Craig Ringer 2019-12-13 07:24:44 UTC
BTW there's a simple patch on #25265 to fix the other issue reported
Comment 6 Craig Ringer 2020-07-13 06:51:50 UTC
I'm no longer seeing this on Fedora 32 and 5.6.19-300.fc32.x86_64 so I'm marking it withdrawn.