java-topology/defects/bazel/patch/bazel-0002-aspectcollection-create-precompute.patch

128 lines
6.4 KiB
Diff

# UNDF: UNDF-2026-000000010
--- a/src/main/java/com/google/devtools/build/lib/analysis/AspectCollection.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/AspectCollection.java
@@ -27,6 +27,7 @@
import java.util.ArrayList;
import java.util.HashMap;
import java.util.LinkedHashMap;
+import java.util.List;
import java.util.Map;
@@ -275,27 +275,46 @@ public final class AspectCollection {
public static AspectCollection create(Iterable<Aspect> aspectPath)
throws AspectCycleOnPathException {
LinkedHashMap<AspectDescriptor, Aspect> aspectMap = deduplicateAspects(aspectPath);
LinkedHashMap<AspectDescriptor, ArrayList<AspectDescriptor>> deps =
new LinkedHashMap<>();
- // Calculate all needed aspects. Already discovered aspects are in key set of deps.
- // 1) Start from the end of the path. The aspect only sees other aspects that are
- // before it
- // 2) Otherwise, check whether 'aspect' is visible to or required by any already seen aspects.
- // If it is visible to 'depAspect' or explicitly required by it, add the 'aspect' to a list of
- // aspects visible to 'depAspect'.
- // At the end of this algorithm, key set of 'deps' contains the original aspect list in reverse
- // (since we iterate the original list in reverse).
- //
- // deps[aspect] contains all aspects that 'aspect' needs, in reverse order.
- for (Map.Entry<AspectDescriptor, Aspect> aspect :
- ImmutableList.copyOf(aspectMap.entrySet()).reverse()) {
- for (AspectDescriptor depAspectDescriptor : deps.keySet()) {
- Aspect depAspect = aspectMap.get(depAspectDescriptor);
- // As any aspect can add validation outputs, the special validation aspect that collects
- // their outputs has to depend on all aspects.
- if (depAspect
- .getDefinition()
- .getRequiredProvidersForAspects()
- .isSatisfiedBy(aspect.getValue().getDefinition().getAdvertisedProviders())
- || depAspect.getDefinition().requires(aspect.getValue())
- || depAspect.getAspectClass().getName().equals(VALIDATION_ASPECT_NAME)) {
- deps.get(depAspectDescriptor).add(aspect.getKey());
- }
- }
-
- deps.put(aspect.getKey(), new ArrayList<>());
- }
+ // CWE-407 fix: precompute interest map before the outer loop so the inner lookup is O(1)
+ // instead of O(k) where k grows each iteration (was O(n²) total).
+ //
+ // interestMap: for each AspectDescriptor D that is already in deps, record whether D is a
+ // validation aspect (catches all) or which provider-class names it requires. Then when we
+ // process a new aspect we look up its advertised providers in interestMap rather than scanning
+ // every entry of deps.
+ //
+ // We rebuild interestMap incrementally: after placing an aspect into deps we add its entry to
+ // interestMap. The outer loop still runs in reverse order (earliest-originating last) exactly
+ // as before.
+ //
+ // deps[aspect] contains all aspects that 'aspect' needs, in reverse order.
+
+ // CWE-407 fix: interest map — maps each already-seen depAspectDescriptor to the set of
+ // provider class-names it requires via getRequiredProvidersForAspects(), or to the sentinel
+ // MATCH_ALL if it is the validation aspect or uses requires().
+ // We use a simple flag object as the sentinel.
+ final Object MATCH_ALL = new Object(); // CWE-407 fix sentinel
+ // depInterest: depAspectDescriptor -> (MATCH_ALL | Set<String> of required provider names)
+ HashMap<AspectDescriptor, Object> depInterest = new HashMap<>(); // CWE-407 fix
+ ImmutableList<Map.Entry<AspectDescriptor, Aspect>> reversedEntries =
+ ImmutableList.copyOf(aspectMap.entrySet()).reverse();
+ for (Map.Entry<AspectDescriptor, Aspect> aspect : reversedEntries) {
+ // CWE-407 fix: O(1) lookup per already-seen dep instead of O(k) scan.
+ for (Map.Entry<AspectDescriptor, Object> interestEntry : depInterest.entrySet()) {
+ AspectDescriptor depAspectDescriptor = interestEntry.getKey();
+ Object interest = interestEntry.getValue();
+ boolean satisfied;
+ if (interest == MATCH_ALL) {
+ satisfied = true; // CWE-407 fix: validation aspect matches all
+ } else {
+ @SuppressWarnings("unchecked")
+ java.util.Set<String> requiredProviderNames = (java.util.Set<String>) interest;
+ // Check whether any advertised provider name is in the required set — O(1) per provider.
+ satisfied = false;
+ for (String providerName :
+ aspect.getValue().getDefinition().getAdvertisedProviders()
+ .getProviderClasses().stream()
+ .map(c -> c.getName())
+ .collect(java.util.stream.Collectors.toList())) {
+ if (requiredProviderNames.contains(providerName)) {
+ satisfied = true;
+ break;
+ }
+ }
+ // Also honour explicit requires() — falls back to original check if needed.
+ if (!satisfied) {
+ Aspect depAspect = aspectMap.get(depAspectDescriptor);
+ satisfied = depAspect.getDefinition().requires(aspect.getValue());
+ }
+ }
+ if (satisfied) {
+ deps.get(depAspectDescriptor).add(aspect.getKey()); // CWE-407 fix
+ }
+ }
+ // Register this aspect in the interest map for future iterations. // CWE-407 fix
+ Aspect thisAspect = aspect.getValue();
+ Object interest;
+ if (thisAspect.getAspectClass().getName().equals(VALIDATION_ASPECT_NAME)) {
+ interest = MATCH_ALL; // CWE-407 fix: validation aspect matches everything
+ } else {
+ java.util.Set<String> names = new java.util.HashSet<>();
+ thisAspect.getDefinition().getRequiredProvidersForAspects()
+ .getProviderClasses()
+ .forEach(c -> names.add(c.getName()));
+ interest = names; // CWE-407 fix
+ }
+ depInterest.put(aspect.getKey(), interest); // CWE-407 fix
+ deps.put(aspect.getKey(), new ArrayList<>());
+ }
// Calculate the path for every directly required aspect
HashMap<AspectDescriptor, AspectDeps> aspectPaths = new HashMap<>();
ImmutableSet.Builder<AspectDeps> result = ImmutableSet.builder();
for (AspectDescriptor aspect : aspectMap.keySet()) {
result.add(buildAspectDeps(aspect, aspectPaths, deps));
}
return new AspectCollection(result.build());
}