From 7eb5d98fac2423941cc487658dcfeb5ef1ea82b0 Mon Sep 17 00:00:00 2001 From: Kunal Pathak Date: Tue, 30 Nov 2021 14:28:19 -0800 Subject: [PATCH] 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 --- src/coreclr/jit/codegenarm64.cpp | 2 +- src/coreclr/jit/compiler.h | 4 +++- src/coreclr/jit/emitarm64.cpp | 36 ++++++++++++++++---------------- src/coreclr/jit/emitxarch.cpp | 4 ++-- src/coreclr/jit/instrsarm64.h | 6 +++--- 5 files changed, 27 insertions(+), 25 deletions(-) diff --git a/src/coreclr/jit/codegenarm64.cpp b/src/coreclr/jit/codegenarm64.cpp index a0654de6c62242..076df275868a36 100644 --- 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 index 9f9b904fa6ea39..00a1e5ac6046a7 100644 --- 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 index cee57456782e16..1bcda1dd0a88a3 100644 --- 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 index 4dd0d44a856385..d472b1393518fb 100644 --- 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 index fbd74c1030522c..c89b006a418970 100644 --- 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)