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

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

Closed
swift-ci opened this issue Jul 7, 2020 · 6 comments
Assignees
Labels

Comments

@swift-ci
Copy link
Contributor

swift-ci commented Jul 7, 2020

Previous ID SR-13165
Radar None
Original Reporter 3405691582 (JIRA User)
Type Task
Status Resolved
Resolution Done
Additional Detail from JIRA
Votes 0
Component/s
Labels Task
Assignee @spevans
Priority Medium

md5: 690e563998a653a367ea95c34a707ee2

Issue Description:

stdlib/public/runtime/CMakeLists.txt splits ImageInspectionELF.cpp out from swift_runtime_sources into its own library, swiftImageInspectionShared. There appear to still be references from Errors.cpp in swift_runtime_sources to lookupSymbol in ImageInspectionELF.cpp, which means when SWIFT_BUILD_STATIC_STDLIB, if swiftImageInspectionShared is not referred to, complaints about undefined symbols when lazy binding will occur.

This additional error output fouls up the expectation in a number of unit tests, see pr #32736. This only occurs when SWIFT_BUILD_STATIC_STDLIB. (Specifically, the reference in Errors.cpp is only when SWIFT_SUPPORTS_BACKTRACE_REPORTING too.)

Is making this split truly necessary? I suspect it may not be, but I'd have to do some testing on Linux machines to see what the effect would be. See pr #5394, SR-648.

@swift-ci
Copy link
Contributor Author

swift-ci commented Jul 7, 2020

Comment by 3405691582 (JIRA)

cc @spevans

@spevans
Copy link
Contributor

spevans commented Jul 7, 2020

The reason for the split into swiftImageInspectionShared and swiftImageInspectionStatic is to support -static-stdlib and -static-executable

When building with -static-stdlib the exectuable is a dynamic executable that links in the .a versions of libswiftCore libswifDispatch etc but to implement swift::lookupSymbol it uses dladdr and links in -ldl as is common for standard ELF shared executables on Linux.

When building a fully static executable using -static-executable the libdl functions cant be used so swift::lookupSymbol is implemented by directly parsing the ELF sections in the static binary.

So because there are 2 different implementations of swift::lookupSymbol there has to be 2 different libswiftImageInspection{Shared,Static}.a libraries.

Note that -static-executable is currently broken, this was due to CMakefile.txt changes made a while ago, there is a PR to fix it #29039 but it ended up stagnating.

@spevans
Copy link
Contributor

spevans commented Jul 7, 2020

Note swift::lookupSymbol is used for stacktraces but dladdr wont work in static binaries

Example:

$ cat dladdr-test.c 
#include <stdio.h>
#define  __USE_GNU 1
#include <dlfcn.h>

int main() {
        Dl_info info = {};
        if (dladdr(main, &info)) {
                printf("fname:\t%s\n", info.dli_fname);
                printf("base:\t%p\n", info.dli_fbase);
                printf("sname:\t%s\ns", info.dli_sname);
                printf("saddr:\t%p\n", info.dli_saddr);
        } else {
                puts("Cant dladdr() main");
        }
        return 0;
}
# Ubuntu 18.04
$ clang -Wall -o dynamic-dladdr dladdr-test.c -ldl
$ ./dynamic-dladdr 
fname:  ./dynamic-dladdr
base:   0x400000
sname:  (null)
ssaddr: (nil)

$ clang -Wall -static -o static-dladdr dladdr-test.c -ldl
$ ./static-dladdr 
Cant dladdr() main
# OpenBSD 6.6

$ clang -Wall -o dynamic-dladdr dladdr-test.c
$ ./dynamic-dladdr                                                                                                                
fname:  ./dynamic-dladdr
base:   0x160e3000
sname:  (null)
ssaddr: 0x0

$ clang -Wall -static -o static-dladdr dladdr-test.c  
$ ./static-dladdr                                                                                                                 
Wrong dl symbols!
fname:  (null)
base:   0x0
sname:  (null)
ssaddr: 0x0

@spevans
Copy link
Contributor

spevans commented Oct 5, 2020

There is a new version of my PR @ #34180 that removes the split at the cost of swift::lookupSymbol not working in static executables. However that code was very Linux specific in how it found the loaded ELF exectuable and would not have work on OpenBSD anyway.

@spevans
Copy link
Contributor

spevans commented Oct 9, 2020

The split has been removed now and there is no separate swiftImageInspectionShared anymore.

@swift-ci
Copy link
Contributor Author

swift-ci commented Oct 9, 2020

Comment by 3405691582 (JIRA)

Thank you!

@swift-ci swift-ci transferred this issue from apple/swift-issues Apr 25, 2022
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants