| Message ID | 20221215162905.3960806-1-arnd@kernel.org |
|---|---|
| State | New |
| Headers | show |
| Series | media: camss: csiphy-3ph: avoid undefined behavior | expand |
On Thu, 15 Dec 2022 at 17:29, Arnd Bergmann <arnd@kernel.org> wrote: > > From: Arnd Bergmann <arnd@arndb.de> > > Marking a case of the switch statement as unreachable means the > compiler treats it as undefined behavior, which is then caught by > an objtool warning: > > drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.o: warning: objtool: csiphy_lanes_enable() falls through to next function csiphy_lanes_disable() > > Instead of simply continuing execution at a random place of the > driver, print a warning and return from to the caller, which > makes it possible to understand what happens and avoids the > warning. > > Fixes: 53655d2a0ff2 ("media: camss: csiphy-3ph: add support for SM8250 CSI DPHY") > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > --- > drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c b/drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c > index 451a4c9b3d30..04baa80494c6 100644 > --- a/drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c > +++ b/drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c > @@ -429,7 +429,8 @@ static void csiphy_gen2_config_lanes(struct csiphy_device *csiphy, > array_size = ARRAY_SIZE(lane_regs_sm8250[0]); > break; > default: > - unreachable(); > + WARN(1, "unknown cspi version\n"); > + return; > } > > for (l = 0; l < 5; l++) { > -- > 2.35.1 > Reviewed-by: Robert Foss <robert.foss@linaro.org>
On Thu, Dec 15, 2022 at 05:28:46PM +0100, Arnd Bergmann wrote: > From: Arnd Bergmann <arnd@arndb.de> > > Marking a case of the switch statement as unreachable means the > compiler treats it as undefined behavior, which is then caught by > an objtool warning: > > drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.o: warning: objtool: csiphy_lanes_enable() falls through to next function csiphy_lanes_disable() > > Instead of simply continuing execution at a random place of the > driver, print a warning and return from to the caller, which > makes it possible to understand what happens and avoids the > warning. > > Fixes: 53655d2a0ff2 ("media: camss: csiphy-3ph: add support for SM8250 CSI DPHY") > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > --- > drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c b/drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c > index 451a4c9b3d30..04baa80494c6 100644 > --- a/drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c > +++ b/drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c > @@ -429,7 +429,8 @@ static void csiphy_gen2_config_lanes(struct csiphy_device *csiphy, > array_size = ARRAY_SIZE(lane_regs_sm8250[0]); > break; > default: > - unreachable(); > + WARN(1, "unknown cspi version\n"); > + return; > } So no real objection here; but unreachable() does have an objtool annotation inside, so clearly the compiler managed to defeat that -- perhaps we should look at that too.
On Fri, Dec 16, 2022, at 12:23, Peter Zijlstra wrote: > On Thu, Dec 15, 2022 at 05:28:46PM +0100, Arnd Bergmann wrote: >> diff --git a/drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c b/drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c >> index 451a4c9b3d30..04baa80494c6 100644 >> --- a/drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c >> +++ b/drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c >> @@ -429,7 +429,8 @@ static void csiphy_gen2_config_lanes(struct csiphy_device *csiphy, >> array_size = ARRAY_SIZE(lane_regs_sm8250[0]); >> break; >> default: >> - unreachable(); >> + WARN(1, "unknown cspi version\n"); >> + return; >> } > > So no real objection here; but unreachable() does have an objtool > annotation inside, so clearly the compiler managed to defeat that -- > perhaps we should look at that too. Ah, I forgot this annotation existed. I see that this particular objtool warning only happens with clang (I used version 14.0.6), but it does not happen with gcc-12.2. I see the function ends in jmp .LBB3_45 .LBB3_54: #APP .Ltmp0: .section .discard.unreachable,"",@progbits .Ltmp1: .long .Ltmp0-.Ltmp1 .text #NO_APP .Lfunc_end3: .size csiphy_lanes_enable, .Lfunc_end3-csiphy_lanes_enable # -- End function full assembler output at https://pastebin.com/F8HGCUvk, object file attached. Arnd
diff --git a/drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c b/drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c index 451a4c9b3d30..04baa80494c6 100644 --- a/drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c +++ b/drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c @@ -429,7 +429,8 @@ static void csiphy_gen2_config_lanes(struct csiphy_device *csiphy, array_size = ARRAY_SIZE(lane_regs_sm8250[0]); break; default: - unreachable(); + WARN(1, "unknown cspi version\n"); + return; } for (l = 0; l < 5; l++) {