Bug 419503

Summary: s390x: Avoid modifying registers returned from isel functions
Product: [Developer tools] valgrind Reporter: Andreas Arnez <arnez>
Component: vexAssignee: Andreas Arnez <arnez>
Status: RESOLVED FIXED    
Severity: normal    
Priority: NOR    
Version First Reported In: unspecified   
Target Milestone: ---   
Platform: Other   
OS: Linux   
Latest Commit: Version Fixed/Implemented In:
Sentry Crash Report:
Attachments: Original "CHECK" markers for host_s390_isel.c by Julian
Drop register arg to s390_isel_int1_expr()
Fix typos in comments for sub_from_SP and add_to_SP in isel
Introduce and exploit new ALU operator S390_ALU_ILIH
Fix Iex_Load instruction selectors for F128/D128
Drop spurious register moves in CDAS instruction selector

Description Andreas Arnez 2020-04-01 17:30:38 UTC
As discussed with Julian Seward, host_s390_isel.c contains some questionable logic where registers returned from isel functions are potentially modified, or where it is not sufficiently made clear that they are always left unmodified.

As a general rule, rather than modifying a register returned by one of the isel functions, the register should be copied first and the copy modified instead.
Comment 1 Andreas Arnez 2020-04-01 17:50:23 UTC
Created attachment 127167 [details]
Original "CHECK" markers for host_s390_isel.c by Julian

This is a "patch" that marks the questionable logic in host_s390_isel.c Julian detected during his review.
Comment 2 Andreas Arnez 2020-04-03 18:05:31 UTC
Created attachment 127245 [details]
Drop register arg to s390_isel_int1_expr()
Comment 3 Andreas Arnez 2020-04-03 18:06:15 UTC
Created attachment 127246 [details]
Fix typos in comments for sub_from_SP and add_to_SP in isel
Comment 4 Andreas Arnez 2020-04-03 18:06:56 UTC
Created attachment 127247 [details]
Introduce and exploit new ALU operator  S390_ALU_ILIH
Comment 5 Andreas Arnez 2020-04-03 18:07:55 UTC
Created attachment 127248 [details]
Fix Iex_Load instruction selectors for F128/D128
Comment 6 Andreas Arnez 2020-04-03 18:08:40 UTC
Created attachment 127249 [details]
Drop spurious register moves in CDAS instruction selector
Comment 7 Andreas Arnez 2020-04-03 18:10:08 UTC
See the above attachments for fixes of the findings related to this Bug.
Comment 8 Andreas Arnez 2020-04-08 18:00:33 UTC
Pushed all of the changes proposed above:

6a90a15b9 - s390x: Drop spurious register moves in CDAS instruction selector
4970e2002 - s390x: Fix Iex_Load instruction selectors for F128/D128 types
4e9763c61 - s390x: Introduce and exploit new ALU operator S390_ALU_ILIH
1008ab726 - s390x: Fix typos in comments for sub_from_SP and add_to_SP in isel