Skip to content

Conversation

@michael-schwarz
Copy link
Member

  • Removes redundant warning facilities from malloc_null
  • Fixes interprocedural aspect:
    • If a function f(int x) is called as f(y), nullness information should be transferred from y in the callee state to x in the caller state
    • Adds a test where contexts are disabled, and one relies only on malloc_null

Closes #1691.

@michael-schwarz michael-schwarz added cleanup Refactoring, clean-up bug unsound labels Jul 17, 2025
@sim642 sim642 self-requested a review July 17, 2025 14:39
@sim642
Copy link
Member

sim642 commented Jul 21, 2025

I added a few more NOWARNs to the old tests and it revealed even more weirdness with the malloc_null analysis:

  1. It warned on one sizeof(*v) although that doesn't actually evaluate the argument, it just gets the type, which is statically known. The base analysis uses Cil.constFold which takes care of it for us so the base analysis never even sees it as evaluation of *v.
  2. If sizeof(*v) was done with v being uninitialized (maybe NULL, maybe not), then the same thing did not warn. So I think the analysis is weird and possibly wrong in other ways as well: because v is possibly NULL initially, malloc_null doesn't have it as such in its state and doesn't handle it soundly even. But this becomes irrelevant when it's only for path-sensitivity anyway: it just doesn't induce a path-split on the entry state immediately.

@michael-schwarz
Copy link
Member Author

malloc_null doesn't have any warning facilities after this PR, so the weirdness must come from the base analysis (maybe exposed by doing more path-splitting)?

@sim642
Copy link
Member

sim642 commented Jul 21, 2025

Sorry, I meant the warnings from malloc_null as they are before this PR. So some of the NOWARNs that I added failed before but pass now that only base does the warnings.

@sim642 sim642 added this to the v2.6.0 milestone Jul 21, 2025
@michael-schwarz michael-schwarz merged commit 05b3499 into master Jul 21, 2025
19 checks passed
@michael-schwarz michael-schwarz deleted the issue_1691 branch July 21, 2025 17:25
sim642 added a commit to sim642/opam-repository that referenced this pull request Sep 5, 2025
CHANGES:

* Add division by zero analysis (goblint/analyzer#1764).
* Add bitfield domain (goblint/analyzer#1623).
* Add weakly-relational C-2PO pointer analysis (goblint/analyzer#1485).
* Add widening delay (goblint/analyzer#1358, goblint/analyzer#1442, goblint/analyzer#1483).
* Add narrowing of globals to top-down solver (goblint/analyzer#1636).
* Add weak dependencies to top-down solver (goblint/analyzer#1746, goblint/analyzer#1747).
* Add YAML ghost witness generation (goblint/analyzer#1394).
* Remove GraphML witness generation (goblint/analyzer#1732, goblint/analyzer#1733, goblint/analyzer#1738).
* Use C standard option for preprocessing (goblint/analyzer#1807).
* Add bash completion for array options (goblint/analyzer#1670, goblint/analyzer#1705, goblint/analyzer#1750).
* Make `malloc(0)` semantics configurable (goblint/analyzer#1418, goblint/analyzer#1777).
* Update path-sensitive analyses (goblint/analyzer#1785, goblint/analyzer#1791, goblint/analyzer#1792).
* Fix evaluation of library function arguments (goblint/analyzer#1758, goblint/analyzer#1761).
* Optimize affine equalities analysis using sparse matrices (goblint/analyzer#1459, goblint/analyzer#1625).
* Prepare for parallelism (goblint/analyzer#1708, goblint/analyzer#1744, goblint/analyzer#1748, goblint/analyzer#1781, goblint/analyzer#1790).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug cleanup Refactoring, clean-up unsound

Projects

None yet

Development

Successfully merging this pull request may close these issues.

malloc_null: Interprocedural analysis unsound

3 participants