Re: [Buildroot] [PATCH 2/2] package/gdb: use stat() privided by the system

Sergio Durigan Junior
2018-09-12T17:11:38Z

On Wednesday, September 12 2018, Pedro Alves wrote:

> On 09/11/2018 07:21 PM, Sergio Durigan Junior wrote:
>> On Tuesday, September 11 2018, Pedro Alves wrote:
>>
>>> On 09/11/2018 01:38 AM, Sergio Durigan Junior wrote:
>>>
>>>> This is happening because, before the commit mentioned above,
>>>> 'common-utils.c' (which gets transformed into 'common-utils-ipa.c'
>>>> during the gdbserver build) wasn't calling 'stat'. It doesn't seem like
>>>> a regression; it seems like a hidden problem that was uncovered by the
>>>> need of 'stat'.
>>>>
>>>> I don't know why this problem is manifesting only when compiling IPA,
>>>> and not when compiling 'common-utils.c' during GDB's/gdbserver's build.
>>>
>>> Because the IPA doesn't link with gnulib. And the answer to that wouldn't
>>> be as simple as "just link it in", because the IPA objects are supposed
>>> to be compiled with -fPIC and -fvisibility=hidden. So we'd need a third
>>> build of gnulib for the IPA.
>>
>> That explains it. It seems strange to me that we still include the
>> gnulib headers when compiling IPA; I confess I just assumed IPA was
>> linking with gnulib because of this.
>
> In order _not_ to include the gnulib headers, that would mean that we'd
> be OK with going back to making sure we do any necessary portability
> autoconf/#ifdefery ourselves in all of common and gdbserver code that is
> used by the IPA, which doesn't sound very appealing to me.
> I'd think it better / simpler maintenance-wise going forward, to work in the
> direction of linking with gnulib if/when we find a need.
>
> The IPA has so few dependencies by design that in practice just using the
> headers should be fine, with gnulib ironing-out any header-only portability
> issues. It's quite likely that a port that can use the IPA won't really need
> any gnu_foo replacement function. And if it does, then it should be
> better to use gnulib's replacements instead of duplicating what gnulib
> already does, I'd think.

I guess I don't really understand what's going on behind the curtains
here, then. I thought that, since IPA doesn't link with gnulib, then we
shouldn't be compiling it using "... -I./../gnulib/import
-Ibuild-gnulib-gdbserver/import ...", since it can be confusing for the
reader to determine "oh, gnulib's header files are being included, but
the library is not actually *linking* with gnulib". I understand that
it'd be simpler to just build a third gnulib just for linking with IPA,
but it seems like we're not going to do that, at least not right now.

Anyway, this is all very unimportant and I don't want to waste people's
time; I'm just trying to understand and to express the difficulty that I
had while examining the logs (i.e., determining that IPA wasn't linking
with gnulib, despite the inclusion of the headers).

>>
>>> It doesn't seem like this code that calls stat (is_regular_file?) is
>>> useful for the IPA, so a quicker/simpler fix would be to simply move
>>> that function out of common-utils.c into some other file that is not
>>> shared with the IPA.
>>
>> It's not useful for IPA; it could be moved to common/filestuff.c, for
>> example.
>
> Right.
>
>> But IMHO, we should probably have an explicit file just for
>> IPA, because otherwise we'll forget about this restriction and re-add
>> some 'stat' calls to common-utils.c.
>
> I'm afraid I don't understand what you mean here.

This problem happened because (a) IPA doesn't link with gnulib, (b) IPA
uses common-utils.c, and (c) common-utils.c calls 'stat', which, in a
cross-compilation environment, gets replaced by gnulib's 'rpl_stat'. We
want to solve the problem by tackling (c) and moving the 'stat' call to
another file that is not used by IPA.

My point is that, some time in the future, someone might not remember/be
aware of this problem and reintroduce a call to 'stat' on
common-utils.c, which would reintroduce this problem (assuming nothing
is done on gnulib's side to fix this).

I wrote the "proposal" above without completely understanding the
problem. Indeed, it doesn't make sense to have a separate, IPA-specific
file for common-utils.c. I guess the best approach here would be,
again, to compile a separate version of gnulib for IPA.

Thanks,

--
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF 31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible