STR in Thumb 32 decode problem

Bug #1922887 reported by JIANG Muhui
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
QEMU
Fix Released
Undecided
Unassigned

Bug Description

Hi

It seems that QEMU does not have a proper check on the STR instruction in Thumb32 mode.

Specifically, the machine code is 0xf84f0ddd, which is 0b1111 1000 0100 1111 0000 1101 1101 1101.
This is an STR (immediate, Thumb) instruction with a T4 encoding scheme.

The symbols is

Rn = 1111
Rt = 0000
P = 1
U = 0
W = 1

The decode ASL is below:

if P == ‘1’ && U == ‘1’ && W == ‘0’ then SEE STRT;
if Rn == ‘1101’ && P == ‘1’ && U == ‘0’ && W == ‘1’ && imm8 == ‘00000100’ then SEE PUSH;
if Rn == ‘1111’ || (P == ‘0’ && W == ‘0’) then UNDEFINED;
t = UInt(Rt); n = UInt(Rn); imm32 = ZeroExtend(imm8, 32);
index = (P == ‘1’); add = (U == ‘1’); wback = (W == ‘1’);
if t == 15 || (wback && n == t) then UNPREDICTABLE;

When Rn == 1111, it should be an undefined instruction, which should raise SEGILL signal. However, it seems that QEMU does not check this constraint, which should be a bug. Many thanks

Regards
Muhui

Tags: arm
Peter Maydell (pmaydell)
tags: added: arm
Revision history for this message
Peter Maydell (pmaydell) wrote :

NB that for Arm mode the equivalent case is UNPREDICTABLE only in the writeback case (and has defined behaviour for non-writeback), so we need to enforce the UNDEF on rn==15 for Thumb decode only.

Revision history for this message
Peter Maydell (pmaydell) wrote :

Patch sent to list: if you could test it against whatever your test case was that would be helpful.
https://<email address hidden>/

PS: out of interest, why/how were you checking should-UNDEF cases ?

Changed in qemu:
status: New → In Progress
Revision history for this message
JIANG Muhui (muhui) wrote :

We just test the patched version. It looks well. Now QEMU would raise SEGILL signals, which should be the right behavior.

We are not checking should-UNDEF cases in particular. This is a case we observed and checked manually when doing a research project with QEMU.

Revision history for this message
Thomas Huth (th-huth) wrote :
Changed in qemu:
status: In Progress → Fix Committed
Thomas Huth (th-huth)
Changed in qemu:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.