search

aya-rs / bpf-linker

Simple BPF static linker
Apache License 2.0
182 stars 41 forks source link

Provide `LLVMTypeWrapper` trait #223

Open vadorovsky opened 1 week ago

vadorovsky commented 1 week ago

Instead of keeping raw pointer fields (of types like LLVMValueRef, LLVMMetadataRef) public and defining constructors with different names, provide the LLVMTypeWrapper trait with from_ptr() as_ptr() method for all of them.

This will allow to convert all safe wrappers from and to raw pointers with one method, which is going to be helpful for building macros for them.

Depends on #222

This change is Reviewable

vadorovsky commented 1 week ago

src/llvm/types/mod.rs line 7 at r2 (raw file):

Previously, tamird (Tamir Duberstein) wrote…
why is it unsafe?

The most of pointer types coming from LLVM-C API (through llvm-sys) are more vague than the C++ ones.

There is LLVMValueRef which can be pretty much anything - Value, Instruction, GlobalVariable or any of dozens of classes inheriting from Value. Therefore constructing a wrapper corresponding to a C++ type (e.g. Instruction) from LLVMValueRef is unsafe, unless you perform a direct check.

But good that you asked. I didn't think about it before, but there are functions like IsAMDNode or IsAInstruction for performing such checks. Let me check if they cover all the types. If yes, I could make it method safe and just make sure we have checks everywhere. If no, I would either insist on keeping this unsafe or provide a separate method/trait for types for which the check functions exist.