We want -Dverifyall to be a true early-verify mode. Therefore MethodInfo.verifyEnter() and NativeFunction.verifyEnter() should never be called at runtime. in builds without a jit, it should be possible for MethodInfo/Env->impl32 to be const. Currently, verifyall is eager but not 100% eager, it is still triggered by a call to verifyEnter of a root method (a script init).
Created attachment 358372 [details] [diff] [review] add assert(false) to lazy-verify, eager-verify main in verifyEnter, if config.verifyall is set, then assert(false). Kick off eager verification by verifying the main method just before we call it. this successfully runs simple abc's without asserting. but: - we want to verify earlier than just before we call the first method - it doesn't directly handle non-entry point scripts in the abc. if they aren't referenced by the main script they might not be early verified. posted here for early review
Created attachment 358397 [details] [diff] [review] more thorough eager verification, tested on esc repl - renamed enq functions to enqFunction and enqTraits - renamed processVerifyQueue to verifyEarly() - removed dead code from MethodInfo/NativeMethod.verifyEnter(). only assert when config.verifyall enabled - enqueue activationObject traits when OP_newactivation is seen; some activation objects have generated init methods, which need early verification too. - process script traits and init methods in prepareActionPool, before any init methods are called. Tested this on esc compiling parse.es, as well as esc's REPL. Should be a good test because it involves loading multiple abc's on the shell command line, processing abc's with multiple scripts (builtin & shell.abc), as well as early-verifying dynamically loaded abc's (esc compiles on the fly). Uncomment the print statement near the bottom of AvmCore.cpp to see the method names printed in the order they are verified, e.g. $ shell/avmshell -Dverifyall -Dinterp <esc files> main.es.abc -- ... pre verify ESC::Flags$cinit() pre verify compile() pre verify global$init() pre verify evaluateInScopeArray() pre verify global$init() pre verify <anonymous ../src/esc-env.es:56>() pre verify global$init() pre verify repl() pre verify ?() pre verify eval() es> print("Hello World") pre verify global$init() Hello World es>
BTW, using this to verify the shell toplevel code there are two unverified functions, but I have not looked into it. Otherwise this fix works very well for me.
i'll investigate, if theres a bug i'll fix it before pushing. if those 2 methods really are unreachable, that'll be interesting too.
From what i can tell using said verbose mode and abcdump, all the builtin, shell, and user.abc methodinfo's are getting verified. point me to a test case? otherwise i think this is ready to go.. will update and push soon.
You should just push it, I will investigate separately.
Status: NEW → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
QE: Should look into adding the test that Edwin mentions in comment #2, running esc with -Dverifyall
Created attachment 409114 [details] [diff] [review] Add --verify switch to runtests This patch adds --verify to runtests. When enabled, this will compare the # of functions found with abcdump vs. the number of functions found with "-Dverifyall -Dverbose=verify" and produce an error if the # of functions differ.
Created attachment 409179 [details] [diff] [review] Updated patch - Check and build abcdump.abc - Remove need for recompiling .abcs - Set vmargs so they are part of config
Test changes pushed redux changeset 2954 7a936553b2e9
Flags: in-testsuite? → in-testsuite+
You need to log in before you can comment on or make changes to this bug.