Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[lld][COFF][NFC] Factor out exception table sorting. #72518

Merged
merged 1 commit into from
Nov 17, 2023

Conversation

cjacek
Copy link
Contributor

@cjacek cjacek commented Nov 16, 2023

This is a preparation for ARM64EC support, which needs to sort both ARM and x86_64 tables separately.

@llvmbot
Copy link
Collaborator

llvmbot commented Nov 16, 2023

@llvm/pr-subscribers-lld-coff

@llvm/pr-subscribers-lld

Author: Jacek Caban (cjacek)

Changes

This is a preparation for ARM64EC support, which needs to sort both ARM and x86_64 tables separately.


Full diff: https://github.com/llvm/llvm-project/pull/72518.diff

1 Files Affected:

  • (modified) lld/COFF/Writer.cpp (+33-24)
diff --git a/lld/COFF/Writer.cpp b/lld/COFF/Writer.cpp
index 0477c094c0ac82c..76c9b7604021f17 100644
--- a/lld/COFF/Writer.cpp
+++ b/lld/COFF/Writer.cpp
@@ -247,7 +247,8 @@ class Writer {
   void writeBuildId();
   void writePEChecksum();
   void sortSections();
-  void sortExceptionTable();
+  template <typename T> void sortExceptionTable(Chunk *first, Chunk *last);
+  void sortExceptionTables();
   void sortCRTSectionChunks(std::vector<Chunk *> &chunks);
   void addSyntheticIdata();
   void sortBySectionOrder(std::vector<Chunk *> &chunks);
@@ -751,7 +752,7 @@ void Writer::run() {
     }
     writeSections();
     prepareLoadConfig();
-    sortExceptionTable();
+    sortExceptionTables();
 
     // Fix up the alignment in the TLS Directory's characteristic field,
     // if a specific alignment value is needed
@@ -2164,41 +2165,49 @@ void Writer::writeBuildId() {
 }
 
 // Sort .pdata section contents according to PE/COFF spec 5.5.
-void Writer::sortExceptionTable() {
-  if (!firstPdata)
+template <typename T>
+void Writer::sortExceptionTable(Chunk *first, Chunk *last) {
+  if (!first)
     return;
-  llvm::TimeTraceScope timeScope("Sort exception table");
   // We assume .pdata contains function table entries only.
+
   auto bufAddr = [&](Chunk *c) {
     OutputSection *os = ctx.getOutputSection(c);
     return buffer->getBufferStart() + os->getFileOff() + c->getRVA() -
            os->getRVA();
   };
-  uint8_t *begin = bufAddr(firstPdata);
-  uint8_t *end = bufAddr(lastPdata) + lastPdata->getSize();
+  uint8_t *begin = bufAddr(first);
+  uint8_t *end = bufAddr(last) + last->getSize();
+  if ((end - begin) % sizeof(T) != 0) {
+    fatal("unexpected .pdata size: " + Twine(end - begin) +
+          " is not a multiple of " + Twine(sizeof(T)));
+  }
+
+  parallelSort(MutableArrayRef<T>((T *)begin, (T *)end),
+               [](const T &a, const T &b) { return a.begin < b.begin; });
+}
+
+// Sort .pdata section contents according to PE/COFF spec 5.5.
+void Writer::sortExceptionTables() {
+  llvm::TimeTraceScope timeScope("Sort exception table");
+
+  struct EntryX64 {
+    ulittle32_t begin, end, unwind;
+  };
+  struct EntryArm {
+    ulittle32_t begin, unwind;
+  };
+
   if (ctx.config.machine == AMD64) {
-    struct Entry { ulittle32_t begin, end, unwind; };
-    if ((end - begin) % sizeof(Entry) != 0) {
-      fatal("unexpected .pdata size: " + Twine(end - begin) +
-            " is not a multiple of " + Twine(sizeof(Entry)));
-    }
-    parallelSort(
-        MutableArrayRef<Entry>((Entry *)begin, (Entry *)end),
-        [](const Entry &a, const Entry &b) { return a.begin < b.begin; });
+    sortExceptionTable<EntryX64>(firstPdata, lastPdata);
     return;
   }
   if (ctx.config.machine == ARMNT || ctx.config.machine == ARM64) {
-    struct Entry { ulittle32_t begin, unwind; };
-    if ((end - begin) % sizeof(Entry) != 0) {
-      fatal("unexpected .pdata size: " + Twine(end - begin) +
-            " is not a multiple of " + Twine(sizeof(Entry)));
-    }
-    parallelSort(
-        MutableArrayRef<Entry>((Entry *)begin, (Entry *)end),
-        [](const Entry &a, const Entry &b) { return a.begin < b.begin; });
+    sortExceptionTable<EntryArm>(firstPdata, lastPdata);
     return;
   }
-  lld::errs() << "warning: don't know how to handle .pdata.\n";
+  if (firstPdata)
+    lld::errs() << "warning: don't know how to handle .pdata.\n";
 }
 
 // The CRT section contains, among other things, the array of function

@llvmbot
Copy link
Collaborator

llvmbot commented Nov 16, 2023

@llvm/pr-subscribers-platform-windows

Author: Jacek Caban (cjacek)

Changes

This is a preparation for ARM64EC support, which needs to sort both ARM and x86_64 tables separately.


Full diff: https://github.com/llvm/llvm-project/pull/72518.diff

1 Files Affected:

  • (modified) lld/COFF/Writer.cpp (+33-24)
diff --git a/lld/COFF/Writer.cpp b/lld/COFF/Writer.cpp
index 0477c094c0ac82c..76c9b7604021f17 100644
--- a/lld/COFF/Writer.cpp
+++ b/lld/COFF/Writer.cpp
@@ -247,7 +247,8 @@ class Writer {
   void writeBuildId();
   void writePEChecksum();
   void sortSections();
-  void sortExceptionTable();
+  template <typename T> void sortExceptionTable(Chunk *first, Chunk *last);
+  void sortExceptionTables();
   void sortCRTSectionChunks(std::vector<Chunk *> &chunks);
   void addSyntheticIdata();
   void sortBySectionOrder(std::vector<Chunk *> &chunks);
@@ -751,7 +752,7 @@ void Writer::run() {
     }
     writeSections();
     prepareLoadConfig();
-    sortExceptionTable();
+    sortExceptionTables();
 
     // Fix up the alignment in the TLS Directory's characteristic field,
     // if a specific alignment value is needed
@@ -2164,41 +2165,49 @@ void Writer::writeBuildId() {
 }
 
 // Sort .pdata section contents according to PE/COFF spec 5.5.
-void Writer::sortExceptionTable() {
-  if (!firstPdata)
+template <typename T>
+void Writer::sortExceptionTable(Chunk *first, Chunk *last) {
+  if (!first)
     return;
-  llvm::TimeTraceScope timeScope("Sort exception table");
   // We assume .pdata contains function table entries only.
+
   auto bufAddr = [&](Chunk *c) {
     OutputSection *os = ctx.getOutputSection(c);
     return buffer->getBufferStart() + os->getFileOff() + c->getRVA() -
            os->getRVA();
   };
-  uint8_t *begin = bufAddr(firstPdata);
-  uint8_t *end = bufAddr(lastPdata) + lastPdata->getSize();
+  uint8_t *begin = bufAddr(first);
+  uint8_t *end = bufAddr(last) + last->getSize();
+  if ((end - begin) % sizeof(T) != 0) {
+    fatal("unexpected .pdata size: " + Twine(end - begin) +
+          " is not a multiple of " + Twine(sizeof(T)));
+  }
+
+  parallelSort(MutableArrayRef<T>((T *)begin, (T *)end),
+               [](const T &a, const T &b) { return a.begin < b.begin; });
+}
+
+// Sort .pdata section contents according to PE/COFF spec 5.5.
+void Writer::sortExceptionTables() {
+  llvm::TimeTraceScope timeScope("Sort exception table");
+
+  struct EntryX64 {
+    ulittle32_t begin, end, unwind;
+  };
+  struct EntryArm {
+    ulittle32_t begin, unwind;
+  };
+
   if (ctx.config.machine == AMD64) {
-    struct Entry { ulittle32_t begin, end, unwind; };
-    if ((end - begin) % sizeof(Entry) != 0) {
-      fatal("unexpected .pdata size: " + Twine(end - begin) +
-            " is not a multiple of " + Twine(sizeof(Entry)));
-    }
-    parallelSort(
-        MutableArrayRef<Entry>((Entry *)begin, (Entry *)end),
-        [](const Entry &a, const Entry &b) { return a.begin < b.begin; });
+    sortExceptionTable<EntryX64>(firstPdata, lastPdata);
     return;
   }
   if (ctx.config.machine == ARMNT || ctx.config.machine == ARM64) {
-    struct Entry { ulittle32_t begin, unwind; };
-    if ((end - begin) % sizeof(Entry) != 0) {
-      fatal("unexpected .pdata size: " + Twine(end - begin) +
-            " is not a multiple of " + Twine(sizeof(Entry)));
-    }
-    parallelSort(
-        MutableArrayRef<Entry>((Entry *)begin, (Entry *)end),
-        [](const Entry &a, const Entry &b) { return a.begin < b.begin; });
+    sortExceptionTable<EntryArm>(firstPdata, lastPdata);
     return;
   }
-  lld::errs() << "warning: don't know how to handle .pdata.\n";
+  if (firstPdata)
+    lld::errs() << "warning: don't know how to handle .pdata.\n";
 }
 
 // The CRT section contains, among other things, the array of function

lld/COFF/Writer.cpp Outdated Show resolved Hide resolved
lld/COFF/Writer.cpp Outdated Show resolved Hide resolved
@cjacek cjacek merged commit ec42d54 into llvm:main Nov 17, 2023
2 of 3 checks passed
@cjacek
Copy link
Contributor Author

cjacek commented Nov 17, 2023

Thanks for reviews. I merged with a small fixup to check firstPdata earlier. Previous version was fine in practice, but I think it could upset sanitizers because of uninitialized lastPdata being passed.

@cjacek cjacek deleted the lld-sort-pdata branch November 17, 2023 18:42
sr-tream pushed a commit to sr-tream/llvm-project that referenced this pull request Nov 20, 2023
This is a preparation for ARM64EC support, which needs to sort both ARM
and x86_64 tables separately.
zahiraam pushed a commit to zahiraam/llvm-project that referenced this pull request Nov 20, 2023
This is a preparation for ARM64EC support, which needs to sort both ARM
and x86_64 tables separately.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants