pm: use of wrong pm_set_lowest?

[cgundogan]

(Thanks for hinting @MichelRottleuthner)

When I add USEMODULE += pm_layered to the Makefile of the default example and when I build for the samr21-xpro board, I get the following:

% arm-none-eabi-objdump -S bin/samr21-xpro/default.elf | grep pm_set_lowest
000005dc <pm_set_lowest>:
void __attribute__((weak)) pm_set_lowest(void) {}
        pm_set_lowest();
     a6a:       f7ff fdb7       bl      5dc <pm_set_lowest>

There are three pm_set_lowest implementations in the code base:

1. drivers/periph_common/pm.c <= __atribute__((weak))
2. cpu/cortexm_common/periph/pm.c
3. sys/pm_layered/pm.c

My understandig is, that the implementation in 3. is the correct one (given that I include the pm_layered module). However, as can be seen above, the implementation defined in 1. is chosen.
The last line of the make command (with QUIET=0) contains:

arm-none-eabi-gcc
RIOT/examples/default/bin/samr21-xpro/newlib_syscalls_default/syscalls.o
RIOT/examples/default/bin/samr21-xpro/cpu/vectors.o
-o
RIOT/examples/default/bin/samr21-xpro/default.elf
-Wl,--start-group
RIOT/examples/default/bin/samr21-xpro/default.a

RIOT/examples/default/bin/samr21-xpro/at86rf2xx.a
RIOT/examples/default/bin/samr21-xpro/auto_init.a
RIOT/examples/default/bin/samr21-xpro/auto_init_gnrc_netif.a
RIOT/examples/default/bin/samr21-xpro/auto_init_saul.a
RIOT/examples/default/bin/samr21-xpro/board.a
RIOT/examples/default/bin/samr21-xpro/core.a
RIOT/examples/default/bin/samr21-xpro/cortexm_common.a
RIOT/examples/default/bin/samr21-xpro/cortexm_common_periph.a
RIOT/examples/default/bin/samr21-xpro/cpu.a
RIOT/examples/default/bin/samr21-xpro/div.a
RIOT/examples/default/bin/samr21-xpro/fmt.a
RIOT/examples/default/bin/samr21-xpro/gnrc.a
RIOT/examples/default/bin/samr21-xpro/gnrc_netapi.a
RIOT/examples/default/bin/samr21-xpro/gnrc_netdev2.a
RIOT/examples/default/bin/samr21-xpro/gnrc_netif.a
RIOT/examples/default/bin/samr21-xpro/gnrc_netif_hdr.a
RIOT/examples/default/bin/samr21-xpro/gnrc_netreg.a
RIOT/examples/default/bin/samr21-xpro/gnrc_pkt.a
RIOT/examples/default/bin/samr21-xpro/gnrc_pktbuf_static.a
RIOT/examples/default/bin/samr21-xpro/gnrc_pktdump.a
RIOT/examples/default/bin/samr21-xpro/ieee802154.a
RIOT/examples/default/bin/samr21-xpro/isrpipe.a
RIOT/examples/default/bin/samr21-xpro/luid.a
RIOT/examples/default/bin/samr21-xpro/netdev2_ieee802154.a
RIOT/examples/default/bin/samr21-xpro/netopt.a
RIOT/examples/default/bin/samr21-xpro/newlib_syscalls_default.a
RIOT/examples/default/bin/samr21-xpro/od.a
RIOT/examples/default/bin/samr21-xpro/periph.a
RIOT/examples/default/bin/samr21-xpro/periph_common.a
RIOT/examples/default/bin/samr21-xpro/phydat.a
RIOT/examples/default/bin/samr21-xpro/pm_layered.a
RIOT/examples/default/bin/samr21-xpro/ps.a
RIOT/examples/default/bin/samr21-xpro/sam0_common_periph.a
RIOT/examples/default/bin/samr21-xpro/saul.a
RIOT/examples/default/bin/samr21-xpro/saul_reg.a
RIOT/examples/default/bin/samr21-xpro/shell.a
RIOT/examples/default/bin/samr21-xpro/shell_commands.a
RIOT/examples/default/bin/samr21-xpro/sys.a
RIOT/examples/default/bin/samr21-xpro/tsrb.a
RIOT/examples/default/bin/samr21-xpro/uart_stdio.a
RIOT/examples/default/bin/samr21-xpro/xtimer.a
-lm
-Wl,--end-group

-Wl,-Map=RIOT/examples/default/bin/samr21-xpro/default.map
-Wl,--cref
-LRIOT/cpu/sam0_common/ldscripts
-LRIOT/cpu/samd21/ldscripts
-LRIOT/cpu/cortexm_common/ldscripts
-Tsamr21g18a.ld
-Wl,--fatal-warnings
-mcpu=cortex-m0plus
-mlittle-endian
-mthumb
-mfloat-abi=soft
-mno-thumb-interwork
-ggdb
-g3
-Os
-static
-lgcc
-nostartfiles
-Wl,--gc-sections
-specs=nano.specs
-lc
-lnosys

The above output conforms with what we do in https://github.com/RIOT-OS/RIOT/blob/master/Makefile.modules#L6-L7 : The modules are sorted alphabetically and thus, pm_layered is passed to the linker quite lately and is (AFAIK) ignored by the first matching pm_set_lowest in periph_common/pm.c.

Using weak symbols in static libraries has other semantics than in shared ones, i.e. with a static library the symbol lookup stops at the first symbol – even if it is just weak and an object file with a strong symbol is also included in the library archive. On Linux, the linker option --whole-archive changes that behavior.

(s. https://en.wikipedia.org/wiki/Weak_symbol#Limitations)
I tried to prepend --whole-archive to the linker flags, but encountered a lot more issues with that.

When I manually move the line

RIOT/examples/default/bin/samr21-xpro/pm_layered.a

above

RIOT/examples/default/bin/samr21-xpro/periph_common.a

and do the linking manually, then I get the desired behavior:

% arm-none-eabi-objdump -S bin/samr21-xpro/default.elf | grep pm_set_lowest
000005dc <pm_set_lowest>:
void pm_set_lowest(void)
     5de:       4d0f            ldr     r5, [pc, #60]   ; (61c <pm_set_lowest+0x40>)
     5ee:       d106            bne.n   5fe <pm_set_lowest+0x22>
     5f6:       d102            bne.n   5fe <pm_set_lowest+0x22>
     60a:       d102            bne.n   612 <pm_set_lowest+0x36>
    pm_set_lowest();
     628:       f7ff ffd8       bl      5dc <pm_set_lowest>
        pm_set_lowest();
     ab6:       f7ff fd91       bl      5dc <pm_set_lowest>

Any ideas on how to fix, or am I doing something wrong here?

[cgundogan]

Obviously, nm is reporting the same:

% arm-none-eabi-nm bin/samr21-xpro/default.elf | grep pm_set_lowest 
000005dc W pm_set_lowest

vs.
(manually, corrected linking)

% arm-none-eabi-nm bin/samr21-xpro/default.elf | grep pm_set_lowest
000005dc T pm_set_lowest

[kaspar030]

There are three |pm_set_lowest| implementations in the code base: 1. |drivers/periph_common/pm.c| <= |__atribute__((weak))| 2. |cpu/cortexm_common/periph/pm.c| 3. |sys/pm_layered/pm.c| My understandig is, that the implementation in 3. is the correct one (given that I include the |pm_layered| module). However, as can be seen above, the implementation defined in 1. is chosen.

See #5757.

[cgundogan]

Thanks for the hint!

I am wondering with what setup the pm_layered module was tested, as I can't activate the functionality out of the box?

[cgundogan]

seems like I completely overlooked #6655. Sorry! I still keep my issue open as it provides additional info, though.

[travisgriggs]

We've just run into his same issue with the samd21. So glad you documented what's going on here. We were pulling our hair out. Any hope of a solution emerging?

[kYc0o]

I don't know if there's a proposed solution yet but looking at the status of the PRs marked for this release I think this issue should be also postponed.

[immesys]

Just a reminder, this is present in 2017.04 release: power management doesn't work on samr21.

[roberthartung]

@cgundogan This should be fixed with 2017.10, could you verify again?

[kaspar030]

Fixed by #7726.