search

sifive / freedom-metal

Bare Metal Compatibility Library for the Freedom Platform
Other
154 stars 47 forks source link

Why does freedom-metal have any 'exit' handling? #240

Open keith-packard opened 4 years ago

keith-packard commented 4 years ago

Given that there's no well-defined behavior for 'exit' in an embedded environment, it seems odd that crt0.S includes calls to atexit and exit. I'd suggest removing all of that and having crt0.S cause a trap if main ever returns.

Changing this would allow an embedded RTOS using freedom metal to define it's own _exit function, and we'd be able to build newlib-nano with per-thread atexit handling if desired.

bsousi5 commented 4 years ago

Exit was done for RTL test run

On Apr 2, 2020, at 12:42 PM, Keith Packard notifications@github.com wrote:

Given that there's no well-defined behavior for 'exit' in an embedded environment, it seems odd that crt0.S includes calls to atexit and exit. I'd suggest removing all of that and having crt0.S cause a trap if main ever returns.

Changing this would allow an embedded RTOS using freedom metal to define it's own _exit function, and we'd be able to build newlib-nano with per-thread atexit handling if desired.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/sifive/freedom-metal/issues/240, or unsubscribe https://github.com/notifications/unsubscribe-auth/AIQNVX3VXGRWRDS7ZTS4WQ3RKTTDRANCNFSM4L23ACKA.

nategraff-sifive commented 4 years ago

Nowhere do we even use destructors in freedom metal. I added them just because "well, we have constructors". I think it's reasonable to remove them if they're just bloating everyone's applications to no benefit.

nategraff-sifive commented 4 years ago

As far as the exit goes, the return value from main does get sent to the shutdown handler so that test status can be reported in QEMU and RTL simulation.

keith-packard commented 4 years ago

Exit was done for RTL test run

Any idea what that's supposed to do? crt0.S will just branch to whatever ra is pointing at, which is not likely to be helpful.

keith-packard commented 4 years ago

As far as the exit goes, the return value from main does get sent to the shutdown handler so that test status can be reported in QEMU and RTL simulation.

Right, and semihosting also has exit support, which I've been using for testing picolibc under QEMU.

So, I think my concern is more about what crt0.S does than whether anything using freedom-metal should be able to call exit sensibly. I'd like to remove the atexit and exit calls from that code, and have it trap if main returns instead. This would avoid pulling in unnecessary libc code when building 'normal' embedded applications.

Ideally, we'd find a way to make QEMU and the RTL simulator generate an error so that tests which return from main fail instead of hang forever.

bsousi5 commented 4 years ago

You may already aware of this. But we would not want to make the change or merge this change for the next couple of week.

On Apr 2, 2020, at 2:00 PM, Keith Packard notifications@github.com wrote:

As far as the exit goes, the return value from main does get sent to the shutdown handler so that test status can be reported in QEMU and RTL simulation.

Right, and semihosting also has exit support, which I've been using for testing picolibc under QEMU.

So, I think my concern is more about what crt0.S does than whether anything using freedom-metal should be able to call exit sensibly. I'd like to remove the atexit and exit calls from that code, and have it trap if main returns instead. This would avoid pulling in unnecessary libc code when building 'normal' embedded applications.

Ideally, we'd find a way to make QEMU and the RTL simulator generate an error so that tests which return from main fail instead of hang forever.

— You are receiving this because you were assigned. Reply to this email directly, view it on GitHub https://github.com/sifive/freedom-metal/issues/240#issuecomment-608083539, or unsubscribe https://github.com/notifications/unsubscribe-auth/AIQNVX7PEMRAQJWEZD6NMW3RKT4HDANCNFSM4L23ACKA.

keith-packard commented 4 years ago

You may already aware of this. But we would not want to make the change or merge this change for the next couple of week.

There's no hurry, just another architecture change that we might consider. It's "harmless" as the code should never get executed, it just wastes memory.