From b45c844ce78b3f636996ead7e88aee9c220e5a92 Mon Sep 17 00:00:00 2001 From: Sam Delmerico Date: Thu, 13 Jul 2023 10:52:57 -0400 Subject: [PATCH 1/3] add rust_toolchain_rustc_prebuilt module type Sandboxing the rust rules requires having explicit inputs for the rust toolchain. This module type makes the rustc prebuilt and its dependencies explicit as a module that can be depended on by all rust rules. Bug: 286077158 Test: rust sandboxing topic + go test Change-Id: If2b80b32e329e6a6ee11ba824de868cf04714553 --- android/prebuilt_build_tool.go | 6 +---- cc/testing.go | 1 + rust/toolchain_library.go | 48 ++++++++++++++++++++++++++++++++++ 3 files changed, 50 insertions(+), 5 deletions(-) diff --git a/android/prebuilt_build_tool.go b/android/prebuilt_build_tool.go index aeae20f48..17b323067 100644 --- a/android/prebuilt_build_tool.go +++ b/android/prebuilt_build_tool.go @@ -17,7 +17,7 @@ package android import "path/filepath" func init() { - RegisterModuleType("prebuilt_build_tool", prebuiltBuildToolFactory) + RegisterModuleType("prebuilt_build_tool", NewPrebuiltBuildTool) } type prebuiltBuildToolProperties struct { @@ -101,10 +101,6 @@ var _ HostToolProvider = &prebuiltBuildTool{} // prebuilt_build_tool is to declare prebuilts to be used during the build, particularly for use // in genrules with the "tools" property. -func prebuiltBuildToolFactory() Module { - return NewPrebuiltBuildTool() -} - func NewPrebuiltBuildTool() Module { module := &prebuiltBuildTool{} module.AddProperties(&module.properties) diff --git a/cc/testing.go b/cc/testing.go index d1632aaa6..dbdee9edf 100644 --- a/cc/testing.go +++ b/cc/testing.go @@ -35,6 +35,7 @@ func RegisterRequiredBuildComponentsForTest(ctx android.RegistrationContext) { multitree.RegisterApiImportsModule(ctx) + ctx.RegisterModuleType("prebuilt_build_tool", android.NewPrebuiltBuildTool) ctx.RegisterModuleType("cc_benchmark", BenchmarkFactory) ctx.RegisterModuleType("cc_object", ObjectFactory) ctx.RegisterModuleType("cc_genrule", GenRuleFactory) diff --git a/rust/toolchain_library.go b/rust/toolchain_library.go index 326d52924..801c0c271 100644 --- a/rust/toolchain_library.go +++ b/rust/toolchain_library.go @@ -21,6 +21,8 @@ import ( "android/soong/android" "android/soong/rust/config" + + "github.com/google/blueprint/proptools" ) // This module is used to compile the rust toolchain libraries @@ -33,6 +35,8 @@ func init() { rustToolchainLibraryRlibFactory) android.RegisterModuleType("rust_toolchain_library_dylib", rustToolchainLibraryDylibFactory) + android.RegisterModuleType("rust_toolchain_rustc_prebuilt", + rustToolchainRustcPrebuiltFactory) } type toolchainLibraryProperties struct { @@ -101,3 +105,47 @@ func rustSetToolchainSource(ctx android.LoadHookContext) { func GetRustPrebuiltVersion(ctx android.LoadHookContext) string { return ctx.AConfig().GetenvWithDefault("RUST_PREBUILTS_VERSION", config.RustDefaultVersion) } + +type toolchainRustcPrebuiltProperties struct { + // path to rustc prebuilt, relative to the top of the toolchain source + Toolchain_prebuilt_src *string + // path to deps, relative to the top of the toolchain source + Toolchain_deps []string + // path to deps, relative to module directory + Deps []string +} + +func rustToolchainRustcPrebuiltFactory() android.Module { + module := android.NewPrebuiltBuildTool() + module.AddProperties(&toolchainRustcPrebuiltProperties{}) + android.AddLoadHook(module, func(ctx android.LoadHookContext) { + var toolchainProps *toolchainRustcPrebuiltProperties + for _, p := range ctx.Module().GetProperties() { + toolchainProperties, ok := p.(*toolchainRustcPrebuiltProperties) + if ok { + toolchainProps = toolchainProperties + } + } + + if toolchainProps.Toolchain_prebuilt_src == nil { + ctx.PropertyErrorf("toolchain_prebuilt_src", "must set path to rustc prebuilt") + } + + prefix := "linux-x86/" + GetRustPrebuiltVersion(ctx) + deps := make([]string, 0, len(toolchainProps.Toolchain_deps)+len(toolchainProps.Deps)) + for _, d := range toolchainProps.Toolchain_deps { + deps = append(deps, path.Join(prefix, d)) + } + deps = append(deps, toolchainProps.Deps...) + + props := struct { + Src *string + Deps []string + }{ + Src: proptools.StringPtr(path.Join(prefix, *toolchainProps.Toolchain_prebuilt_src)), + Deps: deps, + } + ctx.AppendProperties(&props) + }) + return module +} From 553edff9dda3369612b20c75fa0640b5db17a30d Mon Sep 17 00:00:00 2001 From: Sam Delmerico Date: Thu, 13 Jul 2023 11:15:37 -0400 Subject: [PATCH 2/3] add crate_root property to rust modules The crate_root property will be used to specify the entry point for a rustc compilation. This will allow the srcs property to be used to collect all src inputs to rustc rather than just the entry point. Bug: 286077158 Test: m libnum_traits Change-Id: I1a167182305dcc11cc927d562ceed622153111d3 --- rust/binary.go | 9 +++++++-- rust/compiler.go | 11 +++++++++++ rust/library.go | 18 ++++++++++-------- rust/toolchain_library.go | 19 +++++++++++++------ 4 files changed, 41 insertions(+), 16 deletions(-) diff --git a/rust/binary.go b/rust/binary.go index 1e24bebab..2c9f64dd9 100644 --- a/rust/binary.go +++ b/rust/binary.go @@ -137,9 +137,14 @@ func (binary *binaryDecorator) preferRlib() bool { func (binary *binaryDecorator) compile(ctx ModuleContext, flags Flags, deps PathDeps) buildOutput { fileName := binary.getStem(ctx) + ctx.toolchain().ExecutableSuffix() - srcPath, _ := srcPathFromModuleSrcs(ctx, binary.baseCompiler.Properties.Srcs) outputFile := android.PathForModuleOut(ctx, fileName) ret := buildOutput{outputFile: outputFile} + var crateRootPath android.Path + if binary.baseCompiler.Properties.Crate_root == nil { + crateRootPath, _ = srcPathFromModuleSrcs(ctx, binary.baseCompiler.Properties.Srcs) + } else { + crateRootPath = android.PathForModuleSrc(ctx, *binary.baseCompiler.Properties.Crate_root) + } flags.RustFlags = append(flags.RustFlags, deps.depFlags...) flags.LinkFlags = append(flags.LinkFlags, deps.depLinkFlags...) @@ -154,7 +159,7 @@ func (binary *binaryDecorator) compile(ctx ModuleContext, flags Flags, deps Path } binary.baseCompiler.unstrippedOutputFile = outputFile - ret.kytheFile = TransformSrcToBinary(ctx, srcPath, deps, flags, outputFile).kytheFile + ret.kytheFile = TransformSrcToBinary(ctx, crateRootPath, deps, flags, outputFile).kytheFile return ret } diff --git a/rust/compiler.go b/rust/compiler.go index e6a7a9356..d6c52e8d4 100644 --- a/rust/compiler.go +++ b/rust/compiler.go @@ -73,6 +73,15 @@ type BaseCompilerProperties struct { // If no source file is defined, a single generated source module can be defined to be used as the main source. Srcs []string `android:"path,arch_variant"` + // Entry point that is passed to rustc to begin the compilation. E.g. main.rs or lib.rs. + // When this property is set, + // * sandboxing is enabled for this module, and + // * the srcs attribute is interpreted as a list of all source files potentially + // used in compilation, including the entrypoint, and + // * compile_data can be used to add additional files used in compilation that + // not directly used as source files. + Crate_root *string `android:"path,arch_variant"` + // name of the lint set that should be used to validate this module. // // Possible values are "default" (for using a sensible set of lints @@ -511,6 +520,8 @@ func srcPathFromModuleSrcs(ctx ModuleContext, srcs []string) (android.Path, andr ctx.PropertyErrorf("srcs", "only a single generated source module can be defined without a main source file.") } + // TODO: b/297264540 - once all modules are sandboxed, we need to select the proper + // entry point file from Srcs rather than taking the first one paths := android.PathsForModuleSrc(ctx, srcs) return paths[srcIndex], paths[1:] } diff --git a/rust/library.go b/rust/library.go index 3f031c10c..7432a12be 100644 --- a/rust/library.go +++ b/rust/library.go @@ -489,7 +489,7 @@ func (library *libraryDecorator) compile(ctx ModuleContext, flags Flags, deps Pa var outputFile android.ModuleOutPath var ret buildOutput var fileName string - srcPath := library.srcPath(ctx, deps) + crateRootPath := library.crateRootPath(ctx, deps) if library.sourceProvider != nil { deps.srcProviderFiles = append(deps.srcProviderFiles, library.sourceProvider.Srcs()...) @@ -536,13 +536,13 @@ func (library *libraryDecorator) compile(ctx ModuleContext, flags Flags, deps Pa // Call the appropriate builder for this library type if library.rlib() { - ret.kytheFile = TransformSrctoRlib(ctx, srcPath, deps, flags, outputFile).kytheFile + ret.kytheFile = TransformSrctoRlib(ctx, crateRootPath, deps, flags, outputFile).kytheFile } else if library.dylib() { - ret.kytheFile = TransformSrctoDylib(ctx, srcPath, deps, flags, outputFile).kytheFile + ret.kytheFile = TransformSrctoDylib(ctx, crateRootPath, deps, flags, outputFile).kytheFile } else if library.static() { - ret.kytheFile = TransformSrctoStatic(ctx, srcPath, deps, flags, outputFile).kytheFile + ret.kytheFile = TransformSrctoStatic(ctx, crateRootPath, deps, flags, outputFile).kytheFile } else if library.shared() { - ret.kytheFile = TransformSrctoShared(ctx, srcPath, deps, flags, outputFile).kytheFile + ret.kytheFile = TransformSrctoShared(ctx, crateRootPath, deps, flags, outputFile).kytheFile } if library.rlib() || library.dylib() { @@ -585,13 +585,15 @@ func (library *libraryDecorator) compile(ctx ModuleContext, flags Flags, deps Pa return ret } -func (library *libraryDecorator) srcPath(ctx ModuleContext, _ PathDeps) android.Path { +func (library *libraryDecorator) crateRootPath(ctx ModuleContext, _ PathDeps) android.Path { if library.sourceProvider != nil { // Assume the first source from the source provider is the library entry point. return library.sourceProvider.Srcs()[0] - } else { + } else if library.baseCompiler.Properties.Crate_root == nil { path, _ := srcPathFromModuleSrcs(ctx, library.baseCompiler.Properties.Srcs) return path + } else { + return android.PathForModuleSrc(ctx, *library.baseCompiler.Properties.Crate_root) } } @@ -606,7 +608,7 @@ func (library *libraryDecorator) rustdoc(ctx ModuleContext, flags Flags, return android.OptionalPath{} } - return android.OptionalPathForPath(Rustdoc(ctx, library.srcPath(ctx, deps), + return android.OptionalPathForPath(Rustdoc(ctx, library.crateRootPath(ctx, deps), deps, flags)) } diff --git a/rust/toolchain_library.go b/rust/toolchain_library.go index 801c0c271..e118f92be 100644 --- a/rust/toolchain_library.go +++ b/rust/toolchain_library.go @@ -40,8 +40,10 @@ func init() { } type toolchainLibraryProperties struct { - // path to the toolchain source, relative to the top of the toolchain source - Toolchain_src *string `android:"arch_variant"` + // path to the toolchain crate root, relative to the top of the toolchain source + Toolchain_crate_root *string `android:"arch_variant"` + // path to the rest of the toolchain srcs, relative to the top of the toolchain source + Toolchain_srcs []string `android:"arch_variant"` } type toolchainLibraryDecorator struct { @@ -87,15 +89,20 @@ func initToolchainLibrary(module *Module, library *libraryDecorator) android.Mod func rustSetToolchainSource(ctx android.LoadHookContext) { if toolchainLib, ok := ctx.Module().(*Module).compiler.(*toolchainLibraryDecorator); ok { prefix := "linux-x86/" + GetRustPrebuiltVersion(ctx) - newSrcs := []string{path.Join(prefix, android.String(toolchainLib.Properties.Toolchain_src))} + versionedCrateRoot := path.Join(prefix, android.String(toolchainLib.Properties.Toolchain_crate_root)) + versionedSrcs := make([]string, len(toolchainLib.Properties.Toolchain_srcs)) + for i, src := range toolchainLib.Properties.Toolchain_srcs { + versionedSrcs[i] = path.Join(prefix, src) + } type props struct { - Srcs []string + Crate_root *string + Srcs []string } p := &props{} - p.Srcs = newSrcs + p.Crate_root = &versionedCrateRoot + p.Srcs = versionedSrcs ctx.AppendProperties(p) - } else { ctx.ModuleErrorf("Called rustSetToolchainSource on a non-Rust Module.") } From 1f9bb26656b10a367e862270cdcdfba8214e522a Mon Sep 17 00:00:00 2001 From: Sam Delmerico Date: Fri, 26 May 2023 16:11:43 -0400 Subject: [PATCH 3/3] allow Ninja variables in RuleBuilder API The RuleBuilder API would not expand Ninja variables because the variables would be written verbatim to the sandbox manifest file. This commit allows a rule to specify that the manifest file should be written in an un-escaped format so that Ninja variables are expanded before writing the manifest file. Bug: 286077158 Test: rust sandboxing topic + go test Change-Id: I1915431f6e24d04d343dacc213c9079674ec8251 --- android/rule_builder.go | 41 +++++++++++++++-- android/rule_builder_test.go | 86 +++++++++++++++++++++++++++++++----- cmd/sbox/sbox.go | 3 ++ 3 files changed, 115 insertions(+), 15 deletions(-) diff --git a/android/rule_builder.go b/android/rule_builder.go index 777c1cfc3..97582db2c 100644 --- a/android/rule_builder.go +++ b/android/rule_builder.go @@ -474,13 +474,23 @@ func (r *RuleBuilder) depFileMergerCmd(depFiles WritablePaths) *RuleBuilderComma Inputs(depFiles.Paths()) } +// BuildWithNinjaVars adds the built command line to the build graph, with dependencies on Inputs and Tools, and output files for +// Outputs. This function will not escape Ninja variables, so it may be used to write sandbox manifests using Ninja variables. +func (r *RuleBuilder) BuildWithUnescapedNinjaVars(name string, desc string) { + r.build(name, desc, false) +} + // Build adds the built command line to the build graph, with dependencies on Inputs and Tools, and output files for // Outputs. func (r *RuleBuilder) Build(name string, desc string) { + r.build(name, desc, true) +} + +func (r *RuleBuilder) build(name string, desc string, ninjaEscapeCommandString bool) { name = ninjaNameEscape(name) if len(r.missingDeps) > 0 { - r.ctx.Build(pctx, BuildParams{ + r.ctx.Build(r.pctx, BuildParams{ Rule: ErrorRule, Outputs: r.Outputs(), Description: desc, @@ -619,12 +629,35 @@ func (r *RuleBuilder) Build(name string, desc string) { name, r.sboxManifestPath.String(), r.outDir.String()) } - // Create a rule to write the manifest as a the textproto. + // Create a rule to write the manifest as textproto. pbText, err := prototext.Marshal(&manifest) if err != nil { ReportPathErrorf(r.ctx, "sbox manifest failed to marshal: %q", err) } - WriteFileRule(r.ctx, r.sboxManifestPath, string(pbText)) + if ninjaEscapeCommandString { + WriteFileRule(r.ctx, r.sboxManifestPath, string(pbText)) + } else { + // We need to have a rule to write files that is + // defined on the RuleBuilder's pctx in order to + // write Ninja variables in the string. + // The WriteFileRule function above rule can only write + // raw strings because it is defined on the android + // package's pctx, and it can't access variables defined + // in another context. + r.ctx.Build(r.pctx, BuildParams{ + Rule: r.ctx.Rule(r.pctx, "unescapedWriteFile", blueprint.RuleParams{ + Command: `rm -rf ${out} && cat ${out}.rsp > ${out}`, + Rspfile: "${out}.rsp", + RspfileContent: "${content}", + Description: "write file", + }, "content"), + Output: r.sboxManifestPath, + Description: "write sbox manifest " + r.sboxManifestPath.Base(), + Args: map[string]string{ + "content": string(pbText), + }, + }) + } // Generate a new string to use as the command line of the sbox rule. This uses // a RuleBuilderCommand as a convenience method of building the command line, then @@ -724,7 +757,7 @@ func (r *RuleBuilder) Build(name string, desc string) { } r.ctx.Build(r.pctx, BuildParams{ - Rule: r.ctx.Rule(pctx, name, blueprint.RuleParams{ + Rule: r.ctx.Rule(r.pctx, name, blueprint.RuleParams{ Command: proptools.NinjaEscape(commandString), CommandDeps: proptools.NinjaEscapeList(tools.Strings()), Restat: r.restat, diff --git a/android/rule_builder_test.go b/android/rule_builder_test.go index 86647eb22..a6b3a27a1 100644 --- a/android/rule_builder_test.go +++ b/android/rule_builder_test.go @@ -28,6 +28,17 @@ import ( "android/soong/shared" ) +var ( + pctx_ruleBuilderTest = NewPackageContext("android/soong/rule_builder") + pctx_ruleBuilderTestSubContext = NewPackageContext("android/soong/rule_builder/config") +) + +func init() { + pctx_ruleBuilderTest.Import("android/soong/rule_builder/config") + pctx_ruleBuilderTest.StaticVariable("cmdFlags", "${config.ConfigFlags}") + pctx_ruleBuilderTestSubContext.StaticVariable("ConfigFlags", "--some-clang-flag") +} + func builderContext() BuilderContext { return BuilderContextForTesting(TestConfig("out", nil, "", map[string][]byte{ "ld": nil, @@ -496,11 +507,13 @@ func testRuleBuilderFactory() Module { type testRuleBuilderModule struct { ModuleBase properties struct { - Srcs []string + Srcs []string + Flags []string - Restat bool - Sbox bool - Sbox_inputs bool + Restat bool + Sbox bool + Sbox_inputs bool + Unescape_ninja_vars bool } } @@ -518,8 +531,9 @@ func (t *testRuleBuilderModule) GenerateAndroidBuildActions(ctx ModuleContext) { rspFileContents2 := PathsForSource(ctx, []string{"rsp_in2"}) manifestPath := PathForModuleOut(ctx, "sbox.textproto") - testRuleBuilder_Build(ctx, in, implicit, orderOnly, validation, out, outDep, outDir, - manifestPath, t.properties.Restat, t.properties.Sbox, t.properties.Sbox_inputs, + testRuleBuilder_Build(ctx, in, implicit, orderOnly, validation, t.properties.Flags, + out, outDep, outDir, + manifestPath, t.properties.Restat, t.properties.Sbox, t.properties.Sbox_inputs, t.properties.Unescape_ninja_vars, rspFile, rspFileContents, rspFile2, rspFileContents2) } @@ -543,17 +557,18 @@ func (t *testRuleBuilderSingleton) GenerateBuildActions(ctx SingletonContext) { rspFileContents2 := PathsForSource(ctx, []string{"rsp_in2"}) manifestPath := PathForOutput(ctx, "singleton/sbox.textproto") - testRuleBuilder_Build(ctx, in, implicit, orderOnly, validation, out, outDep, outDir, - manifestPath, true, false, false, + testRuleBuilder_Build(ctx, in, implicit, orderOnly, validation, nil, out, outDep, outDir, + manifestPath, true, false, false, false, rspFile, rspFileContents, rspFile2, rspFileContents2) } func testRuleBuilder_Build(ctx BuilderContext, in Paths, implicit, orderOnly, validation Path, + flags []string, out, outDep, outDir, manifestPath WritablePath, - restat, sbox, sboxInputs bool, + restat, sbox, sboxInputs, unescapeNinjaVars bool, rspFile WritablePath, rspFileContents Paths, rspFile2 WritablePath, rspFileContents2 Paths) { - rule := NewRuleBuilder(pctx, ctx) + rule := NewRuleBuilder(pctx_ruleBuilderTest, ctx) if sbox { rule.Sbox(outDir, manifestPath) @@ -564,6 +579,7 @@ func testRuleBuilder_Build(ctx BuilderContext, in Paths, implicit, orderOnly, va rule.Command(). Tool(PathForSource(ctx, "cp")). + Flags(flags). Inputs(in). Implicit(implicit). OrderOnly(orderOnly). @@ -577,7 +593,11 @@ func testRuleBuilder_Build(ctx BuilderContext, in Paths, implicit, orderOnly, va rule.Restat() } - rule.Build("rule", "desc") + if unescapeNinjaVars { + rule.BuildWithUnescapedNinjaVars("rule", "desc") + } else { + rule.Build("rule", "desc") + } } var prepareForRuleBuilderTest = FixtureRegisterWithContext(func(ctx RegistrationContext) { @@ -792,3 +812,47 @@ func TestRuleBuilderHashInputs(t *testing.T) { }) } } + +func TestRuleBuilderWithNinjaVarEscaping(t *testing.T) { + bp := ` + rule_builder_test { + name: "foo_sbox_escaped_ninja", + flags: ["${cmdFlags}"], + sbox: true, + sbox_inputs: true, + } + rule_builder_test { + name: "foo_sbox", + flags: ["${cmdFlags}"], + sbox: true, + sbox_inputs: true, + unescape_ninja_vars: true, + } + ` + result := GroupFixturePreparers( + prepareForRuleBuilderTest, + FixtureWithRootAndroidBp(bp), + ).RunTest(t) + + escapedNinjaMod := result.ModuleForTests("foo_sbox_escaped_ninja", "").Rule("writeFile") + AssertStringDoesContain( + t, + "", + escapedNinjaMod.BuildParams.Args["content"], + "$${cmdFlags}", + ) + + unescapedNinjaMod := result.ModuleForTests("foo_sbox", "").Rule("unescapedWriteFile") + AssertStringDoesContain( + t, + "", + unescapedNinjaMod.BuildParams.Args["content"], + "${cmdFlags}", + ) + AssertStringDoesNotContain( + t, + "", + unescapedNinjaMod.BuildParams.Args["content"], + "$${cmdFlags}", + ) +} diff --git a/cmd/sbox/sbox.go b/cmd/sbox/sbox.go index fc56dd526..3364f503f 100644 --- a/cmd/sbox/sbox.go +++ b/cmd/sbox/sbox.go @@ -119,6 +119,9 @@ func run() error { } manifest, err := readManifest(manifestFile) + if err != nil { + return err + } if len(manifest.Commands) == 0 { return fmt.Errorf("at least one commands entry is required in %q", manifestFile)