Re: [Xenomai] [Xenomai-git] Jan Kiszka : lib/cobalt: Rework minimum stack size enforcement

Re: [Xenomai] [Xenomai-git] Jan Kiszka : lib/cobalt: Rework minimum stack size enforcement

[Gilles Chanteperdrix]

On Tue, Apr 21, 2015 at 05:22:34PM +0200, git repository hosting wrote:
> +COBALT_IMPL(int, pthread_attr_setstack, (pthread_attr_t *attr, void *stackaddr,
> +					 size_t stacksize))
> +{
> +	if (stacksize < COBALT_STACKSIZE_MIN)
> +		return -EINVAL;
> +	return __STD(pthread_attr_setstack)(attr, stackaddr, stacksize);
> +}

As I said, there is no reason for Xenomai to enforce a task size
larger than PTHREAD_STACK_MIN. This breaks POSIX compliance, and is
useless.

-- 
					    Gilles.

Re: [Xenomai] [Xenomai-git] Jan Kiszka : lib/cobalt: Rework minimum stack size enforcement

[Jan Kiszka]

On 2015-04-21 17:27, Gilles Chanteperdrix wrote:
> On Tue, Apr 21, 2015 at 05:22:34PM +0200, git repository hosting wrote:
>> +COBALT_IMPL(int, pthread_attr_setstack, (pthread_attr_t *attr, void *stackaddr,
>> +					 size_t stacksize))
>> +{
>> +	if (stacksize < COBALT_STACKSIZE_MIN)
>> +		return -EINVAL;
>> +	return __STD(pthread_attr_setstack)(attr, stackaddr, stacksize);
>> +}
> 
> As I said, there is no reason for Xenomai to enforce a task size
> larger than PTHREAD_STACK_MIN. This breaks POSIX compliance, and is
> useless.

Then we should not use printf & Co. from any trampoline function, e.g.
vxworks/taskLib.c:task_trampoline. I only tried with a local skin, but I
bet the issue is reproducible with that one.

For that reason, I went down the conservative path.

Jan

-- 
Siemens AG, Corporate Technology, CT RTC ITP SES-DE
Corporate Competence Center Embedded Linux

Re: [Xenomai] [Xenomai-git] Jan Kiszka : lib/cobalt: Rework minimum stack size enforcement

[Gilles Chanteperdrix]

On Tue, Apr 21, 2015 at 06:04:42PM +0200, Jan Kiszka wrote:
> On 2015-04-21 17:27, Gilles Chanteperdrix wrote:
> > On Tue, Apr 21, 2015 at 05:22:34PM +0200, git repository hosting wrote:
> >> +COBALT_IMPL(int, pthread_attr_setstack, (pthread_attr_t *attr, void *stackaddr,
> >> +					 size_t stacksize))
> >> +{
> >> +	if (stacksize < COBALT_STACKSIZE_MIN)
> >> +		return -EINVAL;
> >> +	return __STD(pthread_attr_setstack)(attr, stackaddr, stacksize);
> >> +}
> > 
> > As I said, there is no reason for Xenomai to enforce a task size
> > larger than PTHREAD_STACK_MIN. This breaks POSIX compliance, and is
> > useless.
> 
> Then we should not use printf & Co. from any trampoline function, e.g.
> vxworks/taskLib.c:task_trampoline. I only tried with a local skin, but I
> bet the issue is reproducible with that one.
> 
> For that reason, I went down the conservative path.

The path you took breaks compatibility with POSIX. On a compliant
system, pthread_attr_setstacksize(attr, PTHREAD_STACK_MIN) is not
supposed to return an error.

-- 
					    Gilles.

Re: [Xenomai] [Xenomai-git] Jan Kiszka : lib/cobalt: Rework minimum stack size enforcement

[Jan Kiszka]

On 2015-04-21 18:06, Gilles Chanteperdrix wrote:
> On Tue, Apr 21, 2015 at 06:04:42PM +0200, Jan Kiszka wrote:
>> On 2015-04-21 17:27, Gilles Chanteperdrix wrote:
>>> On Tue, Apr 21, 2015 at 05:22:34PM +0200, git repository hosting wrote:
>>>> +COBALT_IMPL(int, pthread_attr_setstack, (pthread_attr_t *attr, void *stackaddr,
>>>> +					 size_t stacksize))
>>>> +{
>>>> +	if (stacksize < COBALT_STACKSIZE_MIN)
>>>> +		return -EINVAL;
>>>> +	return __STD(pthread_attr_setstack)(attr, stackaddr, stacksize);
>>>> +}
>>>
>>> As I said, there is no reason for Xenomai to enforce a task size
>>> larger than PTHREAD_STACK_MIN. This breaks POSIX compliance, and is
>>> useless.
>>
>> Then we should not use printf & Co. from any trampoline function, e.g.
>> vxworks/taskLib.c:task_trampoline. I only tried with a local skin, but I
>> bet the issue is reproducible with that one.
>>
>> For that reason, I went down the conservative path.
> 
> The path you took breaks compatibility with POSIX. On a compliant
> system, pthread_attr_setstacksize(attr, PTHREAD_STACK_MIN) is not
> supposed to return an error.

That is true. We have to pick from one evil: fail non-compliant here or
crash later.

Jan

-- 
Siemens AG, Corporate Technology, CT RTC ITP SES-DE
Corporate Competence Center Embedded Linux

Re: [Xenomai] [Xenomai-git] Jan Kiszka : lib/cobalt: Rework minimum stack size enforcement

[Gilles Chanteperdrix]

On Tue, Apr 21, 2015 at 06:12:15PM +0200, Jan Kiszka wrote:
> On 2015-04-21 18:06, Gilles Chanteperdrix wrote:
> > On Tue, Apr 21, 2015 at 06:04:42PM +0200, Jan Kiszka wrote:
> >> On 2015-04-21 17:27, Gilles Chanteperdrix wrote:
> >>> On Tue, Apr 21, 2015 at 05:22:34PM +0200, git repository hosting wrote:
> >>>> +COBALT_IMPL(int, pthread_attr_setstack, (pthread_attr_t *attr, void *stackaddr,
> >>>> +					 size_t stacksize))
> >>>> +{
> >>>> +	if (stacksize < COBALT_STACKSIZE_MIN)
> >>>> +		return -EINVAL;
> >>>> +	return __STD(pthread_attr_setstack)(attr, stackaddr, stacksize);
> >>>> +}
> >>>
> >>> As I said, there is no reason for Xenomai to enforce a task size
> >>> larger than PTHREAD_STACK_MIN. This breaks POSIX compliance, and is
> >>> useless.
> >>
> >> Then we should not use printf & Co. from any trampoline function, e.g.
> >> vxworks/taskLib.c:task_trampoline. I only tried with a local skin, but I
> >> bet the issue is reproducible with that one.
> >>
> >> For that reason, I went down the conservative path.
> > 
> > The path you took breaks compatibility with POSIX. On a compliant
> > system, pthread_attr_setstacksize(attr, PTHREAD_STACK_MIN) is not
> > supposed to return an error.
> 
> That is true. We have to pick from one evil: fail non-compliant here or
> crash later.

The point is: you do not know if it will crash later, you can assume
that the user calling pthread_attr_setstack knows what he is doing.
Any user calling this interface should. We are doing real-time in C,
not making an API for tutorials in ADA, or C++, let us try not to
prevent the user from doing what he wants.

For pthread_attr_setstacksize, you can round to the larger size and
remain POSIX compliant.

-- 
					    Gilles.

Re: [Xenomai] [Xenomai-git] Jan Kiszka : lib/cobalt: Rework minimum stack size enforcement

[Jan Kiszka]

On 2015-04-21 18:16, Gilles Chanteperdrix wrote:
> On Tue, Apr 21, 2015 at 06:12:15PM +0200, Jan Kiszka wrote:
>> On 2015-04-21 18:06, Gilles Chanteperdrix wrote:
>>> On Tue, Apr 21, 2015 at 06:04:42PM +0200, Jan Kiszka wrote:
>>>> On 2015-04-21 17:27, Gilles Chanteperdrix wrote:
>>>>> On Tue, Apr 21, 2015 at 05:22:34PM +0200, git repository hosting wrote:
>>>>>> +COBALT_IMPL(int, pthread_attr_setstack, (pthread_attr_t *attr, void *stackaddr,
>>>>>> +					 size_t stacksize))
>>>>>> +{
>>>>>> +	if (stacksize < COBALT_STACKSIZE_MIN)
>>>>>> +		return -EINVAL;
>>>>>> +	return __STD(pthread_attr_setstack)(attr, stackaddr, stacksize);
>>>>>> +}
>>>>>
>>>>> As I said, there is no reason for Xenomai to enforce a task size
>>>>> larger than PTHREAD_STACK_MIN. This breaks POSIX compliance, and is
>>>>> useless.
>>>>
>>>> Then we should not use printf & Co. from any trampoline function, e.g.
>>>> vxworks/taskLib.c:task_trampoline. I only tried with a local skin, but I
>>>> bet the issue is reproducible with that one.
>>>>
>>>> For that reason, I went down the conservative path.
>>>
>>> The path you took breaks compatibility with POSIX. On a compliant
>>> system, pthread_attr_setstacksize(attr, PTHREAD_STACK_MIN) is not
>>> supposed to return an error.
>>
>> That is true. We have to pick from one evil: fail non-compliant here or
>> crash later.
> 
> The point is: you do not know if it will crash later, you can assume
> that the user calling pthread_attr_setstack knows what he is doing.

The crash can be caused by the the library code and, thus, can happen
even if the application code would not crash. That is my problem.

> Any user calling this interface should. We are doing real-time in C,
> not making an API for tutorials in ADA, or C++, let us try not to
> prevent the user from doing what he wants.
> 
> For pthread_attr_setstacksize, you can round to the larger size and
> remain POSIX compliant.

Is it specified that this call invalidate a stack address set earlier? I
don't see this.

Jan

-- 
Siemens AG, Corporate Technology, CT RTC ITP SES-DE
Corporate Competence Center Embedded Linux

Re: [Xenomai] [Xenomai-git] Jan Kiszka : lib/cobalt: Rework minimum stack size enforcement

[Gilles Chanteperdrix]

On Tue, Apr 21, 2015 at 06:20:28PM +0200, Jan Kiszka wrote:
> On 2015-04-21 18:16, Gilles Chanteperdrix wrote:
> > On Tue, Apr 21, 2015 at 06:12:15PM +0200, Jan Kiszka wrote:
> >> On 2015-04-21 18:06, Gilles Chanteperdrix wrote:
> >>> On Tue, Apr 21, 2015 at 06:04:42PM +0200, Jan Kiszka wrote:
> >>>> On 2015-04-21 17:27, Gilles Chanteperdrix wrote:
> >>>>> On Tue, Apr 21, 2015 at 05:22:34PM +0200, git repository hosting wrote:
> >>>>>> +COBALT_IMPL(int, pthread_attr_setstack, (pthread_attr_t *attr, void *stackaddr,
> >>>>>> +					 size_t stacksize))
> >>>>>> +{
> >>>>>> +	if (stacksize < COBALT_STACKSIZE_MIN)
> >>>>>> +		return -EINVAL;
> >>>>>> +	return __STD(pthread_attr_setstack)(attr, stackaddr, stacksize);
> >>>>>> +}
> >>>>>
> >>>>> As I said, there is no reason for Xenomai to enforce a task size
> >>>>> larger than PTHREAD_STACK_MIN. This breaks POSIX compliance, and is
> >>>>> useless.
> >>>>
> >>>> Then we should not use printf & Co. from any trampoline function, e.g.
> >>>> vxworks/taskLib.c:task_trampoline. I only tried with a local skin, but I
> >>>> bet the issue is reproducible with that one.
> >>>>
> >>>> For that reason, I went down the conservative path.
> >>>
> >>> The path you took breaks compatibility with POSIX. On a compliant
> >>> system, pthread_attr_setstacksize(attr, PTHREAD_STACK_MIN) is not
> >>> supposed to return an error.
> >>
> >> That is true. We have to pick from one evil: fail non-compliant here or
> >> crash later.
> > 
> > The point is: you do not know if it will crash later, you can assume
> > that the user calling pthread_attr_setstack knows what he is doing.
> 
> The crash can be caused by the the library code and, thus, can happen
> even if the application code would not crash. That is my problem.

Yes, so what, increase the stack. The same goes with glibc when
using printf, you know that PTHREAD_STACK_MIN is not sufficient, so
you should pass a larger size. I suspect it goes this way for any
posix implementation. Once again, if an applications chooses to use
these calls, it should know what it is doing.

> 
> > Any user calling this interface should. We are doing real-time in C,
> > not making an API for tutorials in ADA, or C++, let us try not to
> > prevent the user from doing what he wants.
> > 
> > For pthread_attr_setstacksize, you can round to the larger size and
> > remain POSIX compliant.
> 
> Is it specified that this call invalidate a stack address set earlier? I
> don't see this.

The way I understand the spec, the stack and stacksize attribute are
separate, and setting stacksize sets the size of the stack that will
be allocated automatically. Now if you set both, maybe stacksize is
ignored and the sized passed to setstack is used. But I would not
expect the size passed to setstack to be used for the stacksize
attribute.

-- 
					    Gilles.

Re: [Xenomai] [Xenomai-git] Jan Kiszka : lib/cobalt: Rework minimum stack size enforcement

[Jan Kiszka]

On 2015-04-21 18:32, Gilles Chanteperdrix wrote:
> On Tue, Apr 21, 2015 at 06:20:28PM +0200, Jan Kiszka wrote:
>> On 2015-04-21 18:16, Gilles Chanteperdrix wrote:
>>> On Tue, Apr 21, 2015 at 06:12:15PM +0200, Jan Kiszka wrote:
>>>> On 2015-04-21 18:06, Gilles Chanteperdrix wrote:
>>>>> On Tue, Apr 21, 2015 at 06:04:42PM +0200, Jan Kiszka wrote:
>>>>>> On 2015-04-21 17:27, Gilles Chanteperdrix wrote:
>>>>>>> On Tue, Apr 21, 2015 at 05:22:34PM +0200, git repository hosting wrote:
>>>>>>>> +COBALT_IMPL(int, pthread_attr_setstack, (pthread_attr_t *attr, void *stackaddr,
>>>>>>>> +					 size_t stacksize))
>>>>>>>> +{
>>>>>>>> +	if (stacksize < COBALT_STACKSIZE_MIN)
>>>>>>>> +		return -EINVAL;
>>>>>>>> +	return __STD(pthread_attr_setstack)(attr, stackaddr, stacksize);
>>>>>>>> +}
>>>>>>>
>>>>>>> As I said, there is no reason for Xenomai to enforce a task size
>>>>>>> larger than PTHREAD_STACK_MIN. This breaks POSIX compliance, and is
>>>>>>> useless.
>>>>>>
>>>>>> Then we should not use printf & Co. from any trampoline function, e.g.
>>>>>> vxworks/taskLib.c:task_trampoline. I only tried with a local skin, but I
>>>>>> bet the issue is reproducible with that one.
>>>>>>
>>>>>> For that reason, I went down the conservative path.
>>>>>
>>>>> The path you took breaks compatibility with POSIX. On a compliant
>>>>> system, pthread_attr_setstacksize(attr, PTHREAD_STACK_MIN) is not
>>>>> supposed to return an error.
>>>>
>>>> That is true. We have to pick from one evil: fail non-compliant here or
>>>> crash later.
>>>
>>> The point is: you do not know if it will crash later, you can assume
>>> that the user calling pthread_attr_setstack knows what he is doing.
>>
>> The crash can be caused by the the library code and, thus, can happen
>> even if the application code would not crash. That is my problem.
> 
> Yes, so what, increase the stack. The same goes with glibc when
> using printf, you know that PTHREAD_STACK_MIN is not sufficient, so
> you should pass a larger size. I suspect it goes this way for any
> posix implementation. Once again, if an applications chooses to use
> these calls, it should know what it is doing.

Possibly the crash was limited to the case where the application set a
stack address and Xenomai messed up the size. I'm rechecking this right
now, and if we are lucky, PTHREAD_STACK_MIN turns out to be fine for
Xenomai as well.

> 
>>
>>> Any user calling this interface should. We are doing real-time in C,
>>> not making an API for tutorials in ADA, or C++, let us try not to
>>> prevent the user from doing what he wants.
>>>
>>> For pthread_attr_setstacksize, you can round to the larger size and
>>> remain POSIX compliant.
>>
>> Is it specified that this call invalidate a stack address set earlier? I
>> don't see this.
> 
> The way I understand the spec, the stack and stacksize attribute are
> separate, and setting stacksize sets the size of the stack that will
> be allocated automatically. Now if you set both, maybe stacksize is
> ignored and the sized passed to setstack is used. But I would not
> expect the size passed to setstack to be used for the stacksize
> attribute.

glibc uses the same internal stacksize field for both calls.

Jan

-- 
Siemens AG, Corporate Technology, CT RTC ITP SES-DE
Corporate Competence Center Embedded Linux

Re: [Xenomai] [Xenomai-git] Jan Kiszka : lib/cobalt: Rework minimum stack size enforcement

[Gilles Chanteperdrix]

On Tue, Apr 21, 2015 at 06:37:04PM +0200, Jan Kiszka wrote:
> On 2015-04-21 18:32, Gilles Chanteperdrix wrote:
> > On Tue, Apr 21, 2015 at 06:20:28PM +0200, Jan Kiszka wrote:
> >> On 2015-04-21 18:16, Gilles Chanteperdrix wrote:
> >>> On Tue, Apr 21, 2015 at 06:12:15PM +0200, Jan Kiszka wrote:
> >>>> On 2015-04-21 18:06, Gilles Chanteperdrix wrote:
> >>>>> On Tue, Apr 21, 2015 at 06:04:42PM +0200, Jan Kiszka wrote:
> >>>>>> On 2015-04-21 17:27, Gilles Chanteperdrix wrote:
> >>>>>>> On Tue, Apr 21, 2015 at 05:22:34PM +0200, git repository hosting wrote:
> >>>>>>>> +COBALT_IMPL(int, pthread_attr_setstack, (pthread_attr_t *attr, void *stackaddr,
> >>>>>>>> +					 size_t stacksize))
> >>>>>>>> +{
> >>>>>>>> +	if (stacksize < COBALT_STACKSIZE_MIN)
> >>>>>>>> +		return -EINVAL;
> >>>>>>>> +	return __STD(pthread_attr_setstack)(attr, stackaddr, stacksize);
> >>>>>>>> +}
> >>>>>>>
> >>>>>>> As I said, there is no reason for Xenomai to enforce a task size
> >>>>>>> larger than PTHREAD_STACK_MIN. This breaks POSIX compliance, and is
> >>>>>>> useless.
> >>>>>>
> >>>>>> Then we should not use printf & Co. from any trampoline function, e.g.
> >>>>>> vxworks/taskLib.c:task_trampoline. I only tried with a local skin, but I
> >>>>>> bet the issue is reproducible with that one.
> >>>>>>
> >>>>>> For that reason, I went down the conservative path.
> >>>>>
> >>>>> The path you took breaks compatibility with POSIX. On a compliant
> >>>>> system, pthread_attr_setstacksize(attr, PTHREAD_STACK_MIN) is not
> >>>>> supposed to return an error.
> >>>>
> >>>> That is true. We have to pick from one evil: fail non-compliant here or
> >>>> crash later.
> >>>
> >>> The point is: you do not know if it will crash later, you can assume
> >>> that the user calling pthread_attr_setstack knows what he is doing.
> >>
> >> The crash can be caused by the the library code and, thus, can happen
> >> even if the application code would not crash. That is my problem.
> > 
> > Yes, so what, increase the stack. The same goes with glibc when
> > using printf, you know that PTHREAD_STACK_MIN is not sufficient, so
> > you should pass a larger size. I suspect it goes this way for any
> > posix implementation. Once again, if an applications chooses to use
> > these calls, it should know what it is doing.
> 
> Possibly the crash was limited to the case where the application set a
> stack address and Xenomai messed up the size. I'm rechecking this right
> now, and if we are lucky, PTHREAD_STACK_MIN turns out to be fine for
> Xenomai as well.
> 
> > 
> >>
> >>> Any user calling this interface should. We are doing real-time in C,
> >>> not making an API for tutorials in ADA, or C++, let us try not to
> >>> prevent the user from doing what he wants.
> >>>
> >>> For pthread_attr_setstacksize, you can round to the larger size and
> >>> remain POSIX compliant.
> >>
> >> Is it specified that this call invalidate a stack address set earlier? I
> >> don't see this.
> > 
> > The way I understand the spec, the stack and stacksize attribute are
> > separate, and setting stacksize sets the size of the stack that will
> > be allocated automatically. Now if you set both, maybe stacksize is
> > ignored and the sized passed to setstack is used. But I would not
> > expect the size passed to setstack to be used for the stacksize
> > attribute.
> 
> glibc uses the same internal stacksize field for both calls.

Well, I guess it assumes that an application that uses both calls is
broken anyway.

-- 
					    Gilles.

Re: [Xenomai] [Xenomai-git] Jan Kiszka : lib/cobalt: Rework minimum stack size enforcement

[Jan Kiszka]

On 2015-04-21 18:37, Jan Kiszka wrote:
> Possibly the crash was limited to the case where the application set a
> stack address and Xenomai messed up the size. I'm rechecking this right
> now, and if we are lucky, PTHREAD_STACK_MIN turns out to be fine for
> Xenomai as well.

Too bad, it wasn't that easy. Just try this, even without Xenomai:

#include <stdio.h>
#include <pthread.h>
#include <limits.h>

void *thread_func(void *arg)
{
	fprintf(stderr, "crash %s\n", "me");
	return NULL;
}

int main(int argc, char *argv[])
{
	pthread_t thread;
	pthread_attr_t attr;

	pthread_attr_init(&attr);
	pthread_attr_setstacksize(&attr, PTHREAD_STACK_MIN);

	pthread_create(&thread, &attr, thread_func, NULL);

	pthread_join(thread, NULL);

	return 0;
}

Crashes on my x86-64 boxes all the time. stdout/printf is fine. Some
internal glibc function requires a lot of stack space.

The problem is that we trigger the very same pattern with warning() in
Xenomai. When that is called by the thread trampoline, the user cannot
run threads with otherwise totally fine PTHREAD_STACK_MIN.

Now we can
- ask the user to specify for more stack (by contract)
- reject too small stacks (my patches)
- warn about too small stacks, but accept them (maybe a compromise)
- simply ignore this

Jan

-- 
Siemens AG, Corporate Technology, CT RTC ITP SES-DE
Corporate Competence Center Embedded Linux

Re: [Xenomai] [Xenomai-git] Jan Kiszka : lib/cobalt: Rework minimum stack size enforcement

[Gilles Chanteperdrix]

On Tue, Apr 21, 2015 at 07:52:15PM +0200, Jan Kiszka wrote:
> On 2015-04-21 18:37, Jan Kiszka wrote:
> > Possibly the crash was limited to the case where the application set a
> > stack address and Xenomai messed up the size. I'm rechecking this right
> > now, and if we are lucky, PTHREAD_STACK_MIN turns out to be fine for
> > Xenomai as well.
> 
> Too bad, it wasn't that easy. Just try this, even without Xenomai:
> 
> #include <stdio.h>
> #include <pthread.h>
> #include <limits.h>
> 
> void *thread_func(void *arg)
> {
> 	fprintf(stderr, "crash %s\n", "me");
> 	return NULL;
> }
> 
> int main(int argc, char *argv[])
> {
> 	pthread_t thread;
> 	pthread_attr_t attr;
> 
> 	pthread_attr_init(&attr);
> 	pthread_attr_setstacksize(&attr, PTHREAD_STACK_MIN);
> 
> 	pthread_create(&thread, &attr, thread_func, NULL);
> 
> 	pthread_join(thread, NULL);
> 
> 	return 0;
> }
> 
> Crashes on my x86-64 boxes all the time. stdout/printf is fine. Some
> internal glibc function requires a lot of stack space.

Well, as I said several time in that thread, using printf with glibc
and PTHREAD_STACK_MIN crashes. Yes, this is an issue I had noticed a
long time ago. I always assumed it was because printf used alloca or
allocated a large buffer on stack by some other mean.

> 
> The problem is that we trigger the very same pattern with warning() in
> Xenomai. When that is called by the thread trampoline, the user cannot
> run threads with otherwise totally fine PTHREAD_STACK_MIN.
> 
> Now we can
> - ask the user to specify for more stack (by contract)
> - reject too small stacks (my patches)
> - warn about too small stacks, but accept them (maybe a compromise)
> - simply ignore this

This is not our business. Really.

-- 
					    Gilles.

Re: [Xenomai] [Xenomai-git] Jan Kiszka : lib/cobalt: Rework minimum stack size enforcement

[Jan Kiszka]

On 2015-04-21 19:56, Gilles Chanteperdrix wrote:
> On Tue, Apr 21, 2015 at 07:52:15PM +0200, Jan Kiszka wrote:
>> On 2015-04-21 18:37, Jan Kiszka wrote:
>>> Possibly the crash was limited to the case where the application set a
>>> stack address and Xenomai messed up the size. I'm rechecking this right
>>> now, and if we are lucky, PTHREAD_STACK_MIN turns out to be fine for
>>> Xenomai as well.
>>
>> Too bad, it wasn't that easy. Just try this, even without Xenomai:
>>
>> #include <stdio.h>
>> #include <pthread.h>
>> #include <limits.h>
>>
>> void *thread_func(void *arg)
>> {
>> 	fprintf(stderr, "crash %s\n", "me");
>> 	return NULL;
>> }
>>
>> int main(int argc, char *argv[])
>> {
>> 	pthread_t thread;
>> 	pthread_attr_t attr;
>>
>> 	pthread_attr_init(&attr);
>> 	pthread_attr_setstacksize(&attr, PTHREAD_STACK_MIN);
>>
>> 	pthread_create(&thread, &attr, thread_func, NULL);
>>
>> 	pthread_join(thread, NULL);
>>
>> 	return 0;
>> }
>>
>> Crashes on my x86-64 boxes all the time. stdout/printf is fine. Some
>> internal glibc function requires a lot of stack space.
> 
> Well, as I said several time in that thread, using printf with glibc
> and PTHREAD_STACK_MIN crashes. Yes, this is an issue I had noticed a
> long time ago. I always assumed it was because printf used alloca or
> allocated a large buffer on stack by some other mean.
> 
>>
>> The problem is that we trigger the very same pattern with warning() in
>> Xenomai. When that is called by the thread trampoline, the user cannot
>> run threads with otherwise totally fine PTHREAD_STACK_MIN.
>>
>> Now we can
>> - ask the user to specify for more stack (by contract)
>> - reject too small stacks (my patches)
>> - warn about too small stacks, but accept them (maybe a compromise)
>> - simply ignore this
> 
> This is not our business. Really.
> 

It is our business as we change the interface for the user in a
non-configurable way. The bare minimum is documentation, but the more I
think about, the better is the warning variant: violates no
specification and still improves the debugging situation of - again -
valid programs under plain Linux.

Jan

-- 
Siemens AG, Corporate Technology, CT RTC ITP SES-DE
Corporate Competence Center Embedded Linux

Re: [Xenomai] [Xenomai-git] Jan Kiszka : lib/cobalt: Rework minimum stack size enforcement

[Gilles Chanteperdrix]

On Tue, Apr 21, 2015 at 08:04:33PM +0200, Jan Kiszka wrote:
> On 2015-04-21 19:56, Gilles Chanteperdrix wrote:
> > On Tue, Apr 21, 2015 at 07:52:15PM +0200, Jan Kiszka wrote:
> >> On 2015-04-21 18:37, Jan Kiszka wrote:
> >>> Possibly the crash was limited to the case where the application set a
> >>> stack address and Xenomai messed up the size. I'm rechecking this right
> >>> now, and if we are lucky, PTHREAD_STACK_MIN turns out to be fine for
> >>> Xenomai as well.
> >>
> >> Too bad, it wasn't that easy. Just try this, even without Xenomai:
> >>
> >> #include <stdio.h>
> >> #include <pthread.h>
> >> #include <limits.h>
> >>
> >> void *thread_func(void *arg)
> >> {
> >> 	fprintf(stderr, "crash %s\n", "me");
> >> 	return NULL;
> >> }
> >>
> >> int main(int argc, char *argv[])
> >> {
> >> 	pthread_t thread;
> >> 	pthread_attr_t attr;
> >>
> >> 	pthread_attr_init(&attr);
> >> 	pthread_attr_setstacksize(&attr, PTHREAD_STACK_MIN);
> >>
> >> 	pthread_create(&thread, &attr, thread_func, NULL);
> >>
> >> 	pthread_join(thread, NULL);
> >>
> >> 	return 0;
> >> }
> >>
> >> Crashes on my x86-64 boxes all the time. stdout/printf is fine. Some
> >> internal glibc function requires a lot of stack space.
> > 
> > Well, as I said several time in that thread, using printf with glibc
> > and PTHREAD_STACK_MIN crashes. Yes, this is an issue I had noticed a
> > long time ago. I always assumed it was because printf used alloca or
> > allocated a large buffer on stack by some other mean.
> > 
> >>
> >> The problem is that we trigger the very same pattern with warning() in
> >> Xenomai. When that is called by the thread trampoline, the user cannot
> >> run threads with otherwise totally fine PTHREAD_STACK_MIN.
> >>
> >> Now we can
> >> - ask the user to specify for more stack (by contract)
> >> - reject too small stacks (my patches)
> >> - warn about too small stacks, but accept them (maybe a compromise)
> >> - simply ignore this
> > 
> > This is not our business. Really.
> > 
> 
> It is our business as we change the interface for the user in a
> non-configurable way.

Again: we should not change the interface for the user. Period.
Whatever he passes to pthread_attr_setstacksize or
pthread_attr_setstack gets used by the glibc. The average user
should not use these interfaces, the users who uses them should know
what he is doing.

What we can change is the default stack size used, pick a stack size
which does not cause printf to segfault (for reasonable string
sizes, of say 80 characters), and hide that in pthread_attr_init.
Because the average user is not going to pass a stack size and can
expect the default to work reasonably well.

> The bare minimum is documentation,

Well, all the books on the pthread API warn you that
pthread_attr_setstack is not a good idea. Maybe even the open group
documentation and the linux documentation do.

> but the more I
> think about, the better is the warning variant: violates no
> specification and still improves the debugging situation of - again -
> valid programs under plain Linux.

I would rather think of something else: a probable stack overflow
detector in the kernel, which prints a message with printk. A
probable stack overflow is easy to do in kernel-space, as the FCSE
code demonstrates. At least this way, the user who uses
PTHREAD_STACK_MIN and knows what he is doing does not get a moronic
warning. Only in case of stack overflow do you get a message, and
you get it even if you passed 2 * PTHREAD_STACK_MIN but overflowed
that.

-- 
					    Gilles.

Re: [Xenomai] [Xenomai-git] Jan Kiszka : lib/cobalt: Rework minimum stack size enforcement

[Gilles Chanteperdrix]

On Tue, Apr 21, 2015 at 08:16:02PM +0200, Gilles Chanteperdrix wrote:
> On Tue, Apr 21, 2015 at 08:04:33PM +0200, Jan Kiszka wrote:
> > On 2015-04-21 19:56, Gilles Chanteperdrix wrote:
> > > On Tue, Apr 21, 2015 at 07:52:15PM +0200, Jan Kiszka wrote:
> > >> On 2015-04-21 18:37, Jan Kiszka wrote:
> > >>> Possibly the crash was limited to the case where the application set a
> > >>> stack address and Xenomai messed up the size. I'm rechecking this right
> > >>> now, and if we are lucky, PTHREAD_STACK_MIN turns out to be fine for
> > >>> Xenomai as well.
> > >>
> > >> Too bad, it wasn't that easy. Just try this, even without Xenomai:
> > >>
> > >> #include <stdio.h>
> > >> #include <pthread.h>
> > >> #include <limits.h>
> > >>
> > >> void *thread_func(void *arg)
> > >> {
> > >> 	fprintf(stderr, "crash %s\n", "me");
> > >> 	return NULL;
> > >> }
> > >>
> > >> int main(int argc, char *argv[])
> > >> {
> > >> 	pthread_t thread;
> > >> 	pthread_attr_t attr;
> > >>
> > >> 	pthread_attr_init(&attr);
> > >> 	pthread_attr_setstacksize(&attr, PTHREAD_STACK_MIN);
> > >>
> > >> 	pthread_create(&thread, &attr, thread_func, NULL);
> > >>
> > >> 	pthread_join(thread, NULL);
> > >>
> > >> 	return 0;
> > >> }
> > >>
> > >> Crashes on my x86-64 boxes all the time. stdout/printf is fine. Some
> > >> internal glibc function requires a lot of stack space.
> > > 
> > > Well, as I said several time in that thread, using printf with glibc
> > > and PTHREAD_STACK_MIN crashes. Yes, this is an issue I had noticed a
> > > long time ago. I always assumed it was because printf used alloca or
> > > allocated a large buffer on stack by some other mean.
> > > 
> > >>
> > >> The problem is that we trigger the very same pattern with warning() in
> > >> Xenomai. When that is called by the thread trampoline, the user cannot
> > >> run threads with otherwise totally fine PTHREAD_STACK_MIN.
> > >>
> > >> Now we can
> > >> - ask the user to specify for more stack (by contract)
> > >> - reject too small stacks (my patches)
> > >> - warn about too small stacks, but accept them (maybe a compromise)
> > >> - simply ignore this
> > > 
> > > This is not our business. Really.
> > > 
> > 
> > It is our business as we change the interface for the user in a
> > non-configurable way.
> 
> Again: we should not change the interface for the user. Period.
> Whatever he passes to pthread_attr_setstacksize or
> pthread_attr_setstack gets used by the glibc. The average user
> should not use these interfaces, the users who uses them should know
> what he is doing.
> 
> What we can change is the default stack size used, pick a stack size
> which does not cause printf to segfault (for reasonable string
> sizes, of say 80 characters), and hide that in pthread_attr_init.
> Because the average user is not going to pass a stack size and can
> expect the default to work reasonably well.
> 
> > The bare minimum is documentation,
> 
> Well, all the books on the pthread API warn you that
> pthread_attr_setstack is not a good idea. Maybe even the open group
> documentation and the linux documentation do.

Nothing in the open group documentation, but the Linux documentation
states: 

These functions are provided for applications that must ensure that
a thread's stack is placed in a particular location. For most
applications, this is not necessary, and the use of these functions
should be avoided.

-- 
					    Gilles.

Re: [Xenomai] [Xenomai-git] Jan Kiszka : lib/cobalt: Rework minimum stack size enforcement

[Jan Kiszka]

On 2015-04-21 20:16, Gilles Chanteperdrix wrote:
> On Tue, Apr 21, 2015 at 08:04:33PM +0200, Jan Kiszka wrote:
>> On 2015-04-21 19:56, Gilles Chanteperdrix wrote:
>>> On Tue, Apr 21, 2015 at 07:52:15PM +0200, Jan Kiszka wrote:
>>>> On 2015-04-21 18:37, Jan Kiszka wrote:
>>>>> Possibly the crash was limited to the case where the application set a
>>>>> stack address and Xenomai messed up the size. I'm rechecking this right
>>>>> now, and if we are lucky, PTHREAD_STACK_MIN turns out to be fine for
>>>>> Xenomai as well.
>>>>
>>>> Too bad, it wasn't that easy. Just try this, even without Xenomai:
>>>>
>>>> #include <stdio.h>
>>>> #include <pthread.h>
>>>> #include <limits.h>
>>>>
>>>> void *thread_func(void *arg)
>>>> {
>>>> 	fprintf(stderr, "crash %s\n", "me");
>>>> 	return NULL;
>>>> }
>>>>
>>>> int main(int argc, char *argv[])
>>>> {
>>>> 	pthread_t thread;
>>>> 	pthread_attr_t attr;
>>>>
>>>> 	pthread_attr_init(&attr);
>>>> 	pthread_attr_setstacksize(&attr, PTHREAD_STACK_MIN);
>>>>
>>>> 	pthread_create(&thread, &attr, thread_func, NULL);
>>>>
>>>> 	pthread_join(thread, NULL);
>>>>
>>>> 	return 0;
>>>> }
>>>>
>>>> Crashes on my x86-64 boxes all the time. stdout/printf is fine. Some
>>>> internal glibc function requires a lot of stack space.
>>>
>>> Well, as I said several time in that thread, using printf with glibc
>>> and PTHREAD_STACK_MIN crashes. Yes, this is an issue I had noticed a
>>> long time ago. I always assumed it was because printf used alloca or
>>> allocated a large buffer on stack by some other mean.
>>>
>>>>
>>>> The problem is that we trigger the very same pattern with warning() in
>>>> Xenomai. When that is called by the thread trampoline, the user cannot
>>>> run threads with otherwise totally fine PTHREAD_STACK_MIN.
>>>>
>>>> Now we can
>>>> - ask the user to specify for more stack (by contract)
>>>> - reject too small stacks (my patches)
>>>> - warn about too small stacks, but accept them (maybe a compromise)
>>>> - simply ignore this
>>>
>>> This is not our business. Really.
>>>
>>
>> It is our business as we change the interface for the user in a
>> non-configurable way.
> 
> Again: we should not change the interface for the user. Period.
> Whatever he passes to pthread_attr_setstacksize or
> pthread_attr_setstack gets used by the glibc. The average user
> should not use these interfaces, the users who uses them should know
> what he is doing.
> 
> What we can change is the default stack size used, pick a stack size
> which does not cause printf to segfault (for reasonable string
> sizes, of say 80 characters), and hide that in pthread_attr_init.
> Because the average user is not going to pass a stack size and can
> expect the default to work reasonably well.
> 
>> The bare minimum is documentation,
> 
> Well, all the books on the pthread API warn you that
> pthread_attr_setstack is not a good idea. Maybe even the open group
> documentation and the linux documentation do.

"My application works fine for normal Linux, but when I build it for
Xenomai, it crashes in libc, and only Xenomai libs are in the backtrace."

That's the user experience, specifically the delta will fall back on us.

> 
>> but the more I
>> think about, the better is the warning variant: violates no
>> specification and still improves the debugging situation of - again -
>> valid programs under plain Linux.
> 
> I would rather think of something else: a probable stack overflow
> detector in the kernel, which prints a message with printk. A
> probable stack overflow is easy to do in kernel-space, as the FCSE
> code demonstrates. At least this way, the user who uses
> PTHREAD_STACK_MIN and knows what he is doing does not get a moronic
> warning. Only in case of stack overflow do you get a message, and
> you get it even if you passed 2 * PTHREAD_STACK_MIN but overflowed
> that.

There are already guard pages around normally allocates stacks. However,
the pointer first of all goes back to Xenomai, not the application.

Jan

-- 
Siemens AG, Corporate Technology, CT RTC ITP SES-DE
Corporate Competence Center Embedded Linux

Re: [Xenomai] [Xenomai-git] Jan Kiszka : lib/cobalt: Rework minimum stack size enforcement

[Gilles Chanteperdrix]

On Tue, Apr 21, 2015 at 08:25:59PM +0200, Jan Kiszka wrote:
> On 2015-04-21 20:16, Gilles Chanteperdrix wrote:
> > On Tue, Apr 21, 2015 at 08:04:33PM +0200, Jan Kiszka wrote:
> >> On 2015-04-21 19:56, Gilles Chanteperdrix wrote:
> >>> On Tue, Apr 21, 2015 at 07:52:15PM +0200, Jan Kiszka wrote:
> >>>> On 2015-04-21 18:37, Jan Kiszka wrote:
> >>>>> Possibly the crash was limited to the case where the application set a
> >>>>> stack address and Xenomai messed up the size. I'm rechecking this right
> >>>>> now, and if we are lucky, PTHREAD_STACK_MIN turns out to be fine for
> >>>>> Xenomai as well.
> >>>>
> >>>> Too bad, it wasn't that easy. Just try this, even without Xenomai:
> >>>>
> >>>> #include <stdio.h>
> >>>> #include <pthread.h>
> >>>> #include <limits.h>
> >>>>
> >>>> void *thread_func(void *arg)
> >>>> {
> >>>> 	fprintf(stderr, "crash %s\n", "me");
> >>>> 	return NULL;
> >>>> }
> >>>>
> >>>> int main(int argc, char *argv[])
> >>>> {
> >>>> 	pthread_t thread;
> >>>> 	pthread_attr_t attr;
> >>>>
> >>>> 	pthread_attr_init(&attr);
> >>>> 	pthread_attr_setstacksize(&attr, PTHREAD_STACK_MIN);
> >>>>
> >>>> 	pthread_create(&thread, &attr, thread_func, NULL);
> >>>>
> >>>> 	pthread_join(thread, NULL);
> >>>>
> >>>> 	return 0;
> >>>> }
> >>>>
> >>>> Crashes on my x86-64 boxes all the time. stdout/printf is fine. Some
> >>>> internal glibc function requires a lot of stack space.
> >>>
> >>> Well, as I said several time in that thread, using printf with glibc
> >>> and PTHREAD_STACK_MIN crashes. Yes, this is an issue I had noticed a
> >>> long time ago. I always assumed it was because printf used alloca or
> >>> allocated a large buffer on stack by some other mean.
> >>>
> >>>>
> >>>> The problem is that we trigger the very same pattern with warning() in
> >>>> Xenomai. When that is called by the thread trampoline, the user cannot
> >>>> run threads with otherwise totally fine PTHREAD_STACK_MIN.
> >>>>
> >>>> Now we can
> >>>> - ask the user to specify for more stack (by contract)
> >>>> - reject too small stacks (my patches)
> >>>> - warn about too small stacks, but accept them (maybe a compromise)
> >>>> - simply ignore this
> >>>
> >>> This is not our business. Really.
> >>>
> >>
> >> It is our business as we change the interface for the user in a
> >> non-configurable way.
> > 
> > Again: we should not change the interface for the user. Period.
> > Whatever he passes to pthread_attr_setstacksize or
> > pthread_attr_setstack gets used by the glibc. The average user
> > should not use these interfaces, the users who uses them should know
> > what he is doing.
> > 
> > What we can change is the default stack size used, pick a stack size
> > which does not cause printf to segfault (for reasonable string
> > sizes, of say 80 characters), and hide that in pthread_attr_init.
> > Because the average user is not going to pass a stack size and can
> > expect the default to work reasonably well.
> > 
> >> The bare minimum is documentation,
> > 
> > Well, all the books on the pthread API warn you that
> > pthread_attr_setstack is not a good idea. Maybe even the open group
> > documentation and the linux documentation do.
> 
> "My application works fine for normal Linux, but when I build it for
> Xenomai, it crashes in libc, and only Xenomai libs are in the backtrace."
> 
> That's the user experience, specifically the delta will fall back on us.

If you use pthread_attr_setstack you get what you deserved. "This
function should be avoided", the documentation says. So one of our
usual question will be "do you get the same problem if you use the
default stack size ?". If you use pthread_attr_setstacksize we can
arrange to increase the stack size, because the size passed to this
function is a minimum, and the implementation is free to allocate a
larger stack, so, as the documentation says, this function is more
apropriate. If you use both, your application is broken, so this is
a non issue.

> 
> > 
> >> but the more I
> >> think about, the better is the warning variant: violates no
> >> specification and still improves the debugging situation of - again -
> >> valid programs under plain Linux.
> > 
> > I would rather think of something else: a probable stack overflow
> > detector in the kernel, which prints a message with printk. A
> > probable stack overflow is easy to do in kernel-space, as the FCSE
> > code demonstrates. At least this way, the user who uses
> > PTHREAD_STACK_MIN and knows what he is doing does not get a moronic
> > warning. Only in case of stack overflow do you get a message, and
> > you get it even if you passed 2 * PTHREAD_STACK_MIN but overflowed
> > that.
> 
> There are already guard pages around normally allocates stacks.

The point is, the kernel does not print anything when you hit the
gard page, we can add that printk.


> However,
> the pointer first of all goes back to Xenomai, not the application.

Good. We can put the printk in xenomai then.

-- 
					    Gilles.

Re: [Xenomai] [Xenomai-git] Jan Kiszka : lib/cobalt: Rework minimum stack size enforcement

[Gilles Chanteperdrix]

On Tue, Apr 21, 2015 at 08:25:59PM +0200, Jan Kiszka wrote:
> On 2015-04-21 20:16, Gilles Chanteperdrix wrote:
> > On Tue, Apr 21, 2015 at 08:04:33PM +0200, Jan Kiszka wrote:
> >> On 2015-04-21 19:56, Gilles Chanteperdrix wrote:
> >>> On Tue, Apr 21, 2015 at 07:52:15PM +0200, Jan Kiszka wrote:
> >>>> On 2015-04-21 18:37, Jan Kiszka wrote:
> >>>>> Possibly the crash was limited to the case where the application set a
> >>>>> stack address and Xenomai messed up the size. I'm rechecking this right
> >>>>> now, and if we are lucky, PTHREAD_STACK_MIN turns out to be fine for
> >>>>> Xenomai as well.
> >>>>
> >>>> Too bad, it wasn't that easy. Just try this, even without Xenomai:
> >>>>
> >>>> #include <stdio.h>
> >>>> #include <pthread.h>
> >>>> #include <limits.h>
> >>>>
> >>>> void *thread_func(void *arg)
> >>>> {
> >>>> 	fprintf(stderr, "crash %s\n", "me");
> >>>> 	return NULL;
> >>>> }
> >>>>
> >>>> int main(int argc, char *argv[])
> >>>> {
> >>>> 	pthread_t thread;
> >>>> 	pthread_attr_t attr;
> >>>>
> >>>> 	pthread_attr_init(&attr);
> >>>> 	pthread_attr_setstacksize(&attr, PTHREAD_STACK_MIN);
> >>>>
> >>>> 	pthread_create(&thread, &attr, thread_func, NULL);
> >>>>
> >>>> 	pthread_join(thread, NULL);
> >>>>
> >>>> 	return 0;
> >>>> }
> >>>>
> >>>> Crashes on my x86-64 boxes all the time. stdout/printf is fine. Some
> >>>> internal glibc function requires a lot of stack space.
> >>>
> >>> Well, as I said several time in that thread, using printf with glibc
> >>> and PTHREAD_STACK_MIN crashes. Yes, this is an issue I had noticed a
> >>> long time ago. I always assumed it was because printf used alloca or
> >>> allocated a large buffer on stack by some other mean.
> >>>
> >>>>
> >>>> The problem is that we trigger the very same pattern with warning() in
> >>>> Xenomai. When that is called by the thread trampoline, the user cannot
> >>>> run threads with otherwise totally fine PTHREAD_STACK_MIN.
> >>>>
> >>>> Now we can
> >>>> - ask the user to specify for more stack (by contract)
> >>>> - reject too small stacks (my patches)
> >>>> - warn about too small stacks, but accept them (maybe a compromise)
> >>>> - simply ignore this
> >>>
> >>> This is not our business. Really.
> >>>
> >>
> >> It is our business as we change the interface for the user in a
> >> non-configurable way.
> > 
> > Again: we should not change the interface for the user. Period.
> > Whatever he passes to pthread_attr_setstacksize or
> > pthread_attr_setstack gets used by the glibc. The average user
> > should not use these interfaces, the users who uses them should know
> > what he is doing.
> > 
> > What we can change is the default stack size used, pick a stack size
> > which does not cause printf to segfault (for reasonable string
> > sizes, of say 80 characters), and hide that in pthread_attr_init.
> > Because the average user is not going to pass a stack size and can
> > expect the default to work reasonably well.
> > 
> >> The bare minimum is documentation,
> > 
> > Well, all the books on the pthread API warn you that
> > pthread_attr_setstack is not a good idea. Maybe even the open group
> > documentation and the linux documentation do.
> 
> "My application works fine for normal Linux, but when I build it for
> Xenomai, it crashes in libc, and only Xenomai libs are in the backtrace."
> 
> That's the user experience, specifically the delta will fall back on us.
> 
> > 
> >> but the more I
> >> think about, the better is the warning variant: violates no
> >> specification and still improves the debugging situation of - again -
> >> valid programs under plain Linux.
> > 
> > I would rather think of something else: a probable stack overflow
> > detector in the kernel, which prints a message with printk. A
> > probable stack overflow is easy to do in kernel-space, as the FCSE
> > code demonstrates. At least this way, the user who uses
> > PTHREAD_STACK_MIN and knows what he is doing does not get a moronic
> > warning. Only in case of stack overflow do you get a message, and
> > you get it even if you passed 2 * PTHREAD_STACK_MIN but overflowed
> > that.
> 
> There are already guard pages around normally allocates stacks.

The issue is not with normally allocated stacks, there is no issue
with normally allocated stacks. The issue is with stacks passed with
pthread_attr_setstack. And in that case, the linux documentation
says:

When an application employs pthread_attr_setstack(), it takes over
the responsibility of allocating the stack. Any guard size value
that was set using pthread_attr_setguardsize(3) is ignored. If
deemed necessary, it is the application's responsibility to allocate
a guard area (one or more pages protected against reading and
writing) to handle the possibility of stack overflow.

So in that case, there is no guard page.

-- 
					    Gilles.

Re: [Xenomai] [Xenomai-git] Jan Kiszka : lib/cobalt: Rework minimum stack size enforcement

[Gilles Chanteperdrix]

On Tue, Apr 21, 2015 at 06:20:28PM +0200, Jan Kiszka wrote:
> On 2015-04-21 18:16, Gilles Chanteperdrix wrote:
> > On Tue, Apr 21, 2015 at 06:12:15PM +0200, Jan Kiszka wrote:
> >> On 2015-04-21 18:06, Gilles Chanteperdrix wrote:
> >>> On Tue, Apr 21, 2015 at 06:04:42PM +0200, Jan Kiszka wrote:
> >>>> On 2015-04-21 17:27, Gilles Chanteperdrix wrote:
> >>>>> On Tue, Apr 21, 2015 at 05:22:34PM +0200, git repository hosting wrote:
> >>>>>> +COBALT_IMPL(int, pthread_attr_setstack, (pthread_attr_t *attr, void *stackaddr,
> >>>>>> +					 size_t stacksize))
> >>>>>> +{
> >>>>>> +	if (stacksize < COBALT_STACKSIZE_MIN)
> >>>>>> +		return -EINVAL;
> >>>>>> +	return __STD(pthread_attr_setstack)(attr, stackaddr, stacksize);
> >>>>>> +}
> >>>>>
> >>>>> As I said, there is no reason for Xenomai to enforce a task size
> >>>>> larger than PTHREAD_STACK_MIN. This breaks POSIX compliance, and is
> >>>>> useless.
> >>>>
> >>>> Then we should not use printf & Co. from any trampoline function, e.g.
> >>>> vxworks/taskLib.c:task_trampoline. I only tried with a local skin, but I
> >>>> bet the issue is reproducible with that one.
> >>>>
> >>>> For that reason, I went down the conservative path.
> >>>
> >>> The path you took breaks compatibility with POSIX. On a compliant
> >>> system, pthread_attr_setstacksize(attr, PTHREAD_STACK_MIN) is not
> >>> supposed to return an error.
> >>
> >> That is true. We have to pick from one evil: fail non-compliant here or
> >> crash later.
> > 
> > The point is: you do not know if it will crash later, you can assume
> > that the user calling pthread_attr_setstack knows what he is doing.
> 
> The crash can be caused by the the library code and, thus, can happen
> even if the application code would not crash. That is my problem.
> 
> > Any user calling this interface should. We are doing real-time in C,
> > not making an API for tutorials in ADA, or C++, let us try not to
> > prevent the user from doing what he wants.
> > 
> > For pthread_attr_setstacksize, you can round to the larger size and
> > remain POSIX compliant.
> 
> Is it specified that this call invalidate a stack address set earlier? I
> don't see this.

Really, if 
setstack(attr, stack, PTHREAD_STACK_MIN)
then
setstacksize(attr, PTHREAD_STACK_MIN * 2)
modifies the stacksize of the stack passed as argument of setstack,
then I would find the implementation rather broken.

I would expect either the stacksize attribute to be ignored after
the stack attribute, or indeed the stacksize attribute to invalidate
the stack attribute.

Anyway, an application using both would be broken too.

-- 
					    Gilles.