Bug 110830 - -Wanalyzer-use-of-uninitialized-value false negative due to use-after-free::supercedes_p.
Summary: -Wanalyzer-use-of-uninitialized-value false negative due to use-after-free::s...
Status: UNCONFIRMED
Alias: None
Product: gcc
Classification: Unclassified
Component: analyzer (show other bugs)
Version: 14.0
: P3 normal
Target Milestone: ---
Assignee: David Malcolm
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2023-07-27 12:41 UTC by Benjamin Priour
Modified: 2023-09-07 21:02 UTC (History)
0 users

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


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Benjamin Priour 2023-07-27 12:41:41 UTC
state_machine::supercedes_p is called when two diagnostics are emitted for the same statement, without regarding the path that led to this statement.
See reproducer on trunk https://godbolt.org/z/GqebW5s5h 

#include <stdlib.h>

extern int ext();

int* foo()
{
  int *p = 0;
  if (ext() > 5)
  {
    p = malloc (sizeof(int));
    *p = 0;
    free (p);
    return p;
  }
  else
    return malloc(sizeof(int));
}

void test()
{
  int *y = foo(); // (*)
  int x = 4 + *y;
  free (y);
}

At statement (*) both -Wanalyzer-use-of-uninitialized-value and -Wanalyzer-use-after-free should be emitted as solving the latter won't impact the former, since they result from two independent branches. But use_after_free::supercedes_p hides the other.

In the case of a false positive -Wanalyzer-use-after-free, or simply one ignored by the user, the adjacent -Wanalyzer-use-of-uninitialized-value would therefore never be emitted.
Comment 1 David Malcolm 2023-07-31 22:40:34 UTC
For reference, I implemented use_after_free::supercedes_p in commit g:33255ad3ac14e3953750fe0f2d82b901c2852ff6 as part of the gcc 12 (re)implementation of -Wanalyzer-use-of-uninitialized-value.
Comment 2 David Malcolm 2023-07-31 22:43:29 UTC
The "supercedes_p" logic is called in diagnostic_manager::emit_saved_diagnostics here:
  best_candidates.handle_interactions (this);

I *think* every saved_diagnostic ought to have a non-NULL m_best_epath by the time this is called.
Comment 3 GCC Commits 2023-09-07 21:02:11 UTC
The trunk branch has been updated by Benjamin Priour <vultkayn@gcc.gnu.org>:

https://gcc.gnu.org/g:7d2274b9e346f44f8f6598b9dbb9fa95259274a2

commit r14-3794-g7d2274b9e346f44f8f6598b9dbb9fa95259274a2
Author: benjamin priour <vultkayn@gcc.gnu.org>
Date:   Fri Sep 1 20:21:41 2023 +0200

    analyzer: Call off a superseding when diagnostics are unrelated [PR110830]
    
    Before this patch, a saved_diagnostic would supersede another at
    the same statement if and only its vfunc supercedes_p returned true
    for the other diagnostic's kind.
    That both warning were unrelated - i.e. resolving one would not fix
    the other - was not considered in making the above choice.
    
    This patch makes it so that two saved_diagnostics taking a different
    outcome of at least one common conditional branching cannot supersede
    each other.
    
    Signed-off-by: Benjamin Priour <vultkayn@gcc.gnu.org>
    Co-authored-by: David Malcolm <dmalcolm@redhat.com>
    Signed-off-by: David Malcolm <dmalcolm@redhat.com>
    
    gcc/analyzer/ChangeLog:
    
            PR analyzer/110830
            * diagnostic-manager.cc
            (compatible_epaths_p): New function.
            (saved_diagnostic::supercedes_p): Now calls the above
            to determine if the diagnostics do overlap and the superseding
            may proceed.
    
    gcc/testsuite/ChangeLog:
    
            PR analyzer/110830
            * c-c++-common/analyzer/pr110830.c: New test.