[libc][bazel] Prevent LIBC_NAMESPACE leakage #70455
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@llvm/pr-subscribers-libc Author: Guillaume Chatelet (gchatelet) ChangesRight now This PR removes the Patch is 37.14 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/70455.diff 9 Files Affected:
diff --git a/utils/bazel/llvm-project-overlay/libc/BUILD.bazel b/utils/bazel/llvm-project-overlay/libc/BUILD.bazel
index 3ae68193dccd2b2..f2449cbfc17045d 100644
--- a/utils/bazel/llvm-project-overlay/libc/BUILD.bazel
+++ b/utils/bazel/llvm-project-overlay/libc/BUILD.bazel
@@ -11,7 +11,6 @@ load(
"libc_math_function",
"libc_support_library",
)
-load(":libc_namespace.bzl", "LIBC_NAMESPACE")
load(":platforms.bzl", "PLATFORM_CPU_ARM64", "PLATFORM_CPU_X86_64")
package(
@@ -63,29 +62,16 @@ config_setting(
flag_values = {":mpfr": "system"},
)
-# This empty root library helps us add an include path to this directory
-# using the 'includes' attribute. The strings listed in the includes attribute
-# are relative paths wrt this library but are inherited by the dependents
-# appropriately. Hence, using this as a root dependency avoids adding include
-# paths of the kind "../../" to other libc targets.
-cc_library(
- name = "libc_root",
- defines = ["LIBC_NAMESPACE=" + LIBC_NAMESPACE],
- includes = ["."],
-)
-
############################## Support libraries #############################
libc_support_library(
name = "__support_macros_properties_architectures",
hdrs = ["src/__support/macros/properties/architectures.h"],
- deps = [":libc_root"],
)
libc_support_library(
name = "__support_macros_properties_compiler",
hdrs = ["src/__support/macros/properties/compiler.h"],
- deps = [":libc_root"],
)
libc_support_library(
@@ -93,14 +79,12 @@ libc_support_library(
hdrs = ["src/__support/macros/properties/cpu_features.h"],
deps = [
"__support_macros_properties_architectures",
- ":libc_root",
],
)
libc_support_library(
name = "__support_macros_config",
hdrs = ["src/__support/macros/config.h"],
- deps = [":libc_root"],
)
libc_support_library(
@@ -108,7 +92,6 @@ libc_support_library(
hdrs = ["src/__support/macros/attributes.h"],
deps = [
":__support_macros_properties_architectures",
- ":libc_root",
],
)
@@ -119,7 +102,6 @@ libc_support_library(
":__support_macros_attributes",
":__support_macros_config",
":__support_macros_properties_compiler",
- ":libc_root",
],
)
@@ -128,7 +110,6 @@ libc_support_library(
hdrs = ["src/__support/macros/sanitizer.h"],
deps = [
":__support_macros_config",
- ":libc_root",
],
)
@@ -141,7 +122,6 @@ libc_support_library(
deps = [
":__support_macros_attributes",
":__support_macros_properties_architectures",
- ":libc_root",
],
)
@@ -150,7 +130,6 @@ libc_support_library(
hdrs = ["src/__support/CPP/algorithm.h"],
deps = [
":__support_macros_attributes",
- ":libc_root",
],
)
@@ -159,7 +138,6 @@ libc_support_library(
hdrs = ["src/__support/CPP/array.h"],
deps = [
":__support_macros_attributes",
- ":libc_root",
],
)
@@ -171,7 +149,6 @@ libc_support_library(
":__support_macros_attributes",
":__support_macros_config",
":__support_macros_sanitizer",
- ":libc_root",
],
)
@@ -180,7 +157,6 @@ libc_support_library(
hdrs = ["src/__support/CPP/bitset.h"],
deps = [
":__support_macros_attributes",
- ":libc_root",
],
)
@@ -190,7 +166,6 @@ libc_support_library(
deps = [
":__support_cpp_type_traits",
":__support_macros_attributes",
- ":libc_root",
],
)
@@ -198,7 +173,6 @@ libc_support_library(
name = "__support_cpp_expected",
hdrs = ["src/__support/CPP/expected.h"],
deps = [
- ":libc_root",
],
)
@@ -209,14 +183,12 @@ libc_support_library(
"__support_cpp_type_traits",
"__support_cpp_utility",
"__support_macros_attributes",
- ":libc_root",
],
)
libc_support_library(
name = "__support_cpp_limits",
hdrs = ["src/__support/CPP/limits.h"],
- deps = [":libc_root"],
)
libc_support_library(
@@ -225,7 +197,6 @@ libc_support_library(
hdrs = ["src/__support/CPP/new.h"],
deps = [
":__support_common",
- ":libc_root",
],
)
@@ -234,7 +205,6 @@ libc_support_library(
hdrs = ["src/__support/CPP/optional.h"],
deps = [
":__support_cpp_utility",
- ":libc_root",
],
)
@@ -244,7 +214,6 @@ libc_support_library(
deps = [
":__support_cpp_array",
":__support_cpp_type_traits",
- ":libc_root",
],
)
@@ -253,7 +222,6 @@ libc_support_library(
hdrs = ["src/__support/CPP/string_view.h"],
deps = [
":__support_common",
- ":libc_root",
],
)
@@ -275,7 +243,6 @@ libc_support_library(
":__support_common",
":__support_cpp_string_view",
":__support_integer_to_string",
- ":libc_root",
":string_memory_utils",
":string_utils",
],
@@ -340,7 +307,6 @@ libc_support_library(
deps = [
":__support_macros_attributes",
":__support_macros_config",
- ":libc_root",
],
)
@@ -357,7 +323,6 @@ libc_support_library(
deps = [
":__support_cpp_type_traits",
":__support_macros_attributes",
- ":libc_root",
],
)
@@ -368,7 +333,6 @@ libc_support_library(
":__support_cpp_type_traits",
":__support_macros_attributes",
":__support_macros_properties_architectures",
- ":libc_root",
],
)
@@ -377,7 +341,6 @@ libc_support_library(
hdrs = ["src/__support/arg_list.h"],
deps = [
":__support_common",
- ":libc_root",
],
)
@@ -386,7 +349,6 @@ libc_support_library(
hdrs = ["src/__support/c_string.h"],
deps = [
":__support_cpp_string",
- ":libc_root",
],
)
@@ -396,7 +358,6 @@ libc_support_library(
deps = [
":__support_common",
":__support_cpp_expected",
- ":libc_root",
],
)
@@ -415,7 +376,6 @@ libc_support_library(
":__support_fputil_fp_bits",
":__support_libc_assert",
":__support_uint",
- ":libc_root",
],
)
@@ -425,7 +385,6 @@ libc_support_library(
deps = [
":__support_cpp_type_traits",
":__support_named_pair",
- ":libc_root",
],
)
@@ -437,7 +396,6 @@ libc_support_library(
":__support_common",
":__support_cpp_type_traits",
":__support_number_pair",
- ":libc_root",
],
)
@@ -454,7 +412,6 @@ libc_support_library(
":__support_macros_attributes",
":__support_macros_optimization",
":__support_number_pair",
- ":libc_root",
],
)
@@ -463,7 +420,6 @@ libc_support_library(
hdrs = ["src/__support/UInt128.h"],
deps = [
":__support_uint",
- ":libc_root",
],
)
@@ -471,7 +427,6 @@ libc_support_library(
name = "__support_str_to_num_result",
hdrs = ["src/__support/str_to_num_result.h"],
deps = [
- ":libc_root",
],
)
@@ -493,7 +448,6 @@ libc_support_library(
":__support_cpp_span",
":__support_cpp_string_view",
":__support_cpp_type_traits",
- ":libc_root",
],
)
@@ -505,7 +459,6 @@ libc_support_library(
":__support_macros_attributes",
":__support_osutil_io",
":__support_osutil_quick_exit",
- ":libc_root",
],
)
@@ -559,7 +512,6 @@ libc_support_library(
":__support_common",
":__support_cpp_type_traits",
":__support_fputil_fp_bits",
- ":libc_root",
],
)
@@ -573,7 +525,6 @@ libc_support_library(
":__support_error_or",
":__support_threads_mutex",
":errno",
- ":libc_root",
],
)
@@ -591,7 +542,6 @@ libc_support_library(
libc_support_library(
name = "__support_named_pair",
hdrs = ["src/__support/named_pair.h"],
- deps = [":libc_root"],
)
libc_support_library(
@@ -602,7 +552,6 @@ libc_support_library(
":__support_macros_attributes",
":__support_macros_config",
":__support_named_pair",
- ":libc_root",
],
)
@@ -616,7 +565,6 @@ libc_support_library(
":__support_cpp_type_traits",
":__support_fputil_fenv_impl",
":__support_fputil_fp_bits",
- ":libc_root",
":math_utils",
],
)
@@ -630,7 +578,6 @@ libc_support_library(
":__support_fputil_fp_bits",
":__support_fputil_manipulation_functions",
":__support_fputil_normal_float",
- ":libc_root",
],
)
@@ -642,7 +589,6 @@ libc_support_library(
":__support_fputil_fenv_impl",
":__support_fputil_fp_bits",
":__support_fputil_rounding_mode",
- ":libc_root",
],
)
@@ -660,7 +606,6 @@ libc_support_library(
":__support_macros_properties_architectures",
":__support_macros_sanitizer",
":errno",
- ":libc_root",
],
)
@@ -669,7 +614,6 @@ libc_support_library(
hdrs = ["src/__support/FPUtil/rounding_mode.h"],
deps = [
":__support_macros_attributes",
- ":libc_root",
],
)
@@ -679,7 +623,6 @@ libc_support_library(
deps = [
":__support_fputil_platform_defs",
":__support_uint128",
- ":libc_root",
],
)
@@ -695,7 +638,6 @@ libc_support_library(
":__support_fputil_float_properties",
":__support_fputil_platform_defs",
":__support_uint128",
- ":libc_root",
],
)
@@ -711,7 +653,6 @@ libc_support_library(
":__support_fputil_platform_defs",
":__support_integer_to_string",
":__support_uint128",
- ":libc_root",
],
)
@@ -728,7 +669,6 @@ libc_support_library(
":__support_fputil_fp_bits",
":__support_fputil_rounding_mode",
":__support_uint128",
- ":libc_root",
],
)
@@ -746,7 +686,6 @@ libc_support_library(
":__support_fputil_platform_defs",
":__support_macros_optimization",
":__support_uint128",
- ":libc_root",
],
)
@@ -759,7 +698,6 @@ libc_support_library(
":__support_fputil_fenv_impl",
":__support_fputil_fp_bits",
":__support_fputil_rounding_mode",
- ":libc_root",
],
)
@@ -770,7 +708,6 @@ libc_support_library(
":__support_common",
":__support_cpp_type_traits",
":__support_fputil_fp_bits",
- ":libc_root",
],
)
@@ -779,7 +716,6 @@ libc_support_library(
hdrs = ["src/__support/FPUtil/PlatformDefs.h"],
deps = [
":__support_common",
- ":libc_root",
],
)
@@ -812,7 +748,6 @@ libc_support_library(
":__support_fputil_platform_defs",
":__support_fputil_rounding_mode",
":__support_uint128",
- ":libc_root",
],
)
@@ -844,7 +779,6 @@ libc_support_library(
":__support_macros_optimization",
":__support_macros_properties_cpu_features",
":__support_uint128",
- ":libc_root",
],
)
@@ -891,7 +825,6 @@ libc_support_library(
":__support_macros_optimization",
":__support_macros_properties_architectures",
":__support_macros_properties_cpu_features",
- ":libc_root",
],
)
@@ -902,7 +835,6 @@ libc_support_library(
":__support_common",
":__support_fputil_multiply_add",
":__support_number_pair",
- ":libc_root",
],
)
@@ -911,7 +843,6 @@ libc_support_library(
hdrs = ["src/__support/FPUtil/triple_double.h"],
deps = [
":__support_common",
- ":libc_root",
],
)
@@ -925,7 +856,6 @@ libc_support_library(
":__support_fputil_multiply_add",
":__support_macros_optimization",
":__support_uint",
- ":libc_root",
],
)
@@ -940,7 +870,6 @@ libc_support_library(
deps = [
":__support_common",
":__support_cpp_bit",
- ":libc_root",
],
)
@@ -954,7 +883,6 @@ libc_support_library(
":__support_common",
":__support_cpp_string_view",
":__support_osutil_syscall",
- ":libc_root",
":string_utils",
],
)
@@ -968,7 +896,6 @@ libc_support_library(
],
deps = [
":__support_osutil_syscall",
- ":libc_root",
],
)
@@ -993,7 +920,6 @@ libc_support_library(
":__support_integer_to_string",
":__support_macros_attributes",
":errno",
- ":libc_root",
],
)
@@ -1010,7 +936,6 @@ libc_support_library(
deps = [
":__support_cpp_atomic",
":__support_osutil_syscall",
- ":libc_root",
],
)
@@ -1180,7 +1105,6 @@ libc_support_library(
"__support_cpp_type_traits",
":__support_common",
":errno",
- ":libc_root",
],
)
@@ -1191,7 +1115,6 @@ libc_support_library(
deps = [
":__support_fputil_triple_double",
":__support_number_pair",
- ":libc_root",
],
)
@@ -1207,7 +1130,6 @@ libc_support_library(
":__support_fputil_fp_bits",
":__support_fputil_multiply_add",
":__support_fputil_nearest_integer",
- ":libc_root",
],
)
@@ -1217,7 +1139,6 @@ libc_support_library(
deps = [
":__support_fputil_fp_bits",
":__support_fputil_polyeval",
- ":libc_root",
":range_reduction",
],
)
@@ -1235,7 +1156,6 @@ libc_support_library(
":__support_fputil_nearest_integer",
":__support_fputil_polyeval",
":common_constants",
- ":libc_root",
":math_utils",
],
)
@@ -1252,7 +1172,6 @@ libc_support_library(
":__support_fputil_multiply_add",
":__support_fputil_nearest_integer",
":__support_fputil_polyeval",
- ":libc_root",
":math_utils",
],
)
@@ -2221,7 +2140,6 @@ libc_support_library(
":__support_macros_optimization",
":__support_macros_properties_architectures",
":__support_macros_properties_cpu_features",
- ":libc_root",
],
)
@@ -2232,7 +2150,6 @@ libc_support_library(
":__support_common",
":__support_cpp_bitset",
":__support_macros_optimization",
- ":libc_root",
":string_memory_utils",
],
)
@@ -2692,7 +2609,6 @@ libc_support_library(
deps = [
":__support_cpp_string_view",
":__support_fputil_fp_bits",
- ":libc_root",
],
)
@@ -2701,7 +2617,6 @@ libc_support_library(
hdrs = ["src/stdio/printf_core/printf_config.h"],
defines = PRINTF_COPTS,
deps = [
- ":libc_root",
],
)
@@ -2719,7 +2634,6 @@ libc_support_library(
":__support_ctype_utils",
":__support_fputil_fp_bits",
":__support_str_to_integer",
- ":libc_root",
":printf_config",
":printf_core_structs",
],
@@ -2740,7 +2654,6 @@ libc_support_library(
":__support_ctype_utils",
":__support_fputil_fp_bits",
":__support_str_to_integer",
- ":libc_root",
":printf_config",
":printf_core_structs",
],
@@ -2754,7 +2667,6 @@ libc_support_library(
deps = [
":__support_cpp_string_view",
":__support_macros_optimization",
- ":libc_root",
":printf_core_structs",
":string_memory_utils",
],
@@ -2791,7 +2703,6 @@ libc_support_library(
":__support_libc_assert",
":__support_uint",
":__support_uint128",
- ":libc_root",
":printf_core_structs",
":printf_writer",
],
@@ -2804,7 +2715,6 @@ libc_support_library(
defines = PRINTF_COPTS,
deps = [
":__support_arg_list",
- ":libc_root",
":printf_converter",
":printf_core_structs",
":printf_parser",
diff --git a/utils/bazel/llvm-project-overlay/libc/libc_build_rules.bzl b/utils/bazel/llvm-project-overlay/libc/libc_build_rules.bzl
index 0f4f3a40d380574..8314b3d7d4e7270 100644
--- a/utils/bazel/llvm-project-overlay/libc/libc_build_rules.bzl
+++ b/utils/bazel/llvm-project-overlay/libc/libc_build_rules.bzl
@@ -4,43 +4,64 @@
"""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 _package_path(label):
+ """Returns the path to the package of 'label'.
+
+ Args:
+ label: label. The label to return the package path of.
+
+ For example, package_path("@foo//bar:BUILD") returns 'external/foo/bar'.
+ """
+ return paths.join(Label(label).workspace_root, Label(label).package)
+
+def libc_common_copts():
+ return [
+ "-I" + _package_path("@llvm-project//libc"),
+ "-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 = True, **kwargs)
def libc_function(
name,
srcs,
weak = False,
- deps = None,
copts = None,
local_defines = None,
**kwargs):
@@ -64,28 +85,25 @@ 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_library(
+ name = libc_internal_target(name),
+ hidden = False,
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))"]
@@ -93,8 +111,8 @@ def libc_function(
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_def...
[truncated]
|
|
I like the move to every target being through a |
This helps downstream transformations Also inline function - it's not used elsewhere.
|
Thx for the review @michaelrj-google . Can you PTAL? |
lntue
reviewed
Oct 30, 2023
| @@ -48,7 +48,7 @@ def _libc_library(name, hidden, copts = [], deps = [], **kwargs): | |||
| # Any library which does not define a public function should be listed with | |||
| # libc_support_library. | |||
| def libc_support_library(name, **kwargs): | |||
| _libc_library(name = name, hidden = True, **kwargs) | |||
| _libc_library(name = name, hidden = False, **kwargs) | |||
There was a problem hiding this comment.
Why are we not hiding support library symbols anymore?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
@lntue does it look good to you?
michaelrj-google
approved these changes
Nov 1, 2023
lntue
approved these changes
Nov 6, 2023
gchatelet
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Right now
LIBC_NAMESPACEdefinition is added to all rules that depend uponany libc function. This have far-reaching effects which prevents llvm-libc
development when the installed libc is itself llvm-libc.
This PR removes the
:libc_roottarget entirely and consistently use theprovided libc BUILD actions for compilation and testing. This makes sure that we
don't leak
LIBC_NAMESPACEto our client.