Linux-Next Archive on lore.kernel.org
 help / color / Atom feed
* linux-next: build warning after merge of the akpm-current tree
@ 2017-11-13  5:42 Stephen Rothwell
  2017-11-13  8:09 ` Michal Hocko
  0 siblings, 1 reply; 14+ messages in thread
From: Stephen Rothwell @ 2017-11-13  5:42 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Linux-Next Mailing List, Linux Kernel Mailing List, Michal Hocko

Hi Andrew,

After merging the akpm-current tree, today's linux-next build (powerpc
ppc64_defconfig) produced this warning:

In file included from include/linux/mmzone.h:17:0,
                 from include/linux/mempolicy.h:10,
                 from mm/mempolicy.c:70:
mm/mempolicy.c: In function 'mpol_to_str':
include/linux/nodemask.h:107:41: warning: the address of 'nodes' will always evaluate as 'true' [-Waddress]
 #define nodemask_pr_args(maskp) (maskp) ? MAX_NUMNODES : 0, (maskp) ? (maskp)->bits : NULL
                                         ^
mm/mempolicy.c:2817:11: note: in expansion of macro 'nodemask_pr_args'
           nodemask_pr_args(&nodes));
           ^
include/linux/nodemask.h:107:69: warning: the address of 'nodes' will always evaluate as 'true' [-Waddress]
 #define nodemask_pr_args(maskp) (maskp) ? MAX_NUMNODES : 0, (maskp) ? (maskp)->bits : NULL
                                                                     ^
mm/mempolicy.c:2817:11: note: in expansion of macro 'nodemask_pr_args'
           nodemask_pr_args(&nodes));
           ^

Introduced by commit

  b2c1ed23bdc1 ("mm: simplify nodemask printing")

-- 
Cheers,
Stephen Rothwell

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: linux-next: build warning after merge of the akpm-current tree
  2017-11-13  5:42 linux-next: build warning after merge of the akpm-current tree Stephen Rothwell
@ 2017-11-13  8:09 ` Michal Hocko
  2017-11-13  8:23   ` Michal Hocko
  2017-11-13 11:43   ` Arnd Bergmann
  0 siblings, 2 replies; 14+ messages in thread
From: Michal Hocko @ 2017-11-13  8:09 UTC (permalink / raw)
  To: Stephen Rothwell
  Cc: Andrew Morton, Linux-Next Mailing List, Linux Kernel Mailing List

On Mon 13-11-17 16:42:06, Stephen Rothwell wrote:
> Hi Andrew,
> 
> After merging the akpm-current tree, today's linux-next build (powerpc
> ppc64_defconfig) produced this warning:
> 
> In file included from include/linux/mmzone.h:17:0,
>                  from include/linux/mempolicy.h:10,
>                  from mm/mempolicy.c:70:
> mm/mempolicy.c: In function 'mpol_to_str':
> include/linux/nodemask.h:107:41: warning: the address of 'nodes' will always evaluate as 'true' [-Waddress]
>  #define nodemask_pr_args(maskp) (maskp) ? MAX_NUMNODES : 0, (maskp) ? (maskp)->bits : NULL
>                                          ^
> mm/mempolicy.c:2817:11: note: in expansion of macro 'nodemask_pr_args'
>            nodemask_pr_args(&nodes));
>            ^

Hmm, this warning is quite surprising to me. Sure in this particular
case maskp will always be non-NULL so we always expand to
	MAX_NUMNODES, maskp->bits
which is what we want. But we have other users which may be NULL. Does
anybody understan why this warns at all?
-- 
Michal Hocko
SUSE Labs

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: linux-next: build warning after merge of the akpm-current tree
  2017-11-13  8:09 ` Michal Hocko
@ 2017-11-13  8:23   ` Michal Hocko
  2017-11-13 11:43   ` Arnd Bergmann
  1 sibling, 0 replies; 14+ messages in thread
From: Michal Hocko @ 2017-11-13  8:23 UTC (permalink / raw)
  To: Stephen Rothwell
  Cc: Andrew Morton, Linux-Next Mailing List, Linux Kernel Mailing List

On Mon 13-11-17 09:09:55, Michal Hocko wrote:
> On Mon 13-11-17 16:42:06, Stephen Rothwell wrote:
> > Hi Andrew,
> > 
> > After merging the akpm-current tree, today's linux-next build (powerpc
> > ppc64_defconfig) produced this warning:
> > 
> > In file included from include/linux/mmzone.h:17:0,
> >                  from include/linux/mempolicy.h:10,
> >                  from mm/mempolicy.c:70:
> > mm/mempolicy.c: In function 'mpol_to_str':
> > include/linux/nodemask.h:107:41: warning: the address of 'nodes' will always evaluate as 'true' [-Waddress]
> >  #define nodemask_pr_args(maskp) (maskp) ? MAX_NUMNODES : 0, (maskp) ? (maskp)->bits : NULL
> >                                          ^
> > mm/mempolicy.c:2817:11: note: in expansion of macro 'nodemask_pr_args'
> >            nodemask_pr_args(&nodes));
> >            ^
> 
> Hmm, this warning is quite surprising to me. Sure in this particular
> case maskp will always be non-NULL so we always expand to
> 	MAX_NUMNODES, maskp->bits
> which is what we want. But we have other users which may be NULL. Does
> anybody understan why this warns at all?

Strange I played with the following minimal test case and it warns only
for the explicit &m use while n is clearly never null as well. This all
smells like -Waddress is just confused (at least with my gcc 7.2.0-12

#include <stdio.h>

#define MAX_NUMNODES 10
struct mask {
	void *bits;
};
#define nodemask_pr_args(maskp) (maskp) ? MAX_NUMNODES : 0, (maskp) ? (maskp)->bits : NULL

int foo(void)
{
	struct mask m;
	struct mask *n = &m;

	printf("%*p\n", nodemask_pr_args(&m));
	printf("%*p\n", nodemask_pr_args(n));

	return 0;
}
-- 
Michal Hocko
SUSE Labs

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: linux-next: build warning after merge of the akpm-current tree
  2017-11-13  8:09 ` Michal Hocko
  2017-11-13  8:23   ` Michal Hocko
@ 2017-11-13 11:43   ` Arnd Bergmann
  2017-11-13 11:54     ` Michal Hocko
  2017-11-16 22:44     ` Stephen Rothwell
  1 sibling, 2 replies; 14+ messages in thread
From: Arnd Bergmann @ 2017-11-13 11:43 UTC (permalink / raw)
  To: Michal Hocko
  Cc: Stephen Rothwell, Andrew Morton, Linux-Next Mailing List,
	Linux Kernel Mailing List

On Mon, Nov 13, 2017 at 9:09 AM, Michal Hocko <mhocko@kernel.org> wrote:
> On Mon 13-11-17 16:42:06, Stephen Rothwell wrote:
>> Hi Andrew,
>>
>> After merging the akpm-current tree, today's linux-next build (powerpc
>> ppc64_defconfig) produced this warning:
>>
>> In file included from include/linux/mmzone.h:17:0,
>>                  from include/linux/mempolicy.h:10,
>>                  from mm/mempolicy.c:70:
>> mm/mempolicy.c: In function 'mpol_to_str':
>> include/linux/nodemask.h:107:41: warning: the address of 'nodes' will always evaluate as 'true' [-Waddress]
>>  #define nodemask_pr_args(maskp) (maskp) ? MAX_NUMNODES : 0, (maskp) ? (maskp)->bits : NULL
>>                                          ^
>> mm/mempolicy.c:2817:11: note: in expansion of macro 'nodemask_pr_args'
>>            nodemask_pr_args(&nodes));
>>            ^
>
> Hmm, this warning is quite surprising to me. Sure in this particular
> case maskp will always be non-NULL so we always expand to
>         MAX_NUMNODES, maskp->bits
> which is what we want. But we have other users which may be NULL. Does
> anybody understan why this warns at all?

As I understand it, the warning tries to address a common typo of accidentally
testing the pointer to a stack object for being non-NULL, rather than the object
pointed to for being non-zero.

Adding an extra '!= NULL' comparison gets rid of the warning for me:

#define nodemask_pr_args(maskp)  \
   ((maskp) != NULL) ? MAX_NUMNODES : 0, \
   ((maskp) != NULL) ?(maskp)->bits : NULL

       Arnd

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: linux-next: build warning after merge of the akpm-current tree
  2017-11-13 11:43   ` Arnd Bergmann
@ 2017-11-13 11:54     ` Michal Hocko
  2017-11-13 12:24       ` Arnd Bergmann
  2017-11-13 12:29       ` Michal Hocko
  2017-11-16 22:44     ` Stephen Rothwell
  1 sibling, 2 replies; 14+ messages in thread
From: Michal Hocko @ 2017-11-13 11:54 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Stephen Rothwell, Andrew Morton, Linux-Next Mailing List,
	Linux Kernel Mailing List

On Mon 13-11-17 12:43:08, Arnd Bergmann wrote:
> On Mon, Nov 13, 2017 at 9:09 AM, Michal Hocko <mhocko@kernel.org> wrote:
> > On Mon 13-11-17 16:42:06, Stephen Rothwell wrote:
> >> Hi Andrew,
> >>
> >> After merging the akpm-current tree, today's linux-next build (powerpc
> >> ppc64_defconfig) produced this warning:
> >>
> >> In file included from include/linux/mmzone.h:17:0,
> >>                  from include/linux/mempolicy.h:10,
> >>                  from mm/mempolicy.c:70:
> >> mm/mempolicy.c: In function 'mpol_to_str':
> >> include/linux/nodemask.h:107:41: warning: the address of 'nodes' will always evaluate as 'true' [-Waddress]
> >>  #define nodemask_pr_args(maskp) (maskp) ? MAX_NUMNODES : 0, (maskp) ? (maskp)->bits : NULL
> >>                                          ^
> >> mm/mempolicy.c:2817:11: note: in expansion of macro 'nodemask_pr_args'
> >>            nodemask_pr_args(&nodes));
> >>            ^
> >
> > Hmm, this warning is quite surprising to me. Sure in this particular
> > case maskp will always be non-NULL so we always expand to
> >         MAX_NUMNODES, maskp->bits
> > which is what we want. But we have other users which may be NULL. Does
> > anybody understan why this warns at all?
> 
> As I understand it, the warning tries to address a common typo of accidentally
> testing the pointer to a stack object for being non-NULL, rather than the object
> pointed to for being non-zero.
> 
> Adding an extra '!= NULL' comparison gets rid of the warning for me:
> 
> #define nodemask_pr_args(maskp)  \
>    ((maskp) != NULL) ? MAX_NUMNODES : 0, \
>    ((maskp) != NULL) ?(maskp)->bits : NULL

OK, that is a reasonable workaround. I was talking to our gcc guy and
he suggested to report a bug for this. Andrew, could you fold the
explicit != NULL check into the patch please?

-- 
Michal Hocko
SUSE Labs

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: linux-next: build warning after merge of the akpm-current tree
  2017-11-13 11:54     ` Michal Hocko
@ 2017-11-13 12:24       ` Arnd Bergmann
  2017-11-13 12:29       ` Michal Hocko
  1 sibling, 0 replies; 14+ messages in thread
From: Arnd Bergmann @ 2017-11-13 12:24 UTC (permalink / raw)
  To: Michal Hocko
  Cc: Stephen Rothwell, Andrew Morton, Linux-Next Mailing List,
	Linux Kernel Mailing List

On Mon, Nov 13, 2017 at 12:54 PM, Michal Hocko <mhocko@kernel.org> wrote:
> On Mon 13-11-17 12:43:08, Arnd Bergmann wrote:
>> On Mon, Nov 13, 2017 at 9:09 AM, Michal Hocko <mhocko@kernel.org> wrote:
>> > On Mon 13-11-17 16:42:06, Stephen Rothwell wrote:
>> >> Hi Andrew,
>> >>
>> >> After merging the akpm-current tree, today's linux-next build (powerpc
>> >> ppc64_defconfig) produced this warning:
>> >>
>> >> In file included from include/linux/mmzone.h:17:0,
>> >>                  from include/linux/mempolicy.h:10,
>> >>                  from mm/mempolicy.c:70:
>> >> mm/mempolicy.c: In function 'mpol_to_str':
>> >> include/linux/nodemask.h:107:41: warning: the address of 'nodes' will always evaluate as 'true' [-Waddress]
>> >>  #define nodemask_pr_args(maskp) (maskp) ? MAX_NUMNODES : 0, (maskp) ? (maskp)->bits : NULL
>> >>                                          ^
>> >> mm/mempolicy.c:2817:11: note: in expansion of macro 'nodemask_pr_args'
>> >>            nodemask_pr_args(&nodes));
>> >>            ^
>> >
>> > Hmm, this warning is quite surprising to me. Sure in this particular
>> > case maskp will always be non-NULL so we always expand to
>> >         MAX_NUMNODES, maskp->bits
>> > which is what we want. But we have other users which may be NULL. Does
>> > anybody understan why this warns at all?
>>
>> As I understand it, the warning tries to address a common typo of accidentally
>> testing the pointer to a stack object for being non-NULL, rather than the object
>> pointed to for being non-zero.
>>
>> Adding an extra '!= NULL' comparison gets rid of the warning for me:
>>
>> #define nodemask_pr_args(maskp)  \
>>    ((maskp) != NULL) ? MAX_NUMNODES : 0, \
>>    ((maskp) != NULL) ?(maskp)->bits : NULL
>
> OK, that is a reasonable workaround. I was talking to our gcc guy and
> he suggested to report a bug for this.

That might also be useful. Some warnings in gcc get disabled when
they show up inside of a macro, and that could presumably be done
here too.

      Arnd

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: linux-next: build warning after merge of the akpm-current tree
  2017-11-13 11:54     ` Michal Hocko
  2017-11-13 12:24       ` Arnd Bergmann
@ 2017-11-13 12:29       ` Michal Hocko
  1 sibling, 0 replies; 14+ messages in thread
From: Michal Hocko @ 2017-11-13 12:29 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Stephen Rothwell, Andrew Morton, Linux-Next Mailing List,
	Linux Kernel Mailing List

On Mon 13-11-17 12:54:30, Michal Hocko wrote:
> On Mon 13-11-17 12:43:08, Arnd Bergmann wrote:
> > On Mon, Nov 13, 2017 at 9:09 AM, Michal Hocko <mhocko@kernel.org> wrote:
> > > On Mon 13-11-17 16:42:06, Stephen Rothwell wrote:
> > >> Hi Andrew,
> > >>
> > >> After merging the akpm-current tree, today's linux-next build (powerpc
> > >> ppc64_defconfig) produced this warning:
> > >>
> > >> In file included from include/linux/mmzone.h:17:0,
> > >>                  from include/linux/mempolicy.h:10,
> > >>                  from mm/mempolicy.c:70:
> > >> mm/mempolicy.c: In function 'mpol_to_str':
> > >> include/linux/nodemask.h:107:41: warning: the address of 'nodes' will always evaluate as 'true' [-Waddress]
> > >>  #define nodemask_pr_args(maskp) (maskp) ? MAX_NUMNODES : 0, (maskp) ? (maskp)->bits : NULL
> > >>                                          ^
> > >> mm/mempolicy.c:2817:11: note: in expansion of macro 'nodemask_pr_args'
> > >>            nodemask_pr_args(&nodes));
> > >>            ^
> > >
> > > Hmm, this warning is quite surprising to me. Sure in this particular
> > > case maskp will always be non-NULL so we always expand to
> > >         MAX_NUMNODES, maskp->bits
> > > which is what we want. But we have other users which may be NULL. Does
> > > anybody understan why this warns at all?
> > 
> > As I understand it, the warning tries to address a common typo of accidentally
> > testing the pointer to a stack object for being non-NULL, rather than the object
> > pointed to for being non-zero.
> > 
> > Adding an extra '!= NULL' comparison gets rid of the warning for me:
> > 
> > #define nodemask_pr_args(maskp)  \
> >    ((maskp) != NULL) ? MAX_NUMNODES : 0, \
> >    ((maskp) != NULL) ?(maskp)->bits : NULL
> 
> OK, that is a reasonable workaround. I was talking to our gcc guy and
> he suggested to report a bug for this.

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82963

-- 
Michal Hocko
SUSE Labs

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: linux-next: build warning after merge of the akpm-current tree
  2017-11-13 11:43   ` Arnd Bergmann
  2017-11-13 11:54     ` Michal Hocko
@ 2017-11-16 22:44     ` Stephen Rothwell
  2017-11-17  3:53       ` Stephen Rothwell
  1 sibling, 1 reply; 14+ messages in thread
From: Stephen Rothwell @ 2017-11-16 22:44 UTC (permalink / raw)
  To: Michal Hocko, Andrew Morton
  Cc: Arnd Bergmann, Linux-Next Mailing List, Linux Kernel Mailing List

Hi all,

On Mon, 13 Nov 2017 12:43:08 +0100 Arnd Bergmann <arnd@arndb.de> wrote:
>
> On Mon, Nov 13, 2017 at 9:09 AM, Michal Hocko <mhocko@kernel.org> wrote:
> > On Mon 13-11-17 16:42:06, Stephen Rothwell wrote:  
> >>
> >> After merging the akpm-current tree, today's linux-next build (powerpc
> >> ppc64_defconfig) produced this warning:
> >>
> >> In file included from include/linux/mmzone.h:17:0,
> >>                  from include/linux/mempolicy.h:10,
> >>                  from mm/mempolicy.c:70:
> >> mm/mempolicy.c: In function 'mpol_to_str':
> >> include/linux/nodemask.h:107:41: warning: the address of 'nodes' will always evaluate as 'true' [-Waddress]
> >>  #define nodemask_pr_args(maskp) (maskp) ? MAX_NUMNODES : 0, (maskp) ? (maskp)->bits : NULL
> >>                                          ^
> >> mm/mempolicy.c:2817:11: note: in expansion of macro 'nodemask_pr_args'
> >>            nodemask_pr_args(&nodes));
> >>            ^  
> >
> > Hmm, this warning is quite surprising to me. Sure in this particular
> > case maskp will always be non-NULL so we always expand to
> >         MAX_NUMNODES, maskp->bits
> > which is what we want. But we have other users which may be NULL. Does
> > anybody understan why this warns at all?  
> 
> As I understand it, the warning tries to address a common typo of accidentally
> testing the pointer to a stack object for being non-NULL, rather than the object
> pointed to for being non-zero.
> 
> Adding an extra '!= NULL' comparison gets rid of the warning for me:
> 
> #define nodemask_pr_args(maskp)  \
>    ((maskp) != NULL) ? MAX_NUMNODES : 0, \
>    ((maskp) != NULL) ?(maskp)->bits : NULL
> 
>        Arnd

This warning now exists in Linus' tree :-(

-- 
Cheers,
Stephen Rothwell

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: linux-next: build warning after merge of the akpm-current tree
  2017-11-16 22:44     ` Stephen Rothwell
@ 2017-11-17  3:53       ` Stephen Rothwell
  2017-11-17  9:36         ` Zhangshaokun
  2017-11-17  9:56         ` Arnd Bergmann
  0 siblings, 2 replies; 14+ messages in thread
From: Stephen Rothwell @ 2017-11-17  3:53 UTC (permalink / raw)
  To: Michal Hocko, Andrew Morton
  Cc: Arnd Bergmann, Linux-Next Mailing List, Linux Kernel Mailing List

Hi all,

On Fri, 17 Nov 2017 09:44:39 +1100 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>
> On Mon, 13 Nov 2017 12:43:08 +0100 Arnd Bergmann <arnd@arndb.de> wrote:
> >
> > On Mon, Nov 13, 2017 at 9:09 AM, Michal Hocko <mhocko@kernel.org> wrote:  
> > > On Mon 13-11-17 16:42:06, Stephen Rothwell wrote:    
> > >>
> > >> After merging the akpm-current tree, today's linux-next build (powerpc
> > >> ppc64_defconfig) produced this warning:
> > >>
> > >> In file included from include/linux/mmzone.h:17:0,
> > >>                  from include/linux/mempolicy.h:10,
> > >>                  from mm/mempolicy.c:70:
> > >> mm/mempolicy.c: In function 'mpol_to_str':
> > >> include/linux/nodemask.h:107:41: warning: the address of 'nodes' will always evaluate as 'true' [-Waddress]
> > >>  #define nodemask_pr_args(maskp) (maskp) ? MAX_NUMNODES : 0, (maskp) ? (maskp)->bits : NULL
> > >>                                          ^
> > >> mm/mempolicy.c:2817:11: note: in expansion of macro 'nodemask_pr_args'
> > >>            nodemask_pr_args(&nodes));
> > >>            ^    
> > >
> > > Hmm, this warning is quite surprising to me. Sure in this particular
> > > case maskp will always be non-NULL so we always expand to
> > >         MAX_NUMNODES, maskp->bits
> > > which is what we want. But we have other users which may be NULL. Does
> > > anybody understan why this warns at all?    
> > 
> > As I understand it, the warning tries to address a common typo of accidentally
> > testing the pointer to a stack object for being non-NULL, rather than the object
> > pointed to for being non-zero.
> > 
> > Adding an extra '!= NULL' comparison gets rid of the warning for me:
> > 
> > #define nodemask_pr_args(maskp)  \
> >    ((maskp) != NULL) ? MAX_NUMNODES : 0, \
> >    ((maskp) != NULL) ?(maskp)->bits : NULL
> > 
> >        Arnd  
> 
> This warning now exists in Linus' tree :-(

Looking closer, it seems that the above workaround doesn't work for my
compiler (gcc v5.2.0):

In file included from include/linux/mmzone.h:17:0,
                 from include/linux/mempolicy.h:10,
                 from mm/mempolicy.c:70:
mm/mempolicy.c: In function 'mpol_to_str':
include/linux/nodemask.h:108:11: warning: the comparison will always evaluate as 'true' for the address of 'nodes' will never be NULL [-Waddress]
  ((maskp) != NULL) ? MAX_NUMNODES : 0,  \
           ^
mm/mempolicy.c:2817:11: note: in expansion of macro 'nodemask_pr_args'
           nodemask_pr_args(&nodes));
           ^
include/linux/nodemask.h:109:11: warning: the comparison will always evaluate as 'true' for the address of 'nodes' will never be NULL [-Waddress]
  ((maskp) != NULL) ? (maskp)->bits : NULL
           ^
mm/mempolicy.c:2817:11: note: in expansion of macro 'nodemask_pr_args'
           nodemask_pr_args(&nodes));
           ^

-- 
Cheers,
Stephen Rothwell

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: linux-next: build warning after merge of the akpm-current tree
  2017-11-17  3:53       ` Stephen Rothwell
@ 2017-11-17  9:36         ` Zhangshaokun
  2017-11-17  9:56         ` Arnd Bergmann
  1 sibling, 0 replies; 14+ messages in thread
From: Zhangshaokun @ 2017-11-17  9:36 UTC (permalink / raw)
  To: Stephen Rothwell, Michal Hocko, Andrew Morton
  Cc: Arnd Bergmann, Linux-Next Mailing List, Linux Kernel Mailing List

Hi,

On 2017/11/17 11:53, Stephen Rothwell wrote:
> Hi all,
> 
> On Fri, 17 Nov 2017 09:44:39 +1100 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>>
>> On Mon, 13 Nov 2017 12:43:08 +0100 Arnd Bergmann <arnd@arndb.de> wrote:
>>>
>>> On Mon, Nov 13, 2017 at 9:09 AM, Michal Hocko <mhocko@kernel.org> wrote:  
>>>> On Mon 13-11-17 16:42:06, Stephen Rothwell wrote:    
>>>>>
>>>>> After merging the akpm-current tree, today's linux-next build (powerpc
>>>>> ppc64_defconfig) produced this warning:
>>>>>
>>>>> In file included from include/linux/mmzone.h:17:0,
>>>>>                  from include/linux/mempolicy.h:10,
>>>>>                  from mm/mempolicy.c:70:
>>>>> mm/mempolicy.c: In function 'mpol_to_str':
>>>>> include/linux/nodemask.h:107:41: warning: the address of 'nodes' will always evaluate as 'true' [-Waddress]
>>>>>  #define nodemask_pr_args(maskp) (maskp) ? MAX_NUMNODES : 0, (maskp) ? (maskp)->bits : NULL
>>>>>                                          ^
>>>>> mm/mempolicy.c:2817:11: note: in expansion of macro 'nodemask_pr_args'
>>>>>            nodemask_pr_args(&nodes));
>>>>>            ^    
>>>>
>>>> Hmm, this warning is quite surprising to me. Sure in this particular
>>>> case maskp will always be non-NULL so we always expand to
>>>>         MAX_NUMNODES, maskp->bits
>>>> which is what we want. But we have other users which may be NULL. Does
>>>> anybody understan why this warns at all?    
>>>
>>> As I understand it, the warning tries to address a common typo of accidentally
>>> testing the pointer to a stack object for being non-NULL, rather than the object
>>> pointed to for being non-zero.
>>>
>>> Adding an extra '!= NULL' comparison gets rid of the warning for me:
>>>
>>> #define nodemask_pr_args(maskp)  \
>>>    ((maskp) != NULL) ? MAX_NUMNODES : 0, \
>>>    ((maskp) != NULL) ?(maskp)->bits : NULL
>>>
>>>        Arnd  
>>
>> This warning now exists in Linus' tree :-(
> 
> Looking closer, it seems that the above workaround doesn't work for my
> compiler (gcc v5.2.0):
> 

I also came across this issue using Linus' tree and my gcc is gcc version 5.4.0 20160609
 (Ubuntu/Linaro 5.4.0-6ubuntu1~16.04.5).

  CC      drivers/clk/renesas/rcar-gen3-cpg.o
In file included from ./include/linux/mmzone.h:17:0,
                 from ./include/linux/mempolicy.h:10,
                 from mm/mempolicy.c:70:
mm/mempolicy.c: In function ‘mpol_to_str’:
./include/linux/nodemask.h:108:11: warning: the comparison will always evaluate as ‘true’ for the address of ‘nodes’ will never be NULL [-Waddress]
  ((maskp) != NULL) ? MAX_NUMNODES : 0,  \
           ^
mm/mempolicy.c:2817:11: note: in expansion of macro ‘nodemask_pr_args’
           nodemask_pr_args(&nodes));
           ^
./include/linux/nodemask.h:109:11: warning: the comparison will always evaluate as ‘true’ for the address of ‘nodes’ will never be NULL [-Waddress]
  ((maskp) != NULL) ? (maskp)->bits : NULL
           ^
mm/mempolicy.c:2817:11: note: in expansion of macro ‘nodemask_pr_args’
           nodemask_pr_args(&nodes));
           ^
  CC      drivers/clk/renesas/renesas-cpg-mssr.o

Thanks,
Shaokun

> In file included from include/linux/mmzone.h:17:0,
>                  from include/linux/mempolicy.h:10,
>                  from mm/mempolicy.c:70:
> mm/mempolicy.c: In function 'mpol_to_str':
> include/linux/nodemask.h:108:11: warning: the comparison will always evaluate as 'true' for the address of 'nodes' will never be NULL [-Waddress]
>   ((maskp) != NULL) ? MAX_NUMNODES : 0,  \
>            ^
> mm/mempolicy.c:2817:11: note: in expansion of macro 'nodemask_pr_args'
>            nodemask_pr_args(&nodes));
>            ^
> include/linux/nodemask.h:109:11: warning: the comparison will always evaluate as 'true' for the address of 'nodes' will never be NULL [-Waddress]
>   ((maskp) != NULL) ? (maskp)->bits : NULL
>            ^
> mm/mempolicy.c:2817:11: note: in expansion of macro 'nodemask_pr_args'
>            nodemask_pr_args(&nodes));
>            ^
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: linux-next: build warning after merge of the akpm-current tree
  2017-11-17  3:53       ` Stephen Rothwell
  2017-11-17  9:36         ` Zhangshaokun
@ 2017-11-17  9:56         ` Arnd Bergmann
  2017-11-17 10:15           ` [PATCH] mm: fix nodemask printing Arnd Bergmann
  1 sibling, 1 reply; 14+ messages in thread
From: Arnd Bergmann @ 2017-11-17  9:56 UTC (permalink / raw)
  To: Stephen Rothwell
  Cc: Michal Hocko, Andrew Morton, Linux-Next Mailing List,
	Linux Kernel Mailing List

On Fri, Nov 17, 2017 at 4:53 AM, Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> Hi all,
>
> On Fri, 17 Nov 2017 09:44:39 +1100 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>>
>> On Mon, 13 Nov 2017 12:43:08 +0100 Arnd Bergmann <arnd@arndb.de> wrote:
>> >
>> > On Mon, Nov 13, 2017 at 9:09 AM, Michal Hocko <mhocko@kernel.org> wrote:
>> > > On Mon 13-11-17 16:42:06, Stephen Rothwell wrote:
>> > >>
>> > >> After merging the akpm-current tree, today's linux-next build (powerpc
>> > >> ppc64_defconfig) produced this warning:
>> > >>
>> > >> In file included from include/linux/mmzone.h:17:0,
>> > >>                  from include/linux/mempolicy.h:10,
>> > >>                  from mm/mempolicy.c:70:
>> > >> mm/mempolicy.c: In function 'mpol_to_str':
>> > >> include/linux/nodemask.h:107:41: warning: the address of 'nodes' will always evaluate as 'true' [-Waddress]
>> > >>  #define nodemask_pr_args(maskp) (maskp) ? MAX_NUMNODES : 0, (maskp) ? (maskp)->bits : NULL
>> > >>                                          ^
>> > >> mm/mempolicy.c:2817:11: note: in expansion of macro 'nodemask_pr_args'
>> > >>            nodemask_pr_args(&nodes));
>> > >>            ^
>> > >
>> > > Hmm, this warning is quite surprising to me. Sure in this particular
>> > > case maskp will always be non-NULL so we always expand to
>> > >         MAX_NUMNODES, maskp->bits
>> > > which is what we want. But we have other users which may be NULL. Does
>> > > anybody understan why this warns at all?
>> >
>> > As I understand it, the warning tries to address a common typo of accidentally
>> > testing the pointer to a stack object for being non-NULL, rather than the object
>> > pointed to for being non-zero.
>> >
>> > Adding an extra '!= NULL' comparison gets rid of the warning for me:
>> >
>> > #define nodemask_pr_args(maskp)  \
>> >    ((maskp) != NULL) ? MAX_NUMNODES : 0, \
>> >    ((maskp) != NULL) ?(maskp)->bits : NULL
>> >
>> >        Arnd
>>
>> This warning now exists in Linus' tree :-(
>
> Looking closer, it seems that the above workaround doesn't work for my
> compiler (gcc v5.2.0):

Right, I see now that all versions from gcc-4.6 to gcc-6 are affected
by this, while 4.5 and
earlier as well as 7 and 8 are not.

I'll try to come up with an alternative workaround, it will probably
be even uglier.

       Arnd

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH] mm: fix nodemask printing
  2017-11-17  9:56         ` Arnd Bergmann
@ 2017-11-17 10:15           ` Arnd Bergmann
  2017-11-20  8:22             ` Michal Hocko
  0 siblings, 1 reply; 14+ messages in thread
From: Arnd Bergmann @ 2017-11-17 10:15 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Michal Hocko, Stephen Rothwell, linux-next, linux-kernel,
	Zhangshaokun, Arnd Bergmann

The cleanup caused build warnings for constant mask pointers:

mm/mempolicy.c: In function ‘mpol_to_str’:
./include/linux/nodemask.h:108:11: warning: the comparison will always evaluate as ‘true’ for the address of ‘nodes’ will never be NULL [-Waddress]

An earlier workaround I suggested was incorporated in the version that
got merged, but that only solved the problem for gcc-7 and higher, while
gcc-4.6 through gcc-6.x still warn.

This changes the printing again to use inline functions that make it
clear to the compiler that the line that does the NULL check has no
idea whether the argument is a constant NULL.

Fixes: 0205f75571e3 ("mm: simplify nodemask printing")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
I've done only minimal build testing on this, but did check it with
all compiler versions this time.
---
 include/linux/nodemask.h | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/include/linux/nodemask.h b/include/linux/nodemask.h
index 15cab3967d6d..1fbde8a880d9 100644
--- a/include/linux/nodemask.h
+++ b/include/linux/nodemask.h
@@ -104,9 +104,16 @@ extern nodemask_t _unused_nodemask_arg_;
  *
  * Can be used to provide arguments for '%*pb[l]' when printing a nodemask.
  */
-#define nodemask_pr_args(maskp)				\
-	((maskp) != NULL) ? MAX_NUMNODES : 0,		\
-	((maskp) != NULL) ? (maskp)->bits : NULL
+#define nodemask_pr_args(maskp)	__nodemask_pr_numnodes(maskp), \
+				__nodemask_pr_bits(maskp)
+static inline unsigned int __nodemask_pr_numnodes(const nodemask_t *m)
+{
+	return m ? MAX_NUMNODES : 0;
+}
+static inline const unsigned long *__nodemask_pr_bits(const nodemask_t *m)
+{
+	return m ? m->bits : NULL;
+}
 
 /*
  * The inline keyword gives the compiler room to decide to inline, or
-- 
2.9.0

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] mm: fix nodemask printing
  2017-11-17 10:15           ` [PATCH] mm: fix nodemask printing Arnd Bergmann
@ 2017-11-20  8:22             ` Michal Hocko
  2017-11-20 11:33               ` Arnd Bergmann
  0 siblings, 1 reply; 14+ messages in thread
From: Michal Hocko @ 2017-11-20  8:22 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Andrew Morton, Stephen Rothwell, linux-next, linux-kernel, Zhangshaokun

On Fri 17-11-17 11:15:45, Arnd Bergmann wrote:
> The cleanup caused build warnings for constant mask pointers:
> 
> mm/mempolicy.c: In function ‘mpol_to_str’:
> ./include/linux/nodemask.h:108:11: warning: the comparison will always evaluate as ‘true’ for the address of ‘nodes’ will never be NULL [-Waddress]
> 
> An earlier workaround I suggested was incorporated in the version that
> got merged, but that only solved the problem for gcc-7 and higher, while
> gcc-4.6 through gcc-6.x still warn.
> 
> This changes the printing again to use inline functions that make it
> clear to the compiler that the line that does the NULL check has no
> idea whether the argument is a constant NULL.
> 
> Fixes: 0205f75571e3 ("mm: simplify nodemask printing")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Thanks for the fixup. It is sad how a questionable warning makes the
code worse... Does it make sense to have the warning enabled?
Gcc bug  [1] suggests there is no great interest into fixing it.

Anyway to the patch
Acked-by: Michal Hocko <mhocko@suse.com>

[1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82963

> ---
> I've done only minimal build testing on this, but did check it with
> all compiler versions this time.
> ---
>  include/linux/nodemask.h | 13 ++++++++++---
>  1 file changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/include/linux/nodemask.h b/include/linux/nodemask.h
> index 15cab3967d6d..1fbde8a880d9 100644
> --- a/include/linux/nodemask.h
> +++ b/include/linux/nodemask.h
> @@ -104,9 +104,16 @@ extern nodemask_t _unused_nodemask_arg_;
>   *
>   * Can be used to provide arguments for '%*pb[l]' when printing a nodemask.
>   */
> -#define nodemask_pr_args(maskp)				\
> -	((maskp) != NULL) ? MAX_NUMNODES : 0,		\
> -	((maskp) != NULL) ? (maskp)->bits : NULL
> +#define nodemask_pr_args(maskp)	__nodemask_pr_numnodes(maskp), \
> +				__nodemask_pr_bits(maskp)
> +static inline unsigned int __nodemask_pr_numnodes(const nodemask_t *m)
> +{
> +	return m ? MAX_NUMNODES : 0;
> +}
> +static inline const unsigned long *__nodemask_pr_bits(const nodemask_t *m)
> +{
> +	return m ? m->bits : NULL;
> +}
>  
>  /*
>   * The inline keyword gives the compiler room to decide to inline, or
> -- 
> 2.9.0

-- 
Michal Hocko
SUSE Labs

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] mm: fix nodemask printing
  2017-11-20  8:22             ` Michal Hocko
@ 2017-11-20 11:33               ` Arnd Bergmann
  0 siblings, 0 replies; 14+ messages in thread
From: Arnd Bergmann @ 2017-11-20 11:33 UTC (permalink / raw)
  To: Michal Hocko
  Cc: Andrew Morton, Stephen Rothwell, Linux-Next Mailing List,
	Linux Kernel Mailing List, Zhangshaokun

On Mon, Nov 20, 2017 at 9:22 AM, Michal Hocko <mhocko@kernel.org> wrote:
> On Fri 17-11-17 11:15:45, Arnd Bergmann wrote:
>> The cleanup caused build warnings for constant mask pointers:
>>
>> mm/mempolicy.c: In function ‘mpol_to_str’:
>> ./include/linux/nodemask.h:108:11: warning: the comparison will always evaluate as ‘true’ for the address of ‘nodes’ will never be NULL [-Waddress]
>>
>> An earlier workaround I suggested was incorporated in the version that
>> got merged, but that only solved the problem for gcc-7 and higher, while
>> gcc-4.6 through gcc-6.x still warn.
>>
>> This changes the printing again to use inline functions that make it
>> clear to the compiler that the line that does the NULL check has no
>> idea whether the argument is a constant NULL.
>>
>> Fixes: 0205f75571e3 ("mm: simplify nodemask printing")
>> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>
> Thanks for the fixup. It is sad how a questionable warning makes the
> code worse... Does it make sense to have the warning enabled?
> Gcc bug  [1] suggests there is no great interest into fixing it.
>
> Anyway to the patch
> Acked-by: Michal Hocko <mhocko@suse.com>
>
> [1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82963

I think I found some real bugs when I started seeing that warning elsewhere,
so I'd rather keep it enabled in general. I'd still agree that the gcc behavior
in macros is questionable though, in particular since they decided to
stop warning for the "!= NULL" case. I've found the commit that changed
gcc-7 behavior and commented again in the bugzilla, but did not reopen.

The inline function was mentioned in bugzilla there[2] as the preferred
workaround.

       Arnd

[2] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=48778

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, back to index

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-13  5:42 linux-next: build warning after merge of the akpm-current tree Stephen Rothwell
2017-11-13  8:09 ` Michal Hocko
2017-11-13  8:23   ` Michal Hocko
2017-11-13 11:43   ` Arnd Bergmann
2017-11-13 11:54     ` Michal Hocko
2017-11-13 12:24       ` Arnd Bergmann
2017-11-13 12:29       ` Michal Hocko
2017-11-16 22:44     ` Stephen Rothwell
2017-11-17  3:53       ` Stephen Rothwell
2017-11-17  9:36         ` Zhangshaokun
2017-11-17  9:56         ` Arnd Bergmann
2017-11-17 10:15           ` [PATCH] mm: fix nodemask printing Arnd Bergmann
2017-11-20  8:22             ` Michal Hocko
2017-11-20 11:33               ` Arnd Bergmann

Linux-Next Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-next/0 linux-next/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-next linux-next/ https://lore.kernel.org/linux-next \
		linux-next@vger.kernel.org
	public-inbox-index linux-next

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-next


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git