Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

shell/sc_can: fix uninitialized warning #9627

Merged
merged 1 commit into from
Jul 27, 2018

Conversation

cladmi
Copy link
Contributor

@cladmi cladmi commented Jul 24, 2018

Contribution description

When compiled for hifive1 board with gcc-7.2.0 this warning was raised:

'ret' may be used uninitialized in this function [-Werror=maybe-uninitialized]

This was first mentioned in RIOT-OS/riotdocker#42 (comment)

Testing

The board does not have enough memory but the error is triggered when compiling with a gcc version 7.2.0:

make -C tests/conn_can/ BOARD=hifive1 RIOT_CI_BUILD=1

Warning is dismissed, it was just a testing procedure error.

Warning

Currently when compiling with riscv-none-embed-gcc (GNU MCU Eclipse RISC-V Embedded GCC, 64-bits) 7.2.0 a linker error is raised

make -C tests/conn_can/ BOARD=hifive1
...
/opt/gnu-mcu-eclipse/riscv-none-gcc/7.2.0-2-20180111-2230/bin/../lib/gcc/riscv-none-embed/7.2.0/../../../../riscv-none-embed/bin/ld: section .stack VMA [0000000080003c00,0000000080003fff] overlaps section .bss VMA [00000000800001f8,0000000080003eaf]
collect2: error: ld returned 1 exit status
/data/riotbuild/riotbase/Makefile.include:356: recipe for target '/data/riotbuild/riotproject/tests/conn_can/bin/hifive1/tests_conn_can.elf' failed
make: *** [/data/riotbuild/riotproject/tests/conn_can/bin/hifive1/tests_conn_can.elf] Error 1
/home/user/Desktop/RIOT_master_old/makefiles/docker.inc.mk:90: recipe for target '..in-docker-container' failed
make: *** [..in-docker-container] Error 2

The goal of this PR is to resolve the linking bug and merge this.

Issues/PRs references

#9405
RIOT-OS/riotdocker#42
Found it during release testing

When compiled for `hifive1` board with `gcc-7.2.0` this warning was raised:

'ret' may be used uninitialized in this function [-Werror=maybe-uninitialized]
@cladmi cladmi added Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation Platform: RISC-V Platform: This PR/issue effects RISC-V-based platforms labels Jul 24, 2018
@cladmi cladmi requested a review from ZetaR60 July 24, 2018 12:03
@cladmi
Copy link
Contributor Author

cladmi commented Jul 24, 2018

@kenrabold Could you look if you have the same issue with linking in this PR ?

I raise the warning when doing make -C tests/conn_can/ BOARD=hifive1

@cladmi cladmi added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR State: waiting for other PR State: The PR requires another PR to be merged first labels Jul 24, 2018
@cladmi
Copy link
Contributor Author

cladmi commented Jul 24, 2018

I add a waiting for other PR because it needs a fix for the linking error before being merged.

@cladmi cladmi added the Area: toolchain Area: toolchains; everything related to compilation, libc, linking, … label Jul 24, 2018
@kenrabold
Copy link
Contributor

The link failure is from insufficient memory on the hifive1 board to hold the program image and RAM sections. The hifive1 board is already in the BOARD_INSUFFICIENT_MEMORY list for the /tests/conn_can Makefile, so this test app should never be compiled for hifive1.

@cladmi
Copy link
Contributor Author

cladmi commented Jul 25, 2018

@kenrabold Thank you for your quick feedback, and sorry for not paying enough attention.

The message was different from what I see in arm boards arm-none-eabi/bin/ld: region 'ram' overflowed by XXXbytes so I thought it could was something else.

Also I was not being careful enough when testing.
I had an error when doing buildtest, that only builds without linking, and after fixing the warning that is here, I tested with make all instead of buildtest or make BOARD=hifive1 RIOT_CI_BUILD=1 which tries to link and does not consider BOARD_INSUFFICIENT_MEMORY, which mislead me.

So in fact no problems with hifive1 board, I am relieved.

@cladmi cladmi added Impact: minor The PR is small in size and might only require a quick look of a knowledgeable reviewer 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 Area: toolchain Area: toolchains; everything related to compilation, libc, linking, … and removed State: waiting for other PR State: The PR requires another PR to be merged first CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Area: toolchain Area: toolchains; everything related to compilation, libc, linking, … Platform: RISC-V Platform: This PR/issue effects RISC-V-based platforms labels Jul 25, 2018
Copy link
Contributor

@ZetaR60 ZetaR60 left a comment

Choose a reason for hiding this comment

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

ACK. I made the same change in #9507 to get hifive1 working properly, but it is probably better as a separate PR. I have done build tests with this change in #9507.

@cladmi cladmi merged commit f996e1c into RIOT-OS:master Jul 27, 2018
@cladmi
Copy link
Contributor Author

cladmi commented Jul 27, 2018

@ZetaR60 Yes I like fixing what is possible in sub-PRs as it helps getting merged early. Also it has been problematic for me in the release tests.

@cladmi
Copy link
Contributor Author

cladmi commented Jul 27, 2018

Backport provided in #9644

@cladmi cladmi deleted the pr/sc_can/uninitialized branch July 27, 2018 14:34
@cladmi cladmi added this to the Release 2018.07 milestone Aug 11, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: toolchain Area: toolchains; everything related to compilation, libc, linking, … CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Impact: minor The PR is small in size and might only require a quick look of a knowledgeable reviewer Process: needs backport Integration Process: The PR is required to be backported to a release or feature branch Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants