Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753176AbdLDQ65 (ORCPT ); Mon, 4 Dec 2017 11:58:57 -0500 Received: from mail-oi0-f66.google.com ([209.85.218.66]:42127 "EHLO mail-oi0-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752486AbdLDQ6v (ORCPT ); Mon, 4 Dec 2017 11:58:51 -0500 X-Google-Smtp-Source: AGs4zMbbZDwcJ+gzLEXzju3WrVKJgyVL6iOMMirTV1z63/8yb92mMwuW/Z1RILvBJrU+Syx4Qd5iD1/WTBdnvUZvPIQ= MIME-Version: 1.0 In-Reply-To: <1595491.VEK6ezTaRq@avalon> References: <20171204144436.272626-1-arnd@arndb.de> <1595491.VEK6ezTaRq@avalon> From: Arnd Bergmann Date: Mon, 4 Dec 2017 17:58:50 +0100 X-Google-Sender-Auth: h7wnLtItVkCwW6rb7V846B_W3oA Message-ID: Subject: Re: [PATCH] drm: msm: avoid false-positive -Wmaybe-uninitialized warning To: Laurent Pinchart Cc: Rob Clark , David Airlie , Archit Taneja , Daniel Vetter , Liviu Dudau , Gustavo Padovan , linux-arm-msm@vger.kernel.org, dri-devel , freedreno@lists.freedesktop.org, Linux Kernel Mailing List Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2392 Lines: 69 On Mon, Dec 4, 2017 at 5:36 PM, Laurent Pinchart wrote: > Hi Arnd, > > Thank you for the patch. > > On Monday, 4 December 2017 16:44:23 EET Arnd Bergmann wrote: >> gcc-8 -fsanitize-coverage=trace-pc produces a false-positive warning: >> >> drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c: In function >> 'mdp5_plane_mode_set.isra.8': >> drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c:1053:3: error: 'crtc_x_r' may be >> used uninitialized in this function [-Werror=maybe-uninitialized] >> >> It's relatively clear from reading the source that this cannot happen, >> and older compilers get it right. This rearranges the code remove >> the two affected variables, which reliably avoids the problem. >> >> Signed-off-by: Arnd Bergmann > > The patch looks good to me, so > > Acked-by: Laurent Pinchart > > However I think it would also be useful to file a bug report for gcc, > especially if older versions got this right. I was rather close to it, and even spent time on a reduced test case with "creduce", which came down to int drm_rect_width_r_0, calc_phase_step_src, calc_scalex_steps_ret, calc_scalex_steps_dest, calc_scaley_steps_ret, calc_scaley_steps_dest, mdp5_plane_mode_set___trans_tmp_2; struct mdp5_hw_pipe { int caps; } * mdp5_plane_mode_set_right_hwpipe; int fn1(int p1) { if (calc_phase_step_src || p1 == 0) return 2; if (calc_phase_step_src > p1) return 5; return 0; } int fn2() { struct mdp5_hw_pipe hwpipe = hwpipe; int src_x_r; if (mdp5_plane_mode_set_right_hwpipe) src_x_r = drm_rect_width_r_0; calc_scalex_steps_ret = fn1(calc_scalex_steps_dest); if (calc_scalex_steps_ret) return calc_scalex_steps_ret; calc_scaley_steps_ret = fn1(calc_scaley_steps_dest); if (calc_scaley_steps_ret) return calc_scaley_steps_ret; if (hwpipe.caps) if (mdp5_plane_mode_set_right_hwpipe) mdp5_plane_mode_set___trans_tmp_2 = src_x_r; return calc_scaley_steps_ret; } This is still not something that is "obviously" wrong, it seems rather that gcc can't keep track of enough state at the same time, which is a fundamental problem but also a bit unpredictable. I've seen many false-positive (and also false-negative) -Wmaybe-uninitialized warnings that are likely easier to fix than this particular one, so I ended up not reporting it. Arnd