From 81fec18a5ef3af8d7bbfa8535ce1fb497972b8c3 Mon Sep 17 00:00:00 2001 From: Liz Kammer Date: Fri, 9 Jun 2023 13:33:45 -0400 Subject: [PATCH] Add property data to gensrcs for additional inputs This enables sandboxing for inputs that are necessary but do not need to have a source file generated. Test: GENRULE_SANDBOXING=true m framework-javastream-protos \ framework-cppstream-protos Change-Id: Id5ca1dab5799c25fa96b564a7d2008c2e7b5382b --- genrule/allowlists.go | 2 -- genrule/genrule.go | 78 ++++++++++++++++++++++++++--------------- genrule/genrule_test.go | 24 +++++++++++++ 3 files changed, 74 insertions(+), 30 deletions(-) diff --git a/genrule/allowlists.go b/genrule/allowlists.go index 2954f8bc1..b0508cba5 100644 --- a/genrule/allowlists.go +++ b/genrule/allowlists.go @@ -47,7 +47,6 @@ var ( } SandboxingDenyModuleList = []string{ - "framework-javastream-protos", "RsBalls-rscript", "CtsRsBlasTestCases-rscript", "pvmfw_fdt_template_rs", @@ -88,7 +87,6 @@ var ( "vm-tests-tf-lib", "hidl_cpp_impl_test_gen-headers", "pandora_experimental-python-gen-src", - "framework-cppstream-protos", "Refocus-rscript", "RSTest_v11-rscript", "RSTest_v16-rscript", diff --git a/genrule/genrule.go b/genrule/genrule.go index 4992625e5..fbe2a5d98 100644 --- a/genrule/genrule.go +++ b/genrule/genrule.go @@ -203,12 +203,13 @@ var _ android.MixedBuildBuildable = (*Module)(nil) type taskFunc func(ctx android.ModuleContext, rawCommand string, srcFiles android.Paths) []generateTask type generateTask struct { - in android.Paths - out android.WritablePaths - depFile android.WritablePath - copyTo android.WritablePaths // For gensrcs to set on gensrcsMerge rule. - genDir android.WritablePath - extraTools android.Paths // dependencies on tools used by the generator + in android.Paths + out android.WritablePaths + depFile android.WritablePath + copyTo android.WritablePaths // For gensrcs to set on gensrcsMerge rule. + genDir android.WritablePath + extraTools android.Paths // dependencies on tools used by the generator + extraInputs map[string][]string cmd string // For gensrsc sharding. @@ -402,30 +403,35 @@ func (g *Module) generateCommonBuildActions(ctx android.ModuleContext) { addLocationLabel(toolFile, toolLocation{paths}) } - includeDirInPaths := ctx.DeviceConfig().BuildBrokenInputDir(g.Name()) - var srcFiles android.Paths - for _, in := range g.properties.Srcs { - paths, missingDeps := android.PathsAndMissingDepsRelativeToModuleSourceDir(android.SourceInput{ - Context: ctx, Paths: []string{in}, ExcludePaths: g.properties.Exclude_srcs, IncludeDirs: includeDirInPaths, - }) - if len(missingDeps) > 0 { - if !ctx.Config().AllowMissingDependencies() { - panic(fmt.Errorf("should never get here, the missing dependencies %q should have been reported in DepsMutator", - missingDeps)) - } + addLabelsForInputs := func(propName string, include, exclude []string) android.Paths { - // If AllowMissingDependencies is enabled, the build will not have stopped when - // the dependency was added on a missing SourceFileProducer module, which will result in nonsensical - // "cmd: label ":..." has no files" errors later. Add a placeholder file to the local label. - // The command that uses this placeholder file will never be executed because the rule will be - // replaced with an android.Error rule reporting the missing dependencies. - ctx.AddMissingDependencies(missingDeps) - addLocationLabel(in, errorLocation{"***missing srcs " + in + "***"}) - } else { - srcFiles = append(srcFiles, paths...) - addLocationLabel(in, inputLocation{paths}) + includeDirInPaths := ctx.DeviceConfig().BuildBrokenInputDir(g.Name()) + var srcFiles android.Paths + for _, in := range include { + paths, missingDeps := android.PathsAndMissingDepsRelativeToModuleSourceDir(android.SourceInput{ + Context: ctx, Paths: []string{in}, ExcludePaths: exclude, IncludeDirs: includeDirInPaths, + }) + if len(missingDeps) > 0 { + if !ctx.Config().AllowMissingDependencies() { + panic(fmt.Errorf("should never get here, the missing dependencies %q should have been reported in DepsMutator", + missingDeps)) + } + + // If AllowMissingDependencies is enabled, the build will not have stopped when + // the dependency was added on a missing SourceFileProducer module, which will result in nonsensical + // "cmd: label ":..." has no files" errors later. Add a placeholder file to the local label. + // The command that uses this placeholder file will never be executed because the rule will be + // replaced with an android.Error rule reporting the missing dependencies. + ctx.AddMissingDependencies(missingDeps) + addLocationLabel(in, errorLocation{"***missing " + propName + " " + in + "***"}) + } else { + srcFiles = append(srcFiles, paths...) + addLocationLabel(in, inputLocation{paths}) + } } + return srcFiles } + srcFiles := addLabelsForInputs("srcs", g.properties.Srcs, g.properties.Exclude_srcs) var copyFrom android.Paths var outputFiles android.WritablePaths @@ -437,12 +443,20 @@ func (g *Module) generateCommonBuildActions(ctx android.ModuleContext) { } // Generate tasks, either from genrule or gensrcs. - for _, task := range g.taskGenerator(ctx, cmd, srcFiles) { + for i, task := range g.taskGenerator(ctx, cmd, srcFiles) { if len(task.out) == 0 { ctx.ModuleErrorf("must have at least one output file") return } + var extraInputs android.Paths + // Only handle extra inputs once as these currently are the same across all tasks + if i == 0 { + for name, values := range task.extraInputs { + extraInputs = append(extraInputs, addLabelsForInputs(name, values, []string{})...) + } + } + // Pick a unique path outside the task.genDir for the sbox manifest textproto, // a unique rule name, and the user-visible description. manifestName := "genrule.sbox.textproto" @@ -551,6 +565,8 @@ func (g *Module) generateCommonBuildActions(ctx android.ModuleContext) { g.rawCommands = append(g.rawCommands, rawCommand) cmd.Text(rawCommand) + cmd.Implicits(srcFiles) // need to be able to reference other srcs + cmd.Implicits(extraInputs) cmd.ImplicitOutputs(task.out) cmd.Implicits(task.in) cmd.ImplicitTools(tools) @@ -820,6 +836,9 @@ func NewGenSrcs() *Module { shard: i, shards: len(shards), extraTools: extraTools, + extraInputs: map[string][]string{ + "data": properties.Data, + }, }) } @@ -844,6 +863,9 @@ type genSrcsProperties struct { // maximum number of files that will be passed on a single command line. Shard_size *int64 + + // Additional files needed for build that are not tooling related. + Data []string `android:"path"` } type bazelGensrcsAttributes struct { diff --git a/genrule/genrule_test.go b/genrule/genrule_test.go index 370fe3d46..7c17db1be 100644 --- a/genrule/genrule_test.go +++ b/genrule/genrule_test.go @@ -604,6 +604,30 @@ func TestGenSrcs(t *testing.T) { "out/soong/.intermediates/gen/gen/gensrcs/in3.h", }, }, + { + name: "data", + prop: ` + tools: ["tool"], + srcs: ["in1.txt", "in2.txt", "in3.txt"], + cmd: "$(location) $(in) --extra_input=$(location baz.txt) > $(out)", + data: ["baz.txt"], + shard_size: 2, + `, + cmds: []string{ + "bash -c '__SBOX_SANDBOX_DIR__/tools/out/bin/tool in1.txt --extra_input=baz.txt > __SBOX_SANDBOX_DIR__/out/in1.h' && bash -c '__SBOX_SANDBOX_DIR__/tools/out/bin/tool in2.txt --extra_input=baz.txt > __SBOX_SANDBOX_DIR__/out/in2.h'", + "bash -c '__SBOX_SANDBOX_DIR__/tools/out/bin/tool in3.txt --extra_input=baz.txt > __SBOX_SANDBOX_DIR__/out/in3.h'", + }, + deps: []string{ + "out/soong/.intermediates/gen/gen/gensrcs/in1.h", + "out/soong/.intermediates/gen/gen/gensrcs/in2.h", + "out/soong/.intermediates/gen/gen/gensrcs/in3.h", + }, + files: []string{ + "out/soong/.intermediates/gen/gen/gensrcs/in1.h", + "out/soong/.intermediates/gen/gen/gensrcs/in2.h", + "out/soong/.intermediates/gen/gen/gensrcs/in3.h", + }, + }, } for _, test := range testcases {