Skip to content

Commit 19d9087

Browse files
committed
update
1 parent dabd433 commit 19d9087

30 files changed

Lines changed: 275 additions & 62 deletions

codeql-custom-queries-cpp/lib/guard_checker.qll

Lines changed: 26 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,29 @@ predicate isGcTrigger(Function function) {
3333
)
3434
}
3535

36+
/**
37+
* Functions that call known allocation/GC-triggering routines (e.g., rb_str_new, rb_ary_push).
38+
* This widens GC trigger detection to cover common Ruby API allocators beyond gc_enter.
39+
*/
40+
predicate isAllocOrGcFunction(Function function) {
41+
exists(FunctionCall call |
42+
call.getEnclosingFunction() = function and
43+
isAllocOrGcCall(call)
44+
)
45+
}
46+
47+
predicate isAllocOrGcCall(FunctionCall call) {
48+
call.getTarget().getName() in [
49+
"rb_str_new", "rb_str_buf_new", "rb_str_resize", "rb_str_concat", "rb_str_append",
50+
"rb_ary_new", "rb_ary_push", "rb_ary_concat", "rb_ary_store",
51+
"rb_hash_new", "rb_hash_aset", "rb_hash_lookup2",
52+
"rb_obj_alloc", "rb_class_new_instance", "rb_funcall",
53+
"ALLOC", "ALLOC_N", "REALLOC_N"
54+
]
55+
or
56+
call.getTarget() instanceof GcTriggerFunction
57+
}
58+
3659
predicate isGcTrigger1(Function function) {
3760
exists(Expr s, Call call |
3861
s.getEnclosingFunction() = function and
@@ -94,7 +117,7 @@ predicate isGcTriggerWithFunctionPointer(Function function) {
94117
}
95118

96119
class GcTriggerFunction extends Function {
97-
GcTriggerFunction() { isGcTrigger(this) }
120+
GcTriggerFunction() { isGcTrigger(this) or isAllocOrGcFunction(this) }
98121
}
99122

100123
class GcTriggerFunctionWithFunctionPointer extends Function {
@@ -109,45 +132,6 @@ class GcTriggerCall extends FunctionCall {
109132
}
110133
}
111134

112-
class GcTriggerCallWithFunctionPointer extends Expr {
113-
GcTriggerCallWithFunctionPointer() {
114-
if this instanceof FunctionCall
115-
then this.(FunctionCall).getTarget() instanceof GcTriggerFunctionWithFunctionPointer
116-
else (
117-
if this instanceof VariableAccess
118-
then this.(VariableAccess).getTarget().getType() instanceof FunctionPointerType
119-
else none()
120-
)
121-
}
122-
}
123-
124-
predicate isArgumentNotSafe(GcTriggerFunction gcTriggerFunc, int i, ValueVariable v) {
125-
exists(GcTriggerCall innerGcTriggerCall, VariableAccess pAccess |
126-
gcTriggerFunc.getAPredecessor+() = innerGcTriggerCall and
127-
pAccess = innerGcTriggerCall.getASuccessor+() and
128-
gcTriggerFunc.getParameter(i).getAnAccess() = pAccess
129-
)
130-
or
131-
exists(GcTriggerCall recursiveGcTriggerCall, int j |
132-
recursiveGcTriggerCall = gcTriggerFunc.getAPredecessor+() and
133-
gcTriggerFunc.getParameter(i).getAnAccess() = recursiveGcTriggerCall.getAnArgumentSubExpr(j) and
134-
isArgumentNotSafe(recursiveGcTriggerCall.getTarget(), j, v)
135-
)
136-
}
137-
138-
139-
/*
140-
predicate pointerPassedNotSafe(GcTriggerCall gtc, ControlFlowNode pAccess, ValueVariable v) {
141-
gtc.getAnArgument() = pAccess and (not gtc.getAnArgument().(ValueAccess).getTarget() = v)
142-
and
143-
exists(GcTriggerCall innerGcTriggerCall, VariableAccess pAccess |
144-
gcTriggerFunc.getAPredecessor+() = innerGcTriggerCall and
145-
pAccess = innerGcTriggerCall.getASuccessor+() and
146-
gcTriggerFunc.getParameter(i).getAnAccess() = pAccess
147-
)
148-
}
149-
*/
150-
151135
predicate needsGuard(
152136
ValueVariable v, PointerVariable innerPointer, GcTriggerCall gtc,
153137
ControlFlowNode pointerAccess, ControlFlowNode innerPointerTaking
@@ -164,6 +148,8 @@ predicate needsGuard(
164148
(
165149
isPointerUsedAfterGcTrigger(pointerAccess, gtc)
166150
or
151+
pointerPassedToGcAlloc(gtc, pointerAccess)
152+
or
167153
exists(GcTriggerCall gtcInter |
168154
gtcInter.getAnArgument() = pointerAccess or gtc.getAnArgument() = innerPointerTaking
169155
)
@@ -214,13 +200,6 @@ string getGuardInsertionLineBR(ValueVariable v) {
214200
else result = v.getParentScope().getLocation().getEndLine().toString()
215201
}
216202

217-
string getGuardInsertionLineBRLA(ValueVariable v) {
218-
exists(ValueAccess lva |
219-
not exists(ValueAccess va | va = lva.getASuccessor+()) and
220-
result = lva.getLocation().getEndLine().toString()
221-
)
222-
}
223-
224203
predicate isTarget(ValueVariable v) {
225204
v.getEnclosingElement() instanceof TopLevelFunction and
226205
// v.getIniti and

codeql-custom-queries-cpp/lib/patterns.qll

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,30 @@ predicate isPointerUsedAfterGcTrigger(
114114
gcTriggerCall.getASuccessor+() = pointerUsageAccess
115115
}
116116

117+
/**
118+
* Interprocedural: pointer argument is passed to a callee that performs an
119+
* allocation/GC-triggering call using that parameter.
120+
*/
121+
predicate pointerPassedToGcAlloc(FunctionCall call, PointerVariableAccess pAccess) {
122+
exists(int i |
123+
call.getAnArgumentSubExpr(i) = pAccess and
124+
calleeParameterUsedInAlloc(call.getTarget(), i)
125+
)
126+
}
127+
128+
predicate calleeParameterUsedInAlloc(Function callee, int idx) {
129+
exists(FunctionCall innerCall, VariableAccess paramUse |
130+
innerCall.getEnclosingFunction() = callee and
131+
isAllocOrGcCall(innerCall) and
132+
(
133+
innerCall.getAnArgument() = paramUse and
134+
paramUse.getTarget() = callee.getParameter(idx)
135+
or
136+
innerCall.getAnArgument().getAChild*() = callee.getParameter(idx).getAnAccess()
137+
)
138+
)
139+
}
140+
117141

118142
/*
119143
predicate passedToGcTrigger(ValueVariable v, ValueAccess initVAccess, FunctionCall gcTriggerCall) {
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
import cpp
2+
import lib.guard_checker
3+
4+
from ValueVariable v
5+
where
6+
not exists(
7+
PointerVariable p, GcTriggerCall gtc, PointerVariableAccess pointerUsageAccess,
8+
PointerDerivationAction innerPointerTaking
9+
|
10+
needsGuard(v, p, gtc, pointerUsageAccess, innerPointerTaking)
11+
) and
12+
hasGuard(v)
13+
select v
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
Quick C fixtures mirroring real RB_GC_GUARD patterns from CRuby (weakmap, prism, yjit) plus a missing-guard case and a redundant-guard case.
2+
3+
## Layout
4+
- `ruby_stubs.h` – minimal VALUE/RB_GC_GUARD/GC-trigger/pointer-extractor shims to keep the fixtures self contained.
5+
- `weakmap_like.c` – pointer extracted via `RSTRING_PTR`, GC trigger happens, guard placed after last pointer use (expected **good guard**).
6+
- `prism_like.c` – loop over locals, inner pointer via `rb_id2name`, guarded (expected **good guard**).
7+
- `yjit_like.c` – multiple VALUEs guarded after GC-triggering hash updates (expected **good guards**).
8+
- `missing_guard.c` – pointer extracted then GC trigger but no guard (expected **missing_guard** finding).
9+
- `redundant_guard.c` – guard without any pointer extraction/GC trigger (expected **redundant_guard** finding).
10+
- `interproc_missing_guard.c` – derived pointer passed to another function that triggers GC and uses the pointer (expected **missing_guard** if interprocedural pointer use is modeled).
11+
- `array_guard.c``RARRAY_PTR` inner pointer used after `rb_ary_push`, guarded (expected **good guard**).
12+
- `data_ptr_guard.c``DATA_PTR` inner pointer used after `rb_str_new`, guarded (expected **good guard**).
13+
- `inline_missing_guard.c` – inline `RSTRING_PTR` argument to `rb_str_new` reused later without guard (expected **missing_guard** via inline pattern).
14+
- `inline_guarded.c` – inline pointer extraction with guard present (expected **good guard**).
15+
- `redundant_inline_guard.c` – pointer extracted but no GC trigger; guard present (expected **redundant_guard**).
16+
- `redundant_param_guard.c` – guard on parameter without pointer extraction or GC trigger (expected **redundant_guard**).
17+
18+
## Quick run with CodeQL
19+
```bash
20+
# From repo root
21+
cd codeql-custom-queries-cpp/test-cases
22+
23+
# Build a small database (requires clang or cc)
24+
codeql database create ../test-db --language=cpp --source-root . \
25+
--command="cc -c *.c"
26+
27+
# Run queries
28+
codeql query run ../missing_guards.ql --database ../test-db --output ../missing_guards.bqrs
29+
codeql bqrs decode --format=csv --output ../missing_guards.csv ../missing_guards.bqrs
30+
31+
codeql query run ../redundant_guards.ql --database ../test-db --output ../redundant_guards.bqrs
32+
codeql bqrs decode --format=csv --output ../redundant_guards.csv ../redundant_guards.bqrs
33+
```
34+
35+
You can swap in other queries (e.g., `good_guards.ql`, `all_guarded_variables.ql`) against the same database.
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
#include "ruby_stubs.h"
2+
3+
/* Array inner pointer survives push; guard the VALUE holding it */
4+
VALUE array_guard_example(VALUE ary) {
5+
VALUE *ptr = (VALUE *)RARRAY_PTR(ary); /* inner pointer */
6+
rb_ary_push(ary, LONG2NUM(42)); /* GC trigger */
7+
VALUE first = ptr[0]; /* pointer reused */
8+
RB_GC_GUARD(ary); /* guard after pointer use */
9+
return first;
10+
}
1.48 KB
Binary file not shown.
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
#include "ruby_stubs.h"
2+
3+
struct foo { int x; };
4+
5+
/* DATA_PTR-derived pointer used after GC trigger; guard the VALUE */
6+
VALUE data_ptr_guard_example(VALUE obj) {
7+
struct foo *p = (struct foo *)DATA_PTR(obj); /* inner pointer */
8+
VALUE res = rb_str_new("foo", 3); /* GC trigger */
9+
p->x = 7; /* pointer reused */
10+
RB_GC_GUARD(obj); /* guard after use */
11+
return res;
12+
}
1.68 KB
Binary file not shown.
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
#include "ruby_stubs.h"
2+
3+
/* Inline pointer extraction reused with guard present */
4+
VALUE inline_guarded(VALUE str) {
5+
VALUE part = rb_str_new(RSTRING_PTR(str), 3); /* GC trigger with inline ptr */
6+
VALUE later = rb_str_new(RSTRING_PTR(str), 2); /* reuse inline ptr pattern */
7+
VALUE out = rb_str_concat(part, later);
8+
RB_GC_GUARD(str); /* guard original VALUE */
9+
return out;
10+
}
1.63 KB
Binary file not shown.

0 commit comments

Comments
 (0)