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)
Tamarin Graveyard
Virtual Machine
Tracking
(Not tracked)
VERIFIED
FIXED
Q4 11 - Anza
People
(Reporter: stejohns, Assigned: rreitmai)
References
Details
Attachments
(2 files, 1 obsolete file)
(deleted),
patch
|
stejohns
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
rreitmai
:
review+
|
Details | Diff | Splinter Review |
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.
Reporter | ||
Comment 1•15 years ago
|
||
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)
Comment 2•15 years ago
|
||
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
Assignee | ||
Comment 3•15 years ago
|
||
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-
Assignee | ||
Updated•15 years ago
|
Blocks: tamarin-debugging
Updated•14 years ago
|
Flags: flashplayer-bug-
Assignee | ||
Comment 4•13 years ago
|
||
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)
Reporter | ||
Comment 5•13 years ago
|
||
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+
Comment 6•13 years ago
|
||
Potential bug here:
bool AvmCore::isVerbose(uint32_t b, MethodInfo* info)
{
bool v = b & config.verbose_vb; <<--------typecasting uint32_t to bool
Assignee | ||
Comment 7•13 years ago
|
||
(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.
Comment 8•13 years ago
|
||
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
Assignee | ||
Updated•13 years ago
|
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Comment 9•13 years ago
|
||
(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 → ---
Comment 10•13 years ago
|
||
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".
Comment 11•13 years ago
|
||
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.
Comment 12•13 years ago
|
||
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
Comment 13•13 years ago
|
||
(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.
Comment 14•13 years ago
|
||
(fix for issue in comment 12 and comment 13.)
Attachment #533974 -
Flags: review?(rreitmai)
Assignee | ||
Comment 15•13 years ago
|
||
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+
Comment 16•13 years ago
|
||
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
Comment 17•13 years ago
|
||
(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.)
Comment 18•13 years ago
|
||
(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.
Comment 19•13 years ago
|
||
Marking resolved/fixed now that Felix, Brent, and Valgrind are happy.
Assignee: stejohns → rreitmai
Status: REOPENED → RESOLVED
Closed: 13 years ago → 13 years ago
Resolution: --- → FIXED
Updated•13 years ago
|
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•