@gebart Can you confirm that this cannot break stuff?

@LudwigKnuepfer Well, it took 5 seconds. ;)

Does this change the resulting binary in any way on a recent (non broken) compiler? (my guess is: no)
Anyway, untested ACK

Does this change the resulting binary in any way on a recent (non broken) compiler?

MD5 differs (arm-none-eabi-gcc (GNU Tools for ARM Embedded Processors) 5.4.1 20160609 (release) [ARM/embedded-5-branch revision 237715], RIOT_VERSION_OVERRIDE set) for examples/default on for at least BOARD=stm32f4discovery.

I looked over the disassembly, using gcc-5.4 this change introduced two new mov instructions to set some registers to zero for initializing the sp and pc variables. The total binary size was not affected though, probably because of alignment.
Also from inspecting the disassembly we can draw the conclusion that the warning is a false positive caused by GCC not keeping track of the fact that the register containing the corrupted variable is not modified between the two if blocks.

So wontfix?

I don't mind either way. The few single bytes of flash that this adds is insignificant on most cortex m mcu. What is your opinion, Ludwig?
If we don't merge this then we should add a comment near the code saying to manually set WERROR=0 when building with the affected compiler version.

If we don't merge this then we should add a comment near the code saying to manually set WERROR=0 when building with the affected compiler version.

We could, though it's gonna look ugly, surround the affected line with push/disable warning/pop pragmas...

I was thinking of a text comment meant for the user to see when they open
the source after getting a compile failure, not any pragmas or anything
like that.

Yeah, good idea. Let's do that.

Which GCC are we talking about? Is a standard compiler from a stable release of a major distribution affected?

@LudwigKnuepfer I encountered this with debian. (don't know wich gcc version, I'm not at home)

@mali Are you referring to debian stable? old-stable?

@LudwigKnuepfer
arm-none-eabi-gcc 4.8.4 on debian Jessie 8.5 (stable)

root@paddy:~# apt-cache policy gcc-arm-none-eabi
gcc-arm-none-eabi:
Installé : 4.8.4-1+11-1
Candidat : 4.8.4-1+11-1
http://ftp.fr.debian.org/debian/ jessie/main amd64 Packages

laurent@paddy:~$ arm-none-eabi-gcc -v
Using built-in specs.
COLLECT_GCC=arm-none-eabi-gcc
COLLECT_LTO_WRAPPER=/usr/lib/gcc/arm-none-eabi/4.8/lto-wrapper
Target: arm-none-eabi
Configured with: ../gcc-4.8.4/configure --build=x86_64-linux-gnu --prefix=/usr --includedir='/usr/lib/include' --mandir='/usr/lib/share/man' --infodir='/usr/lib/share/info' --sysconfdir=/etc --localstatedir=/var --disable-silent-rules --libexecdir='/usr/lib/lib/gcc-arm-none-eabi' --disable-maintainer-mode --disable-dependency-tracking --enable-languages=c,c++ --prefix=/usr/lib --infodir=/usr/share/doc/gcc-arm-none-eabi/info --mandir=/usr/share/man --htmldir=/usr/share/doc/gcc-arm-none-eabi/html --pdfdir=/usr/share/doc/gcc-arm-none-eabi/pdf --bindir=/usr/bin --libexecdir=/usr/lib --libdir=/usr/lib --with-system-zlib --enable-multilib --disable-decimal-float --disable-libffi --disable-libgomp --disable-libmudflap --disable-libquadmath --disable-libssp --disable-libstdcxx-pch --disable-libstdc++-v3 --disable-nls --disable-shared --disable-threads --disable-tls --build=x86_64-linux-gnu --host=x86_64-linux-gnu --target=arm-none-eabi --with-gnu-as --with-gnu-ld --with-headers=no --without-newlib --with-pkgversion=4.8.4-1+11-1 --without-included-gettext --with-multilib-list=armv6-m,armv7-m,armv7e-m,armv7-r INHIBIT_LIBC_CFLAGS=-DUSE_TM_CLONE_REGISTRY=0 AR_FOR_TARGET=arm-none-eabi-ar AS_FOR_TARGET=arm-none-eabi-as LD_FOR_TARGET=arm-none-eabi-ld NM_FOR_TARGET=arm-none-eabi-nm OBJDUMP_FOR_TARGET=arm-none-eabi-objdump RANLIB_FOR_TARGET=arm-none-eabi-ranlib READELF_FOR_TARGET=arm-none-eabi-readelf STRIP_FOR_TARGET=arm-none-eabi-strip
Thread model: single
gcc version 4.8.4 20141219 (release) (4.8.4-1+11-1)

OK, so both ubuntu lts and debian stable will keep this problem around until at least 2018. I'd tend to not force these users to pass compile flags manually.

Regarding the comment of @gebart (#5727 (comment)) this PR adds 2 mov instructions in the assembly to reset the affected variables and prevent the false positive.
Like @LudwigKnuepfer I would prefer to not advise a user to turn off compiler flags manually.

I would propose to keep the initialization and add a statement in the code why it has been introduced.

I agree with your proposal 👍

On Oct 4, 2016 1:50 PM, "BytesGalore" [email protected] wrote:

Regarding the comment of @gebart https://github.com/gebart (#5727
(comment)
#5727 (comment)) this
PR adds 2 mov instructions in the assembly to reset the affected
variables and prevent the false positive.
Like @LudwigKnuepfer https://github.com/LudwigKnuepfer would prefer to
not advise a user to turn off compiler flags manually.

I would propose to keep the initialization and add a statement in the code
why it has been introduced.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#5727 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AATYQurMiUeYuJOu2oUIGDKk_xpz4DGRks5qwj1hgaJpZM4JddGt
.

@kaspar030: would you mind to add the comment as proposed by @BytesGalore? Then we can merge this finally...

• done, immediately squashed

ping