Pop/push slice returns old element

[n0izn0iz]

Pop/push slice returns old element

Description

If you pop an element from a slice (via slice = slice[:len(slice)-1]) and append a new element afterwards, the last element of the slice will be the popped element and not the new element

(Sorry for the confusing issue name I can't find a better short name)

Your environment

• macOS 13.3.1
• go version go1.20.4 darwin/arm64
• gno: master
• commit: d684985f9d146c4b6495508adef57a1ea60ba13a

Steps to reproduce

• Deploy this realm

  
  package bugmin

import ( "strconv" "strings" )

var slice = []string{"undead-element"}

func Pop() { slice = slice[:len(slice)-1] }

func Push() { slice = append(slice, "new-element") }

func Render(path string) string { str := strings.Join(slice, ", ") + "\n\n" str += "len: " + strconv.Itoa(len(slice)) + "\n\n" return str }

- Call `Pop` then `Push`

### Expected behaviour

The slice is `[]string{"new-element"}` after `Pop` and `Push`

### Actual behaviour

The slice is `[]string{"undead-element"}` after `Pop` and `Push`

### Additional info

If you give me some pointer (namely where the slice access operator `[:]` and `append` are handled), I could try debug this

You can work around this by creating a new slice:
```go
func Pop() {
    // slice = slice[:len(slice)-1] // this is bugged

    newSlice := []string{}
    for i := 0; i < len(slice)-1; i++ {
        newSlice = append(newSlice, slice[i])
    }
    slice = newSlice // this works
}

The bug seem to be at storage layer level since this works as expected:

package bugmin

import (
    "strconv"
    "strings"
)

func Render(path string) string {
    slice := []string{"undead-element"}
    slice = slice[:len(slice)-1]
    slice = append(slice, "new-element")

    str := "slice: " + strings.Join(slice, ", ") + "\n\n"
    str += "len(slice): " + strconv.Itoa(len(slice)) + "\n\n"
    return str
}

[moul]

Hey, could you try opening this bug as a new challenge in this folder, please? https://github.com/gnolang/gno/tree/master/gnovm/tests/challenges

[n0izn0iz]

I tried to add it in challenges like so:

package main

var slice = []string{"undead-element"}

func main() {
    slice = slice[:len(slice)-1]
    slice = append(slice, "new-element")

    println(slice)
}

// Output:
// slice[("new-element" string)]

but this does not fail, I think it needs to happen in two separate transactions, I could add a realm in examples/gno.land if you want

[peter7891]

@n0izn0iz

Creating a slice The slice elements are pushed for evaluation here func (m *Machine) doOpCompositeLit()

Then, each of the elements will pass through func (m *Machine) doOpEval()

Then, everything gets assembled here func (m *Machine) doOpArrayLit()

Reading from a slice For the accessing of elements it first goes through func (m *Machine) doOpEval() in the case *SliceExpr:

and gets the value here func (m *Machine) doOpSlice()

[n0izn0iz]

I found something interesting

At this line: https://github.com/gnolang/gno/blob/6d137df03c59a0d78fe0f1336bd3a60d6e4209d9/gnovm/pkg/gnolang/realm.go#L1111 If I set Maxcap: cv.Length instead of Maxcap: cv.Maxcap, it solves my minimal test, this probably breaks other stuff though

Maxcap seems to not be properly handled somewhere, my guess is serialization/de-serialization since the pop/push works when done in a single TX

I'll keep digging ⛏️

[n0izn0iz]

After digging more, I feel that assigning to a slice does not properly "Marks dirty" the slice or it's base array

copyValueWithRefs is never called for the new-element string (it is called for the undead-element string) SetObject is never called with array[("new-element" string)] (it is called for array[("undead-element" string)]) realm.updated does not contains the new slice at the end of the Push tx

[n0izn0iz]

[EDIT]: ignore this post, since the underlying array is the same, this seems normal in the end

Something here does not seem right

https://github.com/gnolang/gno/blob/f50f33de653bd660fe1a2ceae430fd7885e97e9c/gnovm/pkg/gnolang/values.go#L279-L282

oo1 address is the same as oo2 when assigning the slice during Push but pv.TV is (slice[] []string) and tv2 is (slice[("new-element" string)] []string) I'm guessing oo1 and oo2 should be two different objects

seems like it's assigning the new slice to the new slice

not sure what the intended behavior is

(sorry for the raw information and thoughts, I barely understand the code for now)

[n0izn0iz]

Could someone explain

• what is an escaped object?
• what is a real object?
• what is a dirty object? (I think I understand this one but asking just to be sure)

[peter7891]

Could someone explain

• what is an escaped object?
• what is a real object?
• what is a dirty object? (I think I understand this one but asking just to be sure)

These are part of the ownership system. You can see where its invoked here.

[peter7891]

@n0izn0iz i haven't tested it but my hunch is that there is no problem with the Maxcap. From what i can tell by reading the code, it looks like SliceValue does not implement Object. So here the slice is saved as an ArrayValue and the information about the slice view (like which part of the array its pointing to) is then lost. The fact that it works, when you make a new slice can be explained that a new backing array is created for the new slice.

Can you test out this theory? For example, make a 10 element slice and slice = slice[4:5]. If the theory is correct, in the next transaction it should have the slice with all 10 elements.

[n0izn0iz]

@peter7891 this makes a lot of sense

Your test does not work like you expect though

For this realm:

package slice_splice

import (
    "strings"
)

var slice = []string{"0", "1", "2", "3", "4", "5", "6", "7", "8", "9"}

func Splice() []string {
    slice = slice[4:5]
    return slice
}

func Render(path string) string {
    return strings.Join(slice, ",")
}

Render returns 4 after calling Splice

[peter7891]

@peter7891 this makes a lot of sense

Your test does not work like you expect though

For this realm:

package slice_splice

import (
  "strings"
)

var slice = []string{"0", "1", "2", "3", "4", "5", "6", "7", "8", "9"}

func Splice() []string {
  slice = slice[4:5]
  return slice
}

func Render(path string) string {
  return strings.Join(slice, ",")
}

Render returns 4 after calling Splice

Right. My theory was wrong but it eliminated the possibility of there being a bug with the slicing. So, the problem should be with the append step, which was in your initial example. I'll dig more.

[jaekwon]

Maybe it thinks the new slice’s capacity is already hit, then it will first copy to new larger array, as per spec.

[deelawn]

This should be fixed by #1305

[deelawn]

Fixed by #1305