Serious Bug All Targets -- Please Read

[Till Straumann]

Ralf Corsepius wrote:
> Till Straumann wrote:
>> Ralf Corsepius wrote:
>>> Till Straumann wrote:
>>>> Ralf Corsepius wrote:
>>>>> Till Straumann wrote:
>>>>>> Hmm - AFAIK, c99 doesn't specify the exact representation
>>>>>> of 'bool' / '_Bool'. It only says that
>>>>>>
>>>>>> "An object declared as type _Bool is large enough to store the 
>>>>>> values 0 and 1"
>>>>>>
>>>>>> Hence, there is no really safe way to access such
>>>>>> an object from assembly code.
>>>>> Exactly.
>>>>>
>>>>>> I wonder if it wouldn't be better to change the
>>>>>> type of _Context_Switch_necessary & friends
>>>>>> to a type of known width.
>>>>> IMO, no.
>>>>>
>>>>> We should change to code into something which doesn't need to know 
>>>>> the size of a type, rsp. automatically gets it right.
>>>> So how would you do that given the requirement
>>>> that these variables need to be available to
>>>> assembly code, maybe w/o a stack available?
>>> E.g. by rewriting the code in question in C
>> That's what the new PPC framework does.
>>> w/ inline asm. This isn't always possible, but is possible on many 
>>> occasions, even when  stack is not available.
>> w/o stack I'm not sure...
> You can do something like this (Pseudo-C code):
>
> extern bool var;
>
> int _foo_stub(....)
> {
>   asm( ENTRY_SYMBOL(foo) );
>   asm( PROCESS(var) );
>
>   asm( RETURN );
> }
You can't be serious.

>
> I.e. to use a C-function wrapper/stub and insert asm-functions inside 
> of this function. It's a dirty hack, but it works quite well.
>
> A real world example based on this approach in RTEMS can be found in 
> cpukit/score/cpu/sh/context.c.
I pretend I've not seen that...
>
>>> It also somewhat (not always) helps to catch such size mismatches 
>>> early.
>>>
>>> Another possibility would be not to use _Bool for them (Which is the 
>>> type GCC interally uses), but to use an "int"
>> Well - that's what I originally proposed (int32_t actually).
>>
>> IMO changing the type of these critical variables from bool -> int32_t
>> is least intrusive and probably the best solution for a 4.9.2
>> release.
> The problem with this is: Some rtems < 4.9 targets have used short 
> (16bit) booleans.
I see. That is indeed a problem.

I'd propose

a) for 4.9 re-introduce a target-specific type in 
score/cpu/<cpu>/rtems/score/types.h

typedef <target_specific_type> obsolete_boolean;

and declare _Context_Switch_necessary & friends

extern obsolete_boolean _Context_Switch_necessary;

b) for 4.10 use a type of known width and clean-up the assembly code.
It could be useful to introduce a type-name that suggests that it is
being used by assembly code.

-- T.
>
> Ralf
>