[llvm] r336072 - Add an entry for rodata constant merge sections to the default

[Sam Clegg via llvm-commits]

Should we revert this change and reland with the fix when its ready?

Bug is here: https://bugs.llvm.org/show_bug.cgi?id=38165
On Tue, Jul 17, 2018 at 2:20 PM Eric Christopher <echristo at gmail.com> wrote:
>
> Looking. Thanks :)
>
> On Fri, Jul 13, 2018 at 3:10 PM Sam Clegg <sbc at google.com> wrote:
>>
>> I believe this change causes previously valid .S files to fail to compile.
>>
>> The error I'm seeing is on the following line:
>> .section    .rodata.cst8,"a", at progbits
>>
>> This generates:
>> amd64.S:728:44: error: expected the entry size
>>         .section .rodata.cst8,"a", at progbits
>>
>> I guess the entry size here was previously implied by the "cst8" name?
>>
>> The file in question is part of ocaml source:
>> https://github.com/ocaml/ocaml/blob/trunk/runtime/amd64.S#L727
>> On Sun, Jul 1, 2018 at 5:21 PM Eric Christopher via llvm-commits
>> <llvm-commits at lists.llvm.org> wrote:
>> >
>> > Author: echristo
>> > Date: Sun Jul  1 17:16:39 2018
>> > New Revision: 336072
>> >
>> > URL: http://llvm.org/viewvc/llvm-project?rev=336072&view=rev
>> > Log:
>> > Add an entry for rodata constant merge sections to the default
>> > section flags in the ELF assembler. This matches the defaults
>> > given in the rest of MC.
>> >
>> > Fixes PR37997 where we couldn't assemble our own assembly output
>> > without warnings.
>> >
>> > Modified:
>> >     llvm/trunk/lib/MC/MCParser/ELFAsmParser.cpp
>> >     llvm/trunk/test/MC/ELF/extra-section-flags.s
>> >
>> > Modified: llvm/trunk/lib/MC/MCParser/ELFAsmParser.cpp
>> > URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/MC/MCParser/ELFAsmParser.cpp?rev=336072&r1=336071&r2=336072&view=diff
>> > ==============================================================================
>> > --- llvm/trunk/lib/MC/MCParser/ELFAsmParser.cpp (original)
>> > +++ llvm/trunk/lib/MC/MCParser/ELFAsmParser.cpp Sun Jul  1 17:16:39 2018
>> > @@ -486,6 +486,9 @@ static bool hasPrefix(StringRef SectionN
>> >  // defaults.
>> >  static unsigned defaultSectionFlags(StringRef SectionName) {
>> >
>> > +  if (hasPrefix(SectionName, ".rodata.cst"))
>> > +    return ELF::SHF_ALLOC | ELF::SHF_MERGE;
>> > +
>> >    if (hasPrefix(SectionName, ".rodata.") || SectionName == ".rodata1")
>> >      return ELF::SHF_ALLOC;
>> >
>> >
>> > Modified: llvm/trunk/test/MC/ELF/extra-section-flags.s
>> > URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/MC/ELF/extra-section-flags.s?rev=336072&r1=336071&r2=336072&view=diff
>> > ==============================================================================
>> > --- llvm/trunk/test/MC/ELF/extra-section-flags.s (original)
>> > +++ llvm/trunk/test/MC/ELF/extra-section-flags.s Sun Jul  1 17:16:39 2018
>> > @@ -1,10 +1,12 @@
>> >  # RUN: llvm-mc -triple x86_64-unknown-unknown -filetype=obj %s -o /dev/null 2>&1 | FileCheck %s
>> > -
>> > +
>> >  .section .rodata, "ax"
>> >  # CHECK: warning: setting incorrect section attributes for .rodata
>> >  nop
>> >
>> >  .section .rodata, "a"
>> > -# CHECK-NOT: warning:
>> >  nop
>> > +.section .rodata.cst4, "aM", at progbits,8
>> > +nop
>> > +# CHECK-NOT: warning:
>> >
>> >
>> >
>> > _______________________________________________
>> > llvm-commits mailing list
>> > llvm-commits at lists.llvm.org
>> > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits