i#731 re-rel: Convert native rseq PC targets to instrs (#4023)

For i#731 with automatic re-relativization of absolute PC's, in
d6f5fca we simply kept the hardcoded offset for intra-region branch
targets in our native rseq copy.  However, with subsequent mangling
that offset can become incorrect and target the middle of an
instruction, leading to a crash.  We instead take the time to convert
these PC targets to instr_t* targets.

We also tweak the disassembly output to show the instr_t pointer value
for level 3 instructions too, since jumps can target them as well as
synthetic instructions.  This helped with verifying and debugging this
change.

Tested on an inserted system call for locally forcing rseq restarts,
which leads to system call mangling and crashes without this fix.

Issue: #731, #2350

core/arch/disassemble_shared.c

@@ -1,5 +1,5 @@
 /* **********************************************************
- * Copyright (c) 2011-2019 Google, Inc.  All rights reserved.
+ * Copyright (c) 2011-2020 Google, Inc.  All rights reserved.
  * Copyright (c) 2001-2009 VMware, Inc.  All rights reserved.
  * **********************************************************/
 
@@ -1509,13 +1509,8 @@ instrlist_disassemble(dcontext_t *dcontext, app_pc tag, instrlist_t *ilist,
              * as much about raw bytes
              */
             int extra_sz;
-            if (level == 3) {
-                print_file(outfile, " +%-4d %c%d " IF_X64_ELSE("%20s", "%12s"), offs,
-                           instr_is_app(instr) ? 'L' : 'm', level, " ");
-            } else {
-                print_file(outfile, " +%-4d %c%d @" PFX " ", offs,
-                           instr_is_app(instr) ? 'L' : 'm', level, instr);
-            }
+            print_file(outfile, " +%-4d %c%d @" PFX " ", offs,
+                       instr_is_app(instr) ? 'L' : 'm', level, instr);
             extra_sz = print_bytes_to_file(outfile, addr, addr + len, instr);
             instr_disassemble(dcontext, instr, outfile);
             print_file(outfile, "\n");

core/arch/mangle_shared.c

@@ -1086,6 +1086,10 @@ mangle_rseq_insert_native_sequence(dcontext_t *dcontext, instrlist_t *ilist,
      * not stores, and restartable).
      */
     app_pc pc = start;
+    /* Store the PC values for faster conversion of intra-region targets. */
+    generic_table_t *pc2instr =
+        generic_hash_create(dcontext, 6 /*expect few entries*/, 80 /* load factor */, 0,
+                            NULL _IF_DEBUG("pc2instr"));
     PRE(ilist, insert_at, label_start);
     while (pc < end) {
         instr_t *copy = instr_create(dcontext);
@@ -1096,17 +1100,13 @@ mangle_rseq_insert_native_sequence(dcontext_t *dcontext, instrlist_t *ilist,
                                         "Invalid instruction inside rseq region");
             ASSERT_NOT_REACHED();
         }
+        generic_hash_add(dcontext, pc2instr, (ptr_uint_t)get_app_instr_xl8(copy), copy);
         /* Make intra-region branches meta; all others are exit ctis. */
         if ((instr_is_cbr(copy) || instr_is_ubr(copy)) &&
             opnd_is_pc(instr_get_target(copy))) {
             app_pc tgt = opnd_get_pc(instr_get_target(copy));
             if (tgt >= start && tgt < end) {
-                /* We do not want to use the absolute PC and re-relativize: we want
-                 * to use the same relative offset.  (An alternative would be to
-                 * convert the PC operand to an instr_t pointer but that would take
-                 * extra passes.)
-                 */
-                IF_X86(instr_set_rip_rel_valid(copy, false));
+                /* We change the target from a PC to an instr_t* at the end. */
                 PRE(ilist, insert_at, copy);
                 continue;
             }
@@ -1129,6 +1129,31 @@ mangle_rseq_insert_native_sequence(dcontext_t *dcontext, instrlist_t *ilist,
         }
     }
     PRE(ilist, insert_at, label_end);
+    /* Update all intra-region targets to use instr_t* operands.  We can't simply
+     * leave absolute PC's and re-relativize (that would point into the app code).
+     * Nor can we use the hardcoded relative offset by calling
+     * instr_set_rip_rel_valid(, false) because there can be subsequent mangling that
+     * changes the offsets.
+     */
+    for (instr_t *walk = label_start; walk != label_end; walk = instr_get_next(walk)) {
+        if (!instr_is_app(walk) && (instr_is_cbr(walk) || instr_is_ubr(walk))) {
+            ASSERT(opnd_is_pc(instr_get_target(walk)));
+            app_pc tgt_pc = opnd_get_pc(instr_get_target(walk));
+            instr_t *tgt_inst =
+                (instr_t *)generic_hash_lookup(dcontext, pc2instr, (ptr_uint_t)tgt_pc);
+            if (tgt_inst == NULL) {
+                LOG(THREAD, LOG_INTERP, 1,
+                    "%s: pc2instr failed for branch from " PFX " to " PFX "\n",
+                    __FUNCTION__, get_app_instr_xl8(walk), tgt_pc);
+                REPORT_FATAL_ERROR_AND_EXIT(RSEQ_BEHAVIOR_UNSUPPORTED, 3,
+                                            get_application_name(), get_application_pid(),
+                                            "Rseq branch target is mid-instruction");
+                ASSERT_NOT_REACHED();
+            }
+            instr_set_target(walk, opnd_create_instr(tgt_inst));
+        }
+    }
+    generic_hash_destroy(dcontext, pc2instr);
     /* Now mangle from this point. */
     *next_instr = start_mangling;