| Message ID | 20210122002357.370836-4-erik.kaneda@intel.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v2,1/9] ACPICA: Fix exception code class checks | expand |
On Thu, Jan 21, 2021 at 04:23:51PM -0800, Erik Kaneda wrote: > From: Nick Desaulniers <ndesaulniers@google.com> > > ACPICA commit 4b9135f5774caa796ddf826448811e8e7f08ef2f > > GCC 7.1 gained -Wimplicit-fallthrough to warn on implicit fallthrough, > as well as __attribute__((__fallthrough__)) and comments to explicitly > denote that cases of fallthrough were intentional. Clang also supports > this warning and statement attribute, but not the comment form. > > Robert Moore provides additional context about the lint comments being > removed. They were for "an old version of PC-Lint, which we don't use > anymore." Drop those. > > This will help us enable -Wimplicit-fallthrough throughout the Linux > kernel. > > Suggested-by: Robert Moore <robert.moore@intel.com> > Reported-by: Jon Hunter <jonathanh@nvidia.com> > > Link: https://github.com/acpica/acpica/commit/4b9135f5 > Signed-off-by: Nick Desaulniers <ndesaulniers@google.com> > Signed-off-by: Bob Moore <robert.moore@intel.com> > Signed-off-by: Erik Kaneda <erik.kaneda@intel.com> With gcc 6.5 (and presumably each gcc version older than 7.1), this patch results in: drivers/acpi/acpica/dscontrol.c: In function ‘acpi_ds_exec_begin_control_op’: drivers/acpi/acpica/dscontrol.c:65:3: error: ‘ACPI_FALLTHROUGH’ undeclared and similar errors for other files. Guenter > --- > drivers/acpi/acpica/dscontrol.c | 2 +- > drivers/acpi/acpica/dswexec.c | 3 +-- > drivers/acpi/acpica/dswload.c | 2 +- > drivers/acpi/acpica/dswload2.c | 2 +- > drivers/acpi/acpica/exfldio.c | 2 +- > drivers/acpi/acpica/exresop.c | 4 ++-- > drivers/acpi/acpica/exstore.c | 4 ++-- > drivers/acpi/acpica/hwgpe.c | 2 +- > drivers/acpi/acpica/utdelete.c | 2 +- > include/acpi/actypes.h | 6 ++++++ > include/acpi/platform/acgcc.h | 15 +++++++++++++++ > 11 files changed, 32 insertions(+), 12 deletions(-) > > -- > 2.29.2 > > diff --git a/drivers/acpi/acpica/dscontrol.c b/drivers/acpi/acpica/dscontrol.c > index 4b5b6e859f62..b58ffc7acdb9 100644 > --- a/drivers/acpi/acpica/dscontrol.c > +++ b/drivers/acpi/acpica/dscontrol.c > @@ -62,7 +62,7 @@ acpi_ds_exec_begin_control_op(struct acpi_walk_state *walk_state, > } > } > > - /*lint -fallthrough */ > + ACPI_FALLTHROUGH; > > case AML_IF_OP: > /* > diff --git a/drivers/acpi/acpica/dswexec.c b/drivers/acpi/acpica/dswexec.c > index 1d4f8c81028c..4a9799246fae 100644 > --- a/drivers/acpi/acpica/dswexec.c > +++ b/drivers/acpi/acpica/dswexec.c > @@ -598,8 +598,7 @@ acpi_status acpi_ds_exec_end_op(struct acpi_walk_state *walk_state) > break; > } > > - /* Fall through */ > - /*lint -fallthrough */ > + ACPI_FALLTHROUGH; > > case AML_INT_EVAL_SUBTREE_OP: > > diff --git a/drivers/acpi/acpica/dswload.c b/drivers/acpi/acpica/dswload.c > index 27069325b6de..dd97c86f8e41 100644 > --- a/drivers/acpi/acpica/dswload.c > +++ b/drivers/acpi/acpica/dswload.c > @@ -224,7 +224,7 @@ acpi_ds_load1_begin_op(struct acpi_walk_state *walk_state, > break; > } > > - /*lint -fallthrough */ > + ACPI_FALLTHROUGH; > > default: > > diff --git a/drivers/acpi/acpica/dswload2.c b/drivers/acpi/acpica/dswload2.c > index edadbe146506..d9a3dfca7555 100644 > --- a/drivers/acpi/acpica/dswload2.c > +++ b/drivers/acpi/acpica/dswload2.c > @@ -214,7 +214,7 @@ acpi_ds_load2_begin_op(struct acpi_walk_state *walk_state, > break; > } > > - /*lint -fallthrough */ > + ACPI_FALLTHROUGH; > > default: > > diff --git a/drivers/acpi/acpica/exfldio.c b/drivers/acpi/acpica/exfldio.c > index ade35ff1c7ba..cde24e0fa6a8 100644 > --- a/drivers/acpi/acpica/exfldio.c > +++ b/drivers/acpi/acpica/exfldio.c > @@ -434,7 +434,7 @@ acpi_ex_field_datum_io(union acpi_operand_object *obj_desc, > * region_field case and write the datum to the Operation Region > */ > > - /*lint -fallthrough */ > + ACPI_FALLTHROUGH; > > case ACPI_TYPE_LOCAL_REGION_FIELD: > /* > diff --git a/drivers/acpi/acpica/exresop.c b/drivers/acpi/acpica/exresop.c > index 4d1b22971d58..4a0f8b8bfe62 100644 > --- a/drivers/acpi/acpica/exresop.c > +++ b/drivers/acpi/acpica/exresop.c > @@ -198,7 +198,7 @@ acpi_ex_resolve_operands(u16 opcode, > > target_op = AML_DEBUG_OP; > > - /*lint -fallthrough */ > + ACPI_FALLTHROUGH; > > case ACPI_REFCLASS_ARG: > case ACPI_REFCLASS_LOCAL: > @@ -264,7 +264,7 @@ acpi_ex_resolve_operands(u16 opcode, > * Else not a string - fall through to the normal Reference > * case below > */ > - /*lint -fallthrough */ > + ACPI_FALLTHROUGH; > > case ARGI_REFERENCE: /* References: */ > case ARGI_INTEGER_REF: > diff --git a/drivers/acpi/acpica/exstore.c b/drivers/acpi/acpica/exstore.c > index 3adc0a29d890..8fe33051275d 100644 > --- a/drivers/acpi/acpica/exstore.c > +++ b/drivers/acpi/acpica/exstore.c > @@ -96,7 +96,7 @@ acpi_ex_store(union acpi_operand_object *source_desc, > return_ACPI_STATUS(AE_OK); > } > > - /*lint -fallthrough */ > + ACPI_FALLTHROUGH; > > default: > > @@ -422,7 +422,7 @@ acpi_ex_store_object_to_node(union acpi_operand_object *source_desc, > break; > } > > - /* Fallthrough */ > + ACPI_FALLTHROUGH; > > case ACPI_TYPE_DEVICE: > case ACPI_TYPE_EVENT: > diff --git a/drivers/acpi/acpica/hwgpe.c b/drivers/acpi/acpica/hwgpe.c > index b13a4ed5bc63..0c84300e915c 100644 > --- a/drivers/acpi/acpica/hwgpe.c > +++ b/drivers/acpi/acpica/hwgpe.c > @@ -167,7 +167,7 @@ acpi_hw_low_set_gpe(struct acpi_gpe_event_info *gpe_event_info, u32 action) > return (AE_BAD_PARAMETER); > } > > - /*lint -fallthrough */ > + ACPI_FALLTHROUGH; > > case ACPI_GPE_ENABLE: > > diff --git a/drivers/acpi/acpica/utdelete.c b/drivers/acpi/acpica/utdelete.c > index 4c0d4e434196..624a26794d55 100644 > --- a/drivers/acpi/acpica/utdelete.c > +++ b/drivers/acpi/acpica/utdelete.c > @@ -112,7 +112,7 @@ static void acpi_ut_delete_internal_obj(union acpi_operand_object *object) > gpe_block); > } > > - /*lint -fallthrough */ > + ACPI_FALLTHROUGH; > > case ACPI_TYPE_PROCESSOR: > case ACPI_TYPE_THERMAL: > diff --git a/include/acpi/actypes.h b/include/acpi/actypes.h > index 647cb11d0a0a..2a32593691bc 100644 > --- a/include/acpi/actypes.h > +++ b/include/acpi/actypes.h > @@ -1286,4 +1286,10 @@ typedef enum { > > #define ACPI_OPT_END -1 > > +/* Definitions for explicit fallthrough */ > + > +#ifndef ACPI_FALLTHROUGH > +#define ACPI_FALLTHROUGH do {} while(0) > +#endif > + > #endif /* __ACTYPES_H__ */ > diff --git a/include/acpi/platform/acgcc.h b/include/acpi/platform/acgcc.h > index 7d63d03cf507..91f7a02c798a 100644 > --- a/include/acpi/platform/acgcc.h > +++ b/include/acpi/platform/acgcc.h > @@ -54,4 +54,19 @@ typedef __builtin_va_list va_list; > > #define ACPI_USE_NATIVE_MATH64 > > +/* GCC did not support __has_attribute until 5.1. */ > + > +#ifndef __has_attribute > +#define __has_attribute(x) 0 > +#endif > + > +/* > + * Explictly mark intentional explicit fallthrough to silence > + * -Wimplicit-fallthrough in GCC 7.1+. > + */ > + > +#if __has_attribute(__fallthrough__) > +#define ACPI_FALLTHROUGH __attribute__((__fallthrough__)) > +#endif > + > #endif /* __ACGCC_H__ */
On Sat, Jan 23, 2021 at 07:21:04AM -0800, Guenter Roeck wrote: > On Thu, Jan 21, 2021 at 04:23:51PM -0800, Erik Kaneda wrote: > > From: Nick Desaulniers <ndesaulniers@google.com> > > > > ACPICA commit 4b9135f5774caa796ddf826448811e8e7f08ef2f > > > > GCC 7.1 gained -Wimplicit-fallthrough to warn on implicit fallthrough, > > as well as __attribute__((__fallthrough__)) and comments to explicitly > > denote that cases of fallthrough were intentional. Clang also supports > > this warning and statement attribute, but not the comment form. > > > > Robert Moore provides additional context about the lint comments being > > removed. They were for "an old version of PC-Lint, which we don't use > > anymore." Drop those. > > > > This will help us enable -Wimplicit-fallthrough throughout the Linux > > kernel. > > > > Suggested-by: Robert Moore <robert.moore@intel.com> > > Reported-by: Jon Hunter <jonathanh@nvidia.com> > > > > Link: https://github.com/acpica/acpica/commit/4b9135f5 > > Signed-off-by: Nick Desaulniers <ndesaulniers@google.com> > > Signed-off-by: Bob Moore <robert.moore@intel.com> > > Signed-off-by: Erik Kaneda <erik.kaneda@intel.com> > > With gcc 6.5 (and presumably each gcc version older than 7.1), > this patch results in: > > drivers/acpi/acpica/dscontrol.c: In function ‘acpi_ds_exec_begin_control_op’: > drivers/acpi/acpica/dscontrol.c:65:3: error: ‘ACPI_FALLTHROUGH’ undeclared > > and similar errors for other files. > Wait, this differs from the patch in -next. > > +#ifndef ACPI_FALLTHROUGH > > +#define ACPI_FALLTHROUGH do {} while(0) This line is missing in -next. v2 doesn't say what changed in v2. Is that the change ? Guenter
On Sat, Jan 23, 2021 at 4:25 PM Guenter Roeck <linux@roeck-us.net> wrote: > > On Sat, Jan 23, 2021 at 07:21:04AM -0800, Guenter Roeck wrote: > > On Thu, Jan 21, 2021 at 04:23:51PM -0800, Erik Kaneda wrote: > > > From: Nick Desaulniers <ndesaulniers@google.com> > > > > > > ACPICA commit 4b9135f5774caa796ddf826448811e8e7f08ef2f > > > > > > GCC 7.1 gained -Wimplicit-fallthrough to warn on implicit fallthrough, > > > as well as __attribute__((__fallthrough__)) and comments to explicitly > > > denote that cases of fallthrough were intentional. Clang also supports > > > this warning and statement attribute, but not the comment form. > > > > > > Robert Moore provides additional context about the lint comments being > > > removed. They were for "an old version of PC-Lint, which we don't use > > > anymore." Drop those. > > > > > > This will help us enable -Wimplicit-fallthrough throughout the Linux > > > kernel. > > > > > > Suggested-by: Robert Moore <robert.moore@intel.com> > > > Reported-by: Jon Hunter <jonathanh@nvidia.com> > > > > > > Link: https://github.com/acpica/acpica/commit/4b9135f5 > > > Signed-off-by: Nick Desaulniers <ndesaulniers@google.com> > > > Signed-off-by: Bob Moore <robert.moore@intel.com> > > > Signed-off-by: Erik Kaneda <erik.kaneda@intel.com> > > > > With gcc 6.5 (and presumably each gcc version older than 7.1), > > this patch results in: > > > > drivers/acpi/acpica/dscontrol.c: In function ‘acpi_ds_exec_begin_control_op’: > > drivers/acpi/acpica/dscontrol.c:65:3: error: ‘ACPI_FALLTHROUGH’ undeclared > > > > and similar errors for other files. > > > > Wait, this differs from the patch in -next. > > > > +#ifndef ACPI_FALLTHROUGH > > > +#define ACPI_FALLTHROUGH do {} while(0) > > This line is missing in -next. > > v2 doesn't say what changed in v2. Is that the change ? Yes, it is. It should appear in -next on Monday.
diff --git a/drivers/acpi/acpica/dscontrol.c b/drivers/acpi/acpica/dscontrol.c index 4b5b6e859f62..b58ffc7acdb9 100644 --- a/drivers/acpi/acpica/dscontrol.c +++ b/drivers/acpi/acpica/dscontrol.c @@ -62,7 +62,7 @@ acpi_ds_exec_begin_control_op(struct acpi_walk_state *walk_state, } } - /*lint -fallthrough */ + ACPI_FALLTHROUGH; case AML_IF_OP: /* diff --git a/drivers/acpi/acpica/dswexec.c b/drivers/acpi/acpica/dswexec.c index 1d4f8c81028c..4a9799246fae 100644 --- a/drivers/acpi/acpica/dswexec.c +++ b/drivers/acpi/acpica/dswexec.c @@ -598,8 +598,7 @@ acpi_status acpi_ds_exec_end_op(struct acpi_walk_state *walk_state) break; } - /* Fall through */ - /*lint -fallthrough */ + ACPI_FALLTHROUGH; case AML_INT_EVAL_SUBTREE_OP: diff --git a/drivers/acpi/acpica/dswload.c b/drivers/acpi/acpica/dswload.c index 27069325b6de..dd97c86f8e41 100644 --- a/drivers/acpi/acpica/dswload.c +++ b/drivers/acpi/acpica/dswload.c @@ -224,7 +224,7 @@ acpi_ds_load1_begin_op(struct acpi_walk_state *walk_state, break; } - /*lint -fallthrough */ + ACPI_FALLTHROUGH; default: diff --git a/drivers/acpi/acpica/dswload2.c b/drivers/acpi/acpica/dswload2.c index edadbe146506..d9a3dfca7555 100644 --- a/drivers/acpi/acpica/dswload2.c +++ b/drivers/acpi/acpica/dswload2.c @@ -214,7 +214,7 @@ acpi_ds_load2_begin_op(struct acpi_walk_state *walk_state, break; } - /*lint -fallthrough */ + ACPI_FALLTHROUGH; default: diff --git a/drivers/acpi/acpica/exfldio.c b/drivers/acpi/acpica/exfldio.c index ade35ff1c7ba..cde24e0fa6a8 100644 --- a/drivers/acpi/acpica/exfldio.c +++ b/drivers/acpi/acpica/exfldio.c @@ -434,7 +434,7 @@ acpi_ex_field_datum_io(union acpi_operand_object *obj_desc, * region_field case and write the datum to the Operation Region */ - /*lint -fallthrough */ + ACPI_FALLTHROUGH; case ACPI_TYPE_LOCAL_REGION_FIELD: /* diff --git a/drivers/acpi/acpica/exresop.c b/drivers/acpi/acpica/exresop.c index 4d1b22971d58..4a0f8b8bfe62 100644 --- a/drivers/acpi/acpica/exresop.c +++ b/drivers/acpi/acpica/exresop.c @@ -198,7 +198,7 @@ acpi_ex_resolve_operands(u16 opcode, target_op = AML_DEBUG_OP; - /*lint -fallthrough */ + ACPI_FALLTHROUGH; case ACPI_REFCLASS_ARG: case ACPI_REFCLASS_LOCAL: @@ -264,7 +264,7 @@ acpi_ex_resolve_operands(u16 opcode, * Else not a string - fall through to the normal Reference * case below */ - /*lint -fallthrough */ + ACPI_FALLTHROUGH; case ARGI_REFERENCE: /* References: */ case ARGI_INTEGER_REF: diff --git a/drivers/acpi/acpica/exstore.c b/drivers/acpi/acpica/exstore.c index 3adc0a29d890..8fe33051275d 100644 --- a/drivers/acpi/acpica/exstore.c +++ b/drivers/acpi/acpica/exstore.c @@ -96,7 +96,7 @@ acpi_ex_store(union acpi_operand_object *source_desc, return_ACPI_STATUS(AE_OK); } - /*lint -fallthrough */ + ACPI_FALLTHROUGH; default: @@ -422,7 +422,7 @@ acpi_ex_store_object_to_node(union acpi_operand_object *source_desc, break; } - /* Fallthrough */ + ACPI_FALLTHROUGH; case ACPI_TYPE_DEVICE: case ACPI_TYPE_EVENT: diff --git a/drivers/acpi/acpica/hwgpe.c b/drivers/acpi/acpica/hwgpe.c index b13a4ed5bc63..0c84300e915c 100644 --- a/drivers/acpi/acpica/hwgpe.c +++ b/drivers/acpi/acpica/hwgpe.c @@ -167,7 +167,7 @@ acpi_hw_low_set_gpe(struct acpi_gpe_event_info *gpe_event_info, u32 action) return (AE_BAD_PARAMETER); } - /*lint -fallthrough */ + ACPI_FALLTHROUGH; case ACPI_GPE_ENABLE: diff --git a/drivers/acpi/acpica/utdelete.c b/drivers/acpi/acpica/utdelete.c index 4c0d4e434196..624a26794d55 100644 --- a/drivers/acpi/acpica/utdelete.c +++ b/drivers/acpi/acpica/utdelete.c @@ -112,7 +112,7 @@ static void acpi_ut_delete_internal_obj(union acpi_operand_object *object) gpe_block); } - /*lint -fallthrough */ + ACPI_FALLTHROUGH; case ACPI_TYPE_PROCESSOR: case ACPI_TYPE_THERMAL: diff --git a/include/acpi/actypes.h b/include/acpi/actypes.h index 647cb11d0a0a..2a32593691bc 100644 --- a/include/acpi/actypes.h +++ b/include/acpi/actypes.h @@ -1286,4 +1286,10 @@ typedef enum { #define ACPI_OPT_END -1 +/* Definitions for explicit fallthrough */ + +#ifndef ACPI_FALLTHROUGH +#define ACPI_FALLTHROUGH do {} while(0) +#endif + #endif /* __ACTYPES_H__ */ diff --git a/include/acpi/platform/acgcc.h b/include/acpi/platform/acgcc.h index 7d63d03cf507..91f7a02c798a 100644 --- a/include/acpi/platform/acgcc.h +++ b/include/acpi/platform/acgcc.h @@ -54,4 +54,19 @@ typedef __builtin_va_list va_list; #define ACPI_USE_NATIVE_MATH64 +/* GCC did not support __has_attribute until 5.1. */ + +#ifndef __has_attribute +#define __has_attribute(x) 0 +#endif + +/* + * Explictly mark intentional explicit fallthrough to silence + * -Wimplicit-fallthrough in GCC 7.1+. + */ + +#if __has_attribute(__fallthrough__) +#define ACPI_FALLTHROUGH __attribute__((__fallthrough__)) +#endif + #endif /* __ACGCC_H__ */