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

dereferencing var|lent uint(8|16|32) yields wrong value in VM #1407

Closed
alaviss opened this issue Aug 8, 2024 · 2 comments · Fixed by #1409
Closed

dereferencing var|lent uint(8|16|32) yields wrong value in VM #1407

alaviss opened this issue Aug 8, 2024 · 2 comments · Fixed by #1409
Assignees
Labels
bug Something isn't working compiler/vm Embedded virtual machine

Comments

@alaviss
Copy link
Contributor

alaviss commented Aug 8, 2024

Example

proc foo(values: openArray[uint16]) =
  for value in values.items:
    doAssert value == 40000u16:
      "Got: " & $value

foo([40000u16])

Actual Output

$ nim r --backend:vm test.nim
stack trace: (most recent call last)
test.nim(1, 2) NimMain
test.nim(6, 4) test
test.nim(3, 14) foo
assertions.nim(38, 26) failedAssertImpl
assertions.nim(28, 11) raiseAssert
fatal.nim(50, 5) sysFatal
fatal.nim(50, 5) Error: unhandled exception: test.nim(3, 14) `value == 40000'u16` Got: 18446744073709526080 [AssertionDefect]

Additional Information

  • Values between 0 - 32767 (high(int16)) does not exhibit this issue.
  • This only happens if the array was accessed via items.
@alaviss alaviss added bug Something isn't working compiler/vm Embedded virtual machine labels Aug 8, 2024
@zerbina zerbina self-assigned this Aug 8, 2024
@zerbina
Copy link
Collaborator

zerbina commented Aug 8, 2024

Thanks for filing the issue!

The problem is not directly related to openArray and uint16, but instead affects all lent uint(8|16|32) and var uint(8|16|32) dereferences, with the reason being that vmgen doesn't emit the necessary narrowing instructions.

A more minimized reproducer:

proc f(x: (uint8,)): lent uint8 = x[0]

echo f((128'u8,)) # echoes 0xFFFFFFFF_FFFFFF80 (as a non-hex number)

@zerbina zerbina changed the title passing values larger than high(int16) to openArray[uint16] in VM yields corrupted value dereferencing var|lent uint(8|16|32) yields wrong value in VM Aug 8, 2024
github-merge-queue bot pushed a commit that referenced this issue Aug 8, 2024
## Summary

Fix reading from a `var`/`lent` view of an unsigned integer (with bit-
width <= 32) producing incorrect values, when using the VM.

Fixes #1407.

## Details

* change `genDerefView` to take the `cnkDerefView` node as input, so
  that it has access to the type of the result
* the type of the *view* (not the storage type) was previously passed
  to the `genRegLoad` call, resulting in no `NarrowU` instruction to be
  emitted for the memory access, and the value thus being sign-extended
@saem
Copy link
Collaborator

saem commented Aug 8, 2024

Fixed in #1409

@saem saem closed this as completed Aug 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working compiler/vm Embedded virtual machine
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants