New user self-registration is disabled due to spam. For an account please email bugs-admin@lists.llvm.org with your e-mail address and full name.

Bug 48201 - [LLVM 11 regression] MC assembler check, rejects common assembler code as, e.g., generated by GCC
Summary: [LLVM 11 regression] MC assembler check, rejects common assembler code as, e....
Status: RESOLVED FIXED
Alias: None
Product: libraries
Classification: Unclassified
Component: MC (show other bugs)
Version: 11.0
Hardware: All Linux
: P normal
Assignee: Unassigned LLVM Bugs
URL:
Keywords: compile-fail, regression
Depends on:
Blocks: release-11.0.1
  Show dependency tree
 
Reported: 2020-11-17 04:46 PST by Tobias Burnus
Modified: 2020-12-14 14:36 PST (History)
5 users (show)

See Also:
Fixed By Commit(s): 1deff4009e0ae661b03682901bf6932297ce7ea1 700baa009dc6


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Tobias Burnus 2020-11-17 04:46:16 PST
LLVM's linker added a reasonable check in https://reviews.llvm.org/D73999

Such as (from the testsuite):

  .section .foo,"aM",@progbits,1
  .section .foo,"aM",@progbits,4
 error: changed section entsize for .foo, expected: 1 


However, it also now causes an error for:
        .section        .rodata.cst8,"aM",@progbits,8
        .section        .rodata.cst8


The problem is that GCC generates this by default (gcc/varasm.c):


  /* If we have already declared this section, we can use an
     abbreviated form to switch back to it -- unless this section is
     part of a COMDAT groups, in which case GAS requires the full
     declaration every time.  */
  if (!(HAVE_COMDAT_GROUP && (flags & SECTION_LINKONCE))
      && (flags & SECTION_DECLARED))
    { 
      fprintf (asm_out_file, "\t.section\t%s\n", name);
      return;
    }


LLVM and GNU as ("gas") tried to align, cf. discussion
  https://sourceware.org/legacy-ml/binutils/2020-02/msg00091.html
and the GAS patch to turn a warning to an error:
  https://sourceware.org/legacy-ml/binutils/2020-02/msg00129.html


However, while llvm/lib/MC/MCParser/ELFAsmParser.cpp always issues an error:

+  if (Section->getType() != Type)
+    Error(loc, "changed section type for " + SectionName + ", expected: 0x" +
+                   utohexstr(Section->getType()));
etc.

The GNU assembler only does so if flags have been specified,
gas/config/obj-elf.c:

      if (attr != 0)
        { 
          /* If section attributes are specified the second time we see a
             particular section, then check that they are the same as we
             saw the first time.  */
          if (((old_sec->flags ^ flags)
...
Comment 1 Tobias Burnus 2020-11-18 04:34:06 PST
Comment from Jakub Jelenik:
> Specifying section flags just on first .section directive and not others
> is correct, there is no point repeating that and GNU as (but I think many
> other assemblers) has been supporting it that way forever.
> The only time one needs to specify the section flags again is for comdat
> sections because then the section name is not unique, one needs section name
> and comdat pair...
Comment 2 Tobias Burnus 2020-11-24 12:04:57 PST
Suggested patch at https://reviews.llvm.org/D92052
Comment 3 Tobias Burnus 2020-12-14 01:01:25 PST
Patch committed to 'main' as 1deff4009e0ae661b03682901bf6932297ce7ea1
  https://reviews.llvm.org/rG1deff4009e0ae661b03682901bf6932297ce7ea1
Patch review was at https://reviews.llvm.org/D92052

@tstellar: It is a very small fix for a regression; it would be nice if it could be cherry picked. – Thanks.
Comment 4 Tom Stellard 2020-12-14 14:36:16 PST
Merged: 700baa009dc6