Bug 405403

Summary:	s390x disassembler cannot be used on x86
Product:	[Developer tools] valgrind	Reporter:	Ilya Leoshkevich <iii>
Component:	vex	Assignee:	Julian Seward <jseward>
Status:	RESOLVED FIXED
Severity:	normal	CC:	arnez
Priority:	NOR
Version First Reported In:	3.15 SVN
Target Milestone:	---
Platform:	Other
OS:	Linux
Latest Commit:		Version Fixed/Implemented In:
Sentry Crash Report:
Attachments:	patch patch v2

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.