From 59de280a480cd7315de32adb7e4465a5480f7494 Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Mon, 13 Jun 2022 14:24:18 -0700 Subject: [PATCH] Fix -Wl,--exclude-libs for clang runtime libraries THe sanitize code was assuming that the names of the clang runtime library modules were the same as their static library output files, but that's not true after I39e2cf8ae14edf8510276dab38011afaef85822c. Use the dependency to get the name of the library to pass to -Wl,--exclude-libs. Bug: 235624976 Test: TestUbsan Change-Id: If6ca7838800c76f90105fb02d39e8a68cec96314 --- cc/cc.go | 21 ++++++++ cc/linker.go | 8 +-- cc/sanitize.go | 10 +--- cc/sanitize_test.go | 115 ++++++++++++++++++++++++++++++++++++++++++++ cc/testing.go | 27 ++++++++++- 5 files changed, 165 insertions(+), 16 deletions(-) diff --git a/cc/cc.go b/cc/cc.go index 55c0e4830..301509e82 100644 --- a/cc/cc.go +++ b/cc/cc.go @@ -95,6 +95,10 @@ type Deps struct { HeaderLibs []string RuntimeLibs []string + // UnexportedStaticLibs are static libraries that are also passed to -Wl,--exclude-libs= to + // prevent automatically exporting symbols. + UnexportedStaticLibs []string + // Used for data dependencies adjacent to tests DataLibs []string DataBins []string @@ -156,6 +160,7 @@ type PathDeps struct { GeneratedDeps android.Paths Flags []string + LdFlags []string IncludeDirs android.Paths SystemIncludeDirs android.Paths ReexportedDirs android.Paths @@ -678,6 +683,9 @@ type libraryDependencyTag struct { // Whether or not this dependency has to be followed for the apex variants excludeInApex bool + + // If true, don't automatically export symbols from the static library into a shared library. + unexportedSymbols bool } // header returns true if the libraryDependencyTag is tagging a header lib dependency. @@ -1921,6 +1929,8 @@ func (c *Module) GenerateAndroidBuildActions(actx android.ModuleContext) { flags.Local.CommonFlags = append(flags.Local.CommonFlags, "-isystem "+dir.String()) } + flags.Local.LdFlags = append(flags.Local.LdFlags, deps.LdFlags...) + c.flags = flags // We need access to all the flags seen by a source file. if c.sabi != nil { @@ -2368,6 +2378,13 @@ func (c *Module) DepsMutator(actx android.BottomUpMutatorContext) { }, depTag, RewriteSnapshotLib(lib, GetSnapshot(c, &snapshotInfo, actx).StaticLibs)) } + for _, lib := range deps.UnexportedStaticLibs { + depTag := libraryDependencyTag{Kind: staticLibraryDependency, Order: lateLibraryDependency, unexportedSymbols: true} + actx.AddVariationDependencies([]blueprint.Variation{ + {Mutator: "link", Variation: "static"}, + }, depTag, RewriteSnapshotLib(lib, GetSnapshot(c, &snapshotInfo, actx).StaticLibs)) + } + for _, lib := range deps.LateSharedLibs { if inList(lib, sharedLibNames) { // This is to handle the case that some of the late shared libs (libc, libdl, libm, ...) @@ -2866,6 +2883,10 @@ func (c *Module) depsToPaths(ctx android.ModuleContext) PathDeps { panic(fmt.Errorf("unexpected library dependency order %d", libDepTag.Order)) } } + if libDepTag.unexportedSymbols { + depPaths.LdFlags = append(depPaths.LdFlags, + "-Wl,--exclude-libs="+staticLibraryInfo.StaticLibrary.Base()) + } } if libDepTag.static() && !libDepTag.wholeStatic { diff --git a/cc/linker.go b/cc/linker.go index 4e9404c0f..78d2d4186 100644 --- a/cc/linker.go +++ b/cc/linker.go @@ -399,7 +399,7 @@ func (linker *baseLinker) linkerDeps(ctx DepsContext, deps Deps) Deps { if ctx.toolchain().Bionic() { // libclang_rt.builtins has to be last on the command line if !Bool(linker.Properties.No_libcrt) && !ctx.header() { - deps.LateStaticLibs = append(deps.LateStaticLibs, config.BuiltinsRuntimeLibrary(ctx.toolchain())) + deps.UnexportedStaticLibs = append(deps.UnexportedStaticLibs, config.BuiltinsRuntimeLibrary(ctx.toolchain())) } if inList("libdl", deps.SharedLibs) { @@ -422,7 +422,7 @@ func (linker *baseLinker) linkerDeps(ctx DepsContext, deps Deps) Deps { } } else if ctx.toolchain().Musl() { if !Bool(linker.Properties.No_libcrt) && !ctx.header() { - deps.LateStaticLibs = append(deps.LateStaticLibs, config.BuiltinsRuntimeLibrary(ctx.toolchain())) + deps.UnexportedStaticLibs = append(deps.UnexportedStaticLibs, config.BuiltinsRuntimeLibrary(ctx.toolchain())) } } @@ -530,10 +530,6 @@ func (linker *baseLinker) linkerFlags(ctx ModuleContext, flags Flags) Flags { } } - if ctx.toolchain().LibclangRuntimeLibraryArch() != "" { - flags.Global.LdFlags = append(flags.Global.LdFlags, "-Wl,--exclude-libs="+config.BuiltinsRuntimeLibrary(ctx.toolchain())+".a") - } - CheckBadLinkerFlags(ctx, "ldflags", linker.Properties.Ldflags) flags.Local.LdFlags = append(flags.Local.LdFlags, proptools.NinjaAndShellEscapeList(linker.Properties.Ldflags)...) diff --git a/cc/sanitize.go b/cc/sanitize.go index 42a112ed4..c11900b6c 100644 --- a/cc/sanitize.go +++ b/cc/sanitize.go @@ -588,13 +588,6 @@ func toDisableUnsignedShiftBaseChange(flags []string) bool { } func (sanitize *sanitize) flags(ctx ModuleContext, flags Flags) Flags { - minimalRuntimeLib := config.UndefinedBehaviorSanitizerMinimalRuntimeLibrary(ctx.toolchain()) + ".a" - - if sanitize.Properties.MinimalRuntimeDep { - flags.Local.LdFlags = append(flags.Local.LdFlags, - "-Wl,--exclude-libs,"+minimalRuntimeLib) - } - if !sanitize.Properties.SanitizerEnabled && !sanitize.Properties.UbsanRuntimeDep { return flags } @@ -724,7 +717,6 @@ func (sanitize *sanitize) flags(ctx ModuleContext, flags Flags) Flags { if enableMinimalRuntime(sanitize) { flags.Local.CFlags = append(flags.Local.CFlags, strings.Join(minimalRuntimeFlags, " ")) - flags.Local.LdFlags = append(flags.Local.LdFlags, "-Wl,--exclude-libs,"+minimalRuntimeLib) } if Bool(sanitize.Properties.Sanitize.Fuzzer) { @@ -1217,7 +1209,7 @@ func sanitizerRuntimeMutator(mctx android.BottomUpMutatorContext) { } // static executable gets static runtime libs - depTag := libraryDependencyTag{Kind: staticLibraryDependency} + depTag := libraryDependencyTag{Kind: staticLibraryDependency, unexportedSymbols: true} variations := append(mctx.Target().Variations(), blueprint.Variation{Mutator: "link", Variation: "static"}) if c.Device() { diff --git a/cc/sanitize_test.go b/cc/sanitize_test.go index c1ca03408..0e5552f8c 100644 --- a/cc/sanitize_test.go +++ b/cc/sanitize_test.go @@ -201,6 +201,121 @@ func TestAsan(t *testing.T) { t.Run("device", func(t *testing.T) { check(t, result, "android_arm64_armv8-a") }) } +func TestUbsan(t *testing.T) { + bp := ` + cc_binary { + name: "bin_with_ubsan", + host_supported: true, + shared_libs: [ + "libshared", + ], + static_libs: [ + "libstatic", + "libnoubsan", + ], + sanitize: { + undefined: true, + } + } + + cc_binary { + name: "bin_depends_ubsan", + host_supported: true, + shared_libs: [ + "libshared", + ], + static_libs: [ + "libstatic", + "libubsan", + "libnoubsan", + ], + } + + cc_binary { + name: "bin_no_ubsan", + host_supported: true, + shared_libs: [ + "libshared", + ], + static_libs: [ + "libstatic", + "libnoubsan", + ], + } + + cc_library_shared { + name: "libshared", + host_supported: true, + shared_libs: ["libtransitive"], + } + + cc_library_shared { + name: "libtransitive", + host_supported: true, + } + + cc_library_static { + name: "libubsan", + host_supported: true, + sanitize: { + undefined: true, + } + } + + cc_library_static { + name: "libstatic", + host_supported: true, + } + + cc_library_static { + name: "libnoubsan", + host_supported: true, + } + ` + + result := android.GroupFixturePreparers( + prepareForCcTest, + ).RunTestWithBp(t, bp) + + check := func(t *testing.T, result *android.TestResult, variant string) { + staticVariant := variant + "_static" + + minimalRuntime := result.ModuleForTests("libclang_rt.ubsan_minimal", staticVariant) + + // The binaries, one with ubsan and one without + binWithUbsan := result.ModuleForTests("bin_with_ubsan", variant) + binDependsUbsan := result.ModuleForTests("bin_depends_ubsan", variant) + binNoUbsan := result.ModuleForTests("bin_no_ubsan", variant) + + android.AssertStringListContains(t, "missing libclang_rt.ubsan_minimal in bin_with_ubsan static libs", + strings.Split(binWithUbsan.Rule("ld").Args["libFlags"], " "), + minimalRuntime.OutputFiles(t, "")[0].String()) + + android.AssertStringListContains(t, "missing libclang_rt.ubsan_minimal in bin_depends_ubsan static libs", + strings.Split(binDependsUbsan.Rule("ld").Args["libFlags"], " "), + minimalRuntime.OutputFiles(t, "")[0].String()) + + android.AssertStringListDoesNotContain(t, "unexpected libclang_rt.ubsan_minimal in bin_no_ubsan static libs", + strings.Split(binNoUbsan.Rule("ld").Args["libFlags"], " "), + minimalRuntime.OutputFiles(t, "")[0].String()) + + android.AssertStringListContains(t, "missing -Wl,--exclude-libs for minimal runtime in bin_with_ubsan", + strings.Split(binWithUbsan.Rule("ld").Args["ldFlags"], " "), + "-Wl,--exclude-libs="+minimalRuntime.OutputFiles(t, "")[0].Base()) + + android.AssertStringListContains(t, "missing -Wl,--exclude-libs for minimal runtime in bin_depends_ubsan static libs", + strings.Split(binDependsUbsan.Rule("ld").Args["ldFlags"], " "), + "-Wl,--exclude-libs="+minimalRuntime.OutputFiles(t, "")[0].Base()) + + android.AssertStringListDoesNotContain(t, "unexpected -Wl,--exclude-libs for minimal runtime in bin_no_ubsan static libs", + strings.Split(binNoUbsan.Rule("ld").Args["ldFlags"], " "), + "-Wl,--exclude-libs="+minimalRuntime.OutputFiles(t, "")[0].Base()) + } + + t.Run("host", func(t *testing.T) { check(t, result, result.Config.BuildOSTarget.String()) }) + t.Run("device", func(t *testing.T) { check(t, result, "android_arm64_armv8-a") }) +} + type MemtagNoteType int const ( diff --git a/cc/testing.go b/cc/testing.go index ecdae8b15..077fcda3c 100644 --- a/cc/testing.go +++ b/cc/testing.go @@ -73,7 +73,6 @@ func commonDefaultModules() string { nocrt: true, system_shared_libs: [], stl: "none", - srcs: [""], check_elf_files: false, sanitize: { never: true, @@ -84,6 +83,7 @@ func commonDefaultModules() string { name: "libcompiler_rt-extras", defaults: ["toolchain_libs_defaults"], vendor_ramdisk_available: true, + srcs: [""], } cc_prebuilt_library_static { @@ -93,11 +93,13 @@ func commonDefaultModules() string { vendor_available: true, vendor_ramdisk_available: true, native_bridge_supported: true, + srcs: [""], } cc_prebuilt_library_shared { name: "libclang_rt.hwasan", defaults: ["toolchain_libs_defaults"], + srcs: [""], } cc_prebuilt_library_static { @@ -108,6 +110,7 @@ func commonDefaultModules() string { ], vendor_ramdisk_available: true, native_bridge_supported: true, + srcs: [""], } cc_prebuilt_library_static { @@ -116,17 +119,34 @@ func commonDefaultModules() string { "linux_bionic_supported", "toolchain_libs_defaults", ], + srcs: [""], } // Needed for sanitizer cc_prebuilt_library_shared { name: "libclang_rt.ubsan_standalone", defaults: ["toolchain_libs_defaults"], + srcs: [""], } cc_prebuilt_library_static { name: "libclang_rt.ubsan_minimal", defaults: ["toolchain_libs_defaults"], + host_supported: true, + target: { + android_arm64: { + srcs: ["libclang_rt.ubsan_minimal.android_arm64.a"], + }, + android_arm: { + srcs: ["libclang_rt.ubsan_minimal.android_arm.a"], + }, + linux_glibc_x86_64: { + srcs: ["libclang_rt.ubsan_minimal.x86_64.a"], + }, + linux_glibc_x86: { + srcs: ["libclang_rt.ubsan_minimal.x86.a"], + }, + }, } cc_library { @@ -546,6 +566,11 @@ var PrepareForTestWithCcDefaultModules = android.GroupFixturePreparers( "defaults/cc/common/crtend_so.c": nil, "defaults/cc/common/crtend.c": nil, "defaults/cc/common/crtbrand.c": nil, + + "defaults/cc/common/libclang_rt.ubsan_minimal.android_arm64.a": nil, + "defaults/cc/common/libclang_rt.ubsan_minimal.android_arm.a": nil, + "defaults/cc/common/libclang_rt.ubsan_minimal.x86_64.a": nil, + "defaults/cc/common/libclang_rt.ubsan_minimal.x86.a": nil, }.AddToFixture(), // Place the default cc test modules that are common to all platforms in a location that will not