Current application of bitwise-OR to a binary mask always results in True, which seems
inconsistent with the intent of the statement, a likely typo.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
lldb/tools/intel-features/intel-mpx/cli-wrapper-mpxtable.cpp | ||
---|---|---|
66 | According to https://en.cppreference.com/w/cpp/language/operator_precedence, I would read the left operand of the && as
which does not seem like what the original author intended. I absolutely think that the fix is correct, but I just wanted to get everyone's feedback on whether this seems like more than just a "suspicious bitwise expression" (and more like a "mistaken bitwise expression"). All that said, I could be completely, 100% wrong. And, if I am, feel free to ignore me! |
The original change was https://reviews.llvm.org/D29078. Are you able to run the tests with this applied?
lldb/tools/intel-features/intel-mpx/cli-wrapper-mpxtable.cpp | ||
---|---|---|
66 | The corrected code also makes sense given that MPX is some kind of memory protection across ranges. If ((lbound == one_cmpl64 || lbound == one_cmpl32) && ubound == 0) is true then upper bound < lower bound making an invalid range. Which is what I'd expect for some default/uninitialised state (especially if zero size ranges are allowed, so upper == 0 and lower == 0 couldn't be used). |
lldb/tools/intel-features/intel-mpx/cli-wrapper-mpxtable.cpp | ||
---|---|---|
66 | @DavidSpickett I think that you and I are saying the same thing, right? We are both saying that the corrected code looks much "better" than the original? Will |
lldb/tools/intel-features/intel-mpx/cli-wrapper-mpxtable.cpp | ||
---|---|---|
66 |
Yes.
Definitely a mistake that needs correcting. |
lldb/tools/intel-features/intel-mpx/cli-wrapper-mpxtable.cpp | ||
---|---|---|
66 | (I think "suspicious bitwise expression" is what the static analyser would call it just because it can't be 100% sure it is wrong) |
According to https://en.cppreference.com/w/cpp/language/operator_precedence, I would read the left operand of the && as
which does not seem like what the original author intended. I absolutely think that the fix is correct, but I just wanted to get everyone's feedback on whether this seems like more than just a "suspicious bitwise expression" (and more like a "mistaken bitwise expression").
All that said, I could be completely, 100% wrong. And, if I am, feel free to ignore me!