<table border="1" cellspacing="0" cellpadding="8">
    <tr>
        <th>Issue</th>
        <td>
            <a href=https://github.com/llvm/llvm-project/issues/121880>121880</a>
        </td>
    </tr>

    <tr>
        <th>Summary</th>
        <td>
            [Static Analyzer] Incorrect invocation count of `checkEndFunction` callback.
        </td>
    </tr>

    <tr>
      <th>Labels</th>
      <td>
            new issue
      </td>
    </tr>

    <tr>
      <th>Assignees</th>
      <td>
      </td>
    </tr>

    <tr>
      <th>Reporter</th>
      <td>
          shenjunjiekoda
      </td>
    </tr>
</table>

<pre>
    ## Issue Report: Incorrect Callback Count for `checkEndFunction`

### Test Case

```cpp
int leak_partial(Manager *obj, int &k, int y) {
  obj = Manager::CreateObject();
  if (obj == nullptr) {
 k = 1;
    return 1;
  } 
  
  if (k == 0){
 Manager::RemoveObject(obj);
    k = 2;
    return 2;
  } 
  k = 3;
 return 3;
}
```

### Problem Description

In our CSA checker, whenever a function registers `CheckEndFunction`, it should ideally trigger the callback for each return path. However, it currently only triggers twice: once for `return 1` and once for `return 2` on my machine. With different traversal strategies, it might be `return 3` and `return 2`.

In the checker's `EndFunction` callback, the `ExplodedNode` for `return 1` shows the path leading to it, as confirmed by traversing the `FirstPred`. This was verified by printing the `Stmt` obtained through `getStmtForDiagnostics`.

### Root Cause Analysis

The issue is in `ExprEngine.cpp` within `ExprEngine::processEndOfFunction`. During top-frame function analysis, `removeDead` is called first, which generates a set of nodes. The `CheckEndFunction` callbacks are then invoked on these nodes.

```cpp
void ExprEngine::processEndOfFunction(NodeBuilderContext& BC,
                                      ExplodedNode *Pred,
 const ReturnStmt *RS) {
  // ...
  ExplodedNodeSet Dst;
  if (Pred->getLocationContext()->inTopFrame()) {
    // Remove dead symbols.
    ExplodedNodeSet AfterRemovedDead;
 removeDeadOnEndOfFunction(BC, Pred, AfterRemovedDead);

    // Notify checkers.
    for (const auto I : AfterRemovedDead)
 getCheckerManager().runCheckersForEndFunction(BC, Dst, I, *this, RS); // call back here now.
  } else {
 getCheckerManager().runCheckersForEndFunction(BC, Dst, Pred, *this, RS);
 }

  Engine.enqueueEndOfFunction(Dst, RS);
}
```

In this test case, after `removeDead`, the profile node hash values for `return 1` and `return 3` are identical, causing only one node to be inserted. This seems unintended, as these branches should not be merged into a single `EndFunction` callback. By invoking the `EndFunction` callback before `removeDead`, each branch receives a separate callback, including `return 1`, `return 2`, and `return 3`.

```cpp
void ExprEngine::processEndOfFunction(NodeBuilderContext& BC,
 ExplodedNode *Pred,
 const ReturnStmt *RS) {
  // ...
  ExplodedNodeSet Dst;
  if (Pred->getLocationContext()->inTopFrame()) {
    // Notify checkers.
 getCheckerManager().runCheckersForEndFunction(BC, Dst, Pred, *this, RS);

 // Remove dead symbols.
    ExplodedNodeSet AfterRemovedDead;
 removeDeadOnEndOfFunction(BC, Pred, AfterRemovedDead);
  } else {
 getCheckerManager().runCheckersForEndFunction(BC, Dst, Pred, *this, RS);
 }

  Engine.enqueueEndOfFunction(Dst, RS);
}
```

### Additional Observations

Before the change, the identical hash values for these nodes are puzzling. `removeDead` seems to consider external variables in the top stack frame as dead, excluding the value of `k` from the store. This interface should not assume the end of life for top-frame variables, as `int &k` could be modified externally. This may relate to CSA's top-down interprocedural analysis strategy. A suggestion is to initially mark pointers, references, and arrays as live to avoid unexpected reports without significant performance overhead.

### Request for Feedback

I would appreciate any insights or feedback on this issue. If the respected contributors and maintainers find either of these fixes reasonable, I am willing to implement a solution.

</pre>
<img width="1" height="1" alt="" src="http://email.email.llvm.org/o/eJzUWF1v4zrO_jXqDTGB4zQfvchFmkzwFnj3zMHMAHu5kCXa1kSWfCg5aebXLyjbSdpmdhdYHGBPEaCJRZHUQ-ohaRmCqRziWsyfxXz3ILtYe1qHGt2Pzv0wePBaPhRen9cin4l8Bi8hdAhfsfUUxWwDL055IlQRttLaQqoDbH3nIpSeQCwyVaM6fHZ63zkVjXdikYlsw5-kj1V-x8C7Aw4Li6z_qLYV2ca4CBbl4R-tpGikFfnqb9LJCglEvvHFD5FvgYVEvjiM388ifwKxfBbZBsAXP0DMdjBsE7ONmG22hDLil-IHqijylcifxKwXNyWIfDVs4n2us7aNdKPykPRNxx0AhLEjd30iljvov1w1HkZ9GRvrFb1x6Ss2_nh1KR3t6WqjN5p_MJp_MNpLzobng9jwUyx3txi_j8Xv5AuLDewwKDJtilgSeXHgO4Lttw2kkCIx1qcaHR6RQEI5BBgIKxMiUuDwb--En2MUIdS-sxqMRmntGSKZimMaawQ1ZhLnEEpVj2doZawn8H_-xDYHPaojQhftGby76gkQT0YhZ6h3CsdsvMRpkYF0-t5azmveQXOGRqraOJzA302sQZuyRDYFkeQRKUgLIZKMWBkMgzeNqeoIBd4onI3G3tqYXHBNRx4xXSbY3iJ2AYSNsDRLvLbWa9S_eY0scueAofankOQZN75E2rgKogcTWZMMoLwrDTWooTiPx0oyvZG9oRB_J9TsL3yvTYCTDHBEMqXpN7VkXLzZ8i02MSFYRGkcaog1-a6qea3CyMt7TzsjK-dDNCpcobhm4VfvmRG6gLBx0p6DCb3I9xrBJAYyAYwbgKDPruI4MWEsMjiZWL9f629YS15hCJ-d_lLe4DuBXUc9NO2nkmSD12yWo_182-PLd3SHkiFhJzg0qKFkpPobYVQNFTrkzAggIWAEX4LzGgODiL-4F5coB5CEjKcD447-gJyn_DvgoOUuUR690fAfnThfcdY8d8ZqpK13EV-jyBfwvBX5diCXf_93m4FMxSlP-v3KuxDha8pFjjgvf_12y8ki34t8D5PJJP281fUNI-xCfEvHrPyTmH2uMP6_V5KPcXGcyZvXjPvu2z2HbyD0G3sXiz3JgkapIZybwtswGSTeO7EpI1Ivr1PERz4dU-CLe49rQhAGJD4qGOn8rUu_-WjK80gBF3fSjc5XPZayix5egPnsntZsAxXGba9hLCoJhAl1bnge9p5uU250d9dn7ktK8XwT6z7bU8DE7Hl0k9MTEi3XSJyJp8ml7KANOGL9XzsywnfHF9Y_VLCUNv29R_dHhx2-D8ag7mbv3eKXGNgEiNyGKG5DmBsZ5A8XfuTflnxpbH8boZahhqO0HYZflZn3xYCQ656LRnE_swUlu0S7qYJ5NyiOnguJcQEpoh74NyA2ATpnXESne6BkGNihIOlUjWEsrs6nWtQgVai5MfLMR8ZVFv9FlZnA87lnnhte_4UsFFh6wrtIpcrduwSECs1x4MNWMje-qWrGKdul8vQWvgvtXupmOvAHTP90Svxrst09avmT72e6ov9DXPvXJ6hrX7TR2vBeaeFLEZCOKTWG3ui5v4l9NyldhSNbXajmA1Xd9BSJlNru509rXDX52Or0vBN9ynajkQBfIxK7cpRkZGExdWRsMPoWQkwNfGqmZEg5kDjhdbznLJhc4eZILLJD6mLJN2klRE84UB5THZVS4S2tyRC6pj8tciNfgjVl38tfu7iLZwNLikV2mRSZxJI2Jkiv-4Z2PJM9D7YbeQZCy3QVPU8_qUNnC9qfXO9aIhTdkbSXbnEcDM4T2EDoqgpD6iZNgtA4w5Os5QmDDtD6pCY5SZhmDDW47DRIInkO7L01x-SFTLTWOXxtUUXUQGkaD6nx9V0EnulNaZR0EVqk0lMjedDxR6Qapf7Yb-MfHVc_Rm-PqBMr98URTgkk2baEyjAO0nF1CDzoBPAE5bChb1I5XtygT-ClTNEhDIObyrtIpuiip5CO1kjj0pRAAUrjNKCJNRIHs8_M0rxiAEIZvOM4pjYFZAMnY-04yzStxYbHMgnB245x5gM-6PVMP82e5AOup8vZIs-zbP70UK9n5Ww-Xz3K6WpaPi0fpZbT6VI9ymyW6cV8UTyYdZ7l82yaLbNZns2fJovFUj2q1XRZFNlMoRKPGTbS2Im1x2biqXpIR15P8-lqlT1YWaAN6ZVKnjs89YCIPBfz3QOtedOnoquCeMysCTFc1UQTbXoX8y3KaFQ__PxEEvPdzYsWLs19UeAMdnG4QHfetFxL-kNHdl3H2AYugomeKxPrrpgo34h8zy4M_z615PtXEPvkeBD5fjjZcZ3_MwAA__-qKtKn">