[PATCH] D41552: [MC] - Teach llvm-mc to handle comdats whose names are numbers.

[Rafael Avila de Espindola via llvm-commits]

LGTM

A nice followup would be to make sure we produce an error if we can't
parse to the end of the line.

Cheers,
Rafael

George Rimar via Phabricator <reviews at reviews.llvm.org> writes:

> grimar created this revision.
> grimar added a reviewer: rafael.
> Herald added a subscriber: emaste.
>
> Currently llvm-mc ignores COMDATs whose names are numbers,
> for example following code:
>
>   .section .foo,"G", at progbits,123,comdat
>
> would produce no COMDATs at all.
>
> Fixes PR35718.
>
>
> https://reviews.llvm.org/D41552
>
> Files:
>   lib/MC/MCParser/ELFAsmParser.cpp
>   test/MC/ELF/comdat-name-number.s
>
>
> Index: test/MC/ELF/comdat-name-number.s
> ===================================================================
> --- test/MC/ELF/comdat-name-number.s
> +++ test/MC/ELF/comdat-name-number.s
> @@ -0,0 +1,28 @@
> +// RUN: llvm-mc -triple x86_64-pc-linux-gnu %s -filetype=obj -o %t.o 
> +// RUN: llvm-readobj -elf-section-groups %t.o | FileCheck %s
> +
> +// Test that we can handle numeric COMDAT names.
> +
> +.section .foo,"G", at progbits,123,comdat
> +.section .bar,"G", at progbits,abc,comdat
> +
> +// CHECK:      Groups {
> +// CHECK-NEXT:   Group {
> +// CHECK-NEXT:     Name: .group
> +// CHECK-NEXT:     Index:
> +// CHECK-NEXT:     Type: COMDAT
> +// CHECK-NEXT:     Signature: 123
> +// CHECK-NEXT:     Section(s) in group [
> +// CHECK-NEXT:       .foo
> +// CHECK-NEXT:     ]
> +// CHECK-NEXT:   }
> +// CHECK-NEXT:   Group {
> +// CHECK-NEXT:     Name: .group
> +// CHECK-NEXT:     Index:
> +// CHECK-NEXT:     Type: COMDAT
> +// CHECK-NEXT:     Signature: abc
> +// CHECK-NEXT:     Section(s) in group [
> +// CHECK-NEXT:       .bar
> +// CHECK-NEXT:     ]
> +// CHECK-NEXT:   }
> +// CHECK-NEXT: }
> Index: lib/MC/MCParser/ELFAsmParser.cpp
> ===================================================================
> --- lib/MC/MCParser/ELFAsmParser.cpp
> +++ lib/MC/MCParser/ELFAsmParser.cpp
> @@ -423,8 +423,12 @@
>    if (L.isNot(AsmToken::Comma))
>      return TokError("expected group name");
>    Lex();
> -  if (getParser().parseIdentifier(GroupName))
> +  if (L.is(AsmToken::Integer)) {
> +    GroupName = getTok().getString();
> +    Lex();
> +  } else if (getParser().parseIdentifier(GroupName)) {
>      return true;
> +  }
>    if (L.is(AsmToken::Comma)) {
>      Lex();
>      StringRef Linkage;
>
>
> Index: test/MC/ELF/comdat-name-number.s
> ===================================================================
> --- test/MC/ELF/comdat-name-number.s
> +++ test/MC/ELF/comdat-name-number.s
> @@ -0,0 +1,28 @@
> +// RUN: llvm-mc -triple x86_64-pc-linux-gnu %s -filetype=obj -o %t.o 
> +// RUN: llvm-readobj -elf-section-groups %t.o | FileCheck %s
> +
> +// Test that we can handle numeric COMDAT names.
> +
> +.section .foo,"G", at progbits,123,comdat
> +.section .bar,"G", at progbits,abc,comdat
> +
> +// CHECK:      Groups {
> +// CHECK-NEXT:   Group {
> +// CHECK-NEXT:     Name: .group
> +// CHECK-NEXT:     Index:
> +// CHECK-NEXT:     Type: COMDAT
> +// CHECK-NEXT:     Signature: 123
> +// CHECK-NEXT:     Section(s) in group [
> +// CHECK-NEXT:       .foo
> +// CHECK-NEXT:     ]
> +// CHECK-NEXT:   }
> +// CHECK-NEXT:   Group {
> +// CHECK-NEXT:     Name: .group
> +// CHECK-NEXT:     Index:
> +// CHECK-NEXT:     Type: COMDAT
> +// CHECK-NEXT:     Signature: abc
> +// CHECK-NEXT:     Section(s) in group [
> +// CHECK-NEXT:       .bar
> +// CHECK-NEXT:     ]
> +// CHECK-NEXT:   }
> +// CHECK-NEXT: }
> Index: lib/MC/MCParser/ELFAsmParser.cpp
> ===================================================================
> --- lib/MC/MCParser/ELFAsmParser.cpp
> +++ lib/MC/MCParser/ELFAsmParser.cpp
> @@ -423,8 +423,12 @@
>    if (L.isNot(AsmToken::Comma))
>      return TokError("expected group name");
>    Lex();
> -  if (getParser().parseIdentifier(GroupName))
> +  if (L.is(AsmToken::Integer)) {
> +    GroupName = getTok().getString();
> +    Lex();
> +  } else if (getParser().parseIdentifier(GroupName)) {
>      return true;
> +  }
>    if (L.is(AsmToken::Comma)) {
>      Lex();
>      StringRef Linkage;