Skip to content

Error print improvements #10358

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Apr 17, 2019
Merged

Conversation

kjbracey
Copy link
Contributor

@kjbracey kjbracey commented Apr 9, 2019

Description

Fixes two key problems:

  • mbed_error or MBED_ASSERT failure could fail to output if the console had not previously been used
  • mbed_error or MBED_ASSERT failure would be garbled if platform.stdio-buffered-serial was true but platform.stdio-convert-newlines was false.

Getting the console initialized for the first time in a fatal crash situation is always going to be potentially dicey, but we can make a better effort.

(If a platform is really serious about getting crash information out of the serial port, they should make an effort to get it ready in advance. But we don't necessarily want to initialise it if they're not planning to use it in normal operation.)

Fixes #10344, fixes #10242.

Pull request type

[X] Fix
[ ] Refactor
[ ] Target update
[ ] Functionality change
[ ] Docs update
[ ] Test update
[ ] Breaking change

kjbracey added 3 commits April 9, 2019 16:15
The length calculation in UARTSerial::write_unbuffered was wrong,
meaning it would truncate output data to half length.

This would show up if `platform.stdio-buffered-serial` was configured to
true, `platform.stdio-convert-newlines` was still false - `mbed_error`
crashes would be garbled.

This wasn't usually spotted because applications generally have both
settings false or both true, and if newline conversion is on, then
`mbed_error_puts` writes 1 character at a time to FileHandle::write,
avoiding the length error.
Assert failure took a critical section before calling `mbed_error`.

There's no need to take a critical section on assert failure -
mbed_error does not do this, and is designed to operate from normal
contexts.

Avoiding the critical section will improve the chances of console
initialisation due to assert failure working nicely.
Prime the console outside the critical section, improving the chances of
nice initialisation.
@ciarmcom ciarmcom requested review from a team April 9, 2019 15:00
@ciarmcom
Copy link
Member

ciarmcom commented Apr 9, 2019

@kjbracey-arm, thank you for your changes.
@ARMmbed/mbed-os-core @ARMmbed/mbed-os-maintainers please review.

Once a fatal error is in progress, it's not useful to trap RTX errors
or mutex problems, so short-circuit the checks.

This makes it more likely that we may be able to get the console
initialised if it is being written to for the first time by `mbed_error`
in a difficult context - such as an RTX error callback from inside an
SVCall.

For example, the one-line program

   osMutexAcquire(NULL, 0);

will generate an RTX error trap, then `mbed_error` will try to call
`write(STDERR_FILENO)` to print the error, which will prompt mbed_retarget to
construct a singleton `UARTSerial`. This would trap in the mutex
for the singleton or the construction of the UARTSerial itself, if
we didn't allow this leniency. If we clear the mutex checks, then
`UARTSerial::write_unbuffered` will work.
@kjbracey kjbracey force-pushed the error_print_improvements branch from 67cfca4 to b8e80dd Compare April 15, 2019 08:37
@mbed-ci
Copy link

mbed-ci commented Apr 17, 2019

Test run: SUCCESS

Summary: 11 of 11 test jobs passed
Build number : 1
Build artifacts

@0xc0170 0xc0170 merged commit 94898a1 into ARMmbed:master Apr 17, 2019
@kjbracey kjbracey deleted the error_print_improvements branch April 17, 2019 10:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Printing fault information fails if console is not initialized Mbed error info not printed with mbed-os-5.12.0
5 participants