From 925d9e6057946b5034ab20af3fed6d2042a2fe1c Mon Sep 17 00:00:00 2001 From: Kaiyeung Luk Date: Mon, 27 Nov 2023 21:42:30 +0000 Subject: [PATCH 1/4] i#6417: restore output register for syscall. --- ext/drreg/drreg.c | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/ext/drreg/drreg.c b/ext/drreg/drreg.c index 1ed010e2a6e..303729df31e 100644 --- a/ext/drreg/drreg.c +++ b/ext/drreg/drreg.c @@ -385,14 +385,20 @@ drreg_event_bb_analysis(void *drcontext, void *tag, instrlist_t *bb, bool for_tr /* DRi#1849: COND_SRCS here includes addressing regs in dsts */ if (instr_reads_from_reg(inst, reg, DR_QUERY_INCLUDE_COND_SRCS)) value = REG_LIVE; - /* make sure we don't consider writes to sub-regs */ - else if (instr_writes_to_exact_reg(inst, reg, DR_QUERY_INCLUDE_COND_SRCS) - /* A write to a 32-bit reg zeroes the top 32 bits for x86_64 and - * aarch64. - */ - IF_X64(|| - instr_writes_to_exact_reg(inst, reg_64_to_32(reg), - DR_QUERY_INCLUDE_COND_SRCS))) + /* Make sure we don't consider writes to sub-regs. i#6417: in case + * of a syscall, restore the value of the output register since it + * might be used as an input parameter for the kernel. For example, + * ECX is used as an input parameter for mmap2 for length, as well + * as the output parameter. + */ + else if (!instr_is_syscall(inst) && + (instr_writes_to_exact_reg(inst, reg, DR_QUERY_INCLUDE_COND_SRCS) + /* A write to a 32-bit reg zeroes the top 32 bits for x86_64 and + * aarch64. + */ + IF_X64(|| + instr_writes_to_exact_reg(inst, reg_64_to_32(reg), + DR_QUERY_INCLUDE_COND_SRCS)))) value = REG_DEAD; else if (xfer) value = REG_LIVE; From 9bba6036d14c57c6bfcd3a6a5ffcf92de35f6d69 Mon Sep 17 00:00:00 2001 From: Kaiyeung Luk Date: Tue, 5 Dec 2023 18:50:21 +0000 Subject: [PATCH 2/4] Incorporate review comments. --- .../offline-syscall-mmap-aarch64.templatex | 13 +++++ .../offline-syscall-mmap-x86-x32.templatex | 13 +++++ .../offline-syscall-mmap-x86-x64.templatex | 13 +++++ ext/drreg/drreg.c | 40 +++++++++------ suite/tests/CMakeLists.txt | 19 ++++++- suite/tests/common/syscall-mmap.c | 49 +++++++++++++++++++ suite/tests/common/syscall-mmap.expect | 1 + 7 files changed, 132 insertions(+), 16 deletions(-) create mode 100644 clients/drcachesim/tests/offline-syscall-mmap-aarch64.templatex create mode 100644 clients/drcachesim/tests/offline-syscall-mmap-x86-x32.templatex create mode 100644 clients/drcachesim/tests/offline-syscall-mmap-x86-x64.templatex create mode 100644 suite/tests/common/syscall-mmap.c create mode 100644 suite/tests/common/syscall-mmap.expect diff --git a/clients/drcachesim/tests/offline-syscall-mmap-aarch64.templatex b/clients/drcachesim/tests/offline-syscall-mmap-aarch64.templatex new file mode 100644 index 00000000000..84fdd1b80e6 --- /dev/null +++ b/clients/drcachesim/tests/offline-syscall-mmap-aarch64.templatex @@ -0,0 +1,13 @@ +Calling mmap\(0, 0x8765, 0x7, 0x22, -1, 0\) +Output format: +<--record#-> <--instr#->: <---tid---> +------------------------------------------------------------ +.* + +[0-9]+ +[0-9]+: +[0-9]+ + +[0-9]+ +[0-9]+: +[0-9]+ + +[0-9]+ +[0-9]+: +[0-9]+ + +[0-9]+ +[0-9]+: +[0-9]+ + +[0-9]+ +[0-9]+: +[0-9]+ + +[0-9]+ +[0-9]+: +[0-9]+ + +[0-9]+ +[0-9]+: +[0-9]+ +.* diff --git a/clients/drcachesim/tests/offline-syscall-mmap-x86-x32.templatex b/clients/drcachesim/tests/offline-syscall-mmap-x86-x32.templatex new file mode 100644 index 00000000000..8b8386e5f9e --- /dev/null +++ b/clients/drcachesim/tests/offline-syscall-mmap-x86-x32.templatex @@ -0,0 +1,13 @@ +Calling mmap\(0, 0x8765, 0x7, 0x22, -1, 0\) +Output format: +<--record#-> <--instr#->: <---tid---> +------------------------------------------------------------ +.* + +[0-9]+ +[0-9]+: +[0-9]+ + +[0-9]+ +[0-9]+: +[0-9]+ + +[0-9]+ +[0-9]+: +[0-9]+ + +[0-9]+ +[0-9]+: +[0-9]+ + +[0-9]+ +[0-9]+: +[0-9]+ + +[0-9]+ +[0-9]+: +[0-9]+ + +[0-9]+ +[0-9]+: +[0-9]+ +.* diff --git a/clients/drcachesim/tests/offline-syscall-mmap-x86-x64.templatex b/clients/drcachesim/tests/offline-syscall-mmap-x86-x64.templatex new file mode 100644 index 00000000000..29228f89c8c --- /dev/null +++ b/clients/drcachesim/tests/offline-syscall-mmap-x86-x64.templatex @@ -0,0 +1,13 @@ +Calling mmap\(0, 0x8765, 0x7, 0x22, -1, 0\) +Output format: +<--record#-> <--instr#->: <---tid---> +------------------------------------------------------------ +.* + +[0-9]+ +[0-9]+: +[0-9]+ + +[0-9]+ +[0-9]+: +[0-9]+ + +[0-9]+ +[0-9]+: +[0-9]+ + +[0-9]+ +[0-9]+: +[0-9]+ + +[0-9]+ +[0-9]+: +[0-9]+ + +[0-9]+ +[0-9]+: +[0-9]+ + +[0-9]+ +[0-9]+: +[0-9]+ +.* diff --git a/ext/drreg/drreg.c b/ext/drreg/drreg.c index 303729df31e..64dd6b03d21 100644 --- a/ext/drreg/drreg.c +++ b/ext/drreg/drreg.c @@ -385,20 +385,14 @@ drreg_event_bb_analysis(void *drcontext, void *tag, instrlist_t *bb, bool for_tr /* DRi#1849: COND_SRCS here includes addressing regs in dsts */ if (instr_reads_from_reg(inst, reg, DR_QUERY_INCLUDE_COND_SRCS)) value = REG_LIVE; - /* Make sure we don't consider writes to sub-regs. i#6417: in case - * of a syscall, restore the value of the output register since it - * might be used as an input parameter for the kernel. For example, - * ECX is used as an input parameter for mmap2 for length, as well - * as the output parameter. - */ - else if (!instr_is_syscall(inst) && - (instr_writes_to_exact_reg(inst, reg, DR_QUERY_INCLUDE_COND_SRCS) - /* A write to a 32-bit reg zeroes the top 32 bits for x86_64 and - * aarch64. - */ - IF_X64(|| - instr_writes_to_exact_reg(inst, reg_64_to_32(reg), - DR_QUERY_INCLUDE_COND_SRCS)))) + /* make sure we don't consider writes to sub-regs */ + else if (instr_writes_to_exact_reg(inst, reg, DR_QUERY_INCLUDE_COND_SRCS) + /* A write to a 32-bit reg zeroes the top 32 bits for x86_64 and + * aarch64. + */ + IF_X64(|| + instr_writes_to_exact_reg(inst, reg_64_to_32(reg), + DR_QUERY_INCLUDE_COND_SRCS))) value = REG_DEAD; else if (xfer) value = REG_LIVE; @@ -492,6 +486,11 @@ drreg_insert_restore_all(void *drcontext, instrlist_t *bb, instr_t *inst, (instr_is_label(inst) && ((ptr_uint_t)instr_get_note(inst) == DR_NOTE_ANNOTATION || (ptr_uint_t)instr_get_note(inst) == DR_NOTE_REG_BARRIER)) || + /* i#6417: in case of a syscall, restore the values of registers since + * they may be used as input parameters for the kernel. For example, + * ECX is used to store the length for mmap2. + */ + instr_is_syscall(inst) || /* DR slots are not guaranteed across app instrs */ (pt->aflags.slot != MAX_SPILLS && pt->aflags.slot >= (int)ops.num_spill_slots))) { @@ -537,6 +536,11 @@ drreg_insert_restore_all(void *drcontext, instrlist_t *bb, instr_t *inst, (instr_is_label(inst) && ((ptr_uint_t)instr_get_note(inst) == DR_NOTE_ANNOTATION || (ptr_uint_t)instr_get_note(inst) == DR_NOTE_REG_BARRIER)) || + /* i#6417: in case of a syscall, restore the values of registers since + * they may be used as input parameters for the kernel. For example, + * ECX is used to store the length for mmap2. + */ + instr_is_syscall(inst) || /* Treat a partial write as a read, to restore rest of reg */ (instr_writes_to_reg(inst, reg, DR_QUERY_INCLUDE_ALL) && !instr_writes_to_exact_reg(inst, reg, DR_QUERY_INCLUDE_ALL)) || @@ -1351,7 +1355,13 @@ static drreg_status_t drreg_restore_reg_now(void *drcontext, instrlist_t *ilist, instr_t *inst, per_thread_t *pt, reg_id_t reg) { - if (pt->reg[GPR_IDX(reg)].ever_spilled) { + /* i#6417: in case of a syscall, restore the values of registers since + * they may be used as input parameters for the kernel. For example, + * ECX is used to store the length for mmap2. Register used to store the + * output of the syscall is marked as not spilled, so we need to check for + * syscall as well. + */ + if (pt->reg[GPR_IDX(reg)].ever_spilled || instr_is_syscall(inst)) { if (pt->reg[GPR_IDX(reg)].xchg != DR_REG_NULL) { /* XXX i#511: NYI */ return DRREG_ERROR_FEATURE_NOT_AVAILABLE; diff --git a/suite/tests/CMakeLists.txt b/suite/tests/CMakeLists.txt index 43853c9df61..15a22e71653 100644 --- a/suite/tests/CMakeLists.txt +++ b/suite/tests/CMakeLists.txt @@ -491,7 +491,7 @@ endfunction(append_link_flags) set(sve_tests simple_app api.ir api.ir_negative api.ir_v81 api.ir_v82 api.ir_v83 api.ir_v84 api.ir_v86 api.ir_sve api.ir_sve2 api.ir-static api.drdecode common.broadfun - common.fib common.nzcv common.getretaddr common.segfault + common.fib common.nzcv common.getretaddr common.segfault common/syscall-mmap common.allasm_aarch64_isa common.allasm_aarch64_cache allasm_aarch64_prefetch allasm_aarch64_flush libutil.frontend_test libutil.drconfig_test client.call-retarget client.modules client.annotation-concurrency @@ -2145,6 +2145,7 @@ if (X86) # FIXME i#1551, i#1569: port asm to ARM and AArch64 torunonly(common.floatpc_xl8all common.floatpc common/floatpc.c "-translate_fpu_pc" "") endif (X86) tobuild(common.getretaddr common/getretaddr.c) +tobuild(common.syscall-mmap common/syscall-mmap.c) if (X86) # FIXME i#1551, i#1569: port asm to ARM and AArch64 # XXX i#1287: implement for MacOS @@ -4600,6 +4601,22 @@ if (BUILD_CLIENTS) "@-simulator_type@invariant_checker" "") endif () + if (X86) + if (X64) + torunonly_drcacheoff(syscall-mmap-x86-x64 common.syscall-mmap + "-record_replace_retaddr -record_syscall 9|6" + "@-simulator_type@view" "") + else () + torunonly_drcacheoff(syscall-mmap-x86-x32 common.syscall-mmap + "-record_replace_retaddr -record_syscall 192|6" + "@-simulator_type@view" "") + endif () + elseif (AARCH64) + torunonly_drcacheoff(syscall-mmap-aarch64 common.syscall-mmap + "-record_replace_retaddr -record_syscall 90|6" + "@-simulator_type@view" "") + endif() + ########################################################################### # drcpusim tests diff --git a/suite/tests/common/syscall-mmap.c b/suite/tests/common/syscall-mmap.c new file mode 100644 index 00000000000..01196d7d366 --- /dev/null +++ b/suite/tests/common/syscall-mmap.c @@ -0,0 +1,49 @@ +/* ********************************************************** + * Copyright (c) 2023 Google, Inc. All rights reserved. + * **********************************************************/ + +/* + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are met: + * + * * Redistributions of source code must retain the above copyright notice, + * this list of conditions and the following disclaimer. + * + * * Redistributions in binary form must reproduce the above copyright notice, + * this list of conditions and the following disclaimer in the documentation + * and/or other materials provided with the distribution. + * + * * Neither the name of VMware, Inc. nor the names of its contributors may be + * used to endorse or promote products derived from this software without + * specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE + * ARE DISCLAIMED. IN NO EVENT SHALL VMWARE, INC. OR CONTRIBUTORS BE LIABLE + * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL + * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR + * SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER + * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT + * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY + * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH + * DAMAGE. + */ + +#include +#include + +int +main() +{ + void *p; + const size_t size = 0x00008765; + printf("Calling mmap(0, 0x%x, 0x%x, 0x%x, -1, 0)\n", size, + PROT_EXEC | PROT_READ | PROT_WRITE, MAP_ANON | MAP_PRIVATE); + p = mmap(0, size, PROT_EXEC | PROT_READ | PROT_WRITE, MAP_ANON | MAP_PRIVATE, -1, 0); + if (p == MAP_FAILED) { + printf("mmap ERROR 0x%p\n", p); + return 1; + } + return 0; +} diff --git a/suite/tests/common/syscall-mmap.expect b/suite/tests/common/syscall-mmap.expect new file mode 100644 index 00000000000..86fe565ac58 --- /dev/null +++ b/suite/tests/common/syscall-mmap.expect @@ -0,0 +1 @@ +Calling mmap(0, 0x8765, 0x7, 0x22, -1, 0) From c6cf84bcb22efc07fb1de8ee41f6ec9b9d9e0dd2 Mon Sep 17 00:00:00 2001 From: Kaiyeung Luk Date: Tue, 5 Dec 2023 19:33:28 +0000 Subject: [PATCH 3/4] Fix CI test breakages. --- suite/tests/CMakeLists.txt | 37 ++++++++++++++++++------------- suite/tests/common/syscall-mmap.c | 2 +- 2 files changed, 22 insertions(+), 17 deletions(-) diff --git a/suite/tests/CMakeLists.txt b/suite/tests/CMakeLists.txt index 15a22e71653..c87b4f73774 100644 --- a/suite/tests/CMakeLists.txt +++ b/suite/tests/CMakeLists.txt @@ -491,7 +491,7 @@ endfunction(append_link_flags) set(sve_tests simple_app api.ir api.ir_negative api.ir_v81 api.ir_v82 api.ir_v83 api.ir_v84 api.ir_v86 api.ir_sve api.ir_sve2 api.ir-static api.drdecode common.broadfun - common.fib common.nzcv common.getretaddr common.segfault common/syscall-mmap + common.fib common.nzcv common.getretaddr common.segfault common.allasm_aarch64_isa common.allasm_aarch64_cache allasm_aarch64_prefetch allasm_aarch64_flush libutil.frontend_test libutil.drconfig_test client.call-retarget client.modules client.annotation-concurrency @@ -2145,7 +2145,10 @@ if (X86) # FIXME i#1551, i#1569: port asm to ARM and AArch64 torunonly(common.floatpc_xl8all common.floatpc common/floatpc.c "-translate_fpu_pc" "") endif (X86) tobuild(common.getretaddr common/getretaddr.c) -tobuild(common.syscall-mmap common/syscall-mmap.c) + +if (UNIX) + tobuild(common.syscall-mmap common/syscall-mmap.c) +endif () if (X86) # FIXME i#1551, i#1569: port asm to ARM and AArch64 # XXX i#1287: implement for MacOS @@ -4601,21 +4604,23 @@ if (BUILD_CLIENTS) "@-simulator_type@invariant_checker" "") endif () - if (X86) - if (X64) - torunonly_drcacheoff(syscall-mmap-x86-x64 common.syscall-mmap - "-record_replace_retaddr -record_syscall 9|6" - "@-simulator_type@view" "") - else () - torunonly_drcacheoff(syscall-mmap-x86-x32 common.syscall-mmap - "-record_replace_retaddr -record_syscall 192|6" - "@-simulator_type@view" "") + if (UNIX) + if (X86) + if (X64) + torunonly_drcacheoff(syscall-mmap-x86-x64 common.syscall-mmap + "-record_replace_retaddr -record_syscall 9|6" + "@-simulator_type@view" "") + else () + torunonly_drcacheoff(syscall-mmap-x86-x32 common.syscall-mmap + "-record_replace_retaddr -record_syscall 192|6" + "@-simulator_type@view" "") + endif () + elseif (AARCH64) + torunonly_drcacheoff(syscall-mmap-aarch64 common.syscall-mmap + "-record_replace_retaddr -record_syscall 90|6" + "@-simulator_type@view" "") endif () - elseif (AARCH64) - torunonly_drcacheoff(syscall-mmap-aarch64 common.syscall-mmap - "-record_replace_retaddr -record_syscall 90|6" - "@-simulator_type@view" "") - endif() + endif () ########################################################################### # drcpusim tests diff --git a/suite/tests/common/syscall-mmap.c b/suite/tests/common/syscall-mmap.c index 01196d7d366..e78f4d033ee 100644 --- a/suite/tests/common/syscall-mmap.c +++ b/suite/tests/common/syscall-mmap.c @@ -38,7 +38,7 @@ main() { void *p; const size_t size = 0x00008765; - printf("Calling mmap(0, 0x%x, 0x%x, 0x%x, -1, 0)\n", size, + printf("Calling mmap(0, 0x%x, 0x%x, 0x%x, -1, 0)\n", (unsigned int)size, PROT_EXEC | PROT_READ | PROT_WRITE, MAP_ANON | MAP_PRIVATE); p = mmap(0, size, PROT_EXEC | PROT_READ | PROT_WRITE, MAP_ANON | MAP_PRIVATE, -1, 0); if (p == MAP_FAILED) { From 185cd632645919ccc8b891c216d9db212822baa1 Mon Sep 17 00:00:00 2001 From: Kaiyeung Luk Date: Tue, 5 Dec 2023 21:49:28 +0000 Subject: [PATCH 4/4] Rename x86 templatex files, and fix Aarch64 syscall number. --- .../tests/offline-syscall-mmap-aarch64.templatex | 4 ++-- ...atex => offline-syscall-mmap-x86-32.templatex} | 0 ...atex => offline-syscall-mmap-x86-64.templatex} | 0 suite/tests/CMakeLists.txt | 15 ++++++--------- 4 files changed, 8 insertions(+), 11 deletions(-) rename clients/drcachesim/tests/{offline-syscall-mmap-x86-x32.templatex => offline-syscall-mmap-x86-32.templatex} (100%) rename clients/drcachesim/tests/{offline-syscall-mmap-x86-x64.templatex => offline-syscall-mmap-x86-64.templatex} (100%) diff --git a/clients/drcachesim/tests/offline-syscall-mmap-aarch64.templatex b/clients/drcachesim/tests/offline-syscall-mmap-aarch64.templatex index 84fdd1b80e6..1bdd4965302 100644 --- a/clients/drcachesim/tests/offline-syscall-mmap-aarch64.templatex +++ b/clients/drcachesim/tests/offline-syscall-mmap-aarch64.templatex @@ -3,11 +3,11 @@ Output format: <--record#-> <--instr#->: <---tid---> ------------------------------------------------------------ .* - +[0-9]+ +[0-9]+: +[0-9]+ + +[0-9]+ +[0-9]+: +[0-9]+ +[0-9]+ +[0-9]+: +[0-9]+ +[0-9]+ +[0-9]+: +[0-9]+ +[0-9]+ +[0-9]+: +[0-9]+ +[0-9]+ +[0-9]+: +[0-9]+ - +[0-9]+ +[0-9]+: +[0-9]+ + +[0-9]+ +[0-9]+: +[0-9]+ +[0-9]+ +[0-9]+: +[0-9]+ .* diff --git a/clients/drcachesim/tests/offline-syscall-mmap-x86-x32.templatex b/clients/drcachesim/tests/offline-syscall-mmap-x86-32.templatex similarity index 100% rename from clients/drcachesim/tests/offline-syscall-mmap-x86-x32.templatex rename to clients/drcachesim/tests/offline-syscall-mmap-x86-32.templatex diff --git a/clients/drcachesim/tests/offline-syscall-mmap-x86-x64.templatex b/clients/drcachesim/tests/offline-syscall-mmap-x86-64.templatex similarity index 100% rename from clients/drcachesim/tests/offline-syscall-mmap-x86-x64.templatex rename to clients/drcachesim/tests/offline-syscall-mmap-x86-64.templatex diff --git a/suite/tests/CMakeLists.txt b/suite/tests/CMakeLists.txt index c87b4f73774..09b06db527b 100644 --- a/suite/tests/CMakeLists.txt +++ b/suite/tests/CMakeLists.txt @@ -4607,18 +4607,15 @@ if (BUILD_CLIENTS) if (UNIX) if (X86) if (X64) - torunonly_drcacheoff(syscall-mmap-x86-x64 common.syscall-mmap - "-record_replace_retaddr -record_syscall 9|6" - "@-simulator_type@view" "") + torunonly_drcacheoff(syscall-mmap-x86-64 common.syscall-mmap + "-record_syscall 9|6" "@-simulator_type@view" "") else () - torunonly_drcacheoff(syscall-mmap-x86-x32 common.syscall-mmap - "-record_replace_retaddr -record_syscall 192|6" - "@-simulator_type@view" "") + torunonly_drcacheoff(syscall-mmap-x86-32 common.syscall-mmap + "-record_syscall 192|6" "@-simulator_type@view" "") endif () elseif (AARCH64) - torunonly_drcacheoff(syscall-mmap-aarch64 common.syscall-mmap - "-record_replace_retaddr -record_syscall 90|6" - "@-simulator_type@view" "") + torunonly_drcacheoff(syscall-mmap-aarch64 common.syscall-mmap + "-record_syscall 222|6" "@-simulator_type@view" "") endif () endif ()