From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 54545 invoked by alias); 8 Jun 2016 16:42:27 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 54453 invoked by uid 89); 8 Jun 2016 16:42:26 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.6 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,RCVD_IN_DNSWL_LOW,SPF_PASS autolearn=ham version=3.3.2 spammy=invisible X-HELO: mail-pa0-f68.google.com Received: from mail-pa0-f68.google.com (HELO mail-pa0-f68.google.com) (209.85.220.68) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Wed, 08 Jun 2016 16:42:24 +0000 Received: by mail-pa0-f68.google.com with SMTP id ug1so888907pab.1 for ; Wed, 08 Jun 2016 09:42:24 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:references:date:in-reply-to :message-id:user-agent:mime-version:content-transfer-encoding; bh=5krWFgoVqw6Itg6RZErFvBinTnq/McLyLQ0ekbc0XVw=; b=T/bT0xax02vn9g4OQTTvmsGNXrpFOmEXGDeGaDRYl1Mtz25frKDPBKZXoEDdh01erd XDEYMdNqZ2aD48iv8aam0Cai57NWSUv6WNpgjEvq1jh9Atl4j0pH/2bA5N4RBMifXmgK MHCQxoUekcR/6ZP01FS/lW5z1Roj/T5FrBYW2ykwWiShG44Wuhu3T0h7NjOf/0Fa49iS IeMyNPlc4pL/ntUdgAuFNy8Mw/9+8qQNd8jSKTOQ5WaauOw95VKaXFzXauwJZ2+HUTvu 9Pe3TGCE6mbivgolhAQ3vFEx7WKZrCiMXxgveTneABIw7uIclklHVIJdE9C10rsGC4qo xm1w== X-Gm-Message-State: ALyK8tKlgdnaeQFee2qSeCatMR9p8TFh5NvqGgncJjO0BspoIWQhiDGydBbeM8W//x4zXQ== X-Received: by 10.66.84.164 with SMTP id a4mr6639782paz.90.1465404142301; Wed, 08 Jun 2016 09:42:22 -0700 (PDT) Received: from E107787-LIN (gcc113.osuosl.org. [140.211.9.71]) by smtp.gmail.com with ESMTPSA id dr4sm3664466pac.11.2016.06.08.09.42.18 (version=TLS1_2 cipher=AES128-SHA bits=128/128); Wed, 08 Jun 2016 09:42:21 -0700 (PDT) From: Yao Qi To: Pedro Alves Cc: Yao Qi , Jan Kratochvil , gdb-patches@sourceware.org Subject: Re: [patch] aarch64: PR 19806: watchpoints: false negatives -> false positives References: <20160606075945.GA19395@host1.jankratochvil.net> <86eg89w2sr.fsf@gmail.com> <48622de4-dc45-c48f-7172-495b669f2334@redhat.com> <86a8ixvx5k.fsf@gmail.com> <7fabd183-eb46-e916-77f2-f62d5c4e4ce7@redhat.com> Date: Wed, 08 Jun 2016 16:42:00 -0000 In-Reply-To: <7fabd183-eb46-e916-77f2-f62d5c4e4ce7@redhat.com> (Pedro Alves's message of "Tue, 7 Jun 2016 17:04:24 +0100") Message-ID: <86oa7bvdi0.fsf@gmail.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes X-SW-Source: 2016-06/txt/msg00161.txt.bz2 Pedro Alves writes: > In any case, with the gdbarch method, if the target supports watching all > kinds of unaligned addresses/ranges, and reports the correct address in > "T05 watch" stop replies, it then faces the problem that gdb has hardcoded > knowledge of watch region alignment restrictions, and then e.g., watchpoi= nts > on consecutive addresses are misinterpreted, even though there's > no reason for that. > The gdbarch method should first check if reported address is within the range of watchpoint location, if yes, return watch_triggered_yes, tell GDB that the watchpoint is definitely hit. If we can't find any one range of watchpoint location that the report address is within, we'll guess that the target may be monitoring aligned address. We check watchpoint locations again, to see if address is within [align_down (loc->address, 8), loc->address], if yes, return watch_triggered_unknown, tell GDB that watchpoint is hit, but target doesn't tell the address. [note that we'll miss read watchpoint in this approach]. If no luck, return watch_triggered_no. static enum watchpoint_triggered aarch64_linux_watchpoint_triggered (struct gdbarch *gdbarch, CORE_ADDR addr, struct breakpoint *b) { struct bp_location *loc; int unknown =3D 0; for (loc =3D b->loc; loc !=3D NULL; loc =3D loc->next) { if (addr >=3D loc->address && addr < loc->address + loc->length) { /* If the address reported by the target falls in the memory range that watchpoint W is monitoring, the watchpoint is definitely hit. */ return watch_triggered_yes; } else if (addr >=3D align_down (loc->address, 8) && addr < loc->addres= s) { /* The debugging stub, like GDBserver, may adjust the address to meet kernel's alignment requirements (8-aligned for example), we are not sure watchpoint W is hit or not. */ unknown =3D 1; } } if (unknown) return watch_triggered_unknown; else return watch_triggered_no; } > E.g., say gdb believes the machine only supports watching 32-bit-aligned > words. Then with: > > union > { > char buf[4]; > uint32_t force_align; > } global; > > (gdb) watch global.buf[1]; > Hardware watchpoint 1 ... > (gdb) watch global.buf[3]; > Hardware watchpoint 2 ... > > ... if the program writes to global.buf[1], and the target reports > a memory access to 'global.buf + 1', gdb will believe that > watchpoint 2 _also_ triggered, when it did not. That's a false positive = you > can't help with with real machines, but there's no reason a > simulator/emulator has to suffer from that. As I described above, gdbarch method will return watch_triggered_yes for watchpoint 1, and watch_triggered_unknown for watchpoint 2. After GDB checks the value change, it ignores the latter, which is good. The false positive can be eliminated. > >>=20 >>> I think it's actually problematic for real machines, as the restrictions >>> will often depend on process revisions/models. So a gdbarch approach >>> would be undesirable, IMO. >>=20 >> On the real machine, nowadays, the restriction is that address must be >> 8-byte-aligned on linux.=20 > > I think we need to consider all architectures, and the design going > forward, not just Aarch64. For example, PPC has: We put things into gdbarch method, which is to handle the differences of different archs. > > static int > ppc_linux_watchpoint_addr_within_range (struct target_ops *target, > CORE_ADDR addr, > CORE_ADDR start, int length) > { > int mask; > > if (have_ptrace_hwdebug_interface () > && ppc_linux_get_hwcap () & PPC_FEATURE_BOOKE) > return start <=3D addr && start + length >=3D addr; > else if (ppc_linux_get_hwcap () & PPC_FEATURE_BOOKE) > mask =3D 3; > else > mask =3D 7; > > So e.g., here, the alignment restrictions depend on both > the processor model and kernel. This can be in the gdbarch method for ppc-linux. > > (I'll guess that other embedded architectures that gdb supports probably > have similar restrictions that gdb was never taught about.) > Yes, but we should teach GDB to do what we already know of. >> The restriction can only be relaxed and >> may be removed finally in the future, IOW, the restriction won't become >> 16-byte aligned, so we can write the gdbarch method for aarch64-linux >> like this, > > How can gdb determine whether the restriction has been lifted? In the step of checking whether a given address is within the range monitored by watchpoint, gdb doesn't need to determine that. Once the restriction is removed, the address reported by the target is exactly the address of watchpoint, the gdbarch method returns watch_triggered_yes. > The way to do it is probably either by checking kernel version or having = the > ptrace code that "inserts" the watchpoint to first try watching the unali= gned > region exactly as gdb requested, and if that doesn't work, try a wider, > aligned region. Only that target-side ptrace code is aware of these > finer details > and the correct restrictions in effect for the running system. If we put= this > in a gdbarch method, how can the gdbarch method maintain compatibility wi= th > older kernels and at the same time reflect that newer kernels no longer > impose the restriction? The gdbarch method only does some complementary guess if reported address doesn't fall in the range of watchpoint locations. If there is no such restriction, the guess is not used at all. > > It's actually not just simulators/emulators that can have different > watchpoint restrictions from the machine architecture's debug hardware > limitations. This is in good part a debug API issue as well -- > a target may well support watchpoints implemented in a totally different > way -- for example, I believe Solaris supports "unlimited" watchpoints and > address ranges by implementing watchpoints not by using debug registers, = but > instead by changing memory page protections and trapping faults internall= y, > all invisibly to userspace. If they are invisible to user space, we can do nothing, but I don't know how is it related to this issue. > > All this is why I believe that hardcoding this knowledge in gdb, which is > what a gdbarch method does, is not the best approach.=20 These knowledge is only used when there are some limitations in the debugging api, and these knowledge are not harmful once the limitations are fixed. --=20 Yao (=E9=BD=90=E5=B0=A7)