Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Nov 30, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/coreclr/jit/codegenarm64.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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))
{
Expand Down
4 changes: 3 additions & 1 deletion src/coreclr/jit/compiler.h
Original file line number Diff line number Diff line change
Expand Up @@ -11999,7 +11999,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()

Expand Down
36 changes: 18 additions & 18 deletions src/coreclr/jit/emitarm64.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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)
{
Expand Down
4 changes: 2 additions & 2 deletions src/coreclr/jit/emitxarch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions src/coreclr/jit/instrsarm64.h
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down