Bug 108745 - -Wanalyzer-deref-before-check false positives seen in ImageMagick due to checks in macros
Summary: -Wanalyzer-deref-before-check false positives seen in ImageMagick due to chec...
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: analyzer (show other bugs)
Version: 13.0
: P3 normal
Target Milestone: ---
Assignee: David Malcolm
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2023-02-09 19:59 UTC by David Malcolm
Modified: 2023-02-10 23:18 UTC (History)
0 users

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed:


Attachments
Reproducer (540 bytes, text/plain)
2023-02-09 19:59 UTC, David Malcolm
Details

Note You need to log in before you can comment on or make changes to this bug.
Description David Malcolm 2023-02-09 19:59:50 UTC
Created attachment 54441 [details]
Reproducer

Am seeing lots of false positives from -Wanalyzer-deref-before-check on ImageMagick on code like this:

  if ((image->columns == 0) || (image->rows == 0))
    ThrowReaderException("MustSpecifyImageSize");
  return image;

where ThrowReaderException has an "if (image)" embedded within it:

https://godbolt.org/z/74d1aoqTa

<source>: In function 'ReadMAPImage':
<source>:38:6: warning: check of 'image' for NULL after already dereferencing it [-Wanalyzer-deref-before-check]
   38 |   if ((image) != (Image *) NULL) \
      |      ^
<source>:52:5: note: in expansion of macro 'ThrowReaderException'
   52 |     ThrowReaderException("MustSpecifyImageSize");
      |     ^~~~~~~~~~~~~~~~~~~~
  'ReadMAPImage': event 1
    |
    |   51 |   if ((image->columns == 0) || (image->rows == 0))
    |      |        ~~~~~^~~~~~~~~
    |      |             |
    |      |             (1) pointer 'image' is dereferenced here
    |
  'ReadMAPImage': event 2
    |
    |   38 |   if ((image) != (Image *) NULL) \
    |      |      ^
    |      |      |
    |      |      (2) pointer 'image' is checked for NULL here but it was already dereferenced at (1)
<source>:52:5: note: in expansion of macro 'ThrowReaderException'
    |   52 |     ThrowReaderException("MustSpecifyImageSize");
    |      |     ^~~~~~~~~~~~~~~~~~~~
    |
Compiler returned: 0

Presumably we shouldn't warn if the check is hidden inside a macro.
Comment 1 GCC Commits 2023-02-10 23:12:01 UTC
The master branch has been updated by David Malcolm <dmalcolm@gcc.gnu.org>:

https://gcc.gnu.org/g:aa601e30758581837c9ca7b738ec2810a18350f5

commit r13-5811-gaa601e30758581837c9ca7b738ec2810a18350f5
Author: David Malcolm <dmalcolm@redhat.com>
Date:   Fri Feb 10 18:10:21 2023 -0500

    analyzer: don't warn for deref-before-check for checks in macros [PR108745]
    
    Integration testing shows this patch fixes all 9 known false positives
    from -Wanalyzer-deref-before-check within ImageMagick-7.1.0-57, and
    eliminates 34 further as-yet unassessed such diagnostics, without
    eliminating the 1 known true positive.
    
    This improves the rate of true positives for the warning from
    1.56% to 4.76% of the total:
    
    -Wanalyzer-deref-before-check: 1.56% -> 4.76% (GOOD: 1 BAD: 63->20)
        TRUE:  1
       FALSE: 15 ->  6 (-9)
                 ImageMagick-7.1.0-57:  9 ->  0 (-9)
        TODO: 48 -> 14 (-34)
                 ImageMagick-7.1.0-57: 21 ->  1 (-20)
                           qemu-7.2.0: 25 -> 11 (-14)
    
    gcc/analyzer/ChangeLog:
            PR analyzer/108745
            * sm-malloc.cc (deref_before_check::emit): Reject the warning if
            the check occurs within a macro defintion.
    
    gcc/testsuite/ChangeLog:
            PR analyzer/108745
            * gcc.dg/analyzer/deref-before-check-macro-pr108745.c: New test.
            * gcc.dg/analyzer/deref-before-check-macro.c: New test.
    
    Signed-off-by: David Malcolm <dmalcolm@redhat.com>
Comment 2 David Malcolm 2023-02-10 23:18:47 UTC
Should be fixed by the above patch.  
The warning is new in GCC 13 so no backport needed.