CXX allocator concept pass + cpg-analysis/cpg-concepts dependency flip#2774
Open
oxisto wants to merge 6 commits into
Open
CXX allocator concept pass + cpg-analysis/cpg-concepts dependency flip#2774oxisto wants to merge 6 commits into
oxisto wants to merge 6 commits into
Conversation
The previous direction (cpg-concepts depending on cpg-analysis) was driven entirely by one file — PolicyQueries.kt — which is semantically a "query that uses concepts", not a concept definition. Moving it to cpg-analysis (where the cpg.query DSL lives) lets cpg-concepts shed the cpg-analysis dependency, and lets cpg-analysis take cpg-concepts as a clean dependency in turn. Why: the next steps need cpg-analysis to read concept overlays (e.g. ArrayValue consulting an Allocate concept for the size of a malloc/new), and want cpg-language-* to register concept passes via `@RegisterExtraPass` on their frontend. Both of those require concepts to sit *above* analysis, not the other way round. Flipping now avoids a scaffold HasAllocatedSize interface in cpg-core. - cpg-analysis/build.gradle.kts: api(projects.cpgConcepts). - cpg-concepts/build.gradle.kts: drop implementation(projects.cpgAnalysis); add api(projects.cpgCore) since the analysis api dependency previously pulled core in transitively. integrationTestImplementation kept so the PolicyTest in cpg-concepts still reaches the relocated query. - PolicyQueries.kt path moves cpg-concepts → cpg-analysis; package name unchanged so no import updates needed downstream.
`var size: Expression?` on the Allocate operation captures the expression that determines how many bytes/elements the allocation reserves. For C this is the `N` of `malloc(N)`, the `M * N` of `calloc(M, N)`, etc.; language-specific concept passes are free to populate it from whatever shape their allocator takes. `null` when the size can't be derived. The `newAllocate()` builder takes the new field as an optional parameter so existing call sites keep compiling unchanged.
Concept passes that recognise C/C++ stdlib/runtime functions belong with the C/C++ frontend, not in the generic cpg-concepts module. Moves CXXDynamicLoadingPass and CXXEntryPointsPass over and adds the cpg-concepts dependency on cpg-language-cxx so they can keep importing the concept definitions. Why move them at all: this PR is about to add CXXMemoryAllocationPass. Putting that next to its CXX siblings keeps everything consistent. It also unlocks `@RegisterExtraPass` on CXXLanguageFrontend so the passes auto-register with `defaultPasses()` — addressing the long-standing "users had to remember `registerPass<>()` manually" friction. - cpg-language-cxx/build.gradle.kts: implementation(projects.cpgConcepts). - ConceptPass.getConceptOrCreate: internal → protected so subclasses in other modules can still use it. - Package paths preserved; existing integration tests in cpg-concepts/integrationTest keep working unchanged because they already had cpg-language-cxx on the integration test classpath.
Recognises malloc / calloc / realloc and attaches an Allocate operation to each call, populated with `what` (the target variable, when obvious from the surrounding Variable initializer or Assign) and `size` (the allocation expression). For calloc the size is synthesised as a `*` BinaryOperator over the count and element-size arguments — ValueEvaluator constant-folds it when both operands are literals. Annotated CXXLanguageFrontend with @RegisterExtraPass so the pass auto-registers whenever a translation runs with `defaultPasses()` and the CXX frontend is enabled. Callers can still register it manually for custom configurations. Not yet covered: `new` / `new[]` (different AST node), aligned_alloc / posix_memalign (size in different arg positions) — both are one-line extensions to the when-branch.
ArrayValue.getSize previously string-matched `call.name.localName == "malloc"`.
With CXXMemoryAllocationPass now attaching an Allocate concept to every
recognised allocator call, the size lookup becomes generic:
call.overlays.filterIsInstance<Allocate>().firstOrNull()?.size
The same code path works for malloc/calloc/realloc and for any future
language pass that populates Allocate.size — no per-allocator special
cases in cpg-analysis.
Also for the pass itself:
- Memory concept attaches to the Component, not the TranslationUnit:
a C/C++ program has one heap conceptually, and all Allocate ops point
back to that shared concept.
- @dependsOn(EvaluationOrderGraphPass) + @dependsOn(SymbolResolver) so
the pass runs after the call name is resolved and the EOG is built —
newOperation inserts into the EOG path, which requires EOG to exist.
Covers: - malloc with a constant size → Allocate.size is the literal, .what is the bound Variable - malloc via an Assign rather than an initializer → .what still resolves through the LHS Reference - calloc(M, N) → Allocate.size is the synthesised `*` BinaryOperator with both operands intact - realloc(p, N) → .size is the new-size argument, not the old buffer - malloc(n) with a non-constant size → .size is the Reference to `n` (downstream is free to constant-evaluate, interval-bound, or DFG-trace) - a calls of other functions get no Allocate overlay attached
oxisto
requested review from
KuechA,
fwendland,
maximiliankaul and
peckto
as code owners
Contributor
There was a problem hiding this comment.
Pull request overview
This PR moves C/C++ allocator recognition into a CXX-specific concept pass and updates array-size evaluation to consume the language-agnostic Allocate concept, while also flipping the module dependency direction so cpg-analysis depends on cpg-concepts (and cpg-concepts depends only on cpg-core).
Changes:
- Introduce
CXXMemoryAllocationPassto attachAllocate(size, what)overlays tomalloc/calloc/realloccalls and auto-register it in the CXX frontend. - Extend the
Allocateconcept with an optionalsize: Expression?and propagate it through the concept builder. - Flip/adjust Gradle module dependencies and relocate CXX-specific concept passes into
cpg-language-cxx; movePolicyQueries.ktintocpg-analysis.
Reviewed changes
Copilot reviewed 11 out of 14 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| cpg-language-cxx/src/test/resources/allocations/allocations.c | Adds C allocation fixtures used by the new pass tests. |
| cpg-language-cxx/src/test/kotlin/de/fraunhofer/aisec/cpg/passes/concepts/memory/cxx/CXXMemoryAllocationPassTest.kt | Integration tests validating Allocate overlays and extracted sizes. |
| cpg-language-cxx/src/main/kotlin/de/fraunhofer/aisec/cpg/passes/concepts/memory/cxx/CXXMemoryAllocationPass.kt | New CXX pass that recognizes allocator calls and attaches Allocate concepts. |
| cpg-language-cxx/src/main/kotlin/de/fraunhofer/aisec/cpg/passes/concepts/memory/cxx/CXXDynamicLoadingPass.kt | Relocated CXX dynamic loading concept pass into the CXX language module. |
| cpg-language-cxx/src/main/kotlin/de/fraunhofer/aisec/cpg/passes/concepts/flows/cxx/CXXEntryPointsPass.kt | Relocated CXX entry point concept pass into the CXX language module. |
| cpg-language-cxx/src/main/kotlin/de/fraunhofer/aisec/cpg/frontends/cxx/CXXLanguageFrontend.kt | Auto-registers the new memory allocation pass via @RegisterExtraPass. |
| cpg-language-cxx/build.gradle.kts | Adds dependency on cpg-concepts needed for concept passes living in cpg-language-cxx. |
| cpg-concepts/src/main/kotlin/de/fraunhofer/aisec/cpg/passes/concepts/ConceptPass.kt | Widens getConceptOrCreate visibility to support subclasses outside the module. |
| cpg-concepts/src/main/kotlin/de/fraunhofer/aisec/cpg/graph/concepts/memory/MemoryConceptBuilder.kt | Adds size support to newAllocate. |
| cpg-concepts/src/main/kotlin/de/fraunhofer/aisec/cpg/graph/concepts/memory/Memory.kt | Adds size: Expression? to Allocate and updates equality/hashCode accordingly. |
| cpg-concepts/build.gradle.kts | Adjusts dependencies to remove cpg-analysis and expose cpg-core. |
| cpg-analysis/src/main/kotlin/de/fraunhofer/aisec/cpg/queries/concepts/policy/PolicyQueries.kt | Moves policy query utilities into cpg-analysis. |
| cpg-analysis/src/main/kotlin/de/fraunhofer/aisec/cpg/analysis/abstracteval/value/ArrayValue.kt | Uses Allocate.size overlay instead of malloc name matching. |
| cpg-analysis/build.gradle.kts | Adds dependency on cpg-concepts required by the updated analysis code. |
Comment on lines
+116
to
+121
| if (count != null && elemSize != null) { | ||
| call.newBinaryOperator("*").apply { | ||
| lhs = count | ||
| rhs = elemSize | ||
| } | ||
| } else null |
Comment on lines
+21
to
+23
| void malloc_unknown_size(int n) { | ||
| char *p = malloc(n); | ||
| } |
Comment on lines
+132
to
+136
| tu.calls | ||
| .filter { it.name.toString() !in setOf("malloc", "calloc", "realloc") } | ||
| .forEach { call -> | ||
| assertNull(call.overlays.filterIsInstance<Allocate>().firstOrNull()) | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Replaces the
mallocstring-match inArrayValue.getSizewith a generic read of anAllocatememory-concept overlay. The actual recognition of allocator calls moves into a new language-specific pass,CXXMemoryAllocationPass, which lives next to the C/C++ frontend.Along the way:
cpg-analysisnow depends oncpg-conceptsinstead of the other way round. The only coupling was one file (PolicyQueries.kt), which moved to its semantic home in cpg-analysis. The new chain iscpg-language-cxx → cpg-concepts → cpg-analysis → cpg-core.CXXDynamicLoadingPassandCXXEntryPointsPassmoved from cpg-concepts into cpg-language-cxx. cpg-concepts now contains only language-agnostic concept definitions.CXXLanguageFrontendcarries@RegisterExtraPass(CXXMemoryAllocationPass::class)so the pass runs automatically withdefaultPasses()whenever the C/C++ frontend is loaded — no more manualit.registerPass<>()for the common case.Commit walkthrough
PolicyQueries.kt→ cpg-analysis; gradle deps swapped.Allocate.size: Expression?— new field on the existing concept + builder support.CXXDynamicLoadingPass,CXXEntryPointsPass→ cpg-language-cxx.ConceptPass.getConceptOrCreatewidened frominternal→protected.CXXMemoryAllocationPass— matches malloc/calloc/realloc; for calloc the size is a synthesisedcount * elemSizeBinaryOperator(ValueEvaluator constant-folds when both operands are literals).ArrayValue.getSizeto readAllocateoverlays + auto-register the pass.Memoryconcept attaches to theComponent(one heap per program, shared across TUs).Why Expression for
size?A bare
Intonly handlesmalloc(constant). Keeping the originalExpressionnode means:(size.value.value as Number).toLong().malloc(n)): callers can run interval analysis, taint, DFG tracing, etc.count * elemSizefor calloc folds uniformly across literal-times-literal, variable-times-literal, etc.Not covered (one-line extensions, left for follow-ups)
new/new[]— different AST node shape in CDT.aligned_alloc,posix_memalign— size lives in different argument positions.Test plan
:cpg-core:test— green:cpg-analysis:test— green:cpg-language-cxx:test(including newCXXMemoryAllocationPassTest+ existingArraySizeEvaluatorTest,TypedefTest, etc.) — green:cpg-concepts:integrationTest(including the relocatedDynamicLoadingTest,EntryPointTest,PolicyTest) — green🤖 Generated with Claude Code