Re: [patch, avr-libc] Fix atexit.c

Am 10/14/2014 04:07 PM, schrieb Joern Rennecke:
>> atexit.c has several issues which are fixed by the patch below:
>>
>> i)
>> Issuance of calls to functions as registered by atexit() is located in
>> section .fini6a.  As the current linker description does not handle that
>> section it's treated as orphan and located conflicting with other sections:
>
> I actually wrote a binutils patch at the same time as the atexit implementation:
>
> [email protected]:embecosm/avr-binutils-gdb.git branch avr-mainline commit
> 640411f2fb984e70c4ab06154089692db1734a97
>
> I just haven't gotten around to push this to the main repo yet.

Ok, I updated the patch request (#8556) to use .fini8 instead of .fini7 to call 
atexit functions.

>> ii)
>> __atexit_fini is not naked thus will crash the program as it is returning.
>
> Good point.  However, I'd prefer to add a new attribute to just say
> that the function returns by falling through at the end.
> Opinions on how to name this?  "fallthrough" ?  "return_falls_through" ?
> "ctor_dtor" (although that can get confusing with the constructor /
> destructor attribute) ?

Some time ago I thought about adding a new function attribute to avr-gcc that 
injects code at prologue / epilogue.  That way an application could instrument 
particular functions with particular code.  Such an attribute would cover the 
feature you are proposing if the attribute is generic enough.  That new 
attribute would allow to:

- specify the (assembler) code to be injected as plain C-string
- specify where to inject the code:
-- before prologue
-- after prologue
-- instead of prologue
-- before epilogue
-- after epilogue
-- instead of epilogue

Cases like "after epilogue" make only sense for special functions like "naked", 
but you could achieve "naked" also by replacing epilogue with the injected code 
and removing prologue (by injecting "" as replacement for prologue by means of 
a second such attribute).

A naked function could then be modelled as:

__attribute__((instrument ("", replace_prologue),
                instrument ("", replace_epilogue)))
void fun ...

Such instrumentation could provide better control of code generation in cases like:

- instrumenting individual functions with specific for home-made profiling / 
performance measurement
- writing ISRs that need to manipulate stack, e.g. patch return address
- writing startup code as of your use case
- ...

Replacing / removing epilogue by an attribute is not enough:  BBRO and all 
passes running after .prologue_epilogue must not reorder blocks and a function 
must not have more than one epilogue, cf. also GCC PR42240.  And such a 
function must not be inlined or considered for cloning and the emit insn 
representing the code chunk must not be moved across any other insn.

As I had no use case by myself and I never came across a request for such a new 
attribute / feature, I didn't start coding on it,  ... would have been dead 
code in GCC.

For a real application it might also be interesting

- how much registers are saved / would be saved on the stack
- whether the function ends with a tail call
- whether the function needs a frame pointer
- how much frame space is set up by the function
- whether the function is a leaf function
- the C name of the function
- the assembler name of the function
- maybe more

That information could be supplied by special %-codes like scheme.

> Overload the meaning of the section attribute so that .init / .fini
> section 'magically'causes the different return?

These section names are already in widespread use, not only for code to be run 
before main.  Adding magic and implicit behaviour is not a good approach, IMO, 
in particular if a feature is not new and is being used in the field.

> +atexit_finido (void)
>   {
> -  while (__atexit_p)
> +  while (atexit_p)
>       {
> -      void (*fun) (void) = __atexit_p->fun;
> -      __atexit_p = __atexit_p->next;
> -      (*fun) ();
> +      atexit_p->fun();
> +      atexit_p = atexit_p->next;
>
> This introduces a bug.  If the called function calls exit (e.g.
> because it throws
> an error), you get an infinite recursion.  The pointer must be updated before
> the function is called.

Thanks for the hint, updated #8556 accordingly.

Seems my brain has not yet enough weird windings...

Johann