[Bug ld/32961] New: ".pushsection" may introduce unnecessary section dependency which impacts "--gc-sections"

2025-05-12 Thread zhiyuan.lv at linux dot intel.com
https://sourceware.org/bugzilla/show_bug.cgi?id=32961

Bug ID: 32961
   Summary: ".pushsection" may introduce unnecessary section
dependency which impacts "--gc-sections"
   Product: binutils
   Version: 2.42
Status: UNCONFIRMED
  Severity: normal
  Priority: P2
 Component: ld
  Assignee: unassigned at sourceware dot org
  Reporter: zhiyuan.lv at linux dot intel.com
  Target Milestone: ---

While compiling some Linux kernel files with "-ffunction-sections" and linker
parameter "--gc-sections", I found that several unused functions were not
removed as expected, and the root cause is unnecessary section dependencies
introduced by ".pushsection" directive. I simplified the case as below and
would like to get feedback from binutils developers of any existing workable
ways, or we could consider a new feature request to ld. Appreciate all the
input!

The case of test.c:

static inline __attribute__((always_inline)) void test_section(void)
{
asm goto(
"jmp 1f\n"
".pushsection .alt_section, \"ax\"\n"
"1:\n"
" jmp %l[t_label]\n"
".popsection\n"
: :
: : t_label);
t_label:
return;
}

void foo1(void)
{
}

void foo2(void)
{
test_section();
}

void foo3(void)
{
test_section();
}

void entry_func(void)
{
foo1();
foo3();
}

The build command: gcc test.c -ffunction-sections -Wl,-e entry_func
-Wl,--gc-sections -o test -nostdlib -O0

My environment is Ubuntu 24.04, with LD version 2.42.

The expected result is that foo2() is optimized away in generated binary by
linker, but it can still be seen with "objdump -S test".

The root cause is at ".alt_section". If we check the section dependency from
relocation table in test.o generated by "gcc test.c -c -ffunction-sections -o
test.o", foo3 has relocation entry pointing to ".alt_section" and
".alt_section" has relocation try to ".foo2", which prevents LD from removing
foo2.

If I remove the inline attribute of test_section(), the function foo2() can be
removed by linker, because relocation entry of .alt_section will not link to
foo2 or foo3. Also, if there is no "test_section()" call in foo3(), foo2() can
be removed as well.

The linker does not do anything wrong, but in my case of Linux kernel from
"_static_cpu_has()" in arch/x86/include/asm/cpufeature.h, I cannot simply
remove inline and there does not seem to be a good solution.

If all above is correct, could we consider below two options?

1. Add a new type of ".pushsection", say, ".pushnewsection", which will always
create a new section.

2. Let LD be able to ignore some given sections while doing the section
dependency calculation. If a symbol is deleted, simply remove the relocation
entry in the specified "ignored section".

Either one can address above case of test.c. Any comments? Thanks!

-- 
You are receiving this mail because:
You are on the CC list for the bug.


[Bug ld/32961] ".pushsection" may introduce unnecessary section dependency which impacts "--gc-sections"

2025-05-13 Thread zhiyuan.lv at linux dot intel.com
https://sourceware.org/bugzilla/show_bug.cgi?id=32961

--- Comment #12 from Zhiyuan Lv  ---
Hi H.J.,

On Tue, May 13, 2025 at 07:15:14AM +, hjl.tools at gmail dot com wrote:
> https://sourceware.org/bugzilla/show_bug.cgi?id=32961
> 
> --- Comment #8 from H.J. Lu  ---
> (In reply to Zhiyuan Lv from comment #7)
> > And, the GCC version I used is 14.2.0, which will generate per function
> > section of "__patchable_function_entries". Thanks!
> 
> Now, the question is if GCC should generate a unique section name for
> __patchable_function_entries -ffunction-sections.  If I give you a

Just curious: is the same name a problem here? I saw that gcc 14
generates __patchable** sections for each function with the same name,
and they seem to work fine: each section has its own relocation section
respectively. Why does "-ffunction-sections" cause problems? Thanks!

-Zhiyuan

> GCC patch, can you try it?
> 
> -- 
> You are receiving this mail because:
> You are on the CC list for the bug.

-- 
You are receiving this mail because:
You are on the CC list for the bug.


[Bug ld/32961] ".pushsection" may introduce unnecessary section dependency which impacts "--gc-sections"

2025-05-13 Thread zhiyuan.lv at linux dot intel.com
https://sourceware.org/bugzilla/show_bug.cgi?id=32961

--- Comment #9 from Zhiyuan Lv  ---
(In reply to H.J. Lu from comment #8)
> (In reply to Zhiyuan Lv from comment #7)
> > And, the GCC version I used is 14.2.0, which will generate per function
> > section of "__patchable_function_entries". Thanks!
> 
> Now, the question is if GCC should generate a unique section name for
> __patchable_function_entries -ffunction-sections.  If I give you a
> GCC patch, can you try it?

Sure I can try, but need to take some time to build a gcc from source first,
may only be able to try that tomorrow ...

-- 
You are receiving this mail because:
You are on the CC list for the bug.


[Bug ld/32961] ".pushsection" may introduce unnecessary section dependency which impacts "--gc-sections"

2025-05-13 Thread zhiyuan.lv at linux dot intel.com
https://sourceware.org/bugzilla/show_bug.cgi?id=32961

--- Comment #6 from Zhiyuan Lv  ---
(In reply to H.J. Lu from comment #5)
> Created attachment 16083 [details]
> A patch to add --unique-pushsection and .pushuniquesection
> 
> Try this.

Hi H.J, the patch works nicely with my simple test case, but still did not get
expected result while trying with Linux kernel build. I did some investigation
and found it to be related to "-fpatchable-function-entry=16,16". If I add this
gcc option to compile test.c, the relocation entry for .alt_section is
combined, causing the section dependencies again.

I checked the section names of test.o with and without
"patchable-function-entry". without the parameter, section names are:
".alt_section.text.foo1"
".alt_section.text.foo2"

with the parameter, section name is:
"alt_section__patchable_function_entries"

And, another thing weird is that: the macro is used in functions foo2 and foo3,
but the alt_function name suffix is "foo1"/"foo2". Could that be a hint?

BTW, thanks much for the very prompt patches! Appreciate that!

-- 
You are receiving this mail because:
You are on the CC list for the bug.


[Bug ld/32961] ".pushsection" may introduce unnecessary section dependency which impacts "--gc-sections"

2025-05-12 Thread zhiyuan.lv at linux dot intel.com
https://sourceware.org/bugzilla/show_bug.cgi?id=32961

Zhiyuan Lv  changed:

   What|Removed |Added

 CC||hjl.tools at gmail dot com

-- 
You are receiving this mail because:
You are on the CC list for the bug.


[Bug ld/32961] ".pushsection" may introduce unnecessary section dependency which impacts "--gc-sections"

2025-05-12 Thread zhiyuan.lv at linux dot intel.com
https://sourceware.org/bugzilla/show_bug.cgi?id=32961

--- Comment #2 from Zhiyuan Lv  ---
Thanks H.J. for the comments!

(In reply to H.J. Lu from comment #1)
> (In reply to Zhiyuan Lv from comment #0)
> >
> > If all above is correct, could we consider below two options?
> > 
> > 1. Add a new type of ".pushsection", say, ".pushnewsection", which will
> > always create a new section.
> 
> This may work.  Or an assembler option to force a new section for
> .pushsection.  This doesn't require source code changes.  Only a new
> assembler is needed.

Yeah that would be simpler. The only concern is the side effect of always
generating new sections for .pushsection with that assembler option. Is it OK?

> 
> > 2. Let LD be able to ignore some given sections while doing the section
> > dependency calculation. If a symbol is deleted, simply remove the relocation
> > entry in the specified "ignored section".
> 
> Linker can't remove .alt_section since there is only one .alt_section and
> it is used by entry_func.

Right. What I was thinking is to only remove some items in .rela.alt_section if
a symbol to be removed appears in that section. Otherwise I guess linker will
report error on that?

"2" is a little more complicated than "1". So probably the assembler change is
the way to go. Thanks!

> 
> > Either one can address above case of test.c. Any comments? Thanks!

-- 
You are receiving this mail because:
You are on the CC list for the bug.


[Bug ld/32961] ".pushsection" may introduce unnecessary section dependency which impacts "--gc-sections"

2025-05-13 Thread zhiyuan.lv at linux dot intel.com
https://sourceware.org/bugzilla/show_bug.cgi?id=32961

--- Comment #7 from Zhiyuan Lv  ---
And, the GCC version I used is 14.2.0, which will generate per function section
of "__patchable_function_entries". Thanks!

-- 
You are receiving this mail because:
You are on the CC list for the bug.


[Bug ld/32961] ".pushsection" may introduce unnecessary section dependency which impacts "--gc-sections"

2025-05-13 Thread zhiyuan.lv at linux dot intel.com
https://sourceware.org/bugzilla/show_bug.cgi?id=32961

--- Comment #15 from Zhiyuan Lv  ---
(In reply to H.J. Lu from comment #14)
> Created attachment 16086 [details]
> A new patch
> 
> Please use this one.

I tested the latest patch here together with your gcc patch, now both test.c
and kernel build got expected result :-) Below are my test details:

binutils base: top of master branch
gcc base: 14.2.0 (default version in Ubuntu 24.04)

1. compile with "-Wa,--unique-pushsection", the unused functions are removed by
linker as expected.

2. no compile option change, but change ".pushsection" to ".pushuniquesection"
in code, the unused functions are removed by linker as expected.

-- 
You are receiving this mail because:
You are on the CC list for the bug.


[Bug ld/32961] ".pushsection" may introduce unnecessary section dependency which impacts "--gc-sections"

2025-05-13 Thread zhiyuan.lv at linux dot intel.com
https://sourceware.org/bugzilla/show_bug.cgi?id=32961

--- Comment #18 from Zhiyuan Lv  ---
(In reply to H.J. Lu from comment #16)
> (In reply to Zhiyuan Lv from comment #15)
> > (In reply to H.J. Lu from comment #14)
> > > Created attachment 16086 [details]
> > > A new patch
> > > 
> > > Please use this one.
> > 
> > I tested the latest patch here together with your gcc patch, now both test.c
> > and kernel build got expected result :-) Below are my test details:
> 
> Does kernel work correctly?

Not yet. My kernel hang in very early boot. --gc-sections seemed to have
deleted more symbols than kernel needs. I am debugging it which will take some
time and will update here when finish. Thanks!

> 
> > binutils base: top of master branch
> > gcc base: 14.2.0 (default version in Ubuntu 24.04)
> > 
> > 1. compile with "-Wa,--unique-pushsection", the unused functions are removed
> > by linker as expected.
> > 
> > 2. no compile option change, but change ".pushsection" to
> > ".pushuniquesection" in code, the unused functions are removed by linker as
> > expected.

-- 
You are receiving this mail because:
You are on the CC list for the bug.


[Bug ld/32961] ".pushsection" may introduce unnecessary section dependency which impacts "--gc-sections"

2025-05-16 Thread zhiyuan.lv at linux dot intel.com
https://sourceware.org/bugzilla/show_bug.cgi?id=32961

--- Comment #28 from Zhiyuan Lv  ---
(In reply to H.J. Lu from comment #26)
> (In reply to H.J. Lu from comment #25)
> > Created attachment 16096 [details]
> > A patch to add .pushuniqsect
> > 
> > Please retry with .pushuniqsect.
> 
> Jan proposed to use --sectname-subst.  Does it work for you?

Yes, tried this parameter and it also works.

-- 
You are receiving this mail because:
You are on the CC list for the bug.


[Bug ld/32961] ".pushsection" may introduce unnecessary section dependency which impacts "--gc-sections"

2025-05-16 Thread zhiyuan.lv at linux dot intel.com
https://sourceware.org/bugzilla/show_bug.cgi?id=32961

--- Comment #29 from Zhiyuan Lv  ---
(In reply to H.J. Lu from comment #27)
> Created attachment 16097 [details]
> A patch without new directive
> 
> Please try this.

Sorry for the late! I tried the new patch with the test case compilation and
kernel build/run. It works. Thanks!

-- 
You are receiving this mail because:
You are on the CC list for the bug.


[Bug ld/32961] ".pushsection" may introduce unnecessary section dependency which impacts "--gc-sections"

2025-05-15 Thread zhiyuan.lv at linux dot intel.com
https://sourceware.org/bugzilla/show_bug.cgi?id=32961

--- Comment #20 from Zhiyuan Lv  ---
(In reply to H.J. Lu from comment #19)
> (In reply to Zhiyuan Lv from comment #18)
> > (In reply to H.J. Lu from comment #16)
> > > (In reply to Zhiyuan Lv from comment #15)
> > > > (In reply to H.J. Lu from comment #14)
> > > > > Created attachment 16086 [details]
> > > > > A new patch
> > > > > 
> > > > > Please use this one.
> > > > 
> > > > I tested the latest patch here together with your gcc patch, now both 
> > > > test.c
> > > > and kernel build got expected result :-) Below are my test details:
> > > 
> > > Does kernel work correctly?
> > 
> > Not yet. My kernel hang in very early boot. --gc-sections seemed to have
> > deleted more symbols than kernel needs. I am debugging it which will take
> > some time and will update here when finish. Thanks!
> > 
> 
> Sure.  Please find a testcase if something goes wrong.

Hi H.J., my kernel booted up now and is functional. The problem I met
previously is that some useful sections without explicit dependencies were
removed, and using "KEEP" in LD script solved the problem.

I did not use the new option and --gc-sections on the whole kernel, but some
specific files. My usage is pretty special that I need to build a kernel module
but running in different address space from the core kernel. The module is self
contained, meanwhile wants to reuse some functions in kernel. In my experiment
I compile several kernel files into the module, and rely on linker option to
remove unused symbols to minimize the kernel dependency.

Regarding the binutils change, do we need to find more use cases to justify its
value as a new feature? Thanks!

-- 
You are receiving this mail because:
You are on the CC list for the bug.


[Bug ld/32961] ".pushsection" may introduce unnecessary section dependency which impacts "--gc-sections"

2025-05-15 Thread zhiyuan.lv at linux dot intel.com
https://sourceware.org/bugzilla/show_bug.cgi?id=32961

--- Comment #21 from Zhiyuan Lv  ---
(In reply to H.J. Lu from comment #13)
> (In reply to Zhiyuan Lv from comment #12)
> > Hi H.J.,
> > 
> > On Tue, May 13, 2025 at 07:15:14AM +, hjl.tools at gmail dot com wrote:
> > > https://sourceware.org/bugzilla/show_bug.cgi?id=32961
> > > 
> > > --- Comment #8 from H.J. Lu  ---
> > > (In reply to Zhiyuan Lv from comment #7)
> > > > And, the GCC version I used is 14.2.0, which will generate per function
> > > > section of "__patchable_function_entries". Thanks!
> > > 
> > > Now, the question is if GCC should generate a unique section name for
> > > __patchable_function_entries -ffunction-sections.  If I give you a
> > 
> > Just curious: is the same name a problem here? I saw that gcc 14
> > generates __patchable** sections for each function with the same name,
> > and they seem to work fine: each section has its own relocation section
> > respectively. Why does "-ffunction-sections" cause problems? Thanks!
> > 
> 
> It is the similar problem on GCC side. -fpatchable-function-entry doesn't
> work with -ffunction-sections. My GCC patch should fix it.

Could you elaborate more why the two parameters cannot work together?

I am still wondering whether it is possible to avoid compiler change. With GCC
14 in Ubuntu 24.04, I can see that gcc generates __patchable_function_entries
sections per function with the same name, and GAS/LD seems to treat them
separately so they do not introduce unnecessary dependencies. So, the same name
of __patchable_function_entries is not a problem.

IIUC, all we need is unique name for .pushsection, and your latest patch has
already ensured that. Today I also tested that with unmodified gcc 14, all my
tests passed. Thanks!

-- 
You are receiving this mail because:
You are on the CC list for the bug.


[Bug ld/32961] ".pushsection" may introduce unnecessary section dependency which impacts "--gc-sections"

2025-05-15 Thread zhiyuan.lv at linux dot intel.com
https://sourceware.org/bugzilla/show_bug.cgi?id=32961

--- Comment #24 from Zhiyuan Lv  ---
(In reply to H.J. Lu from comment #23)
> Created attachment 16095 [details]
> The final patch
> 
> This is the final patch I am submitting.  Please test.

Tested with both my small test case here and kernel build, all working fine.
Below are the details:

OS: Ubuntu 24.04
GCC: 14.2.0 in Ubuntu default
Binutils: 2.44.50, built with H.J.'s patch on top of master branch
(d1851edfe9d2)

Test cases:
1. compile test.c with assembler new parameter "--unique-pushsection"
2. modify test.c to use ".pushuniquesection" and compile
3. try (2) with "--unique-pushsection" parameter
4. build Linux kernel 6.12 with the new parameter for some specific files

All can see that unused functions were removed as expected, and functionality
OK. Thanks H.J.!!

-- 
You are receiving this mail because:
You are on the CC list for the bug.