<div dir="ltr">Hi David,<div><br></div><div>Thanks for working on a patch, I'll take a look but it may take me a couple days to get to it.</div><div><br></div><div> - Daniel</div></div><div class="gmail_extra"><br><br>
<div class="gmail_quote">On Wed, Mar 13, 2013 at 1:48 PM, David Fang <span dir="ltr"><<a href="mailto:fang@csl.cornell.edu" target="_blank">fang@csl.cornell.edu</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Daniel,<br>
        The resulting LLVM test diffs with my patch are shown here:<br>
<a href="http://llvm.org/bugs/show_bug.cgi?id=14636#c38" target="_blank">http://llvm.org/bugs/show_bug.<u></u>cgi?id=14636#c38</a><br>
<br>
2 tests "failed" in MC/MachO: indirect-symbols.s and symbol-indirect.s<br>
No other LLVM/clang tests regressed (Targets: PPC,X86,ARM).<br>
<br>
The differences are that symbols are re-ordered, which accomplishes the objective of the patch. There are FIXME comments that require a maintainer's scrutiny.<br>
Could you review these differences that result from my patch?<br>
The patch as-is now is in proof-of-concept form.  I'd be happy to revise as you suggest.<br>
If this patch benefits all Mach-O architectures (getting closer to usable integrated assembly), I'd like to get this into trunk.<span class="HOEnZb"><font color="#888888"><br>
<br>
Fang</font></span><div class="HOEnZb"><div class="h5"><br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Michael, Daniel,<br>
        The attached patch I wrote works for me so far on my small test cases: llvm/clang's object file matches the indirect symbol ordering of the /usr/bin/as-assembled object's (tested on my powerpc-darwin8 branch).<br>

Michael, could you try it out on your own test cases?<br>
Daniel, could you kindly comment and review?  Just the technical merits, not style.<br>
         I've also attached the same patch to bug 14636.<br>
<br>
Thanks for taking a look.<br>
<br>
Fang<br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
 Daniel,<br>
   Some quick questions about your proposed recipe:<br>
<br>
> >   Ok, here is what I think the right fix is. Instead of creating the<br>
> >   IndirectSymBase mapping we use to associate sections with their<br>
> >   indirect offset start, in BindIndirectSymbols() we should:<br>
> > > >   1. Add a simple container struct (lets say > >   MachSectionIndirectSymbols)<br>
> >   for tracking the per-section list of indirect symbols. It will keep<br>
> >   the list of symbols in the section and the index of the first > >   indirect<br>
> >   symbol in the section.<br>
<br>
 Like a map<section*, vector<indirect_symbol> >?<br>
<br>
 Instead of keeping track of the first indirect symbol index, could we just<br>
 as easily compute the index offsets later during the second pass:<br>
<br>
 offset = 0;<br>
 for each section S (in order)<br>
   record offset for section S<br>
   [optional] write vector of indirect symbols for S<br>
   offset += S.num_indirect_symbols();<br>
 end for<br>
<br>
 I was able to fix the reserve1 field by doing something like this (bug<br>
 14636, comments 30-33), but I only later discovered that the indirect<br>
 symbols needed to be reordered.<br>
<br>
> >   2. Keep a mapping from sections to the above type.<br>
> >   3. Add a SetVector to record the order of the sections that have<br>
> >   indirect symbols.<br>
<br>
 This I understand, I found SetVector and SmallSetVector in ADT.<br>
<br>
> >   4. During BindIndirectSymbols() we maintain the above information<br>
> >   (populating the MachSectionIndirectSymbols per-section symbol > >   arrays).<br>
<br>
 BindIndirectSymbols looks like it need not change, because it's already<br>
 operating on the assumption of ordered indirect symbols, right?<br>
<br>
> >   5. Once we have scanned all the symbols we make another pass over the<br>
> >   sections (in the order seen via indirect symbols) and assign the > >   start<br>
> >   indices.<br>
<br>
 as I outlined above (offset = start indices).<br>
<br>
> >   6. Update writing of the indirect symbol table to write in the same<br>
> >   order as traversed in #5.<br>
> > > >   Does that make sense? It's more work than your patch but it (a) > >   should<br>
> >   preserve binary compatibility with 'as' in situations where the<br>
> >   indirect symbols don't appear out of order w.r.t. the sections, (b) > >   it<br>
> >   makes somewhere more explicit the relationship between sections and<br>
> >   their list of symbols being in contiguous order.<br>
> > > >   - Daniel<br>
<br>
 Will this re-ordering be acceptable to all Mach-O back-ends?  x86, ARM,<br>
 PPC?<br>
<br>
 Fang<br>
<br>
<br>
</blockquote>
<br>
<br>
</blockquote>
<br>
-- <br>
David Fang<br>
<a href="http://www.csl.cornell.edu/~fang/" target="_blank">http://www.csl.cornell.edu/~<u></u>fang/</a><br>
<br>
</div></div></blockquote></div><br></div>