Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: "Marcin Kościelnicki" <koriakin@0x04.net>
To: Ulrich Weigand <uweigand@de.ibm.com>
Cc: gdb-patches@sourceware.org
Subject: Re: [PATCH 1/3] gdbserver/IPA: Export some functions via global function pointers.
Date: Mon, 14 Mar 2016 14:53:00 -0000	[thread overview]
Message-ID: <56E6D07B.5050306@0x04.net> (raw)
In-Reply-To: <20160314144140.CC00C1537@oc7340732750.ibm.com>

On 14/03/16 15:41, Ulrich Weigand wrote:
> Marcin Kościelnicki wrote:
>
>> On powerpc64, qSymbol for a function returns the function code address,
>> and not the descriptor address.  Since we emit code calling gdb_collect
>> and some other functions, we need the descriptor (no way to know the
>> proper TOC address without it).  To get the descriptor address, make
>> global function pointer variables in the IPA pointing to the relevant
>> functions and read them instead of asking for them directly via qSymbol.
>
> Huh.  This problem already came up last year with Wei-cheng's patches.
> See my reply here:
> https://sourceware.org/ml/gdb-patches/2015-02/msg00838.html
>
> At the time, I suggested two possible fixes by changing how qSymbol works.
> Your approach is yet another fix, however ...
>
> I'm not sure I really like your approach, it seems odd to make common
> code jump through "unnatural" hoops just so that powerpc64 works.
> On the other hand, your approach certainly involves the least amount
> of changes to the current code base.
>
Yeah, I'm not that happy with it either... but we're going to need to 
jump through some hoops in gdbserver anyway - eg. for gdb_collect, we 
need the descriptor address, while for stop_tracing, we need the code 
address.  So we will need some special handling for one of these sets of 
symbols either way, and uglify the common code with it.

> I am somewhat confused about one thing, though.  In your other patch
> https://sourceware.org/ml/gdb-patches/2016-03/msg00201.html
> you seem to imply that qSymbol for function symbols simply does not
> work at all on powerpc64 at the moment.

It works *sometimes* - I'm not sure what it depends on, as I'm not 
familiar with BFD internals, but it seems gdb could have the necessary 
information cached and not need to read the file during qSymbol 
processing.  I, for one, had one hell of a debugging session, since my 
simple test program linked against the IPA had all the IPA symbols 
properly fetched, while the testsuite programs failed at the same. 
Could be something like library load order...
>
> If this is true, how does thread-db support work?  This is the one
> pre-existing user of qSymbol for function symbols in gdbserver.
> I had been under the assumption that this actually works now.  Is
> this not in fact true?
>
> If this is indeed just completely broken at the moment, my
> preferred fix would actually be to change qSymbol to just return
> the function descriptor address (i.e. work as on any other platform)
> and have the function descriptor -> function code address lookup
> be done on the gdbserver side when necessary for thread-db support.
> (This would have been my preferred fix anyway, except for the fact
> that it breaks protocol compatibility.  However, if the current
> implementation simply doesn't *work*, there's no reason to worry
> about compatibility.)

Yep, I thought about it, but there's the protocol compatibility issue...
>
> Bye,
> Ulrich
>


  reply	other threads:[~2016-03-14 14:53 UTC|newest]

Thread overview: 56+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-03-13  2:31 [PATCH 0/3] gdbserver: Add powerpc fast tracepoint support Marcin Kościelnicki
2016-03-13  2:32 ` [PATCH 2/3] IPA: Add alloc_jump_pad_buffer target hook Marcin Kościelnicki
2016-03-18 15:08   ` [PATCH 2/4 v2] " Marcin Kościelnicki
2016-03-29 18:18     ` Ulrich Weigand
2016-03-29 22:04       ` Marcin Kościelnicki
2016-03-30 11:38         ` Ulrich Weigand
2016-03-30 14:50           ` Yao Qi
2016-03-30 14:58             ` Ulrich Weigand
2016-03-31  7:34               ` Yao Qi
2016-03-31 11:37                 ` Ulrich Weigand
2016-03-31  1:16       ` [PATCH 2/4 v3] " Marcin Kościelnicki
2016-03-31 11:38         ` Ulrich Weigand
2016-03-31 13:42           ` Marcin Kościelnicki
2016-04-01 14:43         ` Ulrich Weigand
2016-04-03 12:31           ` [PATCH] IPA: Fix build problem on !HAVE_GETAUXVAL Marcin Kościelnicki
2016-04-03 16:26             ` Ulrich Weigand
2016-04-03 16:28               ` Marcin Kościelnicki
2016-04-04 14:41                 ` Ulrich Weigand
2016-04-05 13:33                   ` [PATCH] IPA: Move getauxval out of #ifndef IN_PROCESS_AGENT Marcin Kościelnicki
2016-04-05 15:04                     ` Ulrich Weigand
2016-04-05 16:55                       ` Marcin Kościelnicki
2016-03-13  2:32 ` [PATCH 3/3] gdbserver: Add powerpc fast tracepoint support Marcin Kościelnicki
2016-03-14 22:10   ` [PATCH 3/4 v2] " Marcin Kościelnicki
2016-03-16 16:58     ` Ulrich Weigand
2016-03-16 17:55       ` Marcin Kościelnicki
2016-03-17  6:30         ` Ulrich Weigand
2016-03-18 15:09           ` [PATCH v2 3/4] " Marcin Kościelnicki
2016-03-29 18:23             ` Ulrich Weigand
2016-03-30 14:52             ` Simon Marchi
2016-03-30 14:57               ` Ulrich Weigand
2016-03-30 15:24                 ` Simon Marchi
2016-03-30 15:28                   ` Simon Marchi
2016-03-30 15:35                     ` Ulrich Weigand
2016-03-31  1:31                       ` Marcin Kościelnicki
2016-03-31 11:39                         ` Ulrich Weigand
2016-03-31 13:45                           ` Marcin Kościelnicki
2016-03-13  2:32 ` [PATCH 1/3] gdbserver/IPA: Export some functions via global function pointers Marcin Kościelnicki
2016-03-14 14:41   ` Ulrich Weigand
2016-03-14 14:53     ` Marcin Kościelnicki [this message]
2016-03-14 17:49       ` Ulrich Weigand
2016-03-22  9:19         ` Marcin Kościelnicki
2016-03-29 18:08           ` Ulrich Weigand
2016-03-29 21:51             ` Pedro Alves
2016-03-30 11:30               ` Ulrich Weigand
2016-03-29 21:52             ` Marcin Kościelnicki
2016-03-30 11:32               ` Ulrich Weigand
2016-03-30 22:02                 ` Marcin Kościelnicki
2016-03-31 18:22                   ` Sergio Durigan Junior
2016-03-31 21:42                     ` [PATCH obv] gdbserver: Fix C++ build errors in tracepoint.c Marcin Kościelnicki
2016-03-14 17:08     ` [PATCH 1/3] gdbserver/IPA: Export some functions via global function pointers Simon Marchi
2016-03-14 17:40       ` Ulrich Weigand
2016-03-14 22:25 ` [PATCH 4/4] gdbserver: Add emit_ops for powerpc Marcin Kościelnicki
2016-03-16 17:16   ` Ulrich Weigand
2016-03-18 15:10     ` [PATCH v2 " Marcin Kościelnicki
2016-03-29 18:25       ` Ulrich Weigand
2016-03-31 13:45         ` Marcin Kościelnicki

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=56E6D07B.5050306@0x04.net \
    --to=koriakin@0x04.net \
    --cc=gdb-patches@sourceware.org \
    --cc=uweigand@de.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox