i#3962 label cb: Add instr_clear_label_callback()

PR #3960 added a call to instr_set_label_callback() to set it to NULL
from instr_clone(), but if the callback is non-NULL an assert fires in
that case.  This only normally happens with emulation labels that turn
into traces, which happens to not occur in our very few tests of
emulation labels (#3173 covers adding more tests).

We fix that by adding instr_clear_label_callback() here and using that
from instr_clone(), since these callbacks are a little different from
other values and it feels best to not clear them using the set
routine.

A test is added by putting a loop around a scatter-gather expansion,
triggering trace creation.  I confirmed that the assert does fire
without this fix with the loop in place.

Fixes #3962

api/docs/release.dox

@@ -205,6 +205,7 @@ Further non-compatibility-affecting changes include:
  - Added dr_is_detaching(), an API to query whether detach is in progress.
  - Added instr_zeroes_zmmh() that returns true if an instruction clears the
    upper bits of a ZMM register with zeros.
+ - Added instr_clear_label_callback().
 
 **************************************************
 <hr>

core/ir/instr_api.h

@@ -1603,6 +1603,11 @@ DR_API
 void
 instr_set_label_callback(instr_t *instr, instr_label_callback_t func);
 
+DR_API
+/** Removes the callback set by instr_set_label_callback(). */
+void
+instr_clear_label_callback(instr_t *instr);
+
 DR_API
 /**
  * Returns true iff \p instr is an IA-32/AMD64 "mov" instruction: either OP_mov_st,

core/ir/instr_shared.c

@@ -142,11 +142,11 @@ instr_clone(void *drcontext, instr_t *orig)
         instr->bytes =
             (byte *)heap_reachable_alloc(dcontext, instr->length HEAPACCT(ACCT_IR));
         memcpy((void *)instr->bytes, (void *)orig->bytes, instr->length);
-    } else if (instr_is_label(orig)) {
+    } else if (instr_is_label(orig) && instr_get_label_callback(instr) != NULL) {
         /* We don't know what this callback does, we can't copy this. The caller that
          * makes the clone needs to take care of this, xref i#3926.
          */
-        instr_set_label_callback(instr, NULL);
+        instr_clear_label_callback(instr);
     }
     if (orig->num_dsts > 0) { /* checking num_dsts, not dsts, b/c of label data */
         instr->dsts = (opnd_t *)heap_alloc(
@@ -1111,6 +1111,17 @@ instr_set_label_callback(instr_t *instr, instr_label_callback_t cb)
     instr->label_cb = cb;
 }
 
+void
+instr_clear_label_callback(instr_t *instr)
+{
+    CLIENT_ASSERT(instr_is_label(instr),
+                  "only set callback functions for label instructions");
+    CLIENT_ASSERT(instr->label_cb != NULL, "label callback function not set");
+    CLIENT_ASSERT(!TEST(INSTR_RAW_BITS_ALLOCATED, instr->flags),
+                  "instruction's raw bits occupying label callback memory");
+    instr->label_cb = NULL;
+}
+
 instr_label_callback_t
 instr_get_label_callback(instr_t *instr)
 {

suite/tests/client-interface/drx-scattergather.c

@@ -503,10 +503,15 @@ test_avx2_avx512_scatter_gather(void)
         return false;
 #    endif
 #    ifdef __AVX__
-    if (!test_avx2_gather(test_avx2_vpgatherdd, ref_sparse_test_buf,
-                          ref_idx32_val32_xmm_ymm_zmm, test_idx32_vec,
-                          output_xmm_ymm_zmm))
-        return false;
+    /* Run in a loop to trigger trace creation and stress things like cloning
+     * (i#3962).
+     */
+    for (int i = 0; i < 100; i++) {
+        if (!test_avx2_gather(test_avx2_vpgatherdd, ref_sparse_test_buf,
+                              ref_idx32_val32_xmm_ymm_zmm, test_idx32_vec,
+                              output_xmm_ymm_zmm))
+            return false;
+    }
     if (!test_avx2_gather(test_avx2_vgatherdps, ref_sparse_test_buf,
                           ref_idx32_val32_xmm_ymm_zmm, test_idx32_vec,
                           output_xmm_ymm_zmm))
@@ -596,7 +601,6 @@ main(void)
      */
     if (test_avx2_avx512_scatter_gather())
         print("AVX2/AVX-512 scatter/gather checks ok\n");
-
     return 0;
 }
 

suite/tests/client-interface/drx-scattergather.templatex

@@ -25,6 +25,105 @@ AVX2 gather ok
 AVX2 gather ok
 AVX2 gather ok
 AVX2 gather ok
+AVX2 gather ok
+AVX2 gather ok
+AVX2 gather ok
+AVX2 gather ok
+AVX2 gather ok
+AVX2 gather ok
+AVX2 gather ok
+AVX2 gather ok
+AVX2 gather ok
+AVX2 gather ok
+AVX2 gather ok
+AVX2 gather ok
+AVX2 gather ok
+AVX2 gather ok
+AVX2 gather ok
+AVX2 gather ok
+AVX2 gather ok
+AVX2 gather ok
+AVX2 gather ok
+AVX2 gather ok
+AVX2 gather ok
+AVX2 gather ok
+AVX2 gather ok
+AVX2 gather ok
+AVX2 gather ok
+AVX2 gather ok
+AVX2 gather ok
+AVX2 gather ok
+AVX2 gather ok
+AVX2 gather ok
+AVX2 gather ok
+AVX2 gather ok
+AVX2 gather ok
+AVX2 gather ok
+AVX2 gather ok
+AVX2 gather ok
+AVX2 gather ok
+AVX2 gather ok
+AVX2 gather ok
+AVX2 gather ok
+AVX2 gather ok
+AVX2 gather ok
+AVX2 gather ok
+AVX2 gather ok
+AVX2 gather ok
+AVX2 gather ok
+AVX2 gather ok
+AVX2 gather ok
+AVX2 gather ok
+AVX2 gather ok
+AVX2 gather ok
+AVX2 gather ok
+AVX2 gather ok
+AVX2 gather ok
+AVX2 gather ok
+AVX2 gather ok
+AVX2 gather ok
+AVX2 gather ok
+AVX2 gather ok
+AVX2 gather ok
+AVX2 gather ok
+AVX2 gather ok
+AVX2 gather ok
+AVX2 gather ok
+AVX2 gather ok
+AVX2 gather ok
+AVX2 gather ok
+AVX2 gather ok
+AVX2 gather ok
+AVX2 gather ok
+AVX2 gather ok
+AVX2 gather ok
+AVX2 gather ok
+AVX2 gather ok
+AVX2 gather ok
+AVX2 gather ok
+AVX2 gather ok
+AVX2 gather ok
+AVX2 gather ok
+AVX2 gather ok
+AVX2 gather ok
+AVX2 gather ok
+AVX2 gather ok
+AVX2 gather ok
+AVX2 gather ok
+AVX2 gather ok
+AVX2 gather ok
+AVX2 gather ok
+AVX2 gather ok
+AVX2 gather ok
+AVX2 gather ok
+AVX2 gather ok
+AVX2 gather ok
+AVX2 gather ok
+AVX2 gather ok
+AVX2 gather ok
+AVX2 gather ok
+AVX2 gather ok
+AVX2 gather ok
 #endif
 #ifdef UNIX
 #ifdef __AVX512F__
@@ -41,17 +140,17 @@ Test updating the AVX2 gather mask register upon translation events
 AVX2/AVX-512 scatter/gather checks ok
 #ifdef X64
 #ifdef __AVX512F__
-event_exit, 71 scatter/gather instructions
+event_exit, 269 scatter/gather instructions
 #elif defined(__AVX__)
-event_exit, 17 scatter/gather instructions
+event_exit, 215 scatter/gather instructions
 #else
 event_exit, 0 scatter/gather instructions
 #endif
 #else
 #ifdef __AVX512F__
-event_exit, 23 scatter/gather instructions
+event_exit, 221 scatter/gather instructions
 #elif defined(__AVX__)
-event_exit, 5 scatter/gather instructions
+event_exit, 203 scatter/gather instructions
 #else
 event_exit, 0 scatter/gather instructions
 #endif