search

s390guy / SATK

Toolkit for creating baremetal programs targeting mainframe compatible systems
GNU General Public License v3.0
42 stars 6 forks source link

Unsigned immediate values erroneously treated as signed #12

Closed Fish-Git closed 4 years ago

Fish-Git commented 4 years ago

Some unsigned I2 immediate operand values are being erroneously treated as RI2 signed values instead, resulting in bogus "out of range" errors being generated.

The attached test program also tests for extended mnemonics support for some instructions which should be valid according to the Principles of Operation but which ASMA is not properly recognizing.

I am not that concerned about the lack of extended mnemonics support (I was just trying to be thorough).

I am much more concerned about the incorrect signed/unsigned handling of the immediate operands for the instructions shown. *`()`**

Thanks.

 

p.s. The version ASMA is reporting for itself is still 0.2.0, even though I know for a fact that it's been updated many times since 0.2.0 was first released. You might want to try a bit harder to keep your reported version number accurate!   ;-)

The version of ASMA I am currently using is the latest and greatest as/of today: commit 842aa3672e1a0c494933a3cc5fbc6ded758c8f55: "Normalize copyright information displayed when ASMA is run".

 

*`()`** _I have not bothered to create a test case to test ALL instructions with immediate operands. I'll leave that to you._

Fish-Git commented 4 years ago

(Oops! Forgot to attach the files! Fixed in original report and repeated here)

s390guy commented 4 years ago

Where are you finding the definitions for the extended mnemonics? I realize that is not your first concern, but I do not recognize most of them at all. I can't fix them without knowing how they are constructed.

Also, how is this impacting your work?

s390guy commented 4 years ago

Oh, and thank you!

Fish-Git commented 4 years ago

Where are you finding the definitions for the extended mnemonics? I realize that is not your first concern, but I do not recognize most of them at all. I can't fix them without knowing how they are constructed.

In the Principles of Operation manual of course!  (SA22-7832-12):

Also, how is this impacting your work?

At the moment it's not. As I said, I was just trying to be thorough.

I wanted to check whether the various extended Jump mnemonics were supported or not and decided to also check whether other extended mnemonics were supported or not as well, leading to the test program you now see.

To be honest, I created it mostly to help you, not me!

s390guy commented 4 years ago

Thanks again very much!

I was afraid that was going to be your answer. You have to read the details for the instruction to even find them. Yuck. No nice place to go. I have relied heavily on the list of instructions in the appendix, but they do not tell the whole story as some of the signed vs. unsigned problems you have found reveal.

Again, appreciate your efforts.

I am sort of close to finishing a major effort relating to boot loaders. You know, the infamous 90% done. LOL.

Fish-Git commented 4 years ago

You know, the infamous 90% done. LOL.

(LOL!) Yep. I'm very familiar with it!   :)

(Ninety-ninety rule):

"The first 90 percent of the code accounts for the first 90 percent of the development time. The remaining 10 percent of the code accounts for the other 90 percent of the development time."

--- Tom Cargill, Bell Labs

s390guy commented 4 years ago

A complete review of the latest Principles of Operation manual was performed for each instruction including one or more immediate values. 125 instructions were found, 12 of which were for vector instructions not yet implemented in ASMA. Of the remaining 113 instructions, 22 generated the out of bounds errors. All 22 instructions were corrected by using the correct format.

The following instructions were corrected: ALFI, ALGFI, CLFI, CLGFI, IIHF, IIHH, IIHL, IILF, IILF, IILL, LLIHF, LLILF, NIHF, NILF, OIHF, OILF, SLFI, SLGFI, VGBM, VREPI, XIHF, and XILF.

s390guy commented 4 years ago

During the review of the immediate instructions it became very clear that all instructions need to be reviewed for defined extended mnemonic instructions. Any instruction with a mask is a candidate. A few others have extended mnemonics for instruction content other than masks.

When ASMA was conceived, the only usage of extended mnemonics was for branch masks using the first mask field. Enhancements to the Machine Specification Language will be required for many if not all of the new mnemonic extensions. Another full review will also be needed. The review will identify the requirements for the MSL enhancements. From there the extended mnemonics can be defined and implemented. The instruction builder and MSL handling modules will require enhancements as well.

s390guy commented 4 years ago

With the commits performed for these corrections, this issue is being closed.

Fish-Git commented 4 years ago

The following instructions were corrected: ALFI, ALGFI, CLFI, CLGFI, IIHF, IIHH, IIHL, IILF, IILF, IILL, LLIHF, LLILF, NIHF, NILF, OIHF, OILF, SLFI, SLGFI, VGBM, VREPI, XIHF, and XILF.

Thank you, Harold! I'll try to get around to testing your changes as soon as I can. (Currently still busy with TXF! I want to get it released TODAY if possible!)

s390guy commented 4 years ago

As you can is fine.