[TableGen] Fix operand constraint checking problem. #85859
Conversation
Currently operand constraint checks on "$dest = $src" are inadvertently accepting any token that contains "=". This has surprising results, e.g, "$dest != $src" is accepted as a constraint but then treated as "=". This patch ensures that only exactly the token "=" is accepted.
| StringRef::size_type pos = CStr.find_first_of('='); | ||
| if (pos == StringRef::npos) | ||
| if (pos == StringRef::npos || CStr.find_first_of(" \t", pos) != (pos + 1) || | ||
| CStr.find_last_of(" \t", pos) != (pos - 1)) |
There was a problem hiding this comment.
Is it possible for pos to be 0 if the constraint starts with =? Would CStr.find_last_of(" \t", pos) return StringRef::npos in that case? Is that equal to (pos-1) since StringRef::npos is the largest unsigned number.
There was a problem hiding this comment.
Won’t this be caught by a different check finding the two operands of an infix operator? If a constraint starts with =, the LHS is missing.
There was a problem hiding this comment.
Right @topperc -- for the corner case =, the behavior is as you describe, so that npos == pos-1 and the condition on pos+1 fires instead. In other cases, we'll drop through to the following check on the operands as @davidchisnall mentions.
I've added these and other corner cases to the ConstraintChecking8.td test file (consolidated).
There was a problem hiding this comment.
The wrapping of pos - 1 to alias with StringRef::npos feels weird to me. Should we add pos == 0 as one of the conditions in this if?
There was a problem hiding this comment.
Sure, that's fine too. Commit coming shortly.
There was a problem hiding this comment.
Done (pos == 0 check).
And remove ConstraintChecking9.td.
unsigned wraparound behavior for the `pos-1` check.
Currently operand constraint checks on "$dest = $src" are inadvertently accepting any token that contains "=". This has surprising results, e.g, "$dest != $src" is accepted as a constraint but then treated as "=". This patch ensures that only exactly the token "=" is accepted.
Currently operand constraint checks on "$dest = $src" are inadvertently accepting any token that contains "=". This has surprising results, e.g, "$dest != $src" is accepted as a constraint but then treated as "=".
This patch ensures that only exactly the token "=" is accepted.