Skip to content

Commit

Permalink
Back out 3 [Implicit Context] diffs
Browse files Browse the repository at this point in the history
Reviewed By: jtwarren

Differential Revision: D57238835

fbshipit-source-id: 5faa6979c1f14698c2691bd198aeda9e030aac4d
  • Loading branch information
Bin Liu authored and pull[bot] committed May 22, 2024
1 parent 5b66bb8 commit f9000fe
Show file tree
Hide file tree
Showing 77 changed files with 1,089 additions and 534 deletions.
7 changes: 7 additions & 0 deletions hphp/doc/bytecode.specification
Original file line number Diff line number Diff line change
Expand Up @@ -3239,6 +3239,13 @@ SetImplicitContextByValue [C:?Obj] -> [C:?Obj]

Sets the implicit context to %1 and returns the previous implicit context.

VerifyImplicitContextState [] -> []

Verifies whether the current state of the implicit context is valid with
respect to the current calling context. Raises warning or throws an
exception if the state is invalid, set to soft implicit context or cleared.
Can only be used in memoized wrapper functions.

CreateSpecialImplicitContext [C:Int C:?Str] -> [C:?Obj]

Creates a special implicit context as if by calling the
Expand Down
6 changes: 6 additions & 0 deletions hphp/doc/ir.specification
Original file line number Diff line number Diff line change
Expand Up @@ -2318,6 +2318,12 @@ ReqBindJmp and CallFuncEntry.
Raises a warning to indicate a deployment boundary violation resulting from caller
attempting to call the the callee in S0 in context `ctx`.

| RaiseImplicitContextStateInvalid<func>, ND, NA, NF

Raises a warning or throws an exception for invalid, soft implicit context
or cleared states.
Can only be used in memoized wrapper functions.

| RaiseStrToClassNotice, ND, S(Str), NF

Raise a notice if a class is being loaded from a string e.g. in a `$c::f()`
Expand Down
19 changes: 14 additions & 5 deletions hphp/hack/src/hackc/emitter/emit_memoize_function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,8 +92,15 @@ pub(crate) fn emit_wrapper_function<'a, 'd>(
.iter()
.any(|tp| tp.reified.is_reified() || tp.reified.is_soft_reified());
let should_emit_implicit_context = hhbc::is_keyed_by_ic_memoize(attributes.iter());
// TODO: Also add coeffects in the decision to make ic inaccessible, implemented later in this stack
let should_make_ic_inaccessible: bool = !should_emit_implicit_context;
let is_make_ic_inaccessible_memoize = hhbc::is_make_ic_inaccessible_memoize(attributes.iter());
let is_soft_make_ic_inaccessible_memoize =
hhbc::is_soft_make_ic_inaccessible_memoize(attributes.iter());
let should_make_ic_inaccessible =
if is_make_ic_inaccessible_memoize || is_soft_make_ic_inaccessible_memoize {
Some(is_soft_make_ic_inaccessible_memoize)
} else {
None
};
let mut env = Env::default(Arc::clone(&fd.namespace)).with_scope(scope);
let (instrs, decl_vars) = make_memoize_function_code(
emitter,
Expand Down Expand Up @@ -143,7 +150,7 @@ fn make_memoize_function_code<'a, 'd>(
is_async: bool,
is_reified: bool,
should_emit_implicit_context: bool,
should_make_ic_inaccessible: bool,
should_make_ic_inaccessible: Option<bool>,
) -> Result<(InstrSeq, Vec<StringId>)> {
let (fun, decl_vars) = if hhas_params.is_empty() && !is_reified && !should_emit_implicit_context
{
Expand Down Expand Up @@ -184,7 +191,7 @@ fn make_memoize_function_with_params_code<'a, 'd>(
is_async: bool,
is_reified: bool,
should_emit_implicit_context: bool,
should_make_ic_inaccessible: bool,
should_make_ic_inaccessible: Option<bool>,
) -> Result<(InstrSeq, Vec<StringId>)> {
let param_count = hhas_params.len();
let notfound = e.label_gen_mut().next_regular();
Expand Down Expand Up @@ -253,6 +260,7 @@ fn make_memoize_function_with_params_code<'a, 'd>(
begin_label,
emit_body::emit_method_prolog(e, env, pos, hhas_params, ast_params, &[])?,
deprecation_body,
instr::verify_implicit_context_state(),
emit_memoize_helpers::param_code_sets(hhas_params.len(), Local::new(first_unnamed_idx)),
reified_memokeym,
ic_memokey,
Expand Down Expand Up @@ -300,7 +308,7 @@ fn make_memoize_function_no_params_code<'a, 'd>(
deprecation_info: Option<&[TypedValue]>,
renamed_id: hhbc::FunctionName,
is_async: bool,
should_make_ic_inaccessible: bool,
should_make_ic_inaccessible: Option<bool>,
) -> Result<(InstrSeq, Vec<StringId>)> {
let notfound = e.label_gen_mut().next_regular();
let suspended_get = e.label_gen_mut().next_regular();
Expand All @@ -319,6 +327,7 @@ fn make_memoize_function_no_params_code<'a, 'd>(
let ic_stash_local = Local::new(0);
let instrs = InstrSeq::gather(vec![
deprecation_body,
instr::verify_implicit_context_state(),
if is_async {
InstrSeq::gather(vec![
instr::memo_get_eager(notfound, suspended_get, LocalRange::EMPTY),
Expand Down
46 changes: 32 additions & 14 deletions hphp/hack/src/hackc/emitter/emit_memoize_helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,19 +74,37 @@ pub fn get_implicit_context_memo_key(local: Local) -> InstrSeq {
])
}

fn ic_set(local: Local) -> InstrSeq {
InstrSeq::gather(vec![
instr::cns_e(hhbc::ConstName::intern("HH\\MEMOIZE_IC_TYPE_INACCESSIBLE")),
instr::null(),
instr::create_special_implicit_context(),
instr::set_implicit_context_by_value(),
instr::set_l(local),
instr::pop_c(),
])
fn ic_set(local: Local, soft: bool) -> InstrSeq {
if soft {
InstrSeq::gather(vec![
instr::cns_e(hhbc::ConstName::intern(
"HH\\MEMOIZE_IC_TYPE_SOFT_INACCESSIBLE",
)),
instr::null(),
instr::create_special_implicit_context(),
instr::set_implicit_context_by_value(),
instr::set_l(local),
instr::pop_c(),
])
} else {
InstrSeq::gather(vec![
instr::null_uninit(),
instr::null_uninit(),
instr::f_call_func_d(
FCallArgs::new(FCallArgsFlags::default(), 1, 0, vec![], vec![], None, None),
hhbc::FunctionName::intern(
"HH\\ImplicitContext\\_Private\\create_ic_inaccessible_context",
),
),
instr::set_implicit_context_by_value(),
instr::set_l(local),
instr::pop_c(),
])
}
}

pub fn ic_restore(local: Local, should_make_ic_inaccessible: bool) -> InstrSeq {
if should_make_ic_inaccessible {
pub fn ic_restore(local: Local, should_make_ic_inaccessible: Option<bool>) -> InstrSeq {
if should_make_ic_inaccessible.is_some() {
InstrSeq::gather(vec![
instr::c_get_l(local),
instr::set_implicit_context_by_value(),
Expand All @@ -101,11 +119,11 @@ pub fn with_possible_ic(
label_gen: &mut LabelGen,
local: Local,
instrs: InstrSeq,
should_make_ic_inaccessible: bool,
should_make_ic_inaccessible: Option<bool>,
) -> InstrSeq {
if should_make_ic_inaccessible {
if let Some(soft) = should_make_ic_inaccessible {
InstrSeq::gather(vec![
ic_set(local),
ic_set(local, soft),
create_try_catch(
label_gen,
None,
Expand Down
29 changes: 24 additions & 5 deletions hphp/hack/src/hackc/emitter/emit_memoize_method.rs
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,14 @@ fn make_memoize_wrapper_method<'a, 'd>(
Flags::SHOULD_EMIT_IMPLICIT_CONTEXT,
hhbc::is_keyed_by_ic_memoize(attributes.iter()),
);
arg_flags.set(
Flags::SHOULD_MAKE_IC_INACCESSIBLE,
hhbc::is_make_ic_inaccessible_memoize(attributes.iter()),
);
arg_flags.set(
Flags::SHOULD_SOFT_MAKE_IC_INACCESSIBLE,
hhbc::is_soft_make_ic_inaccessible_memoize(attributes.iter()),
);
let mut args = Args {
info,
method,
Expand Down Expand Up @@ -356,9 +364,13 @@ fn make_memoize_method_with_params_code<'a, 'd>(
len: key_count.try_into().unwrap(),
};
let ic_stash_local = Local::new((key_count) as usize + first_unnamed_idx);
// TODO: add the coeffects to this decision, done in a later diff on this stack
let should_make_ic_inaccessible: bool =
!args.flags.contains(Flags::SHOULD_EMIT_IMPLICIT_CONTEXT);
let should_make_ic_inaccessible = if args.flags.contains(Flags::SHOULD_MAKE_IC_INACCESSIBLE)
|| args.flags.contains(Flags::SHOULD_SOFT_MAKE_IC_INACCESSIBLE)
{
Some(args.flags.contains(Flags::SHOULD_SOFT_MAKE_IC_INACCESSIBLE))
} else {
None
};
let instrs = InstrSeq::gather(vec![
begin_label,
emit_body::emit_method_prolog(emitter, env, pos, hhas_params, args.params, &[])?,
Expand All @@ -368,6 +380,7 @@ fn make_memoize_method_with_params_code<'a, 'd>(
} else {
instr::check_this()
},
instr::verify_implicit_context_state(),
emit_memoize_helpers::param_code_sets(hhas_params.len(), Local::new(first_unnamed_idx)),
reified_memokeym,
ic_memokey,
Expand Down Expand Up @@ -446,15 +459,21 @@ fn make_memoize_method_no_params_code<'a, 'd>(
None,
);
let ic_stash_local = Local::new(0);
// we are in a no parameter function that sets no zoned IC either, default to IC inaccessible
let should_make_ic_inaccessible: bool = true;
let should_make_ic_inaccessible = if args.flags.contains(Flags::SHOULD_MAKE_IC_INACCESSIBLE)
|| args.flags.contains(Flags::SHOULD_SOFT_MAKE_IC_INACCESSIBLE)
{
Some(args.flags.contains(Flags::SHOULD_SOFT_MAKE_IC_INACCESSIBLE))
} else {
None
};
let instrs = InstrSeq::gather(vec![
deprecation_body,
if args.method.static_ {
instr::empty()
} else {
instr::check_this()
},
instr::verify_implicit_context_state(),
if args.flags.contains(Flags::IS_ASYNC) {
InstrSeq::gather(vec![
instr::memo_get_eager(notfound, suspended_get, LocalRange::EMPTY),
Expand Down
1 change: 1 addition & 0 deletions hphp/hack/src/hackc/emitter/label_rewriter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -371,6 +371,7 @@ where
| Opcode::UnsetL(..)
| Opcode::UnsetM(..)
| Opcode::Vec(..)
| Opcode::VerifyImplicitContextState
| Opcode::VerifyOutType(..)
| Opcode::VerifyParamType(..)
| Opcode::VerifyParamTypeTS(..)
Expand Down
1 change: 1 addition & 0 deletions hphp/hack/src/hackc/ir/assemble/func.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1453,6 +1453,7 @@ impl FunctionParser<'_> {
"unreachable" => I::Terminator(T::Unreachable),
"unset" => self.parse_unset(tok, loc)?,
"unsetm" => self.parse_member_op(tok, mnemonic, loc)?,
"verify_implicit_context_state" => I::Hhbc(H::VerifyImplicitContextState(loc)),
"verify_out_type" => parse_instr!(tok, I::Hhbc(Hhbc::VerifyOutType(p0, p1, loc)), <p0:self.vid> "," <p1:self.lid>),
"verify_param_type" => parse_instr!(tok, I::Hhbc(Hhbc::VerifyParamType(p0, p1, loc)), <p0:self.vid> "," <p1:self.lid>),
"verify_param_type_ts" => parse_instr!(tok, I::Hhbc(Hhbc::VerifyParamTypeTS(p0, p1, loc)), <p0:self.vid> "," <p1:self.lid>),
Expand Down
1 change: 1 addition & 0 deletions hphp/hack/src/hackc/ir/conversions/bc_to_ir/instrs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1096,6 +1096,7 @@ fn convert_opcode(ctx: &mut Context<'_>, opcode: &Opcode) -> bool {
Opcode::UGetCUNop => todo!(),
Opcode::UnsetG => simple!(Hhbc::UnsetG),
Opcode::UnsetL => simple!(Hhbc::UnsetL),
Opcode::VerifyImplicitContextState => simple!(Hhbc::VerifyImplicitContextState),
Opcode::VerifyOutType => simple!(Hhbc::VerifyOutType),
Opcode::VerifyParamType => simple!(Hhbc::VerifyParamType),
Opcode::VerifyParamTypeTS => simple!(Hhbc::VerifyParamTypeTS),
Expand Down
1 change: 1 addition & 0 deletions hphp/hack/src/hackc/ir/conversions/ir_to_bc/emitter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -582,6 +582,7 @@ impl<'b> InstrEmitter<'b> {
let local = self.lookup_local(lid);
Opcode::UnsetL(local)
}
Hhbc::VerifyImplicitContextState(_) => Opcode::VerifyImplicitContextState,
Hhbc::VerifyOutType(_, pid, _) => {
let local = self.lookup_local(pid);
Opcode::VerifyOutType(local)
Expand Down
1 change: 1 addition & 0 deletions hphp/hack/src/hackc/ir/conversions/ir_to_bc/push_count.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ impl<'a> PushCount<'a> for instr::Hhbc {
| Hhbc::ThrowNonExhaustiveSwitch(_)
| Hhbc::UnsetG(..)
| Hhbc::UnsetL(..)
| Hhbc::VerifyImplicitContextState(_)
| Hhbc::VerifyParamTypeTS(..) => 0,

// --- 1 pushed value
Expand Down
4 changes: 0 additions & 4 deletions hphp/hack/src/hackc/ir/conversions/textual/hack.rs
Original file line number Diff line number Diff line change
Expand Up @@ -137,8 +137,6 @@ pub(crate) enum Hhbc {
Concat,
#[decl(fn hhbc_concat(...) -> *HackMixed)]
ConcatN,
#[decl(fn hhbc_create_special_implicit_context(*HackMixed) -> *HackMixed)]
CreateSpecialImplicitContext,
#[decl(fn hhbc_div(*HackMixed, *HackMixed) -> *HackMixed)]
Div,
#[decl(fn hhbc_exit(*HackMixed) -> noreturn)]
Expand Down Expand Up @@ -235,8 +233,6 @@ pub(crate) enum Hhbc {
Print,
#[decl(fn hhbc_record_reified_generic(*HackMixed) -> *HackMixed)]
RecordReifiedGeneric,
#[decl(fn hhbc_set_implicit_context_by_value(*HackMixed) -> *HackMixed)]
SetImplicitContextByValue,
#[decl(fn hhbc_shl(*HackMixed, *HackMixed) -> *HackMixed)]
Shl,
#[decl(fn hhbc_shr(*HackMixed, *HackMixed) -> *HackMixed)]
Expand Down
6 changes: 4 additions & 2 deletions hphp/hack/src/hackc/ir/conversions/textual/lower/instrs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -169,8 +169,6 @@ impl LowerInstrs<'_> {
Hhbc::Pow(..) => hack::Hhbc::Pow,
Hhbc::Print(..) => hack::Hhbc::Print,
Hhbc::RecordReifiedGeneric(..) => hack::Hhbc::RecordReifiedGeneric,
Hhbc::SetImplicitContextByValue(..) => hack::Hhbc::SetImplicitContextByValue,
Hhbc::CreateSpecialImplicitContext(..) => hack::Hhbc::CreateSpecialImplicitContext,
Hhbc::Shl(..) => hack::Hhbc::Shl,
Hhbc::Shr(..) => hack::Hhbc::Shr,
Hhbc::Sub(..) => hack::Hhbc::Sub,
Expand Down Expand Up @@ -786,6 +784,10 @@ impl TransformInstr for LowerInstrs<'_> {
Instr::Hhbc(Hhbc::VerifyRetTypeTS([obj, ts], loc)) => {
self.verify_ret_type_ts(builder, obj, ts, loc)
}
Instr::Hhbc(Hhbc::VerifyImplicitContextState(_)) => {
// no-op
Instr::tombstone()
}
Instr::Hhbc(Hhbc::LIterFree(id, loc)) => {
let lid = iter_var_name(id);
let value = builder.emit(Instr::Hhbc(Hhbc::CGetL(lid, loc)));
Expand Down
1 change: 1 addition & 0 deletions hphp/hack/src/hackc/ir/ir_core/instr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -556,6 +556,7 @@ pub enum Hhbc {
ThrowNonExhaustiveSwitch(LocId),
UnsetG(ValueId, LocId),
UnsetL(LocalId, LocId),
VerifyImplicitContextState(LocId),
VerifyOutType(ValueId, LocalId, LocId),
VerifyParamType(ValueId, LocalId, LocId),
VerifyParamTypeTS(ValueId, LocalId, LocId),
Expand Down
3 changes: 3 additions & 0 deletions hphp/hack/src/hackc/ir/print/print.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1235,6 +1235,9 @@ fn print_hhbc(w: &mut dyn Write, ctx: &FuncContext, func: &Func, hhbc: &Hhbc) ->
Hhbc::UnsetL(lid, _) => {
write!(w, "unset {}", FmtLid(lid))?;
}
Hhbc::VerifyImplicitContextState(_) => {
write!(w, "verify_implicit_context_state")?;
}
Hhbc::VerifyOutType(vid, lid, _) => {
write!(
w,
Expand Down
1 change: 1 addition & 0 deletions hphp/hack/src/hackc/sem_diff/local_info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,7 @@ impl LocalInfo {
| Opcode::UnsetG
| Opcode::UnsetM(..)
| Opcode::Vec(..)
| Opcode::VerifyImplicitContextState
| Opcode::VerifyOutType(..)
| Opcode::VerifyParamType(..)
| Opcode::VerifyParamTypeTS(..)
Expand Down
3 changes: 2 additions & 1 deletion hphp/hack/src/hackc/sem_diff/sequence.rs
Original file line number Diff line number Diff line change
Expand Up @@ -354,7 +354,8 @@ fn is_cow_instr(instr: &NodeInstr) -> bool {

// Verify
NodeInstr::Opcode(
Opcode::VerifyOutType(..)
Opcode::VerifyImplicitContextState
| Opcode::VerifyOutType(..)
| Opcode::VerifyParamType(..)
| Opcode::VerifyParamTypeTS(..)
| Opcode::VerifyRetNonNullC
Expand Down
4 changes: 3 additions & 1 deletion hphp/hack/src/hackc/sem_diff/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1323,7 +1323,8 @@ fn is_checkpoint_instr(instr: &NodeInstr) -> bool {

// Verify Instructions
NodeInstr::Opcode(
Opcode::VerifyOutType(..)
Opcode::VerifyImplicitContextState
| Opcode::VerifyOutType(..)
| Opcode::VerifyParamType(..)
| Opcode::VerifyParamTypeTS(..)
| Opcode::VerifyRetNonNullC
Expand Down Expand Up @@ -1653,6 +1654,7 @@ fn clean_opcode(opcode: &Opcode) -> Opcode {
| Opcode::True
| Opcode::UGetCUNop
| Opcode::UnsetG
| Opcode::VerifyImplicitContextState
| Opcode::VerifyRetNonNullC
| Opcode::VerifyRetTypeC
| Opcode::VerifyRetTypeTS
Expand Down
12 changes: 1 addition & 11 deletions hphp/hack/src/hackc/test/infer/foreach.hack
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

// TEST-CHECK-BAL: define $root.check_foreach
// CHECK: define $root.check_foreach($this: *void, $x: .notnull *HackVec) : *void {
// CHECK: local $index: *void, iter0: *void, $0: *void
// CHECK: local $index: *void, iter0: *void
// CHECK: #b0:
// CHECK: n0 = $builtins.hack_new_dict($builtins.hack_string("kind"), $builtins.hack_int(20), $builtins.hack_string("generic_types"), $builtins.hhbc_new_vec($builtins.hack_new_dict($builtins.hack_string("kind"), $builtins.hack_int(4))))
// CHECK: n1: *HackMixed = load &$x
Expand All @@ -15,7 +15,6 @@
// CHECK: n5: *HackMixed = load &$0
// CHECK: n6 = $builtins.hhbc_iter_init(&iter0, null, &$index, n5)
// CHECK: jmp b2, b7
// CHECK: .handlers b9
// CHECK: #b2:
// CHECK: prune $builtins.hack_is_true(n6)
// CHECK: jmp b3
Expand All @@ -31,7 +30,6 @@
// CHECK: n13: *HackMixed = load &iter0
// CHECK: n14 = $builtins.hhbc_liter_free(n13)
// CHECK: throw n12
// CHECK: .handlers b9
// CHECK: #b5:
// CHECK: prune $builtins.hack_is_true(n11)
// CHECK: jmp b8
Expand All @@ -42,14 +40,6 @@
// CHECK: prune ! $builtins.hack_is_true(n6)
// CHECK: jmp b8
// CHECK: #b8:
// CHECK: jmp b10
// CHECK: .handlers b9
// CHECK: #b9(n15: *HackMixed):
// CHECK: store &$0 <- null: *HackMixed
// CHECK: throw n15
// CHECK: #b10:
// CHECK: store &$0 <- null: *HackMixed
// CHECK: store &$0 <- null: *HackMixed
// CHECK: ret null
// CHECK: }
function check_foreach(vec<string> $x): void {
Expand Down
Loading

0 comments on commit f9000fe

Please sign in to comment.