[PATCH] D89795: [jitlink][ELF] Allocate SHN_COMMON symbols as uninitialized blocks in __common

[Stefan Gränitz via Phabricator via llvm-commits]

sgraenitz created this revision.
sgraenitz added reviewers: lhames, jaredwy, anarazel, drmeister.
Herald added a subscriber: hiraditya.
Herald added a project: LLVM.
sgraenitz requested review of this revision.

Symbols with special section index SHN_COMMON (0xfff2) haven't been handled so far and caused an invalid section error.

This is a more or less straightforward use of the code commented out at the end of the function. I checked with the ELF spec, that the symbol value gives the alignment. However, the spec has an extra paragraph for special handling in dynamic linkers. I am not sure how much it's relevant here:

  When the dynamic linker encounters a reference to a symbol that resolves to a definition of type STT_COMMON, it may (but is not required to) change its symbol resolution rules as follows: instead of binding the reference to the first symbol found with the given name, the dynamic linker searches for the first symbol with that name with type other than STT_COMMON. If no such symbol is found, it looks for the STT_COMMON definition of that name that has the largest size. 

What do you think?


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D89795

Files:
  llvm/lib/ExecutionEngine/JITLink/ELF_x86_64.cpp


Index: llvm/lib/ExecutionEngine/JITLink/ELF_x86_64.cpp
===================================================================
--- llvm/lib/ExecutionEngine/JITLink/ELF_x86_64.cpp
+++ llvm/lib/ExecutionEngine/JITLink/ELF_x86_64.cpp
@@ -511,6 +511,15 @@
         // I am not sure on If this is going to hold as an invariant. Revisit.
         if (!Name)
           return Name.takeError();
+
+        // Allocate uninitialized common blocks.
+        if (SymRef.isCommon()) {
+          // st_value holds alignment constraints for SHN_COMMON symbols
+          G->addCommonSymbol(*Name, Scope::Default, getCommonSection(), 0,
+                             SymRef.st_size, SymRef.getValue(), false);
+          continue;
+        }
+
         // TODO: weak and hidden
         if (SymRef.isExternal())
           bindings = {Linkage::Strong, Scope::Default};
@@ -551,18 +560,12 @@
           JITSymbolTable[SymbolIndex] = &S;
         }
 
-        //  }
         // TODO: The following has to be implmented.
         // leaving commented out to save time for future patchs
         /*
           G->addAbsoluteSymbol(*Name, SymRef.getValue(), SymRef.st_size,
           Linkage::Strong, Scope::Default, false);
-
-          if(SymRef.isCommon()) {
-            G->addCommonSymbol(*Name, Scope::Default, getCommonSection(), 0, 0,
-          SymRef.getValue(), false);
-          }
-  */
+        */
       }
     }
     return Error::success();


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D89795.299361.patch
Type: text/x-patch
Size: 1437 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20201020/fbf47f97/attachment.bin>