On Mon, Dec 4, 2017 at 5:36 PM, Laurent Pinchart
<laurent.pinchart@xxxxxxxxxxxxxxxx> 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@xxxxxxxx>
>
> The patch looks good to me, so
>
> Acked-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
>
> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
