Skip to content
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.

Sign up for GitHub

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

Prevent crash on ARM32 #1412

Closed
wants to merge 3 commits into from
Closed

Prevent crash on ARM32 #1412

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
fe6114c
Prevent crash on ARM32
kant2002 Aug 15, 2021
2088a63
Update src/coreclr/jit/unwind.cpp
kant2002 Aug 15, 2021
db81176
Address PR feedback
kant2002 Aug 15, 2021
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
7 changes: 6 additions & 1 deletion src/coreclr/jit/unwind.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,12 @@ void Compiler::unwindPushPopMaskCFI(regMaskTP regMask, bool isFloat)
regMaskTP regBit = isFloat ? genRegMask(REG_FP_FIRST) : 1;

for (regNumber regNum = isFloat ? REG_FP_FIRST : REG_FIRST; regNum < REG_COUNT;
regNum = REG_NEXT(regNum), regBit <<= 1)
#if TARGET_ARM
regNum = isFloat ? REG_NEXT(REG_NEXT(regNum)) : REG_NEXT(regNum), regBit <<= isFloat ? 2 : 1
#else
regNum = REG_NEXT(regNum), regBit <<= 1
#endif
)
{
if (regBit > regMask)
{
Expand Down
80 changes: 16 additions & 64 deletions src/coreclr/jit/unwindarm.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,100 +71,52 @@ short Compiler::mapRegNumToDwarfReg(regNumber reg)
dwarfReg = 15;
break;
case REG_F0:
dwarfReg = 64;
break;
case REG_F1:
dwarfReg = 65;
dwarfReg = 256;
break;
case REG_F2:
dwarfReg = 66;
break;
case REG_F3:
dwarfReg = 67;
dwarfReg = 257;
break;
case REG_F4:
dwarfReg = 68;
break;
case REG_F5:
dwarfReg = 69;
dwarfReg = 258;
break;
case REG_F6:
dwarfReg = 70;
break;
case REG_F7:
dwarfReg = 71;
dwarfReg = 259;
break;
case REG_F8:
dwarfReg = 72;
break;
case REG_F9:
dwarfReg = 73;
dwarfReg = 260;
break;
case REG_F10:
dwarfReg = 74;
break;
case REG_F11:
dwarfReg = 75;
dwarfReg = 261;
break;
case REG_F12:
dwarfReg = 76;
break;
case REG_F13:
dwarfReg = 77;
dwarfReg = 262;
break;
case REG_F14:
dwarfReg = 78;
break;
case REG_F15:
dwarfReg = 79;
dwarfReg = 263;
break;
case REG_F16:
dwarfReg = 80;
break;
case REG_F17:
dwarfReg = 81;
dwarfReg = 264;
break;
case REG_F18:
dwarfReg = 82;
break;
case REG_F19:
dwarfReg = 83;
dwarfReg = 265;
break;
case REG_F20:
dwarfReg = 84;
break;
case REG_F21:
dwarfReg = 85;
dwarfReg = 266;
break;
case REG_F22:
dwarfReg = 86;
break;
case REG_F23:
dwarfReg = 87;
dwarfReg = 267;
break;
case REG_F24:
dwarfReg = 88;
break;
case REG_F25:
dwarfReg = 89;
dwarfReg = 268;
break;
case REG_F26:
dwarfReg = 90;
break;
case REG_F27:
dwarfReg = 91;
dwarfReg = 269;
break;
case REG_F28:
dwarfReg = 92;
break;
case REG_F29:
dwarfReg = 93;
dwarfReg = 270;
break;
case REG_F30:
dwarfReg = 94;
break;
case REG_F31:
dwarfReg = 95;
dwarfReg = 271;
break;
default:
noway_assert(!"unexpected REG_NUM");
Expand Down
92 changes: 68 additions & 24 deletions src/coreclr/tools/aot/ObjWriter/debugInfo/dwarf/dwarfGen.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ static int GetDwarfRegNum(Triple::ArchType ArchType, int RegNum) {
case RegNumArm::REGNUM_PC: return 15;
// fp registers
default:
return RegNum - static_cast<int>(RegNumArm::REGNUM_COUNT) + 64;
return (RegNum - static_cast<int>(RegNumArm::REGNUM_COUNT)) / 2 + 256;
}
case Triple::aarch64: // fall through
case Triple::aarch64_be:
Expand Down Expand Up @@ -284,6 +284,54 @@ static int GetDwarfFpRegNum(Triple::ArchType ArchType)
}
}

static int GetRegOpSize(int DwarfRegNum) {
if (DwarfRegNum <= 31) {
return 1;
}
else if (DwarfRegNum < 128) {
return 2;
}
else if (DwarfRegNum < 16384) {
return 3;
}
else {
assert(false && "Too big register number");
return 0;
}
}

static void EmitBreg(MCObjectStreamer* Streamer, int DwarfRegNum, StringRef bytes) {
if (DwarfRegNum <= 31) {
Streamer->EmitIntValue(DwarfRegNum + dwarf::DW_OP_breg0, 1);
}
else {
Streamer->EmitIntValue(dwarf::DW_OP_bregx, 1);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to the dwarf manual: "The DW_OP_bregx operation has two operands: a register which is specified by an unsigned LEB128 number, followed by a signed LEB128 offset.". It would be nice to emit the offset as part of this function instead of emitting it in the calling code.

Streamer->EmitULEB128IntValue(DwarfRegNum);
}
Streamer->EmitBytes(bytes);
}

static void EmitBreg(MCObjectStreamer* Streamer, int DwarfRegNum, int value) {
if (DwarfRegNum <= 31) {
Streamer->EmitIntValue(DwarfRegNum + dwarf::DW_OP_breg0, 1);
}
else {
Streamer->EmitIntValue(dwarf::DW_OP_bregx, 1);
Streamer->EmitULEB128IntValue(DwarfRegNum);
}
Streamer->EmitSLEB128IntValue(value);
}

static void EmitReg(MCObjectStreamer* Streamer, int DwarfRegNum) {
if (DwarfRegNum <= 31) {
Streamer->EmitIntValue(DwarfRegNum + dwarf::DW_OP_reg0, 1);
}
else {
Streamer->EmitIntValue(dwarf::DW_OP_regx, 1);
Streamer->EmitULEB128IntValue(DwarfRegNum);
}
}

static void EmitVarLocation(MCObjectStreamer *Streamer,
const ICorDebugInfo::NativeVarInfo &VarInfo,
bool IsLocList = false) {
Expand All @@ -307,22 +355,21 @@ static void EmitVarLocation(MCObjectStreamer *Streamer,
case ICorDebugInfo::VLT_REG: {
DwarfRegNum = GetDwarfRegNum(ArchType, VarInfo.loc.vlReg.vlrReg);
if (IsByRef) {
Len = 2;
Len = 1 + GetRegOpSize(DwarfRegNum);
if (IsLocList) {
Streamer->EmitIntValue(Len, 2);
} else {
Streamer->EmitULEB128IntValue(Len);
}
Streamer->EmitIntValue(DwarfRegNum + dwarf::DW_OP_breg0, 1);
Streamer->EmitSLEB128IntValue(0);
EmitBreg(Streamer, DwarfRegNum, 0);
} else {
Len = 1;
Len = GetRegOpSize(DwarfRegNum);
if (IsLocList) {
Streamer->EmitIntValue(Len, 2);
} else {
Streamer->EmitULEB128IntValue(Len);
}
Streamer->EmitIntValue(DwarfRegNum + dwarf::DW_OP_reg0, 1);
EmitReg(Streamer, DwarfRegNum);
}

break;
Expand All @@ -331,6 +378,7 @@ static void EmitVarLocation(MCObjectStreamer *Streamer,
IsByRef = true;
case ICorDebugInfo::VLT_STK2:
IsStk2 = true;
case ICorDebugInfo::VLT_FPSTK:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you actually hit this case? I would not expect it to be hit on. Floating point stack is a legacy 32-bit x86 that are not using anymore even on 32-bit x86.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is hit. I was tired of seeing assertion. This change was fishy to me too. But I do not know where to start looking. With that hint, I try to pin-point what can cause code to hit that location.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have looked around. VLT_FPSTK is actually used for regular local variables that do not live on floating point stack. It does not look right; but it may done that way for some legacy reasons. So your change is ok.

case ICorDebugInfo::VLT_STK: {
DwarfBaseRegNum = GetDwarfRegNum(ArchType, IsStk2 ? VarInfo.loc.vlStk2.vls2BaseReg :
VarInfo.loc.vlStk.vlsBaseReg);
Expand All @@ -342,24 +390,22 @@ static void EmitVarLocation(MCObjectStreamer *Streamer,
StringRef OffsetRepr = OSE.str();

if (IsByRef) {
Len = OffsetRepr.size() + 2;
Len = OffsetRepr.size() + 1 + GetRegOpSize(DwarfBaseRegNum);
if (IsLocList) {
Streamer->EmitIntValue(Len, 2);
} else {
Streamer->EmitULEB128IntValue(Len);
}
Streamer->EmitIntValue(DwarfBaseRegNum + dwarf::DW_OP_breg0, 1);
Streamer->EmitBytes(OffsetRepr);
EmitBreg(Streamer, DwarfBaseRegNum, OffsetRepr);
Streamer->EmitIntValue(dwarf::DW_OP_deref, 1);
} else {
Len = OffsetRepr.size() + 1;
Len = OffsetRepr.size() + GetRegOpSize(DwarfBaseRegNum);
if (IsLocList) {
Streamer->EmitIntValue(Len, 2);
} else {
Streamer->EmitULEB128IntValue(Len);
}
Streamer->EmitIntValue(DwarfBaseRegNum + dwarf::DW_OP_breg0, 1);
Streamer->EmitBytes(OffsetRepr);
EmitBreg(Streamer, DwarfBaseRegNum, OffsetRepr);
}

break;
Expand All @@ -368,18 +414,19 @@ static void EmitVarLocation(MCObjectStreamer *Streamer,
DwarfRegNum = GetDwarfRegNum(ArchType, VarInfo.loc.vlRegReg.vlrrReg1);
DwarfRegNum2 = GetDwarfRegNum(ArchType, VarInfo.loc.vlRegReg.vlrrReg2);

Len = (1 /* DW_OP_reg */ + 1 /* DW_OP_piece */ + 1 /* Reg size */) * 2;
Len = (GetRegOpSize(DwarfRegNum2) /* DW_OP_reg */ + 1 /* DW_OP_piece */ + 1 /* Reg size */)
+ (GetRegOpSize(DwarfRegNum) /* DW_OP_reg */ + 1 /* DW_OP_piece */ + 1 /* Reg size */);
if (IsLocList) {
Streamer->EmitIntValue(Len, 2);
} else {
Streamer->EmitULEB128IntValue(Len + 1);
}

Streamer->EmitIntValue(DwarfRegNum2 + dwarf::DW_OP_reg0, 1);
EmitReg(Streamer, DwarfRegNum2);
Streamer->EmitIntValue(dwarf::DW_OP_piece, 1);
Streamer->EmitULEB128IntValue(TargetPointerSize);

Streamer->EmitIntValue(DwarfRegNum + dwarf::DW_OP_reg0, 1);
EmitReg(Streamer, DwarfRegNum);
Streamer->EmitIntValue(dwarf::DW_OP_piece, 1);
Streamer->EmitULEB128IntValue(TargetPointerSize);

Expand All @@ -399,8 +446,8 @@ static void EmitVarLocation(MCObjectStreamer *Streamer,
VarInfo.loc.vlStkReg.vlsrStk.vlsrsOffset, OSE);
StringRef OffsetRepr = OSE.str();

Len = (1 /* DW_OP_reg */ + 1 /* DW_OP_piece */ + 1 /* Reg size */) +
(1 /*DW_OP_breg */ + OffsetRepr.size() + 1 /* DW_OP_piece */ + 1 /* Reg size */);
Len = (GetRegOpSize(DwarfRegNum) /* DW_OP_reg */ + 1 /* DW_OP_piece */ + 1 /* Reg size */) +
(GetRegOpSize(DwarfBaseRegNum) /*DW_OP_breg */ + OffsetRepr.size() + 1 /* DW_OP_piece */ + 1 /* Reg size */);

if (IsLocList) {
Streamer->EmitIntValue(Len, 2);
Expand All @@ -409,28 +456,25 @@ static void EmitVarLocation(MCObjectStreamer *Streamer,
}

if (IsRegStk) {
Streamer->EmitIntValue(DwarfRegNum + dwarf::DW_OP_reg0, 1);
EmitReg(Streamer, DwarfRegNum);
Streamer->EmitIntValue(dwarf::DW_OP_piece, 1);
Streamer->EmitULEB128IntValue(TargetPointerSize);

Streamer->EmitIntValue(DwarfBaseRegNum + dwarf::DW_OP_breg0, 1);
Streamer->EmitBytes(OffsetRepr);
EmitBreg(Streamer, DwarfBaseRegNum, OffsetRepr);
Streamer->EmitIntValue(dwarf::DW_OP_piece, 1);
Streamer->EmitULEB128IntValue(TargetPointerSize);
} else {
Streamer->EmitIntValue(DwarfBaseRegNum + dwarf::DW_OP_breg0, 1);
Streamer->EmitBytes(OffsetRepr);
EmitBreg(Streamer, DwarfBaseRegNum, OffsetRepr);
Streamer->EmitIntValue(dwarf::DW_OP_piece, 1);
Streamer->EmitULEB128IntValue(TargetPointerSize);

Streamer->EmitIntValue(DwarfRegNum + dwarf::DW_OP_reg0, 1);
EmitReg(Streamer, DwarfRegNum);
Streamer->EmitIntValue(dwarf::DW_OP_piece, 1);
Streamer->EmitULEB128IntValue(TargetPointerSize);
}

break;
}
case ICorDebugInfo::VLT_FPSTK:
case ICorDebugInfo::VLT_FIXED_VA:
assert(false && "Unsupported varloc type!");
default:
Expand Down