Add minimal printf #11051

evedon · 2019-07-15T16:27:17Z

Description

This PR imports https://github.com/ARMmbed/minimal-printf into mbed-os.
To replace the standard implementation of the printf functions with the ones from the minimal-printf library, the custom minimal-printf profile should be used when compiling with mbed-cli. For example:

 $ mbed compile -t <toolchain> -m <target> --profile release --profile minimal-printf

Minimal-printf supports floating point and stream printing by default. It has the following restrictions:

All floating points are treated as %f.
No support for inf, infinity or nan
All flags and precision modifiers are ignored.

Pelion Device Management Client example compiled with GCC_ARM, release profile compared to current master (with std printf):

Total Static RAM memory (data + bss): 57712(+0) bytes
Total Flash memory (text + data): 370186(-17490) bytes

Pull request type

[ ] Fix
[ ] Refactor
[ ] Target update
[X] Functionality change
[X] Docs update
[X] Test update
[ ] Breaking change

Reviewers

@bulislaw @hugueskamba

Release Notes

To replace the standard implementation of the printf functions with the ones from the minimal-printf library, the custom minimal-printf profile should be used when compiling with mbed-cli. For example:

 $ mbed compile -t <toolchain> -m <target> --profile release --profile minimal-printf

Supports %d %u %X %p %s Doesn't use malloc

Use mbed_app.json to enable floating points. Automatically ignore flags and width specifiers.

This makes it possible to use the minimal printf and snprintf through mbed_retarget.cpp

Add support for width specifiers

Add compile profiles

This is used by mbed-os-linker-report and doesn't increase the code size.

Add "-g" to 'release' and 'develop' profiles

ssize_t doesn't exist at all in armcc.

Remove references to ssize_t in the code

This commit adds mostly integer (and buffer) overflow checks for the current buffer index (`result` variable).

- Improved comments to explain the checks on 'result'. - Check for non-NULL format specifier.

[BUGFIX][IOTUC-18] Library fixes

The minimal-printf implementation supports a number of length modifiers (j, z and t) that are not supported by the native mbed OS libc implementation. The compliance test has tests for these modifiers, which means that it isn't possible to check the output of mbed-printf against a known good implementation (libc's printf) when running on mbed OS. This, in turn, can give the impression that the tests for these modifiers pass, when that might not be the case. To address this issue, this PR removes the tests for these modifiers in mbed OS. This PR was created because some of the tests for these modifiers actually fail in Linux, for example: ``` >>> Running case ARMmbed#3: 'printf %u'... hhu: 0 hhu: 0 hhu: 255 hhu: 255 hu: 0 hu: 0 hu: 65535 hu: 65535 u: 0 u: 0 u: 4294967295 u: 4294967295 lu: 0 lu: 0 lu: 4294967295 lu: 4294967295 llu: 0 llu: 0 llu: 18446744073709551615 llu: 18446744073709551615 ju: 0 ju: 0 ju: 4294967295 ju: 18446744073709551615 :188::FAIL: Expected 7 Was 16 >>> 'printf %u': 0 passed, 1 failed with reason 'Assertion Failed' ```

In the implementation, don't always display double hex digits when printing with "%X". This is in line with the behaviour observed both in mbed OS's printf (Newlib) and Linux's printf (glibc). In the tests, always compare the baseline result with the result returned by the minimal printf implementation, instead of comparing with a constant value.

The printf(3) man page says "The functions snprintf() and vsnprintf() do not write more than size bytes (including the terminating null byte ('\0')). If the output was truncated due to this limit, then the return value is the number of characters (excluding the terminating null byte) which would have been written to the final string if enough space had been available." The last part of this spec (returning the number of characters which would have been written to the string if enough space had been available) was not implemented in minimal-printf. This PR changes that by redirecting all the processed characters through the "minimal_printf_putchar" helper function. This function will not write to the buffer if there's no space left, but it will always increment the number of written characters, in order to match the above description. minimal_printf_putchar also contains checks for overflows, so these checks are no longer needed in the rest of the code. Other changes: - In some cases, mbed_minimal_formatted_string didn't put the string terminator in the output buffer. This PR ensures that this always happens. - Fixed a bug in printed hexadecimal numbers introduced by a previous commit. - Added buffer overflow tests for snprintf.

Implementation and test fixes

Change the default output from STDIO UART to SWO by overriding: "minimal-printf.console-output": "SWO"

evedon · 2019-08-01T11:55:16Z

This PR needs cleanup for the Git history as it seems to contain many merge commits and therefore looks like has more changes than what it actually has.

Please remove the merge commits and rebase your work on top of master. It makes history and review much cleaner. Thanks.

I have squashed all the commits done for the integration in mbed-os and review comments into the last two commits. All previous commits come from the original library and it would beb useful to keep them.

All comments have now been addressed.
@bulislaw could you review?

evedon · 2019-08-01T11:56:27Z

CI started

mbed-ci · 2019-08-01T17:23:35Z

Test run: FAILED

Summary: 3 of 4 test jobs failed
Build number : 4
Build artifacts

Failed test jobs:

jenkins-ci/mbed-os-ci_build-IAR
jenkins-ci/mbed-os-ci_build-GCC_ARM
jenkins-ci/mbed-os-ci_build-ARM

mbed-ci · 2019-08-01T18:32:27Z

Test run: FAILED

Summary: 1 of 11 test jobs failed
Build number : 5
Build artifacts

Failed test jobs:

jenkins-ci/mbed-os-ci_exporter

mbed-ci · 2019-08-02T10:57:53Z

Test run: SUCCESS

Summary: 11 of 11 test jobs passed
Build number : 6
Build artifacts

evedon · 2019-08-02T11:13:13Z

Run 5 seemed to have failed due to a license issue

19:25:57  [2019-08-01T18:25:57.065Z] + grep Unable to connect to the license server ../exporter_build_log_NUCLEO_L073RZ_gnuarmeclipse.log
[Pipeline] echo
19:25:57  [2019-08-01T18:25:57.079Z] Throwing the error

Job restarted and now successful.

evedon · 2019-08-02T11:21:09Z

@SeppoTakalo Are you happy with the commit history from the original library + three extra commits for this PR?

TESTS/mbed_platform/minimal-printf/compliance/README.md

platform/source/minimal-printf/mbed_printf_armlink_overrides.c

tools/profiles/minimal-printf.json

mbed-ci · 2019-08-05T20:23:09Z

Test run: SUCCESS

Summary: 11 of 11 test jobs passed
Build number : 7
Build artifacts

hugueskamba

LGTM but optimization of the arm toolchain should improved in the future as per #11051 (comment).

evedon · 2019-08-06T10:12:58Z

Any review from @ARMmbed/mbed-os-tools ?

evedon · 2019-08-07T14:01:19Z

Any review from @ARMmbed/mbed-os-tools ?

The part of this PR which affects tools has already been reviewed (and modified) by @madchutney.

PR is ready to go.

kjbracey · 2019-08-12T06:55:54Z

I see that there can be some ambiguity and possibly wrong behaviour between the retarget code for the console and minimal-printf. So if I understand your suggestion correctly, the next step would be to compile out the retarget code if minimal-printf is used.

My issue is that this conflates two independent things - "minimal printf" and "simplified stdin/out retargetting", and attempts to short-circuit the retargetting in printf itself, without actually removing it.

This PR fails with a very simple test program:

int main() {
    printf("2+2=%d\n", 2+2);
    puts("done");
}

The minimal printf should not be trying to work around the retargetting - it should just use fputc.

If you want to simplify the retargetting, do it properly in a separate PR - that could make it so that fputc() always wrote to the serial port.

And the two options - simplified printf and simplified retargetting should be totally independent of each other. There's no reason to link them.

Marcus Chang and others added 30 commits October 14, 2017 20:55

Initial commit

0284019

Minimal printf

9a21fe5

Supports %d %u %X %p %s Doesn't use malloc

Update README.md

45afbf6

Add mbed initialization

15e566f

Fix compile warnings

63d33c6

Update README.md

a70a533

Support for floating points (disabled by default)

4fd19a0

Use mbed_app.json to enable floating points. Automatically ignore flags and width specifiers.

Add variable list versions of printf and snprintf

2206ae5

This makes it possible to use the minimal printf and snprintf through mbed_retarget.cpp

Add support for width specifiers

3be1165

Add test case

3da2057

Fixed bug in getting absolute value

8ec1d47

Merge pull request ARMmbed#4 from ARMmbed/width

809c7ec

Add support for width specifiers

Fix file permissions

ab700c0

Update README.md

dddce56

Add compile profiles

cad10ad

Merge pull request ARMmbed#5 from ARMmbed/add_profiles

48d35c4

Add compile profiles

Add "-g" to 'release' and 'develop' profiles

2e988a3

This is used by mbed-os-linker-report and doesn't increase the code size.

Merge pull request ARMmbed#6 from ARMmbed/add_debug_info

d805e84

Add "-g" to 'release' and 'develop' profiles

Remove references to ssize_t in the code

75845c3

ssize_t doesn't exist at all in armcc.

Merge pull request ARMmbed#7 from ARMmbed/remove_ssize_t

8a2d3a0

Remove references to ssize_t in the code

Add missing include stdbool.h

de7024f

[BUGFIX][IOTUC-18] Library fixes

a5f0716

This commit adds mostly integer (and buffer) overflow checks for the current buffer index (`result` variable).

Address code review issues

2a01a54

- Improved comments to explain the checks on 'result'. - Check for non-NULL format specifier.

Merge pull request ARMmbed#10 from ARMmbed/fixes

66d5739

[BUGFIX][IOTUC-18] Library fixes

TEST_ASSERT_EQUAL_STRING always tests against the base version

541455d

Merge pull request ARMmbed#11 from ARMmbed/fix_mbed_tests

1b20647

Implementation and test fixes

Optional SWO output instead of UART

cbdb3fe

Change the default output from STDIO UART to SWO by overriding: "minimal-printf.console-output": "SWO"

evedon removed the needs: work label Aug 1, 2019

bulislaw approved these changes Aug 1, 2019

View reviewed changes

evedon added the release-version: 5.14.0-rc1 label Aug 1, 2019

Fixed greentea header include path

896500b

evedon added needs: CI and removed needs: review labels Aug 2, 2019

evedon added ready for merge and removed needs: CI labels Aug 2, 2019

hugueskamba reviewed Aug 5, 2019

View reviewed changes

TESTS/mbed_platform/minimal-printf/compliance/README.md Show resolved Hide resolved

platform/source/minimal-printf/mbed_printf_armlink_overrides.c Show resolved Hide resolved

tools/profiles/minimal-printf.json Show resolved Hide resolved

Removed no-builtin-printf for uARM

d6b197a

hugueskamba approved these changes Aug 6, 2019

View reviewed changes

0xc0170 approved these changes Aug 9, 2019

View reviewed changes

0xc0170 merged commit fafd0a5 into ARMmbed:master Aug 9, 2019

0xc0170 changed the title ~~Minimal printf~~ Add minimal printf Aug 9, 2019

0xc0170 removed the ready for merge label Aug 9, 2019

0xc0170 mentioned this pull request Aug 13, 2019

gnuarmeclipse: cxxsupport failures for few targets on master #11209

Closed

evedon mentioned this pull request Aug 14, 2019

Moved partial profile to a subdirectory #11213

Merged

evedon deleted the minimal-printf branch August 15, 2019 08:41

evedon mentioned this pull request Aug 15, 2019

Minimal-printf should not bypass the retargetting code #11234

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add minimal printf #11051

Add minimal printf #11051

Uh oh!

evedon commented Jul 15, 2019 •

edited

Loading

Uh oh!

evedon commented Aug 1, 2019

Uh oh!

evedon commented Aug 1, 2019

Uh oh!

mbed-ci commented Aug 1, 2019

Uh oh!

mbed-ci commented Aug 1, 2019

Uh oh!

mbed-ci commented Aug 2, 2019

Uh oh!

evedon commented Aug 2, 2019

Uh oh!

evedon commented Aug 2, 2019

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mbed-ci commented Aug 5, 2019

Uh oh!

hugueskamba left a comment

Uh oh!

evedon commented Aug 6, 2019

Uh oh!

evedon commented Aug 7, 2019

Uh oh!

kjbracey commented Aug 12, 2019

Uh oh!

Uh oh!

Add minimal printf #11051

Add minimal printf #11051

Uh oh!

Conversation

evedon commented Jul 15, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Pull request type

Reviewers

Release Notes

Uh oh!

evedon commented Aug 1, 2019

Uh oh!

evedon commented Aug 1, 2019

Uh oh!

mbed-ci commented Aug 1, 2019

Test run: FAILED

Uh oh!

mbed-ci commented Aug 1, 2019

Test run: FAILED

Uh oh!

mbed-ci commented Aug 2, 2019

Test run: SUCCESS

Uh oh!

evedon commented Aug 2, 2019

Uh oh!

evedon commented Aug 2, 2019

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mbed-ci commented Aug 5, 2019

Test run: SUCCESS

Uh oh!

hugueskamba left a comment

Choose a reason for hiding this comment

Uh oh!

evedon commented Aug 6, 2019

Uh oh!

evedon commented Aug 7, 2019

Uh oh!

kjbracey commented Aug 12, 2019

Uh oh!

Uh oh!

evedon commented Jul 15, 2019 •

edited

Loading