From patchwork Wed Jan 15 20:36:45 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Luis Machado X-Patchwork-Id: 37399 Received: (qmail 73021 invoked by alias); 15 Jan 2020 20:37:10 -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 72990 invoked by uid 89); 15 Jan 2020 20:37:10 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-23.7 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.1 spammy= X-HELO: mail-qv1-f68.google.com Received: from mail-qv1-f68.google.com (HELO mail-qv1-f68.google.com) (209.85.219.68) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 15 Jan 2020 20:37:00 +0000 Received: by mail-qv1-f68.google.com with SMTP id f16so8025647qvi.4 for ; Wed, 15 Jan 2020 12:36:59 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=from:to:cc:subject:date:message-id; bh=n66qu53Ni/IHB67n9uK9RSoQyVwh5fHCEzLHDTY/C4U=; b=LHXGXO3lew/M7LIppLilxFV4SVMXS82EPWu3QUVlKOyA/8SqDA7OJBI7+PrfXZm+m3 TaVHO+Ir0uwu62S7Chx1DZWiEB7jiQIm9tM05AKqZTqhrlSzgqcGDrLFIJTe4WbePVMG ltBd9eSMBvHRFAeOvzx6Rx3PjtZ0rZJhuY1YqojsTSYFJ4JlsQGOevi1Ic8GMmZfQ/pj aTozg1ivPCZ8VJjW16w3ovStnZMse18IYtHCznSNZHpxd6k1PZLvirDKdkN1u0lD/1RL d0ZqeoKu93FYbwDWmRpI9dqc4HNie5Ty4Kak2KO6ftkm6ua16pUDs2dkhzajV4DgkudP 1mFw== Return-Path: Received: from localhost.localdomain ([179.177.239.131]) by smtp.gmail.com with ESMTPSA id v5sm10252180qtc.64.2020.01.15.12.36.55 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 15 Jan 2020 12:36:56 -0800 (PST) From: Luis Machado To: gdb-patches@sourceware.org Cc: alan.hayward@arm.com Subject: [PATCH] Harden gdb.base/step-over-syscall.exp Date: Wed, 15 Jan 2020 17:36:45 -0300 Message-Id: <20200115203645.26360-1-luis.machado@linaro.org> X-IsSubscribed: yes There are a couple problems with this test. First --- gdb.base/step-over-syscall.exp records the address of a syscall instruction within fork/vfork/clone functions and also the address of the instruction after that syscall instruction. It uses these couples addresses to make sure we stepped over a syscall instruction (fork/vfork/clone events) correctly. The way the test fetches the addresses of the instructions is by stepi-ing its way through the fork/vfork/clone functions until it finds a match for a syscall. Then it stepi's once again to get the address of the next instruction. This assumes that stepi-ing over a syscall is working correctly and landing in the right PC. This is not the case for AArch64/Linux, where we're landing a couple instructions after the syscall in some cases. The following patch lets the test execute as before, but adds a new instruction address check using the x command as opposed to stepi. I didn't want to change how the test works since we may also be interested in checking if stepi-ing over the syscall under different conditions (displaced stepping on/off) yields the same results. I don't feel strongly about this, so i'm OK with changing how we compare PC's for the entire test if folks decide it is reasonable. Second -- FAIL: gdb.base/step-over-syscall.exp: vfork: displaced=off: continue to vfork (3rd time) (the program exited) FAIL: gdb.base/step-over-syscall.exp: vfork: displaced=off: continue to syscall insn vfork (the program is no longer running) FAIL: gdb.base/step-over-syscall.exp: vfork: displaced=off: single step over vfork (the program is no longer running) Depending on the glibc version we may have different code generated for the fork/vfork/clone functions. I ran into the situation where vfork for newer glibc's on AArch64/Linux is very short, so "break vfork" will put a breakpoint right at the syscall instruction, which is something the testcase isn't expecting (a off-by-1 of sorts). The patch adds extra code to handle this case. If the test detects we're already sitting at a syscall instruction, it records the address and moves on to record the address after that particular instruction. Another measure is to "break *$syscall" instead of "break $syscall". That guarantees we're stopping at the first instruction of the syscall function, if it ever happens that the syscall instruction is the first instruction of those functions. With these changes i can fix some failures for aarch64-linux-gnu and also expose the problems i've reported here: https://sourceware.org/ml/gdb-patches/2019-12/msg01071.html These tests now fail for aarch64-linux-gnu (patch for this is going through reviews): FAIL: gdb.base/step-over-syscall.exp: vfork: displaced=off: pc after stepi matches insn addr after syscall FAIL: gdb.base/step-over-syscall.exp: vfork: displaced=on: pc after stepi matches insn addr after syscall I've queued a test run on the buildbot to make sure things are sane for other architectures, and i've tested it on aarch64-linux-gnu. gdb/testsuite/ChangeLog: 2020-01-15 Luis Machado * gdb.base/step-over-syscall.exp (setup): Check if we're already sitting at a syscall instruction when we hit the syscall function's breakpoint. Check PC against one obtained with the x command. (step_over_syscall): Don't continue to the syscall instruction if we're already there. --- gdb/testsuite/gdb.base/step-over-syscall.exp | 91 ++++++++++++++------ 1 file changed, 66 insertions(+), 25 deletions(-) diff --git a/gdb/testsuite/gdb.base/step-over-syscall.exp b/gdb/testsuite/gdb.base/step-over-syscall.exp index b373c169c0..4d9488b1d4 100644 --- a/gdb/testsuite/gdb.base/step-over-syscall.exp +++ b/gdb/testsuite/gdb.base/step-over-syscall.exp @@ -46,7 +46,8 @@ proc_with_prefix check_pc_after_cross_syscall { syscall syscall_insn_next_addr } proc setup { syscall } { global gdb_prompt syscall_insn - + global hex + set next_insn_addr 0 set testfile "step-over-$syscall" clean_restart $testfile @@ -62,7 +63,7 @@ proc setup { syscall } { gdb_test_no_output "set displaced-stepping off" \ "set displaced-stepping off during test setup" - gdb_test "break $syscall" "Breakpoint \[0-9\]* at .*" + gdb_test "break \*$syscall" "Breakpoint \[0-9\]* at .*" gdb_test "continue" "Continuing\\..*Breakpoint \[0-9\]+, (.* in |__libc_|)$syscall \\(\\).*" \ "continue to $syscall (1st time)" @@ -75,37 +76,72 @@ proc setup { syscall } { # Hit the breakpoint on $syscall for the second time. In this time, # the address of syscall insn and next insn of syscall are recorded. - gdb_test "display/i \$pc" ".*" - - # Single step until we see a syscall insn or we reach the - # upper bound of loop iterations. - set msg "find syscall insn in $syscall" - set steps 0 - set max_steps 1000 - gdb_test_multiple "stepi" $msg { - -re ".*$syscall_insn.*$gdb_prompt $" { - pass $msg + # Check if the first instruction we stopped at is the syscall one. + set syscall_insn_addr 0 + set test "fetch first stop pc" + gdb_test_multiple "display/i \$pc" $test { + -re "display/i .*: x/i .*=> ($hex) .*:.*$syscall_insn.*$gdb_prompt $" { + set syscall_insn_addr $expect_out(1,string) + pass $test } - -re "x/i .*=>.*\r\n$gdb_prompt $" { - incr steps - if {$steps == $max_steps} { - fail $msg - } else { - send_gdb "stepi\n" - exp_continue + -re "display/i.*" { + pass $test + } + } + + # If we are not at the syscall instruction yet, keep looking for it with + # stepi commands. + if {$syscall_insn_addr == 0} { + # Single step until we see a syscall insn or we reach the + # upper bound of loop iterations. + set msg "find syscall insn in $syscall" + set steps 0 + set max_steps 1000 + gdb_test_multiple "stepi" $msg { + -re ".*$syscall_insn.*$gdb_prompt $" { + pass $msg + } + -re "x/i .*=>.*\r\n$gdb_prompt $" { + incr steps + if {$steps == $max_steps} { + fail $msg + } else { + send_gdb "stepi\n" + exp_continue + } } } + + if {$steps == $max_steps} { + return { -1, -1 } + } + + set syscall_insn_addr [get_hexadecimal_valueof "\$pc" "0" \ + "pc before stepi"] } - if {$steps == $max_steps} { - return { -1, -1 } + # We have found the syscall instruction. Now record the next instruction. + # Use the X command instead of stepi since we can't guarantee + # stepi is working properly. + set test "pc after syscall instruction" + gdb_test_multiple "x/2i \$pc" $test { + -re "x/2i .*=> $hex .*:.*$syscall_insn.* ($hex) .*:.*$gdb_prompt $" { + set next_insn_addr $expect_out(2,string) + pass $test + } } - set syscall_insn_addr [get_hexadecimal_valueof "\$pc" "0" \ - "pc before stepi"] if {[gdb_test "stepi" "x/i .*=>.*" "stepi $syscall insn"] != 0} { return { -1, -1 } } + + set pc_after_stepi [get_hexadecimal_valueof "\$pc" "0" \ + "pc after stepi with x command"] + + if {$next_insn_addr != $pc_after_stepi} { + fail "pc after stepi matches insn addr after syscall" + } + return [list $syscall_insn_addr [get_hexadecimal_valueof "\$pc" \ "0" "pc after stepi"]] } @@ -156,8 +192,13 @@ proc step_over_syscall { syscall } { } } - gdb_test "continue" "Continuing\\..*Breakpoint \[0-9\]+, .*" \ - "continue to syscall insn $syscall" + # Check if the syscall breakpoint is at the syscall instruction + # address. If so, no need to continue, otherwise we will run the + # inferior to completion. + if {$syscall_insn_addr != [get_hexadecimal_valueof "\$pc" "0"]} { + gdb_test "continue" "Continuing\\..*Breakpoint \[0-9\]+, .*" \ + "continue to syscall insn $syscall" + } gdb_test_no_output "set displaced-stepping $displaced"