rustc_codegen_llvm should mark more of its internal methods as unsafe

[clarfonthey]

Just following up from this comment I made here: #85532 (comment)

While LLVM compiled with asserts ensures that no undefined behaviour occurs when calling methods, it does not when compiled without asserts, and this should really be reflected in the signatures for all the various internal methods that just call out to LLVM FFI.

Right now, it's very easy to trigger UB when you're writing an intrinsic, and while it's common for C functions to have all sorts of undocumented preconditions, we should not extend this habit into Rust.

For example, const_array will trivially trigger UB if any of the Values passed into it are not actually constant:

• https://doc.rust-lang.org/nightly/nightly-rustc/rustc_codegen_llvm/builder/struct.Builder.html#method.const_array
• https://llvm.org/doxygen/group__LLVMCCoreValueConstantComposite.html#ga3e37d5cc97d6e4da63f6eaa22e469075

And extract_value will trigger UB if the index is out of bounds for the given Value:

• https://doc.rust-lang.org/nightly/nightly-rustc/src/rustc_codegen_llvm/builder.rs.html#978-981
• https://llvm.org/doxygen/group__LLVMCCoreInstructionBuilder.html#ga21935fb1e744e161e726611778ac1618

Whereas something like type_i1 is fine and will always be safe to call:

• https://doc.rust-lang.org/nightly/nightly-rustc/rustc_codegen_llvm/context/struct.CodegenCx.html#method.type_i1
• https://llvm.org/doxygen/group__LLVMCCoreTypeInt.html#ga390b4c486c780eed40002b07933d13df

Sure, this will "introduce" unsafe code to, for example, the intrinsics lowering, but the code was already unsafe, and this is just documenting that.

@rustbot label T-compiler

[RalfJung]

Yeah I also ran into this, e.g. calling AddFunctionAttributes or AddCallSiteAttributes on the wrong thing / with an out-of-bounds index can lead to segfaults. These should all be unsafe functions with documented preconditions (or, ideally, get a safe wrapper).

[clarfonthey]

In my case, I was getting far worse than segfaults: illegal instructions. And worse, since it was UB, the resulting errors did not occur with a useful stack trace pointing out where the error was, which is why I mentioned assertions being so helpful. rustc can handle segfaults sometimes in FFI, but SIGILL will always abort the process, meaning the only way to properly debug is to open up the core dump in a debugger.

[RalfJung]

TBH I don't remember whether it was SEGFAULT or SIGILL... it was an abrupt abort and I debugged it by staring hard at my code until I realized where the wrong call must be.^^

[apiraino]

WG-prioritization assigning priority (Zulip discussion).

@rustbot label -I-prioritize +P-low

[jieyouxu]

Triage: this is probably more of a tracking issue for cg_llvm so someone interested can try to minimize + annotate things as unsafe and document their expected usage contracts to make cg_llvm less footgunny.

[Noratrieb]

Not a soundness hole in the language.