GOFF: Only register sections within MCObjectStreamer::changeSection #150183
Conversation
Created using spr 1.3.5-bogner [skip ci]
Created using spr 1.3.5-bogner
|
@llvm/pr-subscribers-mc Author: Fangrui Song (MaskRay) ChangesregisterSection should only be called by Full diff: https://github.com/llvm/llvm-project/pull/150183.diff 1 Files Affected:
diff --git a/llvm/lib/MC/MCGOFFStreamer.cpp b/llvm/lib/MC/MCGOFFStreamer.cpp
index b7021915e7b70..7828065a7f360 100644
--- a/llvm/lib/MC/MCGOFFStreamer.cpp
+++ b/llvm/lib/MC/MCGOFFStreamer.cpp
@@ -26,19 +26,15 @@ GOFFObjectWriter &MCGOFFStreamer::getWriter() {
return static_cast<GOFFObjectWriter &>(getAssembler().getWriter());
}
-// Make sure that all section are registered in the correct order.
-static void registerSectionHierarchy(MCAssembler &Asm, MCSectionGOFF *Section) {
- if (Section->isRegistered())
- return;
- if (Section->getParent())
- registerSectionHierarchy(Asm, Section->getParent());
- Asm.registerSection(*Section);
-}
-
void MCGOFFStreamer::changeSection(MCSection *Section, uint32_t Subsection) {
- registerSectionHierarchy(getAssembler(),
- static_cast<MCSectionGOFF *>(Section));
- MCObjectStreamer::changeSection(Section, Subsection);
+ // Make sure that all section are registered in the correct order.
+ SmallVector<MCSectionGOFF *> Sections;
+ for (auto *S = static_cast<MCSectionGOFF *>(Section); S; S = S->getParent())
+ Sections.push_back(S);
+ while (!Sections.empty()) {
+ auto *S = Sections.pop_back_val();
+ MCObjectStreamer::changeSection(S, 0);
+ }
}
MCStreamer *llvm::createGOFFStreamer(MCContext &Context,
|
llvm/lib/MC/MCGOFFStreamer.cpp
Outdated
| Sections.push_back(S); | ||
| while (!Sections.empty()) { | ||
| auto *S = Sections.pop_back_val(); | ||
| MCObjectStreamer::changeSection(S, 0); |
There was a problem hiding this comment.
Looks like this now completely ignores the Subsection parameter?
There was a problem hiding this comment.
subsection is a GNU Assembler feature. In LLVM only ELF targets use it. Does GOFF use it?
There was a problem hiding this comment.
Not as such in the file format, but the LLVM back-end currently uses subsections to subdivide the text section for proper placement of the PPA annotations, see GOFF::SK_PPA1 and GOFF::SK_PPA2.
There was a problem hiding this comment.
Updated. I think GOFF team needs to add tests to show that incorrect changeSection would lead to PPA1/PPA2 failures.
Created using spr 1.3.5-bogner [skip ci]
Created using spr 1.3.5-bogner
Created using spr 1.3.5-bogner [skip ci]
Created using spr 1.3.5-bogner [skip ci]
MaskRay
changed the base branch from
users/MaskRay/spr/main.goff-only-register-sections-within-mcobjectstreamerchangesection
to
main
MaskRay
deleted the
users/MaskRay/spr/goff-only-register-sections-within-mcobjectstreamerchangesection
branch
…geSection registerSection should only be called by MCObjectStreamer::changeSection. This will be utilized by a pending change to move initial fragment allocation from MCContext::createSection to MCStreamer::changeSection, resolving some issues (Fragments should only be created when using MCSteramer, not during `MCContext::getELFSection` calls) Pull Request: llvm/llvm-project#150183
registerSection should only be called by MCObjectStreamer::changeSection. This will be utilized by a pending change to move initial fragment allocation from MCContext::createSection to MCStreamer::changeSection, resolving some issues (Fragments should only be created when using MCSteramer, not during `MCContext::getELFSection` calls) Pull Request: llvm#150183
3 participants
registerSection should only be called by
MCObjectStreamer::changeSection. This will be utilized by a pending
change to move initial fragment allocation from MCContext::createSection
to MCStreamer::changeSection, resolving some issues (Fragments should
only be created when using MCSteramer, not during
MCContext::getELFSectioncalls)