[PATCH] D41559: [MC] - Disallow invalid section groups declarations.

[Rafael Avila de Espindola via llvm-commits]

LGTM.

Thanks!

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

> grimar created this revision.
> grimar added a reviewer: rafael.
> Herald added a subscriber: emaste.
>
> This fixes parseGroup() so that it always sets error condition on error.
> Previously it was not done, because `parseIdentifier` looks never do that,
> assuiming that caller should do it if he wants to.
>
> So previously cases from test were silently accepted and produced broken output.
>
>
> https://reviews.llvm.org/D41559
>
> Files:
>   lib/MC/MCParser/ELFAsmParser.cpp
>   test/MC/ELF/comdat-declaration-errors.s
>
>
> Index: test/MC/ELF/comdat-declaration-errors.s
> ===================================================================
> --- test/MC/ELF/comdat-declaration-errors.s
> +++ test/MC/ELF/comdat-declaration-errors.s
> @@ -0,0 +1,14 @@
> +// RUN: not llvm-mc -triple x86_64-pc-linux-gnu %s \
> +// RUN:   -filetype=obj -o %t.o 2>&1 | FileCheck %s
> +
> +// Check we error out on incorrect COMDATs declarations
> +// and not just silently ingnore them.
> +
> +// CHECK:      error: invalid group name
> +// CHECK-NEXT: .section .foo,"G", at progbits,-abc,comdat
> +
> +// CHECK:      error: invalid linkage
> +// CHECK-NEXT: .section .bar,"G", at progbits,abc,-comdat
> +
> +.section .foo,"G", at progbits,-abc,comdat
> +.section .bar,"G", at progbits,abc,-comdat
> Index: lib/MC/MCParser/ELFAsmParser.cpp
> ===================================================================
> --- lib/MC/MCParser/ELFAsmParser.cpp
> +++ lib/MC/MCParser/ELFAsmParser.cpp
> @@ -427,13 +427,13 @@
>      GroupName = getTok().getString();
>      Lex();
>    } else if (getParser().parseIdentifier(GroupName)) {
> -    return true;
> +    return TokError("invalid group name");
>    }
>    if (L.is(AsmToken::Comma)) {
>      Lex();
>      StringRef Linkage;
>      if (getParser().parseIdentifier(Linkage))
> -      return true;
> +      return TokError("invalid linkage");
>      if (Linkage != "comdat")
>        return TokError("Linkage must be 'comdat'");
>    }
>
>
> Index: test/MC/ELF/comdat-declaration-errors.s
> ===================================================================
> --- test/MC/ELF/comdat-declaration-errors.s
> +++ test/MC/ELF/comdat-declaration-errors.s
> @@ -0,0 +1,14 @@
> +// RUN: not llvm-mc -triple x86_64-pc-linux-gnu %s \
> +// RUN:   -filetype=obj -o %t.o 2>&1 | FileCheck %s
> +
> +// Check we error out on incorrect COMDATs declarations
> +// and not just silently ingnore them.
> +
> +// CHECK:      error: invalid group name
> +// CHECK-NEXT: .section .foo,"G", at progbits,-abc,comdat
> +
> +// CHECK:      error: invalid linkage
> +// CHECK-NEXT: .section .bar,"G", at progbits,abc,-comdat
> +
> +.section .foo,"G", at progbits,-abc,comdat
> +.section .bar,"G", at progbits,abc,-comdat
> Index: lib/MC/MCParser/ELFAsmParser.cpp
> ===================================================================
> --- lib/MC/MCParser/ELFAsmParser.cpp
> +++ lib/MC/MCParser/ELFAsmParser.cpp
> @@ -427,13 +427,13 @@
>      GroupName = getTok().getString();
>      Lex();
>    } else if (getParser().parseIdentifier(GroupName)) {
> -    return true;
> +    return TokError("invalid group name");
>    }
>    if (L.is(AsmToken::Comma)) {
>      Lex();
>      StringRef Linkage;
>      if (getParser().parseIdentifier(Linkage))
> -      return true;
> +      return TokError("invalid linkage");
>      if (Linkage != "comdat")
>        return TokError("Linkage must be 'comdat'");
>    }