Repository: davidlattimore/wild (rust)

Issue:
An ifunc test fails with `-pie`
The following test-case fails:
```c
#include <stdio.h>

void foo() {}
int bar() __attribute__((ifunc("resolve_bar")));
void *resolve_bar() { return foo; }
void *bar2 = bar;

int main() {
  printf("%p %p\n", bar, bar2);
}
```

```
❯ gcc /tmp/ifunc.c -c -fPIC && gcc ifunc.o -pie -B ~/Programming/wild
WARNING: wild: --plugin /usr/lib64/gcc/x86_64-suse-linux/14/liblto_plugin.so is not yet supported
Error: Failed copying from ifunc.o (768 (3/0)) to output file

Caused by:
    0: Failed to apply relocations in section `.data.rel` of ifunc.o
    1: Failed to apply relocation of type R_X86_64_64 to symbol `bar` (76 local=7) in file #768 (3/0) (ifunc.o) (IFUNC | CAN_BYPASS_GOT) at offset 0x0
    2: Insufficient .rela.dyn (relative) allocation. Setting WILD_VERIFY_ALLOCATIONS=1 might give more info
collect2: error: ld returned 255 exit status
```

Pull request:
Fix direct references to ifuncs in relocatable executables
Fixes #580

Patch:
diff --git a/libwild/src/elf_writer.rs b/libwild/src/elf_writer.rs
index 9da33c0..0068c32 100644
--- a/libwild/src/elf_writer.rs
+++ b/libwild/src/elf_writer.rs
@@ -2247,6 +2247,7 @@ fn write_absolute_relocation<A: Arch>(
             resolution.dynamic_symbol_index()?,
             DynamicRelocationKind::Absolute,
         )?;
+
         Ok(0)
     } else if table_writer.output_kind.is_relocatable() && !resolution.is_absolute() {
         let address = resolution.value_with_addend(
@@ -2256,10 +2257,10 @@ fn write_absolute_relocation<A: Arch>(
             &layout.merged_strings,
             &layout.merged_string_start_addresses,
         )?;
+
         table_writer.write_address_relocation::<A>(place, address as i64)?;
+
         Ok(0)
-    } else if resolution.value_flags.contains(ValueFlags::IFUNC) {
-        Ok(resolution.plt_address()?.wrapping_add(addend as u64))
     } else {
         resolution.value_with_addend(
             addend,
diff --git a/libwild/src/layout.rs b/libwild/src/layout.rs
index 6c36c02..8db0c48 100644
--- a/libwild/src/layout.rs
+++ b/libwild/src/layout.rs
@@ -2548,7 +2548,8 @@ fn process_relocation<A: Arch>(
 
         if args.is_relocatable()
             && rel_info.kind == RelocationKind::Absolute
-            && symbol_value_flags.contains(ValueFlags::ADDRESS)
+            && (symbol_value_flags.contains(ValueFlags::ADDRESS)
+                | symbol_value_flags.contains(ValueFlags::IFUNC))
         {
             if section_is_writable {
                 common.allocate(part_id::RELA_DYN_RELATIVE, elf::RELA_ENTRY_SIZE);
@@ -4344,6 +4345,10 @@ impl Resolution {
         merged_strings: &OutputSectionMap<MergedStringsSection>,
         merged_string_start_addresses: &MergedStringStartAddresses,
     ) -> Result<u64> {
+        if self.value_flags.contains(ValueFlags::IFUNC) {
+            return Ok(self.plt_address()?.wrapping_add(addend as u64));
+        }
+
         // For most symbols, `raw_value` won't be zero, so we can save ourselves from looking up the
         // section to see if it's a string-merge section. For string-merge symbols with names,
         // `raw_value` will have already been computed, so we can avoid computing it again.
diff --git a/linker-diff/src/arch.rs b/linker-diff/src/arch.rs
index 01a2b31..10cc0bf 100644
--- a/linker-diff/src/arch.rs
+++ b/linker-diff/src/arch.rs
@@ -135,10 +135,6 @@ pub(crate) trait RType: Copy + Debug + Display + Eq + PartialEq {
     fn opt_relocation_info(self) -> Option<RelocationKindInfo>;
 
     fn dynamic_relocation_kind(self) -> Option<DynamicRelocationKind>;
-
-    fn is_relative(self) -> bool {
-        self.dynamic_relocation_kind() == Some(DynamicRelocationKind::Relative)
-    }
 }
 
 pub(crate) trait RelaxationKind: Copy + Clone + Debug + Eq + PartialEq {
diff --git a/linker-diff/src/asm_diff.rs b/linker-diff/src/asm_diff.rs
index aea0c16..d6e6b44 100644
--- a/linker-diff/src/asm_diff.rs
+++ b/linker-diff/src/asm_diff.rs
@@ -1866,6 +1866,7 @@ impl<'data> RelaxationTester<'data> {
                 .relocation_at_address(self.section_address + relaxation_match.start)
             {
                 let r_type = get_r_type(runtime_relocation);
+
                 if let Some(symbol) = self.symtab_entry_for_relocation(runtime_relocation) {
                     return Ok(Reference {
                         referent: Referent::DynamicRelocation(DynamicRelocation {
@@ -1875,9 +1876,26 @@ impl<'data> RelaxationTester<'data> {
                         }),
                         indirection: Indirection::Direct,
                     });
-                } else if r_type.is_relative() {
-                    merged_value = merged_value.wrapping_add(runtime_relocation.addend() as u64);
-                    continue;
+                }
+
+                match r_type.dynamic_relocation_kind() {
+                    Some(DynamicRelocationKind::Relative) => {
+                        merged_value =
+                            merged_value.wrapping_add(runtime_relocation.addend() as u64);
+                        continue;
+                    }
+                    Some(DynamicRelocationKind::Irelative) => {
+                        return Ok(Reference {
+                            referent: Referent::IFunc(determine_ifunc_name(
+                                runtime_relocation.addend() as u64,
+                                self.bin,
+                            )),
+                            indirection: Indirection::Direct,
+                        });
+                    }
+                    _ => {
+                        bail!("Unhandled dynamic relocation {r_type}");
+                    }
                 }
             }
 
@@ -1894,7 +1912,9 @@ impl<'data> RelaxationTester<'data> {
 
             let relative_to = self.get_relative_to::<A>(offset, relocation_info)?;
 
-            if is_non_pointer_relocation(relocation_info.kind) {
+            if is_non_pointer_relocation(relocation_info.kind)
+                || (self.bin.address_index.is_relocatable && relocation_info.kind.is_absolute())
+            {
                 is_pointer = false;
             }
 
@@ -3168,22 +3188,28 @@ impl<'data> GotIndex<'data> {
     }
 }
 
+/// Returns the name of the ifunc resolver given the resolver address. We choose to return the name
+/// of the resolver rather than the ifunc name because GNU ld and lld are inconsistent with where
+/// they point the symbol for the ifunc, whereas the resolver's symbol consistently points at the
+/// address of the resolver.
 fn determine_ifunc_name<'data>(address: u64, bin: &Binary<'data>) -> Option<SymbolName<'data>> {
     bin.address_index
         .symbols_at_address(address)
         .iter()
         .filter_map(|symbol_index| {
             let symbol = bin.elf_file.symbol_by_index(*symbol_index).ok()?;
-            if symbol.elf_symbol().st_type() == object::elf::STT_GNU_IFUNC {
-                Some(SymbolName {
-                    bytes: symbol.name_bytes().ok()?,
-                    version: None,
-                })
-            } else {
-                None
+
+            // We're likely to get symbols of type STT_GNU_IFUNC. The resolver should be just a
+            // regular function and that's what we want.
+            if symbol.elf_symbol().st_type() != object::elf::STT_FUNC {
+                return None;
             }
+
+            Some(SymbolName {
+                bytes: symbol.name_bytes().ok()?,
+                version: None,
+            })
         })
-        // If multiple symbols meet the criteria, we arbitrarily select the later one when sorted.
         .max()
 }
 
diff --git a/linker-utils/src/elf.rs b/linker-utils/src/elf.rs
index e97b44c..d4fb34d 100644
--- a/linker-utils/src/elf.rs
+++ b/linker-utils/src/elf.rs
@@ -575,6 +575,16 @@ pub enum RelocationKind {
     None,
 }
 
+impl RelocationKind {
+    #[must_use]
+    pub fn is_absolute(self) -> bool {
+        matches!(
+            self,
+            RelocationKind::Absolute | RelocationKind::AbsoluteAArch64
+        )
+    }
+}
+
 #[derive(Copy, Clone, Debug, PartialEq, Eq)]
 pub enum DynamicRelocationKind {
     Copy,


Test patch:
diff --git a/wild/tests/integration_tests.rs b/wild/tests/integration_tests.rs
index 1f72089..226a617 100644
--- a/wild/tests/integration_tests.rs
+++ b/wild/tests/integration_tests.rs
@@ -2158,6 +2158,7 @@ fn integration_test(
         "symbol-versions.c",
         "copy-relocations.c",
         "force-undefined.c",
+        "libc-ifunc.c",
         "libc-integration.c",
         "rust-integration.rs",
         "rust-integration-dynamic.rs",
diff --git a/wild/tests/sources/libc-ifunc.c b/wild/tests/sources/libc-ifunc.c
new file mode 100644
index 0000000..bb7ab78
--- /dev/null
+++ b/wild/tests/sources/libc-ifunc.c
@@ -0,0 +1,28 @@
+//#CompArgs:-fPIC -g
+//#LinkerDriver:gcc
+//#LinkArgs:-pie -Wl,-z,now
+//#DiffIgnore:section.rodata
+// GNU ld emits an extra IRELATIVE relocation, while LLD and Wild instead point to the PLT entry. So
+// we need to diff against lld, which isn't currently enabled for cross compilation.
+//#Cross:false
+//#EnableLinker:lld
+
+int foo() {
+  return 42;
+}
+
+int bar() __attribute__((ifunc("resolve_bar")));
+
+void *resolve_bar() { return foo; }
+
+typedef int (*int_f_ptr_t)(void);
+
+int_f_ptr_t bar2 = bar;
+
+int main() {
+  if (bar() != 42) {
+    return bar();
+  }
+
+  return bar2();
+}