From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 4611 invoked by alias); 14 Feb 2017 13:35:13 -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 4579 invoked by uid 89); 14 Feb 2017 13:35:10 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=AWL,BAYES_00,RP_MATCHES_RCVD,SPF_PASS autolearn=ham version=3.3.2 spammy=optimal, debugged, H*RU:HELO, Hx-spam-relays-external:HELO X-HELO: mga01.intel.com Received: from mga01.intel.com (HELO mga01.intel.com) (192.55.52.88) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 14 Feb 2017 13:35:00 +0000 Received: from orsmga005.jf.intel.com ([10.7.209.41]) by fmsmga101.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 14 Feb 2017 05:34:58 -0800 X-ExtLoop1: 1 Received: from labpc305.iul.intel.com (HELO [172.28.50.132]) ([172.28.50.132]) by orsmga005.jf.intel.com with ESMTP; 14 Feb 2017 05:34:57 -0800 Subject: Re: [PATCH V7] amd64-mpx: initialize bnd register before performing inferior calls. To: Pedro Alves , qiyaoltc@gmail.com, brobecker@adacore.com References: <1485875613-31975-1-git-send-email-walfred.tedeschi@intel.com> <53d42bb6-3b83-6213-4087-6d30e7d837de@redhat.com> <217a8c13-b7d0-7fe6-56b5-85ff53ce097a@intel.com> <88c7180f-8843-a148-425a-2adf56c6d0bf@redhat.com> <32693426-fbaf-8345-04c7-e2c329d6ec6e@intel.com> Cc: gdb-patches@sourceware.org From: "Tedeschi, Walfred" Message-ID: Date: Tue, 14 Feb 2017 13:35:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.7.0 MIME-Version: 1.0 In-Reply-To: <32693426-fbaf-8345-04c7-e2c329d6ec6e@intel.com> Content-Type: text/plain; charset="windows-1252"; format="flowed" Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes X-SW-Source: 2017-02/txt/msg00383.txt.bz2 On 02/13/2017 09:33 AM, Tedeschi, Walfred wrote: > > > On 02/08/2017 01:27 PM, Pedro Alves wrote: >> On 02/07/2017 08:56 AM, Tedeschi, Walfred wrote: >>> On 01/31/2017 03:13 PM, Walfred Tedeschi wrote: >>>>> This patch initializes the bnd registers before executing the=20 >>>>> inferior >>>>> call. BND registers can be in arbitrary values at the moment of the >>>>> inferior call. In case the function being called uses as part of the >>>>> parameters bnd register, e.g. when passing a pointer as parameter,=20 >>>>> the >>>>> current value of the register will be used. This can cause boundary >>>>> violations that are not due to a real bug or even desired by the=20 >>>>> user. >>>>> In this sense the best to be done is set the bnd registers to allow >>>>> access to the whole memory, i.e. initialized state, before pushing=20 >>>>> the >>>>> inferior call. >>>> This explains the reason for clearing better ... >>> Yes, it was my intention. Do you see value to have the patch in then? >> I think I do. > At this point i think it is woth to mention a bit more on the usage=20 > model I had in mind. > > At the inferior call time all BND registers are set to the INIT state. > That covers 90% of the usage case and keeps the actual behavior of GDB. > In case it is desirable to investigate any bound violation user can=20 > add a breakpoint on the prolog of the function and > set the BND register there, I understand that this is the normal use=20 > case for the debugger as well. In reality it did not work as expected. When an inferior call is done=20 and there is a breakpoint in the executed GDB issues a message like: "The program being debugged stopped while in a function called from GDB. Evaluation of the expression containing the function (foo) will be abandoned. When the function is done executing, GDB will silently stop." Also after that no signal generated within the function call is reported. Not sure if this a limitation or a bug. In case you think it is a bug i=20 can add some entries in Bugzilla. On the other hand i see then the following option: Add the code to INIT the BND regs based on a set/show option. In case not set user will have to take care of setting and restoring the=20 register himself. As optimal solution we could also provide 4 debugger variables, or=20 convenience variables to keep the register values chosen by the user. Use those to set the registers before the inferior call. I have the impression that this option is an overkill. Would you agree with the creation of the new set and show variable to=20 INIT the BND registers before the inferior call? Thanks again and best regards, /Fred >> For passing local pointers to some function, it might be >> that GDB could be able to figure out which bound registers >> contains the bound for a given variable, or if spilled, where >> to find then, and set up the call to use the right bounds, but >> I have no idea of how to retrieve that information. I suspect >> that it's not a mapping we could retrieve from the dwarf? And >> then there's also the case of passing pointers to global >> variables, and pointers to memory that gdb malloc's into the >> inferior, like for array/string coercion: > ABI defines which BND is used for which parameter and what to do when=20 > it is needed to pass more bounds than BND registers available. > >> (gdb) p strlen ("hello") >> >> this will alloc a block of memory in the inferior for >> "hello", by calling malloc in the inferior. If strlen >> is compiled to do BND checks, would we need to setup the >> BND registers to the range of the pointer returned by malloc ? > Yes we would need to set the BND in GDB in this scenario. > However we could simply let that attribution to the user. > My point of view is that this could complicate a lot the code to cover=20 > very little use. > >> >> I've not used BND myself, so I don't have any experience >> with it. But my impression is that disabling BND for >> infcalls makes infcalls work again on BND-enabled systems, and >> that we could perhaps try to do something smart to re-enable it >> in some infcall cases, if there's sufficient benefit. >> >> Now, a question is: could this auto-clearing of BND registers >> get in the way of some use cases? E.g., could a user want to poke >> at the BND registers manually before calling a function in the >> inferior, in order to debug BND-related code. >> If so, that may call for a new option users could tweak >> if necessary. > I was also considering this model as my first implementation. > But this way I suppose that it will be even more complicated. > We will need special commands for the architecture, special=20 > documentation... > > if we set afterwards: Inferior call + Break point + register set. > we should not need any additional set and show for the architecture. > Hover as you pointed out in the other e-mail it is better to increase=20 > testing and document it. >> Thanks, >> Pedro Alves >> > Thanks a lot for your review! > > /Fred Intel Deutschland GmbH Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany Tel: +49 89 99 8853-0, www.intel.de Managing Directors: Christin Eisenschmid, Christian Lamprechter Chairperson of the Supervisory Board: Nicole Lau Registered Office: Munich Commercial Register: Amtsgericht Muenchen HRB 186928