Skip to content

Commit

Permalink
rust: Do not bundle static libraries into internal libraries
Browse files Browse the repository at this point in the history
When a Rust rlib/staticlib links to a C static library, it should not
bundle its object files, unless it gets installed. This solves cases of
multiple symbol definition when there is a diamond dependency graph.

The only case we want to bundle is when a Rust staticlib link-whole
another static library. In non-Rust case we achieve that by extracting
objects, but rustc can do it for us. As extra bonus, rustc can bundle
static libraries built with a CustomTarget.
  • Loading branch information
xclaesse committed Nov 8, 2023
1 parent bf6f612 commit b3aae77
Show file tree
Hide file tree
Showing 10 changed files with 91 additions and 12 deletions.
30 changes: 21 additions & 9 deletions mesonbuild/backend/ninjabackend.py
Original file line number Diff line number Diff line change
Expand Up @@ -1969,7 +1969,7 @@ def generate_rust_target(self, target: build.BuildTarget) -> None:
# have to hope that the default cases of +whole-archive are sufficient.
# See: https://github.com/rust-lang/rust/issues/99429
if mesonlib.version_compare(rustc.version, '>= 1.63.0'):
whole_archive = '+whole-archive,-bundle'
whole_archive = '+whole-archive'
else:
whole_archive = ''

Expand All @@ -1979,6 +1979,15 @@ def generate_rust_target(self, target: build.BuildTarget) -> None:
else:
verbatim = ''

def _link_library(libname: str, static: bool, modifiers: T.Optional[T.List[str]] = None, bundle: bool = False):
_type = 'static' if static else 'dylib'
modifiers = modifiers or []
if not bundle and static:
modifiers.append('-bundle')
if modifiers:
_type += ':' + ','.join(modifiers)
args.extend(['-l', f'{_type}={libname}'])

linkdirs = mesonlib.OrderedSet()
external_deps = target.external_deps.copy()
target_deps = target.get_dependencies()
Expand Down Expand Up @@ -2035,10 +2044,14 @@ def generate_rust_target(self, target: build.BuildTarget) -> None:
need_link = False

if need_link:
_type = 'static' if isinstance(d, build.StaticLibrary) else 'dylib'
if modifiers:
_type += ':' + ','.join(modifiers)
args += ['-l', f'{_type}={os.path.basename(lib)}']
static = isinstance(d, build.StaticLibrary)
# rlib does not support bundling. That probably could cause
# unusable installed rlib if they link to uninstalled static
# libraries. Installing rlib is not something we generally
# support anyway.
# https://github.com/rust-lang/rust/issues/108081
bundle = cratetype == 'staticlib' and link_whole
_link_library(os.path.basename(lib), static, modifiers, bundle)

for e in external_deps:
for a in e.get_link_args():
Expand All @@ -2051,13 +2064,12 @@ def generate_rust_target(self, target: build.BuildTarget) -> None:
lib, ext = os.path.splitext(lib)
if lib.startswith('lib'):
lib = lib[3:]
_type = 'static' if a.endswith(('.a', '.lib')) else 'dylib'
args.extend(['-l', f'{_type}={lib}'])
static = a.endswith(('.a', '.lib'))
_link_library(lib, static)
elif a.startswith('-L'):
args.append(a)
elif a.startswith('-l'):
_type = 'static' if e.static else 'dylib'
args.extend(['-l', f'{_type}={a[2:]}'])
_link_library(a[2:], e.static)
for d in linkdirs:
if d == '':
d = '.'
Expand Down
10 changes: 7 additions & 3 deletions mesonbuild/build.py
Original file line number Diff line number Diff line change
Expand Up @@ -1434,7 +1434,7 @@ def link_whole(self, targets, promoted: bool = False):
msg += "Use the 'pic' option to static_library to build with PIC."
raise InvalidArguments(msg)
self.check_can_link_together(t)
if isinstance(self, StaticLibrary) and not self.uses_rust():
if isinstance(self, StaticLibrary):
# When we're a static library and we link_whole: to another static
# library, we need to add that target's objects to ourselves.
self._bundle_static_library(t, promoted)
Expand All @@ -1461,7 +1461,10 @@ def get_internal_static_libraries_recurse(self, result: OrderedSet[Target]) -> N
t.get_internal_static_libraries_recurse(result)

def _bundle_static_library(self, t: T.Union[Target, CustomTargetIndex], promoted: bool = False) -> None:
if isinstance(t, (CustomTarget, CustomTargetIndex)) or t.uses_rust():
if self.uses_rust():
# Rustc can bundle static libraries, no need to extract objects.
self.link_whole_targets.append(t)
elif isinstance(t, (CustomTarget, CustomTargetIndex)) or t.uses_rust():
# To extract objects from a custom target we would have to extract
# the archive, WIP implementation can be found in
# https://github.com/mesonbuild/meson/pull/9218.
Expand All @@ -1476,7 +1479,8 @@ def _bundle_static_library(self, t: T.Union[Target, CustomTargetIndex], promoted
m += (f' Meson had to promote link to link_whole because {self.name!r} is installed but not {t.name!r},'
f' and thus has to include objects from {t.name!r} to be usable.')
raise InvalidArguments(m)
self.objects.append(t.extract_all_objects())
else:
self.objects.append(t.extract_all_objects())

def check_can_link_together(self, t: BuildTargetTypes) -> None:
links_with_rust_abi = isinstance(t, BuildTarget) and t.uses_rust_abi()
Expand Down
4 changes: 4 additions & 0 deletions test cases/rust/20 transitive dependencies/diamond/func.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
int c_func(void);
int c_func(void) {
return 123;
}
5 changes: 5 additions & 0 deletions test cases/rust/20 transitive dependencies/diamond/main.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
int r3(void);

int main_func(void) {
return r3() == 246 ? 0 : 1;
}
25 changes: 25 additions & 0 deletions test cases/rust/20 transitive dependencies/diamond/meson.build
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
# Regression test for a diamond dependency graph:
# ┌►R1┐
# main-►R3─┤ ├─►C1
# └►R2┘
# Both libr1.rlib and libr2.rlib used to contain func.c.o. That was causing
# libr3.rlib to have duplicated func.c.o and then libmain.so failed to link:
# multiple definition of `c_func'.

libc1 = static_library('c1', 'func.c')
libr1 = static_library('r1', 'r1.rs', link_with: libc1)
libr2 = static_library('r2', 'r2.rs', link_with: libc1)
libr3 = static_library('r3', 'r3.rs',
link_with: [libr1, libr2],
rust_abi: 'c',
)
shared_library('main', 'main.c', link_whole: [libr3])

# Same dependency graph, but r3 is now installed. Since c1, r1 and r2 are
# not installed, r3 must contain them.
libr3 = static_library('r3-installed', 'r3.rs',
link_with: [libr1, libr2],
rust_abi: 'c',
install: true,
)
shared_library('main-installed', 'main.c', link_with: [libr3])
9 changes: 9 additions & 0 deletions test cases/rust/20 transitive dependencies/diamond/r1.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
extern "C" {
fn c_func() -> i32;
}

pub fn r1() -> i32 {
unsafe {
c_func()
}
}
9 changes: 9 additions & 0 deletions test cases/rust/20 transitive dependencies/diamond/r2.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
extern "C" {
fn c_func() -> i32;
}

pub fn r2() -> i32 {
unsafe {
c_func()
}
}
4 changes: 4 additions & 0 deletions test cases/rust/20 transitive dependencies/diamond/r3.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
#[no_mangle]
pub fn r3() -> i32 {
r1::r1() + r2::r2()
}
2 changes: 2 additions & 0 deletions test cases/rust/20 transitive dependencies/meson.build
Original file line number Diff line number Diff line change
Expand Up @@ -25,3 +25,5 @@ exe = executable('footest', 'foo.c',
link_with: foo,
)
test('footest', exe)

subdir('diamond')
5 changes: 5 additions & 0 deletions test cases/rust/20 transitive dependencies/test.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"installed": [
{"type": "file", "file": "usr/lib/libr3-installed.a"}
]
}

0 comments on commit b3aae77

Please sign in to comment.