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

CONFIGURE: fix linking error in libcurl conftest for switch host #2716

Open
wants to merge 1 commit into
base: master
from

Conversation

@justinkb
Copy link

@justinkb justinkb commented Jan 4, 2021

CONFIGURE: fix linking error in libcurl conftest for switch host

When targeting Nintendo Switch, the libcurl conftest fails as follows:

                        #include <curl/curl.h>
                        int main(int argc, char *argv[]) {
                                int x;

                                curl_easy_setopt(NULL,CURLOPT_URL,NULL);
                                x=CURL_ERROR_SIZE;

                                x=CURLOPT_WRITEFUNCTION;
                                x=CURLOPT_WRITEDATA;

                                x=CURLOPT_ERRORBUFFER;
                                x=CURLOPT_STDERR;
                                x=CURLOPT_VERBOSE;

                                curl_version_info_data *data = curl_version_info(CURLVERSION_NOW);
                                if (data->features & CURL_VERSION_SSL)
                                        return 0;
                                return 1;
                        }

aarch64-none-elf-g++ -march=armv8-a+crc+crypto -mtune=cortex-a57 -mtp=soft -fPIC -ftls-model=local-exec -L/opt/devkitpro/portlibs/switch/lib -L/opt/devkitpro/libnx/lib -L/opt/devkitpro/libnx/lib -L/opt/devkitpro/portlibs/switch/lib -march=armv8-a+crc+crypto -mtune=cortex-a57 -mtp=soft -fPIC -ftls-model=local-exec -O2 -ffunction-sections -fdata-sections -D__SWITCH__ -I/opt/devkitpro/portlibs/switch/include -isystem/opt/devkitpro/libnx/include -U__STRICT_ANSI__ -W -Wno-unused-parameter -Wno-empty-body -fno-operator-names -std=c++11 -march=armv8-a -mtune=cortex-a57 -mtp=soft -fPIE -ftls-model=local-exec -ffunction-sections -fdata-sections -I/opt/devkitpro/libnx/include -I/opt/devkitpro/portlibs/switch/include -O3 -Wuninitialized ./scummvm-conf.cpp -o ./scummvm-conf.elf -DCURL_STATICLIB -I/opt/devkitpro/portlibs/switch/include -L/opt/devkitpro/portlibs/switch/lib -lcurl -lnx -lz -lnx
/opt/devkitpro/devkitA64/bin/../lib/gcc/aarch64-none-elf/10.2.0/../../../../aarch64-none-elf/bin/ld: /opt/devkitpro/libnx/lib/libnx.a(switch_crt0.o): in function `_start':
/home/fincs/pacman-packages/libnx/src/libnx-4.0.0/nx/source/runtime/switch_crt0.s:6: multiple definition of `_start'; /opt/devkitpro/devkitA64/bin/../lib/gcc/aarch64-none-elf/10.2.0/../../../../aarch64-none-elf/lib/pic/crt0.o:(.text+0x0): first defined here
/opt/devkitpro/devkitA64/bin/../lib/gcc/aarch64-none-elf/10.2.0/../../../../aarch64-none-elf/bin/ld: /opt/devkitpro/libnx/lib/libnx.a(switch_crt0.o): in function `__nx_mod0':
/home/fincs/pacman-packages/libnx/src/libnx-4.0.0/nx/source/runtime/switch_crt0.s:81: undefined reference to `_DYNAMIC'
/opt/devkitpro/devkitA64/bin/../lib/gcc/aarch64-none-elf/10.2.0/../../../../aarch64-none-elf/bin/ld: /home/fincs/pacman-packages/libnx/src/libnx-4.0.0/nx/source/runtime/switch_crt0.s:81: undefined reference to `__eh_frame_hdr_start'
/opt/devkitpro/devkitA64/bin/../lib/gcc/aarch64-none-elf/10.2.0/../../../../aarch64-none-elf/bin/ld: /home/fincs/pacman-packages/libnx/src/libnx-4.0.0/nx/source/runtime/switch_crt0.s:81: undefined reference to `__eh_frame_hdr_end'
/opt/devkitpro/devkitA64/bin/../lib/gcc/aarch64-none-elf/10.2.0/../../../../aarch64-none-elf/bin/ld: /home/fincs/pacman-packages/libnx/src/libnx-4.0.0/nx/source/runtime/switch_crt0.s:81: undefined reference to `__got_start__'
/opt/devkitpro/devkitA64/bin/../lib/gcc/aarch64-none-elf/10.2.0/../../../../aarch64-none-elf/bin/ld: /home/fincs/pacman-packages/libnx/src/libnx-4.0.0/nx/source/runtime/switch_crt0.s:81: undefined reference to `__got_end__'
/opt/devkitpro/devkitA64/bin/../lib/gcc/aarch64-none-elf/10.2.0/../../../../aarch64-none-elf/bin/ld: /opt/devkitpro/libnx/lib/libnx.a(newlib.o): in function `newlibSetup':
/home/fincs/pacman-packages/libnx/src/libnx-4.0.0/nx/source/runtime/newlib.c:435: undefined reference to `__tls_start'
/opt/devkitpro/devkitA64/bin/../lib/gcc/aarch64-none-elf/10.2.0/../../../../aarch64-none-elf/bin/ld: /home/fincs/pacman-packages/libnx/src/libnx-4.0.0/nx/source/runtime/newlib.c:435: undefined reference to `__tls_start'
/opt/devkitpro/devkitA64/bin/../lib/gcc/aarch64-none-elf/10.2.0/../../../../aarch64-none-elf/bin/ld: /home/fincs/pacman-packages/libnx/src/libnx-4.0.0/nx/source/runtime/newlib.c:438: undefined reference to `__tdata_lma_end'
/opt/devkitpro/devkitA64/bin/../lib/gcc/aarch64-none-elf/10.2.0/../../../../aarch64-none-elf/bin/ld: /home/fincs/pacman-packages/libnx/src/libnx-4.0.0/nx/source/runtime/newlib.c:438: undefined reference to `__tdata_lma'
/opt/devkitpro/devkitA64/bin/../lib/gcc/aarch64-none-elf/10.2.0/../../../../aarch64-none-elf/bin/ld: /home/fincs/pacman-packages/libnx/src/libnx-4.0.0/nx/source/runtime/newlib.c:438: undefined reference to `__tdata_lma_end'
/opt/devkitpro/devkitA64/bin/../lib/gcc/aarch64-none-elf/10.2.0/../../../../aarch64-none-elf/bin/ld: /home/fincs/pacman-packages/libnx/src/libnx-4.0.0/nx/source/runtime/newlib.c:438: undefined reference to `__tdata_lma'
/opt/devkitpro/devkitA64/bin/../lib/gcc/aarch64-none-elf/10.2.0/../../../../aarch64-none-elf/bin/ld: /opt/devkitpro/libnx/lib/libnx.a(argv.o): in function `argvSetup':
/home/fincs/pacman-packages/libnx/src/libnx-4.0.0/nx/source/runtime/argv.c:48: undefined reference to `__argdata__'
/opt/devkitpro/devkitA64/bin/../lib/gcc/aarch64-none-elf/10.2.0/../../../../aarch64-none-elf/bin/ld: /home/fincs/pacman-packages/libnx/src/libnx-4.0.0/nx/source/runtime/argv.c:48: undefined reference to `__argdata__'
/opt/devkitpro/devkitA64/bin/../lib/gcc/aarch64-none-elf/10.2.0/../../../../aarch64-none-elf/bin/ld: /opt/devkitpro/libnx/lib/libnx.a(thread.o): in function `threadCreate':
/home/fincs/pacman-packages/libnx/src/libnx-4.0.0/nx/source/kernel/thread.c:98: undefined reference to `__tls_start'
/opt/devkitpro/devkitA64/bin/../lib/gcc/aarch64-none-elf/10.2.0/../../../../aarch64-none-elf/bin/ld: /home/fincs/pacman-packages/libnx/src/libnx-4.0.0/nx/source/kernel/thread.c:98: undefined reference to `__tls_end'
/opt/devkitpro/devkitA64/bin/../lib/gcc/aarch64-none-elf/10.2.0/../../../../aarch64-none-elf/bin/ld: /home/fincs/pacman-packages/libnx/src/libnx-4.0.0/nx/source/kernel/thread.c:98: undefined reference to `__tls_end'
/opt/devkitpro/devkitA64/bin/../lib/gcc/aarch64-none-elf/10.2.0/../../../../aarch64-none-elf/bin/ld: /home/fincs/pacman-packages/libnx/src/libnx-4.0.0/nx/source/kernel/thread.c:98: undefined reference to `__tls_start'
/opt/devkitpro/devkitA64/bin/../lib/gcc/aarch64-none-elf/10.2.0/../../../../aarch64-none-elf/bin/ld: /home/fincs/pacman-packages/libnx/src/libnx-4.0.0/nx/source/kernel/thread.c:170: undefined reference to `__tdata_lma_end'
/opt/devkitpro/devkitA64/bin/../lib/gcc/aarch64-none-elf/10.2.0/../../../../aarch64-none-elf/bin/ld: /home/fincs/pacman-packages/libnx/src/libnx-4.0.0/nx/source/kernel/thread.c:170: undefined reference to `__tdata_lma'
/opt/devkitpro/devkitA64/bin/../lib/gcc/aarch64-none-elf/10.2.0/../../../../aarch64-none-elf/bin/ld: /home/fincs/pacman-packages/libnx/src/libnx-4.0.0/nx/source/kernel/thread.c:170: undefined reference to `__tdata_lma_end'
/opt/devkitpro/devkitA64/bin/../lib/gcc/aarch64-none-elf/10.2.0/../../../../aarch64-none-elf/bin/ld: /home/fincs/pacman-packages/libnx/src/libnx-4.0.0/nx/source/kernel/thread.c:170: undefined reference to `__tdata_lma'
/opt/devkitpro/devkitA64/bin/../lib/gcc/aarch64-none-elf/10.2.0/../../../../aarch64-none-elf/bin/ld: /opt/devkitpro/libnx/lib/libnx.a(thread.o): in function `threadClose':
/home/fincs/pacman-packages/libnx/src/libnx-4.0.0/nx/source/kernel/thread.c:235: undefined reference to `__tls_end'
/opt/devkitpro/devkitA64/bin/../lib/gcc/aarch64-none-elf/10.2.0/../../../../aarch64-none-elf/bin/ld: /home/fincs/pacman-packages/libnx/src/libnx-4.0.0/nx/source/kernel/thread.c:235: undefined reference to `__tls_start'
/opt/devkitpro/devkitA64/bin/../lib/gcc/aarch64-none-elf/10.2.0/../../../../aarch64-none-elf/bin/ld: /home/fincs/pacman-packages/libnx/src/libnx-4.0.0/nx/source/kernel/thread.c:235: undefined reference to `__tls_start'
/opt/devkitpro/devkitA64/bin/../lib/gcc/aarch64-none-elf/10.2.0/../../../../aarch64-none-elf/bin/ld: /home/fincs/pacman-packages/libnx/src/libnx-4.0.0/nx/source/kernel/thread.c:235: undefined reference to `__tls_end'
collect2: error: ld returned 1 exit status
return code: 1

This is caused by the requirement of the devkitA64 toolchain to use the switch.specs when building against libnx. Now, for actually building scummvm on host switch, this -specs option is correctly set here https://github.com/scummvm/scummvm/blob/master/configure#L3207 but this LIBS var isn't used during the conftest, so it fails. We can't just append LIBS to LIBCURL_LIBS or add just the -specs=$DEVKITPRO/libnx/switch.specs option to it, since then we'll get problems when linking the libcurl-referencing object files into the final scummvm binary: the invocation will contain the -specs option twice (once via LIBS and once via LIBCURL_LIBS) and it will fail since it'll try to rename the link that doesn't exist anymore (the first -specs option will have already renamed it).

This change simply adds the -specs option to the conftest macro invocation if and only if the host is switch, which admittedly is kind of unpleasing aesthetically, but it does make everything work properly.

@justinkb
Copy link
Author

@justinkb justinkb commented Jan 4, 2021

I should note that viewing the buildbot logs for recent switch builds I didn't see this conftest issue - https://buildbot.scummvm.org/builders/master-switch/builds/5065/steps/configure/logs/stdio

Do you have any information about what version of the toolchain packages for nintendo switch the buildbot currently employs? Might be caused by a recently introduced issue with the DKP toolchain

EDIT: narrowed this down to a change that occurred with the switch from libnx-3.3.0-2 to libnx-4.0.0-1
EDIT2: I have since verified that this change is backwards compatible with libnx-3.3.0-2 installs

@lephilousophe lephilousophe self-requested a review Jan 4, 2021
@lephilousophe
Copy link
Member

@lephilousophe lephilousophe commented Jan 4, 2021

I didn't meet this problem with an up to date Switch toolchain last time I checked.
I will try tomorrow.
In addition, I would set append to LIBCURL_LIBS in the switch above.

@justinkb
Copy link
Author

@justinkb justinkb commented Jan 5, 2021

@lephilousophe
Copy link
Member

@lephilousophe lephilousophe commented Jan 6, 2021

So, I checked devkitA64 docker image. It's working because they still are on libnx 3.

Anyway, I just tested another fix which should be cleaner.
Instead of modifying curl part, could you edit line 3207 and replace:
append_var LIBS "-specs=$DEVKITPRO/libnx/switch.specs"
by
append_var LDFLAGS "-specs=$DEVKITPRO/libnx/switch.specs"

This should ensure it's used when checking for libcurl and not added twice.
This patch works with current devkitA64.

@justinkb
Copy link
Author

@justinkb justinkb commented Jan 6, 2021

@justinkb
Copy link
Author

@justinkb justinkb commented Jan 7, 2021

Tested your change, works perfectly! Should I close this pull request in favor of your changes?

@lephilousophe
Copy link
Member

@lephilousophe lephilousophe commented Jan 8, 2021

Or you can force push on your branch to make sure we use a patch that really works. :)
Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.