i#3544 RV64: Assert AUIPC instruction imm alignment (DynamoRIO#6254)

Adds an assert to `encode_u_immpc_opnd` to ensure that the lower 12 bits
of imm are all 0s.

Also, in order to share `.expect` files between static link and dynamic
link tests, we added the ability to ignore specific lines in
runcmp.cmake and removed the dependence on the external `diff` command.
Note that we fall back to `STREQUAL` when the expected file is too
large. We can use ZIP_LISTS to provide better performance, but this
feature requires CMake 3.17, but bumping to 3.17 requires some effort on
the Windows side.

Related: DynamoRIO#6208 
Issue: DynamoRIO#3544

core/ir/aarch64/encode.c

@@ -219,7 +219,7 @@ encoding_possible(decode_info_t *di, instr_t *in, const instr_info_t *ii)
 void
 decode_info_init_for_instr(decode_info_t *di, instr_t *instr)
 {
-    ASSERT_NOT_IMPLEMENTED(false); /* FIXME i#1569 */
+    di->check_reachable = false;
 }
 
 byte *
@@ -265,7 +265,7 @@ instr_encode_arch(dcontext_t *dcontext, instr_t *instr, byte *copy_pc, byte *fin
                     instr_disassemble_to_buffer(dcontext, instr, disas_instr,
                                                 MAX_INSTR_DIS_SZ);
                     SYSLOG_INTERNAL_ERROR("Internal Error: Failed to encode instruction:"
-                                          " '%s'\n",
+                                          " '%s'",
                                           disas_instr);
                 }
             });

core/ir/encode_shared.c

@@ -76,6 +76,7 @@ bool
 instr_is_encoding_possible(instr_t *instr)
 {
     decode_info_t di;
+    decode_info_init_for_instr(&di, instr);
 
     return encoding_possible(&di, instr, NULL);
 }
@@ -99,7 +100,6 @@ get_encoding_info(instr_t *instr)
     const instr_info_t *info = instr_get_instr_info(instr);
     decode_info_t di;
     decode_info_init_for_instr(&di, instr);
-    IF_AARCHXX(di.check_reachable = false;)
 
     while (!encoding_possible(&di, instr, info)) {
         info = get_next_instr_info(info);

core/ir/riscv64/codec.h

@@ -35,6 +35,8 @@
 
 #include <stdint.h>
 
+#include "decode_private.h"
+
 #include "decode.h"
 
 /* Interface to the RISC-V instruction codec (encoder/decoder).

core/ir/riscv64/encode.c

@@ -91,8 +91,7 @@ encoding_possible(decode_info_t *di, instr_t *in, const instr_info_t *ii)
 void
 decode_info_init_for_instr(decode_info_t *di, instr_t *instr)
 {
-    /* FIXME i#3544: Not implemented */
-    ASSERT_NOT_IMPLEMENTED(false);
+    di->check_reachable = false;
 }
 
 byte *
@@ -130,7 +129,7 @@ instr_encode_arch(dcontext_t *dcontext, instr_t *instr, byte *copy_pc, byte *fin
                 instr_disassemble_to_buffer(dcontext, instr, disas_instr,
                                             MAX_INSTR_DIS_SZ);
                 SYSLOG_INTERNAL_ERROR("Internal Error: Failed to encode instruction:"
-                                      " '%s'\n",
+                                      " '%s'",
                                       disas_instr);
             }
         });

suite/tests/CMakeLists.txt

@@ -1459,6 +1459,7 @@ function(torun test key source native standalone_dr dr_ops exe_ops added_out pas
       -D cmp=${CMAKE_CURRENT_BINARY_DIR}/${expectbase}.expect
       -D code=${${key}_code}
       -D capture=${${key}_runcmp_capture}
+      -D ignore_matching_lines=${${key}_runcmp_ignore_matching_lines}
       -P ${runcmp_script})
     # No support for regex here (ctest can't handle large regex)
     set(ALREADY_REGEX ON)
@@ -1893,7 +1894,7 @@ if (NOT ANDROID)
   # XXX i#1874: get working on Android
   tobuild_api(api.ir api/ir_${sfx}.c "" "" OFF OFF OFF)
   if (AARCH64 OR RISCV64)
-    # The ir_aarch64.expect file is too large for CMake's regexes.
+    # The ir_{aarch64,riscv64}.expect file is too large for CMake's regexes.
     set(api.ir_runcmp "${CMAKE_CURRENT_SOURCE_DIR}/runcmp.cmake")
     set(api.ir_runcmp_capture "stderr")
   endif ()
@@ -1937,10 +1938,16 @@ endif ()
 
 # test static decoder library
 tobuild_api(api.ir-static api/ir_${sfx}.c "" "" ON OFF OFF)
-if (AARCH64 OR RISCV64)
+if (AARCH64)
   # The ir_aarch64.expect file is too large for CMake's regexes.
   set(api.ir-static_runcmp "${CMAKE_CURRENT_SOURCE_DIR}/runcmp.cmake")
   set(api.ir-static_runcmp_capture "stderr")
+elseif (RISCV64)
+  # The ir_riscv64.expect file is too large for CMake's regexes.
+  set(api.ir-static_runcmp "${CMAKE_CURRENT_SOURCE_DIR}/runcmp.cmake")
+  set(api.ir-static_runcmp_capture "stderr")
+  # Internal error lines are ignored as they are not printed in static builds.
+  set(api.ir-static_runcmp_ignore_matching_lines "^<.+>")
 endif ()
 
 if (ARM)

suite/tests/api/ir_riscv64.c

@@ -91,6 +91,16 @@ test_instr_encoding(void *dc, uint opcode, instr_t *instr)
     return pc;
 }
 
+static void
+test_instr_encoding_failure(void *dc, uint opcode, app_pc instr_pc, instr_t *instr)
+{
+    byte *pc;
+
+    pc = instr_encode_to_copy(dc, instr, buf, instr_pc);
+    ASSERT(pc == NULL);
+    instr_destroy(dc, instr);
+}
+
 static void
 test_instr_encoding_jal_or_branch(void *dc, uint opcode, instr_t *instr)
 {
@@ -1104,6 +1114,12 @@ test_jump_and_branch(void *dc)
     instr = INSTR_CREATE_auipc(dc, opnd_create_reg(DR_REG_A0),
                                opnd_create_pc(pc + (3 << 12)));
     test_instr_encoding_auipc(dc, OP_auipc, pc, instr);
+
+    instr = INSTR_CREATE_auipc(dc, opnd_create_reg(DR_REG_A0),
+                               opnd_create_pc(pc + (3 << 12)));
+    /* This is expected to fail since we are using an unaligned PC (i.e. target_pc -
+     * instr_encode_pc has non-zero lower 12 bits). */
+    test_instr_encoding_failure(dc, OP_auipc, pc + 4, instr);
     instr = INSTR_CREATE_jal(dc, opnd_create_reg(DR_REG_A0), opnd_create_pc(pc));
     test_instr_encoding_jal_or_branch(dc, OP_jal, instr);
     instr = INSTR_CREATE_jalr(dc, opnd_create_reg(DR_REG_A0), opnd_create_reg(DR_REG_A1),

suite/tests/api/ir_riscv64.expect

@@ -221,6 +221,7 @@ c.xor  a5 -> fp
 c.sub  a5 -> fp
 test_integer_arith complete
 lui    0x2a -> a0
+<Internal Error: Failed to encode instruction: 'auipc  0x0000004000016234 -> a0'>
 jalr   a1 0x2a -> a0
 c.li   31 -> a1
 c.lui  1 -> a1

suite/tests/runcmp.cmake

@@ -36,6 +36,7 @@
 #     should have intra-arg space=@@ and inter-arg space=@ and ;=!
 # * cmp = file containing output to compare stdout to
 # * capture = if "stderr", captures stderr instead of stdout
+# * ignore_matching_lines = optional regex pattern, matching lines will not be diffed
 
 # intra-arg space=@@ and inter-arg space=@
 string(REGEX REPLACE "@@" " " cmd "${cmd}")
@@ -51,38 +52,64 @@ if (cmd_result)
   message(FATAL_ERROR "*** ${cmd} failed (${cmd_result}): ${cmd_err}***\n")
 endif (cmd_result)
 
-# get expected output
-# we assume it has already been processed w/ regex => literal, etc.
-file(READ "${cmp}" str)
-
-# We do not support regex b/c ctest can't handle big regex:
-#   "RegularExpression::compile(): Expression too big."
-# so we use STREQUAL.
-
-# We used to implement a "diff -b" via cmake regex on the output and expect
-# file, but that gets really slow (30s for 750KB strings) so now we require
-# strict matches.
-
 set(output "${cmd_out}")
 if ("${capture}" STREQUAL "stderr")
   set(output "${cmd_err}")
 endif ()
 
-if (NOT "${output}" STREQUAL "${str}")
-  # make it easier to debug
-  set(tmp "${cmp}-out")
-  file(WRITE "${tmp}" "${output}")
-  set(tmp2 "${cmp}-expect")
-  file(WRITE "${tmp2}" "${str}")
+# Get expected output, we assume it has already been processed w/
+# regex => literal, etc.
+file(READ "${cmp}" expect)
 
-  set(diffcmd "diff")
-  execute_process(COMMAND ${diffcmd} ${tmp} ${tmp2}
-    RESULT_VARIABLE dcmd_result
-    ERROR_VARIABLE dcmd_err
-    OUTPUT_VARIABLE dcmd_out)
+# Convert file contents into a CMake list, where each element in the list is one
+# line of the file.
+string(REGEX MATCHALL "([^\n]+)\n" output "${output}")
+string(REGEX MATCHALL "([^\n]+)\n" expect "${expect}")
 
-  message(STATUS "diff: ${dcmd_out}")
+if (NOT "${ignore_matching_lines}" STREQUAL "")
+  # Filter out all the ignored lines.
+  set(filtered_expect)
+  foreach (item ${expect})
+    if (NOT "${item}" MATCHES "${ignore_matching_lines}")
+      list(APPEND filtered_expect ${item})
+    endif ()
+  endforeach ()
+else ()
+  set(filtered_expect ${expect})
+endif ()
+
+list(LENGTH filtered_expect filtered_expect_length)
+list(LENGTH output output_length)
+
+set(lists_identical TRUE)
+if (NOT ${filtered_expect_length} EQUAL ${output_length})
+  set(lists_identical FALSE)
+endif ()
 
-  message(FATAL_ERROR "output in ${tmp} failed to match expected output in ${tmp2}")
+if (lists_identical)
+  if (filtered_expect_length GREATER "1000")
+    # CMake list(GET ...) operation is slow, do STREQUAL for large files, give up
+    # the diff output.
+    # XXX: Use ZIP_LISTS for better performance, which requires CMake 3.17.
+    if(NOT filtered_expect STREQUAL output)
+      set(lists_identical FALSE)
+    endif ()
+  else ()
+    math(EXPR len "${filtered_expect_length} - 1")
+    foreach (index RANGE ${len})
+      list(GET filtered_expect ${index} item1)
+      list(GET output ${index} item2)
+      if (NOT ${item1} STREQUAL ${item2})
+          set(lists_identical FALSE)
+          message(STATUS "The first difference:")
+          message(STATUS "< ${item1}")
+          message(STATUS "> ${item2}")
+          break ()
+      endif ()
+    endforeach ()
+  endif ()
+endif (lists_identical)
 
+if (NOT lists_identical)
+  message(FATAL_ERROR "failed to match expected output")
 endif ()