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

[mikisch81]

@kjbracey-arm When building PR #10155, I encounter link failures in target ARCH_PRO which doesn't define USTICKER and gets undefined symbols for us_ticker_irq_handler:

[DEBUG] Errors: BUILD/tests/ARCH_PRO/GCC_ARM/TESTS/mbed_hal/sleep/TESTS/mbed_hal/sleep/main.o: In function `sleep_usticker_test':
[DEBUG] Errors: /home/micsch01/miki_dev/mbed-os-example-blinky/mbed-os/./TESTS/mbed_hal/sleep/main.cpp:61: undefined reference to `set_us_ticker_irq_handler'
[DEBUG] Errors: /home/micsch01/miki_dev/mbed-os-example-blinky/mbed-os/./TESTS/mbed_hal/sleep/main.cpp:92: undefined reference to `set_us_ticker_irq_handler'
[DEBUG] Errors: BUILD/tests/ARCH_PRO/GCC_ARM/targets/TARGET_NXP/TARGET_LPC176X/us_ticker.o: In function `us_ticker_init':
[DEBUG] Errors: /home/micsch01/miki_dev/mbed-os-example-blinky/mbed-os/./targets/TARGET_NXP/TARGET_LPC176X/us_ticker.c:66: undefined reference to `us_ticker_irq_handler'
[DEBUG] Errors: collect2: error: ld returned 1 exit status

[mikisch81]

Why does this target builds usticker_api.c but doesn't define DEVICE_USTICKER?

[kjbracey]

Seems like a possible error to me. Seems likely that target should have USTICKER on. Previously there were no conditionals on DEVICE_USTICKER, it was assumed, so things like Ticker would always have worked anyway.

[mikisch81]

@ARMmbed/team-nxp Is it possible that target ARCH_PRO needs to add "device_has_add": ["USTICKER"]?

[maclobdell]

Arch Pro is a Seeed Studio platform, not managed by NXP.

[mmahadevan108]

@ARMmbed/team-seeed . Can you help take a look at this question related to the ARCH_PRO