From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 36991 invoked by alias); 13 Jul 2017 16:55:17 -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 36976 invoked by uid 89); 13 Jul 2017 16:55:16 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.4 required=5.0 tests=BAYES_00,FREEMAIL_FROM,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,RCVD_IN_DNSWL_NONE,RCVD_IN_SORBS_SPAM,SPF_PASS autolearn=ham version=3.3.2 spammy=H*r:sk:clients X-HELO: mail-it0-f65.google.com Received: from mail-it0-f65.google.com (HELO mail-it0-f65.google.com) (209.85.214.65) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 13 Jul 2017 16:55:14 +0000 Received: by mail-it0-f65.google.com with SMTP id v193so8223967itc.2 for ; Thu, 13 Jul 2017 09:55:14 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:references:date:in-reply-to :message-id:user-agent:mime-version:content-transfer-encoding; bh=DiHFgk8Fp8pcTGxXrDBlsPNLNk8jXUG0rkqzxowRgn4=; b=I42wyPGPhT0rewGeRxkqWSuuIcq0j5e4+lmxwYsNhiAqxcDSukpuUA1EOB0BAAhGVt AcIq0A6lB50rYHm+bpc+eg3eW4dhkv0oL/QdPNi1BRYPYdfnyPkjSdONTzi3XL0cgufK 6oChxkklUQPOHVfx05vOP42iSqyaYzJTVNMtZcm1LkWBNf3mQKX5IqlVfuJK6/oZtUyf lGILZdKaNMUdvFV6q6M6Ybn1beyxeT5zr6A8T/5mPDHY8eao3eBYDsv/Vt7f6W5R70Hp Uc3+LeIGxCt1x/wu6Aa4jHHFwyTRIwWK2pPVm6uGJxRaUH8je1e9QHdX5gC4zrGOXceT cbhw== X-Gm-Message-State: AIVw113mDbiCsoc9zezipd2oC1GlLVO1jBsoTNAt9mK/69auN9Y/iUhJ McZkHOHABd0fgg== X-Received: by 10.36.40.196 with SMTP id h187mr14473168ith.43.1499964912676; Thu, 13 Jul 2017 09:55:12 -0700 (PDT) Received: from E107787-LIN (static.42.136.251.148.clients.your-server.de. [148.251.136.42]) by smtp.gmail.com with ESMTPSA id x77sm3089018ita.12.2017.07.13.09.55.10 (version=TLS1_2 cipher=AES128-SHA bits=128/128); Thu, 13 Jul 2017 09:55:11 -0700 (PDT) From: Yao Qi To: John Baldwin Cc: gdb-patches@sourceware.org, Simon Marchi , Phil Muldoon Subject: Re: [PATCH 1/2] Include the fs_base and gs_base registers in amd64 target descriptions. References: <20170627224948.99138-1-jhb@FreeBSD.org> <2907792.3mtlvelY3m@ralph.baldwin.cx> Date: Thu, 13 Jul 2017 16:55:00 -0000 In-Reply-To: <2907792.3mtlvelY3m@ralph.baldwin.cx> (John Baldwin's message of "Wed, 12 Jul 2017 13:03:03 -0700") Message-ID: <86y3rsp991.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: 2017-07/txt/msg00157.txt.bz2 John Baldwin writes: > I'm not sure why these should actually be equal at all. In theory we > are resolving two different target descriptions which should have each > called tdesc_create_reg(). I don't see anything that follows a > singleton-like pattern so that if two tdesc's create the same register > with the same name they point to the same 'struct tdesc_reg *' though > that seems to be what is happening for other registers other than > fs_base and gs_base. The tdesc_reg is created for each target description, so there are multiple tdesc_reg objects created. That is, the tdesc_reg of "fs_base" are different among these tdesc_amd64_*_linux. However, the tdesc_reg is a singleton in each target description. Note that we can't use singleton for each register globally, because tdesc_reg.target_regnum can be different in different target descriptions. For example, fs_base's regnum is 57 in "bare-metal" amd64 target descriptions, while it is 58 in "linux" amd64 target descriptions, because of 64bit-linux.xml. > > I noticed that amd64_init_abi() in amd64-tdep.c invokes > tdesc_numbered_register earlier than is done for other registers on x86. > In particular, all the other registers are "added" via > tdesc_numbered_register in i386_validate_tdesc() which is called after > gdbarch_init_abi() (and thus after any OS-dependent hooks have had a > chance to complete). On a whim I tried deferring adding fs_base and > gs_base until i386_validate_tdesc(). This does seem to fix the crash > for me on a CentOS 7 VM I have lying around. The fs_base and gs_base > registers still work for me on FreeBSD. They do not show up in > info registers on the CentOS VM, but perhaps it is too old to have > the relevant ptrace ops (I know it's too old to have the updated Your patch fixes the crash, but I can't see fs_base and gs_base on my machine either. > struct user_reg). What I don't really understand though is why this > works. I also don't fully understand why 'data->arch_regs' is supposed > to always hold the same pointer values as in 'target_desc' in > tdesc_use_registers(). because each tdesc_reg is a singleton among the target description. The reason Simon observed that we have different "fs_base" tdesc_reg added and removed from the hash table is that they are from different target descriptions. GDB crashes in tdesc_use_registers. The arguments tdesc and tdesc_data are not consistent, tdesc is amd64 linux target description, but tdesc_data was got when tdesc is amd64 target description, so the tdesc_reg in tdesc_data are from amd64 target description as well. So, we push a "fs_base" from one target description and want to remove a "fs_base" from another target description. Does this answer your question? I think maybe there is some "better" fix that is to keep tdesc and tdes_data consistent. However, I don't think it further. Since current GDB trunk is unusable on x86_64-linux, it is better get a fix soon. > > diff --git a/gdb/amd64-tdep.c b/gdb/amd64-tdep.c > index 9ff7dfc513..3196ef75a1 100644 > --- a/gdb/amd64-tdep.c > +++ b/gdb/amd64-tdep.c > @@ -3061,15 +3061,7 @@ amd64_init_abi (struct gdbarch_info info, struct g= dbarch *gdbarch) >=20=20 > if (tdesc_find_feature (tdesc, "org.gnu.gdb.i386.segments") !=3D NULL) > { > - const struct tdesc_feature *feature =3D > - tdesc_find_feature (tdesc, "org.gnu.gdb.i386.segments"); > - struct tdesc_arch_data *tdesc_data_segments =3D > - (struct tdesc_arch_data *) info.tdep_info; > - > - tdesc_numbered_register (feature, tdesc_data_segments, > - AMD64_FSBASE_REGNUM, "fs_base"); > - tdesc_numbered_register (feature, tdesc_data_segments, > - AMD64_GSBASE_REGNUM, "gs_base"); > + tdep->fsbase_regnum =3D AMD64_FSBASE_REGNUM; > } >=20=20 There is one line, so braces are not needed. > if (tdesc_find_feature (tdesc, "org.gnu.gdb.i386.pkeys") !=3D NULL) > diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c > index bd728f03dc..1c8263cc87 100644 > --- a/gdb/i386-tdep.c > +++ b/gdb/i386-tdep.c > @@ -8199,7 +8199,7 @@ i386_validate_tdesc_p (struct gdbarch_tdep *tdep, > const struct tdesc_feature *feature_core; >=20=20 > const struct tdesc_feature *feature_sse, *feature_avx, *feature_mpx, > - *feature_avx512, *feature_pkeys; > + *feature_avx512, *feature_pkeys, *feature_segments; Indentation looks wrong. Did you run regression test on x86_64-linux? --=20 Yao (=E9=BD=90=E5=B0=A7)