Use the right format of bkpt/brk for align that falls after jump (#62106

) * Always use int3/bkpt * Use right formatting for brk instruction * Rename to brk_unix and brk_windows * jit format
dotnet · Nov 30, 2021 · 7eb5d98 · 7eb5d98
1 parent 84b55d9
commit 7eb5d98
Show file tree

Hide file tree

Showing 5 changed files with 27 additions and 25 deletions.
diff --git a/src/coreclr/jit/codegenarm64.cpp b/src/coreclr/jit/codegenarm64.cpp
@@ -3080,7 +3080,7 @@ void CodeGen::genCodeForCmpXchg(GenTreeCmpXchg* treeNode)
 
 instruction CodeGen::genGetInsForOper(genTreeOps oper, var_types type)
 {
-    instruction ins = INS_brk;
+    instruction ins = INS_BREAKPOINT;
 
     if (varTypeIsFloating(type))
     {

diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h
@@ -12000,7 +12000,9 @@ const instruction INS_SQRT = INS_vsqrt;
 const instruction        INS_MULADD = INS_madd;
 inline const instruction INS_BREAKPOINT_osHelper()
 {
-    return TargetOS::IsUnix ? INS_brk : INS_bkpt;
+    // GDB needs the encoding of brk #0
+    // Windbg needs the encoding of brk #F000
+    return TargetOS::IsUnix ? INS_brk_unix : INS_brk_windows;
 }
 #define INS_BREAKPOINT INS_BREAKPOINT_osHelper()
 

diff --git a/src/coreclr/jit/emitarm64.cpp b/src/coreclr/jit/emitarm64.cpp
@@ -3649,21 +3649,20 @@ void emitter::emitIns_I(instruction ins, emitAttr attr, ssize_t imm)
     insFormat fmt = IF_NONE;
 
     /* Figure out the encoding format of the instruction */
-    switch (ins)
+    if (ins == INS_BREAKPOINT)
     {
-        case INS_brk:
-            if ((imm & 0x0000ffff) == imm)
-            {
-                fmt = IF_SI_0A;
-            }
-            else
-            {
-                assert(!"Instruction cannot be encoded: IF_SI_0A");
-            }
-            break;
-        default:
-            unreached();
-            break;
+        if ((imm & 0x0000ffff) == imm)
+        {
+            fmt = IF_SI_0A;
+        }
+        else
+        {
+            assert(!"Instruction cannot be encoded: IF_SI_0A");
+        }
+    }
+    else
+    {
+        unreached();
     }
     assert(fmt != IF_NONE);
 
@@ -11459,15 +11458,16 @@ size_t emitter::emitOutputInstr(insGroup* ig, instrDesc* id, BYTE** dp)
                 // then add "bkpt" instruction.
                 instrDescAlign* alignInstr = (instrDescAlign*)id;
 
-                if (emitComp->compStressCompile(Compiler::STRESS_EMITTER, 50) &&
-                    (alignInstr->idaIG != alignInstr->idaLoopHeadPredIG) && !skipIns)
+                if (emitComp->compStressCompile(Compiler::STRESS_EMITTER, 50) && alignInstr->isPlacedAfterJmp &&
+                    !skipIns)
                 {
                     // There is no good way to squeeze in "bkpt" as well as display it
                     // in the disassembly because there is no corresponding instrDesc for
                     // it. As such, leave it as is, the "0xD43E0000" bytecode will be seen
                     // next to the nop instruction in disasm.
                     // e.g. D43E0000          align   [4 bytes for IG07]
-                    // ins = INS_bkpt;
+                    ins = INS_BREAKPOINT;
+                    fmt = IF_SI_0A;
                 }
 #endif
             }
@@ -14610,7 +14610,7 @@ emitter::insExecutionCharacteristics emitter::getInsExecutionCharacteristics(ins
             }
             break;
 
-        case IF_SN_0A: // bkpt, brk, nop
+        case IF_SN_0A: // nop, yield, align
 
             if (id->idIns() == INS_align)
             {

diff --git a/src/coreclr/jit/emitxarch.cpp b/src/coreclr/jit/emitxarch.cpp
@@ -10037,7 +10037,7 @@ BYTE* emitter::emitOutputAlign(insGroup* ig, instrDesc* id, BYTE* dst)
     // For cases where 'align' was placed behing a 'jmp' in an IG that does not
     // immediately preced the loop IG, we do not know in advance the offset of
     // IG having loop. For such cases, skip the padding calculation validation.
-    bool validatePadding = alignInstr->idaIG == alignInstr->idaLoopHeadPredIG;
+    bool validatePadding = !alignInstr->isPlacedAfterJmp;
 #endif
 
     // Candidate for loop alignment
@@ -10076,7 +10076,7 @@ BYTE* emitter::emitOutputAlign(insGroup* ig, instrDesc* id, BYTE* dst)
     // then add "int3" instruction. Since int3 takes 1 byte, we would only add
     // it if paddingToAdd >= 1 byte.
 
-    if (emitComp->compStressCompile(Compiler::STRESS_EMITTER, 50) && !validatePadding && paddingToAdd >= 1)
+    if (emitComp->compStressCompile(Compiler::STRESS_EMITTER, 50) && alignInstr->isPlacedAfterJmp && paddingToAdd >= 1)
     {
         size_t int3Code = insCodeMR(INS_BREAKPOINT);
         // There is no good way to squeeze in "int3" as well as display it

diff --git a/src/coreclr/jit/instrsarm64.h b/src/coreclr/jit/instrsarm64.h
@@ -1566,10 +1566,10 @@ INST1(nop,         "nop",          0,      IF_SN_0A,  0xD503201F)
 INST1(yield,       "yield",        0,      IF_SN_0A,  0xD503203F)
                                    //  yield                        SN_0A  1101010100000011 0010000000111111   D503 203F
 
-INST1(bkpt,        "bkpt",         0,      IF_SN_0A,  0xD43E0000)
-                                   //  brpt                         SN_0A  1101010000111110 0000000000000000   D43E 0000   0xF000
+INST1(brk_windows, "brk_windows",  0,      IF_SI_0A,  0xD43E0000)
+                                   //  brk (windows)                SI_0A  1101010000111110 0000000000000000   D43E 0000   0xF000
 
-INST1(brk,         "brk",          0,      IF_SI_0A,  0xD4200000)
+INST1(brk_unix,    "brk_unix",     0,      IF_SI_0A,  0xD4200000)
                                    //  brk     imm16                SI_0A  11010100001iiiii iiiiiiiiiii00000   D420 0000   imm16
 
 INST1(dsb,         "dsb",          0,      IF_SI_0B,  0xD503309F)