Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Turn opmask_with_dsts()/is_evex_mask condition into an assertion. #7159

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 9 additions & 3 deletions core/ir/disassemble_shared.c
Original file line number Diff line number Diff line change
Expand Up @@ -1035,7 +1035,9 @@ instr_disassemble_opnds_noimplicit(char *buf, size_t bufsz, size_t *sofar DR_PAR
*/
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and DR syntax should never get into this function.

Do we need an assert for this?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there an issue# that this bug fixes? Either way, would be nice to note the goal (cleanup/bug-fix) in the PR description.

optype = 0;
});
bool is_evex_mask = optype_is_evex_mask_arch(optype) && opmask_with_dsts();
bool is_evex_mask = optype_is_evex_mask_arch(optype);
CLIENT_ASSERT(!is_evex_mask || opmask_with_dsts(),
"Anything here with evex mask should be opmask_with_dsts()");
if (!is_evex_mask) {
print_to_buffer(buf, bufsz, sofar, "");
printing = opnd_disassemble_noimplicit(buf, bufsz, sofar, dcontext, instr,
Expand Down Expand Up @@ -1077,7 +1079,9 @@ instr_disassemble_opnds_noimplicit(char *buf, size_t bufsz, size_t *sofar DR_PAR
(i == 0 && opnd_is_reg(opnd) && reg_is_fp(opnd_get_reg(opnd))));
});
if (print) {
bool is_evex_mask = optype_is_evex_mask_arch(optype) && opmask_with_dsts();
bool is_evex_mask = optype_is_evex_mask_arch(optype);
CLIENT_ASSERT(!is_evex_mask || opmask_with_dsts(),
"Anything here with evex mask should be opmask_with_dsts()");
print_to_buffer(buf, bufsz, sofar, is_evex_mask ? " {" : "");
prev = opnd_disassemble_noimplicit(buf, bufsz, sofar, dcontext, instr, optype,
opnd, prev && !is_evex_mask,
Expand All @@ -1092,8 +1096,10 @@ instr_disassemble_opnds_noimplicit(char *buf, size_t bufsz, size_t *sofar DR_PAR
CLIENT_ASSERT(IF_X86_ELSE(true, false), "evex mask can only exist for x86.");
optype = instr_info_opnd_type(info, !dsts_first(), mask_index);
CLIENT_ASSERT(!instr_is_opmask(instr) && opnd_is_reg(opnd) &&
reg_is_opmask(opnd_get_reg(opnd)) && opmask_with_dsts(),
reg_is_opmask(opnd_get_reg(opnd)),
"evex mask must always be the first source.");
CLIENT_ASSERT(opmask_with_dsts(),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe better if this is the first thing in this if-block.

"Anything here with evex mask should be opmask_with_dsts()");
print_to_buffer(buf, bufsz, sofar, " {");
opnd_disassemble_noimplicit(buf, bufsz, sofar, dcontext, instr, optype, opnd,
false, multiple_encodings, dsts_first(), &mask_index);
Expand Down
Loading