tests/libc_newlib: add test for newlib-nano inclusion by cladmi · Pull Request #9599 · RIOT-OS/RIOT

cladmi · 2018-07-18T14:37:43Z

Contribution description

This test verifies that when used, newlib-nano is correctly included, both
header and also linked.

This is a standalone test and is less invasive than doing a global test like what I tried in #9512

I want to use it to debug non handled developers computers.

Testing

I tested the following setups:

native (no newlib)
pic32-wifire (newlib) using the flasher PR
samr21-xpro (newlib-nano)

Issues/PRs references

#9512

~~This should not be merged before #9398 as currently llvm is not handled properly but it should not block adding tests in murdock.~~
I now think it should be included before #9398, I thought it would be all fixed faster.
I already have a fix for the llvm/clang issue #9513

miri64 · 2018-07-19T13:04:17Z

Do we really need to run tests on this PR everytime ;-)?

cladmi · 2018-07-19T13:06:18Z

I was still getting a redis error from the build output.

jcarrano

Good. I can pass the test, but I can't fail it (Arch linux).

jcarrano · 2018-07-23T13:40:46Z

tests/libc_newlib/README.md

+About
+=====
+
+Terifies if newlib/newlib-nano is correctly included by the build system


Can you include instructions on how to force a failure of this test? Then people can test the test.

The link time test (currently done at runtime) can be tested by commenting this line:

RIOT/makefiles/libc/newlib.mk

Line 17 in db6cb1a

export LINKFLAGS += -specs=nano.specs

And the compile time test can be tested by commenting this one:

RIOT/makefiles/libc/newlib.mk

Line 80 in db6cb1a

INCLUDES := -isystem $(NEWLIB_NANO_INCLUDE_DIR) $(INCLUDES)

Tested both tests and it failed as expected.

jcarrano · 2018-07-23T13:42:01Z

tests/libc_newlib/main.c

+#ifdef MODULE_NEWLIB
+#ifdef MODULE_NEWLIB_NANO
+    /* Nano maps iprintf to printf */
+    TEST_ASSERT(iprintf == printf);


This can be tested by binutils, so there's no need to run the application.

I will try something for this.

jcarrano · 2018-07-23T13:48:37Z

My test results (`BOARD=stm32f4discovery`, Arch Linux):

2018-07-23 15:39:23,382 - INFO # main(): This is RIOT! (Version: 2018.10-devel-27-ga2307-batman-test_newlibnano)
2018-07-23 15:39:23,383 - INFO # Newlib/nano test
2018-07-23 15:39:23,383 - INFO # .
2018-07-23 15:39:23,383 - INFO # OK (1 tests)

System

Installed compiler toolchains 
-----------------------------
             native gcc: gcc (GCC) 8.1.1 20180531
      arm-none-eabi-gcc: arm-none-eabi-gcc (Arch Repository) 8.1.0
                avr-gcc: missing
       mips-mti-elf-gcc: missing
             msp430-gcc: missing
   riscv-none-embed-gcc: missing
                  clang: clang version 6.0.1 (tags/RELEASE_601/final)

Installed compiler libs
-----------------------
   arm-none-eabi-newlib: "3.0.0"
    mips-mti-elf-newlib: missing
riscv-none-embed-newlib: missing
               avr-libc: missing (missing)

Installed development tools
---------------------------
                  cmake: cmake version 3.11.4
               cppcheck: Cppcheck 1.84
                doxygen: 1.8.14
                 flake8: 3.5.0 (mccabe: 0.6.1, pycodestyle: 2.4.0, pyflakes: 2.0.0) CPython 3.6.6 on Linux
                    git: git version 2.18.0
             coccinelle: missing

jcarrano · 2018-07-23T13:49:40Z

Open question: How can we make it so that we force the user to run the test?

cladmi · 2018-07-23T14:36:31Z

I added a test in the Makefile. Currently not documented but I should move the documentation out of main.c to the makefile.

cladmi · 2018-07-23T14:40:03Z

I also tested wih TOOLCHAIN=llvm it currently fails during compilation because this is not fixed #9513.
I commented some parts to be able to run the test with llvm-nm at it also worked.

cladmi · 2018-07-23T16:20:31Z

I split the NM change in a separate PR.

@jcarrano Tell me if you like the new tests. I will update the documentation after

@miri64 I now think it should be included before #9398, I thought it would be all fixed faster.
I already have a fix for the llvm/clang issue #9513. What do you think ?
I would really like to go forward for fixing newlib for the release.

jcarrano · 2018-07-24T09:06:22Z

tests/libc_newlib/Makefile

+  endif
+endif
+
+test-newlib: $(ELFFILE)


A dumber approach without sed (checks the number of different addresses)

# make N_PRINTFS = 2 if non-nano, 1 if nano test-newlib: $(ELFFILE) PRINTFS=$$($(NM) -v $^ | grep '$ i\?printf$'); \ PRINTF_ADDRS=$$(echo "$${PRINTFS}" | cut -d' ' -f 1 | uniq | wc -l); \ # ensure both symbols are there if test $$(echo "$${PRINTFS}" | wc -l) = 2 ; then \ echo "[FAILED] Expected printf and iprintf to be present, found $${PRINTFS}" ; \ exit 1; \ elif test $${PRINTF_ADDRS} != $(N_PRINTFS) ; then \ echo "[FAILED] Expected $(N_PRINTFS) different printfs"; \ exit 1; \ fi

BTW, could we define a standard set of shell functions to be included in all makefiles. I would like to have a 'die' function:

die() { echo "${1}" >&2 exit 1 } # one-line "do stuff and exit with error if it fails" test -e fileXX.txt || die "file not found"

Using more sub commands, and not simpler for me to understand as it uses two concepts, number of printfs + addresses instead of just comparing addresses.

sed is standard enough and is working properly.

Adding common functions makefile would be a different PR with its own problem.

Also for me echo; exit can be understood in one blink and not complex enough that I would like it replaced by another function that I would need to check. Also it can be done with oneliner as

test -e fileXX.txt || { echo "file not found"; exit 1};

jcarrano · 2018-07-25T09:15:31Z

@cladmi The nm test is good.

I now think it should be included before #9398.

I think this PR should have a high priority, the newlib thing is a bug waiting to happen.

jcarrano

Good. We need another PR to make this test compulsory.

This test verifies that when used, newlib-nano is correctly included, both header and also linked.

cladmi · 2018-07-25T10:28:57Z

Reminder, for backporting I would also need to add the NM changes.

cladmi · 2018-07-25T11:40:42Z

Backport provided in #9633

cladmi force-pushed the pr/make/newlib/add_test_for_newlibnano branch from d2cd568 to a23078e Compare July 19, 2018 12:55

cladmi added the Area: toolchain Area: toolchains; everything related to compilation, libc, linking, … label Jul 19, 2018

jcarrano reviewed Jul 23, 2018

View reviewed changes

cladmi mentioned this pull request Jul 23, 2018

toolchain/gnu: also define NM #9624

Merged

cladmi added State: waiting for other PR State: The PR requires another PR to be merged first and removed State: waiting for other PR State: The PR requires another PR to be merged first labels Jul 23, 2018

jcarrano reviewed Jul 24, 2018

View reviewed changes

cladmi force-pushed the pr/make/newlib/add_test_for_newlibnano branch from 207e634 to 3e29c10 Compare July 24, 2018 11:51

jcarrano approved these changes Jul 25, 2018

View reviewed changes

tests/libc_newlib: add test for newlib-nano inclusion

db00922

This test verifies that when used, newlib-nano is correctly included, both header and also linked.

cladmi force-pushed the pr/make/newlib/add_test_for_newlibnano branch from 0a7f6a1 to db00922 Compare July 25, 2018 09:43

cladmi added Process: needs backport Integration Process: The PR is required to be backported to a release or feature branch and removed State: waiting for other PR State: The PR requires another PR to be merged first labels Jul 25, 2018

cladmi 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 Jul 25, 2018

jcarrano merged commit 0a53b61 into RIOT-OS:master Jul 25, 2018

This was referenced Jul 25, 2018

murdock: also compile with LLVM/clang #9398

Merged

toolchain/gnu: also define NM [backport 2018.07] #9632

Merged

tests/libc_newlib: add test for newlib-nano inclusion [backport 2018.07] #9633

Merged

cladmi mentioned this pull request Jul 25, 2018

newlib.mk: llvm, fix newlib-nano header not used #9513

Merged

cladmi deleted the pr/make/newlib/add_test_for_newlibnano branch July 25, 2018 13:01

cladmi mentioned this pull request Jul 25, 2018

WIP newlib.mk: fix regressions and global cleanups, tracking evolutions of newlib.mk #9512

Draft

4 tasks

cladmi mentioned this pull request Aug 2, 2018

newlib.mk: llvm, fix newlib-nano header not used [backport 2018.07] #9685

Merged

cladmi added this to the Release 2018.07 milestone Aug 11, 2018

jcarrano mentioned this pull request Sep 12, 2019

pkg/tlsf: Fix the way system functions are overriden. #12031

Merged

maribu mentioned this pull request Aug 19, 2025

tests/sys/libc_newlib: drop test #21669

Merged

Conversation

cladmi commented Jul 18, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Contribution description

Testing

Issues/PRs references

Uh oh!

miri64 commented Jul 19, 2018

Uh oh!

cladmi commented Jul 19, 2018

Uh oh!

jcarrano left a comment

Choose a reason for hiding this comment

Uh oh!

jcarrano Jul 23, 2018

Choose a reason for hiding this comment

Uh oh!

cladmi Jul 23, 2018

Choose a reason for hiding this comment

Uh oh!

jcarrano Jul 23, 2018

Choose a reason for hiding this comment

Uh oh!

jcarrano Jul 23, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cladmi Jul 23, 2018

Choose a reason for hiding this comment

Uh oh!

jcarrano commented Jul 23, 2018

My test results (BOARD=stm32f4discovery, Arch Linux):

System

Uh oh!

jcarrano commented Jul 23, 2018

Uh oh!

cladmi commented Jul 23, 2018

Uh oh!

cladmi commented Jul 23, 2018

Uh oh!

cladmi commented Jul 23, 2018

Uh oh!

jcarrano Jul 24, 2018

Choose a reason for hiding this comment

Uh oh!

cladmi Jul 24, 2018

Choose a reason for hiding this comment

Uh oh!

jcarrano commented Jul 25, 2018

Uh oh!

jcarrano left a comment

Choose a reason for hiding this comment

Uh oh!

cladmi commented Jul 25, 2018

Uh oh!

cladmi commented Jul 25, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

cladmi commented Jul 18, 2018 •

edited

Loading

jcarrano Jul 23, 2018 •

edited

Loading

My test results (`BOARD=stm32f4discovery`, Arch Linux):