Skip to content

Commit

Permalink
i#2360: consistently label nzcvqg as arithmetic on ARM (#2362)
Browse files Browse the repository at this point in the history
DR considers the Q and GE flags to be arithmetic for saving, restoring, and
in the EFLAGS_ARITH constant, yet not in the EFLAGS_{READ,WRITE}_ARITH
defines.  We correct that here by including them in all arithmetic
defines.

Re-enables the drreg test that was broken because of this.

Relaxes a drreg assert on re-spilling to the aflags slot with different
regs, which is exposed by this fix.  This relaxation mirrors what we
already do on restoring aflags.

Fixes #2360
  • Loading branch information
derekbruening authored Apr 18, 2017
1 parent c839a54 commit 9da55c6
Show file tree
Hide file tree
Showing 4 changed files with 14 additions and 15 deletions.
15 changes: 6 additions & 9 deletions core/arch/instr.h
Original file line number Diff line number Diff line change
Expand Up @@ -2863,8 +2863,9 @@ enum {
# define EFLAGS_READ_GE 0x00000020 /**< Reads GE (>= for parallel arithmetic). */
# define EFLAGS_READ_NZCV (EFLAGS_READ_N | EFLAGS_READ_Z |\
EFLAGS_READ_C | EFLAGS_READ_V)
# define EFLAGS_READ_ARITH EFLAGS_READ_NZCV /**< Reads all arithmetic flags. */
# define EFLAGS_READ_ALL (EFLAGS_READ_NZCV | EFLAGS_READ_GE) /**< Reads all flags. */
/** Platform-independent macro for reads all arithmetic flags. */
# define EFLAGS_READ_ARITH (EFLAGS_READ_NZCV | EFLAGS_READ_Q | EFLAGS_READ_GE)
# define EFLAGS_READ_ALL (EFLAGS_READ_ARITH) /**< Reads all flags. */
# define EFLAGS_READ_NON_PRED EFLAGS_READ_GE /**< Flags not read by predicates. */
# define EFLAGS_WRITE_N 0x00000040 /**< Reads N (negative). */
# define EFLAGS_WRITE_Z 0x00000080 /**< Reads Z (zero). */
Expand All @@ -2874,13 +2875,9 @@ enum {
# define EFLAGS_WRITE_GE 0x00000800 /**< Reads GE (>= for parallel arithmetic). */
# define EFLAGS_WRITE_NZCV (EFLAGS_WRITE_N | EFLAGS_WRITE_Z |\
EFLAGS_WRITE_C | EFLAGS_WRITE_V)
# define EFLAGS_WRITE_ARITH EFLAGS_WRITE_NZCV /**< Reads all arithmetic flags. */
# define EFLAGS_WRITE_ALL (EFLAGS_WRITE_NZCV | EFLAGS_WRITE_GE) /**< Reads all flags. */

/** Platform-independent macro for reads all arithmetic flags. */
# define EFLAGS_READ_ARITH EFLAGS_READ_NZCV
/** Platform-independent macor for writes all arithmetic flags. */
# define EFLAGS_WRITE_ARITH EFLAGS_WRITE_NZCV
/** Platform-independent macro for writes all arithmetic flags. */
# define EFLAGS_WRITE_ARITH (EFLAGS_WRITE_NZCV | EFLAGS_WRITE_Q | EFLAGS_WRITE_GE)
# define EFLAGS_WRITE_ALL (EFLAGS_WRITE_ARITH) /**< Writes all flags. */

/** Converts an EFLAGS_WRITE_* value to the corresponding EFLAGS_READ_* value. */
# define EFLAGS_WRITE_TO_READ(x) ((x) >> 6)
Expand Down
5 changes: 4 additions & 1 deletion ext/drreg/drreg.c
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,10 @@ spill_reg(void *drcontext, per_thread_t *pt, reg_id_t reg, uint slot,
"%s @%d."PFX" %s %d\n", __FUNCTION__, pt->live_idx, instr_get_app_pc(where),
get_register_name(reg), slot);
ASSERT(pt->slot_use[slot] == DR_REG_NULL ||
pt->slot_use[slot] == reg, "internal tracking error");
pt->slot_use[slot] == reg ||
/* aflags can be saved and restored using different regs */
slot == AFLAGS_SLOT,
"internal tracking error");
pt->slot_use[slot] = reg;
if (slot < ops.num_spill_slots) {
dr_insert_write_raw_tls(drcontext, ilist, where, tls_seg,
Expand Down
6 changes: 3 additions & 3 deletions suite/tests/api/ir_arm.c
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/* **********************************************************
* Copyright (c) 2015-2016 Google, Inc. All rights reserved.
* Copyright (c) 2015-2017 Google, Inc. All rights reserved.
* **********************************************************/

/*
Expand Down Expand Up @@ -88,8 +88,8 @@ test_pred(void *dc)
(dc, opnd_create_reg(DR_REG_R0), opnd_create_reg(DR_REG_R1),
opnd_create_reg(DR_REG_R1)),
DR_PRED_EQ);
ASSERT(instr_get_eflags(inst, DR_QUERY_INCLUDE_COND_SRCS) == EFLAGS_READ_ALL);
ASSERT(instr_get_eflags(inst, 0) == EFLAGS_READ_ARITH);
ASSERT(instr_get_eflags(inst, DR_QUERY_INCLUDE_COND_SRCS) == EFLAGS_READ_ARITH);
ASSERT(instr_get_eflags(inst, 0) == (EFLAGS_READ_ARITH & (~EFLAGS_READ_GE)));
instr_free(dc, inst);
inst = INSTR_CREATE_sel
(dc, opnd_create_reg(DR_REG_R0), opnd_create_reg(DR_REG_R1),
Expand Down
3 changes: 1 addition & 2 deletions suite/tests/client-interface/drreg-test.dll.c
Original file line number Diff line number Diff line change
Expand Up @@ -156,11 +156,10 @@ event_app_instruction(void *drcontext, void *tag, instrlist_t *bb, instr_t *inst
CHECK(res == DRREG_SUCCESS, "unreserve should work");

/* test aflags */
/* FIXME i#2360: ARM's arith flags are inconsistent */
/* FIXME i#2263: AArch64 fails to mark what flags are used in the IR, breaking
* use of aflags in instrumentation.
*/
#ifndef AARCHXX
#ifndef AARCH64
res = drreg_reserve_aflags(drcontext, bb, inst);
CHECK(res == DRREG_SUCCESS, "reserve of aflags should work");
res = drreg_restore_app_aflags(drcontext, bb, inst);
Expand Down

0 comments on commit 9da55c6

Please sign in to comment.