Bug 405403 - s390x disassembler cannot be used on x86
Summary: s390x disassembler cannot be used on x86
Status: RESOLVED FIXED
Alias: None
Product: valgrind
Classification: Developer tools
Component: vex (show other bugs)
Version: 3.15 SVN
Platform: Other Linux
: NOR normal
Target Milestone: ---
Assignee: Julian Seward
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-03-12 18:36 UTC by Ilya Leoshkevich
Modified: 2019-03-15 14:54 UTC (History)
1 user (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments
patch (360.89 KB, application/mbox)
2019-03-12 18:36 UTC, Ilya Leoshkevich
Details
patch v2 (362.50 KB, text/plain)
2019-03-13 15:40 UTC, Ilya Leoshkevich
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Ilya Leoshkevich 2019-03-12 18:36:55 UTC
Created attachment 118753 [details]
patch

Certain projects, e.g. https://github.com/angr/angr-dev, use VEX as an
intermediate representation for static binary code analysis. In order to
use them to analyze S/390 code on Intel, one needs to resolve endianness
issues in the disassembler.
Comment 1 Andreas Arnez 2019-03-13 13:31:09 UTC
Ilya, thanks for the patch.  Your patch makes the code independent from endianness and a bit shorter than before.  In my view this is a useful clean-up, and I haven't seen any problems with it in my testing.

So I'd like to push this, if there are no objections.  Does anyone see any issues with Ilya's approach?
Comment 2 Julian Seward 2019-03-13 14:15:33 UTC
(In reply to Andreas Arnez from comment #1)

Andreas, if you think the patch is OK, and won't cause regressions, then
fine, land it.  It is however a huge patch and it would be nice to have
a few lines of description in the commit message explaining what it does.

This will, I assume, create rebase issues for any other unlanded s390
front end patches.  But IIUC, that's only the initial z14 work.  Yes?
Comment 3 Ilya Leoshkevich 2019-03-13 15:40:30 UTC
Created attachment 118775 [details]
patch v2
Comment 4 Ilya Leoshkevich 2019-03-13 15:42:01 UTC
I've expanded the commit message to indicate not only what the patch is for, but also what it does and why.

There are also two additional changes in v2:

- #pragma pack was removed 

- last_execute_target is now also loaded byte-by-byte
Comment 5 Andreas Arnez 2019-03-15 13:49:46 UTC
(In reply to Ilya Leoshkevich from comment #4)
> I've expanded the commit message to indicate not only what the patch is for,
> but also what it does and why.
> 
> There are also two additional changes in v2:
> 
> - #pragma pack was removed 
> 
> - last_execute_target is now also loaded byte-by-byte

All these changes look good to me.  I think this is ready to push.

(In reply to Julian Seward from comment #2)
> This will, I assume, create rebase issues for any other unlanded s390
> front end patches.  But IIUC, that's only the initial z14 work.  Yes?

Right, as far as I know, that is the only pending change that may be affected.
Comment 6 Andreas Arnez 2019-03-15 14:54:01 UTC
Pushed as commit #7e9113cb7a249e0fae2a365462c6b0164de11566.