From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751770AbdCBX71 (ORCPT ); Thu, 2 Mar 2017 18:59:27 -0500 Received: from mx1.redhat.com ([209.132.183.28]:54078 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751719AbdCBX7Y (ORCPT ); Thu, 2 Mar 2017 18:59:24 -0500 Date: Thu, 2 Mar 2017 17:05:58 -0600 From: Josh Poimboeuf To: Arnd Bergmann Cc: Thomas Gleixner , Ingo Molnar , "H. Peter Anvin" , x86@kernel.org, Linux Kernel Mailing List , Denys Vlasenko Subject: Re: [PATCH] [RFC] x86: avoid -mtune=atom for objtool warnings Message-ID: <20170302230558.bhmo4q62v7tqjl2n@treble> References: <20161011155146.icyl3zewdvmms2h2@treble> <2252957.Vm1BYSSRqP@wuerfel> <20170301144008.3ocnbvry4sbl3cnu@treble> <20170301165355.w2kmdmbql2f2ouzg@treble> <20170302010342.fshczxfiiz3txgac@treble> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.6.0.1 (2016-04-01) X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.29]); Thu, 02 Mar 2017 23:06:01 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Mar 02, 2017 at 11:49:49PM +0100, Arnd Bergmann wrote: > On Thu, Mar 2, 2017 at 2:03 AM, Josh Poimboeuf wrote: > > On Wed, Mar 01, 2017 at 11:42:54PM +0100, Arnd Bergmann wrote: > >> On Wed, Mar 1, 2017 at 5:53 PM, Josh Poimboeuf wrote: > >> > On Wed, Mar 01, 2017 at 04:27:29PM +0100, Arnd Bergmann wrote: > >> > >> > I see no apparent reason for the ud2. > >> > >> It's the possible division by zero. This change would avoid the ud2: > >> > >> diff --git a/drivers/i2c/busses/i2c-img-scb.c b/drivers/i2c/busses/i2c-img-scb.c > >> index db8e8b40569d..a2b09c518225 100644 > >> --- a/drivers/i2c/busses/i2c-img-scb.c > >> +++ b/drivers/i2c/busses/i2c-img-scb.c > >> @@ -1196,6 +1196,8 @@ static int img_i2c_init(struct img_i2c *i2c) > >> clk_khz /= prescale; > >> > >> /* Setup the clock increment value */ > >> + if (clk_khz < 1) > >> + clk_khz = 1; > >> inc = (256 * 16 * bitrate_khz) / clk_khz; > >> > >> /* > > > > Ok, I see what gcc is doing. > > > > clk_khz = clk_get_rate(i2c->scb_clk) / 1000; > > ... > > inc = (256 * 16 * bitrate_khz) / clk_khz; > > > > Because CONFIG_HAVE_CLK isn't set, clk_get_rate() returns 0, which means > > clk_khz is always zero, so the last statement *always* results in a > > divide-by-zero. So that looks like a bug in the code. > > > > However, I'm baffled by how gcc handles it. Instead of: > > > > a) reporting a compile-time warning/error; or > > > > b) letting the #DE (divide error) exception happen; > > > > it inserts a 'ud2', resulting in a #UD (invalid opcode). Why?!? > > Just FYI, I found another one like this: > > 0000000000000000 : > 0: e8 00 00 00 00 callq 5 > 1: R_X86_64_PC32 __fentry__-0x4 > 5: 8b 46 10 mov 0x10(%rsi),%eax > 8: 55 push %rbp > 9: 48 89 e5 mov %rsp,%rbp > c: c1 e0 05 shl $0x5,%eax > f: 48 03 47 48 add 0x48(%rdi),%rax > 13: 8b 00 mov (%rax),%eax > 15: 0f 0b ud2 > 17: 66 0f 1f 84 00 00 00 nopw 0x0(%rax,%rax,1) > 1e: 00 00 > > static inline unsigned long clk_get_rate(struct clk *clk) > { > return 0; > } > > static void hibvt_pwm_get_state(struct pwm_chip *chip, struct pwm_device *pwm, > struct pwm_state *state) > { > struct hibvt_pwm_chip *hi_pwm_chip = to_hibvt_pwm_chip(chip); > void __iomem *base; > u32 freq, value; > > freq = div_u64(clk_get_rate(hi_pwm_chip->clk), 1000000); > base = hi_pwm_chip->base; > > value = readl(base + PWM_CFG0_ADDR(pwm->hwpwm)); > state->period = div_u64(value * 1000, freq); > > value = readl(base + PWM_CFG1_ADDR(pwm->hwpwm)); > state->duty_cycle = div_u64(value * 1000, freq); > > value = readl(base + PWM_CTRL_ADDR(pwm->hwpwm)); > state->enabled = (PWM_ENABLE_MASK & value); > } I assume '-Wdiv-by-zero' is enabled and gcc isn't showing the "division by zero" warning for either of these? The 'ud2' is guaranteed to trigger since the function has no branches. Surely at least the missing warning is a gcc bug. The good news is objtool is flushing these out, albeit with a confusing message. -- Josh