search

wjakob / nanobind

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

Protect a reference to kwargs_in->ob_name with a nullptr test. #593

Closed hawkinsp closed 4 months ago

hawkinsp commented 4 months ago

The current code is C++ undefined behavior if kwargs_in is nullptr.

wjakob commented 4 months ago

This looks potentially bad. Can you explain how this issue is triggered? Why aren't we running into it in the test suite and projects building on nanobind?

hawkinsp commented 4 months ago

I think it's sort of benign but technically UB.

The nanobind test suite crashes if built with clang with -fsanitize=null.

However, the ->ob_impl despite appearances isn't really a dereference since it's really just getting the address of an array (the tuple contents) which the code then ignores.

You can argue this is just an overly aggressive sanitizer, but it is correct: this is a true positive for UB and it's a good idea to avoid it.

wjakob commented 4 months ago

Ok, thank you. Let's include this change in the next patch release then.