Re: Build failure in -next due to 'kbuild: allow archs to select link dead code/data elimination'

[Nicholas Piggin]

On Tue, 13 Sep 2016 13:25:22 -0700
Guenter Roeck <linux@xxxxxxxxxxxx> wrote:

> On 09/12/2016 08:51 PM, Nicholas Piggin wrote:
> > On Mon, 12 Sep 2016 20:17:30 -0700
> > Guenter Roeck <linux@xxxxxxxxxxxx> wrote:
> >  
> >> Hi Nicholas,
> >>
> >> On 09/12/2016 07:00 PM, Nicholas Piggin wrote:  
> >>> On Mon, 12 Sep 2016 15:24:43 -0700
> >>> Guenter Roeck <linux@xxxxxxxxxxxx> wrote:
> >>>  
> >>>> Hi,
> >>>>
> >>>> your commit 'kbuild: allow archs to select link dead code/data elimination'
> >>>> is causing the following build failure in -next when building score:defconfig.
> >>>>
> >>>> arch/score/kernel/built-in.o: In function `work_resched':
> >>>> arch/score/kernel/entry.o:(.text+0xe84):
> >>>> 	relocation truncated to fit: R_SCORE_PC19 against `schedule'
> >>>>
> >>>> Reverting the commit fixes the problem.
> >>>>
> >>>> Please let me know if I can help tracking down the problem.
> >>>> In case you need a score toochain, you can find the one I use at
> >>>> http://server.roeck-us.net/toolchains/score.tgz.
> >>>>
> >>>> Thanks,
> >>>> Guenter  
> >>>
> >>> It's not supposed to have any real effect unless the
> >>> option is selected, but there are a few changes to linker
> >>> script which must be causing it.
> >>>
> >>> There are two changes to vmlinux.lds.h. One is to KEEP a
> >>> few input sections, that *should* be kept anyway. The other
> >>> is to bring in additional sections into their correct output
> >>> section.
> >>>
> >>> Could you try reverting those lines of vmlinux.lds.h that
> >>> change the latter, i.e.:
> >>>
> >>> -		*(.text.hot .text .text.fixup .text.unlikely)		\
> >>> +		*(.text.hot .text .text.fixup .text.unlikely .text.*)	\  
> >>
> >> This is the culprit. After removing " .text.*" it builds fine.  
> >
> > Thanks for testing it. Some architectures have .text.x sections
> > not included here, I should have seen that. We should possibly
> > just revert that line and require implementing archs to do the
> > right thing.
> >  
> I would call the build failure a regression, so it should either be
> reverted, or we'll need some other solution to fix the build failure.

Yes it's definitely a regression and that part should be reverted.
To confirm, the following patch fixes your build?

Thanks,
Nick

commit 0ae28be83b4d6cd03ef5b481487d042f2b91954e
Author: Nicholas Piggin <npiggin@xxxxxxxxx>
Date:   Wed Sep 14 12:24:03 2016 +1000

    kbuild: -ffunction-sections fix for archs with conflicting sections
    
    Enabling -ffunction-sections modified the generic linker script to
    pull .text.* sections into regular TEXT_TEXT section, conflicting
    with some architectures. Revert that change and require archs that
    enable the option to ensure they have no conflicting section names,
    and do the appropriate merging.

    Signed-off-by: Nicholas Piggin <npiggin@xxxxxxxxx>

diff --git a/arch/Kconfig b/arch/Kconfig
index 6d5b631..8248037 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -487,7 +487,9 @@ config LD_DEAD_CODE_DATA_ELIMINATION
 	  This requires that the arch annotates or otherwise protects
 	  its external entry points from being discarded. Linker scripts
 	  must also merge .text.*, .data.*, and .bss.* correctly into
-	  output sections.
+	  output sections. Care must be taken not to pull in unrelated
+	  sections (e.g., '.text.init'). Typically '.' in section names
+	  is used to distinguish them from label names / C identifiers.
 
 config HAVE_CONTEXT_TRACKING
 	bool
diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index ad9d8f9..48dd44f 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -198,9 +198,9 @@
 
 /*
  * .data section
- * -fdata-sections generates .data.identifier which needs to be pulled in
- * with .data, but don't want to pull in .data..stuff which has its own
- * requirements. Same for bss.
+ * LD_DEAD_CODE_DATA_ELIMINATION option enables -fdata-sections generates
+ * .data.identifier which needs to be pulled in with .data, but don't want to
+ * pull in .data..stuff which has its own requirements. Same for bss.
  */
 #define DATA_DATA							\
 	*(.data .data.[0-9a-zA-Z_]*)					\
@@ -434,10 +434,15 @@
 	}
 
 /* .text section. Map to function alignment to avoid address changes
- * during second ld run in second ld pass when generating System.map */
+ * during second ld run in second ld pass when generating System.map
+ * LD_DEAD_CODE_DATA_ELIMINATION option enables -ffunction-sections generates
+ * .text.identifier which needs to be pulled in with .text , but some
+ * architectures define .text.foo which is not intended to be pulled in here.
+ * Those enabling LD_DEAD_CODE_DATA_ELIMINATION must ensure they don't have
+ * conflicting section names, and must pull in .text.[0-9a-zA-Z_]* */
 #define TEXT_TEXT							\
 		ALIGN_FUNCTION();					\
-		*(.text.hot .text .text.fixup .text.unlikely .text.*)	\
+		*(.text.hot .text .text.fixup .text.unlikely)		\
 		*(.ref.text)						\
 	MEM_KEEP(init.text)						\
 	MEM_KEEP(exit.text)						\