Support blank flag in SHT_GROUP sections for ELF

[serge-sans-paille]

Summary

It is possible to use group sections outside of COMDAT, but that's unsupported by lld. This makes lld unusuable

This is a tentative patch that just use the section content, without the duplication logic implied by COMDAT.

[ruiu]

Why are you trying to do this? It is intentional that only GRP_COMDAT is supported. That's the only flag defined by the ELF spec as you can see here: https://docs.oracle.com/cd/E19120-01/open.solaris/819-0690/chapter7-26/index.html

Other flags may be added in the future, but we'd implement them when they are added. Blindly handling all the undefined flags as if they were GRP_COMDAT is simply wrong. We should reject unknown flags.

[peter.smith]

For some background, the static glibc.a in the latest (unreleased) build of Fedora contains these non-comdat group sections.

There are 2 types of group, one called .text.unliklely.group (unlikely is spelled wrongly in the object files) and one called .text.hot.group. The .text.unlikely and .text.hot sections usually come about when the gcc optimizations like -freorder-blocks-and-partition and -freorder-functions are used. However I've never seen them generate an ELF group before, I can't for the life of me reproduce with trunk gcc, and I can't find them in the source code. Looking at the objects in the glibc-static-2.28.9000-28.fc30.i686.rpm I can see a .text.unliklely.group with these contents:

.text.unliklely
.gnu.build.attributes.unliklely
.rel.gnu.build.attributes.unliklely
.gnu.build.attributes..text.unliklely
.rel.gnu.build.attributes..text.unliklely
.text.unliklely
.text.unliklely
.text.unliklely.zzz

I think that the .gnu.build.attributes sections are not yet part of gcc trunk. There is a plugin mentioned in https://fedoraproject.org/wiki/Toolchain/Watermark so it is possible that these groups are only used when this plugin is used. As this is glibc then it would make lld unusable on the latest Fedora distributions. It would be good to try and find out why these groups are being created and whether it is down to the gcc plugin https://bugzilla.redhat.com/show_bug.cgi?id=1451407 mentioned in the Watermark link.

I've not had a chance to look over the code in fine detail yet. At a quick glance:

• I think the approach of just loading the sections is fine.
• I suggest writing the test in assembler as we tend to use that unless an object can't be built using it. It would need .section .text.foo, "axG", @progbits, .group to create the non comdat group.

[serge-sans-paille]

Thanks @psmith for the context. I just found the culprit line in gas assembler: https://github.com/bminor/binutils-gdb/blob/502c64b9ac12cf2a35d3cb55c51e2eefd33a2494/bfd/elf.c#L3590

But as mentionned by @ruiu, I cannot find any evidence of this being a standard behavior :-/ For instance MC/ELFObjectWriter.cpp unconditionnaly write all group sections as COMDAT.

[peter.smith]

I can confirm that the group sections are coming from the annobin tool.
From the man page: https://www.mankier.com/1/annobin

"attach""no-attach"
When gcc compiles code with the -ffunction-sections option active it will place each function into its own section.  When the annobin attach option is active the plugin will attempt to attach the function section to a group containing the notes and relocations for the function.  In that way, if the linker decides to discard the function, it will also know that it should discard the notes and relocations as well.

The default is to enable attach, but the inverse option is available in case the host assembler does not support the .attach_to_group pseudo-op.  If this feature is disabled then note generation for function sections will not work properly.

From https://developers.redhat.com/blog/2018/02/20/annobin-storing-information-binaries/ this appears to be on by default when building Redhat packages.

Both gold and ld.bfd appear to take into account groups when doing garbage collection. As it stands the patch would permit programs to link correctly, but if --gc-sections were on then LLD's implementation wouldn't throw away the .gnu.build.attributes sections (they are not SHF_ALLOC so are always marked live). This would lead to the tools that read the build information potentially giving incorrect results for some programs. How much of a problem this is would depend on the program, and how widely the watermark system is deployed.

[bd1976llvm]

I agree with Rui. My understanding is that groups in ELF were designed specifically to support C++ templates and inline functions which is why COMDAT groups are the only allowed kind.
Interestingly, I have been having a somewhat related discussion on this review: https://reviews.llvm.org/D53234.
Could the annobin tool use SHF_LINK_ORDER instead?

[serge-sans-paille]

As a follow up to this review (and the original bug), a thread started on the binutils mlist: https://www.sourceware.org/ml/binutils/2019-01/msg00046.html

A possible reading of the standard is that flags are additive and the absence of flag - i.e. 0, is a valid value.

[bd1976llvm]

The wording of the standard is unfortunate. This has been brought up and clarified (that COMDATs are the only legal groups currently) on the list previously.

[serge-sans-paille]

@bd1976llvm interesting. Unfortunately I failed to find this thread :-/

[bd1976llvm]

In D56437#1351440, @serge-sans-paille wrote:

@bd1976llvm interesting. Unfortunately I failed to find this thread :-/

https://groups.google.com/d/msg/generic-abi/_CbBM6T6WeM/cSELn86gAQAJ

What I think is happening here is that a mechanism, SHF_LINK_ORDER, was introduced into ELF for marking sections as associated with other sections for the purpose of garbage collection (dead/unused section removal). However, about the same time the GUI toolchain started using non-COMDAT groups in ELF to accomplish the same goal. In order to resolve this we will either need to get agreement from GNU to use SHF_LINK_ORDER instead or we will need to implement support for non-COMDAT ELF groups in LLVM in order to be compatible.

[serge-sans-paille]

@ruiu given you answer to the thread above, I understand we can go on with that review. Anything you're expecting in addition to the current patch?

[peter.smith]

I've had a look at the code in a bit more detail and have left some comments. I think we should state somewhere, either as a comment or in a commit message about how we handle garbage collection/icf for Section Groups. As it stands LLD marks all non-SHF_ALLOC sections as live so the primary use case for the non-comdat section groups (ensure removal of function removes all the build notes associated with it) isn't going to happen.

lld/ELF/InputFiles.cpp

322–323

We are getting more than just the entries now. Looking at how this function is now used (no assumption that the group is comdat), it might be better to inline this code in initializeSections so that the determination of type of group and any comment explaining the support can be in one place?

322–323

LLD coding style tends to favour not including { and } for a single statement if.

437–438

If getShtGroupEntries were inlined here we wouldn't need to mention the type of groups we support here.

Another thing we probably should mention is to what degree we support non-comdat groups. We unconditionally add the members, but we don't honour the garbage collection/icf behaviour implied by the specification for non-SHF_ALLOC sections in the group.

443

IsNew is associated with the hash table. Perhaps change the name to AddGroupMembers?

444

Could do

IsNew = true;
if (Flag !=0) {
        // De-duplicate comdat section groups by their signatures.
        StringRef Signature = getShtGroupSignature(ObjSections, Sec);
        IsNew = ComdatGroups.insert(CachedHashStringRef(Signature)).second;
        this->Sections[I] = &InputSection::Discarded;
}

[ruiu]

Given the discussion at the generic-abi mailing list, I'm fine with handling a group with no flags. However, I'd like you to add more code to this patch so that a group with no flags is actually handled as a group by garbage collector. Do you mind if I ask you do that?

[serge-sans-paille]

@ruiu I'm not that fmailiar with ldd codebase but I'll happily give it a try!

[ruiu]

I *believe* that the following strategy seems like a good way to implement the feature (but I didn't try it personally so if you find it unable to work, take another path):

Every input section have a list of DependentSections. Usually the list is empty, but if the list is not empty, the GC handles it as edges when doing the mark-sweep garbage collection.

I think you can represent a group with DependentSections. If one section in a group is live, all sections in the same group are live. You can pick the first section (an arbitrary section) as a "leader" in a group and add all sections in the group to its DependentSection, and add the leader to every member's DependentSection.

[bd1976llvm]

I strongly feel that we shouldn't implement this. This will tie groups and --gc-sections together when they are orthogonal features; e.g. COMDAT groups will *also* have to be considered by --gc-sections as a single unit. The ability to strip individual sections form COMDAT groups is an important feature. I will add similar comment to the abi list. I'm surprised that gold implements this behavior, back in 2017 gold and bfd-ld disagreed on this.

[serge-sans-paille]

Added rough support for dependencies withing a group.

The test case I'm interested in focuses on SHT_NOTE within the same group as text sections.
This delays the pruning of SHF_GROUP flags until later into the linker processing, because SHT_NOTE were considered as live by default, but what we want is to consider notes *not* within a group as live.

This aprt of the code is somehow clumsy, I'm eager to read your feedbacks.

[peter.smith]

Given the comments from bd1976llvm about the intention vs the wording of the ELF specification, and the restriction in this patch of the behaviour to non-comdat groups (assuming intentional) then I think we should take a step back and decide why we want to make the change and what the requirements are. I think that if we are taking the annobin interpretation of the specification with respect to garbage collection and what to be strictly compliant then we should apply the behaviour to all groups. Another interpretation is that only use case for non-comdat groups is to affect garbage collection so we can handle them differently to comdat groups; given the only use case I've seen in many years is annobin then this could be low risk without adding group member squared extra dependencies for all comdat groups. Alternatively if we just want to support annobin then there may be a better way of doing this without involving groups. There is a relocation from each SHT_NOTE section to the code section that it describes. It could be possible to handle these in the same way as .ARM.exidx sections by making the SHT_NOTE section from the group dependent on the code section it relocates against. If we did that then you'd only need to preserve the SHF_GROUP flag on the SHT_NOTES section and wouldn't need to make all the sections in the group depend on each other.

lld/ELF/InputFiles.cpp

399

This could be defined closer to its first use.

452

I think this comment belongs on line 472

456–457

If I understand it correctly we are adding dependencies between sections that are members of a group for non-comdat groups only? Whatever the interpretation of the spec is with respect to garbage collection the behaviour would apply equally to both comdat and non-comdat groups.

462

The ELF spec says that a SHT_GROUP section must come before any section that is a member of that group so I think all the references will be forward.

lld/ELF/MarkLive.cpp

179 ↗

(On Diff #181995)

typo withing -> within

274 ↗

(On Diff #181995)

LLD doesn't use {} for a single statement loop body.

309 ↗

(On Diff #181995)

I think this would unconditionally discard any SHT_NOTE section that wasn't part of a group as there would likely be no section to make it live. You'd probably need !(IsNote && IsGroupMember)

310 ↗

(On Diff #181995)

No {} here either.

lld/ELF/OutputSections.cpp

95 ↗

(On Diff #181995)

It is true that we should only ever see SHF_GROUP in object files, type ET_REL (such as those produced by Config->Relocatable).

I think that this would only clear the SHF_GROUP flag from the first Input Section in the OutputSection. If there happened to be more than InputSection the SHF_GROUP would get added on line 125. This would need to go prior to line 89.

Will be worth test cases to make sure that SHF_GROUP flag isn't propagated into an OutputSection for executables and shared libraries.

[serge-sans-paille]

Partial support of SHT_GROUP without flag

This does *not* implement full SHT_GROUP semantic, yet it is a simple step forward:
Sections within a group are still considered valid, but they do not behave as
specified by the standard in case of garbage collection.

@psmith considering your remark and the one from @bd1976llvm I 'd rather split the patch in two. This implements basic and partial support for SHT_GROUP. Instead of raising an error, it just acepts the sections as regular sections, and does not implement the dependency part. We can settle on a better support for annobin in another patch, but at least with this patch lld can be used as a normal linker on fedora.

[peter.smith]

I'm happy to split the patch into basic support and consider how we support GC separately.

[serge-sans-paille]

up!

[serge-sans-paille]

@ruiu is this minimal patch ok to you?

[ruiu]

I'm fine with just skipping a group that has no flags, but looks like to me this patch does it in a little complicated way. Do you mind if I ask you to simplify it? I'll also take a quick look at how to simplify it.

[ruiu]

Does this work for you?

https://gist.github.com/rui314/dc6d771f2fd9d4cb9f65ef6a970488b0

[serge-sans-paille]

@ruiu It works fine, I just moved the comdat-related insertion after the flag check. It compiles and validates fine on my setup.

[ruiu]

LGTM

lld/test/ELF/non-comdat-group.test

2

For consistency with other tests, rename this file sht-group-empty.test.

[thakis]

MSVC is printing lots of warnings about getShtGroupEntries(): https://logs.chromium.org/logs/chromium/bb/tryserver.chromium.win/win_upload_clang/471/+/recipes/steps/package_clang/0/stdout

c:\b\rr\tmpd0_670\w\src\third_party\llvm\tools\lld\elf\InputFiles.h(265): warning C4661: 'llvm::ArrayRef<llvm::support::detail::packed_endian_specific_integral<uint32_t,llvm::support::little,1>> lld::elf::ObjFile<llvm::object::ELF32LE>::getShtGroupEntries(const llvm::object::Elf_Shdr_Impl<llvm::object::ELFType<llvm::support::little,false>> &)': no suitable definition provided for explicit template instantiation request
c:\b\rr\tmpd0_670\w\src\third_party\llvm\tools\lld\elf\InputFiles.h(187): note: see declaration of 'lld::elf::ObjFile<llvm::object::ELF32LE>::getShtGroupEntries'
c:\b\rr\tmpd0_670\w\src\third_party\llvm\tools\lld\elf\InputFiles.h(265): warning C4661: 'llvm::ArrayRef<llvm::support::detail::packed_endian_specific_integral<uint32_t,llvm::support::big,1>> lld::elf::ObjFile<llvm::object::ELF32BE>::getShtGroupEntries(const llvm::object::Elf_Shdr_Impl<llvm::object::ELFType<llvm::support::big,false>> &)': no suitable definition provided for explicit template instantiation request
c:\b\rr\tmpd0_670\w\src\third_party\llvm\tools\lld\elf\InputFiles.h(187): note: see declaration of 'lld::elf::ObjFile<llvm::object::ELF32BE>::getShtGroupEntries'
c:\b\rr\tmpd0_670\w\src\third_party\llvm\tools\lld\elf\InputFiles.h(265): warning C4661: 'llvm::ArrayRef<llvm::support::detail::packed_endian_specific_integral<uint32_t,llvm::support::little,1>> lld::elf::ObjFile<llvm::object::ELF64LE>::getShtGroupEntries(const llvm::object::Elf_Shdr_Impl<llvm::object::ELFType<llvm::support::little,true>> &)': no suitable definition provided for explicit template instantiation request
c:\b\rr\tmpd0_670\w\src\third_party\llvm\tools\lld\elf\InputFiles.h(187): note: see declaration of 'lld::elf::ObjFile<llvm::object::ELF64LE>::getShtGroupEntries'
c:\b\rr\tmpd0_670\w\src\third_party\llvm\tools\lld\elf\InputFiles.h(265): warning C4661: 'llvm::ArrayRef<llvm::support::detail::packed_endian_specific_integral<uint32_t,llvm::support::big,1>> lld::elf::ObjFile<llvm::object::ELF64BE>::getShtGroupEntries(const llvm::object::Elf_Shdr_Impl<llvm::object::ELFType<llvm::support::big,true>> &)': no suitable definition provided for explicit template instantiation request
c:\b\rr\tmpd0_670\w\src\third_party\llvm\tools\lld\elf\InputFiles.h(187): note: see declaration of 'lld::elf::ObjFile<llvm::object::ELF64BE>::getShtGroupEntries'
C:\b\rr\tmpd0_670\w\src\third_party\llvm\tools\lld\ELF\InputFiles.cpp(304): warning C4661: 'llvm::ArrayRef<llvm::support::detail::packed_endian_specific_integral<uint32_t,llvm::support::little,1>> lld::elf::ObjFile<llvm::object::ELF32LE>::getShtGroupEntries(const llvm::object::Elf_Shdr_Impl<llvm::object::ELFType<llvm::support::little,false>> &)': no suitable definition provided for explicit template instantiation request
c:\b\rr\tmpd0_670\w\src\third_party\llvm\tools\lld\elf\InputFiles.h(187): note: see declaration of 'lld::elf::ObjFile<llvm::object::ELF32LE>::getShtGroupEntries'
C:\b\rr\tmpd0_670\w\src\third_party\llvm\tools\lld\ELF\InputFiles.cpp(304): warning C4661: 'llvm::ArrayRef<llvm::support::detail::packed_endian_specific_integral<uint32_t,llvm::support::big,1>> lld::elf::ObjFile<llvm::object::ELF32BE>::getShtGroupEntries(const llvm::object::Elf_Shdr_Impl<llvm::object::ELFType<llvm::support::big,false>> &)': no suitable definition provided for explicit template instantiation request
c:\b\rr\tmpd0_670\w\src\third_party\llvm\tools\lld\elf\InputFiles.h(187): note: see declaration of 'lld::elf::ObjFile<llvm::object::ELF32BE>::getShtGroupEntries'
C:\b\rr\tmpd0_670\w\src\third_party\llvm\tools\lld\ELF\InputFiles.cpp(304): warning C4661: 'llvm::ArrayRef<llvm::support::detail::packed_endian_specific_integral<uint32_t,llvm::support::little,1>> lld::elf::ObjFile<llvm::object::ELF64LE>::getShtGroupEntries(const llvm::object::Elf_Shdr_Impl<llvm::object::ELFType<llvm::support::little,true>> &)': no suitable definition provided for explicit template instantiation request
c:\b\rr\tmpd0_670\w\src\third_party\llvm\tools\lld\elf\InputFiles.h(187): note: see declaration of 'lld::elf::ObjFile<llvm::object::ELF64LE>::getShtGroupEntries'
C:\b\rr\tmpd0_670\w\src\third_party\llvm\tools\lld\ELF\InputFiles.cpp(304): warning C4661: 'llvm::ArrayRef<llvm::support::detail::packed_endian_specific_integral<uint32_t,llvm::support::big,1>> lld::elf::ObjFile<llvm::object::ELF64BE>::getShtGroupEntries(const llvm::object::Elf_Shdr_Impl<llvm::object::ELFType<llvm::support::big,true>> &)': no suitable definition provided for explicit template instantiation request
c:\b\rr\tmpd0_670\w\src\third_party\llvm\tools\lld\elf\InputFiles.h(187): note: see declaration of 'lld::elf::ObjFile<llvm::object::ELF64BE>::getShtGroupEntries'
C:\b\rr\tmpd0_670\w\src\third_party\llvm\include\llvm/Support/Casting.h(58): warning C4661: 'llvm::ArrayRef<llvm::support::detail::packed_endian_specific_integral<uint32_t,llvm::support::little,1>> lld::elf::ObjFile<llvm::object::ELF32LE>::getShtGroupEntries(const llvm::object::Elf_Shdr_Impl<llvm::object::ELFType<llvm::support::little,false>> &)': no suitable definition provided for explicit template instantiation request
c:\b\rr\tmpd0_670\w\src\third_party\llvm\tools\lld\elf\InputFiles.h(187): note: see declaration of 'lld::elf::ObjFile<llvm::object::ELF32LE>::getShtGroupEntries'
C:\b\rr\tmpd0_670\w\src\third_party\llvm\include\llvm/Support/Casting.h(58): warning C4661: 'llvm::ArrayRef<llvm::support::detail::packed_endian_specific_integral<uint32_t,llvm::support::big,1>> lld::elf::ObjFile<llvm::object::ELF32BE>::getShtGroupEntries(const llvm::object::Elf_Shdr_Impl<llvm::object::ELFType<llvm::support::big,false>> &)': no suitable definition provided for explicit template instantiation request
c:\b\rr\tmpd0_670\w\src\third_party\llvm\tools\lld\elf\InputFiles.h(187): note: see declaration of 'lld::elf::ObjFile<llvm::object::ELF32BE>::getShtGroupEntries'
C:\b\rr\tmpd0_670\w\src\third_party\llvm\include\llvm/Support/Casting.h(58): warning C4661: 'llvm::ArrayRef<llvm::support::detail::packed_endian_specific_integral<uint32_t,llvm::support::little,1>> lld::elf::ObjFile<llvm::object::ELF64LE>::getShtGroupEntries(const llvm::object::Elf_Shdr_Impl<llvm::object::ELFType<llvm::support::little,true>> &)': no suitable definition provided for explicit template instantiation request
c:\b\rr\tmpd0_670\w\src\third_party\llvm\tools\lld\elf\InputFiles.h(187): note: see declaration of 'lld::elf::ObjFile<llvm::object::ELF64LE>::getShtGroupEntries'
C:\b\rr\tmpd0_670\w\src\third_party\llvm\include\llvm/Support/Casting.h(58): warning C4661: 'llvm::ArrayRef<llvm::support::detail::packed_endian_specific_integral<uint32_t,llvm::support::big,1>> lld::elf::ObjFile<llvm::object::ELF64BE>::getShtGroupEntries(const llvm::object::Elf_Shdr_Impl<llvm::object::ELFType<llvm::support::big,true>> &)': no suitable definition provided for explicit template instantiation request
c:\b\rr\tmpd0_670\w\src\third_party\llvm\tools\lld\elf\InputFiles.h(187): note: see declaration of 'lld::elf::ObjFile<llvm::object::ELF64BE>::getShtGroupEntries'
C:\b\rr\tmpd0_670\w\src\third_party\llvm\include\llvm/ADT/STLExtras.h(125): warning C4661: 'llvm::ArrayRef<llvm::support::detail::packed_endian_specific_integral<uint32_t,llvm::support::little,1>> lld::elf::ObjFile<llvm::object::ELF32LE>::getShtGroupEntries(const llvm::object::Elf_Shdr_Impl<llvm::object::ELFType<llvm::support::little,false>> &)': no suitable definition provided for explicit template instantiation request
c:\b\rr\tmpd0_670\w\src\third_party\llvm\tools\lld\elf\InputFiles.h(187): note: see declaration of 'lld::elf::ObjFile<llvm::object::ELF32LE>::getShtGroupEntries'
C:\b\rr\tmpd0_670\w\src\third_party\llvm\include\llvm/ADT/STLExtras.h(125): warning C4661: 'llvm::ArrayRef<llvm::support::detail::packed_endian_specific_integral<uint32_t,llvm::support::big,1>> lld::elf::ObjFile<llvm::object::ELF32BE>::getShtGroupEntries(const llvm::object::Elf_Shdr_Impl<llvm::object::ELFType<llvm::support::big,false>> &)': no suitable definition provided for explicit template instantiation request
c:\b\rr\tmpd0_670\w\src\third_party\llvm\tools\lld\elf\InputFiles.h(187): note: see declaration of 'lld::elf::ObjFile<llvm::object::ELF32BE>::getShtGroupEntries'
C:\b\rr\tmpd0_670\w\src\third_party\llvm\include\llvm/ADT/STLExtras.h(125): warning C4661: 'llvm::ArrayRef<llvm::support::detail::packed_endian_specific_integral<uint32_t,llvm::support::little,1>> lld::elf::ObjFile<llvm::object::ELF64LE>::getShtGroupEntries(const llvm::object::Elf_Shdr_Impl<llvm::object::ELFType<llvm::support::little,true>> &)': no suitable definition provided for explicit template instantiation request
c:\b\rr\tmpd0_670\w\src\third_party\llvm\tools\lld\elf\InputFiles.h(187): note: see declaration of 'lld::elf::ObjFile<llvm::object::ELF64LE>::getShtGroupEntries'
C:\b\rr\tmpd0_670\w\src\third_party\llvm\include\llvm/ADT/STLExtras.h(125): warning C4661: 'llvm::ArrayRef<llvm::support::detail::packed_endian_specific_integral<uint32_t,llvm::support::big,1>> lld::elf::ObjFile<llvm::object::ELF64BE>::getShtGroupEntries(const llvm::object::Elf_Shdr_Impl<llvm::object::ELFType<llvm::support::big,true>> &)': no suitable definition provided for explicit template instantiation request
c:\b\rr\tmpd0_670\w\src\third_party\llvm\tools\lld\elf\InputFiles.h(187): note: see declaration of 'lld::elf::ObjFile<llvm::object::ELF64BE>::getShtGroupEntries'
C:\b\rr\tmpd0_670\w\src\third_party\llvm\include\llvm/Support/Error.h(790): warning C4661: 'llvm::ArrayRef<llvm::support::detail::packed_endian_specific_integral<uint32_t,llvm::support::little,1>> lld::elf::ObjFile<llvm::object::ELF32LE>::getShtGroupEntries(const llvm::object::Elf_Shdr_Impl<llvm::object::ELFType<llvm::support::little,false>> &)': no suitable definition provided for explicit template instantiation request
c:\b\rr\tmpd0_670\w\src\third_party\llvm\tools\lld\elf\InputFiles.h(187): note: see declaration of 'lld::elf::ObjFile<llvm::object::ELF32LE>::getShtGroupEntries'
C:\b\rr\tmpd0_670\w\src\third_party\llvm\include\llvm/Support/Error.h(790): warning C4661: 'llvm::ArrayRef<llvm::support::detail::packed_endian_specific_integral<uint32_t,llvm::support::big,1>> lld::elf::ObjFile<llvm::object::ELF32BE>::getShtGroupEntries(const llvm::object::Elf_Shdr_Impl<llvm::object::ELFType<llvm::support::big,false>> &)': no suitable definition provided for explicit template instantiation request
c:\b\rr\tmpd0_670\w\src\third_party\llvm\tools\lld\elf\InputFiles.h(187): note: see declaration of 'lld::elf::ObjFile<llvm::object::ELF32BE>::getShtGroupEntries'
C:\b\rr\tmpd0_670\w\src\third_party\llvm\include\llvm/Support/Error.h(790): warning C4661: 'llvm::ArrayRef<llvm::support::detail::packed_endian_specific_integral<uint32_t,llvm::support::little,1>> lld::elf::ObjFile<llvm::object::ELF64LE>::getShtGroupEntries(const llvm::object::Elf_Shdr_Impl<llvm::object::ELFType<llvm::support::little,true>> &)': no suitable definition provided for explicit template instantiation request
c:\b\rr\tmpd0_670\w\src\third_party\llvm\tools\lld\elf\InputFiles.h(187): note: see declaration of 'lld::elf::ObjFile<llvm::object::ELF64LE>::getShtGroupEntries'
C:\b\rr\tmpd0_670\w\src\third_party\llvm\include\llvm/Support/Error.h(790): warning C4661: 'llvm::ArrayRef<llvm::support::detail::packed_endian_specific_integral<uint32_t,llvm::support::big,1>> lld::elf::ObjFile<llvm::object::ELF64BE>::getShtGroupEntries(const llvm::object::Elf_Shdr_Impl<llvm::object::ELFType<llvm::support::big,true>> &)': no suitable definition provided for explicit template instantiation request
c:\b\rr\tmpd0_670\w\src\third_party\llvm\tools\lld\elf\InputFiles.h(187): note: see declaration of 'lld::elf::ObjFile<llvm::object::ELF64BE>::getShtGroupEntries'
C:\b\rr\tmpd0_670\w\src\third_party\depot_tools\win_toolchain\vs_files\818a152b3f1da991c1725d85be19a0f27af6bab4\win_sdk\bin\..\..\VC\Tools\MSVC\14.16.27023\include\type_traits(16707566): warning C4661: 'llvm::ArrayRef<llvm::support::detail::packed_endian_specific_integral<uint32_t,llvm::support::little,1>> lld::elf::ObjFile<llvm::object::ELF32LE>::getShtGroupEntries(const llvm::object::Elf_Shdr_Impl<llvm::object::ELFType<llvm::support::little,false>> &)': no suitable definition provided for explicit template instantiation request
c:\b\rr\tmpd0_670\w\src\third_party\llvm\tools\lld\elf\InputFiles.h(187): note: see declaration of 'lld::elf::ObjFile<llvm::object::ELF32LE>::getShtGroupEntries'
C:\b\rr\tmpd0_670\w\src\third_party\depot_tools\win_toolchain\vs_files\818a152b3f1da991c1725d85be19a0f27af6bab4\win_sdk\bin\..\..\VC\Tools\MSVC\14.16.27023\include\type_traits(16707566): warning C4661: 'llvm::ArrayRef<llvm::support::detail::packed_endian_specific_integral<uint32_t,llvm::support::big,1>> lld::elf::ObjFile<llvm::object::ELF32BE>::getShtGroupEntries(const llvm::object::Elf_Shdr_Impl<llvm::object::ELFType<llvm::support::big,false>> &)': no suitable definition provided for explicit template instantiation request
c:\b\rr\tmpd0_670\w\src\third_party\llvm\tools\lld\elf\InputFiles.h(187): note: see declaration of 'lld::elf::ObjFile<llvm::object::ELF32BE>::getShtGroupEntries'
C:\b\rr\tmpd0_670\w\src\third_party\depot_tools\win_toolchain\vs_files\818a152b3f1da991c1725d85be19a0f27af6bab4\win_sdk\bin\..\..\VC\Tools\MSVC\14.16.27023\include\type_traits(16707566): warning C4661: 'llvm::ArrayRef<llvm::support::detail::packed_endian_specific_integral<uint32_t,llvm::support::little,1>> lld::elf::ObjFile<llvm::object::ELF64LE>::getShtGroupEntries(const llvm::object::Elf_Shdr_Impl<llvm::object::ELFType<llvm::support::little,true>> &)': no suitable definition provided for explicit template instantiation request
c:\b\rr\tmpd0_670\w\src\third_party\llvm\tools\lld\elf\InputFiles.h(187): note: see declaration of 'lld::elf::ObjFile<llvm::object::ELF64LE>::getShtGroupEntries'
C:\b\rr\tmpd0_670\w\src\third_party\depot_tools\win_toolchain\vs_files\818a152b3f1da991c1725d85be19a0f27af6bab4\win_sdk\bin\..\..\VC\Tools\MSVC\14.16.27023\include\type_traits(16707566): warning C4661: 'llvm::ArrayRef<llvm::support::detail::packed_endian_specific_integral<uint32_t,llvm::support::big,1>> lld::elf::ObjFile<llvm::object::ELF64BE>::getShtGroupEntries(const llvm::object::Elf_Shdr_Impl<llvm::object::ELFType<llvm::support::big,true>> &)': no suitable definition provided for explicit template instantiation request
c:\b\rr\tmpd0_670\w\src\third_party\llvm\tools\lld\elf\InputFiles.h(187): note: see declaration of 'lld::elf::ObjFile<llvm::object::ELF64BE>::getShtGroupEntries'

It's probably complaining that a template function is defined out of line.

[ruiu]

Thank you for the heads-up. That function is gone, so we needed to remove that function from the header. I'll do that.

lld/ELF/InputFiles.cpp

StringRef ObjFile<ELFT>::getShtGroupSignature(ArrayRef<Elf_Shdr> Sections,
   // Such SHT_GROUP sections are invalid from the perspective of the ELF
   // standard, but GNU gold 1.14 (the newest version as of July 2017) or
   // older produce such sections as outputs for the -r option, so we need
   // a bug-compatibility.
   if (Signature.empty() && Sym->getType() == STT_SECTION)
     return getSectionName(Sec);
   return Signature;
 }
 
-template <class ELFT>
-ArrayRef<typename ObjFile<ELFT>::Elf_Word>
-ObjFile<ELFT>::getShtGroupEntries(const Elf_Shdr &Sec) {
-  const ELFFile<ELFT> &Obj = this->getObj();
-  ArrayRef<Elf_Word> Entries =
-      CHECK(Obj.template getSectionContentsAsArray<Elf_Word>(&Sec), this);
-  if (Entries.empty() || Entries[0] != GRP_COMDAT)
-    fatal(toString(this) + ": unsupported SHT_GROUP format");
-  return Entries.slice(1);
-}
-
 template <class ELFT> bool ObjFile<ELFT>::shouldMerge(const Elf_Shdr &Sec) {
   // On a regular link we don't merge sections if -O0 (default is -O1). This
   // sometimes makes the linker significantly faster, although the output will
   // be bigger.
   //
   // Doing the same for -r would create a problem as it would combine sections
   // with different sh_entsize. One option would be to just copy every SHF_MERGE
   // section as is to the output. While this would produce a valid ELF file with
   // usable SHF_MERGE sections, tools like (llvm-)?dwarfdump get confused when
void ObjFile<ELFT>::initializeSections(
   const ELFFile<ELFT> &Obj = this->getObj();
 
   ArrayRef<Elf_Shdr> ObjSections = CHECK(Obj.sections(), this);
   uint64_t Size = ObjSections.size();
   this->Sections.resize(Size);
   this->SectionStringTable =
       CHECK(Obj.getSectionStringTable(ObjSections), this);
 
   for (size_t I = 0, E = ObjSections.size(); I < E; I++) {
     if (this->Sections[I] == &InputSection::Discarded)
       continue;
     const Elf_Shdr &Sec = ObjSections[I];
 
     if (Sec.sh_type == ELF::SHT_LLVM_CALL_GRAPH_PROFILE)
       CGProfile = check(
           this->getObj().template getSectionContentsAsArray<Elf_CGProfile>(
               &Sec));
if ((Sec.sh_flags & SHF_EXCLUDE) && !Config->Relocatable) {
       this->Sections[I] = &InputSection::Discarded;
       continue;
     }
 
     switch (Sec.sh_type) {
     case SHT_GROUP: {
       // De-duplicate section groups by their signatures.
       StringRef Signature = getShtGroupSignature(ObjSections, Sec);
-      bool IsNew = ComdatGroups.insert(CachedHashStringRef(Signature)).second;
+
+      const ELFFile<ELFT> &Obj = this->getObj();
+      ArrayRef<Elf_Word> Entries =
+        CHECK(Obj.template getSectionContentsAsArray<Elf_Word>(&Sec), this);
       this->Sections[I] = &InputSection::Discarded;
 
-      // We only support GRP_COMDAT type of group. Get the all entries of the
-      // section here to let getShtGroupEntries to check the type early for us.
-      ArrayRef<Elf_Word> Entries = getShtGroupEntries(Sec);
+      // We only support default and GRP_COMDAT type of group.
+      // Default group members are unconditionnaly added to the target,
+      // and their grouping semantic is *not* honored by the garbage collector.
+      if (Entries.empty() || (Entries[0] != GRP_COMDAT && Entries[0] != 0))
+        fatal(toString(this) + ": unsupported SHT_GROUP format");
+
+      bool AddGroupMembers = true;
+      if(Entries[0] == GRP_COMDAT) {
+        // De-duplicate section groups by their signatures.
+        StringRef Signature = getShtGroupSignature(ObjSections, Sec);
+        AddGroupMembers = ComdatGroups.insert(CachedHashStringRef(Signature)).second;
+
+      }
 
-      // If it is a new section group, we want to keep group members.
       // Group leader sections, which contain indices of group members, are
       // discarded because they are useless beyond this point. The only
       // exception is the -r option because in order to produce re-linkable
       // object files, we want to pass through basically everything.
-      if (IsNew) {
+      // If it is a new section group, we want to keep group members.
+      if (AddGroupMembers) {
         if (Config->Relocatable)
           this->Sections[I] = createInputSection(Sec);
-        continue;
       }
-
+      else {
         // Otherwise, discard group members.
-      for (uint32_t SecIndex : Entries) {
+        for (uint32_t SecIndex : Entries.slice(1)) {
           if (SecIndex >= Size)
             fatal(toString(this) +
                   ": invalid section index in group: " + Twine(SecIndex));
           this->Sections[SecIndex] = &InputSection::Discarded;
         }
+      }
       break;
     }
     case SHT_SYMTAB:
       this->initSymtab(ObjSections, &Sec);
       break;
     case SHT_SYMTAB_SHNDX:
       this->SymtabSHNDX = CHECK(Obj.getSHNDXTable(Sec, ObjSections), this);
       break;

lld/test/ELF/non-comdat-group.test

+# RUN: yaml2obj %s -o %t.o
+# RUN: ld.lld %t.o %t.o -o %t -r
+# RUN: llvm-readobj -s %t | FileCheck %s
+
+# CHECK:     Name: .text.foo
+# CHECK:     Name: .rela.text.foo
+
+--- !ELF
+FileHeader:
+  Class:           ELFCLASS64
+  Data:            ELFDATA2LSB
+  Type:            ET_REL
+  Machine:         EM_X86_64
+Sections:
+  - Name:            .group
+    Type:            SHT_GROUP
+    Link:            .symtab
+    Info:            foo
+    Members:
+      - SectionOrType:    GRP_COMDAT
+      - SectionOrType:    .text.foo
+      - SectionOrType:    .text.bar
+      - SectionOrType:    .note
+  - Name:            .note
+    Type:            SHT_NOTE
+    Flags:           [ SHF_GROUP ]
+  - Name:            .text.foo
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_ALLOC, SHF_EXECINSTR, SHF_GROUP ]
+  - Name:            .text.bar
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_ALLOC, SHF_EXECINSTR, SHF_GROUP ]
+  - Name:            .rela.text.foo
+    Type:            SHT_RELA
+    Flags:           [ SHF_INFO_LINK, SHF_GROUP ]
+    Link:            .symtab
+    Info:            .text.foo
+    Relocations:
+      - Offset:          0x0000000000000000
+        Symbol:          foo
+        Type:            R_X86_64_64
+  - Name:            .rela.text.bar
+    Type:            SHT_RELA
+    Flags:           [ SHF_INFO_LINK, SHF_GROUP ]
+    Link:            .symtab
+    Info:            .text.bar
+    Relocations:
+      - Offset:          0x0000000000000000
+        Symbol:          bar
+        Type:            R_X86_64_64
+Symbols:
+  Global:
+    - Name:            foo
+    - Name:            bar
+