Commit 23b3ced0 authored by Iain Ireland's avatar Iain Ireland
Browse files

Bug 1676639: Add BailoutKind::EagerTruncation r=jandem 

This fixes a fuzz bug where BoxInputsPolicy creates an infallible MToDouble node (with BailoutKind::Unknown, because it's infallible), then range analysis eagerly truncates it, creating an MToNumberInt32 with unknown bailout kind. The fix is to create a new bailout kind for eager truncation and have range analysis set it where appropriate.

(We were talking about doing this earlier, but I held off because I don't understand the truncation code well and I was a bit concerned that it might cause performance regressions. Fortunately, the performance looks good on try.)

Differential Revision: https://phabricator.services.mozilla.com/D98876
parent 42632a40

js/src/jit-test/tests/warp/bug1676639.js

0 → 100644
+7 −0
Original line number Diff line number Diff line 
function foo() {

  return Math.atanh(true === Math.fround(0) | 0) != true;

}

var results = [];

for (var j = 0; j < 50; j++) {

  results.push(foo(0,0));

}

js/src/jit/BaselineBailouts.cpp

+7 −3
Original line number Diff line number Diff line
@@ -2080,6 +2080,13 @@ bool jit::FinishBailoutToBaseline(BaselineBailoutInfo* bailoutInfoArg) { 
      InvalidateAfterBailout(cx, outerScript, "bounds check failure");

      break;



    case BailoutKind::EagerTruncation:

      // An eager truncation generated by range analysis bailed out.

      MOZ_ASSERT(!outerScript->hadEagerTruncationBailout());

      outerScript->setHadEagerTruncationBailout();

      InvalidateAfterBailout(cx, outerScript, "eager range analysis failure");

      break;



    case BailoutKind::TooManyArguments:

      // A funapply or spread call had more than JIT_ARGS_LENGTH_MAX arguments.

      // TODO: Invalidate and disable recompilation if this happens too often.
@@ -2144,9 +2151,6 @@ bool jit::FinishBailoutToBaseline(BaselineBailoutInfo* bailoutInfoArg) { 
      break;



    // Invalid assumption based on baseline code.

    case BailoutKind::OverflowInvalidate:

      outerScript->setHadOverflowBailout();

      [[fallthrough]];

    case BailoutKind::DoubleOutput:

    case BailoutKind::ObjectIdentityOrTypeGuard:

      HandleBaselineInfoBailout(cx, outerScript, innerScript);

js/src/jit/CompileInfo.h

+4 −4
Original line number Diff line number Diff line
@@ -86,7 +86,7 @@ class CompileInfo { 
        osrPc_(osrPc),

        analysisMode_(analysisMode),

        scriptNeedsArgsObj_(scriptNeedsArgsObj),

        hadOverflowBailout_(script->hadOverflowBailout()),

        hadEagerTruncationBailout_(script->hadEagerTruncationBailout()),

        hadSpeculativePhiBailout_(script->hadSpeculativePhiBailout()),

        hadLICMInvalidation_(script->hadLICMInvalidation()),

        hadBoundsCheckBailout_(script->failedBoundsCheck()),
@@ -149,7 +149,7 @@ class CompileInfo { 
        osrPc_(nullptr),

        analysisMode_(Analysis_None),

        scriptNeedsArgsObj_(false),

        hadOverflowBailout_(false),

        hadEagerTruncationBailout_(false),

        hadSpeculativePhiBailout_(false),

        hadLICMInvalidation_(false),

        hadBoundsCheckBailout_(false),
@@ -379,7 +379,7 @@ class CompileInfo { 


  // Check previous bailout states to prevent doing the same bailout in the

  // next compilation.

  bool hadOverflowBailout() const { return hadOverflowBailout_; }

  bool hadEagerTruncationBailout() const { return hadEagerTruncationBailout_; }

  bool hadSpeculativePhiBailout() const { return hadSpeculativePhiBailout_; }

  bool hadLICMInvalidation() const { return hadLICMInvalidation_; }

  bool hadBoundsCheckBailout() const { return hadBoundsCheckBailout_; }
@@ -406,7 +406,7 @@ class CompileInfo { 


  // Record the state of previous bailouts in order to prevent compiling the

  // same function identically the next time.

  bool hadOverflowBailout_;

  bool hadEagerTruncationBailout_;

  bool hadSpeculativePhiBailout_;

  bool hadLICMInvalidation_;

  bool hadBoundsCheckBailout_;

js/src/jit/IonTypes.h

+7 −5
Original line number Diff line number Diff line
@@ -93,6 +93,11 @@ enum class BailoutKind : uint8_t { 
  // Normal bailouts, that don't need to be handled specially when restarting

  // in baseline.



  // An eager truncation generated by range analysis.

  // If this instruction bails out, we will invalidate the current Warp script

  // and mark the EagerTruncationBailout flag on the script.

  EagerTruncation,



  // An inevitable bailout (MBail instruction or type barrier that always bails)

  Inevitable,
@@ -188,9 +193,6 @@ enum class BailoutKind : uint8_t { 
  // Bailouts caused by invalid assumptions based on Baseline code.

  // Causes immediate invalidation.



  // Like BailoutKind::Overflow, but causes immediate invalidation.

  OverflowInvalidate,



  // Used for integer division, multiplication and modulo.

  // If there's a remainder, bails to return a double.

  // Can also signal overflow or result of -0.
@@ -298,6 +300,8 @@ inline const char* BailoutKindString(BailoutKind kind) { 
      return "HoistBoundsCheck";

    case BailoutKind::GenericIon:

      return "GenericIon";

    case BailoutKind::EagerTruncation:

      return "EagerTruncation";



    // Normal bailouts.

    case BailoutKind::Inevitable:
@@ -354,8 +358,6 @@ inline const char* BailoutKindString(BailoutKind kind) { 
      return "InvalidCodePoint";



    // Bailouts caused by invalid assumptions.

    case BailoutKind::OverflowInvalidate:

      return "OverflowInvalidate";

    case BailoutKind::DoubleOutput:

      return "DoubleOutput";

js/src/jit/MIR.h

+1 −8
Original line number Diff line number Diff line
@@ -4817,11 +4817,7 @@ class MUrsh : public MShiftInstruction { 


  MUrsh(MDefinition* left, MDefinition* right, MIRType type)

      : MShiftInstruction(classOpcode, left, right, type),

        bailoutsDisabled_(false) {

    // If this instruction bails out, we will set the HadOverflowBailout flag

    // on the script, which will cause RangeAnalysis to be less aggressive.

    setBailoutKind(BailoutKind::OverflowInvalidate);

  }

        bailoutsDisabled_(false) {}



 public:

  INSTRUCTION_HEADER(Ursh)
@@ -5476,9 +5472,6 @@ class MAdd : public MBinaryArithInstruction { 
  MAdd(MDefinition* left, MDefinition* right, MIRType type)

      : MBinaryArithInstruction(classOpcode, left, right, type) {

    setCommutative();

    // If this instruction bails out, we will set the HadOverflowBailout flag

    // on the script, which will cause RangeAnalysis to be less aggressive.

    setBailoutKind(BailoutKind::OverflowInvalidate);

  }



  MAdd(MDefinition* left, MDefinition* right, TruncateKind truncateKind)