1365699 - Unreachable code in /js/src/jit/x86-shared/Disassembler-x86-shared.cpp

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect)

Description

[Jan Keromnes [:janx]]

When running static analysis on /js/src/jit/x86-shared/Disassembler-x86-shared.cpp with the following command:

    $ run-clang-tidy-4.0.py -j 18 -p obj-x86_64-pc-linux-gnu/ -checks=-*,clang-analyzer-deadcode.DeadStores js/src/jit/x86-shared/

We get the following warnings:

    /home/user/firefox/js/src/jit/x86-shared/Disassembler-x86-shared.cpp:226:13: warning: Value stored to 'l' is never read [clang-analyzer-deadcode.DeadStores]
                l = (c4b >> 2) & 0x1;
                ^
    /home/user/firefox/js/src/jit/x86-shared/Disassembler-x86-shared.cpp:226:13: note: Value stored to 'l' is never read
                l = (c4b >> 2) & 0x1;
                ^
    /home/user/firefox/js/src/jit/x86-shared/Disassembler-x86-shared.cpp:236:13: warning: Value stored to 'l' is never read [clang-analyzer-deadcode.DeadStores]
                l = (c5 >> 2) & 0x1;
                ^
    /home/user/firefox/js/src/jit/x86-shared/Disassembler-x86-shared.cpp:236:13: note: Value stored to 'l' is never read
                l = (c5 >> 2) & 0x1;
                ^

This is because the `switch (m)` at Disassembler-x86-shared.cpp:245 jumps out of the scope (either with a `goto opcode_done` or a `MOZ_CRASH(...)`) before the `if (l != 0) // 256-bit SIMD` check can be reached, causing the variable `l` to never be read.

Comment 1

[Jan Keromnes [:janx]]

Attachment: Fix unreachable code in Disassembler-x86-shared.cpp.

By moving the sanity check above the switch, we make the code reachable again.

Comment 2

[Jan Keromnes [:janx]]

Attachment: Fix unreachable code in Disassembler-x86-shared.cpp.

Fixed to 4-space indent.

Comment 3

[Benjamin Bouvier [:bbouvier] (inactive)]

Comment on attachment 8868698 [details] [diff] [review]
Fix unreachable code in Disassembler-x86-shared.cpp.

Review of attachment 8868698 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good! If this passes jit tests, let's land it. Thanks for the patch!

Comment 4

[Jan Keromnes [:janx]]

Attachment: Fix unreachable code in Disassembler-x86-shared.cpp.

Rebased, r+ carried over, try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=1eeaab1130c2

Comment 5

[Jan Keromnes [:janx]]

The assertion failures in /src/ld/ld.hpp on "OS X 10.7 opt" seem unrelated (see orange try jobs "cgc" and "p"). Please land this patch.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/37e9f6f9d1e8
Fix unreachable code in Disassembler-x86-shared.cpp. r=bbouvier

Comment 7

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/37e9f6f9d1e8