From 1be00d4e480d93c5a3fbd1ec1bf984dade8cfe82 Mon Sep 17 00:00:00 2001 From: Alix Date: Mon, 16 May 2022 22:56:04 +0000 Subject: [PATCH] filter unknown clang cflags in bp2build filtering out no longer useful flags in bp2build conversion Test: cc_library_shared_conversion_test Bug: 231995978 Change-Id: I5172f6d07a8928291bbc11bd8802678b33cc5b1f --- bp2build/cc_library_shared_conversion_test.go | 55 +++++++++++++++++++ cc/bp2build.go | 42 +++++++++++--- 2 files changed, 88 insertions(+), 9 deletions(-) diff --git a/bp2build/cc_library_shared_conversion_test.go b/bp2build/cc_library_shared_conversion_test.go index be096168c..74729e4e1 100644 --- a/bp2build/cc_library_shared_conversion_test.go +++ b/bp2build/cc_library_shared_conversion_test.go @@ -569,3 +569,58 @@ func TestCcLibrarySharedConvertLex(t *testing.T) { }, }) } + +func TestCcLibrarySharedClangUnknownFlags(t *testing.T) { + runCcLibrarySharedTestCase(t, bp2buildTestCase{ + blueprint: soongCcProtoPreamble + `cc_library_shared { + name: "foo", + conlyflags: ["-a", "-finline-functions"], + cflags: ["-b","-finline-functions"], + cppflags: ["-c", "-finline-functions"], + ldflags: ["-d","-finline-functions", "-e"], + include_build_directory: false, +}`, + expectedBazelTargets: []string{ + makeBazelTarget("cc_library_shared", "foo", attrNameToString{ + "conlyflags": `["-a"]`, + "copts": `["-b"]`, + "cppflags": `["-c"]`, + "linkopts": `[ + "-d", + "-e", + ]`, + }), + }, + }) +} + +func TestCCLibraryFlagSpaceSplitting(t *testing.T) { + runCcLibrarySharedTestCase(t, bp2buildTestCase{ + blueprint: soongCcProtoPreamble + `cc_library_shared { + name: "foo", + conlyflags: [ "-include header.h"], + cflags: ["-include header.h"], + cppflags: ["-include header.h"], + version_script: "version_script", + include_build_directory: false, +}`, + expectedBazelTargets: []string{ + makeBazelTarget("cc_library_shared", "foo", attrNameToString{ + "additional_linker_inputs": `["version_script"]`, + "conlyflags": `[ + "-include", + "header.h", + ]`, + "copts": `[ + "-include", + "header.h", + ]`, + "cppflags": `[ + "-include", + "header.h", + ]`, + "linkopts": `["-Wl,--version-script,$(location version_script)"]`, + }), + }, + }) +} diff --git a/cc/bp2build.go b/cc/bp2build.go index fa30d096b..6cd67330f 100644 --- a/cc/bp2build.go +++ b/cc/bp2build.go @@ -20,6 +20,7 @@ import ( "android/soong/android" "android/soong/bazel" + "android/soong/cc/config" "github.com/google/blueprint" @@ -156,7 +157,7 @@ func bp2buildParseStaticOrSharedProps(ctx android.BazelConversionPathContext, mo attrs := staticOrSharedAttributes{} setAttrs := func(axis bazel.ConfigurationAxis, config string, props StaticOrSharedProperties) { - attrs.Copts.SetSelectValue(axis, config, parseCommandLineFlags(props.Cflags, filterOutStdFlag)) + attrs.Copts.SetSelectValue(axis, config, parseCommandLineFlags(props.Cflags, true, filterOutStdFlag)) attrs.Srcs.SetSelectValue(axis, config, android.BazelLabelForModuleSrc(ctx, props.Srcs)) attrs.System_dynamic_deps.SetSelectValue(axis, config, bazelLabelForSharedDeps(ctx, props.System_shared_libs)) @@ -326,16 +327,39 @@ func filterOutStdFlag(flag string) bool { return strings.HasPrefix(flag, "-std=") } -func parseCommandLineFlags(soongFlags []string, filterOut filterOutFn) []string { +func filterOutClangUnknownCflags(flag string) bool { + for _, f := range config.ClangUnknownCflags { + if f == flag { + return true + } + } + return false +} + +func parseCommandLineFlags(soongFlags []string, noCoptsTokenization bool, filterOut ...filterOutFn) []string { var result []string for _, flag := range soongFlags { - if filterOut != nil && filterOut(flag) { + skipFlag := false + for _, filter := range filterOut { + if filter != nil && filter(flag) { + skipFlag = true + } + } + if skipFlag { continue } // Soong's cflags can contain spaces, like `-include header.h`. For // Bazel's copts, split them up to be compatible with the // no_copts_tokenization feature. - result = append(result, strings.Split(flag, " ")...) + if noCoptsTokenization { + result = append(result, strings.Split(flag, " ")...) + } else { + // Soong's Version Script and Dynamic List Properties are added as flags + // to Bazel's linkopts using "($location label)" syntax. + // Splitting on spaces would separate this into two different flags + // "($ location" and "label)" + result = append(result, flag) + } } return result } @@ -362,10 +386,10 @@ func (ca *compilerAttributes) bp2buildForAxisAndConfig(ctx android.BazelConversi // overridden. In Bazel we always allow overriding, via flags; however, this can cause // incompatibilities, so we remove "-std=" flags from Cflag properties while leaving it in other // cases. - ca.copts.SetSelectValue(axis, config, parseCommandLineFlags(props.Cflags, filterOutStdFlag)) - ca.asFlags.SetSelectValue(axis, config, parseCommandLineFlags(props.Asflags, nil)) - ca.conlyFlags.SetSelectValue(axis, config, parseCommandLineFlags(props.Conlyflags, nil)) - ca.cppFlags.SetSelectValue(axis, config, parseCommandLineFlags(props.Cppflags, nil)) + ca.copts.SetSelectValue(axis, config, parseCommandLineFlags(props.Cflags, true, filterOutStdFlag, filterOutClangUnknownCflags)) + ca.asFlags.SetSelectValue(axis, config, parseCommandLineFlags(props.Asflags, true, nil)) + ca.conlyFlags.SetSelectValue(axis, config, parseCommandLineFlags(props.Conlyflags, true, filterOutClangUnknownCflags)) + ca.cppFlags.SetSelectValue(axis, config, parseCommandLineFlags(props.Cppflags, true, filterOutClangUnknownCflags)) ca.rtti.SetSelectValue(axis, config, props.Rtti) } @@ -721,7 +745,7 @@ func (la *linkerAttributes) bp2buildForAxisAndConfig(ctx android.BazelConversion linkerFlags = append(linkerFlags, fmt.Sprintf("-Wl,--dynamic-list,$(location %s)", label.Label)) } - la.linkopts.SetSelectValue(axis, config, linkerFlags) + la.linkopts.SetSelectValue(axis, config, parseCommandLineFlags(linkerFlags, false, filterOutClangUnknownCflags)) la.useLibcrt.SetSelectValue(axis, config, props.libCrt()) if axis == bazel.NoConfigAxis {