Re: [Xenomai-core] [Xenomai-git] Jan Kiszka : Make xnarch_init_timeconv an uninlined weak function

Re: [Xenomai-core] [Xenomai-git] Jan Kiszka : Make xnarch_init_timeconv an uninlined weak function

[Gilles Chanteperdrix]

GIT version control wrote:
> Module: xenomai-jki
> Branch: for-upstream
> Commit: 6b40653e9c3c4a2433bb4e91344fc378eb860f75
> URL:    http://git.xenomai.org/?p=xenomai-jki.git;a=commit;h=6b40653e9c3c4a2433bb4e91344fc378eb860f75
> 
> Author: Jan Kiszka <jan.kiszka@domain.hid>
> Date:   Wed Feb 10 13:24:29 2010 +0100
> 
> Make xnarch_init_timeconv an uninlined weak function
> 
> Otherwise the wrong set of time conversion variables might get
> initialized when using > 1 skin libraries.

If that would be possible, then it is the conversion variables which
should made be weak, not the function.

The way I see it, the posix and native skins currently get a different
set of variables and functions, which works, but with your change, since
there is only one function, only one set of variable gets initialized by
the two function calls. And one skin just broke.

Or am I missing something? Does the patch fix a problem you really had?

-- 
					    Gilles.

Re: [Xenomai-core] [Xenomai-git] Jan Kiszka : Make xnarch_init_timeconv an uninlined weak function

[Jan Kiszka]

Gilles Chanteperdrix wrote:
> GIT version control wrote:
>> Module: xenomai-jki
>> Branch: for-upstream
>> Commit: 6b40653e9c3c4a2433bb4e91344fc378eb860f75
>> URL:    http://git.xenomai.org/?p=xenomai-jki.git;a=commit;h=6b40653e9c3c4a2433bb4e91344fc378eb860f75
>>
>> Author: Jan Kiszka <jan.kiszka@domain.hid>
>> Date:   Wed Feb 10 13:24:29 2010 +0100
>>
>> Make xnarch_init_timeconv an uninlined weak function
>>
>> Otherwise the wrong set of time conversion variables might get
>> initialized when using > 1 skin libraries.
> 
> If that would be possible, then it is the conversion variables which
> should made be weak, not the function.
> 
> The way I see it, the posix and native skins currently get a different
> set of variables and functions, which works, but with your change, since
> there is only one function, only one set of variable gets initialized by
> the two function calls. And one skin just broke.
> 
> Or am I missing something? Does the patch fix a problem you really had?

Frankly, I wasn't able to test in the field yet as replacing the libs
there is non-trivial. But I was able to observe that only one set of
functions is used - which is logical considering the weak marks. And
this breaks due to the static inline initialization.

However, let's mark both functions and variables weak to fix the issue
and avoid leaving unused variables around. Will update my patch in a minute.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

Re: [Xenomai-core] [Xenomai-git] Jan Kiszka : Make xnarch_init_timeconv an uninlined weak function

[Gilles Chanteperdrix]

Jan Kiszka wrote:
> Gilles Chanteperdrix wrote:
>> GIT version control wrote:
>>> Module: xenomai-jki
>>> Branch: for-upstream
>>> Commit: 6b40653e9c3c4a2433bb4e91344fc378eb860f75
>>> URL:    http://git.xenomai.org/?p=xenomai-jki.git;a=commit;h=6b40653e9c3c4a2433bb4e91344fc378eb860f75
>>>
>>> Author: Jan Kiszka <jan.kiszka@domain.hid>
>>> Date:   Wed Feb 10 13:24:29 2010 +0100
>>>
>>> Make xnarch_init_timeconv an uninlined weak function
>>>
>>> Otherwise the wrong set of time conversion variables might get
>>> initialized when using > 1 skin libraries.
>> If that would be possible, then it is the conversion variables which
>> should made be weak, not the function.
>>
>> The way I see it, the posix and native skins currently get a different
>> set of variables and functions, which works, but with your change, since
>> there is only one function, only one set of variable gets initialized by
>> the two function calls. And one skin just broke.
>>
>> Or am I missing something? Does the patch fix a problem you really had?
> 
> Frankly, I wasn't able to test in the field yet as replacing the libs
> there is non-trivial. But I was able to observe that only one set of
> functions is used - which is logical considering the weak marks. And
> this breaks due to the static inline initialization.
> 
> However, let's mark both functions and variables weak to fix the issue
> and avoid leaving unused variables around. Will update my patch in a minute.

Ok. Merged, thanks.

-- 
					    Gilles.

Re: [Xenomai-core] [Xenomai-git] Jan Kiszka : Make xnarch_init_timeconv an uninlined weak function

[Philippe Gerum]

On Wed, 2010-02-10 at 15:01 +0100, Gilles Chanteperdrix wrote:
> Jan Kiszka wrote:
> > Gilles Chanteperdrix wrote:
> >> GIT version control wrote:
> >>> Module: xenomai-jki
> >>> Branch: for-upstream
> >>> Commit: 6b40653e9c3c4a2433bb4e91344fc378eb860f75
> >>> URL:    http://git.xenomai.org/?p=xenomai-jki.git;a=commit;h=6b40653e9c3c4a2433bb4e91344fc378eb860f75
> >>>
> >>> Author: Jan Kiszka <jan.kiszka@domain.hid>
> >>> Date:   Wed Feb 10 13:24:29 2010 +0100
> >>>
> >>> Make xnarch_init_timeconv an uninlined weak function
> >>>
> >>> Otherwise the wrong set of time conversion variables might get
> >>> initialized when using > 1 skin libraries.
> >> If that would be possible, then it is the conversion variables which
> >> should made be weak, not the function.
> >>
> >> The way I see it, the posix and native skins currently get a different
> >> set of variables and functions, which works, but with your change, since
> >> there is only one function, only one set of variable gets initialized by
> >> the two function calls. And one skin just broke.
> >>
> >> Or am I missing something? Does the patch fix a problem you really had?
> > 
> > Frankly, I wasn't able to test in the field yet as replacing the libs
> > there is non-trivial. But I was able to observe that only one set of
> > functions is used - which is logical considering the weak marks. And
> > this breaks due to the static inline initialization.
> > 
> > However, let's mark both functions and variables weak to fix the issue
> > and avoid leaving unused variables around. Will update my patch in a minute.
> 
> Ok. Merged, thanks.
> 

The proper way to get rid of any ambiguous case would be to move that
init code out of line, to the skins/common lib you already had the good
idea to create.

-- 
Philippe.

Re: [Xenomai-core] [Xenomai-git] Jan Kiszka : Make xnarch_init_timeconv an uninlined weak function

[Gilles Chanteperdrix]

Jan Kiszka wrote:
> Gilles Chanteperdrix wrote:
>> GIT version control wrote:
>>> Module: xenomai-jki
>>> Branch: for-upstream
>>> Commit: 6b40653e9c3c4a2433bb4e91344fc378eb860f75
>>> URL:    http://git.xenomai.org/?p=xenomai-jki.git;a=commit;h=6b40653e9c3c4a2433bb4e91344fc378eb860f75
>>>
>>> Author: Jan Kiszka <jan.kiszka@domain.hid>
>>> Date:   Wed Feb 10 13:24:29 2010 +0100
>>>
>>> Make xnarch_init_timeconv an uninlined weak function
>>>
>>> Otherwise the wrong set of time conversion variables might get
>>> initialized when using > 1 skin libraries.
>> If that would be possible, then it is the conversion variables which
>> should made be weak, not the function.
>>
>> The way I see it, the posix and native skins currently get a different
>> set of variables and functions, which works, but with your change, since
>> there is only one function, only one set of variable gets initialized by
>> the two function calls. And one skin just broke.
>>
>> Or am I missing something? Does the patch fix a problem you really had?
> 
> Frankly, I wasn't able to test in the field yet as replacing the libs
> there is non-trivial. But I was able to observe that only one set of
> functions is used - which is logical considering the weak marks. And
> this breaks due to the static inline initialization.
> 
> However, let's mark both functions and variables weak to fix the issue
> and avoid leaving unused variables around. Will update my patch in a minute.

Ok. I am reverting this patch until you provide me with another
solution. It causes latency to segfault purely and simply at startup on
my dual PIII.

-- 
					    Gilles.

Re: [Xenomai-core] [Xenomai-git] Jan Kiszka : Make xnarch_init_timeconv an uninlined weak function

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 1773 bytes --]

Gilles Chanteperdrix wrote:
> Jan Kiszka wrote:
>> Gilles Chanteperdrix wrote:
>>> GIT version control wrote:
>>>> Module: xenomai-jki
>>>> Branch: for-upstream
>>>> Commit: 6b40653e9c3c4a2433bb4e91344fc378eb860f75
>>>> URL:    http://git.xenomai.org/?p=xenomai-jki.git;a=commit;h=6b40653e9c3c4a2433bb4e91344fc378eb860f75
>>>>
>>>> Author: Jan Kiszka <jan.kiszka@domain.hid>
>>>> Date:   Wed Feb 10 13:24:29 2010 +0100
>>>>
>>>> Make xnarch_init_timeconv an uninlined weak function
>>>>
>>>> Otherwise the wrong set of time conversion variables might get
>>>> initialized when using > 1 skin libraries.
>>> If that would be possible, then it is the conversion variables which
>>> should made be weak, not the function.
>>>
>>> The way I see it, the posix and native skins currently get a different
>>> set of variables and functions, which works, but with your change, since
>>> there is only one function, only one set of variable gets initialized by
>>> the two function calls. And one skin just broke.
>>>
>>> Or am I missing something? Does the patch fix a problem you really had?
>> Frankly, I wasn't able to test in the field yet as replacing the libs
>> there is non-trivial. But I was able to observe that only one set of
>> functions is used - which is logical considering the weak marks. And
>> this breaks due to the static inline initialization.
>>
>> However, let's mark both functions and variables weak to fix the issue
>> and avoid leaving unused variables around. Will update my patch in a minute.
> 
> Ok. I am reverting this patch until you provide me with another
> solution. It causes latency to segfault purely and simply at startup on
> my dual PIII.
> 

Cannot reproduce yet. Do you have a backtrace?

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]

Re: [Xenomai-core] [Xenomai-git] Jan Kiszka : Make xnarch_init_timeconv an uninlined weak function

[Gilles Chanteperdrix]

Jan Kiszka wrote:
> Gilles Chanteperdrix wrote:
>> Jan Kiszka wrote:
>>> Gilles Chanteperdrix wrote:
>>>> GIT version control wrote:
>>>>> Module: xenomai-jki
>>>>> Branch: for-upstream
>>>>> Commit: 6b40653e9c3c4a2433bb4e91344fc378eb860f75
>>>>> URL:    http://git.xenomai.org/?p=xenomai-jki.git;a=commit;h=6b40653e9c3c4a2433bb4e91344fc378eb860f75
>>>>>
>>>>> Author: Jan Kiszka <jan.kiszka@domain.hid>
>>>>> Date:   Wed Feb 10 13:24:29 2010 +0100
>>>>>
>>>>> Make xnarch_init_timeconv an uninlined weak function
>>>>>
>>>>> Otherwise the wrong set of time conversion variables might get
>>>>> initialized when using > 1 skin libraries.
>>>> If that would be possible, then it is the conversion variables which
>>>> should made be weak, not the function.
>>>>
>>>> The way I see it, the posix and native skins currently get a different
>>>> set of variables and functions, which works, but with your change, since
>>>> there is only one function, only one set of variable gets initialized by
>>>> the two function calls. And one skin just broke.
>>>>
>>>> Or am I missing something? Does the patch fix a problem you really had?
>>> Frankly, I wasn't able to test in the field yet as replacing the libs
>>> there is non-trivial. But I was able to observe that only one set of
>>> functions is used - which is logical considering the weak marks. And
>>> this breaks due to the static inline initialization.
>>>
>>> However, let's mark both functions and variables weak to fix the issue
>>> and avoid leaving unused variables around. Will update my patch in a minute.
>> Ok. I am reverting this patch until you provide me with another
>> solution. It causes latency to segfault purely and simply at startup on
>> my dual PIII.
>>
> 
> Cannot reproduce yet. Do you have a backtrace?

No. But the problem is probably the same as the one signaled by Henri,
a misplaced weak directive ending up in a symbol with no address at all.
Since the current situation works, I am going to wait for the "clean"
fix which puts some code/data in the src/skins/common directory.

-- 
					    Gilles.

Re: [Xenomai-core] [Xenomai-git] Jan Kiszka : Make xnarch_init_timeconv an uninlined weak function

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 2244 bytes --]

Gilles Chanteperdrix wrote:
> Jan Kiszka wrote:
>> Gilles Chanteperdrix wrote:
>>> Jan Kiszka wrote:
>>>> Gilles Chanteperdrix wrote:
>>>>> GIT version control wrote:
>>>>>> Module: xenomai-jki
>>>>>> Branch: for-upstream
>>>>>> Commit: 6b40653e9c3c4a2433bb4e91344fc378eb860f75
>>>>>> URL:    http://git.xenomai.org/?p=xenomai-jki.git;a=commit;h=6b40653e9c3c4a2433bb4e91344fc378eb860f75
>>>>>>
>>>>>> Author: Jan Kiszka <jan.kiszka@domain.hid>
>>>>>> Date:   Wed Feb 10 13:24:29 2010 +0100
>>>>>>
>>>>>> Make xnarch_init_timeconv an uninlined weak function
>>>>>>
>>>>>> Otherwise the wrong set of time conversion variables might get
>>>>>> initialized when using > 1 skin libraries.
>>>>> If that would be possible, then it is the conversion variables which
>>>>> should made be weak, not the function.
>>>>>
>>>>> The way I see it, the posix and native skins currently get a different
>>>>> set of variables and functions, which works, but with your change, since
>>>>> there is only one function, only one set of variable gets initialized by
>>>>> the two function calls. And one skin just broke.
>>>>>
>>>>> Or am I missing something? Does the patch fix a problem you really had?
>>>> Frankly, I wasn't able to test in the field yet as replacing the libs
>>>> there is non-trivial. But I was able to observe that only one set of
>>>> functions is used - which is logical considering the weak marks. And
>>>> this breaks due to the static inline initialization.
>>>>
>>>> However, let's mark both functions and variables weak to fix the issue
>>>> and avoid leaving unused variables around. Will update my patch in a minute.
>>> Ok. I am reverting this patch until you provide me with another
>>> solution. It causes latency to segfault purely and simply at startup on
>>> my dual PIII.
>>>
>> Cannot reproduce yet. Do you have a backtrace?
> 
> No. But the problem is probably the same as the one signaled by Henri,
> a misplaced weak directive ending up in a symbol with no address at all.
> Since the current situation works, I am going to wait for the "clean"
> fix which puts some code/data in the src/skins/common directory.
> 

Find it in my tree. But it's not yet well tested.

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]

Re: [Xenomai-core] [Xenomai-git] Jan Kiszka : Make xnarch_init_timeconv an uninlined weak function

[Gilles Chanteperdrix]

Jan Kiszka wrote:
> Gilles Chanteperdrix wrote:
>> Jan Kiszka wrote:
>>> Gilles Chanteperdrix wrote:
>>>> Jan Kiszka wrote:
>>>>> Gilles Chanteperdrix wrote:
>>>>>> GIT version control wrote:
>>>>>>> Module: xenomai-jki
>>>>>>> Branch: for-upstream
>>>>>>> Commit: 6b40653e9c3c4a2433bb4e91344fc378eb860f75
>>>>>>> URL:    http://git.xenomai.org/?p=xenomai-jki.git;a=commit;h=6b40653e9c3c4a2433bb4e91344fc378eb860f75
>>>>>>>
>>>>>>> Author: Jan Kiszka <jan.kiszka@domain.hid>
>>>>>>> Date:   Wed Feb 10 13:24:29 2010 +0100
>>>>>>>
>>>>>>> Make xnarch_init_timeconv an uninlined weak function
>>>>>>>
>>>>>>> Otherwise the wrong set of time conversion variables might get
>>>>>>> initialized when using > 1 skin libraries.
>>>>>> If that would be possible, then it is the conversion variables which
>>>>>> should made be weak, not the function.
>>>>>>
>>>>>> The way I see it, the posix and native skins currently get a different
>>>>>> set of variables and functions, which works, but with your change, since
>>>>>> there is only one function, only one set of variable gets initialized by
>>>>>> the two function calls. And one skin just broke.
>>>>>>
>>>>>> Or am I missing something? Does the patch fix a problem you really had?
>>>>> Frankly, I wasn't able to test in the field yet as replacing the libs
>>>>> there is non-trivial. But I was able to observe that only one set of
>>>>> functions is used - which is logical considering the weak marks. And
>>>>> this breaks due to the static inline initialization.
>>>>>
>>>>> However, let's mark both functions and variables weak to fix the issue
>>>>> and avoid leaving unused variables around. Will update my patch in a minute.
>>>> Ok. I am reverting this patch until you provide me with another
>>>> solution. It causes latency to segfault purely and simply at startup on
>>>> my dual PIII.
>>>>
>>> Cannot reproduce yet. Do you have a backtrace?
>> No. But the problem is probably the same as the one signaled by Henri,
>> a misplaced weak directive ending up in a symbol with no address at all.
>> Since the current situation works, I am going to wait for the "clean"
>> fix which puts some code/data in the src/skins/common directory.
>>
> 
> Find it in my tree. But it's not yet well tested.

I do not like it either. Functions which are in src/skins/common should
still be weak, since this lib is included in all the skins libraries.

xnarch_init_timeconv does not need to be exported, it may only be called
in common (bind.c, or timeconv.c, or something like that).

xnarch_tsc_to_ns/ns_to_tsc have no reason to be put in arith.h,
timeconv.h was about just right. Maybe create asm-generic/timeconv.h?

-- 
					    Gilles.

Re: [Xenomai-core] [Xenomai-git] Jan Kiszka : Make xnarch_init_timeconv an uninlined weak function

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 3137 bytes --]

Gilles Chanteperdrix wrote:
> Jan Kiszka wrote:
>> Gilles Chanteperdrix wrote:
>>> Jan Kiszka wrote:
>>>> Gilles Chanteperdrix wrote:
>>>>> Jan Kiszka wrote:
>>>>>> Gilles Chanteperdrix wrote:
>>>>>>> GIT version control wrote:
>>>>>>>> Module: xenomai-jki
>>>>>>>> Branch: for-upstream
>>>>>>>> Commit: 6b40653e9c3c4a2433bb4e91344fc378eb860f75
>>>>>>>> URL:    http://git.xenomai.org/?p=xenomai-jki.git;a=commit;h=6b40653e9c3c4a2433bb4e91344fc378eb860f75
>>>>>>>>
>>>>>>>> Author: Jan Kiszka <jan.kiszka@domain.hid>
>>>>>>>> Date:   Wed Feb 10 13:24:29 2010 +0100
>>>>>>>>
>>>>>>>> Make xnarch_init_timeconv an uninlined weak function
>>>>>>>>
>>>>>>>> Otherwise the wrong set of time conversion variables might get
>>>>>>>> initialized when using > 1 skin libraries.
>>>>>>> If that would be possible, then it is the conversion variables which
>>>>>>> should made be weak, not the function.
>>>>>>>
>>>>>>> The way I see it, the posix and native skins currently get a different
>>>>>>> set of variables and functions, which works, but with your change, since
>>>>>>> there is only one function, only one set of variable gets initialized by
>>>>>>> the two function calls. And one skin just broke.
>>>>>>>
>>>>>>> Or am I missing something? Does the patch fix a problem you really had?
>>>>>> Frankly, I wasn't able to test in the field yet as replacing the libs
>>>>>> there is non-trivial. But I was able to observe that only one set of
>>>>>> functions is used - which is logical considering the weak marks. And
>>>>>> this breaks due to the static inline initialization.
>>>>>>
>>>>>> However, let's mark both functions and variables weak to fix the issue
>>>>>> and avoid leaving unused variables around. Will update my patch in a minute.
>>>>> Ok. I am reverting this patch until you provide me with another
>>>>> solution. It causes latency to segfault purely and simply at startup on
>>>>> my dual PIII.
>>>>>
>>>> Cannot reproduce yet. Do you have a backtrace?
>>> No. But the problem is probably the same as the one signaled by Henri,
>>> a misplaced weak directive ending up in a symbol with no address at all.
>>> Since the current situation works, I am going to wait for the "clean"
>>> fix which puts some code/data in the src/skins/common directory.
>>>
>> Find it in my tree. But it's not yet well tested.
> 
> I do not like it either. Functions which are in src/skins/common should
> still be weak, since this lib is included in all the skins libraries.

Those functions are now in libxeno_common only, so I see no point in
allowing them to be overloaded.

> 
> xnarch_init_timeconv does not need to be exported, it may only be called
> in common (bind.c, or timeconv.c, or something like that).

If xnarch_init_timeconv shall be executed unconditionally, I will
happily make it static inline again and call it from xeno_bind_skin_opt
(which would imply retrieving sysinfo as well).

> 
> xnarch_tsc_to_ns/ns_to_tsc have no reason to be put in arith.h,
> timeconv.h was about just right. Maybe create asm-generic/timeconv.h?
> 

Makes sense.

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]

Re: [Xenomai-core] [Xenomai-git] Jan Kiszka : Make xnarch_init_timeconv an uninlined weak function

[Gilles Chanteperdrix]

Jan Kiszka wrote:
> Gilles Chanteperdrix wrote:
>> Jan Kiszka wrote:
>>> Gilles Chanteperdrix wrote:
>>>> Jan Kiszka wrote:
>>>>> Gilles Chanteperdrix wrote:
>>>>>> Jan Kiszka wrote:
>>>>>>> Gilles Chanteperdrix wrote:
>>>>>>>> GIT version control wrote:
>>>>>>>>> Module: xenomai-jki
>>>>>>>>> Branch: for-upstream
>>>>>>>>> Commit: 6b40653e9c3c4a2433bb4e91344fc378eb860f75
>>>>>>>>> URL:    http://git.xenomai.org/?p=xenomai-jki.git;a=commit;h=6b40653e9c3c4a2433bb4e91344fc378eb860f75
>>>>>>>>>
>>>>>>>>> Author: Jan Kiszka <jan.kiszka@domain.hid>
>>>>>>>>> Date:   Wed Feb 10 13:24:29 2010 +0100
>>>>>>>>>
>>>>>>>>> Make xnarch_init_timeconv an uninlined weak function
>>>>>>>>>
>>>>>>>>> Otherwise the wrong set of time conversion variables might get
>>>>>>>>> initialized when using > 1 skin libraries.
>>>>>>>> If that would be possible, then it is the conversion variables which
>>>>>>>> should made be weak, not the function.
>>>>>>>>
>>>>>>>> The way I see it, the posix and native skins currently get a different
>>>>>>>> set of variables and functions, which works, but with your change, since
>>>>>>>> there is only one function, only one set of variable gets initialized by
>>>>>>>> the two function calls. And one skin just broke.
>>>>>>>>
>>>>>>>> Or am I missing something? Does the patch fix a problem you really had?
>>>>>>> Frankly, I wasn't able to test in the field yet as replacing the libs
>>>>>>> there is non-trivial. But I was able to observe that only one set of
>>>>>>> functions is used - which is logical considering the weak marks. And
>>>>>>> this breaks due to the static inline initialization.
>>>>>>>
>>>>>>> However, let's mark both functions and variables weak to fix the issue
>>>>>>> and avoid leaving unused variables around. Will update my patch in a minute.
>>>>>> Ok. I am reverting this patch until you provide me with another
>>>>>> solution. It causes latency to segfault purely and simply at startup on
>>>>>> my dual PIII.
>>>>>>
>>>>> Cannot reproduce yet. Do you have a backtrace?
>>>> No. But the problem is probably the same as the one signaled by Henri,
>>>> a misplaced weak directive ending up in a symbol with no address at all.
>>>> Since the current situation works, I am going to wait for the "clean"
>>>> fix which puts some code/data in the src/skins/common directory.
>>>>
>>> Find it in my tree. But it's not yet well tested.
>> I do not like it either. Functions which are in src/skins/common should
>> still be weak, since this lib is included in all the skins libraries.
> 
> Those functions are now in libxeno_common only, so I see no point in
> allowing them to be overloaded.

Yes, but libxeno_common is included in libpthread_rt.so and
libnative.so. So, if you link with both libraries, you get
libxeno_common twice.

-- 
					    Gilles.

Re: [Xenomai-core] [Xenomai-git] Jan Kiszka : Make xnarch_init_timeconv an uninlined weak function

[Gilles Chanteperdrix]

Jan Kiszka wrote:
> If xnarch_init_timeconv shall be executed unconditionally, I will
> happily make it static inline again and call it from xeno_bind_skin_opt
> (which would imply retrieving sysinfo as well).

It is executed unconditionally, but needs to be available to kernel
space as well.


-- 
					    Gilles.

Re: [Xenomai-core] [Xenomai-git] Jan Kiszka : Make xnarch_init_timeconv an uninlined weak function

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 3091 bytes --]

Gilles Chanteperdrix wrote:
> Jan Kiszka wrote:
>> Gilles Chanteperdrix wrote:
>>> Jan Kiszka wrote:
>>>> Gilles Chanteperdrix wrote:
>>>>> Jan Kiszka wrote:
>>>>>> Gilles Chanteperdrix wrote:
>>>>>>> Jan Kiszka wrote:
>>>>>>>> Gilles Chanteperdrix wrote:
>>>>>>>>> GIT version control wrote:
>>>>>>>>>> Module: xenomai-jki
>>>>>>>>>> Branch: for-upstream
>>>>>>>>>> Commit: 6b40653e9c3c4a2433bb4e91344fc378eb860f75
>>>>>>>>>> URL:    http://git.xenomai.org/?p=xenomai-jki.git;a=commit;h=6b40653e9c3c4a2433bb4e91344fc378eb860f75
>>>>>>>>>>
>>>>>>>>>> Author: Jan Kiszka <jan.kiszka@domain.hid>
>>>>>>>>>> Date:   Wed Feb 10 13:24:29 2010 +0100
>>>>>>>>>>
>>>>>>>>>> Make xnarch_init_timeconv an uninlined weak function
>>>>>>>>>>
>>>>>>>>>> Otherwise the wrong set of time conversion variables might get
>>>>>>>>>> initialized when using > 1 skin libraries.
>>>>>>>>> If that would be possible, then it is the conversion variables which
>>>>>>>>> should made be weak, not the function.
>>>>>>>>>
>>>>>>>>> The way I see it, the posix and native skins currently get a different
>>>>>>>>> set of variables and functions, which works, but with your change, since
>>>>>>>>> there is only one function, only one set of variable gets initialized by
>>>>>>>>> the two function calls. And one skin just broke.
>>>>>>>>>
>>>>>>>>> Or am I missing something? Does the patch fix a problem you really had?
>>>>>>>> Frankly, I wasn't able to test in the field yet as replacing the libs
>>>>>>>> there is non-trivial. But I was able to observe that only one set of
>>>>>>>> functions is used - which is logical considering the weak marks. And
>>>>>>>> this breaks due to the static inline initialization.
>>>>>>>>
>>>>>>>> However, let's mark both functions and variables weak to fix the issue
>>>>>>>> and avoid leaving unused variables around. Will update my patch in a minute.
>>>>>>> Ok. I am reverting this patch until you provide me with another
>>>>>>> solution. It causes latency to segfault purely and simply at startup on
>>>>>>> my dual PIII.
>>>>>>>
>>>>>> Cannot reproduce yet. Do you have a backtrace?
>>>>> No. But the problem is probably the same as the one signaled by Henri,
>>>>> a misplaced weak directive ending up in a symbol with no address at all.
>>>>> Since the current situation works, I am going to wait for the "clean"
>>>>> fix which puts some code/data in the src/skins/common directory.
>>>>>
>>>> Find it in my tree. But it's not yet well tested.
>>> I do not like it either. Functions which are in src/skins/common should
>>> still be weak, since this lib is included in all the skins libraries.
>> Those functions are now in libxeno_common only, so I see no point in
>> allowing them to be overloaded.
> 
> Yes, but libxeno_common is included in libpthread_rt.so and
> libnative.so. So, if you link with both libraries, you get
> libxeno_common twice.
> 

Do we link libxeno_common statically? Otherwise, this conflict is not
logical to me. Also, there are other symbols in bind.c that are non-weak.

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]

Re: [Xenomai-core] [Xenomai-git] Jan Kiszka : Make xnarch_init_timeconv an uninlined weak function

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 418 bytes --]

Gilles Chanteperdrix wrote:
> Jan Kiszka wrote:
>> If xnarch_init_timeconv shall be executed unconditionally, I will
>> happily make it static inline again and call it from xeno_bind_skin_opt
>> (which would imply retrieving sysinfo as well).
> 
> It is executed unconditionally, but needs to be available to kernel
> space as well.

Not yet (it was POSIX- and Native-only), but we can change this.

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]

Re: [Xenomai-core] [Xenomai-git] Jan Kiszka : Make xnarch_init_timeconv an uninlined weak function

[Gilles Chanteperdrix]

Jan Kiszka wrote:
> Gilles Chanteperdrix wrote:
>> Jan Kiszka wrote:
>>> Gilles Chanteperdrix wrote:
>>>> Jan Kiszka wrote:
>>>>> Gilles Chanteperdrix wrote:
>>>>>> Jan Kiszka wrote:
>>>>>>> Gilles Chanteperdrix wrote:
>>>>>>>> Jan Kiszka wrote:
>>>>>>>>> Gilles Chanteperdrix wrote:
>>>>>>>>>> GIT version control wrote:
>>>>>>>>>>> Module: xenomai-jki
>>>>>>>>>>> Branch: for-upstream
>>>>>>>>>>> Commit: 6b40653e9c3c4a2433bb4e91344fc378eb860f75
>>>>>>>>>>> URL:    http://git.xenomai.org/?p=xenomai-jki.git;a=commit;h=6b40653e9c3c4a2433bb4e91344fc378eb860f75
>>>>>>>>>>>
>>>>>>>>>>> Author: Jan Kiszka <jan.kiszka@domain.hid>
>>>>>>>>>>> Date:   Wed Feb 10 13:24:29 2010 +0100
>>>>>>>>>>>
>>>>>>>>>>> Make xnarch_init_timeconv an uninlined weak function
>>>>>>>>>>>
>>>>>>>>>>> Otherwise the wrong set of time conversion variables might get
>>>>>>>>>>> initialized when using > 1 skin libraries.
>>>>>>>>>> If that would be possible, then it is the conversion variables which
>>>>>>>>>> should made be weak, not the function.
>>>>>>>>>>
>>>>>>>>>> The way I see it, the posix and native skins currently get a different
>>>>>>>>>> set of variables and functions, which works, but with your change, since
>>>>>>>>>> there is only one function, only one set of variable gets initialized by
>>>>>>>>>> the two function calls. And one skin just broke.
>>>>>>>>>>
>>>>>>>>>> Or am I missing something? Does the patch fix a problem you really had?
>>>>>>>>> Frankly, I wasn't able to test in the field yet as replacing the libs
>>>>>>>>> there is non-trivial. But I was able to observe that only one set of
>>>>>>>>> functions is used - which is logical considering the weak marks. And
>>>>>>>>> this breaks due to the static inline initialization.
>>>>>>>>>
>>>>>>>>> However, let's mark both functions and variables weak to fix the issue
>>>>>>>>> and avoid leaving unused variables around. Will update my patch in a minute.
>>>>>>>> Ok. I am reverting this patch until you provide me with another
>>>>>>>> solution. It causes latency to segfault purely and simply at startup on
>>>>>>>> my dual PIII.
>>>>>>>>
>>>>>>> Cannot reproduce yet. Do you have a backtrace?
>>>>>> No. But the problem is probably the same as the one signaled by Henri,
>>>>>> a misplaced weak directive ending up in a symbol with no address at all.
>>>>>> Since the current situation works, I am going to wait for the "clean"
>>>>>> fix which puts some code/data in the src/skins/common directory.
>>>>>>
>>>>> Find it in my tree. But it's not yet well tested.
>>>> I do not like it either. Functions which are in src/skins/common should
>>>> still be weak, since this lib is included in all the skins libraries.
>>> Those functions are now in libxeno_common only, so I see no point in
>>> allowing them to be overloaded.
>> Yes, but libxeno_common is included in libpthread_rt.so and
>> libnative.so. So, if you link with both libraries, you get
>> libxeno_common twice.
>>
> 
> Do we link libxeno_common statically? Otherwise, this conflict is not
> logical to me. Also, there are other symbols in bind.c that are non-weak.

libxeno_common is a "convenience library", which means that when
libpthread_rt.so is assembled, the object files from libxeno_common are
included.

I guess the loader eliminates the duplicates.

-- 
					    Gilles.

Re: [Xenomai-core] [Xenomai-git] Jan Kiszka : Make xnarch_init_timeconv an uninlined weak function

[Jan Kiszka]

Gilles Chanteperdrix wrote:
> Jan Kiszka wrote:
>> Gilles Chanteperdrix wrote:
>>> Jan Kiszka wrote:
>>>> Gilles Chanteperdrix wrote:
>>>>> Jan Kiszka wrote:
>>>>>> Gilles Chanteperdrix wrote:
>>>>>>> Jan Kiszka wrote:
>>>>>>>> Gilles Chanteperdrix wrote:
>>>>>>>>> Jan Kiszka wrote:
>>>>>>>>>> Gilles Chanteperdrix wrote:
>>>>>>>>>>> GIT version control wrote:
>>>>>>>>>>>> Module: xenomai-jki
>>>>>>>>>>>> Branch: for-upstream
>>>>>>>>>>>> Commit: 6b40653e9c3c4a2433bb4e91344fc378eb860f75
>>>>>>>>>>>> URL:    http://git.xenomai.org/?p=xenomai-jki.git;a=commit;h=6b40653e9c3c4a2433bb4e91344fc378eb860f75
>>>>>>>>>>>>
>>>>>>>>>>>> Author: Jan Kiszka <jan.kiszka@domain.hid>
>>>>>>>>>>>> Date:   Wed Feb 10 13:24:29 2010 +0100
>>>>>>>>>>>>
>>>>>>>>>>>> Make xnarch_init_timeconv an uninlined weak function
>>>>>>>>>>>>
>>>>>>>>>>>> Otherwise the wrong set of time conversion variables might get
>>>>>>>>>>>> initialized when using > 1 skin libraries.
>>>>>>>>>>> If that would be possible, then it is the conversion variables which
>>>>>>>>>>> should made be weak, not the function.
>>>>>>>>>>>
>>>>>>>>>>> The way I see it, the posix and native skins currently get a different
>>>>>>>>>>> set of variables and functions, which works, but with your change, since
>>>>>>>>>>> there is only one function, only one set of variable gets initialized by
>>>>>>>>>>> the two function calls. And one skin just broke.
>>>>>>>>>>>
>>>>>>>>>>> Or am I missing something? Does the patch fix a problem you really had?
>>>>>>>>>> Frankly, I wasn't able to test in the field yet as replacing the libs
>>>>>>>>>> there is non-trivial. But I was able to observe that only one set of
>>>>>>>>>> functions is used - which is logical considering the weak marks. And
>>>>>>>>>> this breaks due to the static inline initialization.
>>>>>>>>>>
>>>>>>>>>> However, let's mark both functions and variables weak to fix the issue
>>>>>>>>>> and avoid leaving unused variables around. Will update my patch in a minute.
>>>>>>>>> Ok. I am reverting this patch until you provide me with another
>>>>>>>>> solution. It causes latency to segfault purely and simply at startup on
>>>>>>>>> my dual PIII.
>>>>>>>>>
>>>>>>>> Cannot reproduce yet. Do you have a backtrace?
>>>>>>> No. But the problem is probably the same as the one signaled by Henri,
>>>>>>> a misplaced weak directive ending up in a symbol with no address at all.
>>>>>>> Since the current situation works, I am going to wait for the "clean"
>>>>>>> fix which puts some code/data in the src/skins/common directory.
>>>>>>>
>>>>>> Find it in my tree. But it's not yet well tested.
>>>>> I do not like it either. Functions which are in src/skins/common should
>>>>> still be weak, since this lib is included in all the skins libraries.
>>>> Those functions are now in libxeno_common only, so I see no point in
>>>> allowing them to be overloaded.
>>> Yes, but libxeno_common is included in libpthread_rt.so and
>>> libnative.so. So, if you link with both libraries, you get
>>> libxeno_common twice.
>>>
>> Do we link libxeno_common statically? Otherwise, this conflict is not
>> logical to me. Also, there are other symbols in bind.c that are non-weak.
> 
> libxeno_common is a "convenience library", which means that when
> libpthread_rt.so is assembled, the object files from libxeno_common are
> included.

BTW, what speaks against making it a dynamic library?

> 
> I guess the loader eliminates the duplicates.
> 

It has to. For the same reason, we should be able to clean up
skins/common/current.c now.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

Re: [Xenomai-core] [Xenomai-git] Jan Kiszka : Make xnarch_init_timeconv an uninlined weak function

[Gilles Chanteperdrix]

Jan Kiszka wrote:
> Gilles Chanteperdrix wrote:
>> Jan Kiszka wrote:
>>> Gilles Chanteperdrix wrote:
>>>> Jan Kiszka wrote:
>>>>> Gilles Chanteperdrix wrote:
>>>>>> Jan Kiszka wrote:
>>>>>>> Gilles Chanteperdrix wrote:
>>>>>>>> Jan Kiszka wrote:
>>>>>>>>> Gilles Chanteperdrix wrote:
>>>>>>>>>> Jan Kiszka wrote:
>>>>>>>>>>> Gilles Chanteperdrix wrote:
>>>>>>>>>>>> GIT version control wrote:
>>>>>>>>>>>>> Module: xenomai-jki
>>>>>>>>>>>>> Branch: for-upstream
>>>>>>>>>>>>> Commit: 6b40653e9c3c4a2433bb4e91344fc378eb860f75
>>>>>>>>>>>>> URL:    http://git.xenomai.org/?p=xenomai-jki.git;a=commit;h=6b40653e9c3c4a2433bb4e91344fc378eb860f75
>>>>>>>>>>>>>
>>>>>>>>>>>>> Author: Jan Kiszka <jan.kiszka@domain.hid>
>>>>>>>>>>>>> Date:   Wed Feb 10 13:24:29 2010 +0100
>>>>>>>>>>>>>
>>>>>>>>>>>>> Make xnarch_init_timeconv an uninlined weak function
>>>>>>>>>>>>>
>>>>>>>>>>>>> Otherwise the wrong set of time conversion variables might get
>>>>>>>>>>>>> initialized when using > 1 skin libraries.
>>>>>>>>>>>> If that would be possible, then it is the conversion variables which
>>>>>>>>>>>> should made be weak, not the function.
>>>>>>>>>>>>
>>>>>>>>>>>> The way I see it, the posix and native skins currently get a different
>>>>>>>>>>>> set of variables and functions, which works, but with your change, since
>>>>>>>>>>>> there is only one function, only one set of variable gets initialized by
>>>>>>>>>>>> the two function calls. And one skin just broke.
>>>>>>>>>>>>
>>>>>>>>>>>> Or am I missing something? Does the patch fix a problem you really had?
>>>>>>>>>>> Frankly, I wasn't able to test in the field yet as replacing the libs
>>>>>>>>>>> there is non-trivial. But I was able to observe that only one set of
>>>>>>>>>>> functions is used - which is logical considering the weak marks. And
>>>>>>>>>>> this breaks due to the static inline initialization.
>>>>>>>>>>>
>>>>>>>>>>> However, let's mark both functions and variables weak to fix the issue
>>>>>>>>>>> and avoid leaving unused variables around. Will update my patch in a minute.
>>>>>>>>>> Ok. I am reverting this patch until you provide me with another
>>>>>>>>>> solution. It causes latency to segfault purely and simply at startup on
>>>>>>>>>> my dual PIII.
>>>>>>>>>>
>>>>>>>>> Cannot reproduce yet. Do you have a backtrace?
>>>>>>>> No. But the problem is probably the same as the one signaled by Henri,
>>>>>>>> a misplaced weak directive ending up in a symbol with no address at all.
>>>>>>>> Since the current situation works, I am going to wait for the "clean"
>>>>>>>> fix which puts some code/data in the src/skins/common directory.
>>>>>>>>
>>>>>>> Find it in my tree. But it's not yet well tested.
>>>>>> I do not like it either. Functions which are in src/skins/common should
>>>>>> still be weak, since this lib is included in all the skins libraries.
>>>>> Those functions are now in libxeno_common only, so I see no point in
>>>>> allowing them to be overloaded.
>>>> Yes, but libxeno_common is included in libpthread_rt.so and
>>>> libnative.so. So, if you link with both libraries, you get
>>>> libxeno_common twice.
>>>>
>>> Do we link libxeno_common statically? Otherwise, this conflict is not
>>> logical to me. Also, there are other symbols in bind.c that are non-weak.
>> libxeno_common is a "convenience library", which means that when
>> libpthread_rt.so is assembled, the object files from libxeno_common are
>> included.
> 
> BTW, what speaks against making it a dynamic library?

Complications. Currently the way of linking a native application is to do:

`xeno-config --ldflags` -lnative

Since when linking, the order matter, we can not hide this new library
in xeno-config --ldflags, so we would have to turn this into:

`xeno-config --ldflags` -lnative -lxeno-common

i.e. breaking user way of building applications. I do not think the gain
is worth the trouble: linking against only one xenomai library looks to
me like the most common case.


> 
>> I guess the loader eliminates the duplicates.
>>
> 
> It has to. For the same reason, we should be able to clean up
> skins/common/current.c now.

I am not that confident about that change. By using the weak directive,
we make sure that the same __thread variable is used for instance. If we
do not do that, what prevents the code from using the local symbols in
each library ? IOW, removing the duplicate could work for libxeno_common
symbols when invoked externally (which probably almost never happens),
but not inside the skin libraries. I do not know about that, which is
why I kept the weak declarations.

-- 
					    Gilles.

Re: [Xenomai-core] [Xenomai-git] Jan Kiszka : Make xnarch_init_timeconv an uninlined weak function

[Jan Kiszka]

Gilles Chanteperdrix wrote:
> Jan Kiszka wrote:
>> Gilles Chanteperdrix wrote:
>>> Jan Kiszka wrote:
>>>> Gilles Chanteperdrix wrote:
>>>>> Jan Kiszka wrote:
>>>>>> Gilles Chanteperdrix wrote:
>>>>>>> Jan Kiszka wrote:
>>>>>>>> Gilles Chanteperdrix wrote:
>>>>>>>>> Jan Kiszka wrote:
>>>>>>>>>> Gilles Chanteperdrix wrote:
>>>>>>>>>>> Jan Kiszka wrote:
>>>>>>>>>>>> Gilles Chanteperdrix wrote:
>>>>>>>>>>>>> GIT version control wrote:
>>>>>>>>>>>>>> Module: xenomai-jki
>>>>>>>>>>>>>> Branch: for-upstream
>>>>>>>>>>>>>> Commit: 6b40653e9c3c4a2433bb4e91344fc378eb860f75
>>>>>>>>>>>>>> URL:    http://git.xenomai.org/?p=xenomai-jki.git;a=commit;h=6b40653e9c3c4a2433bb4e91344fc378eb860f75
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Author: Jan Kiszka <jan.kiszka@domain.hid>
>>>>>>>>>>>>>> Date:   Wed Feb 10 13:24:29 2010 +0100
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Make xnarch_init_timeconv an uninlined weak function
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Otherwise the wrong set of time conversion variables might get
>>>>>>>>>>>>>> initialized when using > 1 skin libraries.
>>>>>>>>>>>>> If that would be possible, then it is the conversion variables which
>>>>>>>>>>>>> should made be weak, not the function.
>>>>>>>>>>>>>
>>>>>>>>>>>>> The way I see it, the posix and native skins currently get a different
>>>>>>>>>>>>> set of variables and functions, which works, but with your change, since
>>>>>>>>>>>>> there is only one function, only one set of variable gets initialized by
>>>>>>>>>>>>> the two function calls. And one skin just broke.
>>>>>>>>>>>>>
>>>>>>>>>>>>> Or am I missing something? Does the patch fix a problem you really had?
>>>>>>>>>>>> Frankly, I wasn't able to test in the field yet as replacing the libs
>>>>>>>>>>>> there is non-trivial. But I was able to observe that only one set of
>>>>>>>>>>>> functions is used - which is logical considering the weak marks. And
>>>>>>>>>>>> this breaks due to the static inline initialization.
>>>>>>>>>>>>
>>>>>>>>>>>> However, let's mark both functions and variables weak to fix the issue
>>>>>>>>>>>> and avoid leaving unused variables around. Will update my patch in a minute.
>>>>>>>>>>> Ok. I am reverting this patch until you provide me with another
>>>>>>>>>>> solution. It causes latency to segfault purely and simply at startup on
>>>>>>>>>>> my dual PIII.
>>>>>>>>>>>
>>>>>>>>>> Cannot reproduce yet. Do you have a backtrace?
>>>>>>>>> No. But the problem is probably the same as the one signaled by Henri,
>>>>>>>>> a misplaced weak directive ending up in a symbol with no address at all.
>>>>>>>>> Since the current situation works, I am going to wait for the "clean"
>>>>>>>>> fix which puts some code/data in the src/skins/common directory.
>>>>>>>>>
>>>>>>>> Find it in my tree. But it's not yet well tested.
>>>>>>> I do not like it either. Functions which are in src/skins/common should
>>>>>>> still be weak, since this lib is included in all the skins libraries.
>>>>>> Those functions are now in libxeno_common only, so I see no point in
>>>>>> allowing them to be overloaded.
>>>>> Yes, but libxeno_common is included in libpthread_rt.so and
>>>>> libnative.so. So, if you link with both libraries, you get
>>>>> libxeno_common twice.
>>>>>
>>>> Do we link libxeno_common statically? Otherwise, this conflict is not
>>>> logical to me. Also, there are other symbols in bind.c that are non-weak.
>>> libxeno_common is a "convenience library", which means that when
>>> libpthread_rt.so is assembled, the object files from libxeno_common are
>>> included.
>> BTW, what speaks against making it a dynamic library?
> 
> Complications. Currently the way of linking a native application is to do:
> 
> `xeno-config --ldflags` -lnative
> 
> Since when linking, the order matter, we can not hide this new library
> in xeno-config --ldflags, so we would have to turn this into:
> 
> `xeno-config --ldflags` -lnative -lxeno-common
> 
> i.e. breaking user way of building applications. I do not think the gain
> is worth the trouble: linking against only one xenomai library looks to
> me like the most common case.

What would break when swapping -lnative and -xeno-common arbitrarily?

There is a _lot_ of cleanup potential. E.g. all that redundant __real
wrappers are good candidates for libxeno-common.

> 
> 
>>> I guess the loader eliminates the duplicates.
>>>
>> It has to. For the same reason, we should be able to clean up
>> skins/common/current.c now.
> 
> I am not that confident about that change. By using the weak directive,
> we make sure that the same __thread variable is used for instance. If we
> do not do that, what prevents the code from using the local symbols in
> each library ? IOW, removing the duplicate could work for libxeno_common
> symbols when invoked externally (which probably almost never happens),
> but not inside the skin libraries. I do not know about that, which is
> why I kept the weak declarations.

Mixing weak functions with non-weak variables can cause troubles as
well. Sticking our head into the sand is no good approach, understanding
the requirements for weak or not is better - and eliminating weak would
be best.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

Re: [Xenomai-core] [Xenomai-git] Jan Kiszka : Make xnarch_init_timeconv an uninlined weak function

[Gilles Chanteperdrix]

Jan Kiszka wrote:
> Gilles Chanteperdrix wrote:
>> Jan Kiszka wrote:
>>> Gilles Chanteperdrix wrote:
>>>> Jan Kiszka wrote:
>>>>> Gilles Chanteperdrix wrote:
>>>>>> Jan Kiszka wrote:
>>>>>>> Gilles Chanteperdrix wrote:
>>>>>>>> Jan Kiszka wrote:
>>>>>>>>> Gilles Chanteperdrix wrote:
>>>>>>>>>> Jan Kiszka wrote:
>>>>>>>>>>> Gilles Chanteperdrix wrote:
>>>>>>>>>>>> Jan Kiszka wrote:
>>>>>>>>>>>>> Gilles Chanteperdrix wrote:
>>>>>>>>>>>>>> GIT version control wrote:
>>>>>>>>>>>>>>> Module: xenomai-jki
>>>>>>>>>>>>>>> Branch: for-upstream
>>>>>>>>>>>>>>> Commit: 6b40653e9c3c4a2433bb4e91344fc378eb860f75
>>>>>>>>>>>>>>> URL:    http://git.xenomai.org/?p=xenomai-jki.git;a=commit;h=6b40653e9c3c4a2433bb4e91344fc378eb860f75
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Author: Jan Kiszka <jan.kiszka@domain.hid>
>>>>>>>>>>>>>>> Date:   Wed Feb 10 13:24:29 2010 +0100
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Make xnarch_init_timeconv an uninlined weak function
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Otherwise the wrong set of time conversion variables might get
>>>>>>>>>>>>>>> initialized when using > 1 skin libraries.
>>>>>>>>>>>>>> If that would be possible, then it is the conversion variables which
>>>>>>>>>>>>>> should made be weak, not the function.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> The way I see it, the posix and native skins currently get a different
>>>>>>>>>>>>>> set of variables and functions, which works, but with your change, since
>>>>>>>>>>>>>> there is only one function, only one set of variable gets initialized by
>>>>>>>>>>>>>> the two function calls. And one skin just broke.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Or am I missing something? Does the patch fix a problem you really had?
>>>>>>>>>>>>> Frankly, I wasn't able to test in the field yet as replacing the libs
>>>>>>>>>>>>> there is non-trivial. But I was able to observe that only one set of
>>>>>>>>>>>>> functions is used - which is logical considering the weak marks. And
>>>>>>>>>>>>> this breaks due to the static inline initialization.
>>>>>>>>>>>>>
>>>>>>>>>>>>> However, let's mark both functions and variables weak to fix the issue
>>>>>>>>>>>>> and avoid leaving unused variables around. Will update my patch in a minute.
>>>>>>>>>>>> Ok. I am reverting this patch until you provide me with another
>>>>>>>>>>>> solution. It causes latency to segfault purely and simply at startup on
>>>>>>>>>>>> my dual PIII.
>>>>>>>>>>>>
>>>>>>>>>>> Cannot reproduce yet. Do you have a backtrace?
>>>>>>>>>> No. But the problem is probably the same as the one signaled by Henri,
>>>>>>>>>> a misplaced weak directive ending up in a symbol with no address at all.
>>>>>>>>>> Since the current situation works, I am going to wait for the "clean"
>>>>>>>>>> fix which puts some code/data in the src/skins/common directory.
>>>>>>>>>>
>>>>>>>>> Find it in my tree. But it's not yet well tested.
>>>>>>>> I do not like it either. Functions which are in src/skins/common should
>>>>>>>> still be weak, since this lib is included in all the skins libraries.
>>>>>>> Those functions are now in libxeno_common only, so I see no point in
>>>>>>> allowing them to be overloaded.
>>>>>> Yes, but libxeno_common is included in libpthread_rt.so and
>>>>>> libnative.so. So, if you link with both libraries, you get
>>>>>> libxeno_common twice.
>>>>>>
>>>>> Do we link libxeno_common statically? Otherwise, this conflict is not
>>>>> logical to me. Also, there are other symbols in bind.c that are non-weak.
>>>> libxeno_common is a "convenience library", which means that when
>>>> libpthread_rt.so is assembled, the object files from libxeno_common are
>>>> included.
>>> BTW, what speaks against making it a dynamic library?
>> Complications. Currently the way of linking a native application is to do:
>>
>> `xeno-config --ldflags` -lnative
>>
>> Since when linking, the order matter, we can not hide this new library
>> in xeno-config --ldflags, so we would have to turn this into:
>>
>> `xeno-config --ldflags` -lnative -lxeno-common
>>
>> i.e. breaking user way of building applications. I do not think the gain
>> is worth the trouble: linking against only one xenomai library looks to
>> me like the most common case.
> 
> What would break when swapping -lnative and -xeno-common arbitrarily?

If you are linking with static libraries (which is the case that
matters), the linker would skip all objects from xeno-common completely
since none of their symbols are required, then fail after libnative
because of missing symbols (defined in libxeno_common).

> 
> There is a _lot_ of cleanup potential. E.g. all that redundant __real
> wrappers are good candidates for libxeno-common.

Thanks to the wrap-link.sh script, the __real wrappers are no longer
required. Besides, they are only needed for the posix skin, so there is
no need to put them in libxeno-common. This is something which needs to
be fixed too.

> 
>>
>>>> I guess the loader eliminates the duplicates.
>>>>
>>> It has to. For the same reason, we should be able to clean up
>>> skins/common/current.c now.
>> I am not that confident about that change. By using the weak directive,
>> we make sure that the same __thread variable is used for instance. If we
>> do not do that, what prevents the code from using the local symbols in
>> each library ? IOW, removing the duplicate could work for libxeno_common
>> symbols when invoked externally (which probably almost never happens),
>> but not inside the skin libraries. I do not know about that, which is
>> why I kept the weak declarations.
> 
> Mixing weak functions with non-weak variables can cause troubles as
> well. Sticking our head into the sand is no good approach, understanding
> the requirements for weak or not is better - and eliminating weak would
> be best.

Yes, all non static variables must be made weak. But this should already
be the case in libxeno_common.

Using the weak attribute is the best approach. Having symbols multiply
defined is a bad idea and not "standard compliant". The problem is not
to understand what happens with one instance of a toolchain, the problem
is to know whether the behaviour is the same on all the toolchains of
all the seven architectures supported by Xenomai. The --wrap directive
proved that not all versions of all toolchains on all platforms are equal.

The only alternative I would accept is a guaranteed portable ld
directive that would make all the symbols of the library automagically weak.


-- 
					    Gilles.

Re: [Xenomai-core] [Xenomai-git] Jan Kiszka : Make xnarch_init_timeconv an uninlined weak function

[Jan Kiszka]

Gilles Chanteperdrix wrote:
> Jan Kiszka wrote:
>> Gilles Chanteperdrix wrote:
>>> Jan Kiszka wrote:
>>>> Gilles Chanteperdrix wrote:
>>>>> Jan Kiszka wrote:
>>>>>> Gilles Chanteperdrix wrote:
>>>>>>> Jan Kiszka wrote:
>>>>>>>> Gilles Chanteperdrix wrote:
>>>>>>>>> Jan Kiszka wrote:
>>>>>>>>>> Gilles Chanteperdrix wrote:
>>>>>>>>>>> Jan Kiszka wrote:
>>>>>>>>>>>> Gilles Chanteperdrix wrote:
>>>>>>>>>>>>> Jan Kiszka wrote:
>>>>>>>>>>>>>> Gilles Chanteperdrix wrote:
>>>>>>>>>>>>>>> GIT version control wrote:
>>>>>>>>>>>>>>>> Module: xenomai-jki
>>>>>>>>>>>>>>>> Branch: for-upstream
>>>>>>>>>>>>>>>> Commit: 6b40653e9c3c4a2433bb4e91344fc378eb860f75
>>>>>>>>>>>>>>>> URL:    http://git.xenomai.org/?p=xenomai-jki.git;a=commit;h=6b40653e9c3c4a2433bb4e91344fc378eb860f75
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Author: Jan Kiszka <jan.kiszka@domain.hid>
>>>>>>>>>>>>>>>> Date:   Wed Feb 10 13:24:29 2010 +0100
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Make xnarch_init_timeconv an uninlined weak function
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Otherwise the wrong set of time conversion variables might get
>>>>>>>>>>>>>>>> initialized when using > 1 skin libraries.
>>>>>>>>>>>>>>> If that would be possible, then it is the conversion variables which
>>>>>>>>>>>>>>> should made be weak, not the function.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> The way I see it, the posix and native skins currently get a different
>>>>>>>>>>>>>>> set of variables and functions, which works, but with your change, since
>>>>>>>>>>>>>>> there is only one function, only one set of variable gets initialized by
>>>>>>>>>>>>>>> the two function calls. And one skin just broke.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Or am I missing something? Does the patch fix a problem you really had?
>>>>>>>>>>>>>> Frankly, I wasn't able to test in the field yet as replacing the libs
>>>>>>>>>>>>>> there is non-trivial. But I was able to observe that only one set of
>>>>>>>>>>>>>> functions is used - which is logical considering the weak marks. And
>>>>>>>>>>>>>> this breaks due to the static inline initialization.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> However, let's mark both functions and variables weak to fix the issue
>>>>>>>>>>>>>> and avoid leaving unused variables around. Will update my patch in a minute.
>>>>>>>>>>>>> Ok. I am reverting this patch until you provide me with another
>>>>>>>>>>>>> solution. It causes latency to segfault purely and simply at startup on
>>>>>>>>>>>>> my dual PIII.
>>>>>>>>>>>>>
>>>>>>>>>>>> Cannot reproduce yet. Do you have a backtrace?
>>>>>>>>>>> No. But the problem is probably the same as the one signaled by Henri,
>>>>>>>>>>> a misplaced weak directive ending up in a symbol with no address at all.
>>>>>>>>>>> Since the current situation works, I am going to wait for the "clean"
>>>>>>>>>>> fix which puts some code/data in the src/skins/common directory.
>>>>>>>>>>>
>>>>>>>>>> Find it in my tree. But it's not yet well tested.
>>>>>>>>> I do not like it either. Functions which are in src/skins/common should
>>>>>>>>> still be weak, since this lib is included in all the skins libraries.
>>>>>>>> Those functions are now in libxeno_common only, so I see no point in
>>>>>>>> allowing them to be overloaded.
>>>>>>> Yes, but libxeno_common is included in libpthread_rt.so and
>>>>>>> libnative.so. So, if you link with both libraries, you get
>>>>>>> libxeno_common twice.
>>>>>>>
>>>>>> Do we link libxeno_common statically? Otherwise, this conflict is not
>>>>>> logical to me. Also, there are other symbols in bind.c that are non-weak.
>>>>> libxeno_common is a "convenience library", which means that when
>>>>> libpthread_rt.so is assembled, the object files from libxeno_common are
>>>>> included.
>>>> BTW, what speaks against making it a dynamic library?
>>> Complications. Currently the way of linking a native application is to do:
>>>
>>> `xeno-config --ldflags` -lnative
>>>
>>> Since when linking, the order matter, we can not hide this new library
>>> in xeno-config --ldflags, so we would have to turn this into:
>>>
>>> `xeno-config --ldflags` -lnative -lxeno-common
>>>
>>> i.e. breaking user way of building applications. I do not think the gain
>>> is worth the trouble: linking against only one xenomai library looks to
>>> me like the most common case.
>> What would break when swapping -lnative and -xeno-common arbitrarily?
> 
> If you are linking with static libraries (which is the case that
> matters), the linker would skip all objects from xeno-common completely
> since none of their symbols are required, then fail after libnative
> because of missing symbols (defined in libxeno_common).

OK. But I this is a custom scenario and naturally requires careful
library ordering by the user anyway. For us the rule is simple: put
front-end libs to the front, push the xeno LDFLAGS to the end.

BTW, if you link via "`xeno-config --xeno-ldflags` -lnative --static",
things break already today (as pthread is processed before native).
Adding -lxeno_common to the output of xeno-config would not change the
situation.

> 
>> There is a _lot_ of cleanup potential. E.g. all that redundant __real
>> wrappers are good candidates for libxeno-common.
> 
> Thanks to the wrap-link.sh script, the __real wrappers are no longer
> required. Besides, they are only needed for the posix skin, so there is
> no need to put them in libxeno-common. This is something which needs to
> be fixed too.

wrap-link.sh is a convenience script, I don't think we could make its
use mandatory. So every skin that uses wrapped symbols should remain
hardened.

> 
>>>>> I guess the loader eliminates the duplicates.
>>>>>
>>>> It has to. For the same reason, we should be able to clean up
>>>> skins/common/current.c now.
>>> I am not that confident about that change. By using the weak directive,
>>> we make sure that the same __thread variable is used for instance. If we
>>> do not do that, what prevents the code from using the local symbols in
>>> each library ? IOW, removing the duplicate could work for libxeno_common
>>> symbols when invoked externally (which probably almost never happens),
>>> but not inside the skin libraries. I do not know about that, which is
>>> why I kept the weak declarations.
>> Mixing weak functions with non-weak variables can cause troubles as
>> well. Sticking our head into the sand is no good approach, understanding
>> the requirements for weak or not is better - and eliminating weak would
>> be best.
> 
> Yes, all non static variables must be made weak. But this should already
> be the case in libxeno_common.

Yes, and all static inlines must be made global or the static symbols
they access must become weak - that's where the bugs are hidden.

> 
> Using the weak attribute is the best approach. Having symbols multiply
> defined is a bad idea and not "standard compliant". The problem is not
> to understand what happens with one instance of a toolchain, the problem
> is to know whether the behaviour is the same on all the toolchains of
> all the seven architectures supported by Xenomai. The --wrap directive
> proved that not all versions of all toolchains on all platforms are equal.
> 
> The only alternative I would accept is a guaranteed portable ld
> directive that would make all the symbols of the library automagically weak.

I'm convinced that weak is the non-standard approach here. If we need to
mark commonly used functions weak, it indicates somethings is not yet
optimally organized.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

Re: [Xenomai-core] [Xenomai-git] Jan Kiszka : Make xnarch_init_timeconv an uninlined weak function

[Gilles Chanteperdrix]

Jan Kiszka wrote:
>>>>> BTW, what speaks against making it a dynamic library?
>>>> Complications. Currently the way of linking a native application is to do:
>>>>
>>>> `xeno-config --ldflags` -lnative
>>>>
>>>> Since when linking, the order matter, we can not hide this new library
>>>> in xeno-config --ldflags, so we would have to turn this into:
>>>>
>>>> `xeno-config --ldflags` -lnative -lxeno-common
>>>>
>>>> i.e. breaking user way of building applications. I do not think the gain
>>>> is worth the trouble: linking against only one xenomai library looks to
>>>> me like the most common case.
>>> What would break when swapping -lnative and -xeno-common arbitrarily?
>> If you are linking with static libraries (which is the case that
>> matters), the linker would skip all objects from xeno-common completely
>> since none of their symbols are required, then fail after libnative
>> because of missing symbols (defined in libxeno_common).
> 
> OK. But I this is a custom scenario and naturally requires careful
> library ordering by the user anyway. For us the rule is simple: put
> front-end libs to the front, push the xeno LDFLAGS to the end.

It does not fly either, the -L/usr/xenomai/lib is in the LDFLAGS, and
must be put before -lnative.

It would be nice to to add a --skin option to the xeno-config script,
which handles that. But again, we break the user interface.

that is:
xeno-config --skin=native --ldflags
would spit:
-L/usr/xenomai/lib -lnative -lxeno-common -lpthread -lrt

> 
> BTW, if you link via "`xeno-config --xeno-ldflags` -lnative --static",
> things break already today (as pthread is processed before native).
> Adding -lxeno_common to the output of xeno-config would not change the
> situation.

I guess it works because people usually use some pthread symbols in
their program already.

> 
>>> There is a _lot_ of cleanup potential. E.g. all that redundant __real
>>> wrappers are good candidates for libxeno-common.
>> Thanks to the wrap-link.sh script, the __real wrappers are no longer
>> required. Besides, they are only needed for the posix skin, so there is
>> no need to put them in libxeno-common. This is something which needs to
>> be fixed too.
> 
> wrap-link.sh is a convenience script, I don't think we could make its
> use mandatory. So every skin that uses wrapped symbols should remain
> hardened.

wrap-link.sh is already mandatory if you want to link statically with
the posix skin library. Which is also the case why the __real wrappers
exist at all.

> 
>>>>>> I guess the loader eliminates the duplicates.
>>>>>>
>>>>> It has to. For the same reason, we should be able to clean up
>>>>> skins/common/current.c now.
>>>> I am not that confident about that change. By using the weak directive,
>>>> we make sure that the same __thread variable is used for instance. If we
>>>> do not do that, what prevents the code from using the local symbols in
>>>> each library ? IOW, removing the duplicate could work for libxeno_common
>>>> symbols when invoked externally (which probably almost never happens),
>>>> but not inside the skin libraries. I do not know about that, which is
>>>> why I kept the weak declarations.
>>> Mixing weak functions with non-weak variables can cause troubles as
>>> well. Sticking our head into the sand is no good approach, understanding
>>> the requirements for weak or not is better - and eliminating weak would
>>> be best.
>> Yes, all non static variables must be made weak. But this should already
>> be the case in libxeno_common.
> 
> Yes, and all static inlines must be made global or the static symbols
> they access must become weak - that's where the bugs are hidden.

Not if the static symbols in all instances are initialized with the same
value (which is the case with xnarch_init_timeconv for instance).

> 
>> Using the weak attribute is the best approach. Having symbols multiply
>> defined is a bad idea and not "standard compliant". The problem is not
>> to understand what happens with one instance of a toolchain, the problem
>> is to know whether the behaviour is the same on all the toolchains of
>> all the seven architectures supported by Xenomai. The --wrap directive
>> proved that not all versions of all toolchains on all platforms are equal.
>>
>> The only alternative I would accept is a guaranteed portable ld
>> directive that would make all the symbols of the library automagically weak.
> 
> I'm convinced that weak is the non-standard approach here. If we need to
> mark commonly used functions weak, it indicates somethings is not yet
> optimally organized.

It is definitely non-standard. But works reliably the same way with gcc
on all platforms (well except if you declare some extern symbols weak,
but you are looking for trouble when doing that anyway).

I think we should not worry too much for these issues, as the current
situation works. So, we have plenty of time to decide what to do, and
more urgent issues to treat (such as the file descriptors situation, the
NTP stuff, or the posix signals to name a few).

-- 
					    Gilles.

Re: [Xenomai-core] [Xenomai-git] Jan Kiszka : Make xnarch_init_timeconv an uninlined weak function

[Jan Kiszka]

Gilles Chanteperdrix wrote:
> Jan Kiszka wrote:
>>>>>> BTW, what speaks against making it a dynamic library?
>>>>> Complications. Currently the way of linking a native application is to do:
>>>>>
>>>>> `xeno-config --ldflags` -lnative
>>>>>
>>>>> Since when linking, the order matter, we can not hide this new library
>>>>> in xeno-config --ldflags, so we would have to turn this into:
>>>>>
>>>>> `xeno-config --ldflags` -lnative -lxeno-common
>>>>>
>>>>> i.e. breaking user way of building applications. I do not think the gain
>>>>> is worth the trouble: linking against only one xenomai library looks to
>>>>> me like the most common case.
>>>> What would break when swapping -lnative and -xeno-common arbitrarily?
>>> If you are linking with static libraries (which is the case that
>>> matters), the linker would skip all objects from xeno-common completely
>>> since none of their symbols are required, then fail after libnative
>>> because of missing symbols (defined in libxeno_common).
>> OK. But I this is a custom scenario and naturally requires careful
>> library ordering by the user anyway. For us the rule is simple: put
>> front-end libs to the front, push the xeno LDFLAGS to the end.
> 
> It does not fly either, the -L/usr/xenomai/lib is in the LDFLAGS, and
> must be put before -lnative.

But that must be some old toolchain issue. Normally, that doesn't matter
at all as the library path are first collected, and then linking starts.

> 
> It would be nice to to add a --skin option to the xeno-config script,
> which handles that. But again, we break the user interface.
> 
> that is:
> xeno-config --skin=native --ldflags
> would spit:
> -L/usr/xenomai/lib -lnative -lxeno-common -lpthread -lrt

For sure, that would make it most robust against user mistakes.

> 
>> BTW, if you link via "`xeno-config --xeno-ldflags` -lnative --static",
>> things break already today (as pthread is processed before native).
>> Adding -lxeno_common to the output of xeno-config would not change the
>> situation.
> 
> I guess it works because people usually use some pthread symbols in
> their program already.
> 
>>>> There is a _lot_ of cleanup potential. E.g. all that redundant __real
>>>> wrappers are good candidates for libxeno-common.
>>> Thanks to the wrap-link.sh script, the __real wrappers are no longer
>>> required. Besides, they are only needed for the posix skin, so there is
>>> no need to put them in libxeno-common. This is something which needs to
>>> be fixed too.
>> wrap-link.sh is a convenience script, I don't think we could make its
>> use mandatory. So every skin that uses wrapped symbols should remain
>> hardened.
> 
> wrap-link.sh is already mandatory if you want to link statically with
> the posix skin library. Which is also the case why the __real wrappers
> exist at all.
> 
>>>>>>> I guess the loader eliminates the duplicates.
>>>>>>>
>>>>>> It has to. For the same reason, we should be able to clean up
>>>>>> skins/common/current.c now.
>>>>> I am not that confident about that change. By using the weak directive,
>>>>> we make sure that the same __thread variable is used for instance. If we
>>>>> do not do that, what prevents the code from using the local symbols in
>>>>> each library ? IOW, removing the duplicate could work for libxeno_common
>>>>> symbols when invoked externally (which probably almost never happens),
>>>>> but not inside the skin libraries. I do not know about that, which is
>>>>> why I kept the weak declarations.
>>>> Mixing weak functions with non-weak variables can cause troubles as
>>>> well. Sticking our head into the sand is no good approach, understanding
>>>> the requirements for weak or not is better - and eliminating weak would
>>>> be best.
>>> Yes, all non static variables must be made weak. But this should already
>>> be the case in libxeno_common.
>> Yes, and all static inlines must be made global or the static symbols
>> they access must become weak - that's where the bugs are hidden.
> 
> Not if the static symbols in all instances are initialized with the same
> value (which is the case with xnarch_init_timeconv for instance).

And instantly - you forget about dlopen scenarios which triggered the
bug in timeconv.

> 
>>> Using the weak attribute is the best approach. Having symbols multiply
>>> defined is a bad idea and not "standard compliant". The problem is not
>>> to understand what happens with one instance of a toolchain, the problem
>>> is to know whether the behaviour is the same on all the toolchains of
>>> all the seven architectures supported by Xenomai. The --wrap directive
>>> proved that not all versions of all toolchains on all platforms are equal.
>>>
>>> The only alternative I would accept is a guaranteed portable ld
>>> directive that would make all the symbols of the library automagically weak.
>> I'm convinced that weak is the non-standard approach here. If we need to
>> mark commonly used functions weak, it indicates somethings is not yet
>> optimally organized.
> 
> It is definitely non-standard. But works reliably the same way with gcc
> on all platforms (well except if you declare some extern symbols weak,
> but you are looking for trouble when doing that anyway).
> 
> I think we should not worry too much for these issues, as the current
> situation works. So, we have plenty of time to decide what to do, and
> more urgent issues to treat (such as the file descriptors situation, the
> NTP stuff, or the posix signals to name a few).

Well, I still have a bug to fix, and doing the requested cleanups just
revealed more need for cleanups. So I'm now trying to find a reasonable
point to stop the cleanups (for 2.5).

Please have a look if the version I just pushed gets closer to what is
acceptable.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

Re: [Xenomai-core] [Xenomai-git] Jan Kiszka : Make xnarch_init_timeconv an uninlined weak function

[Gilles Chanteperdrix]

Jan Kiszka wrote:
> And instantly - you forget about dlopen scenarios which triggered the
> bug in timeconv.

I do not see why dlopen would trigger a bug. Could you explain it?

-- 
					    Gilles.

Re: [Xenomai-core] [Xenomai-git] Jan Kiszka : Make xnarch_init_timeconv an uninlined weak function

[Jan Kiszka]

Gilles Chanteperdrix wrote:
> Jan Kiszka wrote:
>> And instantly - you forget about dlopen scenarios which triggered the
>> bug in timeconv.
> 
> I do not see why dlopen would trigger a bug. Could you explain it?

If you dlopen, say, libnative and call a symbol that uses some timeconv
constants, you have to make sure that the init code of libnative
initializes those variables that are later used. Currently this is not
the case as init_timeconv works on libnative variables while the weak
conversion functions may work on a possibly (in our case really)
uninitialized set of some other skin. We can work around it, but it
remains a bug.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

Re: [Xenomai-core] [Xenomai-git] Jan Kiszka : Make xnarch_init_timeconv an uninlined weak function

[Gilles Chanteperdrix]

Jan Kiszka wrote:
> Gilles Chanteperdrix wrote:
>> Jan Kiszka wrote:
>>> And instantly - you forget about dlopen scenarios which triggered the
>>> bug in timeconv.
>> I do not see why dlopen would trigger a bug. Could you explain it?
> 
> If you dlopen, say, libnative and call a symbol that uses some timeconv
> constants, you have to make sure that the init code of libnative
> initializes those variables that are later used.

That is where I do not follow you. dlopen is called after the startup of
the other libs, so either it references its own variables, which are
initialized once its constructor has been called. Or it references the
variables of the posix lib (only native and posix use timeconv in
user-space) which is already initialized.

-- 
					    Gilles.

Re: [Xenomai-core] [Xenomai-git] Jan Kiszka : Make xnarch_init_timeconv an uninlined weak function

[Jan Kiszka]

Gilles Chanteperdrix wrote:
> Jan Kiszka wrote:
>> Gilles Chanteperdrix wrote:
>>> Jan Kiszka wrote:
>>>> And instantly - you forget about dlopen scenarios which triggered the
>>>> bug in timeconv.
>>> I do not see why dlopen would trigger a bug. Could you explain it?
>> If you dlopen, say, libnative and call a symbol that uses some timeconv
>> constants, you have to make sure that the init code of libnative
>> initializes those variables that are later used.
> 
> That is where I do not follow you. dlopen is called after the startup of
> the other libs, so either it references its own variables, which are
> initialized once its constructor has been called. Or it references the
> variables of the posix lib (only native and posix use timeconv in
> user-space) which is already initialized.

In this case, dlopen was part of some constructor, and the ordering
turned out to be "unfortunate".

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

Re: [Xenomai-core] [Xenomai-git] Jan Kiszka : Make xnarch_init_timeconv an uninlined weak function

[Gilles Chanteperdrix]

Jan Kiszka wrote:
> Gilles Chanteperdrix wrote:
>> Jan Kiszka wrote:
>>> Gilles Chanteperdrix wrote:
>>>> Jan Kiszka wrote:
>>>>> And instantly - you forget about dlopen scenarios which triggered the
>>>>> bug in timeconv.
>>>> I do not see why dlopen would trigger a bug. Could you explain it?
>>> If you dlopen, say, libnative and call a symbol that uses some timeconv
>>> constants, you have to make sure that the init code of libnative
>>> initializes those variables that are later used.
>> That is where I do not follow you. dlopen is called after the startup of
>> the other libs, so either it references its own variables, which are
>> initialized once its constructor has been called. Or it references the
>> variables of the posix lib (only native and posix use timeconv in
>> user-space) which is already initialized.
> 
> In this case, dlopen was part of some constructor, and the ordering
> turned out to be "unfortunate".

Ok. Got it now. Will test your patch, but I will probably leave the weak
attribute to the shared functions.

-- 
					    Gilles.

Re: [Xenomai-core] [Xenomai-git] Jan Kiszka : Make xnarch_init_timeconv an uninlined weak function

[Jan Kiszka]

Gilles Chanteperdrix wrote:
> Jan Kiszka wrote:
>> Gilles Chanteperdrix wrote:
>>> Jan Kiszka wrote:
>>>> Gilles Chanteperdrix wrote:
>>>>> Jan Kiszka wrote:
>>>>>> And instantly - you forget about dlopen scenarios which triggered the
>>>>>> bug in timeconv.
>>>>> I do not see why dlopen would trigger a bug. Could you explain it?
>>>> If you dlopen, say, libnative and call a symbol that uses some timeconv
>>>> constants, you have to make sure that the init code of libnative
>>>> initializes those variables that are later used.
>>> That is where I do not follow you. dlopen is called after the startup of
>>> the other libs, so either it references its own variables, which are
>>> initialized once its constructor has been called. Or it references the
>>> variables of the posix lib (only native and posix use timeconv in
>>> user-space) which is already initialized.
>> In this case, dlopen was part of some constructor, and the ordering
>> turned out to be "unfortunate".
> 
> Ok. Got it now. Will test your patch, but I will probably leave the weak
> attribute to the shared functions.

Well, either all or nothing: If you leave it to the runtime services,
you also have to tag the init function (that's what my very first tiny
patch did).

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux