From 0eae52e0debb28dc27b33e915a6ab317839b7c2e Mon Sep 17 00:00:00 2001 From: Liz Kammer Date: Wed, 6 Oct 2021 10:32:26 -0400 Subject: [PATCH] bp2build; Update handling of linker flags Test: build/bazel/ci/bp2build.sh Bug: 197920036 Change-Id: I6e3100574fa0e40bcd8cf0e6af0efd3310aa41bf --- bp2build/build_conversion.go | 11 ++++++---- bp2build/cc_library_conversion_test.go | 24 +++++++++++++++------ cc/bp2build.go | 29 +++++++++++++++++--------- cc/library.go | 12 +++++++++++ cc/linker.go | 6 +++++- 5 files changed, 61 insertions(+), 21 deletions(-) diff --git a/bp2build/build_conversion.go b/bp2build/build_conversion.go index c05a62b07..aa1cf7068 100644 --- a/bp2build/build_conversion.go +++ b/bp2build/build_conversion.go @@ -239,9 +239,7 @@ func NewCodegenContext(config android.Config, context android.Context, mode Code func propsToAttributes(props map[string]string) string { var attributes string for _, propName := range android.SortedStringKeys(props) { - if shouldGenerateAttribute(propName) { - attributes += fmt.Sprintf(" %s = %s,\n", propName, props[propName]) - } + attributes += fmt.Sprintf(" %s = %s,\n", propName, props[propName]) } return attributes } @@ -422,7 +420,8 @@ func generateBazelTarget(ctx bpToBuildContext, m bp2buildModule) BazelTarget { attrs := m.BazelAttributes() props := extractModuleProperties(attrs, true) - delete(props.Attrs, "bp2build_available") + // name is handled in a special manner + delete(props.Attrs, "name") // Return the Bazel target with rule class and attributes, ready to be // code-generated. @@ -457,6 +456,10 @@ func generateSoongModuleTarget(ctx bpToBuildContext, m blueprint.Module) BazelTa depLabels[qualifiedTargetLabel(ctx, depModule)] = true }) } + + for p, _ := range ignoredPropNames { + delete(props.Attrs, p) + } attributes := propsToAttributes(props.Attrs) depLabelList := "[\n" diff --git a/bp2build/cc_library_conversion_test.go b/bp2build/cc_library_conversion_test.go index bfe88f567..8c2fe7bae 100644 --- a/bp2build/cc_library_conversion_test.go +++ b/bp2build/cc_library_conversion_test.go @@ -873,7 +873,7 @@ cc_library { }) } -func TestCcLibraryPackRelocations(t *testing.T) { +func TestCcLibraryFeatures(t *testing.T) { runCcLibraryTestCase(t, bp2buildTestCase{ description: "cc_library pack_relocations test", moduleTypeUnderTest: "cc_library", @@ -884,6 +884,7 @@ cc_library { name: "a", srcs: ["a.cpp"], pack_relocations: false, + allow_undefined_symbols: true, include_build_directory: false, } @@ -893,6 +894,7 @@ cc_library { arch: { x86_64: { pack_relocations: false, + allow_undefined_symbols: true, }, }, include_build_directory: false, @@ -904,25 +906,35 @@ cc_library { target: { darwin: { pack_relocations: false, + allow_undefined_symbols: true, }, }, include_build_directory: false, }`, expectedBazelTargets: []string{`cc_library( name = "a", - linkopts = ["-Wl,--pack-dyn-relocs=none"], + features = [ + "disable_pack_relocations", + "-no_undefined_symbols", + ], srcs = ["a.cpp"], )`, `cc_library( name = "b", - linkopts = select({ - "//build/bazel/platforms/arch:x86_64": ["-Wl,--pack-dyn-relocs=none"], + features = select({ + "//build/bazel/platforms/arch:x86_64": [ + "disable_pack_relocations", + "-no_undefined_symbols", + ], "//conditions:default": [], }), srcs = ["b.cpp"], )`, `cc_library( name = "c", - linkopts = select({ - "//build/bazel/platforms/os:darwin": ["-Wl,--pack-dyn-relocs=none"], + features = select({ + "//build/bazel/platforms/os:darwin": [ + "disable_pack_relocations", + "-no_undefined_symbols", + ], "//conditions:default": [], }), srcs = ["c.cpp"], diff --git a/cc/bp2build.go b/cc/bp2build.go index ebba9d195..f3c999d7c 100644 --- a/cc/bp2build.go +++ b/cc/bp2build.go @@ -373,15 +373,7 @@ type linkerAttributes struct { stripKeepSymbolsList bazel.StringListAttribute stripAll bazel.BoolAttribute stripNone bazel.BoolAttribute -} - -// FIXME(b/187655838): Use the existing linkerFlags() function instead of duplicating logic here -func getBp2BuildLinkerFlags(linkerProperties *BaseLinkerProperties) []string { - flags := linkerProperties.Ldflags - if !BoolDefault(linkerProperties.Pack_relocations, true) { - flags = append(flags, "-Wl,--pack-dyn-relocs=none") - } - return flags + features bazel.StringListAttribute } // bp2BuildParseLinkerProps parses the linker properties of a module, including @@ -408,6 +400,8 @@ func bp2BuildParseLinkerProps(ctx android.TopDownMutatorContext, module *Module) var stripAll bazel.BoolAttribute var stripNone bazel.BoolAttribute + var features bazel.StringListAttribute + for axis, configToProps := range module.GetArchVariantProperties(ctx, &StripProperties{}) { for config, props := range configToProps { if stripProperties, ok := props.(*StripProperties); ok { @@ -426,6 +420,7 @@ func bp2BuildParseLinkerProps(ctx android.TopDownMutatorContext, module *Module) for axis, configToProps := range module.GetArchVariantProperties(ctx, &BaseLinkerProperties{}) { for config, props := range configToProps { if baseLinkerProps, ok := props.(*BaseLinkerProperties); ok { + var axisFeatures []string // Excludes to parallel Soong: // https://cs.android.com/android/platform/superproject/+/master:build/soong/cc/linker.go;l=247-249;drc=088b53577dde6e40085ffd737a1ae96ad82fc4b0 @@ -457,7 +452,15 @@ func bp2BuildParseLinkerProps(ctx android.TopDownMutatorContext, module *Module) headerDeps.SetSelectValue(axis, config, hDeps.export) implementationHeaderDeps.SetSelectValue(axis, config, hDeps.implementation) - linkopts.SetSelectValue(axis, config, getBp2BuildLinkerFlags(baseLinkerProps)) + linkopts.SetSelectValue(axis, config, baseLinkerProps.Ldflags) + if !BoolDefault(baseLinkerProps.Pack_relocations, packRelocationsDefault) { + axisFeatures = append(axisFeatures, "disable_pack_relocations") + } + + if Bool(baseLinkerProps.Allow_undefined_symbols) { + axisFeatures = append(axisFeatures, "-no_undefined_symbols") + } + if baseLinkerProps.Version_script != nil { versionScript.SetSelectValue(axis, config, android.BazelLabelForModuleSrcSingle(ctx, *baseLinkerProps.Version_script)) } @@ -471,6 +474,10 @@ func bp2BuildParseLinkerProps(ctx android.TopDownMutatorContext, module *Module) disallowedArchVariantCrt = true } } + + if axisFeatures != nil { + features.SetSelectValue(axis, config, axisFeatures) + } } } } @@ -559,6 +566,8 @@ func bp2BuildParseLinkerProps(ctx android.TopDownMutatorContext, module *Module) stripKeepSymbolsList: stripKeepSymbolsList, stripAll: stripAll, stripNone: stripNone, + + features: features, } } diff --git a/cc/library.go b/cc/library.go index 58e0e2123..0cffd5667 100644 --- a/cc/library.go +++ b/cc/library.go @@ -259,6 +259,8 @@ type bazelCcLibraryAttributes struct { Static staticOrSharedAttributes Strip stripAttributes + + Features bazel.StringListAttribute } type stripAttributes struct { @@ -340,6 +342,8 @@ func CcLibraryBp2Build(ctx android.TopDownMutatorContext) { Shared: sharedAttrs, Static: staticAttrs, + + Features: linkerAttrs.features, } props := bazel.BazelTargetModuleProperties{ @@ -2407,6 +2411,8 @@ func ccSharedOrStaticBp2BuildMutatorInternal(ctx android.TopDownMutatorContext, Cppflags: compilerAttrs.cppFlags, Conlyflags: compilerAttrs.conlyFlags, Asflags: asFlags, + + Features: linkerAttrs.features, } } else { attrs = &bazelCcLibrarySharedAttributes{ @@ -2435,6 +2441,8 @@ func ccSharedOrStaticBp2BuildMutatorInternal(ctx android.TopDownMutatorContext, All: linkerAttrs.stripAll, None: linkerAttrs.stripNone, }, + + Features: linkerAttrs.features, } } @@ -2464,6 +2472,8 @@ type bazelCcLibraryStaticAttributes struct { Cppflags bazel.StringListAttribute Conlyflags bazel.StringListAttribute Asflags bazel.StringListAttribute + + Features bazel.StringListAttribute } func CcLibraryStaticBp2Build(ctx android.TopDownMutatorContext) { @@ -2492,6 +2502,8 @@ type bazelCcLibrarySharedAttributes struct { Cppflags bazel.StringListAttribute Conlyflags bazel.StringListAttribute Asflags bazel.StringListAttribute + + Features bazel.StringListAttribute } func CcLibrarySharedBp2Build(ctx android.TopDownMutatorContext) { diff --git a/cc/linker.go b/cc/linker.go index 20e377ceb..aaaca7ad4 100644 --- a/cc/linker.go +++ b/cc/linker.go @@ -27,6 +27,10 @@ import ( // This file contains the basic functionality for linking against static libraries and shared // libraries. Final linking into libraries or executables is handled in library.go, binary.go, etc. +const ( + packRelocationsDefault = true +) + type BaseLinkerProperties struct { // list of modules whose object files should be linked into this module // in their entirety. For static library modules, all of the .o files from the intermediate @@ -471,7 +475,7 @@ func (linker *baseLinker) linkerFlags(ctx ModuleContext, flags Flags) Flags { if linker.useClangLld(ctx) { flags.Global.LdFlags = append(flags.Global.LdFlags, fmt.Sprintf("${config.%sGlobalLldflags}", hod)) - if !BoolDefault(linker.Properties.Pack_relocations, true) { + if !BoolDefault(linker.Properties.Pack_relocations, packRelocationsDefault) { flags.Global.LdFlags = append(flags.Global.LdFlags, "-Wl,--pack-dyn-relocs=none") } else if ctx.Device() { // SHT_RELR relocations are only supported at API level >= 30.