wjakob / nanobind

nanobind: tiny and efficient C++/Python bindings
BSD 3-Clause "New" or "Revised" License
2.14k stars 161 forks source link

fix named copy ctor argument #602

Closed WKarel closed 1 month ago

WKarel commented 1 month ago

Currently, binding a copy constructor with a named argument corrupts the stack due to writing past the end of a stack-allocated array.

wjakob commented 1 month ago

Hm, that's not good. But I am curious: why is this new in 2.0.0?

WKarel commented 1 month ago

I've used copy-constructors with named arguments already before 2.0, and did not encounter corrupted stacks. But as I see it, the indexing error has been there since https://github.com/wjakob/nanobind/commit/022935cbb92dfb1d02f90546bf6b34013f90e9e5 Possibly, I've encounted the corrupted stack only now because of a re-ordering of stack variables in the calling function. This branch was not taken in any of the tests, and is probably not taken by many applications.

wjakob commented 1 month ago

I think part of what's so confusing is that the function later adds an implicit argument for self, but that hasn't happened yet at this point in the function. Perhaps you could add a comment mentioning this to avoid similar errors in the future?

wjakob commented 1 month ago

Thanks!