[PATCH] D12682: [MC/ELF] Accept zero for .align directive

[Rafael Espíndola via llvm-commits]

On 7 September 2015 at 20:00, Davide Italiano <dccitaliano at gmail.com> wrote:
> davide created this revision.
> davide added reviewers: Bigcheese, rafael, grosbach.
> davide added a subscriber: llvm-commits.
> davide set the repository for this revision to rL LLVM.
>
> .align directive refuses alignment 0 -- a comment in the code hints this is done for GNU as compatibility, but it seems GNU as accepts .align 0 (and silently rounds up alignment to 1). This patch is an attempt to fix.
>
> $ ./llvm-mc -filetype=obj -triple x86_64-pc-linux-gnu blah.s -o blah
> blah.s:4:8: error: alignment must be a power of 2
> .align 0
>        ^
> $ as blah.s -o blah
> $
>
> $ cat blah.s
> .global _start
> .global bar
>
> .align 0
> .text
> _start:
> bar:
>   movl $bar, %edx
>
>
> Repository:
>   rL LLVM
>
> http://reviews.llvm.org/D12682
>
> Files:
>   lib/MC/MCParser/AsmParser.cpp
>   test/MC/ELF/align-zero.s
>
> Index: test/MC/ELF/align-zero.s
> ===================================================================
> --- test/MC/ELF/align-zero.s
> +++ test/MC/ELF/align-zero.s
> @@ -0,0 +1,24 @@
> +// RUN: llvm-mc -filetype=obj -triple x86_64-pc-linux-gnu %s -o - | llvm-readobj -s | FileCheck %s
> +
> +// Test that an alignment of zero is accepted.
> +
> +  nop
> +  .align 0
> +  .text

Why the nop and the extra ".text"?


> +// CHECK: Section {
> +// CHECK:   Index: 2
> +// CHECK-NEXT:   Name: .text (1)
> +// CHECK-NEXT:   Type: SHT_PROGBITS (0x1)
> +// CHECK-NEXT:   Flags [ (0x6)
> +// CHECK-NEXT:     SHF_ALLOC (0x2)
> +// CHECK-NEXT:     SHF_EXECINSTR (0x4)
> +// CHECK-NEXT:   ]
> +// CHECK-NEXT:   Address: 0x0
> +// CHECK-NEXT:   Offset: 0x40
> +// CHECK-NEXT:   Size: 1
> +// CHECK-NEXT:   Link: 0
> +// CHECK-NEXT:   Info: 0
> +// CHECK-NEXT:   AddressAlignment: 4
> +// CHECK-NEXT:   EntrySize: 0
> +// CHECK-NEXT: }


I don't think the fileCheck helps in here. Any section we create has
an alignment of at least 1 (.text has 4). Just test that we don't
error.

LGTM with the test nits fixed.

Cheers,
Rafael