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

Invalid codegen when unpacking tuple via template #22049

Closed
arnetheduck opened this issue Jun 8, 2023 · 5 comments · Fixed by #22097
Closed

Invalid codegen when unpacking tuple via template #22049

arnetheduck opened this issue Jun 8, 2023 · 5 comments · Fixed by #22097
Assignees

Comments

@arnetheduck
Copy link
Contributor

Description

type A = object
  field: tuple[a, b, c: seq[int]]

func value(v: var A): var tuple[a, b, c: seq[int]] =
  v.field
template get(v: A): tuple[a, b, c: seq[int]] = v.value

var v = A(field: (@[1], @[2], @[3]))
var (a, b, c) = v.get()

echo a, b, c

this generates invalid C code where the var:ness of the returned value is not taken into into account because of the non-var tuple.

The result is memory corruption / crashes due to dereferencing mismatches and invalid pointer casts.

Nim Version

1.6.12

Current Output

[arnetheduck@praeceps nimbus-eth2]$ nim c -r "/tmp/testit.nim"
Hint: used config file '/home/arnetheduck/status/nimbus-eth2/vendor/nimbus-build-system/vendor/Nim/config/nim.cfg' [Conf]
Hint: used config file '/home/arnetheduck/status/nimbus-eth2/vendor/nimbus-build-system/vendor/Nim/config/config.nims' [Conf]
Hint: used config file '/home/arnetheduck/.config/nim/nim.cfg' [Conf]
..........................................................
CC: ../home/arnetheduck/status/nimbus-eth2/vendor/nimbus-build-system/vendor/Nim/lib/system/dollars.nim
CC: ../home/arnetheduck/status/nimbus-eth2/vendor/nimbus-build-system/vendor/Nim/lib/system.nim
CC: testit.nim
Hint:  [Link]
Hint: gc: refc; opt: none (DEBUG BUILD, `-d:release` generates faster code)
26667 lines; 0.362s; 31.699MiB peakmem; proj: /tmp/testit.nim; out: /tmp/testit [SuccessX]
Hint: /tmp/testit  [Exec]
out of memory


### Expected Output

_No response_

### Possible Solution

_No response_

### Additional Information

_No response_
@metagn
Copy link
Collaborator

metagn commented Jun 8, 2023

Smaller case:

type A = object
  field: tuple[a: int]

func value(v: var A): var tuple[a: int] =
  v.field
template get(v: A): tuple[a: int] = v.value

var v = A(field: (a: 1))
echo get(v)[0]
error: 'T2_' is a pointer; did you mean to use '->'?
  T1_[0] = dollar___systemZdollars_u8(T2_.Field0);
                                         ^
                                         ->

@tersec
Copy link
Contributor

tersec commented Jun 8, 2023

A compile-time error from the C compiler is different than a runtime error which the C compiler allowed though, or?

@ringabout
Copy link
Member

Smaller case:

I'm debugging this case, probably nkHiddenSubConv is swallowed somewhere.

@ringabout
Copy link
Member

A compile-time error from the C compiler is different than a runtime error which the C compiler allowed though, or?

True, they are different issues, I have created a new issue for the second case.

@ringabout
Copy link
Member

ringabout commented Jun 9, 2023

After half day's industrious work, now It's obvious that templates implicitly convert types even though it doesn't check the types at all. A dirty workaround that disables implicit conversions for templates works like a charm, solving two issues with one stone.

@ringabout ringabout self-assigned this Jun 9, 2023
Araq pushed a commit that referenced this issue Jun 16, 2023
* fixes #22054; codegen for var tuples conv

* rethink fixes

* add test cases

* templates only

* fixes var tuples

* keep varness no matter what

* fixes typ.isNil

* make it work for generics

* restore isSubrange

* add a test case as requested
bung87 pushed a commit to bung87/Nim that referenced this issue Jul 29, 2023
… varness (nim-lang#22097)

* fixes nim-lang#22054; codegen for var tuples conv

* rethink fixes

* add test cases

* templates only

* fixes var tuples

* keep varness no matter what

* fixes typ.isNil

* make it work for generics

* restore isSubrange

* add a test case as requested
narimiran pushed a commit that referenced this issue Sep 11, 2023
* fixes #22054; codegen for var tuples conv

* rethink fixes

* add test cases

* templates only

* fixes var tuples

* keep varness no matter what

* fixes typ.isNil

* make it work for generics

* restore isSubrange

* add a test case as requested

(cherry picked from commit 77beb15)
narimiran pushed a commit that referenced this issue Sep 11, 2023
* fixes #22054; codegen for var tuples conv

* rethink fixes

* add test cases

* templates only

* fixes var tuples

* keep varness no matter what

* fixes typ.isNil

* make it work for generics

* restore isSubrange

* add a test case as requested

(cherry picked from commit 77beb15)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment