Skip to content

riscv: fix tests/pkg_wolfssl and tests/lwip_gnrc_udp when building with default options [backport 2019.10]#12524

Merged
miri64 merged 2 commits intoRIOT-OS:2019.10-branchfrom
aabadie:backport/2019.10/pr/riscv_fix_build
Oct 21, 2019
Merged

riscv: fix tests/pkg_wolfssl and tests/lwip_gnrc_udp when building with default options [backport 2019.10]#12524
miri64 merged 2 commits intoRIOT-OS:2019.10-branchfrom
aabadie:backport/2019.10/pr/riscv_fix_build

Conversation

@aabadie
Copy link
Copy Markdown
Contributor

@aabadie aabadie commented Oct 21, 2019

Backport of #12502

Contribution description

This PR is an attempt to get a working build on hifive1/hifive1b boards with tests/pkg_wolfssl and tests/lwip_gnrc_udp applications. The failures were reported in RIOT-OS/Release-Specs#140 (comment) and RIOT-OS/Release-Specs#140 (comment).

For the wolfssl test application, removing the optimization flash from the debug flags of the toolchain fixed the issue.
For the lwip_gnrc_udp, I'm less sure of the fix because I have to admit I don't really understand the error message. Disabling any GCC optimizations on the problematic functions fix the build issue. But maybe there's an option that would be better ? Any better suggestion is welcome (I also set the RFC label because of this)

Since this PR is touching the RISV toolchain options, it may have other impacts I didn't think of.

Testing procedure

At least the 2 related applications should still work. But better test all applications on hifive1. (the fixes are more localized now /@miri64)

Issues/PRs references

Reported in RIOT-OS/Release-Specs#140

@aabadie aabadie added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR CI: run tests If set, CI server will run tests on hardware for the labeled PR Platform: RISC-V Platform: This PR/issue effects RISC-V-based platforms Process: release backport Integration Process: The PR is a release backport of a change previously provided to master Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) labels Oct 21, 2019
@aabadie aabadie requested a review from miri64 October 21, 2019 08:39
@miri64
Copy link
Copy Markdown
Member

miri64 commented Oct 21, 2019

Needs rebase.

miri64
miri64 previously approved these changes Oct 21, 2019
Copy link
Copy Markdown
Member

@miri64 miri64 left a comment

Choose a reason for hiding this comment

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

Other than that: Changes are identical to #12502. ACK

aabadie and others added 2 commits October 21, 2019 11:28
Co-authored-by: Martine Lenders <m.lenders@fu-berlin.de>
(cherry picked from commit 300c3ab)
Only when building with riscv toolchain, because the default optimization used can lead to this problem.

(cherry picked from commit 02ff487)
@aabadie aabadie force-pushed the backport/2019.10/pr/riscv_fix_build branch from 57329f3 to 74e8275 Compare October 21, 2019 09:28
@aabadie
Copy link
Copy Markdown
Contributor Author

aabadie commented Oct 21, 2019

rebased

Copy link
Copy Markdown
Member

@miri64 miri64 left a comment

Choose a reason for hiding this comment

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

Still the same. Re-ACK.

@miri64 miri64 merged commit 24398b8 into RIOT-OS:2019.10-branch Oct 21, 2019
@aabadie aabadie deleted the backport/2019.10/pr/riscv_fix_build branch October 22, 2019 11:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR CI: run tests If set, CI server will run tests on hardware for the labeled PR Platform: RISC-V Platform: This PR/issue effects RISC-V-based platforms Process: release backport Integration Process: The PR is a release backport of a change previously provided to master Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants