Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Port bug fixes #3331

Merged
merged 4 commits into from
May 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
81 changes: 53 additions & 28 deletions runtime/sema/check_assignment.go
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,34 @@ func (checker *Checker) checkAssignment(
return
}

func (checker *Checker) rootOfAccessChain(target ast.Expression) (baseVariable *Variable, accessChain []Type) {
var inAccessChain = true

// seek the variable expression (if it exists) at the base of the access chain
for inAccessChain {
switch targetExp := target.(type) {
case *ast.IdentifierExpression:
baseVariable = checker.valueActivations.Find(targetExp.Identifier.Identifier)
if baseVariable != nil {
accessChain = append(accessChain, baseVariable.Type)
}
inAccessChain = false
case *ast.IndexExpression:
target = targetExp.TargetExpression
elementType := checker.Elaboration.IndexExpressionTypes(targetExp).IndexedType.ElementType(true)
accessChain = append(accessChain, elementType)
case *ast.MemberExpression:
target = targetExp.Expression
memberType, _, _, _ := checker.visitMember(targetExp, true)
accessChain = append(accessChain, memberType)
default:
inAccessChain = false
}
}

return
}

// We have to prevent any writes to references, since we cannot know where the value
// pointed to by the reference may have come from. Similarly, we can never safely assign
// to a resource; because resources are moved instead of copied, we cannot currently
Expand Down Expand Up @@ -162,31 +190,7 @@ func (checker *Checker) enforceViewAssignment(assignment ast.Statement, target a
return
}

var baseVariable *Variable
var accessChain = make([]Type, 0)
var inAccessChain = true

// seek the variable expression (if it exists) at the base of the access chain
for inAccessChain {
switch targetExp := target.(type) {
case *ast.IdentifierExpression:
baseVariable = checker.valueActivations.Find(targetExp.Identifier.Identifier)
if baseVariable != nil {
accessChain = append(accessChain, baseVariable.Type)
}
inAccessChain = false
case *ast.IndexExpression:
target = targetExp.TargetExpression
elementType := checker.Elaboration.IndexExpressionTypes(targetExp).IndexedType.ElementType(true)
accessChain = append(accessChain, elementType)
case *ast.MemberExpression:
target = targetExp.Expression
memberType, _, _, _ := checker.visitMember(targetExp, true)
accessChain = append(accessChain, memberType)
default:
inAccessChain = false
}
}
baseVariable, accessChain := checker.rootOfAccessChain(target)

// if the base of the access chain is not a variable, then we cannot make any static guarantees about
// whether or not it is a local struct-kinded variable. E.g. in the case of `(b ? s1 : s2).x`, we can't
Expand Down Expand Up @@ -312,19 +316,23 @@ func (checker *Checker) visitAssignmentValueType(
func (checker *Checker) visitIdentifierExpressionAssignment(
target *ast.IdentifierExpression,
) (targetType Type) {
identifier := target.Identifier.Identifier
identifier := target.Identifier

// check identifier was declared before
variable := checker.findAndCheckValueVariable(target, true)
if variable == nil {
return InvalidType
}

if variable.Type.IsResourceType() {
checker.checkResourceVariableCapturingInFunction(variable, identifier)
}

// check identifier is not a constant
if variable.IsConstant {
checker.report(
&AssignmentToConstantError{
Name: identifier,
Name: identifier.Identifier,
Range: ast.NewRangeFromPositioned(checker.memoryGauge, target),
},
)
Expand Down Expand Up @@ -483,7 +491,24 @@ func (checker *Checker) visitMemberExpressionAssignment(
reportAssignmentToConstant()
}

return memberType
if memberType.IsResourceType() {
// if the member is a resource, check that it is not captured in a function,
// based off the activation depth of the root of the access chain, i.e. `a` in `a.b.c`
// we only want to make this check for transactions, as they are the only "resource-like" types
// (that can contain resources and must destroy them in their `execute` blocks), that are themselves
// not checked by the capturing logic, since they are not themselves resources.
baseVariable, _ := checker.rootOfAccessChain(target)

if baseVariable == nil {
return
}

if _, isTransaction := baseVariable.Type.(*TransactionType); isTransaction {
checker.checkResourceVariableCapturingInFunction(baseVariable, member.Identifier)
}
}

return
}

func IsValidAssignmentTargetExpression(expression ast.Expression) bool {
Expand Down
25 changes: 17 additions & 8 deletions runtime/tests/checker/access_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1826,6 +1826,7 @@ func TestCheckAccessImportGlobalValueVariableDeclarationWithSecondValue(t *testi
`)
require.NoError(t, err)

// these capture x and y because they are created in a different file
_, err = ParseAndCheckWithOptions(t,
`
import x, y, createR from "imported"
Expand All @@ -1849,7 +1850,9 @@ func TestCheckAccessImportGlobalValueVariableDeclarationWithSecondValue(t *testi
},
)

errs := RequireCheckerErrors(t, err, 7)
errs := RequireCheckerErrors(t, err, 9)

// For `x`

require.IsType(t, &sema.InvalidAccessError{}, errs[0])
assert.Equal(t,
Expand All @@ -1859,23 +1862,29 @@ func TestCheckAccessImportGlobalValueVariableDeclarationWithSecondValue(t *testi

require.IsType(t, &sema.ResourceCapturingError{}, errs[1])

require.IsType(t, &sema.AssignmentToConstantError{}, errs[2])
require.IsType(t, &sema.ResourceCapturingError{}, errs[2])

require.IsType(t, &sema.AssignmentToConstantError{}, errs[3])
assert.Equal(t,
"x",
errs[2].(*sema.AssignmentToConstantError).Name,
errs[3].(*sema.AssignmentToConstantError).Name,
)

require.IsType(t, &sema.ResourceCapturingError{}, errs[3])

require.IsType(t, &sema.ResourceCapturingError{}, errs[4])

require.IsType(t, &sema.AssignmentToConstantError{}, errs[5])
// For `y`

require.IsType(t, &sema.ResourceCapturingError{}, errs[5])

require.IsType(t, &sema.ResourceCapturingError{}, errs[6])

require.IsType(t, &sema.AssignmentToConstantError{}, errs[7])
assert.Equal(t,
"y",
errs[5].(*sema.AssignmentToConstantError).Name,
errs[7].(*sema.AssignmentToConstantError).Name,
)

require.IsType(t, &sema.ResourceCapturingError{}, errs[6])
require.IsType(t, &sema.ResourceCapturingError{}, errs[8])
}

func TestCheckContractNestedDeclarationPrivateAccess(t *testing.T) {
Expand Down
141 changes: 141 additions & 0 deletions runtime/tests/checker/resources_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10055,3 +10055,144 @@ func TestCheckIndexingResourceLoss(t *testing.T) {
assert.IsType(t, &sema.InvalidNestedResourceMoveError{}, errs[1])
})
}

func TestCheckInvalidResourceCaptureOnLeft(t *testing.T) {

t.Parallel()

_, err := ParseAndCheck(t, `
fun test() {
var x: @AnyResource? <- nil
fun () {
x <-! []
}
destroy x
}
`)
errs := RequireCheckerErrors(t, err, 1)

assert.IsType(t, &sema.ResourceCapturingError{}, errs[0])
}

func TestCheckInvalidNestedResourceCapture(t *testing.T) {

t.Parallel()

t.Run("on right", func(t *testing.T) {
t.Parallel()

_, err := ParseAndCheck(t, `
transaction {
var x: @AnyResource?
prepare() {
self.x <- nil
}
execute {
fun() {
let y <- self.x
destroy y
}
}
}
`)
require.NoError(t, err)
})

t.Run("resource field on right", func(t *testing.T) {
t.Parallel()

_, err := ParseAndCheck(t, `
resource R {
var x: @AnyResource?
init() {
self.x <- nil
}
fun foo() {
fun() {
let y <- self.x <- nil
destroy y
}
}
}
`)
errs := RequireCheckerErrors(t, err, 1)

assert.IsType(t, &sema.ResourceCapturingError{}, errs[0])
})

t.Run("on left", func(t *testing.T) {
t.Parallel()

_, err := ParseAndCheck(t, `
transaction {
var x: @AnyResource?
prepare() {
self.x <- nil
}
execute {
fun() {
self.x <-! nil
}
destroy self.x
}
}
`)
errs := RequireCheckerErrors(t, err, 1)

assert.IsType(t, &sema.ResourceCapturingError{}, errs[0])
})

t.Run("on left method scope", func(t *testing.T) {
t.Parallel()

_, err := ParseAndCheck(t, `
transaction {
var x: @AnyResource?
prepare() {
self.x <- nil
}
execute {
self.x <-! nil
destroy self.x
}
}
`)
require.NoError(t, err)
})

t.Run("contract self variable on left", func(t *testing.T) {
t.Parallel()

_, err := ParseAndCheck(t, `
contract C {
var x: @AnyResource?
init() {
self.x <- nil
}
fun foo() {
fun() {
self.x <-! nil
}
}
}
`)
require.NoError(t, err)
})

t.Run("contract self variable on left method scope", func(t *testing.T) {
t.Parallel()

_, err := ParseAndCheck(t, `
contract C {
var x: @AnyResource?
init() {
self.x <- nil
}
fun foo() {
self.x <-! nil
}
}
`)
require.NoError(t, err)
})
}
31 changes: 31 additions & 0 deletions runtime/tests/checker/transactions_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -523,3 +523,34 @@ func TestCheckInvalidTransactionSelfMoveIntoDictionaryLiteral(t *testing.T) {

assert.IsType(t, &sema.InvalidMoveError{}, errs[0])
}

func TestCheckInvalidTransactionResourceLoss(t *testing.T) {

t.Parallel()

_, err := ParseAndCheck(t, `
access(all) resource R{}
transaction {
var r: @R?

prepare() {
self.r <- nil
}

execute {
let writeback = fun() {
self.r <-! create R()
}

var x <- self.r

destroy x
writeback()
}
}
`)

errs := RequireCheckerErrors(t, err, 1)

assert.IsType(t, &sema.ResourceCapturingError{}, errs[0])
}