layering_check doesn't work without sandboxing

[fmeum]

Description of the bug:

When compiling C++ targets with the layering_check feature, Bazel only stages the clang module maps of direct dependencies of a target as inputs, not all transitive module maps:

bazel/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationContext.java

Lines 578 to 582 in f5c8deb

	builder.addAll(directModuleMaps);
	builder.addTransitive(nonCodeInputs);
	if (cppModuleMap != null && propagateModuleMapAsActionInput) {
	builder.add(cppModuleMap.getArtifact());
	}

As clang doesn't seem to fail if it can't find a module map file referenced by another module map, this works fine as long as sandboxing is enabled. However, if the compilation action doesn't run sandboxed, clang may load the referenced transitive module maps files, resulting in a "missing dependency declaration" error.

Which category does this issue belong to?

C++ Rules

What's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.

# .bazelrc
build --features=layering_check
build --repo_env=CC=clang

# MODULE.bazel
bazel_dep(name = "abseil-cpp", version = "20240116.1")

$ bazel clean --expunge && bazel --nohome_rc --nosystem_rc build @abseil-cpp//absl/base:throw_delegate --spawn_strategy=sandboxed
INFO: Invocation ID: aa6bf76b-d655-4601-904a-654a621f86ca
INFO: Starting clean (this may take a while). Consider using --async if the clean takes more than several minutes.
Starting local Bazel server and connecting to it...
INFO: Analyzed target @@abseil-cpp~20240116.1//absl/base:throw_delegate (70 packages loaded, 345 targets configured).
INFO: Found 1 target...
Target @@abseil-cpp~20240116.1//absl/base:throw_delegate up-to-date:
  bazel-bin/external/abseil-cpp~20240116.1/absl/base/libthrow_delegate.a
  bazel-bin/external/abseil-cpp~20240116.1/absl/base/libthrow_delegate.so
INFO: Elapsed time: 8.820s, Critical Path: 0.70s
INFO: 15 processes: 10 internal, 5 linux-sandbox.
INFO: Build completed successfully, 15 total actions

$ bazel clean --expunge && bazel --nohome_rc --nosystem_rc build @abseil-cpp//absl/base:throw_delegate --spawn_strategy=standalone
INFO: Invocation ID: 550feb94-c0c0-4f84-a50d-b17a2d73dbe3
INFO: Starting clean (this may take a while). Consider using --async if the clean takes more than several minutes.
Starting local Bazel server and connecting to it...
INFO: Analyzed target @@abseil-cpp~20240116.1//absl/base:throw_delegate (70 packages loaded, 345 targets configured).
ERROR: /home/fhenneke/.cache/bazel/_bazel_fhenneke/1265e716d099d259b9c08d61b24f01b4/external/abseil-cpp~20240116.1/absl/base/BUILD.bazel:339:11: Compiling absl/base/internal/throw_delegate.cc failed: undeclared inclusion(s) in rule '@@abseil-cpp~20240116.1//absl/base:throw_delegate':
this rule is missing dependency declarations for the following files included by 'absl/base/internal/throw_delegate.cc':
  'bazel-out/k8-fastbuild/bin/external/abseil-cpp~20240116.1/absl/base/core_headers.cppmap'
  'bazel-out/k8-fastbuild/bin/external/abseil-cpp~20240116.1/absl/base/atomic_hook.cppmap'
  'bazel-out/k8-fastbuild/bin/external/abseil-cpp~20240116.1/absl/base/errno_saver.cppmap'
  'bazel-out/k8-fastbuild/bin/external/abseil-cpp~20240116.1/absl/base/log_severity.cppmap'
Target @@abseil-cpp~20240116.1//absl/base:throw_delegate failed to build
Use --verbose_failures to see the command lines of failed build steps.
INFO: Elapsed time: 8.400s, Critical Path: 0.59s
INFO: 13 processes: 11 internal, 2 local.
ERROR: Build did NOT complete successfully

Which operating system are you running Bazel on?

Linux

What is the output of bazel info release?

7.0.2

If bazel info release returns development version or (@non-git), tell us how you built Bazel.

No response

What's the output of git remote get-url origin; git rev-parse HEAD ?

No response

Is this a regression? If yes, please try to identify the Bazel commit where the bug was introduced.

No response

Have you found anything relevant by searching the web?

No response

Any other information, logs, or outputs that you want to share?

This was originally discovered in bazelbuild/rules_fuzzing#242 (comment).

[fmeum]

I briefly discussed this with @oquenchil, who mentioned that this was likely a design decision for Blaze. This leads to a few questions:

1. How does include scanning interact with the need to stage (or not to stage) transitive module maps?
2. Is it guaranteed that clang won't fail when it can't find a module map referenced by another module map? This behavior was pretty surprising to me.
3. Since LLVM 17, clang can suggest a module providing a header included from a non-direct dependency (llvm/llvm-project@ab0116e). This convenience feature is similar to JavaBuilder strict deps fixups, but requires transitive module maps. Does Google have an alternative for this feature that doesn't require more inputs?

[keith]

Hit this same case trying to upgrade LLVM to bazel 7.x, not sure yet how I can workaround it

[fmeum]

@keith Does it happen with sandboxing enabled?

[keith]

Yes, but I can't currently repro to test without sandboxing llvm/llvm-project#86297 (comment)

[redsun82]

We stumbled upon this issue here. We need to turn off sandboxing to do static analysis. Our workaround just consists in disabling the feature, which seems to be on by default:

bazel build //your/target --spawn_strategy=local --features=-layering_check

[keith]

Debugging this a bit, it appears the issue is that when sandboxing is disabled the target's .d file ends up with dependencies on more cppmaps than it depends on (presumably from its transitives) which leads to this error, which is actually correct if those were really being depended on. Looking for a fix

[keith]

This is trivially reproducible outside of a bazel as well. It appears clang includes any transitive modulemaps in the dotd file when they are readable (hence the sandboxing requirement here), even if there is no direct dependency on them.

[keith]

submitted a fix here #21832, please leave any feedback there!

[keith]

I noticed that this case is actually super hard to get out of as well I think. Unsurprising since it's theoretically non-hermetic but I think you have to clean expunge in some cases.