Skip to content
This repository has been archived by the owner on Oct 12, 2022. It is now read-only.

fix for Issue 11543 - multiple definition of std.regex with shared library #791

Merged
merged 1 commit into from
May 26, 2014

Conversation

MartinNowak
Copy link
Member

  • Detect copy relocated ModuleInfos when checking for
    conflicting module definitions.

Issue 11543

…brary

- Detect copy relocated ModuleInfos when checking for
  conflicting module definitions.
}
// Module is in .bss of the exe because it was copy relocated
}
else if (!findSegmentForAddr(info, addr))
Copy link
Member Author

Choose a reason for hiding this comment

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

This pull contains another change which is required for dlang/dmd#3551, it will no longer assume that all ModuleInfos are contained in the same segment.

@MartinNowak
Copy link
Member Author

Anyone wants to review this? @andralex

@jwhear
Copy link

jwhear commented May 23, 2014

👍

@WalterBright
Copy link
Member

Auto-merge toggled on

@MartinNowak
Copy link
Member Author

Thanks, waiting for the spurious OSX failure to be retested.

WalterBright added a commit that referenced this pull request May 26, 2014
fix for Issue 11543 - multiple definition of std.regex with shared library
@WalterBright WalterBright merged commit 09ea3d6 into dlang:master May 26, 2014
@MartinNowak MartinNowak deleted the fix11543 branch May 27, 2014 03:38
@MartinNowak
Copy link
Member Author

Mail discussion with @klickverbot:

I'm working on Linux DSO support for LDC right now, and the only thing that still acts up is the module conflict detection code. The issue is that when compiling a D executable without PIC, some ModuleInfoZ's are copy-relocated into the executable's BSS section. I saw that you tried to address this in your fix for issue 11543, but I'm not quite sure how that piece of code is supposed to work:

checkModuleCollisions tries to use __bss_start/_end for detecting the boundaries of the executable's BSS. But as far as I can tell, ld will bind them to the bracketing symbols of libdruntime.so, not those of the main executable. Did you ever check if the code actually works as intended on DMD?

It would be great if you could quickly answer with yes/no as I'm not sure whether I'm missing something that is crucially wrong about my (LDC) codegen right now. Otherwise, I'll probably post to [druntime] later once I figure out what is going on…

Those globally visible symbols are defined by the linker script.

__bss_start = .;
_end = .;

So they are defined in any DSO, but the ones in the executable will override the ones in a shared library.

@dnadlinger
Copy link
Member

@MartinNowak: Okay, a quick test seems to indeed confirm that they are overridden with the bracketing symbols from the executable. So back to the beginning for me in trying to figure out why the check fails on LDC (without the module infos actually being duplicated), but it should be fine for DMD. Apparently I had misread the section maps, need to recheck…

@dnadlinger
Copy link
Member

@MartinNowak: Okay, and now I can't even reproduce the issue at all. Well, speaks for your implementation, I guess. ;)

@MartinNowak
Copy link
Member Author

Well, speaks for your implementation, I guess. ;)

Glad to hear this :).

dnadlinger added a commit to ldc-developers/druntime that referenced this pull request Jul 5, 2014
fix for Issue 11543 - multiple definition of std.regex with shared library

The copy relocations for the ModuleInfo symbols generated
by the linker when building the main executable without PIC
enabled were throwing the module collision detection off.
dnadlinger added a commit to ldc-developers/druntime that referenced this pull request Jul 6, 2014
…s across threads."

This is no longer necessary, as dlang#791
switched Thread.getThis() to use explicit pthreads TLS for a
completely unrelated reason anyway.

This reverts commit db7f8ee.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants