AOT: follow-up to #982: 48 fixes

Source/JavaScriptCore/aot/AOT.cmake

@@ -594,6 +594,18 @@ function(AOT_GENERATE_RUNTIME_FACTS)
     set(JavaScriptCore_SOURCES ${JavaScriptCore_SOURCES} "${JavaScriptCore_AOT_GENERATED_DIR}/AOTRuntimeFactsOfLayoutsIncludes.h" PARENT_SCOPE)
 endfunction()
 
+# It reads the text of the compiler's sources too. What it writes says only that they have passed.
+function(AOT_CHECK_READS_THAT_ASSUME)
+    file(GLOB_RECURSE sources CONFIGURE_DEPENDS "${JAVASCRIPTCORE_DIR}/aot/*.cpp" "${JAVASCRIPTCORE_DIR}/aot/*.h")
+    add_custom_command(
+        OUTPUT "${JavaScriptCore_AOT_GENERATED_DIR}/AOTReadsThatAssumeChecked.h"
+        DEPENDS ${sources} "${JAVASCRIPTCORE_DIR}/aot/codegen/generator/reads-that-assume.ts" "${JAVASCRIPTCORE_DIR}/aot/codegen/AOTReadsThatAssume.txt"
+        COMMAND "${NODE_EXECUTABLE}" "${JAVASCRIPTCORE_DIR}/aot/codegen/generator/reads-that-assume.ts" --javascriptcore "${JAVASCRIPTCORE_DIR}" --out "${JavaScriptCore_AOT_GENERATED_DIR}"
+        VERBATIM
+    )
+    set(JavaScriptCore_SOURCES ${JavaScriptCore_SOURCES} "${JavaScriptCore_AOT_GENERATED_DIR}/AOTReadsThatAssumeChecked.h" PARENT_SCOPE)
+endfunction()
+
 function(AOT_READ_LIBRARY_SOURCES result problemsResult)
     set(problems)
     set(sources)
@@ -696,6 +708,7 @@ AOT_GENERATE_FROM_SCHEMA()
 AOT_GENERATE_BUILTIN_TABLE()
 AOT_GENERATE_VECTOR()
 AOT_GENERATE_RUNTIME_FACTS()
+AOT_CHECK_READS_THAT_ASSUME()
 AOT_COMPUTE_SOURCES()
 AOT_CHECK_SOURCE_LISTS()
 AOT_CHECK_EXPORTED_HEADERS()

Source/JavaScriptCore/aot/Sources.txt

@@ -159,6 +159,7 @@ aot/ir/AOTEffectsOfOperators.cpp
 aot/ir/AOTEffectsOfProtocols.cpp
 aot/ir/AOTEffectsOfRules.cpp
 aot/ir/AOTFieldValueNarrowing.cpp
+aot/ir/AOTForwardWalk.cpp
 aot/ir/AOTIRFunction.cpp
 aot/ir/AOTIRProgram.cpp
 aot/ir/AOTKnownClosures.cpp

Source/JavaScriptCore/aot/analysis/AOTBorrowedParameters.cpp

@@ -848,7 +848,7 @@ void BorrowingBodies::didLower(std::span<const FunctionID> functions)
     }
 }
 
-BorrowedParameters computeBorrowedParameters(const Program& program, BuildContext& build, BorrowingBodies& kept, std::span<const uint8_t> hasChanged, std::span<const uint8_t> recordLooksDifferent)
+BorrowedParameters computeBorrowedParameters(const Program& program, BuildContext& build, BorrowingBodies& kept, std::span<const uint8_t> hasChanged, std::span<const uint8_t> recordLooksDifferent, const BitVector* bodiesToScan)
 {
     const ParallelLoops& loops = build.parallelLoops();
     unsigned numberOfFunctions = program.functions.size();
@@ -860,7 +860,7 @@ BorrowedParameters computeBorrowedParameters(const Program& program, BuildContex
 
     auto scan = [&](unsigned index) {
         const Function& function = program.functions[index];
-        if (!build.isEnabled(FlowResultKind::OperandSummaries, index) || !mayBorrow(program, function, opcodesThatBorrowNothing))
+        if ((bodiesToScan && !bodiesToScan->quickGet(index)) || !build.isEnabled(FlowResultKind::OperandSummaries, index) || !mayBorrow(program, function, opcodesThatBorrowNothing))
             return BorrowingBody { };
         return scanBorrowingBody(program, function, &kept.m_targetLists);
     };

Source/JavaScriptCore/aot/analysis/AOTBorrowedParameters.h

@@ -30,7 +30,8 @@ class BorrowingBodies;
 class Program;
 
 JS_EXPORT_PRIVATE BorrowedParameters computeBorrowedParameters(const Program&, BuildContext&);
-BorrowedParameters computeBorrowedParameters(const Program&, BuildContext&, BorrowingBodies& kept, std::span<const uint8_t> hasChanged, std::span<const uint8_t> recordLooksDifferent);
+// A function whose body is not among those to scan borrows nothing.
+BorrowedParameters computeBorrowedParameters(const Program&, BuildContext&, BorrowingBodies& kept, std::span<const uint8_t> hasChanged, std::span<const uint8_t> recordLooksDifferent, const BitVector* bodiesToScan = nullptr);
 
 inline constexpr unsigned maxBorrowedParameters = 21;
 
@@ -60,7 +61,7 @@ class BorrowedParameters {
     friend bool operator==(const BorrowedParameters&, const BorrowedParameters&) = default;
 
 private:
-    friend BorrowedParameters computeBorrowedParameters(const Program&, BuildContext&, BorrowingBodies&, std::span<const uint8_t>, std::span<const uint8_t>);
+    friend BorrowedParameters computeBorrowedParameters(const Program&, BuildContext&, BorrowingBodies&, std::span<const uint8_t>, std::span<const uint8_t>, const BitVector*);
 
     bool hasBit(FunctionID function, unsigned row, uint32_t entryParameter) const { return entryParameter < maxBorrowedParameters && wordOf(function) >> (row * maxBorrowedParameters + entryParameter) & 1; }
 
@@ -137,7 +138,7 @@ class BorrowingBodies {
     bool wasLoweredSinceScanned(FunctionID id) const { return id.index() < m_wasLoweredSinceScanned.size() && m_wasLoweredSinceScanned[id.index()]; }
 
 private:
-    friend BorrowedParameters computeBorrowedParameters(const Program&, BuildContext&, BorrowingBodies&, std::span<const uint8_t>, std::span<const uint8_t>);
+    friend BorrowedParameters computeBorrowedParameters(const Program&, BuildContext&, BorrowingBodies&, std::span<const uint8_t>, std::span<const uint8_t>, const BitVector*);
 
     Vector<BorrowingBody> m_bodies;
     HandedOnTargetLists m_targetLists;

Source/JavaScriptCore/aot/analysis/AOTGuardFacts.cpp

@@ -1390,12 +1390,17 @@ bool guardProvesSlotFilled(FunctionAnalyses& analyses, const Value& slotOperatio
         return false;
 
     const Program& program = analyses.program();
-    bool testOfCellAnswers = isAnsweredByTestOfCell(program, slotOperation);
+    std::optional<bool> testOfCellAnswers;
+    auto askTestOfCell = [&] {
+        if (!testOfCellAnswers)
+            testOfCellAnswers = isAnsweredByTestOfCell(program, slotOperation);
+        return *testOfCellAnswers;
+    };
     ValueID object = analyses.identityOf(slotOperation.objectOperand());
     bool isProven = std::ranges::any_of(analyses.guardFacts(guard).facts, [&](const GuardFact& fact) {
         if (fact.subject != object)
             return false;
-        return provesSlotFilled(program, analyses.function(), fact, slotOperation.key()) || (testOfCellAnswers && fact.proves(Proof::BirthFieldsFilled)) || isFilledBehindEveryAllocation(analyses, fact, slotOperation);
+        return provesSlotFilled(program, analyses.function(), fact, slotOperation.key()) || (fact.proves(Proof::BirthFieldsFilled) && askTestOfCell()) || isFilledBehindEveryAllocation(analyses, fact, slotOperation);
     });
     RELEASE_ASSERT(mayBeProven || !isProven);
     return isProven;

Source/JavaScriptCore/aot/analysis/AOTHeatGraph.cpp

@@ -535,7 +535,7 @@ struct SeenCode {
 static bool isModuleBodyFunction(const Function& function) { return function.module && function.isModuleBody(); }
 
 // A module is loaded by the entry, or by a module that names it. A closure is made by a body that runs. A call of a callee that nobody knows reaches what has been made and may stand there.
-static SeenCode findWhatSeenCodeReaches(const Program& program, const CallGraph& callGraph, const FlowResults& flow, const HeatGraph& graph, const ParallelArray<CallsOfNode>& calls, const FunctionsUnderNames& names, const Vector<Vector<FunctionID>>& functionsMadeBy, const BitVector& isCalledByHost, const BitVector& hasOnlySeenCallers, const Vector<Vector<uint32_t>>& namesReadBy, const Vector<Vector<FunctionID>>& accessorsReachedFrom, const Vector<Vector<uint32_t>>& holdersReadBy, const Vector<Vector<ModuleID>>& modulesLoadedBy, const Vector<uint8_t>& constructsUnknownValue, EstimateOf estimate, bool hasCallersInOtherImages)
+static SeenCode findWhatSeenCodeReaches(const Program& program, const CallGraph& callGraph, const FlowResults& flow, const HeatGraph& graph, const ParallelArray<CallsOfNode>& calls, const FunctionsUnderNames& names, const Vector<Vector<FunctionID>>& functionsMadeBy, const BitVector& isCalledByHost, const BitVector& hasOnlySeenCallers, const Vector<Vector<uint32_t>>& namesReadBy, const Vector<Vector<FunctionID>>& accessorsReachedFrom, const Vector<Vector<uint32_t>>& holdersReadBy, const Vector<Vector<ModuleID>>& modulesLoadedBy, const Vector<uint8_t>& constructsUnknownValue, EstimateOf estimate)
 {
     unsigned count = graph.numberOfFunctions();
     bool isForClasses = estimate == EstimateOf::Classes;
@@ -608,7 +608,7 @@ static SeenCode findWhatSeenCodeReaches(const Program& program, const CallGraph&
     BitVector isUnderNameThatIsCalled(count);
     BitVector isUnderNameReadAsValue(count);
     auto mayStandWhereValueIsCalled = [&](FunctionID function) {
-        return hasEscaped.quickGet(function.index()) && (hasCallersInOtherImages || !names.isUnderSomeName(function) || isUnderNameReadAsValue.quickGet(function.index()) || seen.isAccessorReachedUnderItsName.quickGet(function.index()) || (evaluatesText && !isLibraryCode(program, program.at(function))));
+        return hasEscaped.quickGet(function.index()) && (!names.isUnderSomeName(function) || isUnderNameReadAsValue.quickGet(function.index()) || seen.isAccessorReachedUnderItsName.quickGet(function.index()) || (evaluatesText && !isLibraryCode(program, program.at(function))));
     };
     bool constructsValue = false;
     Vector<Vector<FunctionID>> functionsWaitingFor(program.modules.size());
@@ -638,8 +638,6 @@ static SeenCode findWhatSeenCodeReaches(const Program& program, const CallGraph&
                 reachIfCalled(waiting);
         }
     };
-    if (hasCallersInOtherImages)
-        reach(graph.unknownCodeNode().index());
     for (const Entry& entry : program.entries)
         load(entry.module);
     if (program.entries.isEmpty()) {
@@ -1533,7 +1531,6 @@ HeatGraph buildHeatGraph(const Program& program, const CallGraph& callGraph, con
     });
     BitVector noFunction(count);
     Vector<double> entriesPerEvent;
-    const bool hasCallersInOtherImages = build.imagesBelow() && build.imagesBelow()->hasPrebuiltLibrary();
     auto addCallsOfNodes = [&](EstimateOf estimate) {
         bool isForClasses = estimate == EstimateOf::Classes;
         const BitVector& hasOnlySeenCallers = isForClasses ? hasOnlySeenCallersByFlow : noFunction;
@@ -1545,7 +1542,7 @@ HeatGraph buildHeatGraph(const Program& program, const CallGraph& callGraph, con
             if (flow.mayRunUnseenCode())
                 calls[graph.unknownCodeNode().index()] = shareAmong(program, flow, graph, isCalledPerEvent, seen, callGraph.functionsCalledByUnknownCode());
         } else
-            seen = findWhatSeenCodeReaches(program, callGraph, flow, graph, calls, names, functionsMadeBy, isCalledByHost, hasOnlySeenCallers, namesReadBy, accessorsReachedFrom, holdersReadBy, modulesLoadedBy, constructsUnknownValue, estimate, hasCallersInOtherImages);
+            seen = findWhatSeenCodeReaches(program, callGraph, flow, graph, calls, names, functionsMadeBy, isCalledByHost, hasOnlySeenCallers, namesReadBy, accessorsReachedFrom, holdersReadBy, modulesLoadedBy, constructsUnknownValue, estimate);
         Vector<double> eventsOfHandlers(FillWith { }, count, 0.0);
         if (!isForClasses) {
             forEachIndexInParallel(loops, graph.m_numberOfNames, [&](unsigned position) {

Source/JavaScriptCore/aot/builtins/AOTBuiltinTable.h

@@ -666,6 +666,9 @@ struct BuiltinHostObject {
     bool keepsStringsOfStoredValues { false };
     // For each of these names the object has an accessor whose getter and setter are native code.
     std::span<const BuiltinName> nativeAccessors { };
+    // The build reads what lies under these names, so native code locks them in a compiled program. The holder is the member that hands out the object.
+    std::span<const BuiltinName> lockedNames { };
+    BuiltinID holder { };
     // The systems on which native code makes the object.
     OptionSet<BuiltinPlatform> platforms { everyBuiltinPlatform };
 };
@@ -1114,13 +1117,11 @@ class BuiltinTableOfBuild {
 
     const BuiltinTable& table() const LIFETIME_BOUND { return m_table; }
     // `members` ascend. Leaves out a locked member that canBeReplacedInTestRuns() refuses. `holdersOfNewNames`: the objects that get a name of their own opened, which the caller keeps.
-    // `opensEveryHolder`: every object takes whatever name canTakeNameInTestRuns() admits.
-    void unlock(std::span<const BuiltinID> members, std::span<const BuiltinID> holdersOfNewNames, bool opensEveryHolder = false);
+    void unlock(std::span<const BuiltinID> members, std::span<const BuiltinID> holdersOfNewNames);
     std::span<const BuiltinID> unlockedMembers() const LIFETIME_BOUND { return m_unlockedMembers.span(); }
 
 private:
     void lockByNames(BuiltinID wholeObject);
-    void openToNames(BuiltinID holder);
 
     BuiltinTableData m_data;
     Vector<Builtin> m_builtins;

Source/JavaScriptCore/aot/builtins/AOTBuiltinTableDescription.cpp

@@ -347,7 +347,7 @@ class TableWriter {
     void write(const BuiltinHostObject& hostObject)
     {
         object("cell"_s, PlainName { hostObject.nativeCell }, "function"_s, hostObject.function, "prototype"_s, hostObject.prototype, "fields"_s, hostObject.fields,
-            "keepsStringsOfStoredValues"_s, hostObject.keepsStringsOfStoredValues, "nativeAccessors"_s, hostObject.nativeAccessors, "platforms"_s, hostObject.platforms);
+            "keepsStringsOfStoredValues"_s, hostObject.keepsStringsOfStoredValues, "nativeAccessors"_s, hostObject.nativeAccessors, "lockedNames"_s, hostObject.lockedNames, "holder"_s, hostObject.holder, "platforms"_s, hostObject.platforms);
     }
 
     void write(const FeatureUseSummary& summary)

Source/JavaScriptCore/aot/builtins/AOTBuiltinTableOfBuild.cpp

@@ -135,42 +135,15 @@ void BuiltinTableOfBuild::lockByNames(BuiltinID wholeObject)
     m_closedKeys.insert(position, BuiltinClosedKeys { wholeObject, m_closedSymbols.last().span(), { } });
 }
 
-// The names that no test run adds stay closed, each by itself.
-void BuiltinTableOfBuild::openToNames(BuiltinID holder)
-{
-    lockByNames(holder);
-    Vector<BuiltinName> closed(m_table.closedKeysOf(holder));
-    size_t position = std::ranges::lower_bound(m_closedKeys, holder, { }, &BuiltinClosedKeys::holder) - m_closedKeys.begin();
-    bool hasRecord = position < m_closedKeys.size() && m_closedKeys[position].holder == holder;
-    if (hasRecord)
-        closed = Vector<BuiltinName>(m_closedKeys[position].names);
-    for (uint32_t index = 0; index < m_table.nameCount(); ++index) {
-        BuiltinName name(index);
-        if (name && !m_table.isSymbolName(name) && !m_table.findOwnMember(holder, name) && !m_table.canTakeNameInTestRuns(holder, name) && !std::ranges::binary_search(m_table.openNamesOf(holder), name))
-            closed.append(name);
-    }
-    AOT::sort(closed);
-    closed.shrink(closed.size() - std::ranges::unique(closed).size());
-    m_closedSymbols.append(WTF::move(closed));
-    if (hasRecord)
-        m_closedKeys[position] = BuiltinClosedKeys { holder, m_closedSymbols.last().span(), { } };
-    else
-        m_closedKeys.insert(position, BuiltinClosedKeys { holder, m_closedSymbols.last().span(), { } });
-    for (uint32_t index = 0; index < m_builtins.size(); ++index) {
-        if (BuiltinID(index) == holder || m_builtins[index].sameObjectAs == holder)
-            m_builtins[index].flags.remove(BuiltinFlag::IsClosedToOtherNames);
-    }
-}
-
-void BuiltinTableOfBuild::unlock(std::span<const BuiltinID> members, std::span<const BuiltinID> holdersOfNewNames, bool opensEveryHolder)
+void BuiltinTableOfBuild::unlock(std::span<const BuiltinID> members, std::span<const BuiltinID> holdersOfNewNames)
 {
     RELEASE_ASSERT(m_builtins.isEmpty());
     for (BuiltinID member : members) {
         // A member that is constant unless written is listed for the sake of an image that does not hold the write: it is not to lock the member at its start.
         if (m_table.contains(member) && (m_table.canBeReplacedInTestRuns(member) || m_table.at(member).mutability == BuiltinMutability::ConstantUnlessWritten))
             m_unlockedMembers.append(member);
     }
-    if (m_unlockedMembers.isEmpty() && holdersOfNewNames.empty() && !opensEveryHolder)
+    if (m_unlockedMembers.isEmpty() && holdersOfNewNames.empty())
         return;
     m_builtins = Vector<Builtin>(m_data.builtins);
     m_lockedObjects = Vector<BuiltinID>(m_data.runtime.lockedObjects);
@@ -185,10 +158,6 @@ void BuiltinTableOfBuild::unlock(std::span<const BuiltinID> members, std::span<c
     }
     for (BuiltinID holder : holdersOfNewNames)
         lockByNames(holder);
-    for (uint32_t index = 0; opensEveryHolder && index < m_builtins.size(); ++index) {
-        if (m_table.canonicalID(BuiltinID(index)) == BuiltinID(index) && m_table.canTakeNameInTestRuns(BuiltinID(index), { }))
-            openToNames(BuiltinID(index));
-    }
     m_data.builtins = m_builtins.span();
     m_data.runtime.lockedMembers = m_lockedMembers.span();
     m_data.runtime.lockedObjects = m_lockedObjects.span();
@@ -217,8 +186,8 @@ void Program::unlockReplaceableBuiltins()
         holdersOfNewNames.append(opened.holder);
         m_namesOpenedSomewhere.set(opened.name.index());
     }
-    m_tableOfBuild.unlock(members.span(), holdersOfNewNames.span(), takesAnyNameFromTests);
-    m_testsReplaceBuiltins = takesAnyNameFromTests || !openedNames.isEmpty() || !losesLockOfTable.isEmpty();
+    m_tableOfBuild.unlock(members.span(), holdersOfNewNames.span());
+    m_testsReplaceBuiltins = !openedNames.isEmpty() || !losesLockOfTable.isEmpty();
     if (unlockedBuiltins().empty() && openedNames.isEmpty())
         return;
 

Source/JavaScriptCore/aot/builtins/generator/build-attachments.ts

@@ -976,8 +976,10 @@ function buildHostObjects(context: AttachmentContext): ModelHostObject[] {
     }));
     const described = declared.function === null ? null : builder.resolve(declared.function, source);
     const prototype = declared.prototype === null ? null : resolveObject(builder, declared.prototype, source);
+    const holder = declared.holder === null ? null : builder.resolve(declared.holder, source);
     if ((declared.function !== null && described === null) || (declared.prototype !== null && prototype === null))
       continue;
+    if (declared.holder !== null && holder === null) continue;
     objects.push({
       cell: builder.internName(declared.cell, "Text"),
       function: described,
@@ -991,6 +993,8 @@ function buildHostObjects(context: AttachmentContext): ModelHostObject[] {
       internalFields: internalFields.flatMap(({ index, uses }) => (uses === null ? [] : [{ index, uses }])),
       keepsStringsOfStoredValues: declared.keepsStringsOfStoredValues,
       nativeAccessors: declared.nativeAccessors.map(name => builder.internComponent(name)),
+      lockedNames: declared.lockedNames.map(name => builder.internComponent(name)),
+      holder,
       platforms: declared.platforms,
       source,
     });

Source/JavaScriptCore/aot/builtins/generator/declarations.ts

@@ -344,6 +344,9 @@ export interface HostObjectDeclaration {
   internalFields: readonly HostObjectInternalFieldSpec[];
   keepsStringsOfStoredValues: boolean;
   nativeAccessors: readonly string[];
+  lockedNames: readonly string[];
+  /** The member that hands out the object. null: no name of it is locked. */
+  holder: BuiltinPath | null;
   /** The systems on which native code makes it. */
   platforms: readonly TargetPlatform[];
 }

Source/JavaScriptCore/aot/builtins/generator/dump.ts

@@ -294,6 +294,8 @@ export interface DumpedHostObject {
   fields: { name: string | null; value: DumpedPlace }[];
   keepsStringsOfStoredValues: boolean;
   nativeAccessors: string[];
+  lockedNames: string[];
+  holder: string | null;
   platforms: TargetPlatform[];
 }
 

Source/JavaScriptCore/aot/builtins/generator/emit-cpp-attachments.ts

@@ -243,6 +243,8 @@ function formatHostObject(context: AttachmentContext, object: ModelHostObject):
     ],
     ["keepsStringsOfStoredValues", object.keepsStringsOfStoredValues ? "true" : null],
     ["nativeAccessors", formatNames(context, object.nativeAccessors)],
+    ["lockedNames", formatNames(context, object.lockedNames)],
+    ["holder", formatId("BuiltinID", object.holder)],
     [
       "platforms",
       formatOptionSet(

Source/JavaScriptCore/aot/builtins/generator/emit-dump.ts

@@ -401,6 +401,8 @@ function dumpHostObject(object: ModelHostObject): DumpedHostObject {
     })),
     keepsStringsOfStoredValues: object.keepsStringsOfStoredValues,
     nativeAccessors: object.nativeAccessors.map(spellingOf),
+    lockedNames: object.lockedNames.map(spellingOf),
+    holder: object.holder?.token ?? null,
     platforms: inListOrder(targetPlatforms, object.platforms),
   };
 }