[Openmp-commits] [openmp] [Libomptarget] Move ELF symbol extraction to the ELF utility (PR #74717)
Joseph Huber via Openmp-commits
openmp-commits at lists.llvm.org
Thu Dec 7 06:20:25 PST 2023
https://github.com/jhuber6 created https://github.com/llvm/llvm-project/pull/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).
>From cd4db38a4c21a9ef9d60b949c010609977ee15dd Mon Sep 17 00:00:00 2001
From: Joseph Huber <huberjn at outlook.com>
Date: Thu, 7 Dec 2023 08:17:11 -0600
Subject: [PATCH] [Libomptarget] Move ELF symbol extraction to the ELF utility
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).
---
.../common/include/GlobalHandler.h | 6 ---
.../common/src/GlobalHandler.cpp | 40 ++++---------------
.../plugins-nextgen/common/src/Utils/ELF.cpp | 24 +++++++++++
.../plugins-nextgen/common/src/Utils/ELF.h | 5 +++
4 files changed, 37 insertions(+), 38 deletions(-)
diff --git a/openmp/libomptarget/plugins-nextgen/common/include/GlobalHandler.h b/openmp/libomptarget/plugins-nextgen/common/include/GlobalHandler.h
index fa788c5e1d02b..fa079ac9660ee 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 0a19148ca4ec6..3a272e228c7df 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 0643d004df17e..305ea7d9c874b 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 b3740a104d613..7b58cbaf59ace 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