[ELF] Update flag propagation rule to ignore discarded output sections

[MaskRay]

Summary

See the updated insert-before.test for the effects: many synthetic
sections are SHF_ALLOC|SHF_WRITE. If they are discarded, we don't want
to propagate their flags to subsequent output section descriptions.

getFirstInputSection(sec) == nullptr can technically be merged into isDiscardable
but I'd like to postpone as not sharing getFirstInputSection(sec) == nullptr may give more refactoring .opportunity.

Depends on D118529.

[bluca]

Tested, this solves the problem with SHF_ALLOC in a SHT_NOTE section.

[peter.smith]

LGTM too.

lld/ELF/LinkerScript.cpp

1153

Trivial language nit, suggest one instead of an. // If sec has at least one input section and is not discarded.

[andrewng]

lld/ELF/LinkerScript.cpp

1152

I believe that all uses of isDiscardable() are now combined with getFirstInputSection() == nullptr, so perhaps getFirstInputSection() == nullptr should be part of isDiscardable()? Although that doesn't really "help" here as you need isEmpty.

[MaskRay]

fix a comment

lld/ELF/LinkerScript.cpp

1152

Agree with your analysis.

isEmpty makes the code sharing not much helpful. Two factors make me think we may need to postpone the code share:

• The code sharing causes getFirstInputSection(sec) to be called one extra time.
• I think we may need to refactor this logic once again. Adding getFirstInputSection(sec) might make refactoring difficult.

1153

Thanks. Fixed.

lld/ELF/LinkerScript.cpp

for (SectionCommand *&cmd : sectionCommands) {
     if (!sec)
       continue;
 
     // Handle align (e.g. ".foo : ALIGN(16) { ... }").
     if (sec->alignExpr)
       sec->alignment =
           std::max<uint32_t>(sec->alignment, sec->alignExpr().getValue());
 
-    // The input section might have been removed (if it was an empty synthetic
-    // section), but we at least know the flags.
-    if (sec->hasInputSections)
+    bool isEmpty = (getFirstInputSection(sec) == nullptr);
+    bool discardable = isEmpty && isDiscardable(*sec);
+    // If sec has at least one input section and not discarded, remember its
+    // flags to be inherited by subsequent output sections. (sec may contain
+    // just one empty synthetic section.)
+    if (sec->hasInputSections && !discardable)
       flags = sec->flags;
 
     // We do not want to keep any special flags for output section
     // in case it is empty.
-    bool isEmpty = (getFirstInputSection(sec) == nullptr);
     if (isEmpty)
       sec->flags = flags & ((sec->nonAlloc ? 0 : (uint64_t)SHF_ALLOC) |
                             SHF_WRITE | SHF_EXECINSTR);
 
     // The code below may remove empty output sections. We should save the
     // specified program headers (if exist) and propagate them to subsequent
     // sections which do not specify program headers.
     // An example of such a linker script is:
     // SECTIONS { .empty : { *(.empty) } :rw
     //            .foo : { *(.foo) } }
     // Note: at this point the order of output sections has not been finalized,
     // because orphans have not been inserted into their expected positions. We
     // will handle them in adjustSectionsAfterSorting().
     if (sec->sectionIndex != UINT32_MAX)
       maybePropagatePhdrs(*sec, defPhdrs);
 
-    if (isEmpty && isDiscardable(*sec)) {
+    if (discardable) {
       sec->markDead();
       cmd = nullptr;
     }
   }
 
   // It is common practice to use very generic linker scripts. So for any
   // given run some of the output sections in the script will be empty.
   // We could create corresponding empty output sections, but that would

lld/test/ELF/linkerscript/insert-before.test

 ## with a main linker script.
 
 # RUN: ld.lld --script %s %t1.o -o %t2
 # RUN: llvm-readelf -S -l %t2 | FileCheck --check-prefix=CHECK2 %s
 # CHECK2:      Name      Type     Address            Off      Size   ES Flg
 # CHECK2-NEXT:           NULL
 # CHECK2-NEXT: .foo.text PROGBITS 000000000020{{.*}} [[#%x,]] 000008 00  AX
 # CHECK2-NEXT: .text     PROGBITS [[#%x,]]           [[#%x,]] 000008 00  AX
-# CHECK2-NEXT: .byte     PROGBITS [[#%x,]]           [[#%x,]] 000001 00  WA
+# CHECK2-NEXT: .byte     PROGBITS [[#%x,]]           [[#%x,]] 000001 00  AX
 # CHECK2-NEXT: .foo.data PROGBITS [[#%x,]]           [[#%x,]] 000008 00  WA
 # CHECK2-NEXT: .data     PROGBITS [[#%x,]]           [[#%x,]] 000008 00  WA
 # CHECK2:      Type      {{.*}} Flg Align
 # CHECK2-NEXT: PHDR      {{.*}} R   0x8
 # CHECK2-NEXT: LOAD      {{.*}} R   0x1000
 # CHECK2-NEXT: LOAD      {{.*}} R E 0x1000
 # CHECK2-NEXT: LOAD      {{.*}} RW  0x1000
 # CHECK2-NEXT: GNU_STACK {{.*}} RW  0
 
 SECTIONS { .byte : { BYTE(0) } } INSERT BEFORE .data;
 
 SECTIONS { .foo.data : { *(.foo.data) } } INSERT BEFORE .data;
 
 ## The input section .foo.text is an orphan. It will be placed in .foo.text
 SECTIONS { .foo.text : {} } INSERT BEFORE .text;