Skip to content

Commit

Permalink
[Backport] Security bug 1198309
Browse files Browse the repository at this point in the history
Cherry-pick of patch originally reviewed on
https://chromium-review.googlesource.com/c/v8/v8/+/2827899:
Merged: [TurboFan] Fix SpeculativeNumberEqual[Number] with undefined

(cherry picked from commit 7c7cdec5373127ad24e75edb2d2d75b25d604850)

Bug: chromium:1198309, v8:5660
No-Try: true
No-Presubmit: true
No-Tree-Checks: true
Change-Id: I9cb5f66643c0c0ab9b18ca953cf85d2f6aa84b42
Reviewed-by: Georg Neis <neis@chromium.org>
Commit-Queue: Nico Hartmann <nicohartmann@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#74038}
Cr-Commit-Position: refs/branch-heads/9.0@{#45}
Cr-Branched-From: bd0108b4c88e0d6f2350cb79b5f363fbd02f3eb7-refs/heads/9.0.257@{#1}
Cr-Branched-From: 349bcc6a075411f1a7ce2d866c3dfeefc2efa39d-refs/heads/master@{#73001}
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>
  • Loading branch information
nico-hartmann authored and mibrunin committed Apr 27, 2021
1 parent 19b53b4 commit a12ed35
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 12 deletions.
35 changes: 23 additions & 12 deletions chromium/v8/src/compiler/representation-change.cc
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,10 @@ Node* RepresentationChanger::GetRepresentationFor(
return GetFloat32RepresentationFor(node, output_rep, output_type,
use_info.truncation());
case MachineRepresentation::kFloat64:
DCHECK_NE(TypeCheckKind::kBigInt, use_info.type_check());
DCHECK(use_info.type_check() == TypeCheckKind::kNone ||
use_info.type_check() == TypeCheckKind::kNumber ||
use_info.type_check() == TypeCheckKind::kNumberOrBoolean ||
use_info.type_check() == TypeCheckKind::kNumberOrOddball);
return GetFloat64RepresentationFor(node, output_rep, output_type,
use_node, use_info);
case MachineRepresentation::kBit:
Expand Down Expand Up @@ -727,15 +730,22 @@ Node* RepresentationChanger::GetFloat64RepresentationFor(
}
} else if (IsAnyTagged(output_rep)) {
if (output_type.Is(Type::Undefined())) {
if (use_info.type_check() == TypeCheckKind::kNumberOrBoolean) {
if (use_info.type_check() == TypeCheckKind::kNumberOrOddball ||
(use_info.type_check() == TypeCheckKind::kNone &&
use_info.truncation().TruncatesOddballAndBigIntToNumber())) {
return jsgraph()->Float64Constant(
std::numeric_limits<double>::quiet_NaN());
} else {
DCHECK(use_info.type_check() == TypeCheckKind::kNone ||
use_info.type_check() == TypeCheckKind::kNumber ||
use_info.type_check() == TypeCheckKind::kNumberOrBoolean);
Node* unreachable = InsertUnconditionalDeopt(
use_node, DeoptimizeReason::kNotANumberOrBoolean);
use_node, use_info.type_check() == TypeCheckKind::kNumber
? DeoptimizeReason::kNotANumber
: DeoptimizeReason::kNotANumberOrBoolean);
return jsgraph()->graph()->NewNode(
jsgraph()->common()->DeadValue(MachineRepresentation::kFloat64),
unreachable);
} else {
return jsgraph()->Float64Constant(
std::numeric_limits<double>::quiet_NaN());
}
} else if (output_rep == MachineRepresentation::kTaggedSigned) {
node = InsertChangeTaggedSignedToInt32(node);
Expand All @@ -747,12 +757,13 @@ Node* RepresentationChanger::GetFloat64RepresentationFor(
output_type.Is(Type::NumberOrHole())) {
// JavaScript 'null' is an Oddball that results in +0 when truncated to
// Number. In a context like -0 == null, which must evaluate to false,
// this truncation must not happen. For this reason we restrict this case
// to when either the user explicitly requested a float (and thus wants
// +0 if null is the input) or we know from the types that the input can
// only be Number | Hole. The latter is necessary to handle the operator
// CheckFloat64Hole. We did not put in the type (Number | Oddball \ Null)
// to discover more bugs related to this conversion via crashes.
// this truncation must not happen. For this reason we restrict this
// case to when either the user explicitly requested a float (and thus
// wants +0 if null is the input) or we know from the types that the
// input can only be Number | Hole. The latter is necessary to handle
// the operator CheckFloat64Hole. We did not put in the type (Number |
// Oddball \ Null) to discover more bugs related to this conversion via
// crashes.
op = simplified()->TruncateTaggedToFloat64();
} else if (use_info.type_check() == TypeCheckKind::kNumber ||
(use_info.type_check() == TypeCheckKind::kNumberOrOddball &&
Expand Down
1 change: 1 addition & 0 deletions chromium/v8/src/deoptimizer/deoptimize-reason.h
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ namespace internal {
V(NotAJavaScriptObject, "not a JavaScript object") \
V(NotAJavaScriptObjectOrNullOrUndefined, \
"not a JavaScript object, Null or Undefined") \
V(NotANumber, "not a Number") \
V(NotANumberOrBoolean, "not a Number or Boolean") \
V(NotANumberOrOddball, "not a Number or Oddball") \
V(NotAnArrayIndex, "not an array index") \
Expand Down

0 comments on commit a12ed35

Please sign in to comment.