[PATCH] D76742: [lld] Add basic symbol table output

[Kellie Medlin via Phabricator via llvm-commits]

Ktwu marked 7 inline comments as done.
Ktwu added inline comments.


================
Comment at: lld/MachO/Writer.cpp:336
         addr += isec->getSize();
+        isec->n_sect = ++n_sect;
       }
----------------
smeenai wrote:
> Hmm. This is gonna be incorrect for multiple sections of the same name, which should get merged together. Section merging is currently broken, as you pointed out.
> 
> I think we need an OutputSection class, similar to OutputSection in LLD ELF, to handle section merging. We have OutputSegments, but the sections in a segment themselves need merging too. That's also a much better place for the `addr` (which is the address in the *output*, not the address from the input) and `n_sect` fields to go.
That makes sense, although I'm afraid any logic I write up for merging probably won't be correct. I'll try referencing what ELF's stuff does.


================
Comment at: lld/test/MachO/symtab_basic.s:15
+# CHECK-NEXT:     ]
+# CHECK-NEXT:     Value:
+# CHECK-NEXT:   }
----------------
smeenai wrote:
> I think we should check the values, to make sure they're written correctly. They should be consistent (they'll depend on the base address, but that should be fixed).
I didn't put down a value because I didn't know, at this stage, how to tell if a value is correct or not. Is there another command I can invoke to see if these values are sane (like if readobj uses the value and hopefully doesn't crash, for example)?


================
Comment at: lld/test/MachO/symtab_basic.s:40
+.text
+.global bar
+.global foo
----------------
int3 wrote:
> maybe put one of these in a different section like `.data`?
Yup, will try that!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D76742/new/

https://reviews.llvm.org/D76742