From patchwork Fri Jan 24 16:35:07 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Luis Machado X-Patchwork-Id: 182963 Delivered-To: patch@linaro.org Received: by 2002:a92:1f12:0:0:0:0:0 with SMTP id i18csp778319ile; Fri, 24 Jan 2020 08:35:25 -0800 (PST) X-Google-Smtp-Source: APXvYqyCTaiAhmmvlJCiolgh7ndqQBVJYy9wNfCEnGzMKjXQIaTVzM3Civg7gMt0S55fRnzEMXDz X-Received: by 2002:a05:6830:14d9:: with SMTP id t25mr3402614otq.258.1579883725319; Fri, 24 Jan 2020 08:35:25 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1579883725; cv=none; d=google.com; s=arc-20160816; b=oK4V7t6wDpEyev5Q8ZzE1Xwl+7M3Zdsl52ncSvkW59tdTtuLEDwB/OQFX6g+BXVma6 B4QTN9Md1gQGLcKpKGiO8lR5iY4MjYPPeJ5lS9SO7nzyKQ6w1G0JbwdGU7P5V3163Sdc rfGvlknsFyKr+fhySTtfd6jVcXn3qVkCiRFCkvIsGUC/Tf6fKskCGpr1xJccjJf8RUAm NC+xAv9+/dP6UcfKvLDZ5I+VO1gUFJ6J/f/PgDtZLSKUEhp1WwSFRrlIiPpdMCzNt8fN K+CyKM12H3+kVttfMH+rrK4CI9hd+FgeC3AF2e/1PWUj0CxVP86vPFwDSVPKpF+Nk66R jiyQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=references:in-reply-to:message-id:date:subject:cc:to:from :dkim-signature:delivered-to:sender:list-help:list-post:list-archive :list-subscribe:list-unsubscribe:list-id:precedence:mailing-list :dkim-signature:domainkey-signature; bh=PMouguYMVs1fpg86GdzLElTiTjSlyJUvefAbve+1+JE=; b=M+7UcG57K1O657YpeGbmVsMVWSMtQS+bnW6gR/G4HjqlaaUjwQRytB9MFTwioNvRAb u8W9ZIZ5YeoefETVwwlqiMy3ycN0l0u8ur4Oqx8zwaMf7Oo/9y++jFI6rcPbSwRUclvr 5Abq0M7lFam7yE41dQAk/vWiu8M/3oLUKYq8leKUrco/bP8XsISxnd74F4Iw6MqO+Khr BI5fByoYK3DZ4gjpZDSlj4DMQT+6C5ls3KW9C3fxBk6M3dyuoYe+a0rna9pKz0bJK9dy FQWxYovqZ2dcfNNyvvloU/KE6eq3KBtFX4RCua7fUPHZDEzJLd0FOSQ79FtZab8U4ka2 PQyg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@sourceware.org header.s=default header.b=sOi2NePZ; dkim=pass header.i=@linaro.org header.s=google header.b=ZCzTfCif; spf=pass (google.com: domain of gdb-patches-return-163500-patch=linaro.org@sourceware.org designates 209.132.180.131 as permitted sender) smtp.mailfrom="gdb-patches-return-163500-patch=linaro.org@sourceware.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linaro.org Return-Path: Received: from sourceware.org (server1.sourceware.org. [209.132.180.131]) by mx.google.com with ESMTPS id c75si20319oib.220.2020.01.24.08.35.25 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 24 Jan 2020 08:35:25 -0800 (PST) Received-SPF: pass (google.com: domain of gdb-patches-return-163500-patch=linaro.org@sourceware.org designates 209.132.180.131 as permitted sender) client-ip=209.132.180.131; Authentication-Results: mx.google.com; dkim=pass header.i=@sourceware.org header.s=default header.b=sOi2NePZ; dkim=pass header.i=@linaro.org header.s=google header.b=ZCzTfCif; spf=pass (google.com: domain of gdb-patches-return-163500-patch=linaro.org@sourceware.org designates 209.132.180.131 as permitted sender) smtp.mailfrom="gdb-patches-return-163500-patch=linaro.org@sourceware.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linaro.org DomainKey-Signature: a=rsa-sha1; c=nofws; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:from:to:cc:subject:date:message-id:in-reply-to :references; q=dns; s=default; b=duVizXFfW3uAebfay6aNbY2lEVNayFn 3GAINOAopyYEKIb1LAvs0OrlZMJjdbfFo6JPFxyYrPYt6tITesHiqbc1dGj6PD8Z MIvR4u1Dsfw927507TORQrcpoUI8IlR5M4zdiCvScxcHRBBpcHR+0yTLtJmSa4zl dZrCEb80BhfA= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:from:to:cc:subject:date:message-id:in-reply-to :references; s=default; bh=bP8+di3/7YgD545j2I/U/FMrlc4=; b=sOi2N ePZBiZQzQpUFXi1XrmoeLoR3xDVJpWLR6Tl827eMaebX0WtkXxTvxjQGJ5BulvgZ 9PUo0COolhusl1lkYFidndMvd7Aqoh05s5hN3OIn6HbQN/YtGLVNA+V1MOI/ieZB Z8azwLYuxvtNMzhTJJkE93aC3hXzvr8QgMoQMc= Received: (qmail 14140 invoked by alias); 24 Jan 2020 16:35:18 -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 14130 invoked by uid 89); 24 Jan 2020 16:35:18 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-23.4 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=sitting, offby1, Harden, harden X-HELO: mail-qt1-f171.google.com Received: from mail-qt1-f171.google.com (HELO mail-qt1-f171.google.com) (209.85.160.171) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 24 Jan 2020 16:35:16 +0000 Received: by mail-qt1-f171.google.com with SMTP id w47so1970771qtk.4 for ; Fri, 24 Jan 2020 08:35:15 -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:in-reply-to:references; bh=PMouguYMVs1fpg86GdzLElTiTjSlyJUvefAbve+1+JE=; b=ZCzTfCifIWrSnjGtjmALtauuNLObThHKWEJhjhV8Iab/xmbFo6sUa99enVEcSP0/kQ 9LtcAGPYpyJUagASBTfj0t5wQijFjQS3AQbnJzLmB0DD9F2jqNnErYWJ2oPbiTYRWXI2 FXr/eRL53BvOjEyVtBIFqvyzUJKgeWqhkOAy2/zb0DcnM2twIthorn7LRN4IUpBIXoBC i7Th8wnqeL5VlXxRAuIjHgnQuWklydUATeWy+B7sY/IpWA+wrCydrwJkIBplh8/vyBI3 I86XXDLZKZ94uV8BSssA8YTZZeHwKgCjMarhXtnyjAo2hFzshd3ZYouQbGdUPQJB6EkL MX9g== Return-Path: Received: from localhost.localdomain ([177.158.80.56]) by smtp.gmail.com with ESMTPSA id q5sm3445057qkf.14.2020.01.24.08.35.11 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 24 Jan 2020 08:35:12 -0800 (PST) From: Luis Machado To: gdb-patches@sourceware.org Cc: Alan.Hayward@arm.com, simark@simark.ca Subject: [PATCH,v2] Harden gdb.base/step-over-syscall.exp Date: Fri, 24 Jan 2020 13:35:07 -0300 Message-Id: <20200124163507.32131-1-luis.machado@linaro.org> In-Reply-To: <20200115203645.26360-1-luis.machado@linaro.org> References: <20200115203645.26360-1-luis.machado@linaro.org> X-IsSubscribed: yes New in v2: - Set initial values to -1 instead of 0. - Rewrote RE to prevent unexpected matching when parsing one character at a time. - Used gdb_assert for an additional check. - Validated with check-read1 Simon, I did some research on checking the syscall numbers to make sure we're calling the right syscall, but there seems to be considerable variation in terms of what registers are used to pass the syscall number and also the syscall number itself. For example, aarch64 seems to use clone for fork/vfork, but arm does not. The syscall number comes in through w0, but it also gets passed via x8 if it goes into the kernel. Given the added complexity and the fact that the test is already breaking into fork/vfork/clone, i think we can be reasonably sure that we are invoking the right syscall. What do you think? Any other ideas? Luis --- 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. 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 gdb/testsuite/ChangeLog: 2020-01-24 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, 64 insertions(+), 27 deletions(-) -- 2.17.1 diff --git a/gdb/testsuite/gdb.base/step-over-syscall.exp b/gdb/testsuite/gdb.base/step-over-syscall.exp index b373c169c0..8e75ec92e5 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 -1 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,39 +76,70 @@ 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 -1 + 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 ".*$gdb_prompt $" { + pass $test + } + } + + # If we are not at the syscall instruction yet, keep looking for it with + # stepi commands. + if {$syscall_insn_addr == -1} { + # 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 $test + } + -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 } + } } - 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 before/after syscall instruction" + gdb_test_multiple "x/2i \$pc" $test { + -re "x/2i .*=> ($hex) .*:.*$syscall_insn.* ($hex) .*:.*$gdb_prompt $" { + set syscall_insn_addr $expect_out(1,string) + set next_insn_addr $expect_out(3,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 } } - return [list $syscall_insn_addr [get_hexadecimal_valueof "\$pc" \ - "0" "pc after stepi"]] + + set pc_after_stepi [get_hexadecimal_valueof "\$pc" "0" \ + "pc after stepi"] + + gdb_assert {$next_insn_addr == $pc_after_stepi} \ + "pc after stepi matches insn addr after syscall" + + return [list $syscall_insn_addr $pc_after_stepi] } proc step_over_syscall { syscall } { @@ -156,8 +188,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"