Closed Bug 542913 Opened 15 years ago Closed 13 years ago

It would sure be handy to be able to specify verbose on a per-method basis

Categories

(Tamarin Graveyard :: Virtual Machine, defect, P3)

defect

Tracking

(Not tracked)

VERIFIED FIXED
Q4 11 - Anza

People

(Reporter: stejohns, Assigned: rreitmai)

References

Details

Attachments

(2 files, 1 obsolete file)

Tracking down verifier or jit bugs can mean sifting thru hundreds of megabytes of output. We should have a way to specify verbosity on a more granular level.
Attached patch Rough patch (obsolete) (deleted) — Splinter Review
Here's an extremely rough, quick&dirty patch that helps a bit: add a global that we can check in MethodInfo::verify, and for matches, temporarily hack the bits. -- There's no way to set this on the commandline yet, you just have to edit avmcore.cpp -- always compiled in for AVMPLUS_VERBOSE mode, which may be overkill I don't know if this patch should ever actually land, but I've found myself re-creating similar hacks several times now for debugging purposes, so something like it is useful.
Attachment #424149 - Flags: review?(rreitmai)
once upon a time it was possible to enable verbose on a specific method using [metadata], however its inconvenient when all you have is a binary to work with.
Flags: flashplayer-qrb+
Priority: -- → P3
Target Milestone: --- → flash10.2
Comment on attachment 424149 [details] [diff] [review] Rough patch marking - for now since this is a good start, but I think with a few tweaks we can wire it up to a command line switch. Maybe say -Dverboseonly="method1,method2,...,methodN" and then have pool->isVerbose() do a check of both the bit in question and the name(s)?
Attachment #424149 - Flags: review?(rreitmai) → review-
Flags: flashplayer-bug-
Target Milestone: Q3 11 - Serrano → Q4 11 - Anza
Assignee: nobody → stejohns
Attached patch ver1 (deleted) — Splinter Review
Provide -Dverbose-only switch which is used to specify methods for which output is enabled. The code is partially based off bug 528375 which introduced MethodRecognizer; a mechanism and syntax for identifying methods. E.g. the following will emit interp and jit verbose output only for methods that contain 'global' in their name or the method named 'Function/prime_val.as$1:prime'. -Dverbose=interp,execpolicy,jit -Dverbose-only "Function/prime_val.as\$1:prime,?global"
Attachment #424149 - Attachment is obsolete: true
Attachment #528491 - Flags: review?(stejohns)
Comment on attachment 528491 [details] [diff] [review] ver1 Review of attachment 528491 [details] [diff] [review]: R+ with nits about comments fixed. ::: core/AvmCore.cpp @@ +274,5 @@ + } + + /** + * If verbosity is restricted per-method, then scan each MethodRecognizer seeing + MethodRecognizer* r = MethodRecognizer::parse(&s, ','); seeing seeing? ::: core/AvmCore.h @@ +103,5 @@ + * whether verbose output is to occur on a given method + * or not. If this list is empty then verbosity is enabled + * for all methods (assuming its on). + */ + * A list of MethodRecognizers that is used to determine comment above is bogus, please fix
Attachment #528491 - Flags: review?(stejohns) → review+
Depends on: 528375
Potential bug here: bool AvmCore::isVerbose(uint32_t b, MethodInfo* info) { bool v = b & config.verbose_vb; <<--------typecasting uint32_t to bool
(In reply to comment #6) > Potential bug here: > bool v = b & config.verbose_vb; <<--------typecasting uint32_t to bool or at least a compiler warning...updated local patch.
changeset: 6312:73725c174dd8 user: Rick Reitmaier <rreitmai@adobe.com> summary: Bug 542913 - It would sure be handy to be able to specify verbose on a per-method basis http://hg.mozilla.org/tamarin-redux/rev/73725c174dd8
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
(In reply to comment #8) > changeset: 6312:73725c174dd8 > user: Rick Reitmaier <rreitmai@adobe.com> > summary: Bug 542913 - It would sure be handy to be able to specify verbose > on a per-method basis > > http://hg.mozilla.org/tamarin-redux/rev/73725c174dd8 Reopening this bug since Valgrind is reporting issues with this change: >$ ../configure.py --target=i686-darwin --enable-debugger --enable-valgrind >$ valgrind shell/avmshell ../test/acceptance/as3/AbcDecoder/simple.abc ==70482== Conditional jump or move depends on uninitialised value(s) ==70482== at 0x4C746: MMgc::GC::TraceConservativePointer(unsigned long, bool) (GC.cpp:2600) ==70482== by 0x4C629: MMgc::GC::MarkItem_ConservativeOrNonGCObject(void const*, unsigned int, MMgc::GCMarkStack::TypeTag, void const*, bool) (GC.cpp:2582) ==70482== by 0x4DFF8: MMgc::GC::MarkRoots(bool, bool) (GC.cpp:2100) ==70482== by 0x4E088: MMgc::GC::MarkNonstackRoots(bool) (GC.cpp:2049) ==70482== by 0x4F77B: MMgc::GC::StartIncrementalMark() (GC.cpp:2019) ==70482== by 0x50A68: MMgc::GC::CollectionWork() (GC.cpp:632) ==70482== by 0x52CCF: MMgc::GCAlloc::Alloc(unsigned long, int) (GC-inlines.h:95) ==70482== by 0x4EFD6: MMgc::GC::Alloc(unsigned long, int) (GC.cpp:772) ==70482== by 0xE3091: avmplus::NamespaceSet::create(MMgc::GC*, avmplus::Namespace*) (GC-inlines.h:145) ==70482== by 0xF7DA3: avmplus::Traits::buildBindings(avmplus::TraitsBindings const*, avmplus::MultinameHashtable<avmplus::Binding_*, avmplus::BindingType>*, unsigned int&, unsigned int&, avmplus::Traits::SlotSizeInfo*, avmplus::Toplevel const*) const (Traits.cpp:791) ==70482== by 0xF84AF: avmplus::Traits::_buildTraitsBindings(avmplus::Toplevel const*, bool) (Traits.cpp:1310) ==70482== by 0xF9B23: avmplus::Traits::resolveSignaturesSelf(avmplus::Toplevel const*) (Traits.cpp:1545)
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Lars/Tommy: Actually I think the valgrind errors (after looking at the stack trace) appears to be an issue in the GC (possibly). The code in TraceConservativePointer() does have a line to inform Valgrind of the memory but is conditional on 'handleInteriorPtrs".
Seems like there's a field of a root that's uninitialized and valgrind complains. But the error being reported here looks more like an error in pageMap, no? If there were problems with val that should have been signaled when it was read. All that said, the condition "handleInteriorPtrs" does not make sense to me in this context, not sure why that's here. The interior pointer handling code does not read the target object.
Found another issue with this patch. In the build system we run the acceptance suite with the following switches: -Dverifyall -Dverbose=verify Now if you run with these switches the shell crashes: $> avmshell -Dverifyall -Dverbose=verify as3/AbcDecoder/simple.abc Program received signal EXC_BAD_ACCESS, Could not access memory. Reason: 13 at address: 0x0000000000000000 0x00000001000a6445 in avmplus::parseVerboseRestrictions (s=0x96315a8e900873e5 <Address 0x96315a8e900873e5 out of bounds>, list=0x1010209b0, console=@0x101020098) at ../core/AvmCore.cpp:275 275 while(*s) (gdb) bt #0 0x00000001000a6445 in avmplus::parseVerboseRestrictions (s=0x96315a8e900873e5 <Address 0x96315a8e900873e5 out of bounds>, list=0x1010209b0, console=@0x101020098) at ../core/AvmCore.cpp:275 #1 0x00000001000a64e3 in avmplus::AvmCore::isVerbose (this=0x101020008, b=536870912, info=0x10124c798) at ../core/AvmCore.cpp:302 #2 0x0000000100148da5 in avmplus::PoolObject::isVerbose (this=0x10103e3a8, flag=536870912, info=0x10124c798) at PoolObject-inlines.h:146 #3 0x00000001001551e5 in avmplus::Verifier::Verifier (this=0x7fff5fbfeb50, info=0x10124c798, ms=0x101256408, toplevel=0x101032f58, abc_env=0x101222480, secondTry=false) at ../core/Verifier.cpp:78 #4 0x00000001000fb60d in avmplus::BaseExecMgr::verifyCommon (this=0x101034098, m=0x10124c798, ms=0x101256408, toplevel=0x101032f58, abc_env=0x101222480, coder=0x7fff5fbfeca0) at ../core/exec.cpp:415 #5 0x00000001000fb932 in avmplus::BaseExecMgr::verifyInterp (this=0x101034098, m=0x10124c798, ms=0x101256408, toplevel=0x101032f58, abc_env=0x101222480) at ../core/exec.cpp:379 #6 0x00000001000fbaa2 in avmplus::BaseExecMgr::verifyMethod (this=0x101034098, m=0x10124c798, toplevel=0x101032f58, abc_env=0x101222480) at ../core/exec.cpp:361 #7 0x00000001000fed98 in avmplus::BaseExecMgr::verifyEarly (this=0x101034098, toplevel=0x101032f58, abc_env=0x101222480) at ../core/exec-verifyall.cpp:160 #8 0x00000001000ff050 in avmplus::BaseExecMgr::notifyAbcPrepared (this=0x101034098, toplevel=0x101032f58, abcEnv=0x101222480) at ../core/exec-verifyall.cpp:53 #9 0x00000001000a4afa in avmplus::AvmCore::initAllScripts (this=0x101020008, toplevel=0x101032f58, abcEnv=0x101222480) at ../core/AvmCore.cpp:795 #10 0x00000001000a6d21 in avmplus::AvmCore::handleActionPool (this=0x101020008, pool=0x10103e3a8, toplevel=0x101032f58, codeContext=0x10112c090) at ../core/AvmCore.cpp:871 #11 0x00000001000a6e35 in avmplus::AvmCore::handleActionBlock (this=0x101020008, code=@0x7fff5fbff130, start=0, toplevel=0x101032f58, ninit=0x0, codeContext=0x10112c090, apiVersion=avmplus::kApiVersion_SWF_13) at ../core/AvmCore.cpp:946 #12 0x000000010003d96a in avmshell::ShellCore::handleArbitraryExecutableContent (this=0x101020008, settings=@0x7fff5fbff4a0, code=@0x7fff5fbff1f0, filename=0x7fff5fbff813 "as3/AbcDecoder/simple.abc") at ../shell/ShellCore.cpp:562 #13 0x000000010003ded7 in avmshell::ShellCore::evaluateFile (this=0x101020008, settings=@0x7fff5fbff4a0, filename=0x7fff5fbff813 "as3/AbcDecoder/simple.abc") at ../shell/ShellCore.cpp:539 #14 0x0000000100031fb2 in avmshell::Shell::singleWorkerHelper (shell=0x101020008, settings=@0x7fff5fbff4a0) at ../shell/avmshell.cpp:215 #15 0x00000001000327db in avmshell::Shell::singleWorker (settings=@0x7fff5fbff4a0) at ../shell/avmshell.cpp:174 #16 0x0000000100032a6d in avmshell::Shell::run (argc=4, argv=0x7fff5fbff688) at ../shell/avmshell.cpp:141 #17 0x000000010005ab40 in main (argc=4, argv=0x7fff5fbff688) at ../shell/avmshellMac.cpp:114
(In reply to comment #12) > Found another issue with this patch. In the build system we run the > acceptance suite with the following switches: > > -Dverifyall -Dverbose=verify > Now if you run with these switches the shell crashes: > > $> avmshell -Dverifyall -Dverbose=verify as3/AbcDecoder/simple.abc > > Program received signal EXC_BAD_ACCESS, Could not access memory. > Reason: 13 at address: 0x0000000000000000 > 0x00000001000a6445 in avmplus::parseVerboseRestrictions > (s=0x96315a8e900873e5 <Address 0x96315a8e900873e5 out of bounds>, > list=0x1010209b0, console=@0x101020098) at ../core/AvmCore.cpp:275 > 275 while(*s) > (gdb) bt > #0 0x00000001000a6445 in avmplus::parseVerboseRestrictions > (s=0x96315a8e900873e5 <Address 0x96315a8e900873e5 out of bounds>, > list=0x1010209b0, console=@0x101020098) at ../core/AvmCore.cpp:275 > #1 0x00000001000a64e3 in avmplus::AvmCore::isVerbose (this=0x101020008, > b=536870912, info=0x10124c798) at ../core/AvmCore.cpp:302 > #2 0x0000000100148da5 in avmplus::PoolObject::isVerbose (this=0x10103e3a8, > flag=536870912, info=0x10124c798) at PoolObject-inlines.h:146 I am pretty sure this is because verboseOnlyArg needs to initialized to NULL in ShellCoreSettings. I mentioned this to Rick yesterday, but I think he might have thought I was talking about verboseOnlyString member in Config, which is different, and gets overwritten by verboseOnlyArg in ShellCore::setup.
(fix for issue in comment 12 and comment 13.)
Attachment #533974 - Flags: review?(rreitmai)
Comment on attachment 533974 [details] [diff] [review] fix uninit'ed ShellCoreSettings.verboseOnlyArg Ah I see now thanks for picking this up Felix and super nice that Valgrind found it.
Attachment #533974 - Flags: review?(rreitmai) → review+
changeset: 6325:b5ac2bcbe4a9 user: Felix S Klock II <fklockii@adobe.com> summary: Bug 542913: fix uninited ShellCoreSettings.verboseOnlyArg (r=rreitmai). http://hg.mozilla.org/tamarin-redux/rev/b5ac2bcbe4a9
(In reply to comment #15) > Comment on attachment 533974 [details] [diff] [review] [review] > fix uninit'ed ShellCoreSettings.verboseOnlyArg > > Ah I see now thanks for picking this up Felix and super nice that Valgrind > found it. (I wasn't using Valgrind; I'm currently assuming the Valgrind problem is something else entirely.)
(In reply to comment #17) > (In reply to comment #15) > > Comment on attachment 533974 [details] [diff] [review] [review] [review] > > fix uninit'ed ShellCoreSettings.verboseOnlyArg > > > > Ah I see now thanks for picking this up Felix and super nice that Valgrind > > found it. > > (I wasn't using Valgrind; I'm currently assuming the Valgrind problem is > something else entirely.) Actually appears that valgrind is now happy with how the current code is. Valgrind no longer complains with tr rev 6325.
Marking resolved/fixed now that Felix, Brent, and Valgrind are happy.
Assignee: stejohns → rreitmai
Status: REOPENED → RESOLVED
Closed: 13 years ago13 years ago
Resolution: --- → FIXED
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: