[Lldb-commits] [libc] [lldb] [llvm] [mlir] [polly] Fix simple bugs (PR #117151)

via lldb-commits lldb-commits at lists.llvm.org
Thu Nov 21 04:38:20 PST 2024


https://github.com/ChuvakHome created https://github.com/llvm/llvm-project/pull/117151

Fix for some mistakes in source code found using PVS Studio.

Inspired by: https://pvs-studio.com/en/blog/posts/cpp/1188/

>From 4bd798da2681d1648c19d10da1160b01e5445804 Mon Sep 17 00:00:00 2001
From: Feng Zou <feng.zou at intel.com>
Date: Thu, 21 Nov 2024 16:13:11 +0800
Subject: [PATCH 1/2] [X86][MC] Add R_X86_64_CODE_4_GOTTPOFF (#116633)

For

  mov name at GOTTPOFF(%rip), %reg
  add name at GOTTPOFF(%rip), %reg

add

  `R_X86_64_CODE_4_GOTTPOFF` = 44

if the instruction starts at 4 bytes before the relocation offset. It's
similar to R_X86_64_GOTTPOFF.

Linker can treat `R_X86_64_CODE_4_GOTTPOFF` as `R_X86_64_GOTTPOFF` or
convert the instructions above to

  mov $name at tpoff, %reg
  add $name at tpoff, %reg

if the first byte of the instruction at the relocation `offset - 4` is
`0xd5` (namely, encoded w/REX2 prefix) when possible.

Binutils patch:
https://github.com/bminor/binutils-gdb/commit/a533c8df598b5ef99c54a13e2b137c98b34b043c
Binutils mailthread:
https://sourceware.org/pipermail/binutils/2023-December/131463.html
ABI discussion:
https://groups.google.com/g/x86-64-abi/c/ACwD-UQXVDs/m/vrgTenKyFwAJ
Blog: https://kanrobert.github.io/rfc/All-about-APX-relocation

Fix bug with parenthesis
---
 bolt/lib/Passes/ShrinkWrapping.cpp            |  4 +-
 .../llvm/BinaryFormat/ELFRelocs/x86_64.def    |  1 +
 .../X86/MCTargetDesc/X86ELFObjectWriter.cpp   |  3 +
 llvm/test/MC/ELF/relocation.s                 | 67 ++++++++++---------
 4 files changed, 41 insertions(+), 34 deletions(-)

diff --git a/bolt/lib/Passes/ShrinkWrapping.cpp b/bolt/lib/Passes/ShrinkWrapping.cpp
index 176321c58dc903..bc4e7e6e386a0e 100644
--- a/bolt/lib/Passes/ShrinkWrapping.cpp
+++ b/bolt/lib/Passes/ShrinkWrapping.cpp
@@ -78,8 +78,8 @@ void CalleeSavedAnalysis::analyzeSaves() {
         // probably dealing with a parameter passed in a stack -- do not mess
         // with it
         if (SRU.isStoreUsed(*FIE,
-                            Prev ? SRU.expr_begin(*Prev) : SRU.expr_begin(BB)),
-            /*IncludeLocalAccesses=*/false) {
+                            Prev ? SRU.expr_begin(*Prev) : SRU.expr_begin(BB),
+            /*IncludeLocalAccesses=*/false)) {
           BlacklistedRegs.set(FIE->RegOrImm);
           CalleeSaved.reset(FIE->RegOrImm);
           Prev = &Inst;
diff --git a/llvm/include/llvm/BinaryFormat/ELFRelocs/x86_64.def b/llvm/include/llvm/BinaryFormat/ELFRelocs/x86_64.def
index 43473d47e32819..94b1ad9c1f9464 100644
--- a/llvm/include/llvm/BinaryFormat/ELFRelocs/x86_64.def
+++ b/llvm/include/llvm/BinaryFormat/ELFRelocs/x86_64.def
@@ -44,3 +44,4 @@ ELF_RELOC(R_X86_64_IRELATIVE,   37)
 ELF_RELOC(R_X86_64_GOTPCRELX,   41)
 ELF_RELOC(R_X86_64_REX_GOTPCRELX,    42)
 ELF_RELOC(R_X86_64_CODE_4_GOTPCRELX,    43)
+ELF_RELOC(R_X86_64_CODE_4_GOTTPOFF,    44)
diff --git a/llvm/lib/Target/X86/MCTargetDesc/X86ELFObjectWriter.cpp b/llvm/lib/Target/X86/MCTargetDesc/X86ELFObjectWriter.cpp
index a57b1335d1437a..aa02934cc99963 100644
--- a/llvm/lib/Target/X86/MCTargetDesc/X86ELFObjectWriter.cpp
+++ b/llvm/lib/Target/X86/MCTargetDesc/X86ELFObjectWriter.cpp
@@ -197,6 +197,9 @@ static unsigned getRelocType64(MCContext &Ctx, SMLoc Loc,
     return ELF::R_X86_64_TLSGD;
   case MCSymbolRefExpr::VK_GOTTPOFF:
     checkIs32(Ctx, Loc, Type);
+    if ((unsigned)Kind == X86::reloc_riprel_4byte_movq_load_rex2 ||
+        (unsigned)Kind == X86::reloc_riprel_4byte_relax_rex2)
+      return ELF::R_X86_64_CODE_4_GOTTPOFF;
     return ELF::R_X86_64_GOTTPOFF;
   case MCSymbolRefExpr::VK_TLSLD:
     checkIs32(Ctx, Loc, Type);
diff --git a/llvm/test/MC/ELF/relocation.s b/llvm/test/MC/ELF/relocation.s
index 80b671aa2c859e..88301f8447bc2a 100644
--- a/llvm/test/MC/ELF/relocation.s
+++ b/llvm/test/MC/ELF/relocation.s
@@ -19,6 +19,8 @@ bar:
         movq	bar, %rdx         # R_X86_64_32S
 .long bar                         # R_X86_64_32
         leaq	foo at GOTTPOFF(%rip), %rax # R_X86_64_GOTTPOFF
+        movq    foo at GOTTPOFF(%rip), %r31 # R_X86_64_CODE_4_GOTTPOFF
+        addq    foo at GOTTPOFF(%rip), %r31 # R_X86_64_CODE_4_GOTTPOFF
         leaq	foo at TLSGD(%rip), %rax    # R_X86_64_TLSGD
         leaq	foo at TPOFF(%rax), %rax    # R_X86_64_TPOFF32
         leaq	foo at TLSLD(%rip), %rdi    # R_X86_64_TLSLD
@@ -67,7 +69,6 @@ pr24486:
 weak_sym:
         .long  pr23272-weak_sym
 
-
 // CHECK:        Section {
 // CHECK:          Name: .rela.text
 // CHECK:          Relocations [
@@ -78,37 +79,39 @@ weak_sym:
 // CHECK-NEXT:       0x22 R_X86_64_32S      .text
 // CHECK-NEXT:       0x26 R_X86_64_32       .text
 // CHECK-NEXT:       0x2D R_X86_64_GOTTPOFF foo 0xFFFFFFFFFFFFFFFC
-// CHECK-NEXT:       0x34 R_X86_64_TLSGD    foo 0xFFFFFFFFFFFFFFFC
-// CHECK-NEXT:       0x3B R_X86_64_TPOFF32  foo 0x0
-// CHECK-NEXT:       0x42 R_X86_64_TLSLD    foo 0xFFFFFFFFFFFFFFFC
-// CHECK-NEXT:       0x49 R_X86_64_DTPOFF32 foo 0x0
-// CHECK-NEXT:       0x4F R_X86_64_GOT64 foo 0x0
-// CHECK-NEXT:       0x59 R_X86_64_GOTOFF64 foo 0x0
-// CHECK-NEXT:       0x62 R_X86_64_32S .text 0x0
-// CHECK-NEXT:       0x69 R_X86_64_PC32 foo 0xFFFFFFFFFFFFFFFC
-// CHECK-NEXT:       0x70 R_X86_64_PC32 foo 0x70
-// CHECK-NEXT:       0x77 R_X86_64_32S .text 0x0
-// CHECK-NEXT:       0x7B R_X86_64_DTPOFF64 foo 0x0
-// CHECK-NEXT:       0x85 R_X86_64_TPOFF64 baz 0x0
-// CHECK-NEXT:       0x8D R_X86_64_PC16 foo 0x8D
-// CHECK-NEXT:       0x8F R_X86_64_PC8 foo 0x8F
-// CHECK-NEXT:       0x91 R_X86_64_PLT32 foo 0xFFFFFFFFFFFFFFFC
-// CHECK-NEXT:       0x98 R_X86_64_PC32 foo 0xFFFFFFFFFFFFFFFB
-// CHECK-NEXT:       0x9F R_X86_64_GOTPC32 _GLOBAL_OFFSET_TABLE_ 0x3
-// CHECK-NEXT:       0xA6 R_X86_64_GOTPC32 _GLOBAL_OFFSET_TABLE_ 0xFFFFFFFFFFFFFFFC
-// CHECK-NEXT:       0xAB R_X86_64_GOTPC32 _GLOBAL_OFFSET_TABLE_ 0x1
-// CHECK-NEXT:       0xB1 R_X86_64_GOTPC64 _GLOBAL_OFFSET_TABLE_ 0x2
-// CHECK-NEXT:       0xB9 R_X86_64_SIZE64 blah 0x0
-// CHECK-NEXT:       0xC1 R_X86_64_SIZE64 blah 0x20
-// CHECK-NEXT:       0xC9 R_X86_64_SIZE64 blah 0xFFFFFFFFFFFFFFE0
-// CHECK-NEXT:       0xD4 R_X86_64_SIZE32 blah 0x0
-// CHECK-NEXT:       0xDB R_X86_64_SIZE32 blah 0x20
-// CHECK-NEXT:       0xE2 R_X86_64_SIZE32 blah 0xFFFFFFFFFFFFFFE0
-// CHECK-NEXT:       0xE6 R_X86_64_GOTPCREL foo 0x0
-// CHECK-NEXT:       0xEA R_X86_64_PLT32 foo 0x0
-// CHECK-NEXT:       0xFE R_X86_64_32 .text 0xFE
-// CHECK-NEXT:       0x103 R_X86_64_PC16 pr23771 0xFFFFFFFFFFFFFFFE
-// CHECK-NEXT:       0x105 R_X86_64_PC32 pr23272 0x0
+// CHECK-NEXT:       0x35 R_X86_64_CODE_4_GOTTPOFF foo 0xFFFFFFFFFFFFFFFC
+// CHECK-NEXT:       0x3D R_X86_64_CODE_4_GOTTPOFF foo 0xFFFFFFFFFFFFFFFC
+// CHECK-NEXT:       0x44 R_X86_64_TLSGD    foo 0xFFFFFFFFFFFFFFFC
+// CHECK-NEXT:       0x4B R_X86_64_TPOFF32  foo 0x0
+// CHECK-NEXT:       0x52 R_X86_64_TLSLD    foo 0xFFFFFFFFFFFFFFFC
+// CHECK-NEXT:       0x59 R_X86_64_DTPOFF32 foo 0x0
+// CHECK-NEXT:       0x5F R_X86_64_GOT64 foo 0x0
+// CHECK-NEXT:       0x69 R_X86_64_GOTOFF64 foo 0x0
+// CHECK-NEXT:       0x72 R_X86_64_32S .text 0x0
+// CHECK-NEXT:       0x79 R_X86_64_PC32 foo 0xFFFFFFFFFFFFFFFC
+// CHECK-NEXT:       0x80 R_X86_64_PC32 foo 0x80
+// CHECK-NEXT:       0x87 R_X86_64_32S .text 0x0
+// CHECK-NEXT:       0x8B R_X86_64_DTPOFF64 foo 0x0
+// CHECK-NEXT:       0x95 R_X86_64_TPOFF64 baz 0x0
+// CHECK-NEXT:       0x9D R_X86_64_PC16 foo 0x9D
+// CHECK-NEXT:       0x9F R_X86_64_PC8 foo 0x9F
+// CHECK-NEXT:       0xA1 R_X86_64_PLT32 foo 0xFFFFFFFFFFFFFFFC
+// CHECK-NEXT:       0xA8 R_X86_64_PC32 foo 0xFFFFFFFFFFFFFFFB
+// CHECK-NEXT:       0xAF R_X86_64_GOTPC32 _GLOBAL_OFFSET_TABLE_ 0x3
+// CHECK-NEXT:       0xB6 R_X86_64_GOTPC32 _GLOBAL_OFFSET_TABLE_ 0xFFFFFFFFFFFFFFFC
+// CHECK-NEXT:       0xBB R_X86_64_GOTPC32 _GLOBAL_OFFSET_TABLE_ 0x1
+// CHECK-NEXT:       0xC1 R_X86_64_GOTPC64 _GLOBAL_OFFSET_TABLE_ 0x2
+// CHECK-NEXT:       0xC9 R_X86_64_SIZE64 blah 0x0
+// CHECK-NEXT:       0xD1 R_X86_64_SIZE64 blah 0x20
+// CHECK-NEXT:       0xD9 R_X86_64_SIZE64 blah 0xFFFFFFFFFFFFFFE0
+// CHECK-NEXT:       0xE4 R_X86_64_SIZE32 blah 0x0
+// CHECK-NEXT:       0xEB R_X86_64_SIZE32 blah 0x20
+// CHECK-NEXT:       0xF2 R_X86_64_SIZE32 blah 0xFFFFFFFFFFFFFFE0
+// CHECK-NEXT:       0xF6 R_X86_64_GOTPCREL foo 0x0
+// CHECK-NEXT:       0xFA R_X86_64_PLT32 foo 0x0
+// CHECK-NEXT:       0x10E R_X86_64_32 .text 0x10E
+// CHECK-NEXT:       0x113 R_X86_64_PC16 pr23771 0xFFFFFFFFFFFFFFFE
+// CHECK-NEXT:       0x115 R_X86_64_PC32 pr23272 0x0
 // CHECK-NEXT:     ]
 // CHECK-NEXT:   }
 

>From ee6ab90efad3153cd5f49f1fd3589143291ae68b Mon Sep 17 00:00:00 2001
From: timurdemenev <311683 at niuitmo.ru>
Date: Thu, 21 Nov 2024 15:33:04 +0300
Subject: [PATCH 2/2] Fix bug with parenthesis, wrong names, invalid pointer
 checking, wrong logical-or operator

---
 bolt/lib/Rewrite/LinuxKernelRewriter.cpp                        | 2 +-
 libc/fuzzing/math/Compare.h                                     | 2 +-
 .../ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp                | 2 +-
 lldb/source/Plugins/Process/Linux/IntelPTSingleBufferTrace.cpp  | 2 +-
 mlir/lib/Dialect/Linalg/IR/LinalgOps.cpp                        | 2 +-
 mlir/lib/Dialect/Vector/IR/ScalableValueBoundsConstraintSet.cpp | 2 +-
 polly/lib/External/isl/isl_local_space.c                        | 2 +-
 7 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/bolt/lib/Rewrite/LinuxKernelRewriter.cpp b/bolt/lib/Rewrite/LinuxKernelRewriter.cpp
index 03b414b71caca7..39ceeffc79c165 100644
--- a/bolt/lib/Rewrite/LinuxKernelRewriter.cpp
+++ b/bolt/lib/Rewrite/LinuxKernelRewriter.cpp
@@ -580,7 +580,7 @@ Error LinuxKernelRewriter::readORCTables() {
       // As such, we can ignore alternative ORC entries. They will be preserved
       // in the binary, but will not get printed in the instruction stream.
       Inst = BF->getInstructionContainingOffset(Offset);
-      if (Inst || BC.MIB->hasAnnotation(*Inst, "AltInst"))
+      if (Inst && BC.MIB->hasAnnotation(*Inst, "AltInst"))
         continue;
 
       return createStringError(
diff --git a/libc/fuzzing/math/Compare.h b/libc/fuzzing/math/Compare.h
index 2b84ad3ab46213..8f06ed9c8cc102 100644
--- a/libc/fuzzing/math/Compare.h
+++ b/libc/fuzzing/math/Compare.h
@@ -20,7 +20,7 @@ ValuesEqual(T x1, T x2) {
   LIBC_NAMESPACE::fputil::FPBits<T> bits2(x2);
   // If either is NaN, we want both to be NaN.
   if (bits1.is_nan() || bits2.is_nan())
-    return bits2.is_nan() && bits2.is_nan();
+    return bits1.is_nan() && bits2.is_nan();
 
   // For all other values, we want the values to be bitwise equal.
   return bits1.uintval() == bits2.uintval();
diff --git a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
index 0083b499656979..c43871b08191db 100644
--- a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
+++ b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
@@ -3278,7 +3278,7 @@ bool AppleObjCRuntimeV2::NonPointerISACache::EvaluateNonPointerISA(
       }
 
       // If the index is still out of range then this isn't a pointer.
-      if (index > m_indexed_isa_cache.size())
+      if (index >= m_indexed_isa_cache.size())
         return false;
 
       LLDB_LOGF(log, "AOCRT::NPI Evaluate(ret_isa = 0x%" PRIx64 ")",
diff --git a/lldb/source/Plugins/Process/Linux/IntelPTSingleBufferTrace.cpp b/lldb/source/Plugins/Process/Linux/IntelPTSingleBufferTrace.cpp
index 8c69989702c2aa..f7a2d1d07142ec 100644
--- a/lldb/source/Plugins/Process/Linux/IntelPTSingleBufferTrace.cpp
+++ b/lldb/source/Plugins/Process/Linux/IntelPTSingleBufferTrace.cpp
@@ -150,7 +150,7 @@ GeneratePerfEventConfigValue(bool enable_tsc,
   if (enable_tsc) {
     if (Expected<uint32_t> offset = ReadIntelPTConfigFile(
             kTSCBitOffsetFile, IntelPTConfigFileType::BitOffset))
-      config |= 1 << *offset;
+      config |= 1ULL << *offset;
     else
       return offset.takeError();
   }
diff --git a/mlir/lib/Dialect/Linalg/IR/LinalgOps.cpp b/mlir/lib/Dialect/Linalg/IR/LinalgOps.cpp
index 26d9d2b091750c..8973e87c063b33 100644
--- a/mlir/lib/Dialect/Linalg/IR/LinalgOps.cpp
+++ b/mlir/lib/Dialect/Linalg/IR/LinalgOps.cpp
@@ -523,7 +523,7 @@ class RegionBuilderHelper {
         isInteger(arg0) && arg0.getType().getIntOrFloatBitWidth() == 1;
     bool tailFloatingPoint =
         isFloatingPoint(arg0) && isFloatingPoint(arg1) && isFloatingPoint(arg2);
-    bool tailInteger = isInteger(arg0) && isInteger(arg1) && isInteger(arg1);
+    bool tailInteger = isInteger(arg0) && isInteger(arg1) && isInteger(arg2);
     OpBuilder::InsertionGuard g(builder);
     builder.setInsertionPointToEnd(&block);
     switch (ternaryFn) {
diff --git a/mlir/lib/Dialect/Vector/IR/ScalableValueBoundsConstraintSet.cpp b/mlir/lib/Dialect/Vector/IR/ScalableValueBoundsConstraintSet.cpp
index 4a826f04e1f1d2..88f02369cb7ab1 100644
--- a/mlir/lib/Dialect/Vector/IR/ScalableValueBoundsConstraintSet.cpp
+++ b/mlir/lib/Dialect/Vector/IR/ScalableValueBoundsConstraintSet.cpp
@@ -107,7 +107,7 @@ ScalableValueBoundsConstraintSet::computeScalableBound(
 
   AffineMap bound = [&] {
     if (boundType == BoundType::EQ && !invalidBound(lowerBound) &&
-        lowerBound[0] == lowerBound[0]) {
+        lowerBound[0] == upperBound[0]) {
       return lowerBound[0];
     } else if (boundType == BoundType::LB && !invalidBound(lowerBound)) {
       return lowerBound[0];
diff --git a/polly/lib/External/isl/isl_local_space.c b/polly/lib/External/isl/isl_local_space.c
index 604e276a0d1f66..1bffcd544eb735 100644
--- a/polly/lib/External/isl/isl_local_space.c
+++ b/polly/lib/External/isl/isl_local_space.c
@@ -254,7 +254,7 @@ isl_size isl_local_space_var_offset(__isl_keep isl_local_space *ls,
 	isl_space *space;
 
 	space = isl_local_space_peek_space(ls);
-	if (space < 0)
+	if (space == NULL)
 		return isl_size_error;
 	switch (type) {
 	case isl_dim_param:



More information about the lldb-commits mailing list