| Summary: | [mips] Fixed unused parameter warnings | ||||||
|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Konstantin Tokarev <annulen> | ||||
| Component: | JavaScriptCore | Assignee: | Nobody <webkit-unassigned> | ||||
| Status: | RESOLVED FIXED | ||||||
| Severity: | Normal | CC: | commit-queue, jbriance, keith_miller, mark.lam, msaboff, saam | ||||
| Priority: | P2 | ||||||
| Version: | WebKit Local Build | ||||||
| Hardware: | Unspecified | ||||||
| OS: | Unspecified | ||||||
| Attachments: |
|
||||||
|
Description
Konstantin Tokarev
2016-01-08 03:44:53 PST
Created attachment 268535 [details]
Patch
Comment on attachment 268535 [details]
Patch
This looks like a bug. Applying this patch would cover up the bug. We should find out who is using these setupArgumentsWithExecState that is ignoring the TrustedImm32.
Mark, it looked as bug for me too, but Julien Brianceau explained me that this is the dummy argument used to pass 64-bit arguments. Source/JavaScriptCore/jit/JITInlines.h:583 defines EABI_32BIT_DUMMY_ARG which is then used in setupArgumentsWithExecState invocations. I see two solutions here:
1) We actually setup the dummy argument, which is not a big deal (one move opcode)
2) We add somethink like this instead to check that this is actually the expected EABI_32BIT_DUMMY_ARG.
RELEASE_ASSERT(arg1.m_value == 0);
Better option for (2) would be ASSERT_UNUSED(arg1, arg1.m_value == 0); (In reply to comment #5) > Better option for (2) would be ASSERT_UNUSED(arg1, arg1.m_value == 0); I've looked at the code some more and see that this is the current accepted practice for dealing with EABI_32BIT_DUMMY_ARG. I think there are better ways to do this, but the current patch is adequate. I will r+ the patch. Comment on attachment 268535 [details] Patch Clearing flags on attachment: 268535 Committed r194766: <http://trac.webkit.org/changeset/194766> All reviewed patches have been landed. Closing bug. |