[Openmp-commits] [openmp] 0ab663d - [Libomptarget] Move ELF symbol extraction to the ELF utility (#74717)

via Openmp-commits openmp-commits at lists.llvm.org
Thu Dec 14 09:04:17 PST 2023


Author: Joseph Huber
Date: 2023-12-14T11:04:13-06:00
New Revision: 0ab663d2023a3187423d19ed8e7b631b7ace6168

URL: https://github.com/llvm/llvm-project/commit/0ab663d2023a3187423d19ed8e7b631b7ace6168
DIFF: https://github.com/llvm/llvm-project/commit/0ab663d2023a3187423d19ed8e7b631b7ace6168.diff

LOG: [Libomptarget] Move ELF symbol extraction to the ELF utility (#74717)

Summary:
We shouldn't have the format specific ELF handling in the generic plugin
manager. This patch moves that out of the implementation and into the
ELF utilities. This patch changes the SHT_NOBITS case to be a hard
error, which should be correct as the existing use already seemed to
return an error if the result was a null pointer.

This also uses a `const_cast`, which is bad practice. However,
rebuilding the `constness` of all of this would be a massive overhaul,
and this matches the previous behaviour (We would take a pointer to the
image that is most likely read-only in the ELF).

Added: 
    

Modified: 
    openmp/libomptarget/plugins-nextgen/common/include/GlobalHandler.h
    openmp/libomptarget/plugins-nextgen/common/src/GlobalHandler.cpp
    openmp/libomptarget/plugins-nextgen/common/src/Utils/ELF.cpp
    openmp/libomptarget/plugins-nextgen/common/src/Utils/ELF.h

Removed: 
    


################################################################################
diff  --git a/openmp/libomptarget/plugins-nextgen/common/include/GlobalHandler.h b/openmp/libomptarget/plugins-nextgen/common/include/GlobalHandler.h
index fa788c5e1d02b2..fa079ac9660ee0 100644
--- a/openmp/libomptarget/plugins-nextgen/common/include/GlobalHandler.h
+++ b/openmp/libomptarget/plugins-nextgen/common/include/GlobalHandler.h
@@ -92,12 +92,6 @@ class GenericGlobalHandlerTy {
   /// Map to store the ELF object files that have been loaded.
   llvm::DenseMap<int32_t, ELF64LEObjectFile> ELFObjectFiles;
 
-  /// Extract the global's information from the ELF image, section, and symbol.
-  virtual Error getGlobalMetadataFromELF(const DeviceImageTy &Image,
-                                         const ELF64LE::Sym &Symbol,
-                                         const ELF64LE::Shdr &Section,
-                                         GlobalTy &ImageGlobal);
-
   /// Actually move memory between host and device. See readGlobalFromDevice and
   /// writeGlobalToDevice for the interface description.
   Error moveGlobalBetweenDeviceAndHost(GenericDeviceTy &Device,

diff  --git a/openmp/libomptarget/plugins-nextgen/common/src/GlobalHandler.cpp b/openmp/libomptarget/plugins-nextgen/common/src/GlobalHandler.cpp
index 0a19148ca4ec6b..3a272e228c7dfe 100644
--- a/openmp/libomptarget/plugins-nextgen/common/src/GlobalHandler.cpp
+++ b/openmp/libomptarget/plugins-nextgen/common/src/GlobalHandler.cpp
@@ -16,10 +16,8 @@
 
 #include "Shared/Utils.h"
 
-#include "llvm/BinaryFormat/ELF.h"
 #include "llvm/Support/Error.h"
 
-#include <cstdint>
 #include <cstring>
 
 using namespace llvm;
@@ -52,27 +50,6 @@ GenericGlobalHandlerTy::getOrCreateELFObjectFile(const GenericDeviceTy &Device,
   return &Result.first->second;
 }
 
-Error GenericGlobalHandlerTy::getGlobalMetadataFromELF(
-    const DeviceImageTy &Image, const ELF64LE::Sym &Symbol,
-    const ELF64LE::Shdr &Section, GlobalTy &ImageGlobal) {
-
-  // The global's address is computed as the image begin + the ELF section
-  // offset + the ELF symbol value except for NOBITS sections that, as the name
-  // suggests, have no bits in the image. We still record the size and use
-  // nullptr to indicate there is no location.
-  if (Section.sh_type == ELF::SHT_NOBITS)
-    ImageGlobal.setPtr(nullptr);
-  else
-    ImageGlobal.setPtr(
-        advanceVoidPtr(Image.getStart(),
-                       Section.sh_offset - Section.sh_addr + Symbol.st_value));
-
-  // Set the global's size.
-  ImageGlobal.setSize(Symbol.st_size);
-
-  return Plugin::success();
-}
-
 Error GenericGlobalHandlerTy::moveGlobalBetweenDeviceAndHost(
     GenericDeviceTy &Device, DeviceImageTy &Image, const GlobalTy &HostGlobal,
     bool Device2Host) {
@@ -156,15 +133,18 @@ Error GenericGlobalHandlerTy::getGlobalMetadataFromImage(
     return Plugin::error("Failed to find global symbol '%s' in the ELF image",
                          ImageGlobal.getName().data());
 
+  auto AddrOrErr = utils::elf::getSymbolAddress(*ELFObj, **SymOrErr);
   // Get the section to which the symbol belongs.
-  auto SecOrErr = ELFObj->getELFFile().getSection((*SymOrErr)->st_shndx);
-  if (!SecOrErr)
-    return Plugin::error("Failed to get ELF section from global '%s': %s",
+  if (!AddrOrErr)
+    return Plugin::error("Failed to get ELF symbol from global '%s': %s",
                          ImageGlobal.getName().data(),
-                         toString(SecOrErr.takeError()).data());
+                         toString(AddrOrErr.takeError()).data());
 
   // Setup the global symbol's address and size.
-  return getGlobalMetadataFromELF(Image, **SymOrErr, **SecOrErr, ImageGlobal);
+  ImageGlobal.setPtr(const_cast<void *>(*AddrOrErr));
+  ImageGlobal.setSize((*SymOrErr)->st_size);
+
+  return Plugin::success();
 }
 
 Error GenericGlobalHandlerTy::readGlobalFromImage(GenericDeviceTy &Device,
@@ -180,10 +160,6 @@ Error GenericGlobalHandlerTy::readGlobalFromImage(GenericDeviceTy &Device,
                          "%u bytes in the ELF image but %u bytes on the host",
                          HostGlobal.getName().data(), ImageGlobal.getSize(),
                          HostGlobal.getSize());
-  if (ImageGlobal.getPtr() == nullptr)
-    return Plugin::error("Transfer impossible because global symbol '%s' has "
-                         "no representation in the image (NOBITS sections)",
-                         HostGlobal.getName().data());
 
   DP("Global symbol '%s' was found in the ELF image and %u bytes will copied "
      "from %p to %p.\n",

diff  --git a/openmp/libomptarget/plugins-nextgen/common/src/Utils/ELF.cpp b/openmp/libomptarget/plugins-nextgen/common/src/Utils/ELF.cpp
index 0643d004df17e6..305ea7d9c874b4 100644
--- a/openmp/libomptarget/plugins-nextgen/common/src/Utils/ELF.cpp
+++ b/openmp/libomptarget/plugins-nextgen/common/src/Utils/ELF.cpp
@@ -261,3 +261,27 @@ utils::elf::getSymbol(const ELFObjectFile<ELF64LE> &ELFObj, StringRef Name) {
 
   return nullptr;
 }
+
+Expected<const void *> utils::elf::getSymbolAddress(
+    const object::ELFObjectFile<object::ELF64LE> &ELFObj,
+    const object::ELF64LE::Sym &Symbol) {
+  const ELFFile<ELF64LE> &ELFFile = ELFObj.getELFFile();
+
+  auto SecOrErr = ELFFile.getSection(Symbol.st_shndx);
+  if (!SecOrErr)
+    return SecOrErr.takeError();
+  const auto &Section = *SecOrErr;
+
+  // A section with SHT_NOBITS occupies no space in the file and has no offset.
+  if (Section->sh_type == ELF::SHT_NOBITS)
+    return createError(
+        "invalid sh_type for symbol lookup, cannot be SHT_NOBITS");
+
+  uint64_t Offset = Section->sh_offset - Section->sh_addr + Symbol.st_value;
+  if (Offset > ELFFile.getBufSize())
+    return createError("invalid offset [" + Twine(Offset) +
+                       "] into ELF file of size [" +
+                       Twine(ELFFile.getBufSize()) + "]");
+
+  return ELFFile.base() + Offset;
+}

diff  --git a/openmp/libomptarget/plugins-nextgen/common/src/Utils/ELF.h b/openmp/libomptarget/plugins-nextgen/common/src/Utils/ELF.h
index b3740a104d6130..7b58cbaf59acef 100644
--- a/openmp/libomptarget/plugins-nextgen/common/src/Utils/ELF.h
+++ b/openmp/libomptarget/plugins-nextgen/common/src/Utils/ELF.h
@@ -25,6 +25,11 @@ namespace elf {
 /// e_machine matches \p target_id; return zero otherwise.
 int32_t checkMachine(__tgt_device_image *Image, uint16_t TargetId);
 
+/// Returns a pointer to the given \p Symbol inside of an ELF object.
+llvm::Expected<const void *> getSymbolAddress(
+    const llvm::object::ELFObjectFile<llvm::object::ELF64LE> &ELFObj,
+    const llvm::object::ELF64LE::Sym &Symbol);
+
 /// Returns the symbol associated with the \p Name in the \p ELFObj. It will
 /// first search for the hash sections to identify symbols from the hash table.
 /// If that fails it will fall back to a linear search in the case of an


        


More information about the Openmp-commits mailing list