i#1273 use drmgr everywhere: update inc2add to use drmgr and drreg

Updates the inc2add sample to use drmgr's app2app event and check the
for_trace parameter.  Changes the sample to always enable its
transformation, so that we can actually test that part of the code.

Also updates inc2add to use drreg for aflags liveness information.

Review-URL: https://codereview.appspot.com/316220043

api/samples/CMakeLists.txt

@@ -1,5 +1,5 @@
 # **********************************************************
-# Copyright (c) 2010-2016 Google, Inc.    All rights reserved.
+# Copyright (c) 2010-2017 Google, Inc.    All rights reserved.
 # Copyright (c) 2009-2010 VMware, Inc.    All rights reserved.
 # **********************************************************
 
@@ -215,7 +215,7 @@ if (X86) # FIXME i#1551, i#1569: port to ARM and AArch64
 endif (X86)
 add_sample_client(div         "div.c"           "drmgr")
 if (X86) # FIXME i#1551, i#1569: port to ARM and AArch64
-  add_sample_client(inc2add     "inc2add.c"       "")
+  add_sample_client(inc2add     "inc2add.c"       "drmgr;drreg")
   add_sample_client(memtrace_x86_binary "memtrace_x86.c;utils.c"
     "drcontainers;drmgr;drreg;drutil;drx")
   add_sample_client(memtrace_x86_text   "memtrace_x86.c;utils.c"

api/samples/inc2add.c

@@ -1,5 +1,5 @@
 /* **********************************************************
- * Copyright (c) 2014 Google, Inc.  All rights reserved.
+ * Copyright (c) 2014-2017 Google, Inc.  All rights reserved.
  * Copyright (c) 2002-2008 VMware, Inc.  All rights reserved.
  * **********************************************************/
 
@@ -42,6 +42,8 @@
  */
 
 #include "dr_api.h"
+#include "drmgr.h"
+#include "drreg.h"
 
 #ifdef WINDOWS
 # define DISPLAY_STRING(msg) dr_messagebox(msg)
@@ -53,45 +55,61 @@
 
 static bool enable;
 
-/* use atomic operations to increment these to avoid the hassle of locking. */
+/* Use atomic operations to increment these to avoid the hassle of locking. */
 static int num_examined, num_converted;
 
-/* replaces inc with add 1, dec with sub 1
- * returns true if successful, false if not
+/* Replaces inc with add 1, dec with sub 1.
+ * Returns true if successful, false if not.
  */
 static bool
 replace_inc_with_add(void *drcontext, instr_t *inst, instrlist_t *trace);
 
 static dr_emit_flags_t
-event_trace(void *drcontext, void *tag, instrlist_t *trace, bool translating);
+event_instruction_change(void *drcontext, void *tag, instrlist_t *bb,
+                         bool for_trace, bool translating);
 
 static void
 event_exit(void);
 
 DR_EXPORT void
 dr_client_main(client_id_t id, int argc, const char *argv[])
 {
+    /* We only used drreg for liveness, not for spilling, so we need no slots. */
+    drreg_options_t ops = {sizeof(ops), 0 /*no slots needed*/, false};
     dr_set_client_name("DynamoRIO Sample Client 'inc2add'",
                        "http://dynamorio.org/issues");
+    if (!drmgr_init() || drreg_init(&ops) != DRREG_SUCCESS)
+        DR_ASSERT(false);
+
+    /* Register for our events: process exit, and code transformation.
+     * We're changing the app's code, rather than just inserting observational
+     * instrumentation.
+     */
     dr_register_exit_event(event_exit);
-    dr_register_trace_event(event_trace);
-    /* this optimization is only worthwhile on the Pentium 4, where
-     * an add of 1 is faster than an inc
+    if (!drmgr_register_bb_app2app_event(event_instruction_change, NULL))
+        DR_ASSERT(false);
+
+    /* Long ago, this optimization would target the Pentium 4 (identified via
+     * "proc_get_family() == FAMILY_PENTIUM_4"), where an add of 1 is faster
+     * than an inc.  For illustration purposes we leave a boolean controlling it
+     * but we turn it on all the time in this sample and leave it for future
+     * work to determine whether to disable it on certain microarchitectures.
      */
-    enable = (proc_get_family() == FAMILY_PENTIUM_4);
-    /* make it easy to tell, by looking at log file, which client executed */
+    enable = true;
+
+    /* Make it easy to tell by looking at the log file which client executed. */
     dr_log(NULL, LOG_ALL, 1, "Client 'inc2add' initializing\n");
 #ifdef SHOW_RESULTS
-    /* also give notification to stderr */
+    /* Also give notification to stderr */
     if (dr_is_notify_on()) {
 # ifdef WINDOWS
-        /* ask for best-effort printing to cmd window.  must be called at init. */
+        /* Ask for best-effort printing to cmd window.  Must be called at init. */
         dr_enable_console_printing();
 # endif
         dr_fprintf(STDERR, "Client inc2add is running\n");
     }
 #endif
-    /* initialize our global variables */
+    /* Initialize our global variables. */
     num_examined = 0;
     num_converted = 0;
 }
@@ -114,109 +132,89 @@ event_exit(void)
     msg[sizeof(msg)/sizeof(msg[0])-1] = '\0';
     DISPLAY_STRING(msg);
 #endif /* SHOW_RESULTS */
+    if (!drmgr_unregister_bb_app2app_event(event_instruction_change) ||
+        drreg_exit() != DRREG_SUCCESS)
+        DR_ASSERT(false);
+    drmgr_exit();
 }
 
-/* replaces all inc with add 1, dec with sub 1
- * if cannot replace (eflags constraints), leaves original instruction alone
+/* Replaces inc with add 1, dec with sub 1.
+ * If cannot replace (eflags constraints), leaves original instruction alone.
  */
 static dr_emit_flags_t
-event_trace(void *drcontext, void *tag, instrlist_t *trace, bool translating)
+event_instruction_change(void *drcontext, void *tag, instrlist_t *bb,
+                         bool for_trace, bool translating)
 {
-    instr_t *instr, *next_instr;
     int opcode;
+    instr_t *instr, *next_instr;
 
-    if (!enable)
+    /* Only bother replacing for hot code, i.e., when for_trace is true, and
+     * when the underlying microarchitecture calls for it.
+     */
+    if (!for_trace || !enable)
         return DR_EMIT_DEFAULT;
 
-#ifdef VERBOSE
-    dr_printf("in dynamorio_trace(tag="PFX")\n", tag);
-    instrlist_disassemble(drcontext, tag, trace, STDOUT);
-#endif
-
-    for (instr = instrlist_first_app(trace); instr != NULL; instr = next_instr) {
-        /* grab next now so we don't go over instructions we insert */
+    for (instr = instrlist_first_app(bb); instr != NULL; instr = next_instr) {
+        /* We're deleting some instrs, so get the next first. */
         next_instr = instr_get_next_app(instr);
         opcode = instr_get_opcode(instr);
         if (opcode == OP_inc || opcode == OP_dec) {
             if (!translating)
                 ATOMIC_INC(num_examined);
-            if (replace_inc_with_add(drcontext, instr, trace)) {
+            if (replace_inc_with_add(drcontext, instr, bb)) {
                 if (!translating)
                     ATOMIC_INC(num_converted);
             }
         }
     }
 
-#ifdef VERBOSE
-    dr_printf("after dynamorio_trace(tag="PFX"):\n", tag);
-    instrlist_disassemble(drcontext, tag, trace, STDOUT);
-#endif
-
     return DR_EMIT_DEFAULT;
 }
 
-/* replaces inc with add 1, dec with sub 1
- * returns true if successful, false if not
+/* Replaces inc with add 1, dec with sub 1.
+ * Returns true if successful, false if not.
  */
 static bool
-replace_inc_with_add(void *drcontext, instr_t *instr, instrlist_t *trace)
+replace_inc_with_add(void *drcontext, instr_t *instr, instrlist_t *bb)
 {
-    instr_t *in;
+    instr_t *new_instr;
     uint eflags;
     int opcode = instr_get_opcode(instr);
-    bool ok_to_replace = false;
 
     DR_ASSERT(opcode == OP_inc || opcode == OP_dec);
 #ifdef VERBOSE
     dr_print_instr(drcontext, STDOUT, instr, "in replace_inc_with_add:\n\t");
 #endif
 
-    /* add/sub writes CF, inc/dec does not, make sure that's ok */
-    for (in = instr; in != NULL; in = instr_get_next(in)) {
-        eflags = instr_get_eflags(in, DR_QUERY_DEFAULT);
-        if ((eflags & EFLAGS_READ_CF) != 0) {
-#ifdef VERBOSE
-            dr_print_instr(drcontext, STDOUT, in,
-                           "\treads CF => cannot replace inc with add: ");
-#endif
-            return false;
-        }
-        if (instr_is_exit_cti(in)) {
-            /* to be more sophisticated, examine instructions at
-             * target of exit cti (if it is a direct branch).
-             * for this example, we give up if we hit a branch.
-             */
-            return false;
-        }
-        /* if writes but doesn't read, ok */
-        if ((eflags & EFLAGS_WRITE_CF) != 0) {
-            ok_to_replace = true;
-            break;
-        }
-    }
-    if (!ok_to_replace) {
+    /* Add/sub writes CF, inc/dec does not, so we make sure that's ok.
+     * We use drreg's liveness analysis, which includes the rest of this block.
+     * To be more sophisticated, we could examine instructions at target of each
+     * direct exit instead of assuming CF is live across any branch.
+     */
+    if (drreg_aflags_liveness(drcontext, instr, &eflags) != DRREG_SUCCESS ||
+        (eflags & EFLAGS_READ_CF) != 0) {
 #ifdef VERBOSE
-        dr_printf("\tno write to CF => cannot replace inc with add\n");
+        dr_printf("\tCF is live, cannot replace inc with add ");
 #endif
         return false;
     }
     if (opcode == OP_inc) {
 #ifdef VERBOSE
         dr_printf("\treplacing inc with add\n");
 #endif
-        in = INSTR_CREATE_add(drcontext, instr_get_dst(instr, 0),
-                              OPND_CREATE_INT8(1));
+        new_instr = INSTR_CREATE_add(drcontext, instr_get_dst(instr, 0),
+                                     OPND_CREATE_INT8(1));
     } else {
 #ifdef VERBOSE
         dr_printf("\treplacing dec with sub\n");
 #endif
-        in = INSTR_CREATE_sub(drcontext, instr_get_dst(instr, 0),
-                              OPND_CREATE_INT8(1));
+        new_instr = INSTR_CREATE_sub(drcontext, instr_get_dst(instr, 0),
+                                     OPND_CREATE_INT8(1));
     }
     if (instr_get_prefix_flag(instr, PREFIX_LOCK))
-        instr_set_prefix_flag(in, PREFIX_LOCK);
-    instr_set_translation(in, instr_get_app_pc(instr));
-    instrlist_replace(trace, instr, in);
+        instr_set_prefix_flag(new_instr, PREFIX_LOCK);
+    instr_set_translation(new_instr, instr_get_app_pc(instr));
+    instrlist_replace(bb, instr, new_instr);
     instr_destroy(drcontext, instr);
     return true;
 }