Bug 419503 - s390x: Avoid modifying registers returned from isel functions
Summary: s390x: Avoid modifying registers returned from isel functions
Status: RESOLVED FIXED
Alias: None
Product: valgrind
Classification: Developer tools
Component: vex (show other bugs)
Version: unspecified
Platform: Other Linux
: NOR normal
Target Milestone: ---
Assignee: Andreas Arnez
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-04-01 17:30 UTC by Andreas Arnez
Modified: 2020-04-08 18:00 UTC (History)
0 users

See Also:
Latest Commit:
Version Fixed In:
Sentry Crash Report:


Attachments
Original "CHECK" markers for host_s390_isel.c by Julian (5.30 KB, patch)
2020-04-01 17:50 UTC, Andreas Arnez
Details
Drop register arg to s390_isel_int1_expr() (4.24 KB, patch)
2020-04-03 18:05 UTC, Andreas Arnez
Details
Fix typos in comments for sub_from_SP and add_to_SP in isel (1.11 KB, patch)
2020-04-03 18:06 UTC, Andreas Arnez
Details
Introduce and exploit new ALU operator S390_ALU_ILIH (9.27 KB, patch)
2020-04-03 18:06 UTC, Andreas Arnez
Details
Fix Iex_Load instruction selectors for F128/D128 (1.41 KB, patch)
2020-04-03 18:07 UTC, Andreas Arnez
Details
Drop spurious register moves in CDAS instruction selector (1.24 KB, patch)
2020-04-03 18:08 UTC, Andreas Arnez
Details

Note You need to log in before you can comment on or make changes to this bug.
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