Skip to content

Commit 3d85870

Browse files
chore: solidify len/cap impurity check for maps/chans and add tests
- Verify and solidify the impurity check for `len()` and `cap()` on maps and channels in `canonicalizer.go`. - Replace "FIX" marker with "Note" to reflect that the logic is implemented and intentional. - Add regression test `TestCanonicalizer_HoistSafety_Chan` in `ir_test.go` to ensure `len(chan)` and `cap(chan)` are not hoisted out of loops. Co-authored-by: xkilldash9x <223238109+xkilldash9x@users.noreply.github.com>
1 parent 11fdbb8 commit 3d85870

2 files changed

Lines changed: 62 additions & 1 deletion

File tree

pkg/analysis/ir/canonicalizer.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -462,7 +462,7 @@ func (c *Canonicalizer) isPureBuiltin(call *ssa.Call) bool {
462462
return false
463463
}
464464

465-
// Security Fix: len() and cap() are impure for Maps and Channels
465+
// Note: len() and cap() are impure for Maps and Channels
466466
// because their length is volatile in concurrent/mutable contexts.
467467
if name == "len" || name == "cap" {
468468
if len(call.Call.Args) > 0 {

pkg/analysis/ir/ir_test.go

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -564,3 +564,64 @@ func TestCanonicalizer_BlockOrderingStability(t *testing.T) {
564564
}
565565
}
566566
}
567+
568+
func TestCanonicalizer_HoistSafety_Chan(t *testing.T) {
569+
t.Parallel()
570+
571+
// Test len(chan) and cap(chan) are not hoisted.
572+
src := `package main
573+
func foo(c chan int) int {
574+
sum := 0
575+
for i := 0; i < 10; i++ {
576+
c <- i
577+
sum += len(c) // This must NOT be hoisted
578+
sum += cap(c) // This must NOT be hoisted
579+
}
580+
return sum
581+
}`
582+
583+
fn := testutil.CompileAndGetFunction(t, src, "foo")
584+
585+
c := ir.NewCanonicalizer(ir.DefaultLiteralPolicy)
586+
defer ir.ReleaseCanonicalizer(c)
587+
588+
out := c.CanonicalizeFunction(fn)
589+
590+
lines := strings.Split(out, "\n")
591+
var loopHeaderIdx int
592+
var lenCallIdx int = -1
593+
var capCallIdx int = -1
594+
loopFound := false
595+
596+
for i, line := range lines {
597+
if strings.Contains(line, "; LoopHeader") {
598+
loopHeaderIdx = i
599+
loopFound = true
600+
}
601+
if strings.Contains(line, "Call <builtin:len>") {
602+
lenCallIdx = i
603+
}
604+
if strings.Contains(line, "Call <builtin:cap>") {
605+
capCallIdx = i
606+
}
607+
}
608+
609+
if !loopFound {
610+
t.Fatal("Failed to detect LoopHeader in canonical output")
611+
}
612+
613+
if lenCallIdx == -1 {
614+
t.Fatal("Failed to find len() call in canonical output")
615+
}
616+
if capCallIdx == -1 {
617+
t.Fatal("Failed to find cap() call in canonical output")
618+
}
619+
620+
// If len() or cap() was hoisted, it would appear in the pre-header (before loop header).
621+
if lenCallIdx < loopHeaderIdx {
622+
t.Errorf("Security Flaw: len(chan) was incorrectly hoisted out of the loop.")
623+
}
624+
if capCallIdx < loopHeaderIdx {
625+
t.Errorf("Security Flaw: cap(chan) was incorrectly hoisted out of the loop.")
626+
}
627+
}

0 commit comments

Comments
 (0)