Bug 123717 – Lacking support on a arm-traditional disassembler.

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[sloanyang]

The arm-traditional assembler generate codes, but it is hard to debug without a dissassembler.

[yangqi3]

This might be a problem. We could try to use the arm objdump code to generate dissambler.

[Csaba Osztrogonác]

It would be great to have a working disassembler for 
ARM traditional too to be able debug bug141193 easily.

I'm trying to make the LLVM based disassembler work on ARM.

[Csaba Osztrogonác]

LLVM dependency was removed from the trunk, now it is a 
bigger task and won't have time for it in the near future.

[Csaba Osztrogonác]

It would be great to have ARM traditional disassembler to
debug bug159408 easily. 

I managed to implement an LLVM based disassembler,
but it is a little bit hacky now. I'm going upstream
it when I finished cleanups.

[Csaba Osztrogonác]

Created attachment 284859 [details]
Patch

[Csaba Osztrogonác]

Comment on attachment 284859 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=284859&action=review

> Source/JavaScriptCore/disassembler/ARMLLVMDisassembler.cpp:66
> +        out.printf("%s%16s: [0x%08lx] %s\n", prefix, pcString, *(static_cast<unsigned long*>(pc)), instructionString);

I meant reinterpret_cast here, will update.

[Csaba Osztrogonác]

Created attachment 284876 [details]
Patch

[Csaba Osztrogonác]

ping?

[Saam Barati]

I'm the wrong person to review, but it seems OK. I'll let others comment.

[Mark Lam]

Comment on attachment 284876 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=284876&action=review

I'm also not the best person to review this, but have a few comments and questions below.

Another question: why can't ARM traditional use the ARMv7 disassembler (which we do have implemented)?  Are the differences that great?

> Source/JavaScriptCore/CMakeLists.txt:897
> +    ${LLVM_LIBRARIES}

If it's not too much trouble, is it possible to make this be conditional on the port?  If we want to move towards using the CMake build system for all ports eventually, this seems like a burden on all ports that don't use LLVM.

> Source/JavaScriptCore/disassembler/ARMLLVMDisassembler.cpp:2
> + * Copyright (C) 2013 Apple Inc. All rights reserved.

I think this should be 2016, not 2013.

> Source/cmake/FindLLVM.cmake:21
> +foreach (_program_name llvm-config llvm-config-3.7 llvm-config-3.6 llvm-config-3.5)
> +    find_program(LLVM_CONFIG_EXE NAMES ${_program_name})
> +    if (LLVM_CONFIG_EXE)
> +        execute_process(COMMAND ${LLVM_CONFIG_EXE} --version OUTPUT_VARIABLE LLVM_VERSION OUTPUT_STRIP_TRAILING_WHITESPACE)
> +        if ("${LLVM_VERSION}" VERSION_LESS "${LLVM_FIND_VERSION}")
> +            unset(LLVM_CONFIG_EXE CACHE)
> +        else ()
> +            break ()
> +        endif ()
> +    endif ()
> +endforeach ()

Naive question: would this work for a cross disassembler?  The llvm library that we want to link to for the disassembler needs to be the one that is for the target arm device.  Presumably, we would be running this makefile on an x86/x86_64 host.  How does find_program knows to find that version here?  It also seem strange to me that we would run the found LLVM_CONFIG_EXE to get its --version info.  What am I not seeing here?

[Csaba Osztrogonác]

Comment on attachment 284876 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=284876&action=review

>> Source/JavaScriptCore/CMakeLists.txt:897
>> +    ${LLVM_LIBRARIES}
> 
> If it's not too much trouble, is it possible to make this be conditional on the port?  If we want to move towards using the CMake build system for all ports eventually, this seems like a burden on all ports that don't use LLVM.

There is no need to add conditional here. LLVMDisassembler is optional and
disabled by default an can be enabled with setting USE_ARM_LLVM_DISASSEMBLER
in the cmake buildsystem (eg. build-jsc --cmakears=-DUSE_ARM_LLVM_DISASSEMBLER=ON)

If we don't set it, cmake won't call find_package(LLVM REQUIRED)
and this variable is an empty string.

>> Source/JavaScriptCore/disassembler/ARMLLVMDisassembler.cpp:2
>> + * Copyright (C) 2013 Apple Inc. All rights reserved.
> 
> I think this should be 2016, not 2013.

I added back the pre-r196729 LLVMDisassembler.cpp which has 2013 copyright.
I removed many lines and changed some lines to make it ARM traditional
only and dump all instructions in hexadecimal format too.

If we update the copyright, I think we should keep the original 
one and add 2016 with our company, not Apple.

>> Source/cmake/FindLLVM.cmake:21
>> +endforeach ()
> 
> Naive question: would this work for a cross disassembler?  The llvm library that we want to link to for the disassembler needs to be the one that is for the target arm device.  Presumably, we would be running this makefile on an x86/x86_64 host.  How does find_program knows to find that version here?  It also seem strange to me that we would run the found LLVM_CONFIG_EXE to get its --version info.  What am I not seeing here?

No, it won't work because of what you noticed. But cross building of JSC/WebKit
on Linux wasn't officially supported ever. Now we can build for ARM target 
on an ARM hardware or on an X86(_64) hardware inside a complete ARM rootFS
emulating almost everything (perl, python, ruby, llvm-config, ...).

As I mentioned in the changelog, I added back exactly the r196749 state 
of this file without a bit modification. It worked fine for years for
LLVM-based FTL JIT, I don't plan to refactor it in the near futere.

[Csaba Osztrogonác]

Based on comments https://bugs.webkit.org/show_bug.cgi?id=154323#c20
and https://bugs.webkit.org/show_bug.cgi?id=154323#c26 , could 
you guys r+ this patch to add the ARM traditional disassembler?

[Csaba Osztrogonác]

(In reply to comment #10)

> Another question: why can't ARM traditional use the ARMv7 disassembler
> (which we do have implemented)?  Are the differences that great?

I missed this question, sorry. 

ARMv7Disassembler is to disassemble Thumb2 instructions generated by
ARMv7Assembler. But ARMAssembler (traditional) generates ARM instructions 
(fixed 32-bit sized), not Thumb2 instructions. They are two different
instruction set.

[Mark Lam]

Comment on attachment 284876 [details]
Patch

r=me

[WebKit Commit Bot]

Comment on attachment 284876 [details]
Patch

Clearing flags on attachment: 284876

Committed r204084: <http://trac.webkit.org/changeset/204084>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Konstantin Tokarev]

>But cross building of JSC/WebKit on Linux wasn't officially supported ever. Now we can build for ARM target on an ARM hardware or on an X86(_64) hardware inside a complete ARM rootFS emulating almost everything (perl, python, ruby, llvm-config, ...).

FWIW, I'm routinely cross-compile Qt port for MIPS and never thought about going through such insanity as you described :)

[Csaba Osztrogonác]

(In reply to comment #17)
> >But cross building of JSC/WebKit on Linux wasn't officially supported ever. Now we can build for ARM target on an ARM hardware or on an X86(_64) hardware inside a complete ARM rootFS emulating almost everything (perl, python, ruby, llvm-config, ...).
> 
> FWIW, I'm routinely cross-compile Qt port for MIPS and never thought about
> going through such insanity as you described :)

I don't understand why do think building in a rootfs (emulating with qemu) is an insanity. I would be very curios how do you would like to run update-webkit<efl|gtk>libs, build-webkit, run-javascriptcore-tests (perl and ruby) in the _same_ X86_64 environment to build for a non X86_64 target. It's not as simple as you need a cross compiler, not at all.

[Konstantin Tokarev]

Ah, if you do not run tests on real hardware it makes sense indeed.

[Csaba Osztrogonác]

(In reply to comment #19)
> Ah, if you do not run tests on real hardware it makes sense indeed.

We don run tests on real hardware with --remote-config-file option of run-javascriptcore-tests. But it calls ldd to collect the dependecies
of the jsc binary. x86-ldd can't do anything with an ARM binary JSC.

Not to mention how complex or impossible to build 20-30 jhbuild dependencies
without qemu.