Intel PT decoder switch case fallthrough cases reported by gcc 7

Intel PT decoder switch case fallthrough cases reported by gcc 7

[Arnaldo Carvalho de Melo]

Hi,

	I've updated the container with Fedora Rawhide I use to build
tools/perf/ and samples/bcc/ and it now comes with gcc 7, where I get
things like:

  CC       /tmp/build/perf/tests/code-reading.o
util/intel-pt-decoder/intel-pt-decoder.c: In function 'intel_pt_walk_psb':
util/intel-pt-decoder/intel-pt-decoder.c:1748:31: error: this statement may fall through [-Werror=implicit-fallthrough=]
    decoder->continuous_period = false;
                               ^
util/intel-pt-decoder/intel-pt-decoder.c:1749:3: note: here
   case INTEL_PT_TIP_PGE:
   ^~~~
util/intel-pt-decoder/intel-pt-decoder.c:1801:4: error: this statement may fall through [-Werror=implicit-fallthrough=]
    intel_pt_clear_tx_flags(decoder);
    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
util/intel-pt-decoder/intel-pt-decoder.c:1802:3: note: here
   case INTEL_PT_TNT:
   ^~~~
util/intel-pt-decoder/intel-pt-decoder.c: In function 'intel_pt_walk_to_ip':
util/intel-pt-decoder/intel-pt-decoder.c:1841:31: error: this statement may fall through [-Werror=implicit-fallthrough=]
    decoder->continuous_period = false;
                               ^
util/intel-pt-decoder/intel-pt-decoder.c:1842:3: note: here
   case INTEL_PT_TIP_PGE:
   ^~~~
  MKDIR    /tmp/build/perf/util/scripting-engines/


This gets solved with a new attribute, that you have to add where in the past
we added:

	/* Fall through */

To indicate that the fall through to the next case statement block is
intentional, so now I have a __fallthrough and I am addressing all the cases,
please check if the ones below are the ones intended for the Intel PT parts,
please Ack or advise, only one seemed like a bug, i.e. a break should be used,
but I'm not sure.

- Arnaldo


diff --git a/tools/perf/util/intel-pt-decoder/intel-pt-decoder.c b/tools/perf/util/intel-pt-decoder/intel-pt-decoder.c
index e4e7dc781d21..d4ed327a4908 100644
--- a/tools/perf/util/intel-pt-decoder/intel-pt-decoder.c
+++ b/tools/perf/util/intel-pt-decoder/intel-pt-decoder.c
@@ -1746,6 +1746,7 @@ static int intel_pt_walk_psb(struct intel_pt_decoder *decoder)
 		switch (decoder->packet.type) {
 		case INTEL_PT_TIP_PGD:
 			decoder->continuous_period = false;
+			__fallthrough;
 		case INTEL_PT_TIP_PGE:
 		case INTEL_PT_TIP:
 			intel_pt_log("ERROR: Unexpected packet\n");
@@ -1799,6 +1800,8 @@ static int intel_pt_walk_psb(struct intel_pt_decoder *decoder)
 			decoder->pge = false;
 			decoder->continuous_period = false;
 			intel_pt_clear_tx_flags(decoder);
+			break;
+
 		case INTEL_PT_TNT:
 			decoder->have_tma = false;
 			intel_pt_log("ERROR: Unexpected packet\n");
@@ -1839,6 +1842,7 @@ static int intel_pt_walk_to_ip(struct intel_pt_decoder *decoder)
 		switch (decoder->packet.type) {
 		case INTEL_PT_TIP_PGD:
 			decoder->continuous_period = false;
+			__fallthrough;
 		case INTEL_PT_TIP_PGE:
 		case INTEL_PT_TIP:
 			decoder->pge = decoder->packet.type != INTEL_PT_TIP_PGD;
diff --git a/tools/perf/util/intel-pt-decoder/intel-pt-pkt-decoder.c b/tools/perf/util/intel-pt-decoder/intel-pt-pkt-decoder.c
index 4f7b32020487..7528ae4f7e28 100644
--- a/tools/perf/util/intel-pt-decoder/intel-pt-pkt-decoder.c
+++ b/tools/perf/util/intel-pt-decoder/intel-pt-pkt-decoder.c
@@ -17,6 +17,7 @@
 #include <string.h>
 #include <endian.h>
 #include <byteswap.h>
+#include <linux/compiler.h>
 
 #include "intel-pt-pkt-decoder.h"
 
@@ -498,6 +499,7 @@ int intel_pt_pkt_desc(const struct intel_pt_pkt *packet, char *buf,
 	case INTEL_PT_FUP:
 		if (!(packet->count))
 			return snprintf(buf, buf_len, "%s no ip", name);
+		__fallthrough;
 	case INTEL_PT_CYC:
 	case INTEL_PT_VMCS:
 	case INTEL_PT_MTC:

Re: Intel PT decoder switch case fallthrough cases reported by gcc 7

[Andi Kleen]

On Thu, Feb 09, 2017 at 01:50:39PM -0300, Arnaldo Carvalho de Melo wrote:
> Hi,
> 
> 	I've updated the container with Fedora Rawhide I use to build
> tools/perf/ and samples/bcc/ and it now comes with gcc 7, where I get
> things like:

FWIW, but it just shows that you should never ship software with -Werror
enabled. New compiler releases add new warnings, and you just randomly
break the build for users who use newer compilers.

It's ok to use in your own builds, but should never be default.

The kernel gets it right, but perf is wrong here.

-Andi

Re: Intel PT decoder switch case fallthrough cases reported by gcc 7

[Arnaldo Carvalho de Melo]

Em Thu, Feb 09, 2017 at 10:25:19AM -0800, Andi Kleen escreveu:
> On Thu, Feb 09, 2017 at 01:50:39PM -0300, Arnaldo Carvalho de Melo wrote:
> > Hi,
> > 
> > 	I've updated the container with Fedora Rawhide I use to build
> > tools/perf/ and samples/bcc/ and it now comes with gcc 7, where I get
> > things like:
> 
> FWIW, but it just shows that you should never ship software with -Werror
> enabled. New compiler releases add new warnings, and you just randomly
> break the build for users who use newer compilers.
> 
> It's ok to use in your own builds, but should never be default.
> 
> The kernel gets it right, but perf is wrong here.

But what about my question? Do you think the changes are ok? I actually
made all be fallthrough, i.e. considered that the existing code was ok.

- Arnaldo

Re: Intel PT decoder switch case fallthrough cases reported by gcc 7

[Andi Kleen]

> But what about my question? Do you think the changes are ok? I actually
> made all be fallthrough, i.e. considered that the existing code was ok.

Yes the changes are fine.

-Andi

Re: Intel PT decoder switch case fallthrough cases reported by gcc 7

[Arnaldo Carvalho de Melo]

Em Thu, Feb 09, 2017 at 11:13:16AM -0800, Andi Kleen escreveu:
> > But what about my question? Do you think the changes are ok? I actually
> > made all be fallthrough, i.e. considered that the existing code was ok.
> 
> Yes the changes are fine.

Thanks!

- Arnaldo