Re: [linux-next:master 9762/11953] mm/page_vma_mapped.c:246 page_vma_mapped_walk() warn: always true condition '(pvmw->nr_pages >= (1 << ( - (12)))) => (0-u64max >= 0)'

[Matthew Wilcox <willy@infradead.org>]

On Mon, Mar 14, 2022 at 04:30:38PM +0300, Dan Carpenter wrote:
> On Sun, Mar 13, 2022 at 04:06:37AM +0000, Matthew Wilcox wrote:
> > On Sun, Mar 13, 2022 at 11:01:09AM +0800, kernel test robot wrote:
> > > tree:   https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git  master
> > > head:   71941773e143369a73c9c4a3b62fbb60736a1182
> > > commit: b786e44a4dbfe64476e7120ec7990b89a37be37d [9762/11953] mm: Convert page_vma_mapped_walk to work on PFNs
> > > config: riscv-randconfig-m031-20220312 (https://download.01.org/0day-ci/archive/20220313/202203131056.WINF40Gt-lkp@intel.com/config )
> > > compiler: riscv64-linux-gcc (GCC) 11.2.0
> > > 
> > > If you fix the issue, kindly add following tag as appropriate
> > > Reported-by: kernel test robot <lkp@intel.com>
> > > 
> > > smatch warnings:
> > > mm/page_vma_mapped.c:246 page_vma_mapped_walk() warn: always true condition '(pvmw->nr_pages >= (1 << ( - (12)))) => (0-u64max >= 0)'
> > 
> > Looks like a duplicate of https://lore.kernel.org/lkml/YgpzPru8aFA5sHOI@casper.infradead.org/ 
> > 
> > Dan, any thoughts?  Do you consider this a false positive from smatch?
> > 
> 
> That's really weird that HPAGE_PMD_NR ends up as zero...  Does it wrap
> to zero?

It's actually weirder than that:

#define HPAGE_PMD_SHIFT ({ BUILD_BUG(); 0; })
#define HPAGE_PMD_ORDER (HPAGE_PMD_SHIFT-PAGE_SHIFT)
#define HPAGE_PMD_NR (1<<HPAGE_PMD_ORDER)

so it should be (1<<(0-12))

> Anyway, it would be easy to silence the warning based on that it's on an
> impossible path but I think that's not the right idea.
> 
> 	if (is_impossible_path())
> 		return;
> 
> A lot of the bugs that Smatch finds are in impossible to reach error
> handling code.  I'll instead silence it based on that the macro
> definition changes.  Add it to smatch_data/kernel.unconstant_macros:
> 
>  
> -       if (!possibly_false_rl(rl_left, expr->op, rl_right)) {
> +       if (!possibly_false_rl(rl_left, expr->op, rl_right) &&
> +           !is_unconstant_macro(expr->left) &&
> +           !is_unconstant_macro(expr->right)) {
>                 char *name = expr_to_str(expr);
>  
>                 sm_warning("always true condition '(%s) => (%s %s %s)'", name,
> 
> I'll test this out overnight, tonight.

Thanks!  I'd be happy to rearrange this code to avoid the smatch report,
but it's not quite clear to me how I could/should do that.