bazelbuild / bazelbuild/bazel

Skyframe crash vulnerability with transitions

Open
#29,132 1 comment 0 reactions 0 assignees View on GitHub
P2 team-Configurability type: bug
Dominant language
Java
Stars
25.8k
Forks
4.6k
Avg merge
2d 18h
Merged PRs (30d)
75

Description

### Description of the bug:

[StarlarkAttrTransitionProviderTest#.testTransitionOnBuildSetting_noSuchTarget](https://github.com/bazelbuild/bazel/blob/31c4c24667be57195b01548952409d891576c877/src/test/java/com/google/devtools/build/lib/analysis/starlark/StarlarkAttrTransitionProviderTest.java#L2725) is a simple test that transitioning on a non-existent flag returns a "no such target" error.

Comment out these lines:

https://github.com/bazelbuild/bazel/blob/31c4c24667be57195b01548952409d891576c877/src/main/java/com/google/devtools/build/lib/analysis/producers/TransitionApplier.java#L120-L124

then run:

```
$ USE_BAZEL_VERSION=9.0.1rc1 bazelisk test //src/test/java/com/google/devtools/build/lib/analysis/starlark:StarlarkAttrTransitionProviderTest --test_filter=.testTransitionOnBuildSetting_noSuchTarget --test_output=errors
```

It now fails with:

```
java.lang.IllegalStateException: Unexpected analysis error: ConfiguredTargetKey{label=//test/starlark:test, config=BuildConfigurationKey[1d3999f38bc198a7ca1cbe23f490a4a0ab9d8b194fcd3960c0c8e6b62206a781]} -> ErrorInfo{exception=com.google.devtools.build.lib.analysis.starlark.StarlarkTransition$TransitionException: com.google.devtools.build.lib.packages.NoSuchTargetException: no such target '//test/starlark:the-answer': target 'the-answer' not declared in package 'test/starlark' defined by /workspace/test/starlark
```

### Analysis:

The test target `//test/starlark:test` has an explicit exec dependency on `$function_transition_allowlist`.

The test intentionally triggers an error on a *different* Skyframe evaluation: the one that tries to load `//test/starlark:the-answer`, which doesn't exist.

Before the suggested change, https://github.com/bazelbuild/bazel/blob/31c4c24667be57195b01548952409d891576c877/src/main/java/com/google/devtools/build/lib/analysis/producers/TransitionApplier.java#L123

runs and completes before the failing thread runs. Since `applyStarlarkTransition` runs the Starlark thread that evaluates the exec transition, that means the exec transition completes successfully before Skyframe finds errors in other evaluations.

After the suggested change, this now runs: https://github.com/bazelbuild/bazel/blob/31c4c24667be57195b01548952409d891576c877/src/main/java/com/google/devtools/build/lib/analysis/producers/TransitionApplier.java#L129

I'm pretty sure that means Skyframe's currently running `Driver` completes, which gives the failing evaluation time to schedule and fail, then a new `Driver` evaluation runs on `applyStarlarkTransition`, which means the exec transition is evaluated after Skyiframe already knows there's a problem elsewhere.

That means that `StarlarkThread` calls:

https://github.com/bazelbuild/bazel/blob/31c4c24667be57195b01548952409d891576c877/src/main/java/net/starlark/java/eval/StarlarkThread.java#L123-L125

which `DependencyResolver` catches [here](https://github.com/bazelbuild/bazel/blob/31c4c24667be57195b01548952409d891576c877/src/main/java/com/google/devtools/build/lib/skyframe/DependencyResolver.java#L749) as an exceptional case, which triggers the failure.

I experimentally verified that sequencing. i..e I *know* the `StarlarkThread` interrupt triggers only after this change, and before this change the whole evaluation completes before the "no such target" evaluation fails. I'm not quite as confident what the principled answer is, although a simple catching of the `InterruptedException` [here](https://github.com/bazelbuild/bazel/blob/31c4c24667be57195b01548952409d891576c877/src/main/java/com/google/devtools/build/lib/analysis/producers/TransitionApplier.java#L158) should practically resolve it.

### Which category does this issue belong to?

Configurability

### Which operating system are you running Bazel on?

Linux

### What is the output of `bazel info release`?

release 9.0.1rc1

Contributor guide

Open the contributing guide

Research direction

Start with the referenced StarlarkAttrTransitionProviderTest.java case and reproduce it using the provided bazelisk command. Then read the cited TransitionApplier.java flow alongside StarlarkThread.java and DependencyResolver.java to understand the interrupted evaluation sequence. Done means the test reports the intended no-such-target error without an unexpected analysis crash.

Written by the indexing model from the issue text.

Assessment

Tech stack
java
Domain
build-system
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
42/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.