Skip to content
This repository has been archived by the owner on Jan 24, 2022. It is now read-only.

reduce the size of default handlers #96

Merged
merged 2 commits into from
Aug 25, 2018
Merged

reduce the size of default handlers #96

merged 2 commits into from
Aug 25, 2018

Conversation

japaric
Copy link
Member

@japaric japaric commented Aug 25, 2018

this commit replaces the two default handler (DefaultDefaultHandler and
DefaultUserHardFault) by a single handler named EndlessLoop. This results in
one less symbol being generated.

Also this new handler uses compiler_fence to avoid the "infinite loops w/o
side effects are undef values" bug in LLVM. compiler_fence is guaranteed to
generate no code so this reduces the size of the handler (when compiler in
release).


As far as I could test this new handler doesn't generate abort instructions when
optimized so it seems like a safe replacement. If we are feeling paranoid then
once #95 we could implement EndlessLoop in assembly.

r? @rust-embedded/cortex-m (anyone)

this commit replaces the two default handler (DefaultDefaultHandler and
DefaultUserHardFault) by a single handler named `EndlessLoop`. This results in
one less symbol being generated.

Also this new handler uses `compiler_fence` to avoid the "infinite loops w/o
side effects are undef values" bug in LLVM. `compiler_fence` is guaranteed to
generate no code so this reduces the size of the handler (when compiler in
release).
@japaric japaric requested a review from a team as a code owner August 25, 2018 17:17
@adamgreig
Copy link
Member

Couple of comments

  • Does this mean you can no longer tell the difference between a HardFault and any other unhandled exception, without defining your own handlers? Not the biggest deal since most debuggers will catch HardFault before it triggers the exception handler anyway, but worth considering. Worth the small reduction in binary size IMO.

  • EndlessLoop vs InfiniteLoop / InfiniteLoopDefaultHandler?

@japaric
Copy link
Member Author

japaric commented Aug 25, 2018

Does this mean you can no longer tell the difference between a HardFault and any other unhandled exception, without defining your own handlers?

With or without this change you can put your breakpoint on the HardFault symbol and that will halt your program before it jumps into the user hard fault handler. (So no ExceptionFrame info at that point). Being able to break after that trampoline requires knowing whether the default user hard fault handler has been overridden or not. If it's the default then break on EndlessLoop; if it has been overridden break on UserHardFault.

But in any case once you are in EndlessLoop (or in DefaultUserHardFault) you can't inspect the exception frame using print *ef because ef is missing from the function arguments. That is a disadvantage. To preserve the ef argument we would need two default handler with different signatures.

EndlessLoop vs InfiniteLoop / InfiniteLoopDefaultHandler?

InfiniteLoopDefaultHandler seems too long

I'll try with two handlers and the compiler_fence.

@japaric
Copy link
Member Author

japaric commented Aug 25, 2018

OK, I have changed to two separate default handlers. This still saves 18 bytes (before it was 20 bytes) and one now can inspect ExceptionFrame from the default user hard fault handler -- without this commit that was not possible because DefaultUserHardFault had the wrong signature.

I have named the default handlers DefaultHandler_ and UserHardFault_ so they show when autocompleting for b Default... and b User....

@adamgreig
Copy link
Member

Thanks! I guess the only question is if the extra two bytes is worth the marginally easier debugging hard faults when using the default handlers. On balance I think it's probably worth it. The new names seem good.

0800069a <UserHardFault_>:
 800069a:	e7fe      	b.n	800069a <UserHardFault_>

0800069c <DefaultHandler_>:
 800069c:	e7fe      	b.n	800069c <DefaultHandler_>

@adamgreig
Copy link
Member

(I'm holding off on bors to check you're happy that this is done and ready, feel free to r= me if so).

Copy link
Contributor

@korken89 korken89 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks goods, gave it a try and it is fine for debugability as well

Copy link
Contributor

@therealprof therealprof left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@japaric is on a roll it seems, very nice!

@japaric
Copy link
Member Author

japaric commented Aug 25, 2018

bors r=adamgreig

bors bot added a commit that referenced this pull request Aug 25, 2018
96: reduce the size of default handlers r=adamgreig a=japaric

this commit replaces the two default handler (DefaultDefaultHandler and
DefaultUserHardFault) by a single handler named `EndlessLoop`. This results in
one less symbol being generated.

Also this new handler uses `compiler_fence` to avoid the "infinite loops w/o
side effects are undef values" bug in LLVM. `compiler_fence` is guaranteed to
generate no code so this reduces the size of the handler (when compiler in
release).

---

As far as I could test this new handler doesn't generate abort instructions when
optimized so it seems like a safe replacement. If we are feeling paranoid then
once #95 we could implement EndlessLoop in assembly.

r? @rust-embedded/cortex-m (anyone)

Co-authored-by: Jorge Aparicio <jorge@japaric.io>
@bors
Copy link
Contributor

bors bot commented Aug 25, 2018

Build succeeded

@bors bors bot merged commit 903e97a into master Aug 25, 2018
@bors bors bot deleted the smaller-defaults branch August 25, 2018 19:03
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants