Re: [PATCH v4 1/2] gdb: Preserve IFUNC marker when finding inferior functions

Muhammad Kamran <[email protected]> writes:

> GDB calls find_function_in_inferior ("malloc") when expression
> evaluation needs to allocate memory in the inferior, e.g. for string
> literal arguments.
>
> The minimal-symbol fallback created a synthetic ordinary function
> pointer from msymbol.value_address ().  If the symbol was a GNU IFUNC,
> this discarded the IFUNC marker, so call_function_by_hand did not
> resolve the symbol before calling it.
>
> Check the minimal symbol kind directly and propagate the GNU IFUNC
> marker to the synthetic function type for mst_text_gnu_ifunc and
> mst_data_gnu_ifunc symbols.  This keeps the existing fallback address
> and return type while allowing inferior calls through IFUNC symbols to
> be resolved correctly.
>
> Extend gdb.base/gnu-ifunc.exp with an internal inferior-call test that
> uses an IFUNC malloc.  The test runs through the existing IFUNC matrix
> for resolver attr, resolver debug info, and resolved-target debug
> info.

Thanks for working on this.  This fix is looking good.  I have a couple
of minor testsuite issues to address, but I've proposed some code inline
below.

I've pretty sure that I've managed to find 3 other bugs in this area
while reviewing this patch.  I don't think we need to block this patch
for fixing these additional issues, and I'll try to explain what I found
inline below.

>
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=34330
> ---
>  .../gdb.base/gnu-ifunc-inferior-call-final.c  | 27 +++++++
>  .../gnu-ifunc-inferior-call-resolver.c        | 43 +++++++++++
>  .../gdb.base/gnu-ifunc-inferior-call.c        | 37 ++++++++++
>  gdb/testsuite/gdb.base/gnu-ifunc.exp          | 73 +++++++++++++++++++
>  gdb/valops.c                                  |  5 ++
>  5 files changed, 185 insertions(+)
>  create mode 100644 gdb/testsuite/gdb.base/gnu-ifunc-inferior-call-final.c
>  create mode 100644 gdb/testsuite/gdb.base/gnu-ifunc-inferior-call-resolver.c
>  create mode 100644 gdb/testsuite/gdb.base/gnu-ifunc-inferior-call.c
>
> diff --git a/gdb/testsuite/gdb.base/gnu-ifunc-inferior-call-final.c b/gdb/testsuite/gdb.base/gnu-ifunc-inferior-call-final.c
> new file mode 100644
> index 00000000000..1a9ae6fdcba
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/gnu-ifunc-inferior-call-final.c
> @@ -0,0 +1,27 @@
> +/* This testcase is part of GDB, the GNU debugger.
> +
> +   Copyright 2026 Free Software Foundation, Inc.
> +
> +   This program is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; either version 3 of the License, or
> +   (at your option) any later version.
> +
> +   This program is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
> +
> +#include <stddef.h>
> +
> +extern char arena[32];
> +
> +void *
> +dummy_malloc (size_t size)
> +{
> +  (void) size;
> +  return arena;
> +}
> diff --git a/gdb/testsuite/gdb.base/gnu-ifunc-inferior-call-resolver.c b/gdb/testsuite/gdb.base/gnu-ifunc-inferior-call-resolver.c
> new file mode 100644
> index 00000000000..217002968de
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/gnu-ifunc-inferior-call-resolver.c
> @@ -0,0 +1,43 @@
> +/* This testcase is part of GDB, the GNU debugger.
> +
> +   Copyright 2026 Free Software Foundation, Inc.
> +
> +   This program is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; either version 3 of the License, or
> +   (at your option) any later version.
> +
> +   This program is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
> +
> +#include <stddef.h>
> +
> +extern void *dummy_malloc (size_t size);
> +
> +typedef void *(*malloc_t) (size_t size);
> +
> +#ifndef IFUNC_RESOLVER_ATTR
> +asm (".type malloc, %gnu_indirect_function");
> +malloc_t
> +malloc (unsigned long hwcap)
> +#else
> +static malloc_t
> +resolve_malloc (void)
> +#endif
> +{
> +#ifndef IFUNC_RESOLVER_ATTR
> +  (void) hwcap;
> +#endif
> +  return dummy_malloc;
> +}
> +
> +#ifdef IFUNC_RESOLVER_ATTR
> +extern void *malloc (size_t size);
> +
> +__typeof (malloc) malloc __attribute__ ((ifunc ("resolve_malloc")));
> +#endif
> diff --git a/gdb/testsuite/gdb.base/gnu-ifunc-inferior-call.c b/gdb/testsuite/gdb.base/gnu-ifunc-inferior-call.c
> new file mode 100644
> index 00000000000..67d195d25f0
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/gnu-ifunc-inferior-call.c
> @@ -0,0 +1,37 @@
> +/* This testcase is part of GDB, the GNU debugger.
> +
> +   Copyright 2026 Free Software Foundation, Inc.
> +
> +   This program is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; either version 3 of the License, or
> +   (at your option) any later version.
> +
> +   This program is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
> +
> +char arena[32];
> +const char *str;
> +
> +int
> +str_in_arena (void)
> +{
> +  return str == arena;
> +}
> +
> +void
> +get_string (const char *s)
> +{
> +  str = s;
> +}
> +
> +int
> +main (void)
> +{
> +  return 0;
> +}
> diff --git a/gdb/testsuite/gdb.base/gnu-ifunc.exp b/gdb/testsuite/gdb.base/gnu-ifunc.exp
> index e6389102ae3..23a5aadbcbf 100644
> --- a/gdb/testsuite/gdb.base/gnu-ifunc.exp
> +++ b/gdb/testsuite/gdb.base/gnu-ifunc.exp
> @@ -25,6 +25,13 @@ set libsrc ${libfile}.c
>  set final_file "${testfile}-final"
>  set final_src ${final_file}.c
>  
> +set infcall_file "${testfile}-inferior-call"
> +set infcall_src ${infcall_file}.c
> +set infcall_resolver_file "${testfile}-inferior-call-resolver"
> +set infcall_resolver_src ${infcall_resolver_file}.c
> +set infcall_final_file "${testfile}-inferior-call-final"
> +set infcall_final_src ${infcall_final_file}.c
> +
>  # Return the binary suffix appended to program and library names to
>  # make each testcase variant unique.
>  proc make_binsuffix {resolver_attr resolver_debug final_debug} {
> @@ -356,6 +363,71 @@ proc misc_tests {resolver_attr resolver_debug final_debug} {
>      }
>  }
>  
> +# Test that GDB resolves a GNU IFUNC minimal symbol when it uses
> +# find_function_in_inferior to make an internal inferior call.  String
> +# literals are copied into the inferior with a call to malloc, so an
> +# IFUNC malloc exercises this path.
> +
> +proc_with_prefix test_inferior_call {resolver_attr resolver_debug final_debug} {
> +    global srcdir subdir
> +    global infcall_file infcall_src
> +    global infcall_resolver_file infcall_resolver_src
> +    global infcall_final_file infcall_final_src
> +
> +    set suffix [make_binsuffix $resolver_attr $resolver_debug $final_debug]
> +    set executable ${infcall_file}-$suffix
> +    set binfile [standard_output_file $executable]
> +    set infcall_lib_so [standard_output_file ${infcall_file}-$suffix.so]
> +    set resolver_obj [standard_output_file ${infcall_resolver_file}-$suffix.o]
> +    set final_obj [standard_output_file ${infcall_final_file}-$suffix.o]
> +
> +    set resolver_opts {additional_flags=-fno-builtin-malloc additional_flags=-fpic}
> +    set final_opts {additional_flags=-fpic}
> +    set shlib_opts {ldflags=-Wl,-z,lazy}
> +    set exec_opts [list debug shlib=$infcall_lib_so]
> +    lappend exec_opts "ldflags=-Wl,-z,lazy"
> +
> +    if {$resolver_attr} {
> +	lappend resolver_opts "additional_flags=-DIFUNC_RESOLVER_ATTR"
> +    }
> +
> +    if {$resolver_debug} {
> +	lappend resolver_opts "debug"
> +    }
> +
> +    if {$final_debug} {
> +	lappend final_opts "debug"
> +    }
> +
> +    if { [gdb_compile ${srcdir}/${subdir}/${infcall_resolver_src} \
> +	      $resolver_obj object $resolver_opts] != ""
> +	 || [gdb_compile ${srcdir}/${subdir}/${infcall_final_src} \
> +		 $final_obj object $final_opts] != ""
> +	 || [gdb_compile_shlib [list $resolver_obj $final_obj] \
> +		 $infcall_lib_so $shlib_opts] != ""
> +	 || [gdb_compile ${srcdir}/${subdir}/${infcall_src} \
> +		 $binfile executable $exec_opts] != "" } {
> +	untested "failed to compile inferior call testcase"
> +	return
> +    }
> +
> +    clean_restart $executable

You should add a line containing:

    gdb_load_shlib $infcall_lib_so

here.  This will copy the shared library to the host and setup the
library paths so the inferior can load the library when testing with
remote host boards.  All the other test in this file do something
similar.  You can see the failures when using 'make check-all-boards'.

> +    if {![runto_main]} {
> +	return
> +    }

In order to get this test passing I added the following code here:

    set malloc_addr {}
    gdb_test_multiple "pipe maint print msymbols | grep \" malloc \"" \
       "look for malloc msyms" \
    {
       -re "($::hex) malloc section \[^\r\n\]+\r\n" {
           lappend malloc_addr $expect_out(1,string)
           exp_continue
       }

       -re "$::gdb_prompt $" {
           gdb_assert {[llength $malloc_addr] > 0} \
               "found at least one malloc symbol"
       }
    }

    set found_correct_malloc false
    set first_addr [lindex $malloc_addr 0]
    set infcall_lib_tail [file tail $infcall_lib_so]
    gdb_test_multiple "info symbol $first_addr" "check first malloc symbol" {
       -re -wrap " in section \[^\r\n\]+ of \[^\r\n\]+/$infcall_lib_tail" {
           set found_correct_malloc true
       }

       -re -wrap " in section \[^\r\n\]+" {
           # Nothing to do.
       }
    }
    if { !$found_correct_malloc } {
       unsupported "found some other malloc symbol"
       return
    }

The cause of all my problems is that older versions of the dynamic
linker export a weak 'malloc' symbol, and in some configurations GDB
ends up finding that symbol before it finds your test's 'malloc' IFUNC.

The above catches tries to figure out when GDB is about to call the
wrong/other function by looking at where the first minimal_symbol is
defined.  If it's in the test's shared library then GDB is going to call
the right function.  If the first minimal_symbol is in some other
location then GDB is not going to call the expected function and the
test will naturally fail.

Now, I don't think GDB _should_ be calling the wrong function here.  I
think we should always be calling the symbol from the test.  The reason
we don't is explained below.

> +
> +    # Without debug info for both the resolver and the resolved target,
> +    # find_function_addr currently loses the synthetic return type after
> +    # resolving the IFUNC.
> +    if {!$resolver_debug && !$final_debug} {
> +	unsupported "internal call resolves IFUNC malloc"
> +	return
> +    }
> +    gdb_test "print (get_string (\"hello-ifunc\"), str_in_arena ())" \
> +	" = 1" \
> +	"internal call resolves IFUNC malloc"
> +}
> +
>  # Test all the combinations of:
>  #
>  # - An ifunc resolver with the same name as the ifunc symbol vs an
> @@ -374,6 +446,7 @@ foreach_with_prefix resolver_attr {0 1} {
>  	    if { [build $resolver_attr $resolver_debug $final_debug] != 0 } {
>  		misc_tests $resolver_attr $resolver_debug $final_debug
>  		set-break $resolver_attr $resolver_debug $final_debug
> +		test_inferior_call $resolver_attr $resolver_debug $final_debug
>  	    }
>  	}
>      }
> diff --git a/gdb/valops.c b/gdb/valops.c
> index ab6fd5079e1..454f093ab68 100644
> --- a/gdb/valops.c
> +++ b/gdb/valops.c
> @@ -138,6 +138,11 @@ find_function_in_inferior (const char *name, struct objfile **objf_p)
>  	  type = lookup_function_type (type);
>  	  type = lookup_pointer_type (type);
>  	  maddr = msymbol.value_address ();
> +	  minimal_symbol_type minsym_type = msymbol.minsym->type ();
> +
> +	  if (minsym_type == mst_text_gnu_ifunc
> +	      || minsym_type == mst_data_gnu_ifunc)
> +	    type->target_type ()->set_is_gnu_ifunc (true);
>  
>  	  if (objf_p)
>  	    *objf_p = objfile;

Here you're patching the minimal_symbol code path.  You'd think that
we'd only get here if we fail to find an actual full debug symbol for
the function.  But how does find_function_in_inferior look for a debug
symbol?  Like this:

  sym = lookup_symbol (name, nullptr, SEARCH_TYPE_DOMAIN, nullptr);

Notice we use SEARCH_TYPE_DOMAIN.  That doesn't seem right.  We're
looking for functions, not types.  I'm pretty sure that this check
always fails, and has always failed.  We get away with it because we
just fall back on the minimal symbol lookup path, and that usually
works.

When we make an inferior call, like "print malloc(10)" we use SEARCH_VFT
in the c-exp.y file.  If we switch to using that here then we suddenly
start to get hits for malloc in some of the cases.  I suspect we might
want to consider using SEARCH_FUNCTION_DOMAIN though as we know we only
need functions.

One problem with fixing the above though is that we end up with more
IFUNC issues as the above doesn't handle them.  But we can see how this
might be handled by looking elsewhere in c-exp.y where we do:

            bound_minimal_symbol resolver
              = find_gnu_ifunc (sym.symbol);
            if (resolver.minsym != NULL)
              pstate->push_new<var_msym_value_operation>
                (resolver);
            else
              pstate->push_new<var_value_operation> (sym);

We could do something similar (see patch below), and indeed this gets
fixes a bunch of the problem cases for me.  But not all.

The final issue will require some deeper fixing.  The malloc symbol
exported by the older linkers is a weak symbol.  The problem is that GDB
doesn't track weak vs strong symbols.  If we did then we would be able
to prefer returning a strong symbol to a weak one, and this would mean
that we always found the test's IFUNC symbol.

I've said all this mostly so there's a record of all the things I found,
I don't think that you need to fix any of these for this patch.

I took a look through all the feedback you received on previous
versions, and I think everything raised has been addressed.  If you're
happy to incorporate the two testsuite fixes I proposed above then I
think this patch is OK.

Approved-By: Andrew Burgess <[email protected]>

I'll take a look at patch 2/2 tomorrow, unless someone else beats me to
it.

Thanks,
Andrew

---

diff --git i/gdb/valops.c w/gdb/valops.c
index c478bdc3f15..f278d7b7cab 100644
--- i/gdb/valops.c
+++ w/gdb/valops.c
@@ -113,52 +113,53 @@ struct value *
 find_function_in_inferior (const char *name, struct objfile **objf_p)
 {
   struct block_symbol sym;
+  bound_minimal_symbol msymbol;
 
-  sym = lookup_symbol (name, nullptr, SEARCH_TYPE_DOMAIN, nullptr);
+  sym = lookup_symbol (name, nullptr, SEARCH_VFT, nullptr);
   if (sym.symbol != NULL)
     {
-      if (objf_p)
-	*objf_p = sym.symbol->objfile ();
+      msymbol = find_gnu_ifunc (sym.symbol);
+      if (msymbol.minsym == nullptr)
+	{
+	  if (objf_p)
+	    *objf_p = sym.symbol->objfile ();
+	  return value_of_variable (sym.symbol, sym.block);
+	}
+    }
+  else
+    msymbol = lookup_minimal_symbol (current_program_space, name);
 
-      return value_of_variable (sym.symbol, sym.block);
+  if (msymbol.minsym != NULL)
+    {
+      struct objfile *objfile = msymbol.objfile;
+      struct gdbarch *gdbarch = objfile->arch ();
+
+      struct type *type;
+      CORE_ADDR maddr;
+      type = lookup_pointer_type (builtin_type (gdbarch)->builtin_char);
+      type = lookup_function_type (type);
+      type = lookup_pointer_type (type);
+      maddr = msymbol.value_address ();
+      minimal_symbol_type minsym_type = msymbol.minsym->type ();
+
+      if (minsym_type == mst_text_gnu_ifunc
+	  || minsym_type == mst_data_gnu_ifunc)
+	type->target_type ()->set_is_gnu_ifunc (true);
+
+      if (objf_p)
+	*objf_p = objfile;
+
+      return value_from_pointer (type, maddr);
     }
   else
     {
-      bound_minimal_symbol msymbol
-	= lookup_minimal_symbol (current_program_space, name);
-
-      if (msymbol.minsym != NULL)
-	{
-	  struct objfile *objfile = msymbol.objfile;
-	  struct gdbarch *gdbarch = objfile->arch ();
-
-	  struct type *type;
-	  CORE_ADDR maddr;
-	  type = lookup_pointer_type (builtin_type (gdbarch)->builtin_char);
-	  type = lookup_function_type (type);
-	  type = lookup_pointer_type (type);
-	  maddr = msymbol.value_address ();
-	  minimal_symbol_type minsym_type = msymbol.minsym->type ();
-
-	  if (minsym_type == mst_text_gnu_ifunc
-	      || minsym_type == mst_data_gnu_ifunc)
-	    type->target_type ()->set_is_gnu_ifunc (true);
-
-	  if (objf_p)
-	    *objf_p = objfile;
-
-	  return value_from_pointer (type, maddr);
-	}
+      if (!target_has_execution ())
+	error (_("evaluation of this expression "
+		 "requires the target program to be active"));
       else
-	{
-	  if (!target_has_execution ())
-	    error (_("evaluation of this expression "
-		     "requires the target program to be active"));
-	  else
-	    error (_("evaluation of this expression requires the "
-		     "program to have a function \"%s\"."),
-		   name);
-	}
+	error (_("evaluation of this expression requires the "
+		 "program to have a function \"%s\"."),
+	       name);
     }
 }