From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 444 invoked by alias); 21 Nov 2014 13:37:04 -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 429 invoked by uid 89); 21 Nov 2014 13:37:03 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.4 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_LOW autolearn=ham version=3.3.2 X-HELO: mail-pa0-f50.google.com Received: from mail-pa0-f50.google.com (HELO mail-pa0-f50.google.com) (209.85.220.50) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Fri, 21 Nov 2014 13:37:02 +0000 Received: by mail-pa0-f50.google.com with SMTP id bj1so4905960pad.37 for ; Fri, 21 Nov 2014 05:37:00 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc:content-type:content-transfer-encoding; bh=9YvIo7dYuirxwdnsv+P7+edujS7nAxMXrKYdh0fdMB0=; b=X7Ra847BptMMqPoTxgiBbUPC+spPIuJCsmUpaTH7GIwitrMhqWdywDfWasGvuAvYKw PYI9mh+pWZNJdzbHa6bJWiBhmbDvHl/KugK1HcVYaBqK+yQwIrwon8Hw4aLDF5KzBy5C N89jra3VVz1MDBJlZA3wjpqRYkiupto557qP4TqqPYNVsZdWJLYliVH2Ea+Z2fFp0ezq Jcll8YM4ulGhnPAIXBApyMZRYiOX4WAet1v9iXIGhfYAymKb4hVUQI7Fng2ZbbwXAE3S RkYB9EjyQsUz6SflQti/mISV8IivRLE7gAzkEjoLS/EvGAIiaJYsLzvHBpcq1LKZ7j2S T5Wg== X-Gm-Message-State: ALoCoQnWvUTJUSUaVOLNGf65KxLI2WCpUnxleYzBaUSLGm2XwSqdn6x+BJYcxKI+xF7L6574reD+ X-Received: by 10.66.222.231 with SMTP id qp7mr6586534pac.39.1416573661715; Fri, 21 Nov 2014 04:41:01 -0800 (PST) MIME-Version: 1.0 Received: by 10.70.31.100 with HTTP; Fri, 21 Nov 2014 04:40:41 -0800 (PST) In-Reply-To: <87k32og51t.fsf@codesourcery.com> References: <1411355243-10812-1-git-send-email-patrick@parcs.ath.cx> <87k32og51t.fsf@codesourcery.com> From: Patrick Palka Date: Fri, 21 Nov 2014 13:37:00 -0000 Message-ID: Subject: Re: [PATCH 1/2] Fix C++ virtual method pointer resolution To: Yao Qi Cc: gdb-patches@sourceware.org Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes X-SW-Source: 2014-11/txt/msg00517.txt.bz2 On Fri, Nov 21, 2014 at 6:35 AM, Yao Qi wrote: > Patrick Palka writes: > > Hi, > Sorry for the delayed review... > >> The issue lies in the initial creation of the virtual method pointer as >> seen by GDB. In gnuv3_make_method_ptr() we fail to shift the vtable >> offset when storing the first word of a virtual method pointer. This is >> important because functions that read this first word (namely the >> callers of gnuv3_decode_method_ptr()) expect that the vtable offset is a >> multiple of "sizeof (vtable_ptrdiff_t)". Also it ensures that the vbit >> tag does not collide with the bits used to store the actual offset. >> >> So when writing the virtual method pointer contents we need to shift the >> vtable offset so as to be in symmetry with what the readers of the >> vtable offset do -- which is, xor the vbit tag and then shift back the >> offset. (The prominent readers of the vtable offset are >> gnuv3_print_method_ptr() and gnuv3_method_ptr_to_value().) > > I am not familiar with how gdb handle c++ virtual method, but your > analysis looks right to me. I spend the whole day reading c++ abi, but > still don't know how to connect the abi with the code here :( It is most likely the case that the GDB encoding is totally not in line with the C++ ABI. I don't think GDB has any tests that check this (e.g. by calling a compiled function that takes a member pointer). > >> diff --git a/gdb/gnu-v3-abi.c b/gdb/gnu-v3-abi.c >> index d5ed355..ccb0be6 100644 >> --- a/gdb/gnu-v3-abi.c >> +++ b/gdb/gnu-v3-abi.c >> @@ -683,7 +683,12 @@ gnuv3_make_method_ptr (struct type *type, gdb_byte = *contents, >> >> if (!gdbarch_vbit_in_delta (gdbarch)) >> { >> - store_unsigned_integer (contents, size, byte_order, value | is_vi= rtual); >> + if (is_virtual !=3D 0) >> + { >> + value =3D value * TYPE_LENGTH (vtable_ptrdiff_type (gdbarch)); > > We need to hoist this shift out of "if" block, so that the path goes to > "else" branch can be covered too. Otherwise, fails in > gdb.cp/method-ptr.exp can't be fixed on arm-linux target (on which > vbit_in_delta is zero). OK. This will at least make GDB consistent with its idea of the encoding of a member pointer, but it will still probably won't be in line with the ABI. > >> + >> +get_debug_format >> + >> +if ![test_debug_format "DWARF 2"] { >> + return 0 >> +} > > Why do we need to check test_debug_format here? Because GDB only supports C++ method pointers with the DWARF debug format. So I'd assume that this test would fail for non-DWARF. > > -- > Yao (=E9=BD=90=E5=B0=A7)