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][NFC] Factor out isCodeSection helper. #69193

Merged
merged 1 commit into from
Oct 16, 2023
Merged

Conversation

cjacek
Copy link
Contributor

@cjacek cjacek commented Oct 16, 2023

Needed for #69099.

@llvmbot
Copy link
Member

llvmbot commented Oct 16, 2023

@llvm/pr-subscribers-lld
@llvm/pr-subscribers-lld-coff

@llvm/pr-subscribers-platform-windows

Author: Jacek Caban (cjacek)

Changes

Needed for #69099.


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

5 Files Affected:

  • (modified) lld/COFF/Chunks.cpp (+2-2)
  • (modified) lld/COFF/Chunks.h (+2)
  • (modified) lld/COFF/Writer.cpp (+1-5)
  • (modified) lld/COFF/Writer.h (+6)
  • (added) lld/test/COFF/arm64ec-reloc.test (+37)
diff --git a/lld/COFF/Chunks.cpp b/lld/COFF/Chunks.cpp
index e17b64df869fe88..4e845afa8947a5f 100644
--- a/lld/COFF/Chunks.cpp
+++ b/lld/COFF/Chunks.cpp
@@ -437,7 +437,7 @@ void SectionChunk::applyRelocation(uint8_t *off,
   // Compute the RVA of the relocation for relative relocations.
   uint64_t p = rva + rel.VirtualAddress;
   uint64_t imageBase = file->ctx.config.imageBase;
-  switch (file->ctx.config.machine) {
+  switch (getMachine()) {
   case AMD64:
     applyRelX64(off, rel.Type, os, s, p, imageBase);
     break;
@@ -551,7 +551,7 @@ static uint8_t getBaserelType(const coff_relocation &rel,
 // Only called when base relocation is enabled.
 void SectionChunk::getBaserels(std::vector<Baserel> *res) {
   for (const coff_relocation &rel : getRelocs()) {
-    uint8_t ty = getBaserelType(rel, file->ctx.config.machine);
+    uint8_t ty = getBaserelType(rel, getMachine());
     if (ty == IMAGE_REL_BASED_ABSOLUTE)
       continue;
     Symbol *target = file->getSymbol(rel.SymbolTableIndex);
diff --git a/lld/COFF/Chunks.h b/lld/COFF/Chunks.h
index 3d605e6ab10c8c8..d14a258fc81e121 100644
--- a/lld/COFF/Chunks.h
+++ b/lld/COFF/Chunks.h
@@ -219,6 +219,8 @@ class SectionChunk final : public Chunk {
   ArrayRef<uint8_t> getContents() const;
   void writeTo(uint8_t *buf) const;
 
+  MachineTypes getMachine() const { return file->getMachineType(); }
+
   // Defend against unsorted relocations. This may be overly conservative.
   void sortRelocations();
 
diff --git a/lld/COFF/Writer.cpp b/lld/COFF/Writer.cpp
index 4f6c2a57f533505..d4f6ee6fde4952c 100644
--- a/lld/COFF/Writer.cpp
+++ b/lld/COFF/Writer.cpp
@@ -1403,11 +1403,7 @@ void Writer::assignAddresses() {
 
     // If /FUNCTIONPADMIN is used, functions are padded in order to create a
     // hotpatchable image.
-    const bool isCodeSection =
-        (sec->header.Characteristics & IMAGE_SCN_CNT_CODE) &&
-        (sec->header.Characteristics & IMAGE_SCN_MEM_READ) &&
-        (sec->header.Characteristics & IMAGE_SCN_MEM_EXECUTE);
-    uint32_t padding = isCodeSection ? config->functionPadMin : 0;
+    uint32_t padding = sec->isCodeSection() ? config->functionPadMin : 0;
 
     for (Chunk *c : sec->chunks) {
       if (padding && c->isHotPatchable())
diff --git a/lld/COFF/Writer.h b/lld/COFF/Writer.h
index 4a74aa7ada59d73..9004bb310d07305 100644
--- a/lld/COFF/Writer.h
+++ b/lld/COFF/Writer.h
@@ -64,6 +64,12 @@ class OutputSection {
   // Used only when the name is longer than 8 bytes.
   void setStringTableOff(uint32_t v) { stringTableOff = v; }
 
+  bool isCodeSection() const {
+    return (header.Characteristics & llvm::COFF::IMAGE_SCN_CNT_CODE) &&
+           (header.Characteristics & llvm::COFF::IMAGE_SCN_MEM_READ) &&
+           (header.Characteristics & llvm::COFF::IMAGE_SCN_MEM_EXECUTE);
+  }
+
   // N.B. The section index is one based.
   uint32_t sectionIndex = 0;
 
diff --git a/lld/test/COFF/arm64ec-reloc.test b/lld/test/COFF/arm64ec-reloc.test
new file mode 100644
index 000000000000000..3060891bfe02e87
--- /dev/null
+++ b/lld/test/COFF/arm64ec-reloc.test
@@ -0,0 +1,37 @@
+REQUIRES: aarch64, x86
+RUN: split-file %s %t.dir && cd %t.dir
+
+Link a mix of ARM64EC and x86_64 data and check that relocations work.
+
+RUN: llvm-mc -filetype=obj -triple=arm64ec-windows arm64ec-data-sym.s -o arm64ec-data-sym.obj
+RUN: llvm-mc -filetype=obj -triple=x86_64-windows x86_64-data-sym.s -o x86_64-data-sym.obj
+RUN: lld-link -out:test.dll -machine:arm64ec arm64ec-data-sym.obj x86_64-data-sym.obj -dll -noentry
+
+RUN: llvm-readobj --hex-dump=.data test.dll | FileCheck -check-prefix=ARM64EC-DATA %s
+ARM64EC-DATA: 0x180001000 00100080 01000000 08100080 01000000
+
+RUN: llvm-readobj --coff-basereloc test.dll | FileCheck -check-prefix=RELOCS %s
+RELOCS:      BaseReloc [
+RELOCS-NEXT:   Entry {
+RELOCS-NEXT:     Type: DIR64
+RELOCS-NEXT:     Address: 0x1000
+RELOCS-NEXT:   }
+RELOCS-NEXT:   Entry {
+RELOCS-NEXT:     Type: DIR64
+RELOCS-NEXT:     Address: 0x1008
+RELOCS-NEXT:   }
+RELOCS-NEXT: ]
+
+#--- arm64ec-data-sym.s
+        .data
+        .globl arm64ec_data_sym
+        .p2align 2, 0x0
+arm64ec_data_sym:
+        .xword arm64ec_data_sym
+
+#--- x86_64-data-sym.s
+        .data
+        .globl x86_64_data_sym
+        .p2align 2, 0x0
+x86_64_data_sym:
+        .quad x86_64_data_sym

Copy link
Member

@mstorsjo mstorsjo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The change itself LGTM (although this should be independent of #69098, I guess it's stacked on top of that just because of you wanting both included in #69099).

@cjacek cjacek merged commit f6f944e into llvm:main Oct 16, 2023
@cjacek
Copy link
Contributor Author

cjacek commented Oct 16, 2023

Oh, right, sorry. I wanted both for #69099, but I could submit just one commit here. Merged, thanks!

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.

3 participants