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

Building GEP instruction shouldn't be unsafe #514

Open
vaivaswatha opened this issue Jul 2, 2024 · 2 comments
Open

Building GEP instruction shouldn't be unsafe #514

vaivaswatha opened this issue Jul 2, 2024 · 2 comments

Comments

@vaivaswatha
Copy link
Contributor

The doc comment on the method has the following statement.

GEP is very likely to segfault if indexes are used incorrectly, and is therefore an unsafe function. Maybe we can change this in the future.

Wrong GEP indices leads to asserts (on debug builds of LLVM), and is perhaps a good enough reason to not have unsafe for this method.

Alternatively, a new inkwell datatype can be introduced for GEP indices, to make them safer. Something similar was done in Haskell. See https://luctielen.com/posts/making_llvm_gep_safer_in_haskell/.

@TheDan64
Copy link
Owner

TheDan64 commented Jul 3, 2024

asserts failing in C translate to UB or segfault on the rust side. So that's the reason for unsafe.

Pretty cool. I've thought about something similar to that haskell post, but never was able to get it working fully

@vaivaswatha
Copy link
Contributor Author

vaivaswatha commented Jul 3, 2024

Another simpler solution (but may be not so elegant) would be to just validate that the indices are valid for the provided pointer type.

This will require re-implementing something like GetElementPtrIns::getIndexedType. Note that the llvm-c API doesn't expose that method. (I tried to ask if they are willing to have it exposed, but they seemed reluctant).

With such a function, we could evaluate whether the indices are valid, and only then call LLVM's gep builder, being sure that the asserts won't hit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants