I agree that that's perhaps better to omit an error message as >> and 532 more references if it is too large, but I don't know how large it can be. Can this be hundreds of lines?

Does that make a visible difference?

I think that the three patches make sense only when they are combined, so could you merge them as a single patch?

LGTM

LGTM

Yes, let's land this. LGTM

LGTM

LGTM

• updated the man page

• rebased
• instead of storing a tail hash, directly store shard-id to SectionPieces

• update comment

• rename the option --undefined-glob.

In D63244#1541879, @jyknight wrote:

Actually linker scripts do support metacharacter escaping (although, it seems lld does not support this correctly, yet).

E.g., this should match only a function named literally "?"

VER {
  global:
   "?";
};

While this matches any one-letter function:

VER {
  global:
   ?;
};

So perhaps this functionality should be given its own command-line argument, e.g. --undefined-glob, so as to not cause any such confusion.

LGTM with the following changes.

• factor out common code and unify two functions into one

• address review comments

LGTM

Yes, we have an internal user who has a proprietary tool to process a -Map output. As to the difference between bfd and gold, they don't look too big, so perhaps most tools can consume both. In this output, I modeled gold.

LGTM

LGTM

LGTM

LGTM

LGTM

Oh sorry I already reverted this. Once you fix the bot, feel free to resubmit (but please watch the bot after that)

Let me revert this change.

I can see why you created a new library, but it still look a bit odd that the actual library code is less than 40 lines. Do you think adding this to Object make sense? This is not really Object-related, but I guess adding a small amount of code there wouldn't harm anyone.

LGTM

LGTM

LGTM

LGTM

How did you verify the correctness of this patch? Did you set up a virtual machine? If that's easy to do, I'd like to set it up too.

LGTM

Got it. I'll commit this patch on behalf of you.

Do you have a commit access?

LGTM

I'm not particularly worried about the performance impact of doubly-expanding response files. If it is proved to be too slow, we need to do something, but until then I wouldn't optimize it.

In D63024#1535918, @mstorsjo wrote:

In D63024#1535891, @ruiu wrote:

Do you have to support --rsp-quoting for the use case that you want to support?

Not for my use case, no. But Reid wanted to move all expansion of response files out from the individual linkers here, to avoid reading (and allocating memory for) the (potentially large) response file twice. And in that case, we must handle --rsp-quoting perfectly here.

I was not trying to force you to use lld in some particular way. I was just trying to help you by suggesting one solution that might work for some use cases, and I didn't know that that wouldn't work for you. If it doesn't work for you, you can continue using lld as a library as you are already doing. We support that as well, and a bug like this should be fixed.

LGTM

In D63042#1535899, @ruiu wrote:

LGTM

Yes, I suspect that there are other globals that are not reset between multiple executions of this linker.

As to how to invoke the linker as a separate process, I wonder if you can run your compiler as a new subprocess of the compiler. If you can do that, you can invoke the linker with a special option to call lld::link immediately on process startup, which effectively transforms your compiler to a linker. This way, you still have to statically link lld to your compiler, but you can run the linker as a separate child process, which is generally preferred because bad thing like this or unknown bugs in the linker doesn't harm your compiler.

Yes, I suspect that there are other globals that are not reset between multiple executions of this linker.

Do you have to support --rsp-quoting for the use case that you want to support? I believe if you are using MinGW, you are always using Unix-style quoting in response files, so if the following two conditions are met, you can conclude that a user is trying to use mingw driver, can't you?

Was my previous comment correct? (I'm not suggesting any change at the moment but I'd like to make sure that we are on the same page.)

• fix tests

LGTM

LGTM

It generally looking good, and I don't have further comments. George, are you happy with this?

• rebase so that the patch does not diverge too much from the trunk

In D62976#1533960, @mstorsjo wrote:

In D62976#1533841, @ruiu wrote:

I guess that my previous comment addresses the issue?

Yes, probably - thanks!

Since we fetch objects iteratively in the COFF linker, we probably need to wait until the normal link has converged, then fetch lazy objs for -u, and then keep iterating until it converges again?

Does this mean that we previously emit a dynamic relocation for a weak undefined symbol that couldn't have been resolved at link time, and this patch changes the behavior so that weak undefined symbols must be resolved to some symbol at link time. Am I correct?

I think jhenderson's LGTM is enough but in case you need one more, LGTM.

Thank you for doing this! I can clearly see that this feature is very useful for defense-in-depth.

LGTM

I guess that my previous comment addresses the issue?

LGTM

Our algorithm for the ELF -u option is as follows:

LGTM

Do you think you can add a test?

LGTM

I'm not sure if I'm a right person to make a decision whether adding a new MRI command is OK or not, but looks like this new command makes sense, and no one seems to be opposing, so LGTM.

Looks like this patch doesn't still fix the issue that rupprecht pointed out. If an MRI script looks like this

LGTM

LGTM

LGTM

LGTM

LGTM

This is LGTM, but I believe you will have to implement -u in a more correct way. IIRC, I implemented -u as -require-defined in the ELF linker first. It seemed to be working fine until we found something weird, investigated it and then found that that was caused by an incorrect implementation of -u. So, if it is not too hard, you probably should implement it in the right way.

LGTM

What is a motivation to support the old-style PLT? If a program really expects that the PLT is in a writable segment, something is wrong with the program...

LGTM