Re: [PATCH] perf intel-pt: Fix undefined shift in intel_pt_bip()

[Arnaldo Carvalho de Melo <acme@kernel.org>]

On Thu, Sep 03, 2026 at 08:31:10AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Low] The patch introduces a build failure by placing a variable declaration after a statement block, which violates the C90 compilation rules enforced in the perf tools build system.
> --

Please address this and consider the other pre-existing issues, not a
requirement to process your series tho.

- Arnaldo
 
> commit 8a68d39b58963034d86457f673e830e359365de8
> Author: Liu Jing <liujing@cmss.chinamobile.com>
> 
> perf intel-pt: Fix undefined shift in intel_pt_bip()
> 
> This commit aims to fix an undefined shift behavior in intel_pt_bip() by
> moving the bounds check before the shift and using an unsigned shift operation.
> 
> > diff --git a/tools/perf/util/intel-pt-decoder/intel-pt-decoder.c b/tools/perf/util/intel-pt-decoder/intel-pt-decoder.c
> > index e733f6b1f7ac5..9abadd0d1c108 100644
> > --- a/tools/perf/util/intel-pt-decoder/intel-pt-decoder.c
> > +++ b/tools/perf/util/intel-pt-decoder/intel-pt-decoder.c
> > @@ -2060,7 +2060,6 @@ static void intel_pt_bbp(struct intel_pt_decoder *decoder)
> >  static void intel_pt_bip(struct intel_pt_decoder *decoder)
> >  {
> >  	uint32_t id = decoder->packet.count;
> > -	uint32_t bit = 1 << id;
> >  	int pos = decoder->blk_type_pos;
> >  
> >  	if (pos < 0 || id >= INTEL_PT_BLK_ITEM_ID_CNT) {
> > @@ -2068,6 +2067,7 @@ static void intel_pt_bip(struct intel_pt_decoder *decoder)
> >  			     id, decoder->blk_type);
> >  		return;
> >  	}
> > +	uint32_t bit = 1U << id;
> 
> [Severity: Low]
> Will this cause a build failure? It looks like the declaration of the bit
> variable is now placed after an if statement block within intel_pt_bip().
> 
> The perf tools build system enables -Wdeclaration-after-statement and promotes
> it to an error. Could this be fixed by separating the declaration and
> initialization, like declaring uint32_t bit; at the top of the function and
> then assigning it after the bounds check?
> 
> >  
> >  	if (decoder->state.items.mask[pos] & bit) {
> >  		intel_pt_log("WARNING: Duplicate block item %u type %d\n",
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260903082128.4972-2-liujing@cmss.chinamobile.com?part=1