Skip to content

murdock: add selected cortex-m boards to LLVM build#9809

Merged
miri64 merged 2 commits intoRIOT-OS:masterfrom
miri64:murdock/enh/llvm-cortex-m
Sep 25, 2018
Merged

murdock: add selected cortex-m boards to LLVM build#9809
miri64 merged 2 commits intoRIOT-OS:masterfrom
miri64:murdock/enh/llvm-cortex-m

Conversation

@miri64
Copy link
Copy Markdown
Member

@miri64 miri64 commented Aug 21, 2018

Contribution description

This allows Murdock also to build selected Cortex-M boards with LLVM.

There might be some discussion needed on the selection of boards. I tried to have one board present for each:

  • CPU architecture (Cortex-M0(+), Cortex-M3, Cortex-M4, ...)
  • Vendor (?) (STM32, EFM32, Kintetis, NRF, ...)

But up until now the selection is pretty arbitrary.

Issues/PRs references

This list was originally selected in #9398.

Depends on #9734 (merged), #9821, #9824 (postponed), and #9808 (merged).

Dependencies due to testing: #10014 (merged), #10024 (merged).

@miri64 miri64 added Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Area: CI Area: Continuous Integration of RIOT components labels Aug 21, 2018
@miri64 miri64 requested review from cladmi and kaspar030 August 21, 2018 09:33
@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Aug 21, 2018

Vendor (?) (STM32, EFM32, Kintetis, NRF, ...)

At least I thought I had kinetis in there :-/. @gebart any frdm board you prefer in that list or should I put mulle in there ;-)?

@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Aug 21, 2018

Murdock likes it :-)

@jnohlgard
Copy link
Copy Markdown
Member

@miri64 pick any kinetis board with a large number of FEATURES_PROVIDED. Mulle is one candidate, any of the frdm boards are also good.

@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Aug 21, 2018

Then I take mulle because it has 9, compared to the frdm's 8 ;-)

@miri64 miri64 force-pushed the murdock/enh/llvm-cortex-m branch from 92a0757 to bb33dbe Compare August 21, 2018 21:15
@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Aug 21, 2018

Rebased to current master and dependencies, added mulle to list of pre-selected boards.

@miri64 miri64 force-pushed the murdock/enh/llvm-cortex-m branch 3 times, most recently from 7c03a50 to c1707fc Compare August 22, 2018 13:28
@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Aug 22, 2018

Rebased to current master and dependencies, as #9808 was merged.

@cladmi
Copy link
Copy Markdown
Contributor

cladmi commented Aug 23, 2018

With #9821 it looks like I can correctly compile examples/javascript with llvm both on ubuntu 16.04 and in the docker image with BOARD=samr21-xpro.

@miri64 miri64 force-pushed the murdock/enh/llvm-cortex-m branch 2 times, most recently from d84c57e to 7661dad Compare September 9, 2018 14:19
@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Sep 9, 2018

Rebased to include current master and #9821.

@miri64 miri64 added State: waiting for other PR State: The PR requires another PR to be merged first CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Sep 9, 2018
@kaspar030 kaspar030 added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Sep 9, 2018
@miri64 miri64 force-pushed the murdock/enh/llvm-cortex-m branch from 7661dad to 80ae93e Compare September 11, 2018 16:32
@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Sep 11, 2018

Rebased to see if #9821 still fixes the jerryscript issues ;-)

@miri64 miri64 force-pushed the murdock/enh/llvm-cortex-m branch from 80ae93e to 4284147 Compare September 11, 2018 17:49
@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Sep 11, 2018

Ooops, the fixes happened in #9824... Included as a new dependency.

@miri64 miri64 removed the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Sep 11, 2018
@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Sep 21, 2018

Maybe a stacksize problem?

I was wrong... a crash is happening before, but in trying to print the thread state, it crashes again within hard_fault_handler again.

@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Sep 21, 2018

Maybe a stacksize problem?

Welp, I was able to pin it down on the thread_create() in gnrc_netif_create(), but when I just add one puts(), tests/gnrc_sixlowpan isn't failing anymore with LLVM:

diff --git a/sys/net/gnrc/netif/gnrc_netif.c b/sys/net/gnrc/netif/gnrc_netif.c
index 61a5cf5..79e6d87 100644
--- a/sys/net/gnrc/netif/gnrc_netif.c
+++ b/sys/net/gnrc/netif/gnrc_netif.c
@@ -1318,6 +1318,7 @@ static void *_gnrc_netif_thread(void *args)
 
     while (1) {
         DEBUG("gnrc_netif: waiting for incoming messages\n");
+        puts("");
         msg_receive(&msg);
         /* dispatch netdev, MAC and gnrc_netapi messages */
         switch (msg.type) {

@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Sep 21, 2018

Even

diff --git a/sys/net/gnrc/netif/gnrc_netif.c b/sys/net/gnrc/netif/gnrc_netif.c
index 61a5cf5..92377a7 100644
--- a/sys/net/gnrc/netif/gnrc_netif.c
+++ b/sys/net/gnrc/netif/gnrc_netif.c
@@ -1318,6 +1318,7 @@ static void *_gnrc_netif_thread(void *args)
 
     while (1) {
         DEBUG("gnrc_netif: waiting for incoming messages\n");
+        write(0, "", 0);
         msg_receive(&msg);
         /* dispatch netdev, MAC and gnrc_netapi messages */
         switch (msg.type) {

is enough to let the test pass oO.

Edit: fixed copy pasta

@kaspar030
Copy link
Copy Markdown
Contributor

Could be an alignment problem. I suggest checking the objdump output.

@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Sep 21, 2018

Could be an alignment problem. I suggest checking the objdump output.

I don't even know what to look for there :-/

@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Sep 21, 2018

>>> where
#0  0xfffffffe in ?? ()
#1  0x00000720 in stdio_write (buffer=0x200042d0, len=18) at /home/mlenders/Repositories/RIOT-OS/RIOT/sys/stdio_uart/stdio_uart.c:72
#2  0x00000126 in _write_r (r=<optimized out>, fd=<optimized out>, data=0x0 <cortex_vector_base>, count=536888016) at /home/mlenders/Repositories/RIOT-OS/RIOT/sys/newlib_syscalls_default/syscalls.c:415
#3  0x0000ab96 in __swrite ()
#4  0x0000a122 in __sflush_r ()
#5  0x0000a172 in _fflush_r ()
#6  0x00009f02 in __swbuf_r ()
#7  0x0000a58e in __sfputc_r ()
#8  0x0000a5ae in __sfputs_r ()
#9  0x0000a616 in _vfprintf_r ()
#10 0x00009d5e in printf ()
#11 0x0000057c in hard_fault_handler (sp=0x20000e88 <_netif_stack+200>, corrupted=<optimized out>, exc_return=4294967293, r4_to_r11_stack=<optimized out>) at /home/mlenders/Repositories/RIOT-OS/RIOT/cpu/cortexm_common/vectors_cortexm.c:272
#12 0x00000508 in hard_fault_default () at /home/mlenders/Repositories/RIOT-OS/RIOT/cpu/cortexm_common/vectors_cortexm.c:166
Backtrace stopped: previous frame identical to this frame (corrupt stack?)

@cladmi
Copy link
Copy Markdown
Contributor

cladmi commented Sep 21, 2018

I added a fix that should fix the libc_newlib test. Basically, there is an issue to compare two function pointers… looks like some pointer optimization. The dedicated PR for reviewing it is #10014

@RIOT-OS RIOT-OS deleted a comment Sep 21, 2018
@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Sep 24, 2018

I don't even know what to look for there :-/

Ok, with some LED toggling and clever puts() setting I was able to pin-point it on the fact that the dummy-device in the tests/gnrc_sixlowpan test isn't providing a link-layer address. While with GNU this doesn't seem to be necessary, LLVM seems to crash the system due to it. I'll try to figure out why.

@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Sep 24, 2018

(the behavior of the test is broken anyway, because the dummy device tells the application its address length is 8, but then when trying to get an address it returns -ENOTSUP, but I'd like to find out why this leads to a crash with LLVM regardless)

@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Sep 24, 2018

This is so weird... I applied the following patch:

diff --git a/sys/net/netdev_test/netdev_test.c b/sys/net/netdev_test/netdev_test.c
index 69883c1..0851d88 100644
--- a/sys/net/netdev_test/netdev_test.c
+++ b/sys/net/netdev_test/netdev_test.c
@@ -93,7 +93,9 @@ static int _get(netdev_t *netdev, netopt_t opt, void *value, size_t max_len)
     netdev_test_t *dev = (netdev_test_t *)netdev;
     int res = -ENOTSUP;     /* option assumed to be not supported */
 
+    puts(netopt2str(opt));
     mutex_lock(&dev->mutex);
+    puts("Ok");
     if (dev->get_cbs[opt] != NULL) {
         res = dev->get_cbs[opt](netdev, value, max_len);
     }

and compiled tests/gnrc_sixlowpan with LLVM. The output on samr21-xpro looks as follows:

2018-09-24 12:44:14,826 - INFO # main(): This is RIOT! (Version: 2018.10-devel-851-gfc32f-sarajevo)
2018-09-24 12:44:14,829 - INFO # RIOT network stack example application
2018-09-24 12:44:14,831 - INFO # NETOPT_DEVICE_TYPE
2018-09-24 12:44:14,831 - INFO # Ok
2018-09-24 12:44:14,833 - INFO # NETOPT_MAX_PACKET_SIZE
2018-09-24 12:44:14,834 - INFO # Ok
2018-09-24 12:44:14,835 - INFO # NETOPT_SRC_LEN
2018-09-24 12:44:14,835 - INFO # Ok
2018-09-24 12:44:14,837 - INFO # NETOPT_ADDRESS_LONG
2018-09-24 12:44:14,837 - INFO # Ok
2018-09-24 12:44:14,839 - INFO # NETOPT_IPV6_IID
2018-09-24 12:44:14,839 - INFO # 
2018-09-24 12:44:14,842 - INFO # Context before hardfault:

So it is to assume the node crashes when the mutex is locked for the IID. The call to the function with the provided output happens here

if (res == -ENOTSUP) {
res = netif->dev->driver->get(netif->dev, opt->opt, opt->data, opt->data_len);
}

So it is only due to the previously mentioned fact, that this device is providing an address length but not an address, that we come to this point. It is strange nevertheless, that the node crashes at this point and the only reason I find logical is that the dev pointer might be wrong.

@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Sep 24, 2018

There is a fix for the link-layer address issue, but the crashing still happens in that test...

@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Sep 24, 2018

I'm pretty sure it doesn't happen in get() this time, however. I set LED_ON(0), when I enter the netdev_test:_get function and LED_OFF(0) when I leave it, but it is of at the point of crash.

@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Sep 24, 2018

It was a stack-size issue within the test after all -.-: #10024.

@miri64 miri64 force-pushed the murdock/enh/llvm-cortex-m branch from 2914232 to 5edfaa3 Compare September 25, 2018 07:50
@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Sep 25, 2018

Rebased to current master and #10014

@miri64 miri64 force-pushed the murdock/enh/llvm-cortex-m branch from 5edfaa3 to 740e772 Compare September 25, 2018 14:49
@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Sep 25, 2018

Rebased to current master.

@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Sep 25, 2018

Murdock should be happy with it now, but let's see.

@miri64 miri64 added the Community: Hack'n'ACK candidate This PR is a candidate for review and discussion during one of RIOT's monthly Hack'n'ACK parties label Sep 25, 2018
Copy link
Copy Markdown
Contributor

@cladmi cladmi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK

In theory I would say that commit order should be inverted to have the fix first and only have working commits, but as it is only on the CI, so normally not subject to bisect I let you swap before merging or not.

@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Sep 25, 2018

Well the second fix happened in reaction to an issue with the first one and since they are of different authorship and address separate issues, they should stay distinct, so I say it's fine to merge them in that order.

@miri64 miri64 merged commit bbc16cc into RIOT-OS:master Sep 25, 2018
@miri64 miri64 deleted the murdock/enh/llvm-cortex-m branch September 25, 2018 16:40
@cladmi
Copy link
Copy Markdown
Contributor

cladmi commented Sep 25, 2018

LLVM support youhou !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: CI Area: Continuous Integration of RIOT components CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR CI: run tests If set, CI server will run tests on hardware for the labeled PR Community: Hack'n'ACK candidate This PR is a candidate for review and discussion during one of RIOT's monthly Hack'n'ACK parties Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants