If a vector index is out of bounds, the result is supposed to be
undefined but is not undefined behavior. Change the legalization
for indexing the vector on the stack so that an out of bounds
index does not create an out of bounds memory access.
Details
- Reviewers
reames eli.friedman • tstellarAMD efriedma
Diff Detail
Event Timeline
lib/CodeGen/SelectionDAG/TargetLowering.cpp | ||
---|---|---|
3714 | I was wondering about this, however I think it's OK. getNode for EXTRACT_VECTOR_ELT turns this into undef for out of bounds. For INSERT_VECTOR_ELT it seems to not, but I also haven't been able to come up with a testcase where this gets hit | |
3726 | This should probably be an assert for now since I think this only happens after vector type legalization. When 3-vectors are eventually added this will need to be handled |
lib/CodeGen/SelectionDAG/TargetLowering.cpp | ||
---|---|---|
3714 | If getNode() is actually enforcing the restriction, fine... otherwise, we're likely to run into issues which are extremely difficult to reproduce because the constant is getting produced by splitting an integer in legalization or something like that. |
lib/CodeGen/SelectionDAG/TargetLowering.cpp | ||
---|---|---|
3714 | Should I fix getNode not doing this for insert_vector_elt to match instead? It seems broken to me that it would for one but not the other |
lib/CodeGen/SelectionDAG/TargetLowering.cpp | ||
---|---|---|
3714 | Actually, thinking about it a bit more, I would prefer not to leave this issue around as a trap if we start using getVectorElementPointer outside of insert/extract legalization, where we might not enforce a "constants are in range" rule. The check isn't doing anything useful anyway. (Of course, fixing getNode would be fine.) |
I'm pretty sure this special-case isn't safe; just because it's a constant doesn't mean it's in range.