Skip to content

Enable unix build for current versions of clang (OSX)#875

Closed
blmorris wants to merge 1 commit intomicropython:masterfrom
blmorris:fix-clang-error
Closed

Enable unix build for current versions of clang (OSX)#875
blmorris wants to merge 1 commit intomicropython:masterfrom
blmorris:fix-clang-error

Conversation

@blmorris
Copy link
Copy Markdown
Contributor

When the unix port is compiled for OSX, the build scripts employ several tricks to make sure that the __bss_start and _end symbols needed by gc are in the right place; the reason is that clang for OSX does not support certain directives which make this more straightforward on gcc.
These tricks seem to work; micropython can be built for OSX with certain older versions of clang.
However, line 118 in py/nlrx64.S contains the .bss directive that triggers a warning in clang version 3.1 (my version) and throws an error in version 4.2 (reported on issue #865)
The warning states 'ignoring directive for now', but apparently it is ignored forever; micropython can be built with this directive enabled or commented out, either way produces identical executables and map files, and the executable passes all tests.
The strange part is that the lines immediately before and after the offending directive are explicitly skipped for OSX, but this line seems to have been intentionally left in.
My question- is this just an oversight? Or is this .bss directive still supposed to do something important that is not getting exercised in the test suite? Also, why does line 118 trigger an error but not line 182? Does that one just get skipped due to logic which I haven't grasped yet? Should that directive also be skipped when building for OSX, just for consistency?

Error on clang version 4.2:

CC ../py/nlrx64.S
clang -I. -I../py -Ibuild -Wall -Werror -ansi -std=gnu99 -DUNIX -DMICROPY_USE_READLINE=1 -DMICROPY_PY_TIME=1 -DMICROPY_PY_TERMIOS=1
-I/opt/local/lib/libffi-3.1/include  -DMICROPY_PY_FFI=1 -Os   -c -o
build/py/nlrx64.o ../py/nlrx64.S
../py/nlrx64.S:118:5: error: unknown directive
    .bss
    ^
make: *** [build/py/nlrx64.o] Error 1

Warning on clang version 3.1:

CC ../py/nlrx64.S
clang -I. -I../py -Ibuild -Wall -Werror -ansi -std=gnu99 -DUNIX -DMICROPY_USE_READLINE=1 -DMICROPY_PY_TIME=1 -DMICROPY_PY_TERMIOS=1 
-I/opt/local/lib/libffi-3.1/include  -DMICROPY_PY_FFI=1 -Os   -c -o 
build/py/nlrx64.o ../py/nlrx64.S
../py/nlrx64.S:118:5: warning: ignoring directive for now
    .bss
(...build process continues after warning...)

…hich triggers warnings or errors in different versions of clang
dpgeorge added a commit that referenced this pull request Sep 26, 2014
It seems that newer versions of clang don't like the .bss directive, so
we don't use it for OSX.

Addressing issues #865 and #875.
@dpgeorge
Copy link
Copy Markdown
Member

@blmorris thanks for working this out.

The strange part is that the lines immediately before and after the offending directive are explicitly skipped for OSX, but this line seems to have been intentionally left in.

The macros are separate because they belong to conceptually different parts of the file: the first, to the function nlr_jump, then second, to the local variable nlr_top. So it's best to keep these things separate.

Also, why does line 118 trigger an error but not line 182?

Line 182 is part of the Cygwin code and is not compiled for OSX.

@dpgeorge
Copy link
Copy Markdown
Member

In 133b083 I have cleaned up the code in the nlr*.S files to make it easier to read and understand. I also fixed the .bss issue that's the subject of this PR. uPy should now compile with different clang versions.

@dpgeorge dpgeorge closed this Sep 26, 2014
@blmorris
Copy link
Copy Markdown
Contributor Author

Thanks for the explanations; makes sense now. I just tested, unix and stmhal do build for me with the new code.

@blmorris blmorris deleted the fix-clang-error branch September 26, 2014 14:55
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.

2 participants