Skip to content

[libc][bazel] Prevent LIBC_NAMESPACE leakage #70455

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Nov 6, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
90 changes: 0 additions & 90 deletions utils/bazel/llvm-project-overlay/libc/BUILD.bazel

Large diffs are not rendered by default.

47 changes: 28 additions & 19 deletions utils/bazel/llvm-project-overlay/libc/libc_build_rules.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -4,43 +4,56 @@

"""LLVM libc starlark rules for building individual functions."""

load("@bazel_skylib//lib:paths.bzl", "paths")
load("@bazel_skylib//lib:selects.bzl", "selects")
load(":libc_namespace.bzl", "LIBC_NAMESPACE")
load(":platforms.bzl", "PLATFORM_CPU_ARM64", "PLATFORM_CPU_X86_64")

LIBC_ROOT_TARGET = ":libc_root"
INTERNAL_SUFFIX = ".__internal__"
def libc_internal_target(name):
return name + ".__internal__"

def _libc_library(name, copts = None, **kwargs):
def libc_common_copts():
root_label = Label(":libc")
libc_include_path = paths.join(root_label.workspace_root, root_label.package)
return [
"-I" + libc_include_path,
"-DLIBC_NAMESPACE=" + LIBC_NAMESPACE,
]

def _libc_library(name, hidden, copts = [], deps = [], **kwargs):
"""Internal macro to serve as a base for all other libc library rules.

Args:
name: Target name.
copts: The special compiler options for the target.
deps: The list of target dependencies if any.
hidden: Whether the symbols should be explicitly hidden or not.
**kwargs: All other attributes relevant for the cc_library rule.
"""
copts = copts or []

# We want all libc sources to be compiled with "hidden" visibility.
# The public symbols will be given "default" visibility explicitly.
# See src/__support/common.h for more information.
copts = copts + ["-fvisibility=hidden"]
if hidden:
copts = copts + ["-fvisibility=hidden"]
native.cc_library(
name = name,
copts = copts,
copts = copts + libc_common_copts(),
deps = deps,
linkstatic = 1,
**kwargs
)

# A convenience var which should be used to list all libc support libraries.
# A convenience function which should be used to list all libc support libraries.
# Any library which does not define a public function should be listed with
# libc_support_library.
libc_support_library = _libc_library
def libc_support_library(name, **kwargs):
_libc_library(name = name, hidden = False, **kwargs)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are we not hiding support library symbols anymore?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hiding the symbols results in internal fuzzers complaining about different visibility between modules:

default visibility for functions and variables [-fvisibility] differs in PCH file vs. current file
module file __redacted__/stl/_objs/stl_cc_library/stl_cc_library.pic.pcm cannot be loaded due to a configuration mismatch with the current compilation [-Wmodule-file-config-mismatch]

hidden = False is actually coherent with how it worked before this patch. I wanted to straighten up the visibility but it has consequences for downstream.

Now that I think about it, it makes sense that both internal targets and libc_support_library are compiled with the same visibility. I'll fix it by using libc_support_library in lieu of _libc_library in libc_function below.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lntue does it look good to you?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM


def libc_function(
name,
srcs,
weak = False,
deps = None,
copts = None,
local_defines = None,
**kwargs):
Expand All @@ -64,37 +77,33 @@ def libc_function(
its deps.
**kwargs: Other attributes relevant for a cc_library. For example, deps.
"""
deps = deps or []

# We use the explicit equals pattern here because append and += mutate the
# original list, where this creates a new list and stores it in deps.
deps = deps + [LIBC_ROOT_TARGET]
copts = copts or []
copts = copts + ["-O3", "-fno-builtin", "-fno-lax-vector-conversions"]

# We compile the code twice, the first target is suffixed with ".__internal__" and contains the
# C++ functions in the "__llvm_libc" namespace. This allows us to test the function in the
# C++ functions in the "LIBC_NAMESPACE" namespace. This allows us to test the function in the
# presence of another libc.
native.cc_library(
name = name + INTERNAL_SUFFIX,
libc_support_library(
name = libc_internal_target(name),
srcs = srcs,
deps = deps,
copts = copts,
linkstatic = 1,
**kwargs
)

# This second target is the llvm libc C function.

# This second target is the llvm libc C function with either a default or hidden visibility.
# All other functions are hidden.
func_attrs = ["__attribute__((visibility(\"default\")))"]
if weak:
func_attrs = func_attrs + ["__attribute__((weak))"]
local_defines = local_defines or ["LIBC_COPT_PUBLIC_PACKAGING"]
local_defines = local_defines + ["LLVM_LIBC_FUNCTION_ATTR='%s'" % " ".join(func_attrs)]
_libc_library(
name = name,
hidden = True,
srcs = srcs,
deps = deps,
copts = copts,
local_defines = local_defines,
**kwargs
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,13 @@

# LLVM libc unittest library.

load("//libc:libc_build_rules.bzl", "libc_support_library")

package(default_visibility = ["//visibility:public"])

licenses(["notice"])

cc_library(
libc_support_library(
name = "test_logger",
srcs = ["TestLogger.cpp"],
hdrs = ["TestLogger.h"],
Expand All @@ -17,11 +19,10 @@ cc_library(
"//libc:__support_cpp_string_view",
"//libc:__support_osutil_io",
"//libc:__support_uint128",
"//libc:libc_root",
],
)

cc_library(
libc_support_library(
name = "LibcUnitTest",
srcs = [
"BazelFilePath.cpp",
Expand Down Expand Up @@ -52,12 +53,11 @@ cc_library(
"//libc:__support_stringutil",
"//libc:__support_uint128",
"//libc:errno",
"//libc:libc_root",
"//llvm:Support",
],
)

cc_library(
libc_support_library(
name = "fp_test_helpers",
srcs = [
"FPExceptMatcher.cpp",
Expand All @@ -79,11 +79,10 @@ cc_library(
"//libc:__support_fputil_fp_bits",
"//libc:__support_fputil_fpbits_str",
"//libc:__support_fputil_rounding_mode",
"//libc:libc_root",
],
)

cc_library(
libc_support_library(
name = "memory_matcher",
srcs = [
"MemoryMatcher.cpp",
Expand All @@ -100,7 +99,7 @@ cc_library(
],
)

cc_library(
libc_support_library(
name = "printf_matcher",
srcs = [
"PrintfMatcher.cpp",
Expand All @@ -116,7 +115,7 @@ cc_library(
],
)

cc_library(
libc_support_library(
name = "string_utils",
hdrs = [
"StringUtils.h",
Expand Down
13 changes: 7 additions & 6 deletions utils/bazel/llvm-project-overlay/libc/test/libc_test_rules.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -6,33 +6,34 @@

libc functions are created though the libc_build_rules.bzl:libc_function.
They come in two flavors:
- the internal one that is scoped into the `__llvm_libc` namespace.
- the internal one that is scoped into the `LIBC_NAMESPACE` namespace.
- the libc one that is the regular C function.

When performing tests we make sure to always use the internal version.
"""

load("//libc:libc_build_rules.bzl", "INTERNAL_SUFFIX")
load("//libc:libc_build_rules.bzl", "libc_common_copts", "libc_internal_target")

def libc_test(name, srcs, libc_function_deps, deps = [], **kwargs):
def libc_test(name, srcs, libc_function_deps = [], copts = [], deps = [], **kwargs):
"""Add target for a libc test.

Args:
name: Test target name
srcs: List of sources for the test.
libc_function_deps: List of libc_function targets used by this test.
copts: The list of options to add to the C++ compilation command.
deps: The list of other libraries to be linked in to the test target.
**kwargs: Attributes relevant for a cc_test. For example, name, srcs.
**kwargs: Attributes relevant for a libc_test. For example, name, srcs.
"""
all_function_deps = libc_function_deps + ["//libc:errno"]
native.cc_test(
name = name,
srcs = srcs,
deps = [d + INTERNAL_SUFFIX for d in all_function_deps] + [
"//libc:libc_root",
deps = [libc_internal_target(d) for d in all_function_deps] + [
"//libc/test/UnitTest:LibcUnitTest",
] + deps,
features = ["-link_llvmlibc"], # Do not link libllvmlibc.a
copts = copts + libc_common_copts(),
linkstatic = 1,
**kwargs
)
Original file line number Diff line number Diff line change
Expand Up @@ -4,119 +4,83 @@

# Tests for LLVM libc CPP functions.

load("//libc/test:libc_test_rules.bzl", "libc_test")

package(default_visibility = ["//visibility:public"])

licenses(["notice"])

cc_test(
libc_test(
name = "atomic_test",
srcs = ["atomic_test.cpp"],
deps = [
"//libc:__support_cpp_atomic",
"//libc:libc_root",
"//libc/test/UnitTest:LibcUnitTest",
],
deps = ["//libc:__support_cpp_atomic"],
)

cc_test(
libc_test(
name = "bitset_test",
srcs = ["bitset_test.cpp"],
deps = [
"//libc:__support_cpp_bitset",
"//libc:libc_root",
"//libc/test/UnitTest:LibcUnitTest",
],
deps = ["//libc:__support_cpp_bitset"],
)

cc_test(
libc_test(
name = "cstddef_test",
srcs = ["cstddef_test.cpp"],
deps = [
"//libc:__support_cpp_cstddef",
"//libc:libc_root",
"//libc/test/UnitTest:LibcUnitTest",
],
deps = ["//libc:__support_cpp_cstddef"],
)

cc_test(
libc_test(
name = "integer_sequence_test",
srcs = ["integer_sequence_test.cpp"],
deps = [
"//libc:__support_cpp_utility",
"//libc:libc_root",
"//libc/test/UnitTest:LibcUnitTest",
],
deps = ["//libc:__support_cpp_utility"],
)

cc_test(
libc_test(
name = "limits_test",
srcs = ["limits_test.cpp"],
deps = [
"//libc:__support_cpp_limits",
"//libc:__support_uint",
"//libc:libc_root",
"//libc/test/UnitTest:LibcUnitTest",
],
)

cc_test(
libc_test(
name = "optional_test",
srcs = ["optional_test.cpp"],
deps = [
"//libc:__support_cpp_optional",
"//libc:libc_root",
"//libc/test/UnitTest:LibcUnitTest",
],
deps = ["//libc:__support_cpp_optional"],
)

cc_test(
libc_test(
name = "span_test",
srcs = ["span_test.cpp"],
deps = [
"//libc:__support_cpp_array",
"//libc:__support_cpp_span",
"//libc:libc_root",
"//libc/test/UnitTest:LibcUnitTest",
],
)

cc_test(
libc_test(
name = "stringstream_test",
srcs = ["stringstream_test.cpp"],
deps = [
"//libc:__support_cpp_span",
"//libc:__support_cpp_stringstream",
"//libc:libc_root",
"//libc/test/UnitTest:LibcUnitTest",
],
)

cc_test(
libc_test(
name = "string_test",
srcs = ["string_test.cpp"],
deps = [
"//libc:__support_cpp_string",
"//libc:libc_root",
"//libc/test/UnitTest:LibcUnitTest",
],
deps = ["//libc:__support_cpp_string"],
)

cc_test(
libc_test(
name = "stringview_test",
srcs = ["stringview_test.cpp"],
deps = [
"//libc:__support_cpp_string_view",
"//libc:libc_root",
"//libc/test/UnitTest:LibcUnitTest",
],
deps = ["//libc:__support_cpp_string_view"],
)

cc_test(
libc_test(
name = "type_traits_test",
srcs = ["type_traits_test.cpp"],
deps = [
"//libc:__support_cpp_type_traits",
"//libc:libc_root",
"//libc/test/UnitTest:LibcUnitTest",
],
deps = ["//libc:__support_cpp_type_traits"],
)
Loading