i#5786: Add precise clean call mangling identification

CMakeLists.txt

@@ -567,7 +567,7 @@ endif (EXISTS "${PROJECT_SOURCE_DIR}/.svn")
 
 # N.B.: when updating this, update the default version in ci-package.yml.
 # We should find a way to share (xref i#1565).
-set(VERSION_NUMBER_DEFAULT "9.0.${VERSION_NUMBER_PATCHLEVEL}")
+set(VERSION_NUMBER_DEFAULT "9.90.${VERSION_NUMBER_PATCHLEVEL}")
 # do not store the default VERSION_NUMBER in the cache to prevent a stale one
 # from preventing future version updates in a pre-existing build dir
 set(VERSION_NUMBER "" CACHE STRING "Version number: leave empty for default")
@@ -1382,7 +1382,7 @@ math(EXPR VERSION_NUMBER_INTEGER
 # 5.0 broke backcompat in drsyms and xmm opnd sizes
 # 4.1 broke backcompat in drsyms + 64-bit core (opcodes + reachability)
 # 4.0 broke backcompat in drmgr, drsyms, drinjectlib, and dr_get_milliseconds()
-set(OLDEST_COMPATIBLE_VERSION_DEFAULT "900")
+set(OLDEST_COMPATIBLE_VERSION_DEFAULT "990")
 set(OLDEST_COMPATIBLE_VERSION "" CACHE STRING
   "Oldest compatible version: leave empty for default")
 if ("${OLDEST_COMPATIBLE_VERSION}" STREQUAL "")

api/docs/release.dox

@@ -136,6 +136,12 @@ changes:
    the register in the structure from struct.value.reg to
    struct.value.reg_and_element_size.reg. The element size can be accessed directly
    via struct.value.reg_and_element_size.element_size.
+ - Changed the size of the #instr_t structure by appending a field which is used
+   for relative offsets while encoding.  The note field is no longer modified
+   during encoding.
+ - Reduced the value of #DR_NOTE_FIRST_RESERVED.  This is not expected to cause
+   problems unless clients are directly choosing high note values without using
+   drmgr_reserve_note_range().
 
 Further non-compatibility-affecting changes include:
  - Added AArchXX support for attaching to a running process.

core/arch/emit_utils_shared.c

@@ -1048,10 +1048,10 @@ encode_with_patch_list(dcontext_t *dcontext, patch_list_t *patch, instrlist_t *i
     }
 
     /* now encode the instructions */
-    /* must set note fields first with offset */
+    /* Must set offset fields first. */
     len = 0;
     for (inst = instrlist_first(ilist); inst; inst = instr_get_next(inst)) {
-        instr_set_note(inst, (void *)(ptr_uint_t)len);
+        inst->offset = len;
         len += instr_length(dcontext, inst);
     }
 

core/arch/mangle_shared.c

@@ -188,6 +188,10 @@ prepare_for_clean_call(dcontext_t *dcontext, clean_call_info_t *cci, instrlist_t
 {
     uint dstack_offs = 0;
 
+    instr_t *start_label = INSTR_CREATE_label(dcontext);
+    instr_set_note(start_label, (void *)DR_NOTE_CALL_SEQUENCE_START);
+    PRE(ilist, instr, start_label);
+
     if (cci == NULL)
         cci = &default_clean_call_info;
 
@@ -427,6 +431,9 @@ cleanup_after_clean_call(dcontext_t *dcontext, clean_call_info_t *cci, instrlist
         PRE(ilist, instr,
             instr_create_restore_from_dcontext(dcontext, REG_XSP, XSP_OFFSET));
     }
+    instr_t *end_label = INSTR_CREATE_label(dcontext);
+    instr_set_note(end_label, (void *)DR_NOTE_CALL_SEQUENCE_END);
+    PRE(ilist, instr, end_label);
 }
 
 bool
@@ -861,10 +868,6 @@ mangle_rseq_create_label(dcontext_t *dcontext, int type, ptr_uint_t data)
 {
     instr_t *label = INSTR_CREATE_label(dcontext);
     instr_set_note(label, (void *)DR_NOTE_RSEQ);
-    /* XXX: The note doesn't surivive encoding, so we also use a flag.  See comment in
-     * instr.h by this flag: maybe we should move a label's note somewhere persistent?
-     */
-    label->flags |= INSTR_RSEQ_ENDPOINT;
     dr_instr_label_data_t *label_data = instr_get_label_data_area(label);
     label_data->data[0] = type;
     label_data->data[1] = data;
@@ -1455,9 +1458,7 @@ mangle_rseq_finalize(dcontext_t *dcontext, instrlist_t *ilist, fragment_t *f)
     cache_pc rseq_start = NULL, rseq_end = NULL, rseq_abort = NULL;
     DEBUG_DECLARE(int label_sets_found = 0;)
     for (instr = instrlist_first(ilist); instr != NULL; instr = instr_get_next(instr)) {
-        if (instr_is_label(instr) &&
-            (instr_get_note(instr) == (void *)DR_NOTE_RSEQ ||
-             TEST(INSTR_RSEQ_ENDPOINT, instr->flags))) {
+        if (instr_is_label(instr) && instr_get_note(instr) == (void *)DR_NOTE_RSEQ) {
             dr_instr_label_data_t *label_data = instr_get_label_data_area(instr);
             switch (label_data->data[0]) {
             case DR_RSEQ_LABEL_ABORT: rseq_abort = pc; break;
@@ -1781,9 +1782,9 @@ d_r_mangle(dcontext_t *dcontext, instrlist_t *ilist, uint *flags INOUT, bool man
 
         if (!instr_is_cti(instr) || instr_is_meta(instr)) {
             if (TEST(INSTR_CLOBBER_RETADDR, instr->flags) && instr_is_label(instr)) {
-                /* move the value to the note field (which the client cannot
+                /* Move the value to the offset field (which the client cannot
                  * possibly use at this point) so we don't have to search for
-                 * this label when we hit the ret instr
+                 * this label when we hit the ret instr.
                  */
                 dr_instr_label_data_t *data = instr_get_label_data_area(instr);
                 instr_t *tmp;
@@ -1799,7 +1800,7 @@ d_r_mangle(dcontext_t *dcontext, instrlist_t *ilist, uint *flags INOUT, bool man
                 for (tmp = instr_get_next(instr); tmp != NULL;
                      tmp = instr_get_next(tmp)) {
                     if (tmp == ret) {
-                        tmp->note = (void *)data->data[1]; /* the value to use */
+                        tmp->offset = data->data[1]; /* the value to use */
                         break;
                     }
                 }

core/arch/x86/emit_utils.c

@@ -1,5 +1,5 @@
 /* **********************************************************
- * Copyright (c) 2010-2021 Google, Inc.  All rights reserved.
+ * Copyright (c) 2010-2022 Google, Inc.  All rights reserved.
  * Copyright (c) 2000-2010 VMware, Inc.  All rights reserved.
  * **********************************************************/
 
@@ -226,48 +226,48 @@ insert_spill_or_restore(dcontext_t *dcontext, cache_pc pc, uint flags, bool spil
     } else
 #endif /* X64 */
         if (shared) {
-        /* mov %ebx, fs:os_tls_offset(tls_offs) */
-        /* trying hard to keep the size of the stub 5 for eax, 6 else */
-        /* FIXME: case 5231 when staying on trace space is better,
-         * when going through this to the IBL routine speed asks for
-         * not adding the prefix.
-         */
-        bool addr16 = (require_addr16 || use_addr_prefix_on_short_disp());
-        if (addr16) {
-            *pc = ADDR_PREFIX_OPCODE;
-            pc++;
-        }
-        *pc = TLS_SEG_OPCODE;
-        pc++;
-        *pc = opcode;
-        pc++;
-        if (reg != REG_XAX) {
-            /* 0x1e for ebx, 0x0e for ecx, 0x06 for eax
-             * w/o addr16 those are 0x1d, 0x0d, 0x05
+            /* mov %ebx, fs:os_tls_offset(tls_offs) */
+            /* trying hard to keep the size of the stub 5 for eax, 6 else */
+            /* FIXME: case 5231 when staying on trace space is better,
+             * when going through this to the IBL routine speed asks for
+             * not adding the prefix.
              */
-            *pc = MODRM_BYTE(0 /*mod*/, reg_get_bits(reg), addr16 ? 6 : 5 /*rm*/);
+            bool addr16 = (require_addr16 || use_addr_prefix_on_short_disp());
+            if (addr16) {
+                *pc = ADDR_PREFIX_OPCODE;
+                pc++;
+            }
+            *pc = TLS_SEG_OPCODE;
             pc++;
-        }
-        if (addr16) {
-            *((ushort *)pc) = os_tls_offset(tls_offs);
-            pc += 2;
+            *pc = opcode;
+            pc++;
+            if (reg != REG_XAX) {
+                /* 0x1e for ebx, 0x0e for ecx, 0x06 for eax
+                 * w/o addr16 those are 0x1d, 0x0d, 0x05
+                 */
+                *pc = MODRM_BYTE(0 /*mod*/, reg_get_bits(reg), addr16 ? 6 : 5 /*rm*/);
+                pc++;
+            }
+            if (addr16) {
+                *((ushort *)pc) = os_tls_offset(tls_offs);
+                pc += 2;
+            } else {
+                *((uint *)pc) = os_tls_offset(tls_offs);
+                pc += 4;
+            }
         } else {
-            *((uint *)pc) = os_tls_offset(tls_offs);
-            pc += 4;
-        }
-    } else {
-        /* mov %ebx,((int)&dcontext)+dc_offs */
-        *pc = opcode;
-        pc++;
-        if (reg != REG_XAX) {
-            /* 0x1d for ebx, 0x0d for ecx, 0x05 for eax */
-            *pc = MODRM_BYTE(0 /*mod*/, reg_get_bits(reg), 5 /*rm*/);
+            /* mov %ebx,((int)&dcontext)+dc_offs */
+            *pc = opcode;
             pc++;
+            if (reg != REG_XAX) {
+                /* 0x1d for ebx, 0x0d for ecx, 0x05 for eax */
+                *pc = MODRM_BYTE(0 /*mod*/, reg_get_bits(reg), 5 /*rm*/);
+                pc++;
+            }
+            IF_X64(ASSERT_NOT_IMPLEMENTED(false));
+            *((uint *)pc) = (uint)(ptr_uint_t)UNPROT_OFFS(dcontext, dc_offs);
+            pc += 4;
         }
-        IF_X64(ASSERT_NOT_IMPLEMENTED(false));
-        *((uint *)pc) = (uint)(ptr_uint_t)UNPROT_OFFS(dcontext, dc_offs);
-        pc += 4;
-    }
     ASSERT(IF_X64_ELSE(false, !shared) ||
            (pc - start_pc) ==
                (reg == REG_XAX ? SIZE_MOV_XAX_TO_TLS(flags, require_addr16)
@@ -420,7 +420,7 @@ nop_pad_ilist(dcontext_t *dcontext, fragment_t *f, instrlist_t *ilist, bool emit
                         ASSERT((int)nop_length == instr_length(dcontext, nop_inst));
                         if (emitting) {
                             /* fixup offsets */
-                            instr_set_note(nop_inst, (void *)(ptr_uint_t)offset);
+                            nop_inst->offset = offset;
                             /* only inc stats for emitting, not for recreating */
                             STATS_PAD_JMPS_ADD(f->flags, num_nops, 1);
                             STATS_PAD_JMPS_ADD(f->flags, nop_bytes, nop_length);
@@ -443,7 +443,7 @@ nop_pad_ilist(dcontext_t *dcontext, fragment_t *f, instrlist_t *ilist, bool emit
             }
         }
         if (emitting)
-            instr_set_note(inst, (void *)(ptr_uint_t)offset); /* used by instr_encode */
+            inst->offset = offset; /* used by instr_encode */
         offset += instr_length(dcontext, inst);
     }
     return start_shift;

core/arch/x86/mangle.c

@@ -1726,8 +1726,8 @@ mangle_return(dcontext_t *dcontext, instrlist_t *ilist, instr_t *instr,
     }
 
     if (TEST(INSTR_CLOBBER_RETADDR, instr->flags)) {
-        /* we put the value in the note field earlier */
-        ptr_uint_t val = (ptr_uint_t)instr->note;
+        /* we put the value in the offset field earlier */
+        ptr_uint_t val = (ptr_uint_t)instr->offset;
         insert_mov_ptr_uint_beyond_TOS(dcontext, ilist, instr, val, retsz);
     }
 

core/emit.c

@@ -1,5 +1,5 @@
 /* **********************************************************
- * Copyright (c) 2012-2021 Google, Inc.  All rights reserved.
+ * Copyright (c) 2012-2022 Google, Inc.  All rights reserved.
  * Copyright (c) 2000-2010 VMware, Inc.  All rights reserved.
  * **********************************************************/
 
@@ -438,9 +438,8 @@ emit_fragment_common(dcontext_t *dcontext, app_pc tag, instrlist_t *ilist, uint
         IF_X64(CLIENT_ASSERT(instr_get_isa_mode(inst) == isa_mode,
                              "single fragment cannot mix x86 and x64 modes"));
         if (!PAD_FRAGMENT_JMPS(flags)) {
-            /* we're going to skip the 2nd pass, save this instr's offset in
-             * the note field (used by instr_encode) */
-            instr_set_note(inst, (void *)(ptr_uint_t)offset);
+            /* We're going to skip the 2nd pass; save the offset for instr_encode. */
+            inst->offset = offset;
         }
         if (instr_ok_to_emit(inst))
             offset += instr_length(dcontext, inst);
@@ -505,7 +504,7 @@ emit_fragment_common(dcontext_t *dcontext, app_pc tag, instrlist_t *ilist, uint
             extra_jmp_padding_body += 2;
             instrlist_append(ilist, INSTR_CREATE_nop(dcontext));
             ASSERT(instr_length(dcontext, instrlist_last(ilist)) == 2);
-            instr_set_note(instrlist_last(ilist), (void *)(ptr_uint_t)offset);
+            instrlist_last(ilist)->offset = offset;
         }
         ASSERT(ALIGNED(offset + extra_jmp_padding_body, PC_LOAD_ADDR_ALIGN));
     }

core/heap.c

@@ -112,18 +112,17 @@ static const uint BLOCK_SIZES[] = {
     /* 40 dbg / 36 rel: */
     ALIGN_FORWARD(sizeof(fragment_t) + sizeof(indirect_linkstub_t), HEAP_ALIGNMENT),
 #if defined(X64)
-    sizeof(instr_t), /* 104 x64 */
 #    ifdef DEBUG
     sizeof(fragment_t) + sizeof(direct_linkstub_t) +
         sizeof(cbr_fallthrough_linkstub_t), /* 112 dbg x64 / 104 rel x64 */
 #    else
-/* release == instr_t */
+    sizeof(instr_t), /* 112 x64 */
 #    endif
 #else
     sizeof(fragment_t) + sizeof(direct_linkstub_t) +
         sizeof(cbr_fallthrough_linkstub_t), /* 60 dbg / 56 rel */
 #    ifndef DEBUG
-    sizeof(instr_t),                        /* 68 */
+    sizeof(instr_t),                        /* 72 */
 #    endif
 #endif
     /* we keep this bucket even though only 10% or so of normal bbs

core/ir/aarch64/codec.c

@@ -229,7 +229,7 @@ encode_pc_off(OUT uint *poff, int bits, byte *pc, instr_t *instr, opnd_t opnd,
     if (opnd.kind == PC_kind)
         off = opnd.value.pc - pc;
     else if (opnd.kind == INSTR_kind)
-        off = (byte *)opnd_get_instr(opnd)->note - (byte *)instr->note;
+        off = (byte *)opnd_get_instr(opnd)->offset - (byte *)instr->offset;
     else
         return false;
     range = (ptr_uint_t)1 << bits;
@@ -783,7 +783,8 @@ encode_opnd_adr_page(int scale, byte *pc, opnd_t opnd, OUT uint *enc_out, instr_
         offset = (ptr_int_t)opnd_get_addr(opnd) -
             (ptr_int_t)((ptr_uint_t)pc >> scale << scale);
     } else if (opnd_is_instr(opnd)) {
-        offset = (ptr_int_t)((byte *)opnd_get_instr(opnd)->note - (byte *)instr->note);
+        offset =
+            (ptr_int_t)((byte *)opnd_get_instr(opnd)->offset - (byte *)instr->offset);
     } else
         return false;
 
@@ -3939,9 +3940,8 @@ encode_opnd_instr(int bit_pos, opnd_t opnd, byte *start_pc, instr_t *containing_
     if (!opnd_is_instr(opnd)) {
         return false;
     }
-    ptr_uint_t val =
-        ((ptr_uint_t)instr_get_note(opnd_get_instr(opnd)) -
-         (ptr_uint_t)instr_get_note(containing_instr) + (ptr_uint_t)start_pc) >>
+    ptr_uint_t val = (opnd_get_instr(opnd)->offset - containing_instr->offset +
+                      (ptr_uint_t)start_pc) >>
         opnd_get_shift(opnd);
 
     uint bits = opnd_size_in_bits(opnd_get_size(opnd));

core/ir/arm/decode_private.h

@@ -1,5 +1,5 @@
 /* **********************************************************
- * Copyright (c) 2014-2015 Google, Inc.  All rights reserved.
+ * Copyright (c) 2014-2022 Google, Inc.  All rights reserved.
  * **********************************************************/
 
 /*
@@ -212,7 +212,7 @@ struct _decode_info_t {
     byte *decorated_pc;
 
     /* For instr_t* target encoding */
-    ptr_int_t cur_note;
+    ptr_int_t cur_offs;
     bool has_instr_opnds;
 
     /* For IT block */

core/ir/arm/encode.c

@@ -1034,12 +1034,12 @@ get_immed_val_shared(decode_info_t *di, opnd_t opnd, bool relative, bool selecte
             CLIENT_ASSERT(opnd_get_shift(opnd) == 0,
                           "relative shifted instr not supported");
             /* For A32, "cur PC" is "PC + 8"; "PC + 4" for Thumb, sometimes aligned */
-            return (ptr_int_t)opnd_get_instr(opnd)->note -
-                (di->cur_note +
+            return (ptr_int_t)opnd_get_instr(opnd)->offset -
+                (di->cur_offs +
                  decode_cur_pc(di->final_pc, di->isa_mode, di->opcode, NULL) -
                  di->final_pc);
         } else {
-            ptr_int_t val = (ptr_int_t)opnd_get_instr(opnd)->note - (di->cur_note) +
+            ptr_int_t val = (ptr_int_t)opnd_get_instr(opnd)->offset - (di->cur_offs) +
                 (ptr_int_t)di->final_pc;
             /* Support insert_mov_instr_addr() by truncating to opnd size */
             uint bits = opnd_size_in_bits(opnd_get_size(opnd));
@@ -1053,9 +1053,9 @@ get_immed_val_shared(decode_info_t *di, opnd_t opnd, bool relative, bool selecte
     } else if (opnd_is_near_pc(opnd)) {
         if (relative) {
             /* For A32, "cur PC" is "PC + 8"; "PC + 4" for Thumb, sometimes aligned */
-            return (ptr_int_t)(
-                opnd_get_pc(opnd) -
-                decode_cur_pc(di->final_pc, di->isa_mode, di->opcode, NULL));
+            return (
+                ptr_int_t)(opnd_get_pc(opnd) -
+                           decode_cur_pc(di->final_pc, di->isa_mode, di->opcode, NULL));
         } else {
             return (ptr_int_t)opnd_get_pc(opnd);
         }
@@ -1353,8 +1353,8 @@ get_abspc_delta(decode_info_t *di, opnd_t opnd)
 {
     /* For A32, "cur PC" is really "PC + 8"; "PC + 4" for Thumb, sometimes aligned */
     if (opnd_is_mem_instr(opnd)) {
-        return (ptr_int_t)opnd_get_instr(opnd)->note -
-            (di->cur_note + decode_cur_pc(di->final_pc, di->isa_mode, di->opcode, NULL) -
+        return (ptr_int_t)opnd_get_instr(opnd)->offset -
+            (di->cur_offs + decode_cur_pc(di->final_pc, di->isa_mode, di->opcode, NULL) -
              di->final_pc) +
             opnd_get_mem_instr_disp(opnd);
     } else {
@@ -3036,7 +3036,7 @@ instr_encode_arch(dcontext_t *dcontext, instr_t *instr, byte *copy_pc, byte *fin
     di.check_reachable = check_reachable;
     di.start_pc = copy_pc;
     di.final_pc = final_pc;
-    di.cur_note = (ptr_int_t)instr->note;
+    di.cur_offs = (ptr_int_t)instr->offset;
     di.encode_state = *get_encode_state(dcontext);
 
     /* We need to track the IT block state even for raw-bits-valid instrs.

core/ir/instr.h

@@ -1,5 +1,5 @@
 /* **********************************************************
- * Copyright (c) 2011-2021 Google, Inc.  All rights reserved.
+ * Copyright (c) 2011-2022 Google, Inc.  All rights reserved.
  * Copyright (c) 2000-2010 VMware, Inc.  All rights reserved.
  * **********************************************************/
 
@@ -170,9 +170,8 @@ enum {
 #else
     /* Indicates an instruction that's part of the rseq endpoint.  We use this in
      * instrlist_t.flags (sort of the same namespace: INSTR_OUR_MANGLING is used there,
-     * but also EDI_VAL_*) and as a version of DR_NOTE_RSEQ that survives encoding
-     * (seems like we could store notes for labels in another field so they do
-     * in fact survive: a union with instr_t.translation?).
+     * but also EDI_VAL_*); we no longer use it on individual instructions since
+     * the label note field DR_NOTE_RSEQ now survives encoding.
      */
     INSTR_RSEQ_ENDPOINT = 0x01000000,
 #endif