Skip to content

Commit 454c149

Browse files
committed
[BOLT][NFC] Fix undefined behavior in encodeAnnotationImm
Fix UBSan-reported issue in MCPlusBuilder::encodeAnnotationImm (left shift of a negative value). Test Plan: ``` ninja check-bolt ... PASS: BOLT-Unit :: Core/./CoreTests/AArch64/MCPlusBuilderTester.Annotation/0 (1 of 140) PASS: BOLT-Unit :: Core/./CoreTests/X86/MCPlusBuilderTester.Annotation/0 (131 of 134) ``` Reviewed By: maksfb, yota9 Differential Revision: https://reviews.llvm.org/D120260
1 parent d7105e7 commit 454c149

File tree

3 files changed

+32
-6
lines changed

3 files changed

+32
-6
lines changed

bolt/include/bolt/Core/MCPlusBuilder.h

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -74,9 +74,9 @@ class MCPlusBuilder {
7474
AllocatorIdTy MaxAllocatorId = 0;
7575

7676
/// We encode Index and Value into a 64-bit immediate operand value.
77-
static int64_t encodeAnnotationImm(unsigned Index, int64_t Value) {
78-
assert(Index < 256 && "annotation index max value exceeded");
79-
assert((Value == (Value << 8) >> 8) && "annotation value out of range");
77+
static int64_t encodeAnnotationImm(uint8_t Index, int64_t Value) {
78+
if (LLVM_UNLIKELY(Value != extractAnnotationValue(Value)))
79+
report_fatal_error("annotation value out of range");
8080

8181
Value &= 0xff'ffff'ffff'ffff;
8282
Value |= (int64_t)Index << 56;
@@ -85,14 +85,13 @@ class MCPlusBuilder {
8585
}
8686

8787
/// Extract annotation index from immediate operand value.
88-
static unsigned extractAnnotationIndex(int64_t ImmValue) {
88+
static uint8_t extractAnnotationIndex(int64_t ImmValue) {
8989
return ImmValue >> 56;
9090
}
9191

9292
/// Extract annotation value from immediate operand value.
9393
static int64_t extractAnnotationValue(int64_t ImmValue) {
94-
ImmValue &= 0xff'ffff'ffff'ffff;
95-
return (ImmValue << 8) >> 8;
94+
return SignExtend64<56>(ImmValue & 0xff'ffff'ffff'ffffULL);
9695
}
9796

9897
MCInst *getAnnotationInst(const MCInst &Inst) const {

bolt/unittests/Core/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ set(LLVM_LINK_COMPONENTS
33
BOLTRewrite
44
DebugInfoDWARF
55
Object
6+
MC
67
${LLVM_TARGETS_TO_BUILD}
78
)
89

bolt/unittests/Core/MCPlusBuilder.cpp

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,3 +110,29 @@ TEST_P(MCPlusBuilderTester, AliasSmallerAX) {
110110
}
111111

112112
#endif // X86_AVAILABLE
113+
114+
TEST_P(MCPlusBuilderTester, Annotation) {
115+
MCInst Inst;
116+
bool Success = BC->MIB->createTailCall(Inst, BC->Ctx->createNamedTempSymbol(),
117+
BC->Ctx.get());
118+
ASSERT_TRUE(Success);
119+
MCSymbol *LPSymbol = BC->Ctx->createNamedTempSymbol("LP");
120+
uint64_t Value = INT32_MIN;
121+
// Test encodeAnnotationImm using this indirect way
122+
BC->MIB->addEHInfo(Inst, MCPlus::MCLandingPad(LPSymbol, Value));
123+
// Round-trip encoding-decoding check for negative values
124+
Optional<MCPlus::MCLandingPad> EHInfo = BC->MIB->getEHInfo(Inst);
125+
ASSERT_TRUE(EHInfo.hasValue());
126+
MCPlus::MCLandingPad LP = EHInfo.getValue();
127+
uint64_t DecodedValue = LP.second;
128+
ASSERT_EQ(Value, DecodedValue);
129+
130+
// Large int64 should trigger an out of range assertion
131+
Value = 0x1FF'FFFF'FFFF'FFFFULL;
132+
Inst.clear();
133+
Success = BC->MIB->createTailCall(Inst, BC->Ctx->createNamedTempSymbol(),
134+
BC->Ctx.get());
135+
ASSERT_TRUE(Success);
136+
ASSERT_DEATH(BC->MIB->addEHInfo(Inst, MCPlus::MCLandingPad(LPSymbol, Value)),
137+
"annotation value out of range");
138+
}

0 commit comments

Comments
 (0)