Fix single stepping over the IT instruction

[tberghammer]

Summary

Fix single stepping over the IT instruction

The ARM instruction emulator had 2 bugs related to the handling of the
IT instruction causing an error in single stepping:

• We haven't initialized the IT mask from the CPSR so if the last instruction of the IT block is a branch and the condition is false then the emulator evaluated the branch what resulted in an incorrect pc for the next instruction.
• The ITSTATE was advanced before the execution of each instruction. As a result the emulator was using the condition of following instruction in every case. The ITSTATE should be edvanced after the execution of an instruction except after an IT instruction.

[clayborg]

Looks good.

A related comment about IT instructions. You need to be careful when software single stepping through instructions that are in the middle of an ITSTATE block. You can NOT use a 16 bit instruction that isn't a BKPT otherwise you change your instructions. Are you guys using the BKPT trap for software breakpoints? If not, you should not step through IT instructions that have 32 bit thumb instructions or you will hose your program. Let me know if you need to know more about this.

[omjavaid]

Looks good. Was there a test failing in testsuite due to this?

[tberghammer]

In D16772#340950, @clayborg wrote:

Looks good.

A related comment about IT instructions. You need to be careful when software single stepping through instructions that are in the middle of an ITSTATE block. You can NOT use a 16 bit instruction that isn't a BKPT otherwise you change your instructions. Are you guys using the BKPT trap for software breakpoints? If not, you should not step through IT instructions that have 32 bit thumb instructions or you will hose your program. Let me know if you need to know more about this.

We are currently using 16 bit UDF instruction and as far as I can tell from some manual testing it is working as expected both when stepping over 16 bit and when stepping over 32 bit instructions. I will look into it why we are using UDF instead of BKPT and will create a separate CL for replacing it.

In D16772#341030, @omjavaid wrote:

Looks good. Was there a test failing in testsuite due to this?

I detected the issue based on TestStandardUnwind but we have no test exercising this feature directly.

P.S.: TestStandardUnwind still failing on some android arm devices because we don't handle the IT instruction for unwinding either. I am working on a fix for that as well.

[clayborg]

In D16772#341629, @tberghammer wrote:

In D16772#340950, @clayborg wrote:

Looks good.

A related comment about IT instructions. You need to be careful when software single stepping through instructions that are in the middle of an ITSTATE block. You can NOT use a 16 bit instruction that isn't a BKPT otherwise you change your instructions. Are you guys using the BKPT trap for software breakpoints? If not, you should not step through IT instructions that have 32 bit thumb instructions or you will hose your program. Let me know if you need to know more about this.

We are currently using 16 bit UDF instruction and as far as I can tell from some manual testing it is working as expected both when stepping over 16 bit and when stepping over 32 bit instructions. I will look into it why we are using UDF instead of BKPT and will create a separate CL for replacing it.

So the problem is this. Lets say you have a ITE (if then else) with the following instructions:

0x1000: 0x1122 ITE
0x1002: 0xaabbccdd Some 32 bit thumb instruction that is then conditional
0x1006: 0xeeff Some 16 bit thumb instruction that is else conditional

If you single step over 0x1000 you will end up at 0x1002, but if you have replaced 0x1002 with a 16 bit UDF (whose opcode we will pretend to be 0x1234) , did you just change you instructions in memory to be:

0x1000: 0x1122 ITE
0x1002: 0x1234 UDF that is then conditional
0x1004: 0xccdd Upper half of orignal 0xaabbccdd instruction that is else conditional
0x1006: 0xeeff Some 16 bit thumb instruction that is now NOT conditional?

This happened to us in debugserver before we switched over to using a BKPT instruction as they are the only instructions which guarantee that they will work correctly for IT blocks. So make a test case that does this, and set the condition both ways and make sure the values in the registers after the entire IT block are the same for running through it with no breakpoints, and also when stepping through it manually. Note that code has been added to LLDB to figure out when it is stopped in an IT Block at an instruction that shouldn't be stopped at, since the BKPT will get hit regardless of the current condition value, and we don't want to stop in this case, so we detect it and continue.

lldb/trunk/source/Plugins/Instruction/ARM/EmulateInstructionARM.cpp

if (success)
                     }
                 }
             }
             else
             {
                 m_opcode_mode = eModeARM;
                 m_opcode.SetOpcode32 (MemARead(read_inst_context, pc, 4, 0, &success), GetByteOrder());
             }
+
+            if (!m_ignore_conditions)
+            {
+                // If we are not ignoreing the conditions then init the it session from the current
+                // value of cpsr.
+                uint32_t it = (Bits32(m_opcode_cpsr, 15, 10) << 2) | Bits32(m_opcode_cpsr, 26, 25);
+                if (it != 0)
+                    m_it_session.InitIT(it);
+            }
         }
     }
     if (!success)
     {
         m_opcode_mode = eModeInvalid;
         m_addr = LLDB_INVALID_ADDRESS;
     }
     return success;
if (m_new_inst_cpsr != m_opcode_cpsr)
             return false;
     }
     return true;
 }
 
 bool
 EmulateInstructionARM::EvaluateInstruction (uint32_t evaluate_options)
 {
-    // Advance the ITSTATE bits to their values for the next instruction.
-    if (m_opcode_mode == eModeThumb && m_it_session.InITBlock())
-        m_it_session.ITAdvance();
-
     ARMOpcode *opcode_data = NULL;
    
     if (m_opcode_mode == eModeThumb)
         opcode_data = GetThumbOpcodeForInstruction (m_opcode.GetOpcode32(), m_arm_isa);
     else if (m_opcode_mode == eModeARM)
         opcode_data = GetARMOpcodeForInstruction (m_opcode.GetOpcode32(), m_arm_isa);
         
     if (opcode_data == NULL)
if (auto_advance_pc)
         if (!success)
             return false;
     }
     
     // Call the Emulate... function.
     success = (this->*opcode_data->callback) (m_opcode.GetOpcode32(), opcode_data->encoding);  
     if (!success)
         return false;
-        
+    // Advance the ITSTATE bits to their values for the next instruction if we haven't just executed
+    // an IT instruction what initialized it.
+    if (m_opcode_mode == eModeThumb && m_it_session.InITBlock() &&
+        opcode_data->callback != &EmulateInstructionARM::EmulateIT)
+        m_it_session.ITAdvance();
+
     if (auto_advance_pc)
     {
         uint32_t after_pc_value = ReadRegisterUnsigned (eRegisterKindDWARF, dwarf_pc, 0, &success);
         if (!success)
             return false;
             
         if (auto_advance_pc && (after_pc_value == orig_pc_value))
         {