Restore -static-executable on Linux. #29039

spevans · 2020-01-07T15:26:25Z

Add back StaticBinaryELF.cpp which provides swift::lookupSymbol()
for statically linked ELF binaries.
Build libswiftImageInspectionStatic.a from StaticBinaryELF.cpp
Update static-executable-args.lnk and replace
libswiftImageInspectionShared with libswiftImageInspectionStatic.
Add driver tests for '-static-executable' on Linux to validate
that output binary is statically linked.
Fix linkage to pthreads to avoid weak linkage issues in a static
executable.

This is a rebase of a previous PR but that didnt work with Ubutnu18.04 only 16.04. This version has been tested with 16.04, 18.04 and 19.10.

Note, as with -static-stdlib, only Dispatch can be used with -static-executable, Foundation cannot at this time.

Integration tests have also been added to catch any future breakage of this change.

spevans · 2020-01-07T15:26:41Z

Please test with following pull request:
swiftlang/swift-integration-tests#69

@swift-ci please test

spevans · 2020-01-07T15:27:00Z

cc @jckarter @weissi

stdlib/public/runtime/StaticBinaryELF.cpp

compnerd · 2020-01-07T16:42:17Z

stdlib/public/runtime/ImageInspectionELF.cpp

@@ -144,6 +152,8 @@ int swift::lookupSymbol(const void *address, SymbolInfo *info) {
  return 1;
 }

+#endif


Why not just use the alternate version always? The cost is nearly negligible - the cost is largely associated with the fact that the information from the ELF header parsing may be cached by the loader, which is not guaranteed.

spevans · 2020-01-10T00:59:15Z

@swift-ci test

swift-ci · 2020-01-10T01:01:44Z

Build failed
Swift Test Linux Platform
Git Sha - 340fd224916675e06cacc6d144f1054bb0b8b7eb

swift-ci · 2020-01-10T01:04:51Z

Build failed
Swift Test OS X Platform
Git Sha - 340fd224916675e06cacc6d144f1054bb0b8b7eb

spevans · 2020-01-10T09:07:01Z

Please test with following pull request:
swiftlang/swift-integration-tests#69

@swift-ci please test

swift-ci · 2020-01-10T10:35:56Z

Build failed
Swift Test Linux Platform
Git Sha - 15ac30fbc2be2fe15d8c53a9ac0a3ed9deedbf69

spevans · 2020-01-10T14:13:56Z

Please test with following pull request:
swiftlang/swift-integration-tests#69

@swift-ci please test linux

swift-ci · 2020-01-10T15:42:02Z

Build failed
Swift Test Linux Platform
Git Sha - 15ac30fbc2be2fe15d8c53a9ac0a3ed9deedbf69

spevans · 2020-01-10T15:49:52Z

Please test with following pull request:
swiftlang/swift-integration-tests#69

@swift-ci please test linux

swift-ci · 2020-01-10T17:18:56Z

Build failed
Swift Test Linux Platform
Git Sha - 15ac30fbc2be2fe15d8c53a9ac0a3ed9deedbf69

spevans · 2020-01-10T20:10:25Z

@swift-ci smoke test linux

spevans · 2020-01-10T21:58:02Z

@swift-ci smoke test linux

spevans · 2020-01-11T00:10:32Z

Please test with following pull request:
swiftlang/swift-integration-tests#69

@swift-ci please test linux

compnerd

I think I missed it or something ... why not use the parsing logic even for the shared linking?

lib/Driver/UnixToolChains.cpp

stdlib/public/runtime/CMakeLists.txt

compnerd · 2020-01-11T02:22:08Z

stdlib/public/runtime/StaticBinaryELF.cpp

+int (*__strong_pthread_once)(pthread_once_t *, void (*)(void)) = pthread_once;
+
+__attribute__((__visibility__("hidden")))
+int (*__strong_pthread_key_create)(pthread_key_t *, void (*)(void *)) = pthread_key_create;


I really wish we had linker options ... -u pthread_self -u pthread_once -u pthead_key_create is so much better than this. sigh

I had used ---defsym=__import_pthread_self=pthread_self options to the linker before but it looks like that doesnt create strong linkage anymore.

--defsym creating a strong reference sounds like a bug. If you want to create a strong reference, you should use --require-defined or -u at the very least.

I didnt know about this option! I had to use -undefined (-u) since ld.gold is being used, --require-defined seems to only work on ld

- Add back StaticBinaryELF.cpp which provides swift::lookupSymbol() for statically linked ELF binaries. - Build libswiftImageInspectionStatic.a from StaticBinaryELF.cpp - Update static-executable-args.lnk and replace libswiftImageInspectionShared with libswiftImageInspectionStatic. - Fix linkage to pthreads to avoid weak linkage issues in a static executable. - Add driver test for '-static-executable' on Linux to validate that output binary is statically linked.

spevans · 2020-01-11T09:01:04Z

Please test with following pull request:
swiftlang/swift-integration-tests#69

@swift-ci please test

swift-ci · 2020-01-11T09:01:40Z

Build failed
Swift Test OS X Platform
Git Sha - 15ac30fbc2be2fe15d8c53a9ac0a3ed9deedbf69

swift-ci · 2020-01-11T09:03:30Z

Build failed
Swift Test Linux Platform
Git Sha - 15ac30fbc2be2fe15d8c53a9ac0a3ed9deedbf69

spevans · 2020-01-11T11:00:14Z

Please test with following pull request:
swiftlang/swift-integration-tests#69

@swift-ci please smoketest macos

spevans · 2020-01-11T11:01:36Z

Please test with following pull request:
swiftlang/swift-integration-tests#69

@swift-ci please smoke test macos

spevans · 2020-01-11T13:40:07Z

Please test with following pull request:
swiftlang/swift-integration-tests#69

@swift-ci please test macos

swift-ci · 2020-01-11T14:50:51Z

Build failed
Swift Test OS X Platform
Git Sha - 3b25795

spevans · 2020-01-11T20:31:13Z

I think I missed it or something ... why not use the parsing logic even for the shared linking?

This will only parse 1 ELF file and wont handle the extra shared libraries that are dynamically linked in or their relocations. There would also be no benefit to replacing a platform's dladdr() even if a full implementation was written.

spevans · 2020-01-12T15:18:06Z

Please test with following pull request:
swiftlang/swift-integration-tests#69

@swift-ci please test macos

swift-ci · 2020-01-12T16:54:23Z

Build failed
Swift Test OS X Platform
Git Sha - 3b25795

compnerd

This will only parse 1 ELF file and wont handle the extra shared libraries that are dynamically linked in or their relocations. There would also be no benefit to replacing a platform's dladdr() even if a full implementation was written.

The registration is per module, and is not shared across dynamically shared libraries. I think that you are focusing too much about the fact that there is a wrapper for the dladdr in this file. That doesn't really belong in there, it was just convenient for it to get placed there I think.

compnerd · 2020-01-12T22:08:18Z

stdlib/public/runtime/StaticBinaryELF.cpp

+int (*__strong_pthread_once)(pthread_once_t *, void (*)(void)) = pthread_once;
+
+__attribute__((__visibility__("hidden")))
+int (*__strong_pthread_key_create)(pthread_key_t *, void (*)(void *)) = pthread_key_create;


--defsym creating a strong reference sounds like a bug. If you want to create a strong reference, you should use --require-defined or -u at the very least.

pthread functions.

spevans · 2020-01-13T11:38:42Z

Please test with following pull request:
swiftlang/swift-integration-tests#69

@swift-ci please test

swift-ci · 2020-01-13T11:39:22Z

Build failed
Swift Test OS X Platform
Git Sha - 3b25795

swift-ci · 2020-01-13T11:41:13Z

Build failed
Swift Test Linux Platform
Git Sha - 3b25795

jckarter · 2020-01-13T19:16:35Z

lookupSymbol should only be used for attempting to symbolicate stack traces, so if it's slow or doesn't work 100% of the time that shouldn't be a showstopper. @drexin had been looking into more robust symbolication using addr2line style DWARF line table decoding which might be able to supersede our use of dladdr as well.

spevans · 2020-01-13T21:24:44Z

Thats correct, its only really used for stack traces hence why it doesnt matter too much if the mmap fails etc. How would the alternative work if there was not DWARF data?

spevans · 2020-01-16T20:33:18Z

@compnerd FYI: the reason for the ImageInspectionShared / ImageInspectionStatic split is mostly due to not wanting to export dladdr in the static case. See the discussion in #5394 from when the lookupSymbol was added.

jckarter · 2020-02-10T19:16:56Z

Thats correct, its only really used for stack traces hence why it doesnt matter too much if the mmap fails etc. How would the alternative work if there was not DWARF data?

For server side work, I think we'd want to encourage building with at least -gline-tables-only as the standard configuration, to enable better quality runtime diagnostics and backtraces. We could still try dladdr as a best effort thing, maybe.

@compnerd FYI: the reason for the ImageInspectionShared / ImageInspectionStatic split is mostly due to not wanting to export dladdr in the static case. See the discussion in #5394 from when the lookupSymbol was added.

IIRC, at some point ImageInspectionStatic also resolved the registration tables completely statically at some point, rather than relying on iterating through images with libdl API that wouldn't be available in a static executable.

spevans · 2020-02-10T22:58:22Z

After checking the man pages and doing a test I found that dladdr only finds symbols in .so libraries and not in the main executable - this probably explains why the stack traces aren't very good on Linux:

$ cat dltest.c
#define _GNU_SOURCE
#include <dlfcn.h>
#include <stdio.h>


void show(void *addr) {
  Dl_info info = {};

  int ret = dladdr(addr, &info);

  if (ret) {
    printf("fname: %s\n", info.dli_fname);
    printf("fbase: %p\n", info.dli_fbase);
    printf("sname: %s\n", info.dli_sname);
    printf("saddr: %p\n\n", info.dli_saddr);
  } else {
    printf("Cant get dladdr\n\n");
  }
}


int main() {
  show(main);
  show(printf);
  return 0;
}

$ clang -Wall -static -O  -o dltest-static dltest.c -ldl
$ ./dltest-static 
Cant get dladdr

Cant get dladdr

$ clang -Wall -O -o dltest-dynamic dltest.c -ldl
$ ./dltest-dynamic 
fname: ./dltest-dynamic
fbase: 0x400000
sname: (null)
saddr: (nil)

fname: ./dltest-dynamic
fbase: 0x400000
sname: printf
saddr: 0x400490

So dladdr cant be used in a static binary.

weissi · 2020-02-11T08:12:25Z

@spevans I think more than main bin vs library, IIRC dladdr can only be used on exported symbols. So it’ll work on printf because that’s exported but it won’t on main or other non-exported functions like anything private in Swift or a static function in C. So I don’t think we can it for anything much in either static or dynamic binaries

shahmishal · 2020-10-01T06:57:55Z

Please update the base branch to main by Oct 5th otherwise the pull request will be closed automatically.

How to change the base branch: (Link)
More detail about the branch update: (Link)

spevans · 2020-10-05T09:20:32Z

An updated and more simplified version of this pr is #34180

compnerd reviewed Jan 7, 2020

View reviewed changes

spevans force-pushed the pr_static_executable_fix branch from 340fd22 to 15ac30f Compare January 10, 2020 00:55

compnerd reviewed Jan 11, 2020

View reviewed changes

spevans force-pushed the pr_static_executable_fix branch from 15ac30f to 3b25795 Compare January 11, 2020 09:00

compnerd requested changes Jan 12, 2020

View reviewed changes

- Use -undefined argument to ld.gold to create strong linkage to

c5f6c69

pthread functions.

spevans mentioned this pull request Jan 13, 2020

linux: remove swiftImageInspectionShared #14877

Closed

spevans mentioned this pull request Jul 7, 2020

[static][stdlib] Any ELF platform can build static binaries. #32736

Open

spevans closed this Oct 5, 2020

spevans mentioned this pull request Jul 7, 2020

[SR-13165] Is splitting ImageInspectionELF.cpp into swiftImageInspectionShared necessary? #55608

Closed

Restore -static-executable on Linux. #29039

Restore -static-executable on Linux. #29039

Conversation

spevans commented Jan 7, 2020

spevans commented Jan 7, 2020

spevans commented Jan 7, 2020

compnerd Jan 7, 2020

Choose a reason for hiding this comment

spevans commented Jan 10, 2020

swift-ci commented Jan 10, 2020

swift-ci commented Jan 10, 2020

spevans commented Jan 10, 2020

swift-ci commented Jan 10, 2020

spevans commented Jan 10, 2020

swift-ci commented Jan 10, 2020

spevans commented Jan 10, 2020

swift-ci commented Jan 10, 2020

spevans commented Jan 10, 2020

spevans commented Jan 10, 2020

spevans commented Jan 11, 2020

compnerd left a comment

Choose a reason for hiding this comment

compnerd Jan 11, 2020

Choose a reason for hiding this comment

spevans Jan 11, 2020

Choose a reason for hiding this comment

compnerd Jan 12, 2020

Choose a reason for hiding this comment

spevans Jan 13, 2020

Choose a reason for hiding this comment

spevans commented Jan 11, 2020

swift-ci commented Jan 11, 2020

swift-ci commented Jan 11, 2020

spevans commented Jan 11, 2020

spevans commented Jan 11, 2020

spevans commented Jan 11, 2020

swift-ci commented Jan 11, 2020

spevans commented Jan 11, 2020

spevans commented Jan 12, 2020

swift-ci commented Jan 12, 2020

compnerd left a comment

Choose a reason for hiding this comment

compnerd Jan 12, 2020

Choose a reason for hiding this comment

spevans commented Jan 13, 2020

swift-ci commented Jan 13, 2020

swift-ci commented Jan 13, 2020

jckarter commented Jan 13, 2020

spevans commented Jan 13, 2020

spevans commented Jan 16, 2020

jckarter commented Feb 10, 2020

spevans commented Feb 10, 2020

weissi commented Feb 11, 2020

shahmishal commented Oct 1, 2020

spevans commented Oct 5, 2020