This is an archive of the discontinued LLVM Phabricator instance.

[lld-macho][reland] Add basic symbol table output
ClosedPublic

Authored by int3 on Apr 28 2020, 4:02 PM.

Details

Summary

This diff implements basic support for writing a symbol table.

Attributes are loosely supported for extern symbols and not at all for
other types.

Initial version by Kellie Medlin <kelliem@fb.com>

Original diff got reverted due to UBSAN erroring over unaligned writes.
That has been fixed in the current diff with the following changes:

diff --git a/lld/MachO/SyntheticSections.cpp b/lld/MachO/SyntheticSections.cpp
--- a/lld/MachO/SyntheticSections.cpp
+++ b/lld/MachO/SyntheticSections.cpp
@@ -133,6 +133,9 @@ SymtabSection::SymtabSection(StringTableSection &stringTableSection)
     : stringTableSection(stringTableSection) {
   segname = segment_names::linkEdit;
   name = section_names::symbolTable;
+  // TODO: When we introduce the SyntheticSections superclass, we should make
+  // all synthetic sections aligned to WordSize by default.
+  align = WordSize;
 }

 size_t SymtabSection::getSize() const {
diff --git a/lld/MachO/Writer.cpp b/lld/MachO/Writer.cpp
--- a/lld/MachO/Writer.cpp
+++ b/lld/MachO/Writer.cpp
@@ -371,6 +371,7 @@ void Writer::assignAddresses(OutputSegment *seg) {
     ArrayRef<InputSection *> sections = p.second;
     for (InputSection *isec : sections) {
       addr = alignTo(addr, isec->align);
+      // We must align the file offsets too to avoid misaligned writes of
+      // structs.
+      fileOff = alignTo(fileOff, isec->align);
       isec->addr = addr;
       addr += isec->getSize();
       fileOff += isec->getFileSize();
@@ -396,6 +397,7 @@ void Writer::writeSections() {
     uint64_t fileOff = seg->fileOff;
     for (auto &sect : seg->getSections()) {
       for (InputSection *isec : sect.second) {
+        fileOff = alignTo(fileOff, isec->align);
         isec->writeTo(buf + fileOff);
         fileOff += isec->getFileSize();
       }

I don't think it's easy to write a test for alignment (that doesn't
involve brittly hard-coding file offsets), so there isn't one... but
UBSAN builds pass now.

Diff Detail

Event Timeline

int3 created this revision.Apr 28 2020, 4:02 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 28 2020, 4:02 PM
int3 updated this revision to Diff 260789.Apr 28 2020, 4:06 PM

add a comment

int3 edited the summary of this revision. (Show Details)Apr 28 2020, 4:07 PM
Harbormaster completed remote builds in B55051: Diff 260789.
This revision was not accepted when it landed; it landed in state Needs Review.Apr 28 2020, 5:18 PM
This revision was automatically updated to reflect the committed changes.

(this was a recommit, so I just landed it without an explicit accept)