[PATCH] ipa-icf: Step to the next caller past a thunk in redirect_all_callers

[PATCH] ipa-icf: Step to the next caller past a thunk in redirect_all_callers

[Kees Cook]

redirect_all_callers walks the callers of N, but steps past a thunk
caller with next_callee, the next call the thunk makes.  A thunk makes
only that one call, so the walk ends there, and so the callers after
the thunk in the list would not be redirected.  This came with the thunk
check in commit 17d1bf760f8 ("re PR target/65263").

gcc/ChangeLog:

	* ipa-icf.cc (redirect_all_callers): Step to the next caller, not
	the next callee, past a thunk.

Signed-off-by: Kees Cook <kees@kernel.org>
---
I found this while looking through ICF code and Richard's recently
proposed changes with inlining, so it's technically unrelated, but it
looked wrong.  But I haven't found a way to reach it: on targets with
symbol aliases a thunk calls a local alias of its function (.LTHUNKn),
whose other callers come from redirections and so are ahead of the thunk
in the list, and on targets without them, such as Darwin, thunks are
expanded to GIMPLE before ICF.  So I have not been able to build a test
case for this, and I suspect it may just be accidentally/effectively
dead code?  But the intent of the function didn't seem to match the
result of the "else"'s list-walk.
---
 gcc/ipa-icf.cc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gcc/ipa-icf.cc b/gcc/ipa-icf.cc
index 64cc722bd7aa..7b524bdf16b1 100644
--- a/gcc/ipa-icf.cc
+++ b/gcc/ipa-icf.cc
@@ -995,7 +995,7 @@ redirect_all_callers (cgraph_node *n, cgraph_node *to)
           nredirected++;
 	}
       else
-	e = e->next_callee;
+	e = e->next_caller;
     }
   for (unsigned i = 0; n->iterate_direct_aliases (i, ref);)
     {
-- 
2.55.0

Re: [PATCH] ipa-icf: Step to the next caller past a thunk in redirect_all_callers

[Richard Biener]

On Thu, 15 Oct 2026, Kees Cook wrote:

> redirect_all_callers walks the callers of N, but steps past a thunk
> caller with next_callee, the next call the thunk makes.  A thunk makes
> only that one call, so the walk ends there, and so the callers after
> the thunk in the list would not be redirected.  This came with the thunk
> check in commit 17d1bf760f8 ("re PR target/65263").

Huh.  This looks obvious to me, thus OK unless Jan or Martin object
soon.

Thanks,
Richard.

> gcc/ChangeLog:
> 
> 	* ipa-icf.cc (redirect_all_callers): Step to the next caller, not
> 	the next callee, past a thunk.
> 
> Signed-off-by: Kees Cook <kees@kernel.org>
> ---
> I found this while looking through ICF code and Richard's recently
> proposed changes with inlining, so it's technically unrelated, but it
> looked wrong.  But I haven't found a way to reach it: on targets with
> symbol aliases a thunk calls a local alias of its function (.LTHUNKn),
> whose other callers come from redirections and so are ahead of the thunk
> in the list, and on targets without them, such as Darwin, thunks are
> expanded to GIMPLE before ICF.  So I have not been able to build a test
> case for this, and I suspect it may just be accidentally/effectively
> dead code?  But the intent of the function didn't seem to match the
> result of the "else"'s list-walk.
> ---
>  gcc/ipa-icf.cc | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/gcc/ipa-icf.cc b/gcc/ipa-icf.cc
> index 64cc722bd7aa..7b524bdf16b1 100644
> --- a/gcc/ipa-icf.cc
> +++ b/gcc/ipa-icf.cc
> @@ -995,7 +995,7 @@ redirect_all_callers (cgraph_node *n, cgraph_node *to)
>            nredirected++;
>  	}
>        else
> -	e = e->next_callee;
> +	e = e->next_caller;
>      }
>    for (unsigned i = 0; n->iterate_direct_aliases (i, ref);)
>      {
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH,
Frankenstrasse 146, 90461 Nuernberg, Germany;
GF: Stefan Gaiser, Jochen Jaser, Abhinav Puri; (HRB 36809, AG Nuernberg)

Re: [PATCH] ipa-icf: Step to the next caller past a thunk in redirect_all_callers

[Martin Jambor]

Hi,

On Mon, Oct 19 2026, Richard Biener wrote:
> On Thu, 15 Oct 2026, Kees Cook wrote:
>
>> redirect_all_callers walks the callers of N, but steps past a thunk
>> caller with next_callee, the next call the thunk makes.  A thunk makes
>> only that one call, so the walk ends there, and so the callers after
>> the thunk in the list would not be redirected.  This came with the thunk
>> check in commit 17d1bf760f8 ("re PR target/65263").
>
> Huh.  This looks obvious to me, thus OK unless Jan or Martin object
> soon.

This is fairly obviously OK.

Thanks for catching this!

Martin


>
> Thanks,
> Richard.
>
>> gcc/ChangeLog:
>> 
>> 	* ipa-icf.cc (redirect_all_callers): Step to the next caller, not
>> 	the next callee, past a thunk.
>> 
>> Signed-off-by: Kees Cook <kees@kernel.org>
>> ---
>> I found this while looking through ICF code and Richard's recently
>> proposed changes with inlining, so it's technically unrelated, but it
>> looked wrong.  But I haven't found a way to reach it: on targets with
>> symbol aliases a thunk calls a local alias of its function (.LTHUNKn),
>> whose other callers come from redirections and so are ahead of the thunk
>> in the list, and on targets without them, such as Darwin, thunks are
>> expanded to GIMPLE before ICF.  So I have not been able to build a test
>> case for this, and I suspect it may just be accidentally/effectively
>> dead code?  But the intent of the function didn't seem to match the
>> result of the "else"'s list-walk.
>> ---
>>  gcc/ipa-icf.cc | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/gcc/ipa-icf.cc b/gcc/ipa-icf.cc
>> index 64cc722bd7aa..7b524bdf16b1 100644
>> --- a/gcc/ipa-icf.cc
>> +++ b/gcc/ipa-icf.cc
>> @@ -995,7 +995,7 @@ redirect_all_callers (cgraph_node *n, cgraph_node *to)
>>            nredirected++;
>>  	}
>>        else
>> -	e = e->next_callee;
>> +	e = e->next_caller;
>>      }
>>    for (unsigned i = 0; n->iterate_direct_aliases (i, ref);)
>>      {
>> 
>
> -- 
> Richard Biener <rguenther@suse.de>
> SUSE Software Solutions Germany GmbH,
> Frankenstrasse 146, 90461 Nuernberg, Germany;
> GF: Stefan Gaiser, Jochen Jaser, Abhinav Puri; (HRB 36809, AG Nuernberg)