From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 128042 invoked by alias); 1 Oct 2018 00:25:35 -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 127905 invoked by uid 89); 1 Oct 2018 00:25:25 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=0.9 required=5.0 tests=AWL,BAYES_50,SPF_PASS autolearn=ham version=3.3.2 spammy=roughly, preceding, H*i:sk:A78C989, Metzger X-HELO: smtp.lse.epita.fr Received: from lse.epita.fr (HELO smtp.lse.epita.fr) (163.5.55.17) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 01 Oct 2018 00:25:23 +0000 Received: from trigger (unknown [37.228.243.214]) by smtp.lse.epita.fr (Postfix) with ESMTPSA id A5E7860D7A; Mon, 1 Oct 2018 02:25:18 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=lse.epita.fr; s=smtp; t=1538353518; bh=ZJ3lsMYxpXvfvlIALPgt9q162Q4dsTUXgYv19YHrteE=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=pDcL9xH1vZqMH4Oqj6oU50aZ41bPEVk/plW8QKslLSwdmOJ/mSksawyroVYtY7A2t xGTzj8o1BXaggBB2/O2UFs7kvYocgBDjtPvQS744Ahl1I0+MlMfN+PBjxkVTeDkTbz oW/p1HWdXoeathULbFKB+LXqaA2KzCfPZUvwC1LY= Date: Mon, 01 Oct 2018 00:25:00 -0000 From: Pierre Marsais To: "Metzger, Markus T" Cc: "gdb-patches@sourceware.org" Subject: Re: [PATCH] Add support for recording xsave x86 instruction Message-ID: <20181001002516.GA31390@trigger> References: <20180921003827.1525-1-pierre.marsais@lse.epita.fr> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.10.1 (2018-07-13) X-SW-Source: 2018-10/txt/msg00000.txt.bz2 Hi, Thanks for the review. On Thu, Sep 27, 2018 at 08:44:44AM +0000, Metzger, Markus T wrote: >> + if (!__get_cpuid_count(0xd, i, &size, &offset, &tmp1, &tmp2)) >> + return -1; > > This would check the native configuration, correct? What if we recorded > remotely on a different x86 box? Oops, yes. I don't find how to query the offsets/sizes remotely, however there is XSTATE areas sizes in gdb/common/x86-xstate.h. I think we can assume that those values are correct. > Also I think that we would need to check the inferior architecture to handle > 32-bit compatibility mode. I'm not sure to follow you. In which cases 32-bit behaves differently than 64-bit ? >> + >> + if (record_full_arch_list_add_mem (tmpu64 + offset, size)) >> + return -1; > > Looks like this assumes the standard (non-compacted) XSAVE format. > > For the compacted format, the offset must be computed by accumulating > the sizes of preceding components. If I'm not mistaken, the compact format is only used by XSAVEC instruction, which doesn't have the same opcode. The XSAVE instruction seems unrelated to this format. >> +if ![istarget "*86*-*linux*"] then { >> + verbose "Skipping i386 reverse tests." >> + return >> +} > > Why exclude 64-bit? Isn't this roughly the same as: [ istarget "x86_64-*linux*" ] && [ istarget "i?86-*linux*" ] thus excluding all arch but 32 and 64 bit x86 ? Anyways it seems to be working on my x86_64 linux with $ make check TESTS=gdb.reverse/i386-xsave-reverse.exp Regards, -- Pierre "Pimzero" MARSAIS, EPITA 2018; GISTRE | ACU | LSE