Skip to content

Commit

Permalink
fix: branch stmts (gnolang#3043)
Browse files Browse the repository at this point in the history
Adds validation for `break`, `continue` and `fallthrough` to disallow
invalid code.
Moves existing validation from the runtime to the preprocessor.

Closes gnolang#2973
  • Loading branch information
petar-dambovaliev authored and r3v4s committed Dec 10, 2024
1 parent 90ee48b commit 18d94f9
Show file tree
Hide file tree
Showing 13 changed files with 151 additions and 58 deletions.
21 changes: 11 additions & 10 deletions gnovm/pkg/gnolang/nodes.go
Original file line number Diff line number Diff line change
Expand Up @@ -149,16 +149,17 @@ func (loc Location) IsZero() bool {
type GnoAttribute string

const (
ATTR_PREPROCESSED GnoAttribute = "ATTR_PREPROCESSED"
ATTR_PREDEFINED GnoAttribute = "ATTR_PREDEFINED"
ATTR_TYPE_VALUE GnoAttribute = "ATTR_TYPE_VALUE"
ATTR_TYPEOF_VALUE GnoAttribute = "ATTR_TYPEOF_VALUE"
ATTR_IOTA GnoAttribute = "ATTR_IOTA"
ATTR_LOCATIONED GnoAttribute = "ATTR_LOCATIONE" // XXX DELETE
ATTR_GOTOLOOP_STMT GnoAttribute = "ATTR_GOTOLOOP_STMT" // XXX delete?
ATTR_LOOP_DEFINES GnoAttribute = "ATTR_LOOP_DEFINES" // []Name defined within loops.
ATTR_LOOP_USES GnoAttribute = "ATTR_LOOP_USES" // []Name loop defines actually used.
ATTR_SHIFT_RHS GnoAttribute = "ATTR_SHIFT_RHS"
ATTR_PREPROCESSED GnoAttribute = "ATTR_PREPROCESSED"
ATTR_PREDEFINED GnoAttribute = "ATTR_PREDEFINED"
ATTR_TYPE_VALUE GnoAttribute = "ATTR_TYPE_VALUE"
ATTR_TYPEOF_VALUE GnoAttribute = "ATTR_TYPEOF_VALUE"
ATTR_IOTA GnoAttribute = "ATTR_IOTA"
ATTR_LOCATIONED GnoAttribute = "ATTR_LOCATIONE" // XXX DELETE
ATTR_GOTOLOOP_STMT GnoAttribute = "ATTR_GOTOLOOP_STMT" // XXX delete?
ATTR_LOOP_DEFINES GnoAttribute = "ATTR_LOOP_DEFINES" // []Name defined within loops.
ATTR_LOOP_USES GnoAttribute = "ATTR_LOOP_USES" // []Name loop defines actually used.
ATTR_SHIFT_RHS GnoAttribute = "ATTR_SHIFT_RHS"
ATTR_LAST_BLOCK_STMT GnoAttribute = "ATTR_LAST_BLOCK_STMT"
)

type Attributes struct {
Expand Down
16 changes: 3 additions & 13 deletions gnovm/pkg/gnolang/op_exec.go
Original file line number Diff line number Diff line change
Expand Up @@ -676,25 +676,15 @@ EXEC_SWITCH:
bs.Active = bs.Body[cs.BodyIndex] // prefill
case FALLTHROUGH:
ss, ok := m.LastFrame().Source.(*SwitchStmt)
// this is handled in the preprocessor
// should never happen
if !ok {
// fallthrough is only allowed in a switch statement
panic("fallthrough statement out of place")
}
if ss.IsTypeSwitch {
// fallthrough is not allowed in type switches
panic("cannot fallthrough in type switch")
}

b := m.LastBlock()
if b.bodyStmt.NextBodyIndex != len(b.bodyStmt.Body) {
// fallthrough is not the final statement
panic("fallthrough statement out of place")
}
// compute next switch clause from BodyIndex (assigned in preprocess)
nextClause := cs.BodyIndex + 1
if nextClause >= len(ss.Clauses) {
// no more clause after the one executed, this is not allowed
panic("cannot fallthrough final case in switch")
}
// expand block size
cl := ss.Clauses[nextClause]
if nn := cl.GetNumNames(); int(nn) > len(b.Values) {
Expand Down
90 changes: 61 additions & 29 deletions gnovm/pkg/gnolang/preprocess.go
Original file line number Diff line number Diff line change
Expand Up @@ -298,6 +298,11 @@ func initStaticBlocks(store Store, ctx BlockNode, bn BlockNode) {
last.Predefine(false, n.VarName)
}
case *SwitchClauseStmt:
blen := len(n.Body)
if blen > 0 {
n.Body[blen-1].SetAttribute(ATTR_LAST_BLOCK_STMT, true)
}

// parent switch statement.
ss := ns[len(ns)-1].(*SwitchStmt)
// anything declared in ss are copied,
Expand Down Expand Up @@ -2137,9 +2142,7 @@ func preprocess1(store Store, ctx BlockNode, n Node) Node {
switch n.Op {
case BREAK:
if n.Label == "" {
if !findBreakableNode(ns) {
panic("cannot break with no parent loop or switch")
}
findBreakableNode(last, store)
} else {
// Make sure that the label exists, either for a switch or a
// BranchStmt.
Expand All @@ -2149,9 +2152,7 @@ func preprocess1(store Store, ctx BlockNode, n Node) Node {
}
case CONTINUE:
if n.Label == "" {
if !findContinuableNode(ns) {
panic("cannot continue with no parent loop")
}
findContinuableNode(last, store)
} else {
if isSwitchLabel(ns, n.Label) {
panic(fmt.Sprintf("invalid continue label %q\n", n.Label))
Expand All @@ -2163,17 +2164,36 @@ func preprocess1(store Store, ctx BlockNode, n Node) Node {
n.Depth = depth
n.BodyIndex = index
case FALLTHROUGH:
if swchC, ok := last.(*SwitchClauseStmt); ok {
// last is a switch clause, find its index in the switch and assign
// it to the fallthrough node BodyIndex. This will be used at
// runtime to determine the next switch clause to run.
swch := lastSwitch(ns)
for i := range swch.Clauses {
if &swch.Clauses[i] == swchC {
// switch clause found
n.BodyIndex = i
break
}
swchC, ok := last.(*SwitchClauseStmt)
if !ok {
// fallthrough is only allowed in a switch statement
panic("fallthrough statement out of place")
}

if n.GetAttribute(ATTR_LAST_BLOCK_STMT) != true {
// no more clause after the one executed, this is not allowed
panic("fallthrough statement out of place")
}

// last is a switch clause, find its index in the switch and assign
// it to the fallthrough node BodyIndex. This will be used at
// runtime to determine the next switch clause to run.
swch := lastSwitch(ns)

if swch.IsTypeSwitch {
// fallthrough is not allowed in type switches
panic("cannot fallthrough in type switch")
}

for i := range swch.Clauses {
if i == len(swch.Clauses)-1 {
panic("cannot fallthrough final case in switch")
}

if &swch.Clauses[i] == swchC {
// switch clause found
n.BodyIndex = i
break
}
}
default:
Expand Down Expand Up @@ -3272,24 +3292,36 @@ func funcOf(last BlockNode) (BlockNode, *FuncTypeExpr) {
}
}

func findBreakableNode(ns []Node) bool {
for _, n := range ns {
switch n.(type) {
case *ForStmt, *RangeStmt, *SwitchClauseStmt:
return true
func findBreakableNode(last BlockNode, store Store) {
for last != nil {
switch last.(type) {
case *FuncLitExpr, *FuncDecl:
panic("break statement out of place")
case *ForStmt:
return
case *RangeStmt:
return
case *SwitchClauseStmt:
return
}

last = last.GetParentNode(store)
}
return false
}

func findContinuableNode(ns []Node) bool {
for _, n := range ns {
switch n.(type) {
case *ForStmt, *RangeStmt:
return true
func findContinuableNode(last BlockNode, store Store) {
for last != nil {
switch last.(type) {
case *FuncLitExpr, *FuncDecl:
panic("continue statement out of place")
case *ForStmt:
return
case *RangeStmt:
return
}

last = last.GetParentNode(store)
}
return false
}

func findBranchLabel(last BlockNode, label Name) (
Expand Down
2 changes: 1 addition & 1 deletion gnovm/tests/files/break0.gno
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,4 @@ func main() {
}

// Error:
// main/files/break0.gno:4:2: cannot break with no parent loop or switch
// main/files/break0.gno:4:2: break statement out of place
2 changes: 1 addition & 1 deletion gnovm/tests/files/cont3.gno
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,4 @@ func main() {
}

// Error:
// main/files/cont3.gno:4:2: cannot continue with no parent loop
// main/files/cont3.gno:4:2: continue statement out of place
17 changes: 17 additions & 0 deletions gnovm/tests/files/for21.gno
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
package main

func main() {
for i := 0; i < 10; i++ {
if i == 1 {
_ = func() int {
continue
return 11
}()
}
println(i)
}
println("wat???")
}

// Error:
// main/files/for21.gno:7:17: continue statement out of place
17 changes: 17 additions & 0 deletions gnovm/tests/files/for22.gno
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
package main

func main() {
for i := 0; i < 10; i++ {
if i == 1 {
_ = func() int {
fallthrough
return 11
}()
}
println(i)
}
println("wat???")
}

// Error:
// main/files/for22.gno:7:17: fallthrough statement out of place
17 changes: 17 additions & 0 deletions gnovm/tests/files/for23.gno
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
package main

func main() {
for i := 0; i < 10; i++ {
if i == 1 {
_ = func() int {
break
return 11
}()
}
println(i)
}
println("wat???")
}

// Error:
// main/files/for23.gno:7:17: break statement out of place
19 changes: 19 additions & 0 deletions gnovm/tests/files/for24.gno
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
package main

func main() {
for i := 0; i < 10; i++ {
if i == 1 {
_ = func() int {
if true {
break
}
return 11
}()
}
println(i)
}
println("wat???")
}

// Error:
// main/files/for24.gno:8:21: break statement out of place
2 changes: 1 addition & 1 deletion gnovm/tests/files/switch8.gno
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,4 @@ func main() {
}

// Error:
// fallthrough statement out of place
// main/files/switch8.gno:5:2: fallthrough statement out of place
2 changes: 1 addition & 1 deletion gnovm/tests/files/switch8b.gno
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,4 @@ func main() {
}

// Error:
// cannot fallthrough final case in switch
// main/files/switch8b.gno:10:3: cannot fallthrough final case in switch
2 changes: 1 addition & 1 deletion gnovm/tests/files/switch8c.gno
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,4 @@ func main() {
}

// Error:
// fallthrough statement out of place
// main/files/switch8c.gno:7:3: fallthrough statement out of place
2 changes: 1 addition & 1 deletion gnovm/tests/files/switch9.gno
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,4 @@ func main() {
}

// Error:
// cannot fallthrough in type switch
// main/files/switch9.gno:9:3: cannot fallthrough in type switch

0 comments on commit 18d94f9

Please sign in to comment.