Bug 245179 - lld: wrong/misleading "SHF_MERGE section size must be a multiple of sh_entsize"
Summary: lld: wrong/misleading "SHF_MERGE section size must be a multiple of sh_entsize"
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: bin (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Some People
Assignee: freebsd-toolchain (Nobody)
URL:
Keywords: needs-qa
Depends on:
Blocks:
 
Reported: 2020-03-30 02:39 UTC by Andrew "RhodiumToad" Gierth
Modified: 2021-11-08 17:43 UTC (History)
4 users (show)

See Also:
andrew: mfc-stable12?
andrew: mfc-stable11?


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Andrew "RhodiumToad" Gierth 2020-03-30 02:39:54 UTC
In function ObjFile<ELFT>::shouldMerge in lld/ELF/InputFiles.cpp:

  if (sec.sh_size % entSize)
    fatal(toString(this) +
          ": SHF_MERGE section size must be a multiple of sh_entsize");

  uint64_t flags = sec.sh_flags;
  if (!(flags & SHF_MERGE))
    return false;

Notice that the size is checked _before_ looking at whether SHF_MERGE is set. This means that the error is produced for sections that do not have SHF_MERGE set at all, which is either misleading or wrong.

Judging by the report in #219717 where this was found, the BFD linker did not care about sh_entsize of non-mergeable sections. If this check is only needed for mergeable sections, then surely it should be made _after_ SHF_MERGE is checked for.

Since this check is skipped when -Wl,-O0 is in effect, I do not believe it is actually necessary to check that size is a multiple of sh_entsize when not merging sections. If I'm wrong about that, though, then the check would need to be both moved to somewhere else and changed to not mention SHF_MERGE in the error message.
Comment 1 Ed Maste freebsd_committer freebsd_triage 2020-03-30 17:34:27 UTC
Would you submit an upstream bug for lld at https://bugs.llvm.org/
Comment 2 Ed Maste freebsd_committer freebsd_triage 2020-03-30 17:35:59 UTC
(I will look at this as soon as I'm able, if it's not handled upstream first.)
Comment 3 Andrew "RhodiumToad" Gierth 2020-03-30 22:31:42 UTC
I don't currently have an account for upstream.
Comment 4 Andrew "RhodiumToad" Gierth 2020-03-31 09:56:05 UTC
Submitted upstream as https://bugs.llvm.org/show_bug.cgi?id=45370
Comment 5 commit-hook freebsd_committer freebsd_triage 2021-11-08 17:31:00 UTC
A commit in branch stable/12 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=2f4812936a2ccaadd260f6bbaacd677e6079d9d0

commit 2f4812936a2ccaadd260f6bbaacd677e6079d9d0
Author:     Dimitry Andric <dim@FreeBSD.org>
AuthorDate: 2021-11-08 10:19:59 +0000
Commit:     Dimitry Andric <dim@FreeBSD.org>
CommitDate: 2021-11-08 17:29:35 +0000

    Fix lld warning "SHF_MERGE section size must be a multiple of sh_entsize"

    Merge commit 56decd982dc0 from llvm git (by Fangrui Song):

      [ELF] Allow invalid sh_size%sh_entsize!=0 for non-SHF_MERGE sections

      Fixes https://bugs.llvm.org/show_bug.cgi?id=45370
      Fixes https://github.com/Clozure/ccl/issues/273

      .stab holds a table of 12-byte entries. GNU as before 2.35 incorrectly
      sets sh_entsize(.stab) to 20 on 64-bit architectures:
      https://sourceware.org/bugzilla/show_bug.cgi?id=25768

      We should not emit the confusing error:
      "SHF_MERGE section size (...) must be a multiple of sh_entsize (20)

      Reviewed By: grimar, psmith

      Differential Revision: https://reviews.llvm.org/D77368

    Direct commit to stable/12, since newer branches have this fix already.

    PR:             245179
    Reported by:    andrew@tao11.riddles.org.uk

 contrib/llvm-project/lld/ELF/InputFiles.cpp | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)