USTICKER still can not be removed from compilation

[mikisch81]

Description

This is a follow-up issue for #9853 which was opened due to 9221 (comment).

After rebasing #9221 I get the following compilation error after moving us_ticker_api.c to the non-secure target and putting an #if DEVICE_USTICKER in mbed_us_ticker_api.c:

Error: L6218E: Undefined symbol get_us_ticker_data (referred from BUILD/ARM_MUSCA_A1_S/ARMC6/mbed-os/drivers/Timer.o).

@ARMmbed/mbed-os-psa @kjbracey-arm @jeromecoutant

Issue request type

[ ] Question
[ ] Enhancement
[X] Bug

[kjbracey]

I suggest you make the definition of Timer::Timer() conditional on DEVICE_USTICKER. TimerEvent will also need the same.

The ARM linker forces programs to obey the C++ "One Definition Rule", so that default constructor referring to get_us_ticker_data forces it to have one definition, even if the default constructor is never called. If the definition doesn't exist, it's not a legal C++ program:

Every program shall contain exactly one definition of every non-inline function or variable that is odr-used in that program; no diagnostic required.

An alternative workaround would be to eliminate the default constructor and instead declare the other constructor with Timer::Timer(const ticker_data_t *data = get_us_ticker_data()), but messing with the constructor overloads might introduce binary compatibility issues we could do without. That would work because naming something in an unused default parameter doesn't count as odr-using it, unlike having it in an unused function.

[mikisch81]

Do you mean that in Timer.cpp do this:

#if DEVICE_USTICKER
Timer::Timer(const ticker_data_t *data) : _running(), _start(), _time(), _ticker_data(data), _lock_deepsleep(true)
{
    reset();
#if DEVICE_LPTICKER
    _lock_deepsleep = (data != get_lp_ticker_data());
#endif
}
#endif // DEVICE_USTICKER

and in TimerEvent.cpp:

#if DEVICE_USTICKER
TimerEvent::TimerEvent(const ticker_data_t *data) : event(), _ticker_data(data)
{
    ticker_set_handler(_ticker_data, (&TimerEvent::irq));
}
#endif // DEVICE_USTICKER

[kjbracey]

Nearly - it's the default constructors Timer::Timer() and TimerEvent::TimerEvent() that need the conditionals - they refer to get_us_ticker_data in their initialiser list.

[ciarmcom]

Internal Jira reference: https://jira.arm.com/browse/MBOCUSTRIA-1020

[mikisch81]

Nearly - it's the default constructors Timer::Timer() and TimerEvent::TimerEvent() that need the conditionals - they refer to get_us_ticker_data in their initialiser list.

Yeah, I meant the default constructors

[mikisch81]

@kjbracey-arm Timer default constructor is being called in the code, for example in mbed_poll.cpp, should we explicitely change all usages to the normal constructor?

[kjbracey]

Well, you'd then be passing get_us_ticker_api to the normal constructor. Doesn't get you anywhere.

This seems like it's starting to unravel. Code like that poll would work fine without a timer, if the parameter passed in was 0 or -1, but it would need special casing to let that still work. I'm starting to look at that incidentally in #10104 - trying to make reduce/eliminate timer pull in for the bare metal RTOS APIs, making sure that you can do untimed semphore/flag waits if timing is totally absent, and also reducing incidental pull-in of timers.

Anyway, I think for now you should instead provide a dummy alternative get_us_ticker_data(), rather than try to eliminate it. Just make it return NULL if DEVICE_USTICKER is false.

That will work as long as no-one is using Timer or Ticker in your build. If you were compiling fine on GCC, then no-one is using it, so you'll be okay.

Alternatively you could provide a dummy ticker_interface_t with stub functions, but probably would be more confusing than useful.

[mikisch81]

OK, Adding an #else in mbed_us_ticker_api.c for the #if DEVICE_USTICKER which has an implementation of get_us_ticker_data() which returns NULL compiles OK, I will issue a PR for that.

[mikisch81]

Opened #10155 to fix this issue