-Wsizeof-array-div in mm/hugetlb.c

DISCUSSION Nathan Chancellor 3 participants 3 messages Sep 17, 2019 Memory management

Share & tools

Nathan Chancellor

author

2019-09-17 07:34 UTC #0 lore raw

Hi all,

Clang recently added a new diagnostic in r371605, -Wsizeof-array-div, that tries to warn when sizeof(X) / sizeof(Y) does not compute the number of elements in an array X (i.e., sizeof(Y) is wrong). See that commit for more details:

https://github.com/llvm/llvm-project/commit/3240ad4ced0d3223149b72a4fc2a4d9b67589427

There is a warning in mm/hugetlb.c in hugetlb_fault_mutex_hash:

mm/hugetlb.c:4055:40: warning: expression does not compute the number of elements in this array; element type is 'unsigned long', not 'u32' (aka 'unsigned int') [-Wsizeof-array-div]

        hash = jhash2((u32 *)&key, sizeof(key)/sizeof(u32), 0);
                                          ~~~ ^
mm/hugetlb.c:4049:16: note: array 'key' declared here
        unsigned long key[2];
                      ^

1 warning generated.

Should this warning be silenced? What is the reasoning behind having key be an array of unsigned longs but representing it as an array of u32s? Would it be better to avoid the cast and have it just be an array of u32s directly? I am not familiar with this code so I may be naive for asking such questions but we'd like to get these warnings cleaned up so that this warning can be useful down the road.

Cheers,
Nathan

Mike Kravetz

2019-09-17 21:06 UTC #1 lore raw

Quoted 19 lines · Nathan Chancellor

Hi all,
Clang recently added a new diagnostic in r371605, -Wsizeof-array-div, that tries to warn when sizeof(X) / sizeof(Y) does not compute the number of elements in an array X (i.e., sizeof(Y) is wrong). See that commit for more details:
https://github.com/llvm/llvm-project/commit/3240ad4ced0d3223149b72a4fc2a4d9b67589427
There is a warning in mm/hugetlb.c in hugetlb_fault_mutex_hash:
mm/hugetlb.c:4055:40: warning: expression does not compute the number of elements in this array; element type is 'unsigned long', not 'u32' (aka 'unsigned int') [-Wsizeof-array-div]
        hash = jhash2((u32 *)&key, sizeof(key)/sizeof(u32), 0);
                                          ~~~ ^
mm/hugetlb.c:4049:16: note: array 'key' declared here
        unsigned long key[2];
                      ^

Quoted 4 lines

1 warning generated.
Should this warning be silenced? What is the reasoning behind having key be an array of unsigned longs but representing it as an array of u32s?

Well, the second argument to jhash2 is "the number of u32's in the key". This is the reason for the sizeof(key)/sizeof(u32) calculation. It certainly is not trying to calculate the number of elements in the array as suggested by the warning.

Quoted 2 lines

Would it be better to avoid the cast and have it just be an array of u32s directly?

I did not write this code, but it is much easier to do the assignments (below) to build the key if the array is unsigned long as opposed to u32.

struct address_space *mapping;
pgoff_t idx;
unsigned long key[2];

        key[0] = (unsigned long) mapping;
        key[1] = idx;

Quoted 3 lines

u32s directly? I am not familiar with this code so I may be naive for asking such questions but we'd like to get these warnings cleaned up so that this warning can be useful down the road.

I suppose it would be possible to change 'key' to be something else besides an array (such as struct or union) to eliminate the warning. But, I would prefer to have some type of directive to indicate the code is ok as is. It is not trying to calculate the number of elements in the array as suspected by the clang diagnostic.

Signature · 1 line

Mike Kravetz

Dávid Bolvanský

2019-09-17 21:09 UTC #2 lore raw

You can use extra parens.

size(arr) / (size(int))

ut 17. 9. 2019 o 23:06 Mike Kravetz <[email protected]> napísal(a):

Quoted 60 lines · Nathan Chancellor

Quoted 8 lines
Hi all,
Clang recently added a new diagnostic in r371605, -Wsizeof-array-div, that tries to warn when sizeof(X) / sizeof(Y) does not compute the number of elements in an array X (i.e., sizeof(Y) is wrong). See that commit for more details:
https://github.com/llvm/llvm-project/commit/3240ad4ced0d3223149b72a4fc2a4d9b67589427
Quoted 11 lines
There is a warning in mm/hugetlb.c in hugetlb_fault_mutex_hash:
mm/hugetlb.c:4055:40: warning: expression does not compute the number of elements in this array; element type is 'unsigned long', not 'u32' (aka 'unsigned int') [-Wsizeof-array-div]
        hash = jhash2((u32 *)&key, sizeof(key)/sizeof(u32), 0);
                                          ~~~ ^
mm/hugetlb.c:4049:16: note: array 'key' declared here
        unsigned long key[2];
                      ^
Quoted 4 lines
1 warning generated.
Should this warning be silenced? What is the reasoning behind having key be an array of unsigned longs but representing it as an array of u32s?
Well, the second argument to jhash2 is "the number of u32's in the key". This is the reason for the sizeof(key)/sizeof(u32) calculation. It certainly
is not trying to calculate the number of elements in the array as suggested by
the warning.
Quoted 2 lines
Would it be better to avoid the cast and have it just be an array of u32s directly?
I did not write this code, but it is much easier to do the assignments (below)
to build the key if the array is unsigned long as opposed to u32.
struct address_space *mapping;
pgoff_t idx;
unsigned long key[2];
        key[0] = (unsigned long) mapping;
        key[1] = idx;
Quoted 3 lines
u32s directly? I am not familiar with this code so I may be naive for asking such questions but we'd like to get these warnings cleaned up so that this warning can be useful down the road.
I suppose it would be possible to change 'key' to be something else besides an array (such as struct or union) to eliminate the warning. But, I would prefer to have some type of directive to indicate the code is ok as is. It is not trying to calculate the number of elements in the array as suspected by the clang diagnostic.
--
Mike Kravetz