Skip to content
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions llvm/lib/ObjCopy/COFF/COFFObject.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,11 @@ namespace coff {
using namespace object;

void Object::addSymbols(ArrayRef<Symbol> NewSymbols) {
size_t RawIndex = 0;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mstorsjo I've noticed that this approach is error-prone as if addSymbols invoked twice, symbols will have duplicated OriginalRawIndex (It doesn't happen now). Looks like we should have this variable as a class member.

Copy link
Member

Choose a reason for hiding this comment

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

I don't see this as a big concern though - we probably only ever call addSymbols once. But I don't mind making RawIndex a class member indeed, if it avoids the risk of an issue.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Created a NextSymbolOriginalIndex similarly to NextSymbolUniqueId. Strictly speaking the name would be NextSymbolOriginalRawIndex. Dropped Raw. Maybe I should've dropped Original.

for (Symbol S : NewSymbols) {
S.UniqueId = NextSymbolUniqueId++;
S.OriginalRawIndex = RawIndex;
RawIndex += 1 + S.Sym.NumberOfAuxSymbols;
Symbols.emplace_back(S);
}
updateSymbols();
Expand Down
1 change: 1 addition & 0 deletions llvm/lib/ObjCopy/COFF/COFFObject.h
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ struct Symbol {
std::optional<size_t> WeakTargetSymbolId;
size_t UniqueId;
size_t RawIndex;
size_t OriginalRawIndex;
bool Referenced;
};

Expand Down
77 changes: 77 additions & 0 deletions llvm/lib/ObjCopy/COFF/COFFWriter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
#include "llvm/ADT/StringRef.h"
#include "llvm/BinaryFormat/COFF.h"
#include "llvm/Object/COFF.h"
#include "llvm/Support/CRC.h"
#include "llvm/Support/Endian.h"
#include "llvm/Support/Errc.h"
#include "llvm/Support/ErrorHandling.h"
#include <cstddef>
Expand Down Expand Up @@ -92,6 +94,79 @@ Error COFFWriter::finalizeSymbolContents() {
return Error::success();
}

Error COFFWriter::finalizeSymIdxContents() {
// CFGuards shouldn't be present in PE
if (Obj.IsPE)
return Error::success();

// Currently handle only sections consisting only of .symidx.
// TODO: other sections such as .impcall and .hybmp$x require more complex
// handling as they have more complex layout.
auto IsSymIdxSection = [](StringRef Name) {
return Name == ".gljmp$y" || Name == ".giats$y" || Name == ".gfids$y" ||
Name == ".gehcont$y";
};

DenseMap<size_t, size_t> SymIdMap;
SmallDenseMap<ssize_t, coff_aux_section_definition *, 4> SecIdMap;
bool NeedUpdate = false;
for (auto &Sym : Obj.getMutableSymbols()) {
NeedUpdate |= Sym.OriginalRawIndex == Sym.RawIndex;
SymIdMap[Sym.OriginalRawIndex] = Sym.RawIndex;

// We collect only definition symbols of the sections to update checksum
if (Sym.Sym.NumberOfAuxSymbols == 1 &&
Sym.Sym.StorageClass == IMAGE_SYM_CLASS_STATIC && Sym.Sym.Value == 0 &&
IsSymIdxSection(Sym.Name))
SecIdMap[Sym.TargetSectionId] =
reinterpret_cast<coff_aux_section_definition *>(
Sym.AuxData[0].Opaque);
}

if (!NeedUpdate)
return Error::success();

for (auto &Sec : Obj.getMutableSections()) {
if (!IsSymIdxSection(Sec.Name))
continue;

ArrayRef<uint8_t> RawIds = Sec.getContents();
// Nothing to do and also CheckSum will be -1 instead of 0 if we recalculate
// it on empty input.
if (RawIds.size() == 0)
continue;

if (!SecIdMap.contains(Sec.UniqueId))
return createStringError(object_error::invalid_symbol_index,
"section '%s' does not have the corresponding "
"symbol or the symbol has unexpected format",
Sec.Name.str().c_str());

// Create updated content
ArrayRef<support::ulittle32_t> Ids(
reinterpret_cast<const support::ulittle32_t *>(RawIds.data()),
RawIds.size() / 4);
std::vector<support::ulittle32_t> NewIds;
for (auto Id : Ids) {
if (!SymIdMap.contains(Id))
return createStringError(object_error::invalid_symbol_index,
"section '%s' contains a .symidx (%d) that is "
"incorrect or was stripped",
Sec.Name.str().c_str(), Id.value());
NewIds.push_back(support::ulittle32_t(SymIdMap[Id]));
}
ArrayRef<uint8_t> NewRawIds(reinterpret_cast<uint8_t *>(NewIds.data()),
RawIds.size());
// Update check sum
JamCRC JC(/*Init=*/0);
JC.update(NewRawIds);
SecIdMap[Sec.UniqueId]->CheckSum = JC.getCRC();
// Set new content
Sec.setOwnedContents(NewRawIds.vec());
}
return Error::success();
}

void COFFWriter::layoutSections() {
for (auto &S : Obj.getMutableSections()) {
if (S.Header.SizeOfRawData > 0)
Expand Down Expand Up @@ -183,6 +258,8 @@ Error COFFWriter::finalize(bool IsBigObj) {
return E;
if (Error E = finalizeSymbolContents())
return E;
if (Error E = finalizeSymIdxContents())
return E;

size_t SizeOfHeaders = 0;
FileAlignment = 1;
Expand Down
1 change: 1 addition & 0 deletions llvm/lib/ObjCopy/COFF/COFFWriter.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ class COFFWriter {
template <class SymbolTy> std::pair<size_t, size_t> finalizeSymbolTable();
Error finalizeRelocTargets();
Error finalizeSymbolContents();
Error finalizeSymIdxContents();
void layoutSections();
Expected<size_t> finalizeStringTable();

Expand Down
Loading