From 3588a8ab075b492fe5b882f4c36fce23ce719b26 Mon Sep 17 00:00:00 2001 From: Shubham Sandeep Rastogi Date: Tue, 6 Aug 2024 15:36:55 -0700 Subject: [PATCH 1/6] Fix CAS failures on rebranch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit rdar://133264719 (🌲6.0+1 CAS failures on rebranch) Because of eb2239299e51d, the LinkerOptions have been moved from MCAssembler to MachObjectWriter, but MachOCASWriter doesn't have access to those linker options, this patch fixes the bug by moving the LinkerOptions to MCObjectWriter instead. --- llvm/include/llvm/MC/MCMachObjectWriter.h | 6 ------ llvm/include/llvm/MC/MCObjectWriter.h | 7 +++++++ llvm/lib/MC/MCMachOStreamer.cpp | 2 +- llvm/lib/MC/MCObjectWriter.cpp | 1 + llvm/lib/MC/MachObjectWriter.cpp | 1 - llvm/test/MC/MachO/linker-options.ll | 2 ++ 6 files changed, 11 insertions(+), 8 deletions(-) diff --git a/llvm/include/llvm/MC/MCMachObjectWriter.h b/llvm/include/llvm/MC/MCMachObjectWriter.h index 24004c6616220..f31ff13757029 100644 --- a/llvm/include/llvm/MC/MCMachObjectWriter.h +++ b/llvm/include/llvm/MC/MCMachObjectWriter.h @@ -174,9 +174,6 @@ class MachObjectWriter : public MCObjectWriter { std::optional PtrAuthABIVersion; bool PtrAuthKernelABIVersion; - // The list of linker options for LC_LINKER_OPTION. - std::vector> LinkerOptions; - MachSymbolData *findSymbolData(const MCSymbol &Sym); void writeWithPadding(StringRef Str, uint64_t Size); @@ -281,9 +278,6 @@ class MachObjectWriter : public MCObjectWriter { void setPtrAuthKernelABIVersion(bool V) override { PtrAuthKernelABIVersion = V; } - std::vector> &getLinkerOptions() { - return LinkerOptions; - } /// @} diff --git a/llvm/include/llvm/MC/MCObjectWriter.h b/llvm/include/llvm/MC/MCObjectWriter.h index 2cb01578f1a5e..dba4c7546e78e 100644 --- a/llvm/include/llvm/MC/MCObjectWriter.h +++ b/llvm/include/llvm/MC/MCObjectWriter.h @@ -33,6 +33,9 @@ class MCValue; /// should be emitted as part of writeObject(). class MCObjectWriter { protected: + // The list of linker options for LC_LINKER_OPTION. + std::vector> LinkerOptions; + /// List of declared file names SmallVector, 0> FileNames; // XCOFF specific: Optional compiler version. @@ -61,6 +64,10 @@ class MCObjectWriter { /// \name High-Level API /// @{ + std::vector> &getLinkerOptions() { + return LinkerOptions; + } + /// Perform any late binding of symbols (for example, to assign symbol /// indices for use when generating relocations). /// diff --git a/llvm/lib/MC/MCMachOStreamer.cpp b/llvm/lib/MC/MCMachOStreamer.cpp index 479b1df250893..59a71d3b85046 100644 --- a/llvm/lib/MC/MCMachOStreamer.cpp +++ b/llvm/lib/MC/MCMachOStreamer.cpp @@ -232,7 +232,7 @@ void MCMachOStreamer::emitAssemblerFlag(MCAssemblerFlag Flag) { } void MCMachOStreamer::emitLinkerOptions(ArrayRef Options) { - getWriter().getLinkerOptions().push_back(Options); + getMCObjectWriter().getLinkerOptions().push_back(Options); } void MCMachOStreamer::emitDataRegion(MCDataRegionType Kind) { diff --git a/llvm/lib/MC/MCObjectWriter.cpp b/llvm/lib/MC/MCObjectWriter.cpp index 818e703514d61..d064fbea05a37 100644 --- a/llvm/lib/MC/MCObjectWriter.cpp +++ b/llvm/lib/MC/MCObjectWriter.cpp @@ -25,6 +25,7 @@ void MCObjectWriter::reset() { EmitAddrsigSection = false; SubsectionsViaSymbols = false; CGProfile.clear(); + LinkerOptions.clear(); } bool MCObjectWriter::isSymbolRefDifferenceFullyResolved( diff --git a/llvm/lib/MC/MachObjectWriter.cpp b/llvm/lib/MC/MachObjectWriter.cpp index a158e431c37b1..df633d2d4e509 100644 --- a/llvm/lib/MC/MachObjectWriter.cpp +++ b/llvm/lib/MC/MachObjectWriter.cpp @@ -63,7 +63,6 @@ void MachObjectWriter::reset() { TargetVariantVersionInfo.SDKVersion = VersionTuple(); PtrAuthABIVersion = std::nullopt; PtrAuthKernelABIVersion = false; - LinkerOptions.clear(); MCObjectWriter::reset(); } diff --git a/llvm/test/MC/MachO/linker-options.ll b/llvm/test/MC/MachO/linker-options.ll index 0f678cd3ad16c..0ddc8c0bce260 100644 --- a/llvm/test/MC/MachO/linker-options.ll +++ b/llvm/test/MC/MachO/linker-options.ll @@ -6,6 +6,8 @@ ; RUN: llc -O0 -mtriple=x86_64-apple-darwin -filetype=obj -o - %s | llvm-readobj --macho-linker-options - > %t ; RUN: FileCheck --check-prefix=CHECK-OBJ < %t %s +; RUN: mkdir -p %t.dir/cas +; RUN: llc -O0 -mtriple=x86_64-apple-darwin -filetype=obj --cas %t.dir/cas -cas-backend -o - %s | llvm-readobj --macho-linker-options - > %t ; CHECK-OBJ: Linker Options { ; CHECK-OBJ: Size: 16 From 381c0683bd8c5b5240108f9801b50bdca1aad988 Mon Sep 17 00:00:00 2001 From: Shubham Sandeep Rastogi Date: Wed, 7 Aug 2024 10:16:38 -0700 Subject: [PATCH 2/6] Move LOHContainer to MCObjectWriter --- llvm/include/llvm/MC/MCMachObjectWriter.h | 4 ---- llvm/include/llvm/MC/MCObjectWriter.h | 6 ++++++ llvm/lib/MC/MCMachOStreamer.cpp | 2 +- llvm/lib/MC/MCObjectWriter.cpp | 1 + 4 files changed, 8 insertions(+), 5 deletions(-) diff --git a/llvm/include/llvm/MC/MCMachObjectWriter.h b/llvm/include/llvm/MC/MCMachObjectWriter.h index f31ff13757029..8d6855573afe2 100644 --- a/llvm/include/llvm/MC/MCMachObjectWriter.h +++ b/llvm/include/llvm/MC/MCMachObjectWriter.h @@ -165,9 +165,6 @@ class MachObjectWriter : public MCObjectWriter { /// @} - // Used to communicate Linker Optimization Hint information. - MCLOHContainer LOHContainer; - VersionInfoType VersionInfo{}; VersionInfoType TargetVariantVersionInfo{}; @@ -219,7 +216,6 @@ class MachObjectWriter : public MCObjectWriter { return SectionOrder; } SectionAddrMap &getSectionAddressMap() { return SectionAddress; } - MCLOHContainer &getLOHContainer() { return LOHContainer; } uint64_t getSectionAddress(const MCSection *Sec) const { return SectionAddress.lookup(Sec); diff --git a/llvm/include/llvm/MC/MCObjectWriter.h b/llvm/include/llvm/MC/MCObjectWriter.h index dba4c7546e78e..e8d8fca2bec6e 100644 --- a/llvm/include/llvm/MC/MCObjectWriter.h +++ b/llvm/include/llvm/MC/MCObjectWriter.h @@ -10,6 +10,7 @@ #define LLVM_MC_MCOBJECTWRITER_H #include "llvm/MC/MCDirectives.h" +#include "llvm/MC/MCLinkerOptimizationHint.h" #include "llvm/MC/MCSymbol.h" #include "llvm/TargetParser/Triple.h" #include @@ -36,6 +37,9 @@ class MCObjectWriter { // The list of linker options for LC_LINKER_OPTION. std::vector> LinkerOptions; + // Used to communicate Linker Optimization Hint information. + MCLOHContainer LOHContainer; + /// List of declared file names SmallVector, 0> FileNames; // XCOFF specific: Optional compiler version. @@ -68,6 +72,8 @@ class MCObjectWriter { return LinkerOptions; } + MCLOHContainer &getLOHContainer() { return LOHContainer; } + /// Perform any late binding of symbols (for example, to assign symbol /// indices for use when generating relocations). /// diff --git a/llvm/lib/MC/MCMachOStreamer.cpp b/llvm/lib/MC/MCMachOStreamer.cpp index 59a71d3b85046..9f9c9cf73181a 100644 --- a/llvm/lib/MC/MCMachOStreamer.cpp +++ b/llvm/lib/MC/MCMachOStreamer.cpp @@ -124,7 +124,7 @@ class MCMachOStreamer : public MCObjectStreamer { } void emitLOHDirective(MCLOHType Kind, const MCLOHArgs &Args) override { - getWriter().getLOHContainer().addDirective(Kind, Args); + getMCObjectWriter().getLOHContainer().addDirective(Kind, Args); } void emitCGProfileEntry(const MCSymbolRefExpr *From, const MCSymbolRefExpr *To, uint64_t Count) override { diff --git a/llvm/lib/MC/MCObjectWriter.cpp b/llvm/lib/MC/MCObjectWriter.cpp index d064fbea05a37..28703b6974748 100644 --- a/llvm/lib/MC/MCObjectWriter.cpp +++ b/llvm/lib/MC/MCObjectWriter.cpp @@ -26,6 +26,7 @@ void MCObjectWriter::reset() { SubsectionsViaSymbols = false; CGProfile.clear(); LinkerOptions.clear(); + LOHContainer.reset(); } bool MCObjectWriter::isSymbolRefDifferenceFullyResolved( From c936fefb3685c26f8a8e3407bf267a0baef61145 Mon Sep 17 00:00:00 2001 From: Shubham Sandeep Rastogi Date: Wed, 7 Aug 2024 10:49:06 -0700 Subject: [PATCH 3/6] Change getwriter() calls to getMCObjectWriter --- llvm/lib/MC/MCMachOStreamer.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/llvm/lib/MC/MCMachOStreamer.cpp b/llvm/lib/MC/MCMachOStreamer.cpp index 9f9c9cf73181a..0c0bcb2e5c327 100644 --- a/llvm/lib/MC/MCMachOStreamer.cpp +++ b/llvm/lib/MC/MCMachOStreamer.cpp @@ -129,7 +129,7 @@ class MCMachOStreamer : public MCObjectStreamer { void emitCGProfileEntry(const MCSymbolRefExpr *From, const MCSymbolRefExpr *To, uint64_t Count) override { if (!From->getSymbol().isTemporary() && !To->getSymbol().isTemporary()) - getWriter().getCGProfile().push_back({From, To, Count}); + getMCObjectWriter().getCGProfile().push_back({From, To, Count}); } void finishImpl() override; @@ -277,8 +277,8 @@ void MCMachOStreamer::emitDarwinTargetVariantBuildVersion( void MCMachOStreamer::EmitPtrAuthABIVersion(unsigned PtrAuthABIVersion, bool PtrAuthKernelABIVersion) { - getWriter().setPtrAuthABIVersion(PtrAuthABIVersion); - getWriter().setPtrAuthKernelABIVersion(PtrAuthKernelABIVersion); + getMCObjectWriter().setPtrAuthABIVersion(PtrAuthABIVersion); + getMCObjectWriter().setPtrAuthKernelABIVersion(PtrAuthKernelABIVersion); } void MCMachOStreamer::emitThumbFunc(MCSymbol *Symbol) { @@ -518,7 +518,7 @@ void MCMachOStreamer::finalizeCGProfileEntry(const MCSymbolRefExpr *&SRE) { void MCMachOStreamer::finalizeCGProfile() { MCAssembler &Asm = getAssembler(); - MCObjectWriter &W = getWriter(); + MCObjectWriter &W = getMCObjectWriter(); if (W.getCGProfile().empty()) return; for (auto &E : W.getCGProfile()) { From 1eb1101192b090aebc44b6ad17c062de1dcf4d5c Mon Sep 17 00:00:00 2001 From: Shubham Sandeep Rastogi Date: Wed, 7 Aug 2024 10:58:05 -0700 Subject: [PATCH 4/6] Move DataRegions to MCObjectWriter --- llvm/include/llvm/MC/MCMachObjectWriter.h | 9 --------- llvm/include/llvm/MC/MCObjectWriter.h | 11 +++++++++++ llvm/lib/MC/MCMachOStreamer.cpp | 4 ++-- llvm/lib/MC/MCObjectWriter.cpp | 1 + llvm/lib/MC/MachObjectWriter.cpp | 11 +++++------ 5 files changed, 19 insertions(+), 17 deletions(-) diff --git a/llvm/include/llvm/MC/MCMachObjectWriter.h b/llvm/include/llvm/MC/MCMachObjectWriter.h index 8d6855573afe2..60f1f6714f923 100644 --- a/llvm/include/llvm/MC/MCMachObjectWriter.h +++ b/llvm/include/llvm/MC/MCMachObjectWriter.h @@ -86,12 +86,6 @@ class MCMachObjectTargetWriter : public MCObjectTargetWriter { class MachObjectWriter : public MCObjectWriter { public: - struct DataRegionData { - MachO::DataRegionType Kind; - MCSymbol *Start; - MCSymbol *End; - }; - // A Major version of 0 indicates that no version information was supplied // and so the corresponding load command should not be emitted. using VersionInfoType = struct { @@ -146,8 +140,6 @@ class MachObjectWriter : public MCObjectWriter { std::vector IndirectSymbols; DenseMap IndirectSymBase; - std::vector DataRegions; - SectionAddrMap SectionAddress; // List of sections in layout order. Virtual sections are after non-virtual @@ -211,7 +203,6 @@ class MachObjectWriter : public MCObjectWriter { std::vector &getIndirectSymbols() { return IndirectSymbols; } - std::vector &getDataRegions() { return DataRegions; } const llvm::SmallVectorImpl &getSectionOrder() const { return SectionOrder; } diff --git a/llvm/include/llvm/MC/MCObjectWriter.h b/llvm/include/llvm/MC/MCObjectWriter.h index e8d8fca2bec6e..b414db5f00c06 100644 --- a/llvm/include/llvm/MC/MCObjectWriter.h +++ b/llvm/include/llvm/MC/MCObjectWriter.h @@ -33,7 +33,16 @@ class MCValue; /// MCAssembler instance, which contains all the symbol and section data which /// should be emitted as part of writeObject(). class MCObjectWriter { +public: + struct DataRegionData { + unsigned Kind; + MCSymbol *Start; + MCSymbol *End; + }; + protected: + std::vector DataRegions; + // The list of linker options for LC_LINKER_OPTION. std::vector> LinkerOptions; @@ -74,6 +83,8 @@ class MCObjectWriter { MCLOHContainer &getLOHContainer() { return LOHContainer; } + std::vector &getDataRegions() { return DataRegions; } + /// Perform any late binding of symbols (for example, to assign symbol /// indices for use when generating relocations). /// diff --git a/llvm/lib/MC/MCMachOStreamer.cpp b/llvm/lib/MC/MCMachOStreamer.cpp index 0c0bcb2e5c327..21625f9f4203b 100644 --- a/llvm/lib/MC/MCMachOStreamer.cpp +++ b/llvm/lib/MC/MCMachOStreamer.cpp @@ -203,11 +203,11 @@ void MCMachOStreamer::emitDataRegion(MachO::DataRegionType Kind) { MCSymbol *Start = getContext().createTempSymbol(); emitLabel(Start); // Record the region for the object writer to use. - getWriter().getDataRegions().push_back({Kind, Start, nullptr}); + getMCObjectWriter().getDataRegions().push_back({Kind, Start, nullptr}); } void MCMachOStreamer::emitDataRegionEnd() { - auto &Regions = getWriter().getDataRegions(); + auto &Regions = getMCObjectWriter().getDataRegions(); assert(!Regions.empty() && "Mismatched .end_data_region!"); auto &Data = Regions.back(); assert(!Data.End && "Mismatched .end_data_region!"); diff --git a/llvm/lib/MC/MCObjectWriter.cpp b/llvm/lib/MC/MCObjectWriter.cpp index 28703b6974748..ed14226d36dcd 100644 --- a/llvm/lib/MC/MCObjectWriter.cpp +++ b/llvm/lib/MC/MCObjectWriter.cpp @@ -27,6 +27,7 @@ void MCObjectWriter::reset() { CGProfile.clear(); LinkerOptions.clear(); LOHContainer.reset(); + DataRegions.clear(); } bool MCObjectWriter::isSymbolRefDifferenceFullyResolved( diff --git a/llvm/lib/MC/MachObjectWriter.cpp b/llvm/lib/MC/MachObjectWriter.cpp index df633d2d4e509..8f2bc100e39a2 100644 --- a/llvm/lib/MC/MachObjectWriter.cpp +++ b/llvm/lib/MC/MachObjectWriter.cpp @@ -49,14 +49,12 @@ void MachObjectWriter::reset() { Relocations.clear(); IndirectSymBase.clear(); IndirectSymbols.clear(); - DataRegions.clear(); SectionAddress.clear(); SectionOrder.clear(); StringTable.clear(); LocalSymbolData.clear(); ExternalSymbolData.clear(); UndefinedSymbolData.clear(); - LOHContainer.reset(); VersionInfo.Major = 0; VersionInfo.SDKVersion = VersionTuple(); TargetVariantVersionInfo.Major = 0; @@ -1096,10 +1094,11 @@ void MachObjectWriter::writeDataInCodeRegion(MCAssembler &Asm) { else report_fatal_error("Data region not terminated"); - LLVM_DEBUG(dbgs() << "data in code region-- kind: " << Data.Kind - << " start: " << Start << "(" << Data.Start->getName() - << ")" << " end: " << End << "(" << Data.End->getName() - << ")" << " size: " << End - Start << "\n"); + LLVM_DEBUG(dbgs() << "data in code region-- kind: " + << (MachO::DataRegionType)Data.Kind << " start: " + << Start << "(" << Data.Start->getName() << ")" + << " end: " << End << "(" << Data.End->getName() << ")" + << " size: " << End - Start << "\n"); W.write(Start); W.write(End - Start); W.write(Data.Kind); From 1db504cbd827363fc5a2281615c1ce359c22e5ea Mon Sep 17 00:00:00 2001 From: Shubham Sandeep Rastogi Date: Wed, 7 Aug 2024 11:07:30 -0700 Subject: [PATCH 5/6] Move IndirectSymbols to MCObjectWriter --- llvm/include/llvm/MC/MCMachObjectWriter.h | 9 --------- llvm/include/llvm/MC/MCObjectWriter.h | 11 +++++++++++ llvm/lib/MC/MCMachOStreamer.cpp | 2 +- llvm/lib/MC/MCObjectWriter.cpp | 1 + 4 files changed, 13 insertions(+), 10 deletions(-) diff --git a/llvm/include/llvm/MC/MCMachObjectWriter.h b/llvm/include/llvm/MC/MCMachObjectWriter.h index 60f1f6714f923..f47af7bd631aa 100644 --- a/llvm/include/llvm/MC/MCMachObjectWriter.h +++ b/llvm/include/llvm/MC/MCMachObjectWriter.h @@ -112,11 +112,6 @@ class MachObjectWriter : public MCObjectWriter { bool operator<(const MachSymbolData &RHS) const; }; - struct IndirectSymbolData { - MCSymbol *Symbol; - MCSection *Section; - }; - /// The target specific Mach-O writer instance. std::unique_ptr TargetObjectWriter; @@ -137,7 +132,6 @@ class MachObjectWriter : public MCObjectWriter { private: DenseMap> Relocations; - std::vector IndirectSymbols; DenseMap IndirectSymBase; SectionAddrMap SectionAddress; @@ -200,9 +194,6 @@ class MachObjectWriter : public MCObjectWriter { bool isFixupKindPCRel(const MCAssembler &Asm, unsigned Kind); - std::vector &getIndirectSymbols() { - return IndirectSymbols; - } const llvm::SmallVectorImpl &getSectionOrder() const { return SectionOrder; } diff --git a/llvm/include/llvm/MC/MCObjectWriter.h b/llvm/include/llvm/MC/MCObjectWriter.h index b414db5f00c06..89623cbbec451 100644 --- a/llvm/include/llvm/MC/MCObjectWriter.h +++ b/llvm/include/llvm/MC/MCObjectWriter.h @@ -41,6 +41,13 @@ class MCObjectWriter { }; protected: + struct IndirectSymbolData { + MCSymbol *Symbol; + MCSection *Section; + }; + + std::vector IndirectSymbols; + std::vector DataRegions; // The list of linker options for LC_LINKER_OPTION. @@ -81,6 +88,10 @@ class MCObjectWriter { return LinkerOptions; } + std::vector &getIndirectSymbols() { + return IndirectSymbols; + } + MCLOHContainer &getLOHContainer() { return LOHContainer; } std::vector &getDataRegions() { return DataRegions; } diff --git a/llvm/lib/MC/MCMachOStreamer.cpp b/llvm/lib/MC/MCMachOStreamer.cpp index 21625f9f4203b..8e96d294559b2 100644 --- a/llvm/lib/MC/MCMachOStreamer.cpp +++ b/llvm/lib/MC/MCMachOStreamer.cpp @@ -297,7 +297,7 @@ bool MCMachOStreamer::emitSymbolAttribute(MCSymbol *Sym, if (Attribute == MCSA_IndirectSymbol) { // Note that we intentionally cannot use the symbol data here; this is // important for matching the string table that 'as' generates. - getWriter().getIndirectSymbols().push_back( + getMCObjectWriter().getIndirectSymbols().push_back( {Symbol, getCurrentSectionOnly()}); return true; } diff --git a/llvm/lib/MC/MCObjectWriter.cpp b/llvm/lib/MC/MCObjectWriter.cpp index ed14226d36dcd..b0e922b669176 100644 --- a/llvm/lib/MC/MCObjectWriter.cpp +++ b/llvm/lib/MC/MCObjectWriter.cpp @@ -28,6 +28,7 @@ void MCObjectWriter::reset() { LinkerOptions.clear(); LOHContainer.reset(); DataRegions.clear(); + IndirectSymbols.clear(); } bool MCObjectWriter::isSymbolRefDifferenceFullyResolved( From 5c51eefec78dec777b59bd9de0e5e65762f09413 Mon Sep 17 00:00:00 2001 From: Shubham Sandeep Rastogi Date: Wed, 7 Aug 2024 11:31:47 -0700 Subject: [PATCH 6/6] Comment out getWriter() function --- llvm/lib/MC/MCMachOStreamer.cpp | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/llvm/lib/MC/MCMachOStreamer.cpp b/llvm/lib/MC/MCMachOStreamer.cpp index 8e96d294559b2..6cc545b5b7c81 100644 --- a/llvm/lib/MC/MCMachOStreamer.cpp +++ b/llvm/lib/MC/MCMachOStreamer.cpp @@ -78,9 +78,12 @@ class MCMachOStreamer : public MCObjectStreamer { MCObjectStreamer::reset(); } - MachObjectWriter &getWriter() { - return static_cast(getAssembler().getWriter()); - } + // This function is commented out downstream because it is unsafe to use a + // MachObjectWriter in the McMachOStreamer which may hold a MachOCASWriter + // instead. + // MachObjectWriter &getWriter() { + // return static_cast(getAssembler().getWriter()); + // } MCObjectWriter &getMCObjectWriter() { return static_cast(getAssembler().getWriter());