Skip to content

Commit f5352be

Browse files
committed
Add CodeQL query to check for allocations not preceeded by ensure_free
Fix several cases where this happened in nifs. Also add a NOLINT comment for cases where the query is not smart enough to remove the couple of false positives. Signed-off-by: Paul Guyot <pguyot@kallisys.net>
1 parent c90f8c9 commit f5352be

File tree

8 files changed

+268
-7
lines changed

8 files changed

+268
-7
lines changed

.github/workflows/codeql-analysis.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ jobs:
6565
with:
6666
languages: ${{ matrix.language }}
6767
build-mode: manual
68-
queries: +./code-queries/term-to-non-term-func.ql,./code-queries/non-term-to-term-func.ql,./code-queries/mismatched-atom-string-length.ql,./code-queries/mismatched-free-type.ql,./code-queries/term-use-after-gc.ql,./code-queries/allocations-exceeding-ensure-free.ql
68+
queries: +./code-queries/term-to-non-term-func.ql,./code-queries/non-term-to-term-func.ql,./code-queries/mismatched-atom-string-length.ql,./code-queries/mismatched-free-type.ql,./code-queries/term-use-after-gc.ql,./code-queries/allocations-exceeding-ensure-free.ql,./code-queries/allocations-without-ensure-free.ql,./code-queries/allocations-without-ensure-free.ql
6969

7070
- name: "Build"
7171
run: |

.github/workflows/esp32-build.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ jobs:
7474
with:
7575
languages: "cpp"
7676
build-mode: manual
77-
queries: +./code-queries/term-to-non-term-func.ql,./code-queries/non-term-to-term-func.ql,./code-queries/mismatched-atom-string-length.ql,./code-queries/mismatched-free-type.ql,./code-queries/term-use-after-gc.ql,./code-queries/allocations-exceeding-ensure-free.ql
77+
queries: +./code-queries/term-to-non-term-func.ql,./code-queries/non-term-to-term-func.ql,./code-queries/mismatched-atom-string-length.ql,./code-queries/mismatched-free-type.ql,./code-queries/term-use-after-gc.ql,./code-queries/allocations-exceeding-ensure-free.ql,./code-queries/allocations-without-ensure-free.ql
7878

7979
- name: Build with idf.py
8080
shell: bash

.github/workflows/pico-build.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ jobs:
9292
with:
9393
languages: "cpp"
9494
build-mode: manual
95-
queries: +./code-queries/term-to-non-term-func.ql,./code-queries/non-term-to-term-func.ql,./code-queries/mismatched-atom-string-length.ql,./code-queries/mismatched-free-type.ql,./code-queries/term-use-after-gc.ql,./code-queries/allocations-exceeding-ensure-free.ql
95+
queries: +./code-queries/term-to-non-term-func.ql,./code-queries/non-term-to-term-func.ql,./code-queries/mismatched-atom-string-length.ql,./code-queries/mismatched-free-type.ql,./code-queries/term-use-after-gc.ql,./code-queries/allocations-exceeding-ensure-free.ql,./code-queries/allocations-without-ensure-free.ql
9696

9797
- name: Build
9898
shell: bash

.github/workflows/stm32-build.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ jobs:
7878
with:
7979
languages: 'cpp'
8080
build-mode: manual
81-
queries: +./code-queries/term-to-non-term-func.ql,./code-queries/non-term-to-term-func.ql,./code-queries/mismatched-atom-string-length.ql,./code-queries/mismatched-free-type.ql,./code-queries/term-use-after-gc.ql,./code-queries/allocations-exceeding-ensure-free.ql
81+
queries: +./code-queries/term-to-non-term-func.ql,./code-queries/non-term-to-term-func.ql,./code-queries/mismatched-atom-string-length.ql,./code-queries/mismatched-free-type.ql,./code-queries/term-use-after-gc.ql,./code-queries/allocations-exceeding-ensure-free.ql,./code-queries/allocations-without-ensure-free.ql
8282

8383
- name: Build
8484
shell: bash

.github/workflows/wasm-build.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ jobs:
6262
with:
6363
languages: ${{matrix.language}}
6464
build-mode: manual
65-
queries: +./code-queries/term-to-non-term-func.ql,./code-queries/non-term-to-term-func.ql,./code-queries/mismatched-atom-string-length.ql,./code-queries/mismatched-free-type.ql,./code-queries/term-use-after-gc.ql,./code-queries/allocations-exceeding-ensure-free.ql
65+
queries: +./code-queries/term-to-non-term-func.ql,./code-queries/non-term-to-term-func.ql,./code-queries/mismatched-atom-string-length.ql,./code-queries/mismatched-free-type.ql,./code-queries/term-use-after-gc.ql,./code-queries/allocations-exceeding-ensure-free.ql,./code-queries/allocations-without-ensure-free.ql
6666

6767
- name: Compile AtomVM and test modules
6868
run: |
Lines changed: 244 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,244 @@
1+
/**
2+
* This file is part of AtomVM.
3+
*
4+
* Copyright 2026 Paul Guyot <pguyot@kallisys.net>
5+
*
6+
* @name Allocations without ensure_free in NIFs and port handlers
7+
* @kind problem
8+
* @problem.severity error
9+
* @id atomvm/allocations-without-ensure-free
10+
* @tags correctness
11+
* @precision high
12+
*
13+
* SPDX-License-Identifier: Apache-2.0 OR LGPL-2.1-or-later
14+
*/
15+
16+
import cpp
17+
import semmle.code.cpp.controlflow.BasicBlocks
18+
19+
/**
20+
* Holds if the function `f` directly calls `memory_heap_alloc`.
21+
*/
22+
predicate directlyCallsHeapAlloc(Function f) {
23+
exists(FunctionCall fc |
24+
fc.getEnclosingFunction() = f and
25+
fc.getTarget().hasName("memory_heap_alloc")
26+
)
27+
}
28+
29+
/**
30+
* Holds if `f` directly calls any memory_ensure_free variant.
31+
*/
32+
pragma[noinline]
33+
predicate directlyCallsEnsureFree(Function f) {
34+
exists(FunctionCall fc |
35+
fc.getEnclosingFunction() = f and
36+
(
37+
fc.getTarget().hasName("memory_ensure_free") or
38+
fc.getTarget().hasName("memory_ensure_free_opt") or
39+
fc.getTarget().hasName("memory_ensure_free_with_roots") or
40+
fc.getTarget().hasName("memory_erl_nif_env_ensure_free") or
41+
fc.getTarget().hasName("memory_init_heap") or
42+
fc.getTarget().hasName("memory_init_heap_root_fragment")
43+
)
44+
)
45+
}
46+
47+
/**
48+
* Holds if `caller` directly calls `callee`.
49+
*/
50+
pragma[noinline]
51+
predicate callEdge(Function caller, Function callee) {
52+
exists(FunctionCall fc |
53+
fc.getEnclosingFunction() = caller and
54+
callee = fc.getTarget()
55+
)
56+
}
57+
58+
/**
59+
* Holds if `f` directly or transitively calls any memory_ensure_free variant.
60+
*/
61+
pragma[nomagic]
62+
predicate callsEnsureFree(Function f) {
63+
directlyCallsEnsureFree(f)
64+
or
65+
exists(Function callee |
66+
callEdge(f, callee) and
67+
callsEnsureFree(callee)
68+
)
69+
}
70+
71+
/**
72+
* Holds if `f` transitively calls `memory_heap_alloc` (directly or through callees)
73+
* AND does not call any ensure_free variant (meaning it relies on the caller to
74+
* have ensured enough heap space).
75+
*/
76+
pragma[nomagic]
77+
predicate transitivelyCallsHeapAllocWithoutEnsureFree(Function f) {
78+
directlyCallsHeapAlloc(f) and not callsEnsureFree(f)
79+
or
80+
not callsEnsureFree(f) and
81+
exists(Function callee |
82+
callEdge(f, callee) and
83+
transitivelyCallsHeapAllocWithoutEnsureFree(callee)
84+
)
85+
}
86+
87+
/**
88+
* Holds if `fc` is a call to one of the memory_ensure_free variants.
89+
*/
90+
predicate isEnsureFreeCall(FunctionCall fc) {
91+
fc.getTarget().hasName("memory_ensure_free")
92+
or
93+
fc.getTarget().hasName("memory_ensure_free_opt")
94+
or
95+
fc.getTarget().hasName("memory_ensure_free_with_roots")
96+
or
97+
fc.getTarget().hasName("memory_erl_nif_env_ensure_free")
98+
}
99+
100+
/**
101+
* Holds if `allocCall` is a function call that transitively calls
102+
* `memory_heap_alloc` without its own ensure_free.
103+
*/
104+
predicate isAllocatingCall(FunctionCall allocCall) {
105+
transitivelyCallsHeapAllocWithoutEnsureFree(allocCall.getTarget()) and
106+
not isEnsureFreeCall(allocCall) and
107+
not allocCall.getTarget().hasName("memory_heap_alloc")
108+
}
109+
110+
/**
111+
* Holds if `f` is a NIF: returns `term` and has parameters (Context *, int, term []).
112+
*/
113+
predicate isNif(Function f) {
114+
f.getNumberOfParameters() = 3 and
115+
f.getType().toString() = "term" and
116+
f.getParameter(0).getType().stripType().(Struct).hasName("Context") and
117+
f.getParameter(2).getType().toString() = "term[]"
118+
}
119+
120+
/**
121+
* Holds if `f` is a port handler: returns `NativeHandlerResult` and has
122+
* a single parameter (Context *).
123+
*/
124+
predicate isPortHandler(Function f) {
125+
f.getNumberOfParameters() = 1 and
126+
f.getType().toString() = "NativeHandlerResult" and
127+
f.getParameter(0).getType().stripType().(Struct).hasName("Context")
128+
}
129+
130+
/**
131+
* Holds if `f` is a NIF or port handler entry point.
132+
*/
133+
predicate isEntryPoint(Function f) {
134+
isNif(f) or isPortHandler(f)
135+
}
136+
137+
/**
138+
* Holds if `bb` contains at least one call to an ensure_free variant
139+
* or to a function that transitively calls ensure_free.
140+
*/
141+
predicate isBarrierBlock(BasicBlock bb) {
142+
exists(FunctionCall fc |
143+
fc.getBasicBlock() = bb and
144+
(
145+
isEnsureFreeCall(fc)
146+
or
147+
callsEnsureFree(fc.getTarget())
148+
)
149+
)
150+
}
151+
152+
/**
153+
* Holds if `bb` is reachable from the entry point of `f` via a path
154+
* that does not pass through any barrier block (a block containing
155+
* an ensure_free call or a call that transitively calls ensure_free).
156+
*/
157+
pragma[nomagic]
158+
predicate reachableFromEntryAvoidingBarriers(BasicBlock bb, Function f) {
159+
bb = f.getEntryPoint().getBasicBlock() and
160+
not isBarrierBlock(bb)
161+
or
162+
exists(BasicBlock pred |
163+
reachableFromEntryAvoidingBarriers(pred, f) and
164+
pred.getASuccessor() = bb and
165+
not isBarrierBlock(bb)
166+
)
167+
}
168+
169+
/**
170+
* Holds if there is an ensure_free call (or a call to a function that
171+
* transitively calls ensure_free) before `allocCall` within the same
172+
* basic block.
173+
*/
174+
predicate hasEnsureFreeBeforeInBlock(FunctionCall allocCall) {
175+
exists(FunctionCall ef, BasicBlock bb, int efIdx, int allocIdx |
176+
(isEnsureFreeCall(ef) or callsEnsureFree(ef.getTarget())) and
177+
bb = allocCall.getBasicBlock() and
178+
ef.getBasicBlock() = bb and
179+
bb.getNode(efIdx) = ef and
180+
bb.getNode(allocIdx) = allocCall and
181+
efIdx < allocIdx
182+
)
183+
}
184+
185+
/**
186+
* Holds if the alloc call has a NOLINT(allocations-without-ensure-free)
187+
* suppression comment on the same line or the line before.
188+
*/
189+
predicate hasSuppressionComment(FunctionCall allocCall) {
190+
exists(Comment c |
191+
c.getLocation().getFile() = allocCall.getLocation().getFile() and
192+
(
193+
c.getLocation().getStartLine() = allocCall.getLocation().getStartLine() or
194+
c.getLocation().getStartLine() = allocCall.getLocation().getStartLine() - 1
195+
) and
196+
c.getContents().matches("%NOLINT(allocations-without-ensure-free)%")
197+
)
198+
}
199+
200+
/**
201+
* Holds if `allocCall` is an allocating call (or direct memory_heap_alloc call)
202+
* inside an entry point that can be reached from the function entry without
203+
* passing through any ensure_free call on every path.
204+
*/
205+
predicate allocWithoutPrecedingEnsureFree(FunctionCall allocCall) {
206+
isEntryPoint(allocCall.getEnclosingFunction()) and
207+
(isAllocatingCall(allocCall) or allocCall.getTarget().hasName("memory_heap_alloc")) and
208+
not hasEnsureFreeBeforeInBlock(allocCall) and
209+
exists(Function f, BasicBlock allocBB |
210+
f = allocCall.getEnclosingFunction() and
211+
allocBB = allocCall.getBasicBlock() and
212+
(
213+
// allocBB is reachable from entry through non-barrier blocks
214+
reachableFromEntryAvoidingBarriers(allocBB, f)
215+
or
216+
// allocBB is itself a barrier (ensure_free is in the block but after the alloc)
217+
// and is reachable from entry through non-barrier blocks
218+
isBarrierBlock(allocBB) and
219+
(
220+
allocBB = f.getEntryPoint().getBasicBlock()
221+
or
222+
exists(BasicBlock pred |
223+
reachableFromEntryAvoidingBarriers(pred, f) and
224+
pred.getASuccessor() = allocBB
225+
)
226+
)
227+
)
228+
)
229+
}
230+
231+
from FunctionCall allocCall, Function entryPoint, string entryKind
232+
where
233+
entryPoint = allocCall.getEnclosingFunction() and
234+
(
235+
isNif(entryPoint) and entryKind = "NIF"
236+
or
237+
isPortHandler(entryPoint) and entryKind = "port handler"
238+
) and
239+
allocWithoutPrecedingEnsureFree(allocCall) and
240+
not hasSuppressionComment(allocCall)
241+
select allocCall,
242+
"Call to " + allocCall.getTarget().getName() +
243+
" allocates without any preceding ensure_free in " + entryKind + " $@.",
244+
entryPoint, entryPoint.getName()

src/libAtomVM/nifs.c

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2929,6 +2929,7 @@ static term nif_erlang_process_info(Context *ctx, int argc, term argv[])
29292929
term ret = term_invalid_term();
29302930
if (ctx == target) {
29312931
size_t term_size;
2932+
// NOLINT(allocations-without-ensure-free) called with NULL heap, only computes size
29322933
if (UNLIKELY(!context_get_process_info(ctx, NULL, &term_size, item, NULL))) {
29332934
globalcontext_get_process_unlock(ctx->global, target);
29342935
RAISE_ERROR(BADARG_ATOM);
@@ -2992,7 +2993,11 @@ static term nif_erlang_system_info(Context *ctx, int argc, term argv[])
29922993
return term_from_literal_binary((const uint8_t *) buf, len, &ctx->heap, ctx->global);
29932994
}
29942995
if (key == ATOMVM_VERSION_ATOM) {
2995-
return term_from_literal_binary((const uint8_t *) ATOMVM_VERSION, strlen(ATOMVM_VERSION), &ctx->heap, ctx->global);
2996+
size_t len = strlen(ATOMVM_VERSION);
2997+
if (memory_ensure_free_opt(ctx, term_binary_heap_size(len), MEMORY_CAN_SHRINK) != MEMORY_GC_OK) {
2998+
RAISE_ERROR(OUT_OF_MEMORY_ATOM);
2999+
}
3000+
return term_from_literal_binary((const uint8_t *) ATOMVM_VERSION, len, &ctx->heap, ctx->global);
29963001
}
29973002
if (key == SYSTEM_VERSION_ATOM) {
29983003
char system_version[256];
@@ -6232,6 +6237,7 @@ static term nif_lists_flatten(Context *ctx, int argc, term argv[])
62326237
break;
62336238
}
62346239

6240+
// NOLINT(allocations-without-ensure-free) ensure_free was called when result_len > tail_len, and this path is only reached in that case
62356241
term *new_list_item = term_list_alloc(&ctx->heap);
62366242
if (prev_term) {
62376243
prev_term[0] = term_list_from_list_ptr(new_list_item);

src/platforms/esp32/components/avm_sys/platform_nifs.c

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -710,6 +710,9 @@ static term nif_esp_get_default_mac(Context *ctx, int argc, term argv[])
710710
esp_err_t err = esp_efuse_mac_get_default(mac);
711711
if (err != ESP_OK) {
712712
ESP_LOGE(TAG, "Unable to read default mac. err=%i", err);
713+
if (UNLIKELY(memory_ensure_free(ctx, TUPLE_SIZE(2)) != MEMORY_GC_OK)) {
714+
RAISE_ERROR(OUT_OF_MEMORY_ATOM);
715+
}
713716
return port_create_error_tuple(ctx, esp_err_to_term(ctx->global, err));
714717
}
715718

@@ -832,6 +835,9 @@ static term nif_esp_task_wdt_deinit(Context *ctx, int argc, term argv[])
832835
return OK_ATOM;
833836
}
834837

838+
if (UNLIKELY(memory_ensure_free(ctx, TUPLE_SIZE(2)) != MEMORY_GC_OK)) {
839+
RAISE_ERROR(OUT_OF_MEMORY_ATOM);
840+
}
835841
term error_tuple = term_alloc_tuple(2, &ctx->heap);
836842
term_put_tuple_element(error_tuple, 0, ERROR_ATOM);
837843
term_put_tuple_element(error_tuple, 1, term_from_int(result));
@@ -927,7 +933,12 @@ static term nif_esp_timer_get_time(Context *ctx, int argc, term argv[])
927933
UNUSED(argv);
928934
UNUSED(argc);
929935

930-
return term_make_maybe_boxed_int64(esp_timer_get_time(), &ctx->heap);
936+
uint64_t val = esp_timer_get_time();
937+
size_t term_size = term_boxed_integer_size(val);
938+
if (UNLIKELY(memory_ensure_free_opt(ctx, term_size, MEMORY_CAN_SHRINK) != MEMORY_GC_OK)) {
939+
RAISE_ERROR(OUT_OF_MEMORY_ATOM);
940+
}
941+
return term_make_maybe_boxed_int64(val, &ctx->heap);
931942
}
932943

933944
//

0 commit comments

Comments
 (0)