[PATCH] D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In D100581#2730743 <https://reviews.llvm.org/D100581#2730743>, @nickdesaulniers wrote:

> Testing Diff 342071 on the mainline Linux kernel, just building x86_64 defconfig triggers 19 instances of this warning; all look legit. ;)
>
> In D100581#2730708 <https://reviews.llvm.org/D100581#2730708>, @dblaikie wrote:
>
>> In D100581#2730557 <https://reviews.llvm.org/D100581#2730557>, @nickdesaulniers wrote:
>>
>>> I see lots of instances from the kernel that look like this when reduced:
>>>
>>>   $ cat foo.c
>>>   int page;
>>>   int put_page_testzero(int);
>>>   void foo (void) {
>>>     int zeroed;
>>>     zeroed = put_page_testzero(page);
>>>     ((void)(sizeof(( long)(!zeroed))));
>>>   }
>>>   $ clang -c -Wall foo.c
>>>   foo.c:4:7: warning: variable 'zeroed' set but not used [-Wunused-but-set-variable]
>>>     int zeroed;
>>>         ^
>>
>> Any idea what the purpose of this code is/why it's not a reasonable thing to warn on?
>
> include/linux/build_bug.h:
>
>   25 /*                                                                                                                                                                          
>   26  * BUILD_BUG_ON_INVALID() permits the compiler to check the validity of the                                                                                                 
>   27  * expression but avoids the generation of any code, even if that expression                                                                                                
>   28  * has side-effects.                                                                                                                                                        
>   29  */                                                                                                                                                                         
>   30 #define BUILD_BUG_ON_INVALID(e) ((void)(sizeof((__force long)(e))))
>
> include/linux/mmdebug.h:
>
>   57 #define VM_BUG_ON(cond) BUILD_BUG_ON_INVALID(cond)                                                                                                                          
>
> mm/internal.h:
>
>   410 static inline unsigned long                                                                                                                                                 
>   411 vma_address(struct page *page, struct vm_area_struct *vma)                                                                                                                  
>   412 {                                                                                                                                                                           
>   413   unsigned long start, end;                                                                                                                                                 
>   414                                                                                                                                                                             
>   415   start = __vma_address(page, vma);                                                                                                                                         
>   416   end = start + thp_size(page) - PAGE_SIZE;                                                                                                                                 
>   417                                                                                                                                                                             
>   418   /* page should be within @vma mapping range */                                                                                                                            
>   419   VM_BUG_ON_VMA(end < vma->vm_start || start >= vma->vm_end, vma);
>
> The warning (in previous revisions was triggering on `end` in `vma_address`.

Ah, fair enough. I guess this means the warning has the same false positive with a value that's initialized unconditional, but then read only in an assert (& thus unread in a non-asserts build)? As much as we're used to accepting that as a limitation of assert, I can see how that'd be good to avoid for new warnings to reduce false positives/code that would require modifications despite there being no bug in it.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100581/new/

https://reviews.llvm.org/D100581