pkg: disable compiler warning for jerryscript and nimble#10503
pkg: disable compiler warning for jerryscript and nimble#10503kaspar030 merged 2 commits intoRIOT-OS:masterfrom
Conversation
smlng
added
Type: bug
smlng
force-pushed
the
pr/pkg/jerryscript_wconversion
branch
2 times, most recently
from
d3b0a2a to
1d0384c
Compare
smlng
changed the title
|
Please note: there might be better ways solving this but as a quick fix this should do to make Murdock happy and get other PRs tested and merged again. |
| # tell LLVM/clang not to be so pedantic with this. | ||
| CFLAGS += -Wno-unused-function | ||
| CFLAGS += -Wno-sometimes-uninitialized | ||
| CFLAGS += -Wno-address-of-packed-member |
There was a problem hiding this comment.
This should actually not be deactivated, but fixed as this can lead to unexpected crashes. I'd rather deactivate building nrf52dk with LLVM (edit: as a quickfix) for now than deactivating this warning.
There was a problem hiding this comment.
well example/nimble_gatt is only build for the nrf52dk with llvm anyway, so the effect of the quick fix is the same. See here https://ci.riot-os.org/RIOT-OS/RIOT/10484/262a04c9cfb390f7ae2aead657a2b62a8a6f7e16/output.html and search for nimble_gatt.
There was a problem hiding this comment.
to that end I would rather disable this warning and get the example build with llvm at least once, blacklisting the nrf52dk as you suggested would lead to not compiling this with llvm at all
There was a problem hiding this comment.
I know, but getting an application built is not the task of a CI builder. It is testing the correct build of an application. As such, even as a quick fix I don't except building with such a critical warning disabled, so that at least in a local environment people might get warned. The right way is to temporarily "drop" support and then resupport once the bug is fixed.
There was a problem hiding this comment.
But maybe someone else has differing opinions, so I'm not blocking because of this, but I won't ACK neither.
There was a problem hiding this comment.
mhm, agree to disagree here.
The bug was only "found" because we updated the CI workers from xenial to bionic yesterday, so it was there all the time, just that the newer LLVM is a bit more strict. So its either dropping the board (and with that llvm) completely or dropping that warning and have llvm compile that thing at least once. And once fixed, re-arm the warning.
I just want to get Murdock "working" again ASAP, to get other (real) PRs merged ...
There was a problem hiding this comment.
the board (and with that llvm) completely
Not true. You can disable just nrf52dk. Yes, we drop LLVM support with that for that particular board, but not for all boards (we don't build with LLVM for all boards anyway).
There was a problem hiding this comment.
The bug is also present in the gcc build. gcc just doesn't warn.
It also doesn't matter on the nrf52dk, as Cortex M4 supports unaligned accesses (they're just a tad slower). I've checked the code, there are only unaligned accesses when some debug mode is activated, the regular case ends up using memcpy.
Thus I think enabling disabling the warning is fine...
|
Well I guess now one of you is responsible for actually fixing the warnings, so yes of course :-) |
Yeah, one of the downsides of maintaining the CI... :) @smlng pls squash! |
smlng
force-pushed
the
pr/pkg/jerryscript_wconversion
branch
from
591c620 to
47a6626
Compare
|
done |
The lazy way would have been to roll back Docker ;-) |
True, and I really thought about it. 😉 Thing is, docker hub builds soooo slow, I was sure that wouldn't have been the fastest solution... |
|
So all green ... |
|
&go. |
Nimble contains a couple of casts that discard alignment information. This causes a warning with clang's -Wno-address-of-packed-member. A previous PR (RIOT-OS#10503) supressed that warning. This commit re-enables them and provides a patch to fix the offending code. The fix has been submitted upstream, see apache/mynewt-nimble#252
|
I provided a slowfix in #10511. |
Nimble contains a couple of casts that discard alignment information. This causes a warning with clang's -Wno-address-of-packed-member. A previous PR (RIOT-OS#10503) supressed that warning. This commit re-enables them and provides a patch to fix the offending code. The fix has been submitted upstream, see apache/mynewt-nimble#252
Nimble contains a couple of casts that discard alignment information. This causes a warning with clang's -Wno-address-of-packed-member. A previous PR (RIOT-OS#10503) supressed that warning. This commit re-enables them and provides a patch to fix the offending code. The fix has been submitted upstream, see apache/mynewt-nimble#252
5 participants
Contribution description
This is (hopefully) a quick fix to make Murdock build jerryscript with LLVM again, since the update of the Murdock-workers to Ubuntu-Bionic all builds fail.
Added a second quick fix for the nimble package
Testing procedure
Issues/PRs references