Skip to content

Commit 76eac4b

Browse files
committed
Crunching false positives
1 parent cc8c412 commit 76eac4b

4 files changed

Lines changed: 118 additions & 1 deletion

File tree

internal/engine/cache.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,10 @@ import (
2828
// v12: C/C++ macro-body scan also captures value-position function pointers (.field = fn / = &fn inside #define), e.g. ops tables defined via a macro.
2929
// v13: C/C++ in-extractor macro expansion of file-scope invocations recovers token-pasted callbacks (CONFIGFS_ATTR/DEVICE_ATTR_RO -> name##_show).
3030
// v14: C/C++ static single-arg DEVICE_ATTR/BUS_ATTR expansion, all-ident scan of expanded macros (DEFINE_SHOW_ATTRIBUTE), capitalized C callee resolution.
31-
const cacheVersion = "v14"
31+
// v15: C/C++ salvage function-pointer refs from file-scope ERROR regions (macro-opened structs like MACHINE_START/DT_MACHINE_START ... MACHINE_END).
32+
// v16: extend that salvage to file-scope assignment_expression/field_expression fragments (machine_desc blocks parse that way when surrounded by other code).
33+
// v17: full-tree salvage of `.field = fn` macro-struct debris (machine_desc) regardless of where tree-sitter scatters it (skips function bodies).
34+
const cacheVersion = "v17"
3235

3336
// extractorCache holds per-extractor facts keyed by a content hash of the files
3437
// the extractor depends on. It is loaded from disk at the start of a snapshot and

internal/extractors/cppextractor/c_test.go

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -571,6 +571,40 @@ static int use(void) { return NE_PTR(3) + ARRAY_SIZE(buf); }
571571
}
572572
}
573573

574+
// TestCMachineDescErrorRegion covers the ARM machine_desc pattern: a struct opened
575+
// by a macro (DT_MACHINE_START) and closed by another (MACHINE_END), whose
576+
// `.field = fn` lines tree-sitter renders as a file-scope ERROR node. The callbacks
577+
// must be recovered.
578+
func TestCMachineDescErrorRegion(t *testing.T) {
579+
// Two blocks after a declaration + a call-valued `.smp` field — the shape that
580+
// makes tree-sitter recover the blocks as bare assignment_expression /
581+
// field_expression fragments rather than one clean ERROR node.
582+
ff := extractProject(t, map[string]string{
583+
"src/board.c": `
584+
static void omap_reserve(void) {}
585+
static void omap_generic_init(void) {}
586+
static void omap2xxx_restart(void) {}
587+
static void mvebu_dt_init(void) {}
588+
static const char *const compat[] = { "x", 0 };
589+
DT_MACHINE_START(OMAP242X_DT, "Generic OMAP2420")
590+
.smp = smp_ops(omap_smp_ops),
591+
.reserve = omap_reserve,
592+
.init_machine = omap_generic_init,
593+
.restart = omap2xxx_restart,
594+
MACHINE_END
595+
DT_MACHINE_START(OMAP243X_DT, "Generic OMAP2430")
596+
.init_machine = mvebu_dt_init,
597+
MACHINE_END
598+
`,
599+
})
600+
mod := mustFact(t, ff, "src")
601+
for _, fn := range []string{"src.omap_reserve", "src.omap_generic_init", "src.omap2xxx_restart", "src.mvebu_dt_init"} {
602+
if !hasRelation(mod, facts.RelCalls, fn) {
603+
t.Errorf("machine_desc callback %s should be referenced, got %+v", fn, mod.Relations)
604+
}
605+
}
606+
}
607+
574608
// TestCArgRefNonFunctionDropped guards the funcNames filter: an argument identifier
575609
// that does not name a real function must NOT produce a RelCalls edge (otherwise a
576610
// data argument could spuriously mark a same-named function as used).

internal/extractors/cppextractor/cpp_ast.go

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,9 @@ func extractFileAST(src []byte, relFile, lang string, macros macroTable) []facts
5555
macros: macros,
5656
}
5757
w.walkDeclList(tree.RootNode())
58+
// Recover callbacks from macro-opened struct initializers (machine_desc), which
59+
// tree-sitter can't parse and scatters as `.field = fn` assignment debris.
60+
w.salvageMacroStructAssigns(tree.RootNode())
5861
return w.out
5962
}
6063

@@ -975,6 +978,43 @@ func (w *astWalker) expandMacroDeclaration(typ, declarator *sitter.Node) bool {
975978
return true
976979
}
977980

981+
// salvageMacroStructAssigns recovers function-pointer references from a
982+
// macro-opened struct initializer — `DT_MACHINE_START(...) .init_machine = fn, ...
983+
// MACHINE_END`. The hidden opening brace makes tree-sitter fail to parse the block;
984+
// depending on the surrounding code it renders the `.field = fn` lines as a chain
985+
// of `assignment_expression`s with a `field_expression` left side, scattered under
986+
// an ERROR node, a bare top-level expression, or a neighbouring declaration. Rather
987+
// than chase every recovery shape, walk the whole tree (skipping function bodies,
988+
// whose assignments walkForCalls already handles) and capture the RHS of every
989+
// `<field> = fn` assignment on the module owner. resolveFuncPtrRefs keeps only the
990+
// real functions, so this is generic (no hard-coded macro names) and safe.
991+
func (w *astWalker) salvageMacroStructAssigns(root *sitter.Node) {
992+
var assigns []*sitter.Node
993+
var walk func(n *sitter.Node)
994+
walk = func(n *sitter.Node) {
995+
if n == nil || n.Kind() == "function_definition" {
996+
return
997+
}
998+
if n.Kind() == "assignment_expression" {
999+
if l := n.ChildByFieldName("left"); l != nil && l.Kind() == "field_expression" {
1000+
assigns = append(assigns, n)
1001+
}
1002+
}
1003+
for i := uint(0); i < n.ChildCount(); i++ {
1004+
walk(n.Child(i))
1005+
}
1006+
}
1007+
walk(root)
1008+
if len(assigns) == 0 {
1009+
return
1010+
}
1011+
w.pushOwner(w.moduleOwner())
1012+
for _, a := range assigns {
1013+
w.emitAssignFuncPtrRef(a)
1014+
}
1015+
w.popOwner()
1016+
}
1017+
9781018
// handleMacroBodyCalls scans a #define replacement list (preproc_def /
9791019
// preproc_function_def) for identifiers used as functions — in call position
9801020
// (`IDENT(`) or value position (`= IDENT` / `.field = IDENT` / `= &IDENT`) — and
Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
package cppextractor
2+
3+
import (
4+
"fmt"
5+
"os"
6+
"strings"
7+
"testing"
8+
9+
sitter "github.com/tree-sitter/go-tree-sitter"
10+
c "github.com/tree-sitter/tree-sitter-c/bindings/go"
11+
)
12+
13+
func TestProbeDa8xxTop(t *testing.T) {
14+
for _, f := range []string{
15+
"/Users/dejan/development/cpp/linux/arch/arm/mach-davinci/da8xx-dt.c",
16+
"/Users/dejan/development/cpp/linux/arch/arm/mach-omap2/board-generic.c",
17+
} {
18+
src, err := os.ReadFile(f)
19+
if err != nil {
20+
t.Skip("no tree")
21+
}
22+
parser := sitter.NewParser()
23+
_ = parser.SetLanguage(sitter.NewLanguage(c.Language()))
24+
tree := parser.Parse(src, nil)
25+
root := tree.RootNode()
26+
fmt.Printf("=== %s ===\n", f)
27+
for i := uint(0); i < root.ChildCount(); i++ {
28+
ch := root.Child(i)
29+
line := strings.SplitN(string(src[ch.StartByte():ch.EndByte()]), "\n", 2)[0]
30+
if len(line) > 50 {
31+
line = line[:50]
32+
}
33+
if strings.Contains(string(src[ch.StartByte():ch.EndByte()]), "MACHINE_START") || ch.Kind() == "ERROR" {
34+
fmt.Printf(" [%d] %s | %q\n", ch.StartPosition().Row+1, ch.Kind(), line)
35+
}
36+
}
37+
parser.Close()
38+
tree.Close()
39+
}
40+
}

0 commit comments

Comments
 (0)