All llvm::User objects store an array of llvm::Use objects representing the operands, either co-allocated preceding the User object, or in a separate, so-called "hung off" allocation. While we have bounds check assertions in User::getOperand(unsigned), the fixed operand accessors (Op<0>()) do not ultimately have such a bounds check. They ultimately call OpFrom, which could check the bounds of the index against OpTraits<U>::operands(). This seems like a common sense safety check, and we should just add it, although it won't catch code which iterates past the end of the operand list because our iterators are pointers. If we want to defend against that kind of error, we could turn User::op_iterator into a wrapped iterator that carries a User* and index, or something equivalent.
Currently, ASan and other dynamic instrumentation bounds checking tools are blind to this category of OOB access for Users that co-allocate the operand list with the User, and iterating one past the end of the use list reads memory from the User instead of reading from the red zones which would detect the OOB access.
I think the simplest way to catch these errors would be to disable operand co-allocation with ASan and other tools, and use hung-off uses for all kinds of users, even fixed operands. However, this probably creates significant code complexity, and blinds us to Asan bugs in the Use co-allocation codepath.
An alternative approach would be to co-allocate an extra Use in the operand list when ASan is enabled, and poison it using the ASan API. Between the two approaches to fixing ASan blindness, I think I favor the second.
All
llvm::User
objects store an array ofllvm::Use
objects representing the operands, either co-allocated preceding the User object, or in a separate, so-called "hung off" allocation. While we have bounds check assertions in User::getOperand(unsigned), the fixed operand accessors (Op<0>()
) do not ultimately have such a bounds check. They ultimately call OpFrom, which could check the bounds of the index againstOpTraits<U>::operands()
. This seems like a common sense safety check, and we should just add it, although it won't catch code which iterates past the end of the operand list because our iterators are pointers. If we want to defend against that kind of error, we could turn User::op_iterator into a wrapped iterator that carries aUser*
and index, or something equivalent.Currently, ASan and other dynamic instrumentation bounds checking tools are blind to this category of OOB access for Users that co-allocate the operand list with the User, and iterating one past the end of the use list reads memory from the User instead of reading from the red zones which would detect the OOB access.
I think the simplest way to catch these errors would be to disable operand co-allocation with ASan and other tools, and use hung-off uses for all kinds of users, even fixed operands. However, this probably creates significant code complexity, and blinds us to Asan bugs in the Use co-allocation codepath.
An alternative approach would be to co-allocate an extra
Use
in the operand list when ASan is enabled, and poison it using the ASan API. Between the two approaches to fixing ASan blindness, I think I favor the second.