From cacdda822a7ddfc1eb421d60733f1c64fcc0403d Mon Sep 17 00:00:00 2001 From: Alexey Toptygin Date: Wed, 28 Mar 2018 16:46:38 -0700 Subject: [PATCH] Remove anonymous class ids from hhas. Summary: We had the unique anonymous class id in the hhas format, but we dindn't use it: as.cpp would unconditionally renumber them. hphpc and hackc would not always produce the same ids, so semdiff had to ignore the differences, but it was only ignoring these differences on closure classes, not proper anonymous classes. Simplify things by not writing these unused ids from either hphpc or hackc, make the ocaml demangler forget about them, and make as.cpp reject units where they're present. Reviewed By: oulgen Differential Revision: D7420745 fbshipit-source-id: 92fc29cfa4d18fef4cb9b850eaec2cb86e5b7c08 --- hphp/hack/src/hhbc/closure_convert.ml | 19 ++++++------------- hphp/hack/src/hhbc/hhbc_string_utils.ml | 21 ++++++++------------- hphp/runtime/vm/as.cpp | 8 ++++---- hphp/runtime/vm/disas.cpp | 10 +++++++++- hphp/test/quick/asm_extnames.hhas | 2 +- hphp/test/slow/anon_class/invalid_class_name.hhas | 7 +++++++ .../slow/anon_class/invalid_class_name.hhas.expectf | 1 + hphp/test/slow/closure/invalid-class-name.hhas | 11 +++++++++++ .../slow/closure/invalid-class-name.hhas.expectf | 1 + 9 files changed, 48 insertions(+), 32 deletions(-) create mode 100644 hphp/test/slow/anon_class/invalid_class_name.hhas create mode 100644 hphp/test/slow/anon_class/invalid_class_name.hhas.expectf create mode 100644 hphp/test/slow/closure/invalid-class-name.hhas create mode 100644 hphp/test/slow/closure/invalid-class-name.hhas.expectf diff --git a/hphp/hack/src/hhbc/closure_convert.ml b/hphp/hack/src/hhbc/closure_convert.ml index df0fefe6d0e..ad9ed46efd8 100644 --- a/hphp/hack/src/hhbc/closure_convert.ml +++ b/hphp/hack/src/hhbc/closure_convert.ml @@ -74,8 +74,6 @@ type state = { (* Free variables computed so far *) captured_vars : ULS.t; captured_this : bool; - (* Total number of closures or anonymous classes created *) - total_count : int; (* Closure classes and hoisted inline classes *) hoisted_classes : class_ list; (* Hoisted inline functions *) @@ -137,7 +135,6 @@ let initial_state = anon_cls_cnt_per_fun = 0; captured_vars = ULS.empty; captured_this = false; - total_count = 0; hoisted_classes = []; hoisted_functions = []; inout_wrappers = []; @@ -354,19 +351,18 @@ let add_class env st cd = { st with hoisted_classes = cd :: st.hoisted_classes }, make_defcls cd n -let make_closure_name total_count env st = +let make_closure_name env st = let per_fun_idx = st.closure_cnt_per_fun in SU.Closures.mangle_closure - (make_scope_name st.namespace env.scope) per_fun_idx total_count + (make_scope_name st.namespace env.scope) per_fun_idx let make_anonymous_class_name env st = let per_fun_idx = st.anon_cls_cnt_per_fun + 1 in - let per_scope_idx = st.total_count in SU.Classes.mangle_anonymous_class - (make_scope_name st.namespace env.scope) per_fun_idx per_scope_idx + (make_scope_name st.namespace env.scope) per_fun_idx let make_closure ~class_num - p total_count env st lambda_vars tparams is_static fd body = + p env st lambda_vars tparams is_static fd body = let md = { m_kind = [Public] @ (if is_static then [Static] else []); m_tparams = fd.f_tparams; @@ -393,7 +389,7 @@ let make_closure ~class_num c_final = false; c_kind = Cnormal; c_is_xhp = false; - c_name = (p, make_closure_name total_count env st); + c_name = (p, make_closure_name env st); c_tparams = tparams; c_extends = [(p, Happly((p, "Closure"), []))]; c_implements = []; @@ -639,7 +635,6 @@ let rec convert_expr env st (p, expr_ as expr) = let st, cls = convert_class env st cls in let st = { st with anon_cls_cnt_per_fun = st.anon_cls_cnt_per_fun + 1; - total_count = st.total_count + 1; hoisted_classes = cls :: st.hoisted_classes } in st, (p, NewAnonClass (args, varargs, cls_condensed)) | Efun (fd, use_vars) -> @@ -697,10 +692,8 @@ and convert_lambda env st p fd use_vars_opt = (* Remember the current capture and defined set across the lambda *) let captured_vars = st.captured_vars in let captured_this = st.captured_this in - let total_count = st.total_count in let static_vars = st.static_vars in let old_function_state = st.current_function_state in - let st = { st with total_count = total_count + 1; } in let st = enter_lambda st in let old_env = env in Option.iter use_vars_opt @@ -760,7 +753,7 @@ and convert_lambda env st p fd use_vars_opt = let inline_fundef, cd, md = make_closure ~class_num - p total_count env st lambda_vars tparams is_static fd block in + p env st lambda_vars tparams is_static fd block in let explicit_use_set = if Option.is_some use_vars_opt then SSet.add (snd inline_fundef.f_name) st.explicit_use_set diff --git a/hphp/hack/src/hhbc/hhbc_string_utils.ml b/hphp/hack/src/hhbc/hhbc_string_utils.ml index 10bd5a3d0ed..a5b7c5a9a6c 100644 --- a/hphp/hack/src/hhbc/hhbc_string_utils.ml +++ b/hphp/hack/src/hhbc/hhbc_string_utils.ml @@ -121,11 +121,10 @@ end module Classes = struct - let mangle_class prefix scope ix count = + let mangle_class prefix scope ix = prefix ^ "$" ^ scope ^ (if ix = 1 then "" else "#" ^ string_of_int ix) - ^ ";" ^ string_of_int count (* Anonymous classes have names of the form * class@anonymous$ scope ix ; num @@ -137,8 +136,8 @@ module Classes = struct * ix ::= * # *) - let mangle_anonymous_class scope ix count = - mangle_class "class@anonymous" scope ix count + let mangle_anonymous_class scope ix = + mangle_class "class@anonymous" scope ix let is_anonymous_class_name n = String_utils.string_starts_with n "class@anonymous" @@ -162,17 +161,13 @@ module Closures = struct if is_closure_name s then let suffix = String_utils.lstrip s "Closure$" in - match Str.split_delim (Str.regexp ";") suffix with - | [prefix; _count] -> - begin match Str.split_delim (Str.regexp "#") prefix with - | [prefix; _] -> Some prefix - | _ -> Some prefix - end - | _ -> None + match Str.split_delim (Str.regexp "#") suffix with + | [prefix; _] -> Some prefix + | _ -> Some suffix else None - let mangle_closure scope ix count = - Classes.mangle_class "Closure" scope ix count + let mangle_closure scope ix = + Classes.mangle_class "Closure" scope ix end (* XHP name mangling *) diff --git a/hphp/runtime/vm/as.cpp b/hphp/runtime/vm/as.cpp index 6006f81ea72..d3fcd506f97 100644 --- a/hphp/runtime/vm/as.cpp +++ b/hphp/runtime/vm/as.cpp @@ -2871,13 +2871,13 @@ void parse_class(AsmState& as) { as.error(".class must have a name"); } if (ParserBase::IsAnonymousClassName(name)) { - // refresh names of anonymous classes - // to make sure they are unique + // assign unique numbers to anonymous classes + // they must not be pre-numbered in the hhas auto p = name.find(';'); if (p != std::string::npos) { - name = name.substr(0, p); - name = HPHP::NewAnonymousClassName(name); + as.error("anonymous class and closure names may not contain ids in hhas"); } + name = HPHP::NewAnonymousClassName(name); } int line0; diff --git a/hphp/runtime/vm/disas.cpp b/hphp/runtime/vm/disas.cpp index 034cbb49e3a..06a267f0c76 100644 --- a/hphp/runtime/vm/disas.cpp +++ b/hphp/runtime/vm/disas.cpp @@ -23,6 +23,7 @@ #include +#include "hphp/parser/parser.h" #include "hphp/runtime/base/repo-auth-type-array.h" #include "hphp/runtime/base/repo-auth-type-codec.h" #include "hphp/runtime/base/variable-serializer.h" @@ -772,10 +773,17 @@ void print_cls_directives(Output& out, const PreClass* cls) { void print_cls(Output& out, const PreClass* cls) { out.indent(); + auto name = cls->name()->toCppString(); + if (ParserBase::IsAnonymousClassName(name)) { + auto p = name.find(';'); + if (p != std::string::npos) { + name = name.substr(0, p); + } + } out.fmt(".class{} {}", opt_attrs(AttrContext::Class, cls->attrs(), &cls->userAttributes(), cls->hoistability() != PreClass::NotHoistable), - cls->name(), + name, format_line_pair(cls)); print_cls_inheritance_list(out, cls); out.fmt(" {{"); diff --git a/hphp/test/quick/asm_extnames.hhas b/hphp/test/quick/asm_extnames.hhas index d209dabcd61..17c2c86e49d 100644 --- a/hphp/test/quick/asm_extnames.hhas +++ b/hphp/test/quick/asm_extnames.hhas @@ -12,7 +12,7 @@ } -.class Uh$Foo::Bar;42 { +.class Uh$Foo::Bar { .default_ctor; .method [public static] print$foo() { String "hi\n" diff --git a/hphp/test/slow/anon_class/invalid_class_name.hhas b/hphp/test/slow/anon_class/invalid_class_name.hhas new file mode 100644 index 00000000000..a60b3c2b994 --- /dev/null +++ b/hphp/test/slow/anon_class/invalid_class_name.hhas @@ -0,0 +1,7 @@ +.main (1,1) { + Int 1 + RetC +} + +.class [nontop] class@anonymous$foo;2 { +} diff --git a/hphp/test/slow/anon_class/invalid_class_name.hhas.expectf b/hphp/test/slow/anon_class/invalid_class_name.hhas.expectf new file mode 100644 index 00000000000..8253f471a2b --- /dev/null +++ b/hphp/test/slow/anon_class/invalid_class_name.hhas.expectf @@ -0,0 +1 @@ +Fatal error: Assembler Error: line 6: anonymous class and closure names may not contain ids in hhas in %s/invalid_class_name.hhas on line -1 diff --git a/hphp/test/slow/closure/invalid-class-name.hhas b/hphp/test/slow/closure/invalid-class-name.hhas new file mode 100644 index 00000000000..0e521a802b5 --- /dev/null +++ b/hphp/test/slow/closure/invalid-class-name.hhas @@ -0,0 +1,11 @@ +.main { + Int 1 + RetC +} + +.class [unique] Closure$foo;1 extends Closure { + .method [public static] (3,3) <"" N > __invoke() isClosureBody { + Null + RetC + } +} diff --git a/hphp/test/slow/closure/invalid-class-name.hhas.expectf b/hphp/test/slow/closure/invalid-class-name.hhas.expectf new file mode 100644 index 00000000000..00f438ca7d6 --- /dev/null +++ b/hphp/test/slow/closure/invalid-class-name.hhas.expectf @@ -0,0 +1 @@ +Fatal error: Assembler Error: line 6: anonymous class and closure names may not contain ids in hhas in %s/invalid-class-name.hhas on line -1 -- 2.11.4.GIT