Skip to content

pkg/cn-cbor: fix unaligned access#21880

Merged
crasbe merged 1 commit intoRIOT-OS:masterfrom
maribu:pkg/cn-cbor/fix-failing-test
Nov 16, 2025
Merged

pkg/cn-cbor: fix unaligned access#21880
crasbe merged 1 commit intoRIOT-OS:masterfrom
maribu:pkg/cn-cbor/fix-failing-test

Conversation

@maribu
Copy link
Copy Markdown
Member

@maribu maribu commented Nov 15, 2025 •
edited
Loading

Contribution description

With GCC 15.2.0, the self test of cn-cbor fails. Using safe unaligned access and standard endian conversions fixes the issue.

Testing procedure

In master

git:(master) ~/Repos/software/RIOT/master ➜ make BOARD=nrf52840dk -C tests/pkg/cn-cbor flash test
make: Entering directory '/home/maribu/Repos/software/RIOT/master/tests/pkg/cn-cbor'
Building application "tests_cn-cbor" for "nrf52840dk" with CPU "nrf52".
[...]
   text	   data	    bss	    dec	    hex	filename
  17696	    132	   3076	  20904	   51a8	/home/maribu/Repos/software/RIOT/master/tests/pkg/cn-cbor/bin/nrf52840dk/tests_cn-cbor.elf
[...]
READY
s
START
main(): This is RIOT! (Version: 2026.01-devel-120-g490b5)
.
tests_cn_cbor.test_parse (tests/pkg/cn-cbor/main.c 150) exp 63 was 0
.
run 2 failures 1
{ "threads": [{ "name": "main", "stack_size": 1536, "stack_used": 768}]}

This PR

git:(pkg/cn-cbor/fix-failing-test) ~/Repos/software/RIOT/master ➜ make BOARD=nrf52840dk -C tests/pkg/cn-cbor flash test
[...]
   text	   data	    bss	    dec	    hex	filename
  17728	    132	   3076	  20936	   51c8	/home/maribu/Repos/software/RIOT/master/tests/pkg/cn-cbor/bin/nrf52840dk/tests_cn-cbor.elf
[...]
READY
s
START
main(): This is RIOT! (Version: 2026.01-devel-121-g6510c62-pkg/cn-cbor/fix-failing-test)
..
OK (2 tests)

Note

It seems that our build system does not properly update the checked out git repo when the custom patches change. As a result $(BUILD_DIR)/pkg/cn-cbor needs to be removed by hand between builds to have correct test results.

Issues/PRs references

None

@maribu maribu added Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Process: needs backport Integration Process: The PR is required to be backported to a release or feature branch labels Nov 15, 2025
@github-actions github-actions bot added the Area: pkg Area: External package ports label Nov 15, 2025
@maribu
pkg/cn-cbor: fix unaligned access
b44aea3 
With GCC 15.2.0, the self test of cn-cbor fails. Using safe unaligned
access and standard endian conversions fixes the issue.
@maribu maribu force-pushed the pkg/cn-cbor/fix-failing-test branch from 6510c62 to b44aea3 Compare November 15, 2025 23:56
@maribu
Copy link
Copy Markdown
Member Author

maribu commented Nov 15, 2025

I just realized with the code now containing no alignment errors, we don't need to add -Wno-cast-align anymore to get it compiled anyway. I squashed it right in.

@riot-ci
Copy link
Copy Markdown

riot-ci commented Nov 16, 2025

Murdock results

✔️ PASSED

b44aea3 pkg/cn-cbor: fix unaligned access

Success Failures Total Runtime
10929 0 10930 11m:43s

Artifacts

@crasbe
Copy link
Copy Markdown
Contributor

crasbe commented Nov 16, 2025

It seems that our build system does not properly update the checked out git repo when the custom patches change. As a result $(BUILD_DIR)/pkg/cn-cbor needs to be removed by hand between builds to have correct test results.

You have to run make distclean -j to clean the build dir too.

crasbe
crasbe reviewed Nov 16, 2025
View reviewed changes
pkg/cn-cbor/patches/0001-Use-unaligned-access-and-endian-helpers.patch
- ret |= hton32p(p+4);
- return ret;
-}
+#define hton16p(p) (htobe16(unaligned_get_u16(p)))
Copy link
Copy Markdown
Contributor

@crasbe crasbe Nov 16, 2025

Choose a reason for hiding this comment

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

Is htobe16 etc available on all the compilers we use?

https://www.gnu.org/software/gnulib/manual/html_node/htobe16.html

This function is missing on many platforms: glibc 2.8, macOS 14, FreeBSD 13.0, NetBSD 7.2, OpenBSD 5.5, Minix 3.1.8, AIX 7.3.1, HP-UX 11.31, Solaris 11.4, Cygwin 1.7.x, mingw, MSVC 14.

glibc 2.8 is from 2009, so it shoooould be good?

Copy link
Copy Markdown
Member Author

@maribu maribu Nov 16, 2025

Choose a reason for hiding this comment

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

We certainly have it: https://github.com/RIOT-OS/RIOT/blob/master/sys/include/endian.h

@crasbe crasbe added this pull request to the merge queue Nov 16, 2025
Merged via the queue into RIOT-OS:master with commit dcf4aa5 Nov 16, 2025
28 checks passed
@maribu
Copy link
Copy Markdown
Member Author

maribu commented Nov 16, 2025

Thx ❤️

@maribu maribu deleted the pkg/cn-cbor/fix-failing-test branch November 16, 2025 18:31
@maribu maribu mentioned this pull request Nov 16, 2025
Merged
@maribu
Copy link
Copy Markdown
Member Author

maribu commented Nov 16, 2025

Backport provided in #21881

@maribu maribu mentioned this pull request Nov 16, 2025
Open
37 tasks
@leandrolanzieri leandrolanzieri added this to the Release 2026.01 milestone Jan 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@crasbe crasbe crasbe approved these changes

Assignees

No one assigned

Labels

Area: pkg Area: External package ports CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Process: needs backport Integration Process: The PR is required to be backported to a release or feature branch Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)

Projects

None yet

Milestone

Release 2026.01

Development

Successfully merging this pull request may close these issues.

4 participants

@maribu @riot-ci @crasbe @leandrolanzieri