tests/lwip_sock_udp: Fix sizeof() for memcpy action (pointed out by riscv gcc).#14
Conversation
| if (nc->state == ND6_NO_ENTRY) { | ||
| nc->state = ND6_REACHABLE; | ||
| memcpy(&nc->next_hop_address, remote6, sizeof(ip6_addr_t)); | ||
| memcpy(&nc->next_hop_address, remote6, sizeof(remote6)); |
There was a problem hiding this comment.
Using
| memcpy(&nc->next_hop_address, remote6, sizeof(remote6)); | |
| memcpy(&nc->next_hop_address, remote6, sizeof(nc->next_hop_address)); |
Gave me yet another warning I did not quite understand, so I decided to do it this way. I think what's happening is that the compiler get's confused about the code path and so it thinks nc->next_hop_address could be an IPv4 address (which it can't as this code is IPv6 specific) and complains about the width because of that.
There was a problem hiding this comment.
What's the error message you are getting?
There was a problem hiding this comment.
Mhhh with how RIOT-OS#12502 is configured now I basically get the same that @fjmolinas reported for the sizeof(ip6_addr_t) before.
[mlenders@mk RIOT]<3 BUILD_IN_DOCKER=1 BOARD=hifive1 make -C tests/lwip_sock_udp/ -j8
make: Verzeichnis „/home/mlenders/Repositories/RIOT-OS/RIOT/tests/lwip_sock_udp“ wird betreten
Launching build container using image "riot/riotbuild:latest".
docker run --rm -t -u "$(id -u)" \
-v '/usr/share/zoneinfo/Europe/Berlin:/etc/localtime:ro' -v '/home/mlenders/Repositories/RIOT-OS/RIOT:/data/riotbuild/riotbase:delegated' -e 'RIOTBASE=/data/riotbuild/riotbase' -e 'CCACHE_BASEDIR=/data/riotbuild/riotbase' -e 'BUILD_DIR=/data/riotbuild/riotbase/build' -e 'RIOTPROJECT=/data/riotbuild/riotbase' -e 'RIOTCPU=/data/riotbuild/riotbase/cpu' -e 'RIOTBOARD=/data/riotbuild/riotbase/boards' -e 'RIOTMAKE=/data/riotbuild/riotbase/makefiles' \
-e 'BOARD=hifive1' \
-w '/data/riotbuild/riotbase/tests/lwip_sock_udp/' \
'riot/riotbuild:latest' make
Building application "tests_lwip_sock_udp" for "hifive1" with MCU "fe310".
[…]
/data/riotbuild/riotbase/tests/lwip_sock_udp/stack.c: In function '_prepare_send_checks':
/data/riotbuild/riotbase/tests/lwip_sock_udp/stack.c:216:13: error: 'memcpy' reading 20 bytes from a region of size 16 [-Werror=stringop-overflow=]
memcpy(&nc->next_hop_address, remote6, sizeof(nc->next_hop_address));
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
If I uncomment the CFLAGS_OPT line in makefiles/arch/riscv.inc.mk (as originally done in RIOT-OS#12502) I get:
/data/riotbuild/riotbase/tests/lwip_sock_udp/stack.c: In function '_prepare_send_checks':
/data/riotbuild/riotbase/tests/lwip_sock_udp/stack.c:216:13: error: 'memcpy' forming offset [17, 20] is out of the bounds [0, 16] of object 'remote6' with type 'uint8_t[16]' {aka 'unsigned char[16]'} [-Werror=array-bounds]
memcpy(&nc->next_hop_address, remote6, sizeof(nc->next_hop_address));
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/data/riotbuild/riotbase/tests/lwip_sock_udp/stack.c:193:13: note: 'remote6' declared here
uint8_t remote6[] = _TEST_ADDR6_REMOTE;
^~~~~~~
There was a problem hiding this comment.
So maybe I should restore the optimization flag to -Os? (because I honestly think it should be the default, for RIOT and constained devices, there's no good reason for using such an optimization flag in debug flags).
That would also allow to not having the ignore the maybe-uninitialized warning in a crypto library (wolfcrypt).
There was a problem hiding this comment.
So maybe I should restore the optimization flag to -Os? (because I honestly think it should be the default, for RIOT and constained devices, there's no good reason for using such an optimization flag in debug flags).
That would also allow to not having the ignore the maybe-uninitialized warning in a crypto library (wolfcrypt).
In general I agree with that, but it is unrelated to the fix at hand, and shouldn't be backported into the release in a knee-jerk-response. RIOT-OS#12502 is a response to an issue found during release testing in the first place, so it should be as least impactful as possible.
There was a problem hiding this comment.
I fully agree. Especially because I retested a build of RIOT-OS#12502 and both tests are building fine with it.
I thought that restoring the -Og optimization flag broke back tests/lwip_sock_udp...
What are waiting for with RIOT-OS#12502 then ?!
There was a problem hiding this comment.
What are waiting for with RIOT-OS#12502 then ?!
A review ;-).
There was a problem hiding this comment.
How about memcpy(&nc->next_hop_address.addr, remote6, sizeof(nc->next_hop_address.addr)); then?
|
Ok, I trust you since you know that code better :) |
This way it compiles on my machine with the docker image.