Skip to content

Fix array vs pointer error in declaration of circuitpython_help_text#686

Merged
tannewt merged 1 commit into
adafruit:masterfrom
jepler:lto-type-diagnostic
Mar 20, 2018
Merged

Fix array vs pointer error in declaration of circuitpython_help_text#686
tannewt merged 1 commit into
adafruit:masterfrom
jepler:lto-type-diagnostic

Conversation

@jepler
Copy link
Copy Markdown

@jepler jepler commented Mar 20, 2018

.. removing the need for -Wno-error=lto-type-mismatch, which is
not available in gcc 5.4.1 (debian stretch) while building the atmel-samd port.

The specific error that occurred was:

../../py/builtin.h:121:19: error: type of 'circuitpython_help_text' does not match original declaration [-Werror]
 extern const char MICROPY_PY_BUILTINS_HELP_TEXT[];
                   ^
../../shared-bindings/help.c:38:13: note: previously declared here
 const char *circuitpython_help_text =
             ^
lto1: all warnings being treated as errors
lto-wrapper: fatal error: /usr/bin/arm-none-eabi-gcc returned 1 exit status
compilation terminated.
/usr/lib/gcc/arm-none-eabi/5.4.1/../../../arm-none-eabi/bin/ld: error: lto-wrapper failed
collect2: error: ld returned 1 exit status

@dhalbert
Copy link
Copy Markdown
Collaborator

@jepler We build with gcc 7.3.1 from https://developer.arm.com/open-source/gnu-toolchain/gnu-rm/downloads (7-2017-q4-major) or https://launchpad.net/~team-gcc-arm-embedded/+archive/ubuntu/ppa (if using Ubuntu). I'd recommend that you download that toolchain and put it in user space or use Ubuntu.

We need the -Wno-error=lto-type-mismatch to fix an issue when building frozen modules with gcc 7.3.1. This issue comes up when doing the BOARD=circuit_playground_express. You can see the issue in the travis builds for that board. So unfortunately this PR will not work for us.

@jepler
Copy link
Copy Markdown
Author

jepler commented Mar 20, 2018

Thanks, I see that failure on travis-ci now. Oddly, I don't replicate the mp_qstr_const_pool problem here, with make -C ports/atmel-samd/ BOARD=circuitplayground_express.

Do you think there's any point in revising this pull request? If not, please feel free to close it up.

Do you think there's any point in opening a documentation pull request? I do not find these toolchain requirements listed in the docs, even after grepping for the URLs you cited above. (some of the other ports/*/README.md do mention specific gcc requirements, but not ports/atmel-samd/README.md)

@jepler
Copy link
Copy Markdown
Author

jepler commented Mar 20, 2018

For instance, a revised patch might use the Linux kernel trick for checking whether a particular argument is accepted by the compiler or not, similar to

cc-option = $(shell if $(CC) $(CFLAGS) $(1) -S -o /dev/null -xc /dev/null \
             > /dev/null 2>&1; then echo "$(1)"; else echo "$(2)"; fi ;)
...
CFLAGS += $(call cc-option,-mieee-fp)

@dhalbert
Copy link
Copy Markdown
Collaborator

I agree we should list the toolchain requirements. That is an oversight: thanks for pointing that out. A PR to document that would be great, if you are willing, or we can just do it in a future PR of ours.

I submitted a gcc bug report for the bogus LTO mismatch warning. It is now fixed in gcc, but will not show up for a while: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81440.

You may also run into other issues with the older gcc, such as code size. Right now we have a few kbytes free on the non-Express builds, but we may try squeezing more stuff in later. At one time we only had a few tens or hundreds of bytes free, so the better optimization on the newer compilers was very helpful.

Building with gcc 5.4.1 (Debian Stretch) with the unsupported
-Wno-error=lto-type-mismatch flag removed, the following diagnostic
occurs:

../../py/builtin.h:121:19: error: type of 'circuitpython_help_text' does not match original declaration [-Werror]
 extern const char MICROPY_PY_BUILTINS_HELP_TEXT[];
                   ^
../../shared-bindings/help.c:38:13: note: previously declared here
 const char *circuitpython_help_text =
             ^
lto1: all warnings being treated as errors
lto-wrapper: fatal error: /usr/bin/arm-none-eabi-gcc returned 1 exit status
@jepler jepler force-pushed the lto-type-diagnostic branch from 15db2ee to 002797a Compare March 20, 2018 03:11
@jepler
Copy link
Copy Markdown
Author

jepler commented Mar 20, 2018

Thanks for the pointer to the upstream bug.

I've revised THIS pull request so that it only changes the declaration, and does NOT modify the compiler flags.

Copy link
Copy Markdown
Member

@tannewt tannewt left a comment

Choose a reason for hiding this comment

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

Thanks for the tweak! Glad to see more people building CircuitPython locally.

@tannewt tannewt merged commit 286e342 into adafruit:master Mar 20, 2018
@jepler jepler deleted the lto-type-diagnostic branch March 23, 2018 01:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants