<div dir="ltr">On Thu, Aug 29, 2013 at 3:15 PM, <a href="mailto:kledzik@apple.com">kledzik@apple.com</a> <span dir="ltr"><<a href="mailto:kledzik@apple.com" target="_blank">kledzik@apple.com</a>></span> wrote:<br><div class="gmail_extra">
<div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><br>
Interesting feature.<br>
<br>
What happens if a.o has an undefine foo which says its fallback is bar, but in b.o an undefine for foo says its fallback is baz? Is that an error, or does each fallback apply to its original usage. The problem is that the Resolver is currently coalescing UndefinedAtoms based on their name. I does not know that fallbacks need to match too.<br>
</blockquote><div><br></div><div><span style="font-size:13px;font-family:arial,sans-serif">Good question. Such corner case behavior is not really defined in the MS </span><span style="font-size:13px;font-family:arial,sans-serif">COFF spec, so I tried to see how MS link.exe behaves. link.exe seems to f</span><span style="font-size:13px;font-family:arial,sans-serif">ollow the latter; it's allowed that the symbols with the same name in </span><span style="font-size:13px;font-family:arial,sans-serif">different object files to have different fallback symbols, and each </span><span style="font-size:13px;font-family:arial,sans-serif">fallback apply to its original usage. Because undefined atoms are as you </span><span style="font-size:13px;font-family:arial,sans-serif">said coalesced by name, this patch is not strictly compatible with </span><span style="font-size:13px;font-family:arial,sans-serif">link.exe. I'd think is okay in practice, as the feature is rarely used, </span><span style="font-size:13px;font-family:arial,sans-serif">though. (The support of this feature is needed because the C++ standard </span><span style="font-size:13px;font-family:arial,sans-serif">library uses it, but regular programs usually don't.)</span> </div>
<div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
Another variant is to have Undefined return a alternate name as a string, rather than returning a new UndefinedAtom object.<br>
<br>
Another approach is to use weak aliases. That is if a.o has an undefine for foo with a fallback of bar, that when parsing that .o file into atoms produces an UndefinedAtom for foo, but also a DefinedAtom with name=foo, weak (mergeAsWeak), hidden (scopeLinkageUnit), size=0, isAlias=true which aliases to bar. So, it any definition of foo does show up, it will override the weak alias. If not the weak alias will be used and change the reference to bar.<br>
</blockquote><div><br></div><span style="font-size:13px;font-family:arial,sans-serif">If any (real) definition of bar will show up, link will have to fail with </span><span style="font-size:13px;font-family:arial,sans-serif">an error message that bar was not found. That's different from weak symbol; </span><span style="font-size:13px;font-family:arial,sans-serif">if no real definition is found, the weak symbol will just be used without </span><span style="font-size:13px;font-family:arial,sans-serif">error. The implementation of isAlias support seems pretty immature. It </span><span style="font-size:13px;font-family:arial,sans-serif">really looked like a stub, and nothing has really been wired up to it..</span><br style="font-size:13px;font-family:arial,sans-serif">
<br style="font-size:13px;font-family:arial,sans-serif"><div><span style="font-size:13px;font-family:arial,sans-serif">Also, a weak symbol with internal linkage sounds odd and hard to imagine a </span><span style="font-size:13px;font-family:arial,sans-serif">correct semantics. Should it be coalesced away with the symbol in other </span><span style="font-size:13px;font-family:arial,sans-serif">file because it's weak, or shouldn't because it's static? Clang does not </span><span style="font-size:13px;font-family:arial,sans-serif">allow the combination of __attribute((weak)) and static, probably to avoid </span><span style="font-size:13px;font-family:arial,sans-serif">the issue.</span></div>
<div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<br>
I don't think the isAlias support is all wired up in lld. But weak aliases are something common in ELF. So would should get it working, and doing so may provide a way to implement this COFF feature.<br>
<br>
One issue is how this interacts with static libraries. The weak alias for foo make look like there is a definition, so any static archives will not be searched for foo. That seems different than what you want. I'm not sure what the gnu linker does about this with weak aliases. We may need a new resolver option as to how to handle this.<br>
<br>
<br>
================<br>
Comment at: lib/Core/Resolver.cpp:214<br>
@@ +213,3 @@<br>
<div class="im">+ // for COFF "weak external" symbol.<br>
+ const UndefinedAtom *fallbackUndefAtom = undefAtom->fallback();<br>
+ if (fallbackUndefAtom) {<br>
</div>----------------<br>
You need to re-test:<br>
if (!_symbolTable.isDefined(undefName)<br>
because searchLibraries() may have already found a definition.<br>
<br>
<br>
<a href="http://llvm-reviews.chandlerc.com/D1550" target="_blank">http://llvm-reviews.chandlerc.com/D1550</a><br>
</blockquote></div><br></div></div>