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

Add -sL swith to pass linker flags at the end of the linker command #497

Closed
wants to merge 1 commit into from

Conversation

llucax
Copy link
Contributor

@llucax llucax commented Nov 7, 2011

There are some cases where linker flags order do matter (for example for
linking libraries). In those cases DMD provide no mechanism for altering
the order. This patches tries to minimize this problem by adding an
option (-sL) to add flags at the end of the linker command line. This
is specially useful to include libraries after the standard library
passed with -debuglib or -defaultlib.

Posted in the devel ML: http://permalink.gmane.org/gmane.comp.lang.d.dmd.devel/2530

@llucax
Copy link
Contributor Author

llucax commented Dec 1, 2011

@WalterBright
Copy link
Member

Which cases matter? My concern is that these kinds of things sound straightforward, and they are in isolation, but things can get pretty confusing when one is confronted with a list of these switches.

@llucax
Copy link
Contributor Author

llucax commented Feb 2, 2012

Maybe you missed the ML link, where I explain the exact situation, I'll copy it here for convenience:

Hi! I'm working on generating stacktraces (in Linux) for unhandled
exceptions and other situations for D1+Tango and I came across a problem
that I can't find any way to fix but to do it in the compiler.

The problem is I need to pass -ldl to the linker, but for some reason it
has to be passed after the standard library. Since DMD pass all the -L
flags before it, I came up with a patch that adds a new -sL flag that
places the passed flags at the end of the linking commands.

I know Walter is not a fan of compiler flags, but I really don't see
other option (besides writing the linker command manually, which is not
really a good option). If you know another way to fix this, please let
me know. If not, please consider reviewing the patch and merging it
after any needed changes. In particular I don't know what to do with the
switch in windows, I don't know if this is needed and I don't know the
windows linker flags, so in the patch I just placed these flags after
the regular linker flags.

@llucax
Copy link
Contributor Author

llucax commented Feb 2, 2012

I also added a comment to the bug report:
http://d.puremagic.com/issues/show_bug.cgi?id=7044#c1

By the way, I don't find the solution elegant at all, but it's the only
solution I could think of. Maybe it would be better to use the same order of
the command line switches when passing them to the linker, even for the
-defaultlib and -debuglib.

Examples:

dmd -L-L/opt/lib -defaultlib=blah -L-lfoo -> gcc -L/opt/lib -lblah -lfoo
dmd -L-lbar -defaultlib=blah -> gcc -lbar -lblah

The problem is what to do then with the switches that are automatically added
by DMD. Maybe those should be just moved to dmd.conf (except for -Xlinker of
course), because I think they are not really needed by the the generated code
but by the runtime, which can be changed via -defaultlib.

There are some cases where linker flags order do matter (for example for
linking libraries). In those cases DMD provide no mechanism for altering
the order. This patches tries to minimize this problem by adding an
option (-sL) to add flags at the *end* of the linker command line. This
is specially useful to include libraries *after* the standard library
passed with -debuglib or -defaultlib.
@ghost
Copy link

ghost commented Jun 29, 2012

@brad-anderson
Copy link
Member

As the person who posted the Stack Overflow question, I welcome a fix for this. Any chance this could be fixed up so that it passes the autotester?

Test Results

@llucax
Copy link
Contributor Author

llucax commented Jul 3, 2012

The patch was done for D1 and the autotester only tests D2. Should be trivial to adapt, I never done it because of the lack of interest in merging it, but I'll be happy to do so if the interest is there.

I don't really like this patch myself though, I think a better solution would be to honour the command line options order when passing them to gcc and move all the runtime-related flags to the dmd.conf. Since the runtime library can be specified via -defaultlib it makes no sense at all to hardcode flags related to the runtime in the compiler.

I'll be willing to code that patch if it has any support from the management.

@andralex
Copy link
Member

What's the status of this? The issue seems to block certain build styles, so we should review this diff and merge it.

@llucax
Copy link
Contributor Author

llucax commented Sep 24, 2012

The status is exactly the same as in the previous message. I never have been very lucky with the acceptance of my patches, so now I'm very wary about putting any work on patches until somebody can confirm not only there is interest in solving the issue but also the solution I'm proposing will be accepted. I'm sorry if I'm asking for too much, but I even wrote a concurrent GC that's sitting there completely wasted. Too much work wasted to keep wasting work.

@andralex
Copy link
Member

I understand. @WalterBright, please confirm whether rebasing would make this pull acceptable, or if not, what would make it so. Thanks!

@yebblies
Copy link
Member

yebblies commented Oct 6, 2012

Is it really unreasonable to just call the linker/gcc yourself once the complexity gets too high? You're already passing linker-specific arguments, so you probably know which linker to call. If there is information missing, maybe being able to get that from dmd would be a more flexible solution for this and other problems.

@llucax
Copy link
Contributor Author

llucax commented Oct 6, 2012

The use case that motivated this patch is a very particular one, D1 Tango. I've implemented stack tracing for unhandled exceptions but I can't make it work with DMD as it is without manual linking. Asking all Tango users to do manual linking seems pretty unreasonable to me, but I can understand D1+Tango being rarer and rarer with time.

That said, I still think is a very bad idea to have the compiler and runtime so coupled that link options that are completely dependant on the runtime implementation hardcoded in the compiler source code. I think those link options should be passed in the configuration file, they are as needed as the location of the runtime library, so for me makes perfect sense to move any runtime-specific switches for the linker to the config file to make the compiler as agnostic as possible concerning the runtime implementation details. This also enables (or at least facilitates) having more than one runtime implementation. I know with the Tango history this might sound like a very bad idea, but I think is not, as long as you use the same API/ABI all should be good with the rest of Phobos, no need for incompatibilities, and making easier to replace the runtime would encourage having new and experimental runtimes (for example, playing with new garbage collector implementations that might need different link options).

So I still think the most reasonable solution for this issue is to make DMD honour the order of the link options when passed to the linker and to move any runtime-specific options to the config file.

What do you think about that?

@andralex
Copy link
Member

andralex commented Oct 6, 2012

@llucax without being an expert in the matters I can say your case sounds reasonable

@llucax
Copy link
Contributor Author

llucax commented Oct 6, 2012

FYI, here is a related discussion in the NG:

http://forum.dlang.org/thread/mailman.1605.1334108859.4860.digitalmars-d@puremagic.com

@llucax
Copy link
Contributor Author

llucax commented Oct 6, 2012

Unfortunately my proposed solution doesn't help to fix the problem with libcurl entirely. In that case you either have to unconditionally link againt libcurl adding the option to the config file, just in case somebody uses the curl bindings, or you still need the ugly -sL I originally proposed in this pull request. :(

@yebblies
Copy link
Member

yebblies commented Oct 6, 2012

@llucax Yes, that sounds right. It makes a lot of sense to be able to configure the way the runtime is linked in the configuration file.

The problem is I need to pass -ldl to the linker, but for some reason it
has to be passed after the standard library.

I can explain this. Libraries are a collection of object files, and when the linker encounters them it searches for all currently undefined symbols and adds the object files that contain them to the current build.
With -ldl before -ldruntime (or whatever it is) on the command line, it goes something like this:

  • Load program object files and resolve every symbol it can
  • Load libdl and search for any symbols it defines that are needed (will find none)
  • Load druntime and add all the unresolved symbols from that
  • Finish linking and complain that those symbols are unresolved.

Being able to specify the linker flags for the runtime as a blob is the correct solution.

Unfortunately my proposed solution doesn't help to fix the problem with libcurl entirely. In that case you either have to unconditionally link againt libcurl adding the option to the config file, just in case somebody uses the curl bindings, or you still need the ugly -sL I originally proposed in this pull request. :(

It actually does. We just need to be able to specify the link commands for phobos too.

@alexrp
Copy link
Member

alexrp commented Oct 6, 2012

Regarding libdl, see #941 which we're going to have to merge sooner or later either way.

@llucax
Copy link
Contributor Author

llucax commented Oct 6, 2012

@yebblies thanks for the clarification about libraries order when linking and about fixing the libcurl problem, I know unconditionally adding all libraries to the linker options fixes it but it might end up adding unnecessary dependencies on programs (unless the linked is smart enough not to link libraries that are not really used) or even worse, you'll get a link error if you don't have libcurl (which might happen if you're compiling programs that actually don't use curl and you don't want to install curl). I really don't know what's best really, if requiring the users that want to use curl to manually add -L-lcurl or making all users, even the ones that don't care about curl at all, install libcurl and get all their programs linked against it.

@alexrp I think adding -ldl is just a patch, I think it would be best to come up with a more general solution so we don't need to change the compiler source code each time the runtime implementation needs different linker options.

@yebblies
Copy link
Member

yebblies commented Oct 6, 2012

The linker will not pull in object files from a library unless they contain symbols that were undefined, so if a program linked correctly without -lcurl, then it should produce an identical binary with -lcurl added.

Adding -lcurl to the linker command line for all programs will behave correctly (not add anything unless curl is used), but will cause an error if curl is not installed. It's possible this could be worked around by having a stub libcurl.lib somewhere where it will be found only if the real one isn't installed. Or we could just ship libcurl with phobos.

@llucax
Copy link
Contributor Author

llucax commented Oct 6, 2012

OK, I think making users installl libcurl (or workaround it if they really don't want to install it, which should only imply changing the dmd.conf) is a small enough price to pay. I think shipping libcurl with phobos doesn't make any sense, at least in .debs, which should just add a dependency to libcurl and that's it.

I'm still willing to rewrite the patch to do what we discussed here but I still prefer to have some kind of green light from @WalterBright before doing so.

@MartinNowak
Copy link
Member

I don't like the idea of adding an -sL switch, because the libraries one needs to append to the end are dependencies of phobos and druntime, i.e. we're failing to link in all dependencies.
Putting the link flags into the config file is a good idea, as it simplifies platform specific configuration.
So I'd propose to find a sensible name for an environment variable and if it's set use that value instead of the hardcoded stuff. Then we could deprecate the hardcoded list as well as the -debugliband the -defaultlib switch.

@llucax
Copy link
Contributor Author

llucax commented Oct 19, 2012

I like deprecating the -debuglib and -defaultlib switches too, but don't think it's possible unless we add some flag to add linker parameters just for debug/no-debug builds. Right now DMD uses -debuglib library if -g or -gc is passed, and -defaultlib otherwise.

I'm not sure how useful is that though, many other libraries you link to will probably not have debug symbols and I'm not sure how bad you want to see the standard lib guts normally. Seems like a niche enough case to stop supporting, maybe (or if it's supposed to be that useful, support it in a more general way).

What I don't understand is why would you use a specific environment variable for that? It should be enough if DMD keep the order of the -L parameters when calling the linker (including the order of -defaultlib and -debuglib).

@MartinNowak
Copy link
Member

What I don't understand is why would you use a specific environment variable for that? It should be enough if DMD keep the order of the -L parameters when calling the linker (including the order of -defaultlib and -debuglib).

Well we need a way to inject defaults.

@llucax
Copy link
Contributor Author

llucax commented Oct 20, 2012

What kind of defaults? Examples?

@MartinNowak
Copy link
Member

What kind of defaults?

What I thought of was basically to move the hardcoded flags into an environment variable (LDPHOBOS or so). That way platform specifics can be maintained through dmd.conf but are still configurable, e.g. in a Makefile.

@llucax
Copy link
Contributor Author

llucax commented Oct 20, 2012

The problem now is not the command line options, so moving that to an environment variable doesn't really help. The problem is (besides the hardcoded flags which is the easiest to change, we just have to remove them) the order of -defaultlib/-debuglib and -L is fixed. You cannot pass linker flags before -debuglib/-defaultlib.

Then I still don't understand why use environment variables (you can set default flags in dmd.conf and Makefiles too), and why using something special for the stdlib (if it's just adding libraries to link to and some other linker flags that are all settable using the current -L flag), except for the case of when compiling with -g (which actually uses -debuglib instead of -defaultlib)? Did you read my other comments entirely and carefully? I really don't see any of my questions answered in your comments.

@MartinNowak
Copy link
Member

You cannot pass linker flags before -debuglib/-defaultlib.

It's the other way round you can only prepend flags.
Every flag following the runtime library (e.g. libphobos2) is only there to resolve dependencies of the runtime itself and the build type (executable, shared lib, static lib). You shouldn't have to change this, but I think it might helpful to make this easily configurable, e.g. to use a different runtime.
You might want to check gcc -v which needs to prepend and append link flags.

@llucax
Copy link
Contributor Author

llucax commented Oct 21, 2012

It's the other way round you can only prepend flags.

Yes, I meant after :) Doesn't matter, the thing is, the order is fixed.

Every flag following the runtime library (e.g. libphobos2) is only there to resolve dependencies of the runtime itself and the build type (executable, shared lib, static lib). You shouldn't have to change this, but I think it might helpful to make this easily configurable, e.g. to use a different runtime.

Yes, we are both saying the same here, we completely agree the whole point of this discussion is allowing to easily replace the runtime/standard library.

You might want to check gcc -v which needs to prepend and append link flags.

I'm not following this.

Anyway, I think it's better to follow this in the bug report. What seems to be clear is that this pull request will be closed because is outdated and not the ideal solution, so all this discussion will be lost. I thought of a solution that's simple, backwards compatible, and works the same way DMD already does in terms of environment variables and switches.

http://d.puremagic.com/issues/show_bug.cgi?id=7044

braddr pushed a commit to braddr/dmd that referenced this pull request Oct 22, 2012
A few improvements to BitArray docs.
@WalterBright
Copy link
Member

See comment 4 at

http://d.puremagic.com/issues/show_bug.cgi?id=7044

Followups should go there.

@llucax
Copy link
Contributor Author

llucax commented Oct 22, 2012

I'll just close this pull request, which will be clearly not merged, let's follow the discussion at bugzilla and when we agree on a solution I will make a new patch and pull request.

@llucax llucax closed this Oct 22, 2012
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.

8 participants