[llvm][MC] Add .pushsection/.popsection support to COFFAsmParser

[cynecx]

Summary

The COFFAsmParser (to my surprise) didn't support the .pushsection and .popsection directives. These directives aren't directly useful, however for frontends that have inline asm support this is really useful. Rust in particular, has support for inline asm, which can be used together with these directives to "emulate" features like static generics. This patch adds support for the two mentioned directives.

[MaskRay]

Please include more diff context. See https://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface

Thanks. This extension looks reasonable to me.

These directives aren't directly useful, however for frontends that have inline asm support this is really useful.

Agree. If you know which frontend is going to use this, consider mentioning it, otherwise I'll just assume some Clang users will adopt the feature.

llvm/lib/MC/MCParser/COFFAsmParser.cpp

71

Add this after addDirectiveHandler<&COFFAsmParser::ParseDirectiveSection>(".section");

371

While changing signature, consider fixing the function case to camelCase.

llvm/test/MC/COFF/section.s

216

Add some tests with arguments (including section flags)?

[cynecx]

Addressed the comments.

[cynecx]

Agree. If you know which frontend is going to use this, consider mentioning it, otherwise I'll just assume some Clang users will adopt the feature.

I've updated the description.

Thank You for reviewing this :)

[MaskRay]

llvm/lib/MC/MCParser/COFFAsmParser.cpp

371

parseSectionArguments

379–380

Don't format this unrelated part.

445

This error is not tested. See llvm-mc ... --defsym ERR=1 in llcm/test/MC for error testing.

llvm/test/MC/COFF/section.s

239

//

don't mix different comment markers.

[cynecx]

Addressed the remaining comments.

[MaskRay]

Thanks!

[cynecx]

The build failure seems unrelated (something in flang is failing, which I haven't touched here).

[MaskRay]

In D152085#4404241, @cynecx wrote:

The build failure seems unrelated (something in flang is failing, which I haven't touched here).

Seems to pass now. Yes, you can ignore unrelated failures.

[cynecx]

@MaskRay It would be great if you could commit these changes since I don't have commit access :)

llvm/lib/MC/MCParser/COFFAsmParser.cpp

class COFFAsmParser : public MCAsmParserExtension {
   void Initialize(MCAsmParser &Parser) override {
     // Call the base implementation.
     MCAsmParserExtension::Initialize(Parser);
 
     addDirectiveHandler<&COFFAsmParser::ParseSectionDirectiveText>(".text");
     addDirectiveHandler<&COFFAsmParser::ParseSectionDirectiveData>(".data");
     addDirectiveHandler<&COFFAsmParser::ParseSectionDirectiveBSS>(".bss");
     addDirectiveHandler<&COFFAsmParser::ParseDirectiveSection>(".section");
+    addDirectiveHandler<&COFFAsmParser::ParseDirectivePushSection>(
+        ".pushsection");
+    addDirectiveHandler<&COFFAsmParser::ParseDirectivePopSection>(
+        ".popsection");
     addDirectiveHandler<&COFFAsmParser::ParseDirectiveDef>(".def");
     addDirectiveHandler<&COFFAsmParser::ParseDirectiveScl>(".scl");
     addDirectiveHandler<&COFFAsmParser::ParseDirectiveType>(".type");
     addDirectiveHandler<&COFFAsmParser::ParseDirectiveEndef>(".endef");
     addDirectiveHandler<&COFFAsmParser::ParseDirectiveSecRel32>(".secrel32");
     addDirectiveHandler<&COFFAsmParser::ParseDirectiveSymIdx>(".symidx");
     addDirectiveHandler<&COFFAsmParser::ParseDirectiveSafeSEH>(".safeseh");
     addDirectiveHandler<&COFFAsmParser::ParseDirectiveSecIdx>(".secidx");
     addDirectiveHandler<&COFFAsmParser::ParseDirectiveLinkOnce>(".linkonce");
     addDirectiveHandler<&COFFAsmParser::ParseDirectiveRVA>(".rva");
     addDirectiveHandler<&COFFAsmParser::ParseDirectiveSymbolAttribute>(".weak");
     addDirectiveHandler<&COFFAsmParser::ParseDirectiveSymbolAttribute>(".weak_anti_dep");
     addDirectiveHandler<&COFFAsmParser::ParseDirectiveCGProfile>(".cg_profile");
 
     // Win64 EH directives.
     addDirectiveHandler<&COFFAsmParser::ParseSEHDirectiveStartProc>(
                                                                    ".seh_proc");
bool ParseSectionDirectiveBSS(StringRef, SMLoc) {
     return ParseSectionSwitch(".bss",
                               COFF::IMAGE_SCN_CNT_UNINITIALIZED_DATA
                             | COFF::IMAGE_SCN_MEM_READ
                             | COFF::IMAGE_SCN_MEM_WRITE,
                               SectionKind::getBSS());
   }
 
   bool ParseDirectiveSection(StringRef, SMLoc);
+  bool parseSectionArguments(StringRef, SMLoc);
+  bool ParseDirectivePushSection(StringRef, SMLoc);
+  bool ParseDirectivePopSection(StringRef, SMLoc);
   bool ParseDirectiveDef(StringRef, SMLoc);
   bool ParseDirectiveScl(StringRef, SMLoc);
   bool ParseDirectiveType(StringRef, SMLoc);
   bool ParseDirectiveEndef(StringRef, SMLoc);
   bool ParseDirectiveSecRel32(StringRef, SMLoc);
   bool ParseDirectiveSecIdx(StringRef, SMLoc);
   bool ParseDirectiveSafeSEH(StringRef, SMLoc);
   bool ParseDirectiveSymIdx(StringRef, SMLoc);
bool COFFAsmParser::ParseSectionName(StringRef &SectionName) {
   if (!getLexer().is(AsmToken::Identifier) && !getLexer().is(AsmToken::String))
     return true;
 
   SectionName = getTok().getIdentifier();
   Lex();
   return false;
 }
 
+bool COFFAsmParser::ParseDirectiveSection(StringRef directive, SMLoc loc) {
+  return parseSectionArguments(directive, loc);
+}
+
 // .section name [, "flags"] [, identifier [ identifier ], identifier]
+// .pushsection <same as above>
 //
 // Supported flags:
 //   a: Ignored.
 //   b: BSS section (uninitialized data)
 //   d: data section (initialized data)
 //   n: "noload" section (removed by linker)
 //   D: Discardable section
 //   r: Readable section
 //   s: Shared section
 //   w: Writable section
 //   x: Executable section
 //   y: Not-readable section (clears 'r')
 //
 // Subsections are not supported.
-bool COFFAsmParser::ParseDirectiveSection(StringRef, SMLoc) {
+bool COFFAsmParser::parseSectionArguments(StringRef, SMLoc) {
   StringRef SectionName;
 
   if (ParseSectionName(SectionName))
     return TokError("expected identifier in directive");
 
   unsigned Flags = COFF::IMAGE_SCN_CNT_INITIALIZED_DATA |
                    COFF::IMAGE_SCN_MEM_READ |
                    COFF::IMAGE_SCN_MEM_WRITE;
 
   if (getLexer().is(AsmToken::Comma)) {
     Lex();
 
     if (getLexer().isNot(AsmToken::String))
       return TokError("expected string in directive");
 
if (Kind.isText()) {
     const Triple &T = getContext().getTargetTriple();
     if (T.getArch() == Triple::arm || T.getArch() == Triple::thumb)
       Flags |= COFF::IMAGE_SCN_MEM_16BIT;
   }
   ParseSectionSwitch(SectionName, Flags, Kind, COMDATSymName, Type);
   return false;
 }
 
+bool COFFAsmParser::ParseDirectivePushSection(StringRef directive, SMLoc loc) {
+  getStreamer().pushSection();
+
+  if (parseSectionArguments(directive, loc)) {
+    getStreamer().popSection();
+    return true;
+  }
+
+  return false;
+}
+
+bool COFFAsmParser::ParseDirectivePopSection(StringRef, SMLoc) {
+  if (!getStreamer().popSection())
+    return TokError(".popsection without corresponding .pushsection");
+  return false;
+}
+
 bool COFFAsmParser::ParseDirectiveDef(StringRef, SMLoc) {
   StringRef SymbolName;
 
   if (getParser().parseIdentifier(SymbolName))
     return TokError("expected identifier in directive");
 
   MCSymbol *Sym = getContext().getOrCreateSymbol(SymbolName);
 

llvm/test/MC/COFF/section.s

 // RUN: llvm-mc -triple i386-pc-win32 -filetype=obj %s | llvm-readobj -S - | FileCheck %s
 // RUN: llvm-mc -triple x86_64-pc-win32 -filetype=obj %s | llvm-readobj -S - | FileCheck %s
+// RUN: not llvm-mc -triple x86_64-pc-win32 -filetype=obj --defsym ERR=1 %s -o /dev/null 2>&1 | FileCheck %s --check-prefix=ERR
 
 .section .foo$bar; .long 1
 .section .foo@bar; .long 1
 .section ABCDEFGHIJKLMNOPQRSTUVWXYZ; .long 1
 .section abcdefghijklmnopqrstuvwxyz; .long 1
 .section _0123456789; .long 1
 
 // CHECK: Sections [

 /// The section name can be quoted.
 .section "@#$-{","n"
 // CHECK:        Section {
 // CHECK-NEXT:     Number:
 // CHECK-NEXT:     Name: @#$-{
 
 // CHECK-NOT:    Section {
 // CHECK:      ]
+
+.section data1; .quad 0
+
+.pushsection data2; .quad 0
+.popsection
+
+// Back to section data1
+.quad 2
+
+// CHECK:       Section {
+// CHECK-NEXT:    Number:
+// CHECK-NEXT:    Name: data1
+// CHECK:         RawDataSize: 16
+
+// CHECK:       Section {
+// CHECK-NEXT:    Number:
+// CHECK-NEXT:    Name: data2
+// CHECK:         RawDataSize: 8
+
+.section .data3,"dw"; .quad 1
+
+.pushsection .data4,"dw"; .quad 1
+.popsection
+
+.pushsection .data5,"dr"; .quad 1
+.popsection
+
+// in section .data3
+.quad 4
+
+// Notice the different section flags here.
+// This shouldn't overwrite the intial section flags.
+.pushsection .data4,"dr"; .quad 1
+.popsection
+
+// CHECK:       Section {
+// CHECK-NEXT:    Number:
+// CHECK-NEXT:    Name: .data3
+// CHECK:         RawDataSize: 16
+// CHECK:         Characteristics [
+// CHECK-NEXT:      IMAGE_SCN_ALIGN_1BYTES
+// CHECK-NEXT:      IMAGE_SCN_CNT_INITIALIZED_DATA
+// CHECK-NEXT:      IMAGE_SCN_MEM_READ
+// CHECK-NEXT:      IMAGE_SCN_MEM_WRITE
+// CHECK-NEXT:    ]
+
+// CHECK:       Section {
+// CHECK-NEXT:    Number:
+// CHECK-NEXT:    Name: .data4
+// CHECK:         RawDataSize: 16
+// CHECK:         Characteristics [
+// CHECK-NEXT:      IMAGE_SCN_ALIGN_1BYTES
+// CHECK-NEXT:      IMAGE_SCN_CNT_INITIALIZED_DATA
+// CHECK-NEXT:      IMAGE_SCN_MEM_READ
+// CHECK-NEXT:      IMAGE_SCN_MEM_WRITE
+// CHECK-NEXT:    ]
+
+// CHECK:       Section {
+// CHECK-NEXT:    Number:
+// CHECK-NEXT:    Name: .data5
+// CHECK:         RawDataSize: 8
+// CHECK:         Characteristics [
+// CHECK-NEXT:      IMAGE_SCN_ALIGN_1BYTES
+// CHECK-NEXT:      IMAGE_SCN_CNT_INITIALIZED_DATA
+// CHECK-NEXT:      IMAGE_SCN_MEM_READ
+// CHECK-NEXT:    ]
+
+.ifdef ERR
+// ERR: :[[#@LINE+1]]:12: error: .popsection without corresponding .pushsection
+.popsection
+.endif