Re: [PATCH 1/3] gdbserver/IPA: Export some functions via global function pointers.

Marcin Kościelnicki
2016-03-30T22:02:57Z

On 30/03/16 13:32, Ulrich Weigand wrote:
> Marcin KoÃ…Â›cielnicki wrote:
>> On 29/03/16 20:08, Ulrich Weigand wrote:
>>> Marcin KoÃ…Â›cielnicki wrote:
>>>> On 14/03/16 18:49, Ulrich Weigand wrote:
>>>>> The more I think about it, the more I tend to agree that your
>>>>> proposal is actually the best solution. I'd still like to give
>>>>> it a couple of days to give others a chance to comment as well ...
>>>>
>>>> Alright, so what should we do about this issue?
>>>
>>> Since nobody came up with a better idea, and since your patch doesn't
>>> actually preclude anybody from implementing any better idea they might
>>> come up later (since it doesn't actually change anything in the
>>> gdbserver protocol), I'd say we just go with your patch for now.
>>
>> Very well, then. For this to be actually useful for powerpc64, I'll
>> also need an ack on the other patch
>> (https://sourceware.org/ml/gdb-patches/2016-03/msg00201.html).
>
> This looks to be resolved now.
>
>>> However, there does seem to be one issue: your patch changes the
>>> interface between gdbserver and the in-process agent in an incompatible
>>> way. Binaries with an old IPA built in will no longer work with a
>>> new gdbserver, since it will will expect exported symbols like
>>> gdb_collect_ptr, which the old binary doesn't export.
>>>
>>> I think it would be preferable to implement a backward-compatible
>>> way where gdbserver checks for the new symbol, and if it isn't
>>> present, falls back to the old symbol.
>>
>> Alright, I can do that, though I seem to recall we don't care about
>> gdbserver/IPA interface compatibility (and IPA is always built as
>> shared, so there's no concern about an executable with old version built
>> in).
>
> And this turns out to be not necessary after all, see the recent
> mail by Pedro. Sorry for the confusion.
>
> I think the patch should be OK now.

Thanks. I've resolved a trivial interaction with the s390 changes
pushed in the meantime (linux-s390-ipa.c needs an identical fix to the
others) and pushed it.

>
> Thanks,
> Ulrich
>