From patchwork Wed Nov 29 00:42:56 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Pedro Alves X-Patchwork-Id: 24585 Received: (qmail 46965 invoked by alias); 29 Nov 2017 00:43:02 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 46945 invoked by uid 89); 29 Nov 2017 00:43:02 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.7 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_SHORT, KB_WAM_FROM_NAME_SINGLEWORD, SPF_HELO_PASS, T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 spammy=sans, hlt, wag X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 29 Nov 2017 00:42:59 +0000 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 664DF7E42A for ; Wed, 29 Nov 2017 00:42:58 +0000 (UTC) Received: from cascais.lan (ovpn04.gateway.prod.ext.ams2.redhat.com [10.39.146.4]) by smtp.corp.redhat.com (Postfix) with ESMTP id ACFCD600D5 for ; Wed, 29 Nov 2017 00:42:57 +0000 (UTC) From: Pedro Alves To: gdb-patches@sourceware.org Subject: [PATCH] Fix displaced-stepping RIP-relative VEX-encoded instructions (AVX) (PR gdb/22499) Date: Wed, 29 Nov 2017 00:42:56 +0000 Message-Id: <1511916176-5271-1-git-send-email-palves@redhat.com> PR gdb/22499 is about a latent bug exposed by the switch to "maint set target-non-stop on" by default on x86-64 GNU/Linux, a while ago. With that on, GDB is also preferring to use displaced-stepping by default. The testcase in the bug is failing because GDB ends up incorrectly displaced-stepping over a RIP-relative VEX-encoded instruction, like this: 0x00000000004007f5 <+15>: c5 fb 10 05 8b 01 00 00 vmovsd 0x18b(%rip),%xmm0 # 0x400988 While RIP-relative instructions need adjustment when relocated to the scratch pad, GDB ends up just copying VEX-encoded instructions to the scratch pad unmodified, with the end result that the inferior ends up executing an instruction that fetches/writes memory from the wrong address... This patch teaches GDB about the VEX-encoding prefixes, fixing the problem, and adds a testcase that fails without the GDB fix. I think we may need a similar treatment for EVEX-encoded instructions, but I didn't address that simply because I couldn't find any EVEX-encoded RIP-relative instruction in the gas testsuite. In any case, this commit is forward progress as-is already. gdb/ChangeLog: yyyy-mm-dd Pedro Alves PR gdb/22499 * amd64-tdep.c (amd64_insn::rex_offset): Rename to... (amd64_insn::enc_prefix_offset): ... this, and tweak comment. (vex2_prefix_p, vex3_prefix_p): New functions. (amd64_get_insn_details): Adjust to rename. Also skip VEX2 and VEX3 prefixes. (fixup_riprel): Set VEX3.!B. gdb/testsuite/ChangeLog: yyyy-mm-dd Pedro Alves PR gdb/22499 * gdb.arch/amd64-disp-step-avx.S: New file. * gdb.arch/amd64-disp-step-avx.exp: New file. --- gdb/amd64-tdep.c | 58 +++++++++-- gdb/testsuite/gdb.arch/amd64-disp-step-avx.S | 70 +++++++++++++ gdb/testsuite/gdb.arch/amd64-disp-step-avx.exp | 139 +++++++++++++++++++++++++ 3 files changed, 258 insertions(+), 9 deletions(-) create mode 100644 gdb/testsuite/gdb.arch/amd64-disp-step-avx.S create mode 100644 gdb/testsuite/gdb.arch/amd64-disp-step-avx.exp diff --git a/gdb/amd64-tdep.c b/gdb/amd64-tdep.c index bcd37ef..0eb3670 100644 --- a/gdb/amd64-tdep.c +++ b/gdb/amd64-tdep.c @@ -1037,8 +1037,9 @@ struct amd64_insn { /* The number of opcode bytes. */ int opcode_len; - /* The offset of the rex prefix or -1 if not present. */ - int rex_offset; + /* The offset of the REX/VEX instruction encoding prefix or -1 if + not present. */ + int enc_prefix_offset; /* The offset to the first opcode byte. */ int opcode_offset; /* The offset to the modrm byte or -1 if not present. */ @@ -1124,6 +1125,22 @@ rex_prefix_p (gdb_byte pfx) return REX_PREFIX_P (pfx); } +/* True if PFX is the start of the 2-byte VEX prefix. */ + +static bool +vex2_prefix_p (gdb_byte pfx) +{ + return pfx == 0xc5; +} + +/* True if PFX is the start of the 3-byte VEX prefix. */ + +static bool +vex3_prefix_p (gdb_byte pfx) +{ + return pfx == 0xc4; +} + /* Skip the legacy instruction prefixes in INSN. We assume INSN is properly sentineled so we don't have to worry about falling off the end of the buffer. */ @@ -1242,19 +1259,30 @@ amd64_get_insn_details (gdb_byte *insn, struct amd64_insn *details) details->raw_insn = insn; details->opcode_len = -1; - details->rex_offset = -1; + details->enc_prefix_offset = -1; details->opcode_offset = -1; details->modrm_offset = -1; /* Skip legacy instruction prefixes. */ insn = amd64_skip_prefixes (insn); - /* Skip REX instruction prefix. */ + /* Skip REX/VEX instruction encoding prefixes. */ if (rex_prefix_p (*insn)) { - details->rex_offset = insn - start; + details->enc_prefix_offset = insn - start; ++insn; } + else if (vex2_prefix_p (*insn)) + { + /* Don't record the offset in this case because this prefix has + no REX.B equivalent. */ + insn += 2; + } + else if (vex3_prefix_p (*insn)) + { + details->enc_prefix_offset = insn - start; + insn += 3; + } details->opcode_offset = insn - start; @@ -1329,10 +1357,22 @@ fixup_riprel (struct gdbarch *gdbarch, amd64_displaced_step_closure *dsc, arch_tmp_regno = amd64_get_unused_input_int_reg (insn_details); tmp_regno = amd64_arch_reg_to_regnum (arch_tmp_regno); - /* REX.B should be unset as we were using rip-relative addressing, - but ensure it's unset anyway, tmp_regno is not r8-r15. */ - if (insn_details->rex_offset != -1) - dsc->insn_buf[insn_details->rex_offset] &= ~REX_B; + /* Position of the not-B bit in the 3-byte VEX prefix (in byte 1). */ + static constexpr gdb_byte VEX3_NOT_B = 0x20; + + /* REX.B should be unset (VEX.!B set) as we were using rip-relative + addressing, but ensure it's unset (set for VEX) anyway, tmp_regno + is not r8-r15. */ + if (insn_details->enc_prefix_offset != -1) + { + gdb_byte *pfx = &dsc->insn_buf[insn_details->enc_prefix_offset]; + if (rex_prefix_p (pfx[0])) + pfx[0] &= ~REX_B; + else if (vex3_prefix_p (pfx[0])) + pfx[1] |= VEX3_NOT_B; + else + gdb_assert_not_reached ("unhandled prefix"); + } regcache_cooked_read_unsigned (regs, tmp_regno, &orig_value); dsc->tmp_regno = tmp_regno; diff --git a/gdb/testsuite/gdb.arch/amd64-disp-step-avx.S b/gdb/testsuite/gdb.arch/amd64-disp-step-avx.S new file mode 100644 index 0000000..5677717 --- /dev/null +++ b/gdb/testsuite/gdb.arch/amd64-disp-step-avx.S @@ -0,0 +1,70 @@ +/* Copyright 2009-2017 Free Software Foundation, Inc. + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see . + + This file is part of the gdb testsuite. + + Test displaced stepping over VEX-encoded RIP-relative AVX + instructions. */ + + .text + + .global main +main: + nop + +/***********************************************/ + +/* Test a VEX2-encoded RIP-relative instruction. */ + + .global test_rip_vex2 +test_rip_vex2: + vmovsd ro_var(%rip),%xmm0 + .global test_rip_vex2 +test_rip_vex2_end: + nop + +/* Test a VEX3-encoded RIP-relative instruction. */ + + .global test_rip_vex3 +test_rip_vex3: + vextractf128 $0x0,%ymm0,var128(%rip) + .global test_rip_vex3 +test_rip_vex3_end: + nop + + /* skip over test data */ + jmp done + +/* RIP-relative ro-data for VEX2 test above. */ + +ro_var: + .8byte 0x1122334455667788 + .8byte 0x8877665544332211 + +/***********************************************/ + +/* All done. */ + +done: + mov $0,%rdi + call exit + hlt + +/* RIP-relative data for VEX3 test above. */ + +.data +var128: + .8byte 0xaa55aa55aa55aa55 + .8byte 0x55aa55aa55aa55aa diff --git a/gdb/testsuite/gdb.arch/amd64-disp-step-avx.exp b/gdb/testsuite/gdb.arch/amd64-disp-step-avx.exp new file mode 100644 index 0000000..a31ca1f --- /dev/null +++ b/gdb/testsuite/gdb.arch/amd64-disp-step-avx.exp @@ -0,0 +1,139 @@ +# Copyright 2009-2017 Free Software Foundation, Inc. + +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + +# This file is part of the gdb testsuite. + +# Test displaced stepping over VEX-encoded RIP-relative AVX +# instructions. + +if { ![istarget x86_64-*-* ] || ![is_lp64_target] } { + verbose "Skipping x86_64 displaced stepping tests." + return +} + +standard_testfile .S + +set additional_flags "-Wa,-g" + +if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile} \ + [list debug $additional_flags]] } { + return -1 +} + +# Get things started. + +gdb_test "set displaced-stepping on" "" +gdb_test "show displaced-stepping" ".* displaced stepping .* is on.*" + +if ![runto_main] then { + fail "can't run to main" + return 0 +} + +# GDB picks a spare register from this list to hold the RIP-relative +# address. +set rip_regs { "rax" "rbx" "rcx" "rdx" "rbp" "rsi" "rdi" } + +# Assign VAL to all the RIP_REGS. + +proc set_regs { val } { + global gdb_prompt + global rip_regs + + foreach reg ${rip_regs} { + gdb_test_no_output "set \$${reg} = ${val}" + } +} + +# Verify all RIP_REGS print as HEX_VAL_RE in hex. + +proc verify_regs { hex_val_re } { + global rip_regs + + foreach reg ${rip_regs} { + gdb_test "p /x \$${reg}" " = ${hex_val_re}" "${reg} expected value" + } +} + +# Set a break at FUNC, which starts with a RIP-relative instruction +# that we want to displaced-step over, and then continue over the +# breakpoint, forcing a displaced-stepping sequence. + +proc disp_step_func { func } { + global srcfile + + set test_start_label "${func}" + set test_end_label "${func}_end" + + gdb_test "break ${test_start_label}" \ + "Breakpoint.*at.* file .*$srcfile, line.*" \ + "break ${test_start_label}" + gdb_test "break ${test_end_label}" \ + "Breakpoint.*at.* file .*$srcfile, line.*" \ + "break ${test_end_label}" + + gdb_test "continue" \ + "Continuing.*Breakpoint.*, ${test_start_label} ().*" \ + "continue to ${test_start_label}" + + # GDB picks a spare register to hold the RIP-relative address. + # Ensure the spare register value is restored properly (rax-rdi, + # sans rsp). + set value "0xdeadbeefd3adb33f" + set_regs $value + + gdb_test "continue" \ + "Continuing.*Breakpoint.*, ${test_end_label} ().*" \ + "continue to ${test_end_label}" + + verify_regs $value +} + +# Test a VEC2-encoded RIP-relative instruction. +with_test_prefix "vex2" { + # This case writes to the 'xmm0' register. Confirm the register's + # value is what we believe it is before the AVX instruction runs. + gdb_test "p /x \$xmm0.uint128" " = 0x0" \ + "xmm0 has expected value before" + + disp_step_func "test_rip_vex2" + + # Confirm the instruction's expected side effects. It should have + # modified xmm0. + gdb_test "p /x \$xmm0.uint128" " = 0x1122334455667788" \ + "xmm0 has expected value after" +} + +# Test a VEC3-encoded RIP-relative instruction. +with_test_prefix "vex3" { + # This case writes to the 'var128' variable. Confirm the + # variable's value is what we believe it is before the AVX + # instruction runs. + gdb_test "p /x (unsigned long long \[2\]) var128" \ + " = \\{0xaa55aa55aa55aa55, 0x55aa55aa55aa55aa\\}" \ + "var128 has expected value before" + + # Run the AVX instruction. + disp_step_func "test_rip_vex3" + + # Confirm the instruction's expected side effects. It should have + # modifed the 'var128' variable. + gdb_test "p /x (unsigned long long \[2\]) var128" \ + " = \\{0x1122334455667788, 0x0\\}" \ + "var128 has expected value after" +} + +# Done, run program to exit. +gdb_continue_to_end "amd64-disp-step-avx"