[PATCH] D120260: [BOLT][NFC] Fix undefined behavior in encodeAnnotationImm

[Amir Ayupov via Phabricator via llvm-commits]

Amir updated this revision to Diff 410917.
Amir added a comment.

Use extractAnnotationValue in the check


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D120260/new/

https://reviews.llvm.org/D120260

Files:
  bolt/include/bolt/Core/MCPlusBuilder.h
  bolt/unittests/Core/CMakeLists.txt
  bolt/unittests/Core/MCPlusBuilder.cpp


Index: bolt/unittests/Core/MCPlusBuilder.cpp
===================================================================
--- bolt/unittests/Core/MCPlusBuilder.cpp
+++ bolt/unittests/Core/MCPlusBuilder.cpp
@@ -109,4 +109,32 @@
   testRegAliases(Triple::x86_64, X86::AX, AliasesAX, AliasesAXCount, true);
 }
 
+TEST_P(MCPlusBuilderTester, Annotation) {
+  if (GetParam() != Triple::x86_64)
+    GTEST_SKIP();
+  MCInst Inst;
+  bool Success = BC->MIB->createCall(Inst, BC->Ctx->createNamedTempSymbol(),
+                                     BC->Ctx.get());
+  ASSERT_TRUE(Success);
+  MCSymbol *LPSymbol = BC->Ctx->createNamedTempSymbol("LP");
+  uint64_t Value = INT32_MIN;
+  // Test encodeAnnotationImm using this indirect way
+  BC->MIB->addEHInfo(Inst, MCPlus::MCLandingPad(LPSymbol, Value));
+  // Round-trip encoding-decoding check for negative values
+  Optional<MCPlus::MCLandingPad> EHInfo = BC->MIB->getEHInfo(Inst);
+  ASSERT_TRUE(EHInfo.hasValue());
+  MCPlus::MCLandingPad LP = EHInfo.getValue();
+  uint64_t DecodedValue = LP.second;
+  ASSERT_EQ(Value, DecodedValue);
+
+  // Large int64 should trigger an out of range assertion
+  Value = 0x1FF'FFFF'FFFF'FFFFULL;
+  MCInst Inst2;
+  bool Success2 = BC->MIB->createCall(Inst2, BC->Ctx->createNamedTempSymbol(),
+                                      BC->Ctx.get());
+  ASSERT_TRUE(Success2);
+  ASSERT_DEATH(BC->MIB->addEHInfo(Inst2, MCPlus::MCLandingPad(LPSymbol, Value)),
+               "annotation value out of range");
+}
+
 #endif // X86_AVAILABLE
Index: bolt/unittests/Core/CMakeLists.txt
===================================================================
--- bolt/unittests/Core/CMakeLists.txt
+++ bolt/unittests/Core/CMakeLists.txt
@@ -3,6 +3,7 @@
   BOLTRewrite
   DebugInfoDWARF
   Object
+  MC
   ${LLVM_TARGETS_TO_BUILD}
   )
 
Index: bolt/include/bolt/Core/MCPlusBuilder.h
===================================================================
--- bolt/include/bolt/Core/MCPlusBuilder.h
+++ bolt/include/bolt/Core/MCPlusBuilder.h
@@ -74,9 +74,9 @@
   AllocatorIdTy MaxAllocatorId = 0;
 
   /// We encode Index and Value into a 64-bit immediate operand value.
-  static int64_t encodeAnnotationImm(unsigned Index, int64_t Value) {
-    assert(Index < 256 && "annotation index max value exceeded");
-    assert((Value == (Value << 8) >> 8) && "annotation value out of range");
+  static int64_t encodeAnnotationImm(uint8_t Index, int64_t Value) {
+    if (LLVM_UNLIKELY(Value != extractAnnotationValue(Value)))
+      report_fatal_error("annotation value out of range");
 
     Value &= 0xff'ffff'ffff'ffff;
     Value |= (int64_t)Index << 56;
@@ -85,14 +85,13 @@
   }
 
   /// Extract annotation index from immediate operand value.
-  static unsigned extractAnnotationIndex(int64_t ImmValue) {
+  static uint8_t extractAnnotationIndex(int64_t ImmValue) {
     return ImmValue >> 56;
   }
 
   /// Extract annotation value from immediate operand value.
   static int64_t extractAnnotationValue(int64_t ImmValue) {
-    ImmValue &= 0xff'ffff'ffff'ffff;
-    return (ImmValue << 8) >> 8;
+    return SignExtend64<56>(ImmValue & 0xff'ffff'ffff'ffffULL);
   }
 
   MCInst *getAnnotationInst(const MCInst &Inst) const {


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D120260.410917.patch
Type: text/x-patch
Size: 3182 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20220223/8cb78471/attachment.bin>