Skip to content
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

Printing fault information fails if console is not initialized #10344

Closed
marcemmers opened this issue Apr 8, 2019 · 8 comments

Comments

Projects
None yet
5 participants
@marcemmers
Copy link
Contributor

commented Apr 8, 2019

Description

I am experiencing some issues where fault information is not printed in certain cases. It doesn't matter if it is an assert or a hardfault that needs to be printed.

I am testing on an nRF52840 but as far as I can tell it is not target related.
Toolchain is IAR 8.30.1
Mbed version 5.11.5

The test code could be as simple as:

int main()
{
    MBED_ASSERT( false );
    return 0;
}

This issue is that the default console is a singleton and as such gets constructed on the first use:

static FileHandle *default_console()

The singleton guards in IAR use a system mutex that eventually goes to the mbed mutex implementation. It tries to acquire a mutex by calling osMutexAcquire() but that fails on IsIrqMasked() because we are in a critical section from mbed_error_puts(const char *str).

If I put a printf("\n") before my assert the fault information is printed successfully because the object is already constructed. I think this is also the reason why it hasn't shown up in the testcases because they all use prints before the error handling tests.

This could be an issue only for IAR because of the way the singleton is handled but I have no means to verify that easily.

Issue request type

[ ] Question
[ ] Enhancement
[x] Bug
@0xc0170

This comment has been minimized.

Copy link
Member

commented Apr 8, 2019

@marcemmers

This comment has been minimized.

Copy link
Contributor Author

commented Apr 8, 2019

The issue seems to be related to #10242 in a way but the changes that seem to have created the issue were released in 5.11.0, see: #8076

@kjbracey-arm

This comment has been minimized.

Copy link
Contributor

commented Apr 8, 2019

I think this may be IAR-only if IAR doesn't actually open stdin or stdout until first use. I've never been terribly clear on what the C libraries do, but I know some have reported power increase after first print, which would be consistent with that.

I guess we could ignore the mutex error if error_in_progress is set. Unsafe, but better than just crashing I guess. Where is the error, exactly? EvrRtxMutexError calling MBED_ERROR1?

@marcemmers

This comment has been minimized.

Copy link
Contributor Author

commented Apr 8, 2019

It certainly does look like a C library issue. It is only when I print a newline to printf that it actually fixed the issue because default it only flushes to stdout on the newline.

The second call to mbed_error from EvrRtxMutexError does not try to print because there is an error in progress so it just halts the system.

Wouldn't it be a better solution to force the construction during startup by making the object global? This should work for all compilers. The optimizer can't remove the memory if not used now anyway which is already being solved by you in #10328. This can still apply to the following example:


#if DEVICE_SERIAL
#  if MBED_CONF_PLATFORM_STDIO_BUFFERED_SERIAL
    static UARTSerial __default_console(STDIO_UART_TX, STDIO_UART_RX, MBED_CONF_PLATFORM_STDIO_BAUD_RATE);
#  else
    static DirectSerial __default_console(STDIO_UART_TX, STDIO_UART_RX, MBED_CONF_PLATFORM_STDIO_BAUD_RATE);
#  endif
#else // DEVICE_SERIAL
    static Sink __default_console;
#endif

static FileHandle *default_console()
{
#if DEVICE_SERIAL && MBED_CONF_PLATFORM_STDIO_BUFFERED_SERIAL
#   if   CONSOLE_FLOWCONTROL == CONSOLE_FLOWCONTROL_RTS
    __default_console.set_flow_control(SerialBase::RTS, STDIO_UART_RTS, NC);
#   elif CONSOLE_FLOWCONTROL == CONSOLE_FLOWCONTROL_CTS
    __default_console.set_flow_control(SerialBase::CTS, NC, STDIO_UART_CTS);
#   elif CONSOLE_FLOWCONTROL == CONSOLE_FLOWCONTROL_RTSCTS
    __default_console.set_flow_control(SerialBase::RTSCTS, STDIO_UART_RTS, STDIO_UART_CTS);
#   endif
#endif
    return &__default_console;
}
@kjbracey-arm

This comment has been minimized.

Copy link
Contributor

commented Apr 8, 2019

Wouldn't it be a better solution to force the construction during startup by making the object global? This should work for all compilers.

From the point of view of "we need this to be on so we can print to it when we crash", yes, that would make sense. Also would improve reception behaviour.

But at the cost of power - any application not currently printing is currently saving power by the fact it isn't opened.

After #10328, I think you could make this change, but I'm a bit wary about impacting run-time behaviour for the crash case, which is why I'm suggesting a change that would only affect the crash.

@marcemmers

This comment has been minimized.

Copy link
Contributor Author

commented Apr 8, 2019

But at the cost of power - any application not currently printing is currently saving power by the fact it isn't opened.

I totally agree with this, I have been working with some applications requiring an absolute minimum consumption so this is an hot issue. On the other hand, if the other compilers open the stdout during init, would it change anything for those compilers? It seems to me it will only result in the same behaviour and would also include IAR this time.

In the end it doesn't matter to me which way too go. For now I'm using the mbed_override_console function as a fix for the issue.

@ciarmcom

This comment has been minimized.

Copy link
Member

commented Apr 8, 2019

@ciarmcom ciarmcom added the mirrored label Apr 8, 2019

@lrusinowicz

This comment has been minimized.

Copy link
Contributor

commented Apr 8, 2019

I remember seeing exactly same sequence under GCC (and FUTURE_SEQUANA) once. Unfortunately, I didn't have time to investigate and report an issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.