rust-lang / rust-lang/rust

`rustc_codegen_llvm` should mark more of its internal methods as `unsafe`

Open
#131,562 6 comments 1 reaction 0 assignees View on GitHub

Nobody has claimed this yet.

A-LLVM C-bug P-low T-compiler
Dominant language
Rust
Stars
119k
Forks
16.1k
PR merge metrics
PR metrics pending

Description

Just following up from this comment I made here: https://github.com/rust-lang/rust/issues/85532#issuecomment-2406526841

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:

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

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

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

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in rustc_codegen_llvm's builder.rs, especially the const_array and extract_value methods, and compare their LLVM FFI preconditions with safe methods such as CodegenCx::type_i1. Audit the other internal methods that call LLVM FFI, then update signatures where their preconditions can permit undefined behavior and check affected intrinsic-lowering callers.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
compilers
Issue type
Refactor
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
38/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.