Skip to content

Commit 0ab663d

Browse files
authored
[Libomptarget] Move ELF symbol extraction to the ELF utility (llvm#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).
1 parent 1fdbdb8 commit 0ab663d

File tree

4 files changed

+37
-38
lines changed

4 files changed

+37
-38
lines changed

openmp/libomptarget/plugins-nextgen/common/include/GlobalHandler.h

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -92,12 +92,6 @@ class GenericGlobalHandlerTy {
9292
/// Map to store the ELF object files that have been loaded.
9393
llvm::DenseMap<int32_t, ELF64LEObjectFile> ELFObjectFiles;
9494

95-
/// Extract the global's information from the ELF image, section, and symbol.
96-
virtual Error getGlobalMetadataFromELF(const DeviceImageTy &Image,
97-
const ELF64LE::Sym &Symbol,
98-
const ELF64LE::Shdr &Section,
99-
GlobalTy &ImageGlobal);
100-
10195
/// Actually move memory between host and device. See readGlobalFromDevice and
10296
/// writeGlobalToDevice for the interface description.
10397
Error moveGlobalBetweenDeviceAndHost(GenericDeviceTy &Device,

openmp/libomptarget/plugins-nextgen/common/src/GlobalHandler.cpp

Lines changed: 8 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,8 @@
1616

1717
#include "Shared/Utils.h"
1818

19-
#include "llvm/BinaryFormat/ELF.h"
2019
#include "llvm/Support/Error.h"
2120

22-
#include <cstdint>
2321
#include <cstring>
2422

2523
using namespace llvm;
@@ -52,27 +50,6 @@ GenericGlobalHandlerTy::getOrCreateELFObjectFile(const GenericDeviceTy &Device,
5250
return &Result.first->second;
5351
}
5452

55-
Error GenericGlobalHandlerTy::getGlobalMetadataFromELF(
56-
const DeviceImageTy &Image, const ELF64LE::Sym &Symbol,
57-
const ELF64LE::Shdr &Section, GlobalTy &ImageGlobal) {
58-
59-
// The global's address is computed as the image begin + the ELF section
60-
// offset + the ELF symbol value except for NOBITS sections that, as the name
61-
// suggests, have no bits in the image. We still record the size and use
62-
// nullptr to indicate there is no location.
63-
if (Section.sh_type == ELF::SHT_NOBITS)
64-
ImageGlobal.setPtr(nullptr);
65-
else
66-
ImageGlobal.setPtr(
67-
advanceVoidPtr(Image.getStart(),
68-
Section.sh_offset - Section.sh_addr + Symbol.st_value));
69-
70-
// Set the global's size.
71-
ImageGlobal.setSize(Symbol.st_size);
72-
73-
return Plugin::success();
74-
}
75-
7653
Error GenericGlobalHandlerTy::moveGlobalBetweenDeviceAndHost(
7754
GenericDeviceTy &Device, DeviceImageTy &Image, const GlobalTy &HostGlobal,
7855
bool Device2Host) {
@@ -156,15 +133,18 @@ Error GenericGlobalHandlerTy::getGlobalMetadataFromImage(
156133
return Plugin::error("Failed to find global symbol '%s' in the ELF image",
157134
ImageGlobal.getName().data());
158135

136+
auto AddrOrErr = utils::elf::getSymbolAddress(*ELFObj, **SymOrErr);
159137
// Get the section to which the symbol belongs.
160-
auto SecOrErr = ELFObj->getELFFile().getSection((*SymOrErr)->st_shndx);
161-
if (!SecOrErr)
162-
return Plugin::error("Failed to get ELF section from global '%s': %s",
138+
if (!AddrOrErr)
139+
return Plugin::error("Failed to get ELF symbol from global '%s': %s",
163140
ImageGlobal.getName().data(),
164-
toString(SecOrErr.takeError()).data());
141+
toString(AddrOrErr.takeError()).data());
165142

166143
// Setup the global symbol's address and size.
167-
return getGlobalMetadataFromELF(Image, **SymOrErr, **SecOrErr, ImageGlobal);
144+
ImageGlobal.setPtr(const_cast<void *>(*AddrOrErr));
145+
ImageGlobal.setSize((*SymOrErr)->st_size);
146+
147+
return Plugin::success();
168148
}
169149

170150
Error GenericGlobalHandlerTy::readGlobalFromImage(GenericDeviceTy &Device,
@@ -180,10 +160,6 @@ Error GenericGlobalHandlerTy::readGlobalFromImage(GenericDeviceTy &Device,
180160
"%u bytes in the ELF image but %u bytes on the host",
181161
HostGlobal.getName().data(), ImageGlobal.getSize(),
182162
HostGlobal.getSize());
183-
if (ImageGlobal.getPtr() == nullptr)
184-
return Plugin::error("Transfer impossible because global symbol '%s' has "
185-
"no representation in the image (NOBITS sections)",
186-
HostGlobal.getName().data());
187163

188164
DP("Global symbol '%s' was found in the ELF image and %u bytes will copied "
189165
"from %p to %p.\n",

openmp/libomptarget/plugins-nextgen/common/src/Utils/ELF.cpp

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -261,3 +261,27 @@ utils::elf::getSymbol(const ELFObjectFile<ELF64LE> &ELFObj, StringRef Name) {
261261

262262
return nullptr;
263263
}
264+
265+
Expected<const void *> utils::elf::getSymbolAddress(
266+
const object::ELFObjectFile<object::ELF64LE> &ELFObj,
267+
const object::ELF64LE::Sym &Symbol) {
268+
const ELFFile<ELF64LE> &ELFFile = ELFObj.getELFFile();
269+
270+
auto SecOrErr = ELFFile.getSection(Symbol.st_shndx);
271+
if (!SecOrErr)
272+
return SecOrErr.takeError();
273+
const auto &Section = *SecOrErr;
274+
275+
// A section with SHT_NOBITS occupies no space in the file and has no offset.
276+
if (Section->sh_type == ELF::SHT_NOBITS)
277+
return createError(
278+
"invalid sh_type for symbol lookup, cannot be SHT_NOBITS");
279+
280+
uint64_t Offset = Section->sh_offset - Section->sh_addr + Symbol.st_value;
281+
if (Offset > ELFFile.getBufSize())
282+
return createError("invalid offset [" + Twine(Offset) +
283+
"] into ELF file of size [" +
284+
Twine(ELFFile.getBufSize()) + "]");
285+
286+
return ELFFile.base() + Offset;
287+
}

openmp/libomptarget/plugins-nextgen/common/src/Utils/ELF.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,11 @@ namespace elf {
2525
/// e_machine matches \p target_id; return zero otherwise.
2626
int32_t checkMachine(__tgt_device_image *Image, uint16_t TargetId);
2727

28+
/// Returns a pointer to the given \p Symbol inside of an ELF object.
29+
llvm::Expected<const void *> getSymbolAddress(
30+
const llvm::object::ELFObjectFile<llvm::object::ELF64LE> &ELFObj,
31+
const llvm::object::ELF64LE::Sym &Symbol);
32+
2833
/// Returns the symbol associated with the \p Name in the \p ELFObj. It will
2934
/// first search for the hash sections to identify symbols from the hash table.
3035
/// If that fails it will fall back to a linear search in the case of an

0 commit comments

Comments
 (0)