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

Load dynamic libraries on darwin #3669

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

kepkin
Copy link

@kepkin kepkin commented Feb 20, 2024

No description provided.

Copy link
Member

@aarzilli aarzilli left a comment

Choose a reason for hiding this comment

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

Did you test this in any way?

if p.BinInfo().GOOS == "darwin" {
images, err := p.conn.getLoadedDynamicLibraries()
if err != nil {
return nil, stopReason, fmt.Errorf("could not load dynamic libraries %s", err)
Copy link
Member

Choose a reason for hiding this comment

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

I think it's better to just log this: logflags.GdbWireLogger().Errorf(...)

Copy link
Author

Choose a reason for hiding this comment

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

5 lines above there is a return statement for ElfUpdateSharedObjects. I believe behaviour for linux and darwin should be the same.

@derekparker
Copy link
Member

I just get a bunch of errors like this:

(dlv) libraries
  0. 0x0 /usr/lib/dyld
    Load error: invalid magic number in record at byte 0x0
  1. 0x0 /System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation
    Load error: open /System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation: no such file or directory
  2. 0x0 /usr/lib/libobjc.A.dylib
    Load error: open /usr/lib/libobjc.A.dylib: no such file or directory
  3. 0x0 /System/Library/PrivateFrameworks/CoreServicesInternal.framework/Versions/A/CoreServicesInternal
    Load error: open /System/Library/PrivateFrameworks/CoreServicesInternal.framework/Versions/A/CoreServicesInternal: no such file or directory
  4. 0x0 /System/Library/Frameworks/Foundation.framework/Versions/C/Foundation
    Load error: open /System/Library/Frameworks/Foundation.framework/Versions/C/Foundation: no such file or directory
  5. 0x0 /usr/lib/liboah.dylib
    Load error: open /usr/lib/liboah.dylib: no such file or directory
  6. 0x0 /usr/lib/libfakelink.dylib
    Load error: open /usr/lib/libfakelink.dylib: no such file or directory
  7. 0x0 /usr/lib/libicucore.A.dylib
    Load error: open /usr/lib/libicucore.A.dylib: no such file or directory
  8. 0x0 /usr/lib/libSystem.B.dylib
    Load error: open /usr/lib/libSystem.B.dylib: no such file or directory
  9. 0x0 /System/Library/PrivateFrameworks/SoftLinking.framework/Versions/A/SoftLinking
    Load error: open /System/Library/PrivateFrameworks/SoftLinking.framework/Versions/A/SoftLinking: no such file or directory
...

@kepkin
Copy link
Author

kepkin commented Feb 26, 2024

I actually debugged our plugin with Krakend in VS Code. But I haven't tried listing library.

It won't load any OSX specific library, but go modules work. Maybe I should filter OSX .dylib?

@kepkin
Copy link
Author

kepkin commented Feb 27, 2024

I've dug a little bit. So those libraries won't load, cause they are universal binaries.

% lipo -archs /usr/lib/dyld
i386 x86_64 arm64e

However GO builds only one architecture (here are my minimal cmd and plugin files)

 % lipo -archs cli-plugin   
arm64
 % lipo -archs plug.so   
arm64

In this PR I would like to give opportunity to debug GO plugin on OSX (I suppose the most common case). And then fix issue with loading libraries which are compiled as universal binaries.

Here is a recording how it works https://asciinema.org/a/ePsIt72qx5C9aLsNe57dKdDEg

Regarding universal binaries. I can either skip them loading or leave as it is now.

@aarzilli
Copy link
Member

In general this is good, however there is a problem. We have a bunch of tests for plugin debugging, these are currently disabled by a runtime.GOOS check in WithPlugins (which is in pkg/proc/test/support.go).
That check should be changed, however when you change it you will see that a bunch of tests in CI will start to fail. The reason is that the vast majority of macOS versions ship with a broken build of dsymutil, specifically the bug fixed by llvm/llvm-project@10539ec.

So I think we (@derekparker and I) need to decide what the behavior of delve is going to be in those cases:

  1. we just let it be silently broken and skip those tests in CI
  2. we only merge this PR after we have code to detect plugins with broken debug_frame sections
  3. ???

@derekparker
Copy link
Member

In general this is good, however there is a problem. We have a bunch of tests for plugin debugging, these are currently disabled by a runtime.GOOS check in WithPlugins (which is in pkg/proc/test/support.go). That check should be changed, however when you change it you will see that a bunch of tests in CI will start to fail. The reason is that the vast majority of macOS versions ship with a broken build of dsymutil, specifically the bug fixed by llvm/llvm-project@10539ec.

So I think we (@derekparker and I) need to decide what the behavior of delve is going to be in those cases:

  1. we just let it be silently broken and skip those tests in CI
  2. we only merge this PR after we have code to detect plugins with broken debug_frame sections
  3. ???

Apologies on the late response here.

I think we go with option 2, and we use that to selectively skip tests in CI.

@kepkin
Copy link
Author

kepkin commented Apr 23, 2024

Hi.

I'm pretty busy on my main job. However if you give me a hint on how to to detect broken debug_frame sections, I might be able to do this on weekends.

@aarzilli
Copy link
Member

I think it would be acceptable to do this by checking what macOSDebugFrameBugWorkaround does (in pkg/proc/bininfo.go). If the workaround is applied then dynamic libraries shouldn't be loaded.

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.

None yet

3 participants