Skip to content

Commit

Permalink
[turbofan] BitcastWordToTagged must not be pure.
Browse files Browse the repository at this point in the history
The BitcastWordToTagged operator is used for bump pointer allocation to
construct the actual HeapObject pointer. The input to this operator is
a naked pointer (derived from the allocation top). If this input value
is live across an allocation, then the resulting tagged pointer is
invalid because the GC might have scavenged new space in the meantime.

That means we must not allow Node splitting (in the Scheduler) for these
instructions, as that could extend the live range of the naked pointer
input across arbitrary code. As such, this operator must not be marked
as pure.

[email protected]
BUG=v8:6059

Review-Url: https://codereview.chromium.org/2739093002
Cr-Commit-Position: refs/heads/master@{#43683}
  • Loading branch information
bmeurer authored and Commit bot committed Mar 9, 2017
1 parent cce8a19 commit 64fbb30
Showing 1 changed file with 17 additions and 1 deletion.
18 changes: 17 additions & 1 deletion src/compiler/machine-operator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,6 @@ MachineType AtomicExchangeRepresentationOf(Operator const* op) {
V(Word32Clz, Operator::kNoProperties, 1, 0, 1) \
V(Word64Clz, Operator::kNoProperties, 1, 0, 1) \
V(BitcastTaggedToWord, Operator::kNoProperties, 1, 0, 1) \
V(BitcastWordToTagged, Operator::kNoProperties, 1, 0, 1) \
V(BitcastWordToTaggedSigned, Operator::kNoProperties, 1, 0, 1) \
V(TruncateFloat64ToWord32, Operator::kNoProperties, 1, 0, 1) \
V(ChangeFloat32ToFloat64, Operator::kNoProperties, 1, 0, 1) \
Expand Down Expand Up @@ -607,6 +606,19 @@ struct MachineOperatorGlobalCache {
ATOMIC_TYPE_LIST(ATOMIC_EXCHANGE)
#undef ATOMIC_EXCHANGE

// The {BitcastWordToTagged} operator must not be marked as pure (especially
// not idempotent), because otherwise the splitting logic in the Scheduler
// might decide to split these operators, thus potentially creating live
// ranges of allocation top across calls or other things that might allocate.
// See https://bugs.chromium.org/p/v8/issues/detail?id=6059 for more details.
struct BitcastWordToTaggedOperator : public Operator {
BitcastWordToTaggedOperator()
: Operator(IrOpcode::kBitcastWordToTagged,
Operator::kEliminatable | Operator::kNoWrite,
"BitcastWordToTagged", 1, 0, 0, 1, 0, 0) {}
};
BitcastWordToTaggedOperator kBitcastWordToTagged;

struct DebugBreakOperator : public Operator {
DebugBreakOperator()
: Operator(IrOpcode::kDebugBreak, Operator::kNoThrow, "DebugBreak", 0,
Expand Down Expand Up @@ -784,6 +796,10 @@ const Operator* MachineOperatorBuilder::UnsafePointerAdd() {
return &cache_.kUnsafePointerAdd;
}

const Operator* MachineOperatorBuilder::BitcastWordToTagged() {
return &cache_.kBitcastWordToTagged;
}

const Operator* MachineOperatorBuilder::DebugBreak() {
return &cache_.kDebugBreak;
}
Expand Down

0 comments on commit 64fbb30

Please sign in to comment.