@samitouri / QOS-React-1 / commits / 81d8115116

[compiler] Fix infinite loop due to uncached applied signatures (#33518)

When we apply new aliasing signatures we can generate new temporaries, which causes the abstract memory model to not converge. The fix is to make sure we cache the applications of these signatures. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/33518). * #33571 * #33558 * #33547 * #33543 * #33533 * #33532 * #33530 * #33526 * #33522 * __->__ #33518

Joseph Savona committed Jun 18, 2025 at 15:43 UTC 81d81151169be4b1b0ad8bd6439e94cfc982bb5a
4 files changed +237 -75
compiler/packages/babel-plugin-react-compiler/src/Inference/AliasingEffects.ts
+13 -2
@@ -8,6 +8,7 @@
8 import {CompilerErrorDetailOptions} from '../CompilerError';
9 import {
10 FunctionExpression,
11 + GeneratedSource,
12 Hole,
13 IdentifierId,
14 ObjectMethod,
@@ -18,6 +19,7 @@ import {
19 ValueReason,
20 } from '../HIR';
21 import {FunctionSignature} from '../HIR/ObjectShape';
22 +import {printSourceLocation} from '../HIR/PrintHIR';
23
24 /**
25 * `AliasingEffect` describes a set of "effects" that an instruction/terminal has on one or
@@ -200,10 +202,19 @@ export function hashEffect(effect: AliasingEffect): string {
202 return [effect.kind, effect.value.identifier.id, effect.reason].join(':');
203 }
204 case 'Impure':
203 - case 'Render':
205 + case 'Render': {
206 + return [effect.kind, effect.place.identifier.id].join(':');
207 + }
208 case 'MutateFrozen':
209 case 'MutateGlobal': {
206 - return [effect.kind, effect.place.identifier.id].join(':');
210 + return [
211 + effect.kind,
212 + effect.place.identifier.id,
213 + effect.error.severity,
214 + effect.error.reason,
215 + effect.error.description,
216 + printSourceLocation(effect.error.loc ?? GeneratedSource),
217 + ].join(':');
218 }
219 case 'Mutate':
220 case 'MutateConditionally':
compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingEffects.ts
+153 -73
@@ -50,12 +50,14 @@ import {
50 } from './InferReferenceEffects';
51 import {
52 assertExhaustive,
53 + getOrInsertDefault,
54 getOrInsertWith,
55 Set_isSuperset,
56 } from '../Utils/utils';
57 import {
58 printAliasingEffect,
59 printAliasingSignature,
60 + printFunction,
61 printIdentifier,
62 printInstruction,
63 printInstructionValue,
@@ -195,12 +197,15 @@ export function inferMutationAliasingEffects(
197 let count = 0;
198 while (queuedStates.size !== 0) {
199 count++;
198 - if (count > 1000) {
200 + if (count > 100) {
201 console.log(
202 'oops infinite loop',
203 fn.id,
204 typeof fn.loc !== 'symbol' ? fn.loc?.filename : null,
205 );
206 + if (DEBUG) {
207 + console.log(printFunction(fn));
208 + }
209 throw new Error('infinite loop');
210 }
211 for (const [blockId, block] of fn.body.blocks) {
@@ -212,6 +217,11 @@ export function inferMutationAliasingEffects(
217
218 statesByBlock.set(blockId, incomingState);
219 const state = incomingState.clone();
220 + if (DEBUG) {
221 + console.log('*************');
222 + console.log(`bb${block.id}`);
223 + console.log('*************');
224 + }
225 inferBlock(context, state, block);
226
227 for (const nextBlockId of eachTerminalSuccessor(block.terminal)) {
@@ -264,7 +274,13 @@ class Context {
274 instructionSignatureCache: Map<Instruction, InstructionSignature> = new Map();
275 effectInstructionValueCache: Map<AliasingEffect, InstructionValue> =
276 new Map();
277 + applySignatureCache: Map<
278 + AliasingSignature,
279 + Map<AliasingEffect, Array<AliasingEffect> | null>
280 + > = new Map();
281 catchHandlers: Map<BlockId, Place> = new Map();
282 + functionSignatureCache: Map<FunctionExpression, AliasingSignature> =
283 + new Map();
284 isFuctionExpression: boolean;
285 fn: HIRFunction;
286 hoistedContextDeclarations: Map<DeclarationId, Place | null>;
@@ -279,6 +295,19 @@ class Context {
295 this.hoistedContextDeclarations = hoistedContextDeclarations;
296 }
297
298 + cacheApplySignature(
299 + signature: AliasingSignature,
300 + effect: Extract<AliasingEffect, {kind: 'Apply'}>,
301 + f: () => Array<AliasingEffect> | null,
302 + ): Array<AliasingEffect> | null {
303 + const inner = getOrInsertDefault(
304 + this.applySignatureCache,
305 + signature,
306 + new Map(),
307 + );
308 + return getOrInsertWith(inner, effect, f);
309 + }
310 +
311 internEffect(effect: AliasingEffect): AliasingEffect {
312 const hash = hashEffect(effect);
313 let interned = this.internedEffects.get(hash);
@@ -352,11 +381,13 @@ function inferBlock(
381 state.appendAlias(handlerParam, instr.lvalue);
382 const kind = state.kind(instr.lvalue).kind;
383 if (kind === ValueKind.Mutable || kind == ValueKind.Context) {
355 - effects.push({
356 - kind: 'Alias',
357 - from: instr.lvalue,
358 - into: handlerParam,
359 - });
384 + effects.push(
385 + context.internEffect({
386 + kind: 'Alias',
387 + from: instr.lvalue,
388 + into: handlerParam,
389 + }),
390 + );
391 }
392 }
393 }
@@ -365,11 +396,11 @@ function inferBlock(
396 } else if (terminal.kind === 'return') {
397 if (!context.isFuctionExpression) {
398 terminal.effects = [
368 - {
399 + context.internEffect({
400 kind: 'Freeze',
401 value: terminal.value,
402 reason: ValueReason.JsxCaptured,
372 - },
403 + }),
404 ];
405 }
406 }
@@ -546,20 +577,21 @@ function applyEffect(
577 break;
578 }
579 case ValueKind.Frozen: {
549 - effects.push({
550 - kind: 'ImmutableCapture',
551 - from: effect.from,
552 - into: effect.into,
553 - });
580 + applyEffect(
581 + context,
582 + state,
583 + {
584 + kind: 'ImmutableCapture',
585 + from: effect.from,
586 + into: effect.into,
587 + },
588 + aliased,
589 + effects,
590 + );
591 break;
592 }
593 default: {
557 - effects.push({
558 - // OK: recording information flow
559 - kind: 'CreateFrom', // prev Alias
560 - from: effect.from,
561 - into: effect.into,
562 - });
594 + effects.push(effect);
595 }
596 }
597 break;
@@ -658,11 +690,17 @@ function applyEffect(
690 }
691 case ValueKind.Frozen: {
692 isMutableReferenceType = false;
661 - effects.push({
662 - kind: 'ImmutableCapture',
663 - from: effect.from,
664 - into: effect.into,
665 - });
693 + applyEffect(
694 + context,
695 + state,
696 + {
697 + kind: 'ImmutableCapture',
698 + from: effect.from,
699 + into: effect.into,
700 + },
701 + aliased,
702 + effects,
703 + );
704 break;
705 }
706 default: {
@@ -684,11 +722,17 @@ function applyEffect(
722 const fromKind = fromValue.kind;
723 switch (fromKind) {
724 case ValueKind.Frozen: {
687 - effects.push({
688 - kind: 'ImmutableCapture',
689 - from: effect.from,
690 - into: effect.into,
691 - });
725 + applyEffect(
726 + context,
727 + state,
728 + {
729 + kind: 'ImmutableCapture',
730 + from: effect.from,
731 + into: effect.into,
732 + },
733 + aliased,
734 + effects,
735 + );
736 let value = context.effectInstructionValueCache.get(effect);
737 if (value == null) {
738 value = {
@@ -746,23 +790,33 @@ function applyEffect(
790 * We're calling a locally declared function, we already know it's effects!
791 * We just have to substitute in the args for the params
792 */
749 - const signature = buildSignatureFromFunctionExpression(
750 - state.env,
751 - functionValues[0],
752 - );
793 + const functionExpr = functionValues[0];
794 + let signature = context.functionSignatureCache.get(functionExpr);
795 + if (signature == null) {
796 + signature = buildSignatureFromFunctionExpression(
797 + state.env,
798 + functionExpr,
799 + );
800 + context.functionSignatureCache.set(functionExpr, signature);
801 + }
802 if (DEBUG) {
803 console.log(
804 `constructed alias signature:\n${printAliasingSignature(signature)}`,
805 );
806 }
758 - const signatureEffects = computeEffectsForSignature(
759 - state.env,
807 + const signatureEffects = context.cacheApplySignature(
808 signature,
761 - effect.into,
762 - effect.receiver,
763 - effect.args,
764 - functionValues[0].loweredFunc.func.context,
765 - effect.loc,
809 + effect,
810 + () =>
811 + computeEffectsForSignature(
812 + state.env,
813 + signature,
814 + effect.into,
815 + effect.receiver,
816 + effect.args,
817 + functionExpr.loweredFunc.func.context,
818 + effect.loc,
819 + ),
820 );
821 if (signatureEffects != null) {
822 if (DEBUG) {
@@ -781,18 +835,24 @@ function applyEffect(
835 break;
836 }
837 }
784 - const signatureEffects =
785 - effect.signature?.aliasing != null
786 - ? computeEffectsForSignature(
838 + let signatureEffects = null;
839 + if (effect.signature?.aliasing != null) {
840 + const signature = effect.signature.aliasing;
841 + signatureEffects = context.cacheApplySignature(
842 + effect.signature.aliasing,
843 + effect,
844 + () =>
845 + computeEffectsForSignature(
846 state.env,
788 - effect.signature.aliasing,
847 + signature,
848 effect.into,
849 effect.receiver,
850 effect.args,
851 [],
852 effect.loc,
794 - )
795 - : null;
853 + ),
854 + );
855 + }
856 if (signatureEffects != null) {
857 if (DEBUG) {
858 console.log('apply aliasing signature effects');
@@ -935,30 +995,42 @@ function applyEffect(
995 effect.value.identifier.declarationId,
996 );
997 if (hoistedAccess != null && hoistedAccess.loc != effect.value.loc) {
938 - effects.push({
998 + applyEffect(
999 + context,
1000 + state,
1001 + {
1002 + kind: 'MutateFrozen',
1003 + place: effect.value,
1004 + error: {
1005 + severity: ErrorSeverity.InvalidReact,
1006 + reason: `This variable is accessed before it is declared, which may prevent it from updating as the assigned value changes over time`,
1007 + description,
1008 + loc: hoistedAccess.loc,
1009 + suggestions: null,
1010 + },
1011 + },
1012 + aliased,
1013 + effects,
1014 + );
1015 + }
1016 +
1017 + applyEffect(
1018 + context,
1019 + state,
1020 + {
1021 kind: 'MutateFrozen',
1022 place: effect.value,
1023 error: {
1024 severity: ErrorSeverity.InvalidReact,
943 - reason: `This variable is accessed before it is declared, which may prevent it from updating as the assigned value changes over time`,
1025 + reason: `This variable is accessed before it is declared, which prevents the earlier access from updating when this value changes over time`,
1026 description,
945 - loc: hoistedAccess.loc,
1027 + loc: effect.value.loc,
1028 suggestions: null,
1029 },
948 - });
949 - }
950 -
951 - effects.push({
952 - kind: 'MutateFrozen',
953 - place: effect.value,
954 - error: {
955 - severity: ErrorSeverity.InvalidReact,
956 - reason: `This variable is accessed before it is declared, which prevents the earlier access from updating when this value changes over time`,
957 - description,
958 - loc: effect.value.loc,
959 - suggestions: null,
1030 },
961 - });
1031 + aliased,
1032 + effects,
1033 + );
1034 } else {
1035 const reason = getWriteErrorReason({
1036 kind: value.kind,
@@ -970,18 +1042,26 @@ function applyEffect(
1042 effect.value.identifier.name.kind === 'named'
1043 ? `Found mutation of \`${effect.value.identifier.name.value}\``
1044 : null;
973 - effects.push({
974 - kind:
975 - value.kind === ValueKind.Frozen ? 'MutateFrozen' : 'MutateGlobal',
976 - place: effect.value,
977 - error: {
978 - severity: ErrorSeverity.InvalidReact,
979 - reason,
980 - description,
981 - loc: effect.value.loc,
982 - suggestions: null,
1045 + applyEffect(
1046 + context,
1047 + state,
1048 + {
1049 + kind:
1050 + value.kind === ValueKind.Frozen
1051 + ? 'MutateFrozen'
1052 + : 'MutateGlobal',
1053 + place: effect.value,
1054 + error: {
1055 + severity: ErrorSeverity.InvalidReact,
1056 + reason,
1057 + description,
1058 + loc: effect.value.loc,
1059 + suggestions: null,
1060 + },
1061 },
984 - });
1062 + aliased,
1063 + effects,
1064 + );
1065 }
1066 }
1067 break;
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/repro-compiler-infinite-loop.expect.md new
+54
@@ -0,0 +1,54 @@
1 +
2 +## Input
3 +
4 +```javascript
5 +// @flow @enableNewMutationAliasingModel
6 +
7 +import fbt from 'fbt';
8 +
9 +component Component() {
10 + const sections = Object.keys(items);
11 +
12 + for (let i = 0; i < sections.length; i += 3) {
13 + chunks.push(
14 + sections.slice(i, i + 3).map(section => {
15 + return <Child />;
16 + })
17 + );
18 + }
19 +
20 + return <Child />;
21 +}
22 +
23 +```
24 +
25 +## Code
26 +
27 +```javascript
28 +import { c as _c } from "react/compiler-runtime";
29 +
30 +import fbt from "fbt";
31 +
32 +function Component() {
33 + const $ = _c(1);
34 + const sections = Object.keys(items);
35 + for (let i = 0; i < sections.length; i = i + 3, i) {
36 + chunks.push(sections.slice(i, i + 3).map(_temp));
37 + }
38 + let t0;
39 + if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
40 + t0 = <Child />;
41 + $[0] = t0;
42 + } else {
43 + t0 = $[0];
44 + }
45 + return t0;
46 +}
47 +function _temp(section) {
48 + return <Child />;
49 +}
50 +
51 +```
52 +
53 +### Eval output
54 +(kind: exception) Fixture not implemented
\ No newline at end of file
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/repro-compiler-infinite-loop.js new
+17
@@ -0,0 +1,17 @@
1 +// @flow @enableNewMutationAliasingModel
2 +
3 +import fbt from 'fbt';
4 +
5 +component Component() {
6 + const sections = Object.keys(items);
7 +
8 + for (let i = 0; i < sections.length; i += 3) {
9 + chunks.push(
10 + sections.slice(i, i + 3).map(section => {
11 + return <Child />;
12 + })
13 + );
14 + }
15 +
16 + return <Child />;
17 +}