Skip to content

Commit

Permalink
[7.4.0] Do not wrap object files in --start-lib/--end-lib for nodep…
Browse files Browse the repository at this point in the history 
…s libraries (bazelbuild#23437)

When linking a nodeps dynamic library that contains object files
produced from tree artifact sources, the library may not contain any
references to the symbols in those files. In this case, when wrapping
the files in `--start-lib/--end-lib`, they may end up being dropped by
the linker, which results in missing symbols compared to a build with
`--dynamic_mode=off`.

This is fixed by not wrapping object files into `--start-lib/--end-lib`
for the nodeps dynamic library link action.

Fixes bazelbuild/rules_cc#230

Closes bazelbuild#23084.

PiperOrigin-RevId: 667945814
Change-Id: Iec148ad1b86626bb5263fecc9b98b0db89f81ad6

Commit
bazelbuild@164ffa5

Co-authored-by: Fabian Meumertzheim <fabian@meumertzhe.im>
  • Loading branch information
@bazel-io @fmeum
bazel-io and fmeum authored Sep 24, 2024
1 parent 52542f2 commit edd4aad
Show file tree
Hide file tree
Showing 2 changed files with 169 additions and 61 deletions.
85 changes: 85 additions & 0 deletions src/test/shell/bazel/cc_integration_test.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2043,4 +2043,89 @@ EOF
"@@$WORKSPACE_NAME//a:a" || fail "build failed"
}

function test_tree_artifact_sources_in_no_deps_library() {
mkdir -p pkg
cat > pkg/BUILD <<'EOF'
load("generate.bzl", "generate_source")
sh_binary(
name = "generate_tool",
srcs = ["generate.sh"],
)
generate_source(
name = "generated_source",
tool = ":generate_tool",
output_dir = "generated",
)
cc_library(
name = "hello_world",
srcs = [":generated_source"],
hdrs = [":generated_source"],
)
cc_test(
name = "testCodegen",
srcs = ["testCodegen.cpp"],
deps = [":hello_world"],
)
EOF
cat > pkg/generate.bzl <<'EOF'
def _generate_source_impl(ctx):
output_dir = ctx.attr.output_dir
files = ctx.actions.declare_directory(output_dir)
ctx.actions.run(
inputs = [],
outputs = [files],
arguments = [files.path],
executable = ctx.executable.tool
)
return [
DefaultInfo(files = depset([files]))
]
generate_source = rule(
implementation = _generate_source_impl,
attrs = {
"output_dir": attr.string(),
"tool": attr.label(executable = True, cfg = "exec")
}
)
EOF
cat > pkg/generate.sh <<'EOF2'
#!/bin/bash
OUTPUT_DIR=$1
cat << EOF > $OUTPUT_DIR/test.hpp
#pragma once
void hello_world();
EOF
cat << EOF > $OUTPUT_DIR/test.cpp
#include "test.hpp"
#include <cstdio>
void hello_world()
{
puts("Hello World!");
}
EOF
EOF2
chmod +x pkg/generate.sh
cat > pkg/testCodegen.cpp <<'EOF'
#include "pkg/generated/test.hpp"
int main() {
hello_world();
return 0;
}
EOF

bazel build //pkg:testCodegen &> "$TEST_log" || fail "Build failed"
}

run_suite "cc_integration_test"
145 changes: 84 additions & 61 deletions tools/cpp/unix_cc_toolchain_config.bzl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -903,11 +903,77 @@ def _impl(ctx):
],
)

libraries_to_link_common_flag_groups = [
flag_group(
flags = ["-Wl,-whole-archive"],
expand_if_true =
"libraries_to_link.is_whole_archive",
expand_if_equal = variable_with_value(
name = "libraries_to_link.type",
value = "static_library",
),
),
flag_group(
flags = ["%{libraries_to_link.object_files}"],
iterate_over = "libraries_to_link.object_files",
expand_if_equal = variable_with_value(
name = "libraries_to_link.type",
value = "object_file_group",
),
),
flag_group(
flags = ["%{libraries_to_link.name}"],
expand_if_equal = variable_with_value(
name = "libraries_to_link.type",
value = "object_file",
),
),
flag_group(
flags = ["%{libraries_to_link.name}"],
expand_if_equal = variable_with_value(
name = "libraries_to_link.type",
value = "interface_library",
),
),
flag_group(
flags = ["%{libraries_to_link.name}"],
expand_if_equal = variable_with_value(
name = "libraries_to_link.type",
value = "static_library",
),
),
flag_group(
flags = ["-l%{libraries_to_link.name}"],
expand_if_equal = variable_with_value(
name = "libraries_to_link.type",
value = "dynamic_library",
),
),
flag_group(
flags = ["-l:%{libraries_to_link.name}"],
expand_if_equal = variable_with_value(
name = "libraries_to_link.type",
value = "versioned_dynamic_library",
),
),
flag_group(
flags = ["-Wl,-no-whole-archive"],
expand_if_true = "libraries_to_link.is_whole_archive",
expand_if_equal = variable_with_value(
name = "libraries_to_link.type",
value = "static_library",
),
),
]

libraries_to_link_feature = feature(
name = "libraries_to_link",
flag_sets = [
flag_set(
actions = all_link_actions + lto_index_actions,
actions = [
ACTION_NAMES.cpp_link_executable,
ACTION_NAMES.cpp_link_dynamic_library,
] + lto_index_actions,
flag_groups = [
flag_group(
iterate_over = "libraries_to_link",
Expand All @@ -919,66 +985,7 @@ def _impl(ctx):
value = "object_file_group",
),
),
flag_group(
flags = ["-Wl,-whole-archive"],
expand_if_true =
"libraries_to_link.is_whole_archive",
expand_if_equal = variable_with_value(
name = "libraries_to_link.type",
value = "static_library",
),
),
flag_group(
flags = ["%{libraries_to_link.object_files}"],
iterate_over = "libraries_to_link.object_files",
expand_if_equal = variable_with_value(
name = "libraries_to_link.type",
value = "object_file_group",
),
),
flag_group(
flags = ["%{libraries_to_link.name}"],
expand_if_equal = variable_with_value(
name = "libraries_to_link.type",
value = "object_file",
),
),
flag_group(
flags = ["%{libraries_to_link.name}"],
expand_if_equal = variable_with_value(
name = "libraries_to_link.type",
value = "interface_library",
),
),
flag_group(
flags = ["%{libraries_to_link.name}"],
expand_if_equal = variable_with_value(
name = "libraries_to_link.type",
value = "static_library",
),
),
flag_group(
flags = ["-l%{libraries_to_link.name}"],
expand_if_equal = variable_with_value(
name = "libraries_to_link.type",
value = "dynamic_library",
),
),
flag_group(
flags = ["-l:%{libraries_to_link.name}"],
expand_if_equal = variable_with_value(
name = "libraries_to_link.type",
value = "versioned_dynamic_library",
),
),
flag_group(
flags = ["-Wl,-no-whole-archive"],
expand_if_true = "libraries_to_link.is_whole_archive",
expand_if_equal = variable_with_value(
name = "libraries_to_link.type",
value = "static_library",
),
),
] + libraries_to_link_common_flag_groups + [
flag_group(
flags = ["-Wl,--end-lib"],
expand_if_equal = variable_with_value(
Expand All @@ -995,6 +1002,22 @@ def _impl(ctx):
),
],
),
# Object file groups may contain symbols that aren't referenced in the same target that
# produces the object files and must thus not be wrapped in --start-lib/--end-lib when
# linking a nodeps dynamic library.
flag_set(
actions = [ACTION_NAMES.cpp_link_nodeps_dynamic_library],
flag_groups = [
flag_group(
iterate_over = "libraries_to_link",
flag_groups = libraries_to_link_common_flag_groups,
),
flag_group(
flags = ["-Wl,@%{thinlto_param_file}"],
expand_if_true = "thinlto_param_file",
),
],
),
],
)

Expand Down

0 comments on commit edd4aad

Please sign in to comment.