Messages in this thread | | | From | Arnd Bergmann <> | Date | Mon, 4 Dec 2017 17:58:50 +0100 | Subject | Re: [PATCH] drm: msm: avoid false-positive -Wmaybe-uninitialized warning |
| |
On Mon, Dec 4, 2017 at 5:36 PM, Laurent Pinchart <laurent.pinchart@ideasonboard.com> 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 <arnd@arndb.de> > > The patch looks good to me, so > > Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > 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
| |