[PATCH] D63274: [RISCV] Avoid overflow when determining number of nops for code align

[Edward Jones via Phabricator via llvm-commits]

edward-jones updated this revision to Diff 204576.
edward-jones added a comment.

Incorporated Lewis' suggested fix.

I realized that the use of this in shouldInsertFixupForCodeAlign() doesn't actually check the return value, and with the changes I've made here it means that shouldInsertFixupForCodeAlign() could read uninitialized memory. I've submitted a separate patch to fix that problem (D63285 <https://reviews.llvm.org/D63285>)


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

https://reviews.llvm.org/D63274

Files:
  lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp
  test/MC/RISCV/align.s


Index: test/MC/RISCV/align.s
===================================================================
--- test/MC/RISCV/align.s
+++ test/MC/RISCV/align.s
@@ -90,6 +90,13 @@
 	ret
 # NORELAX-RELOC-NOT: R_RISCV
 # C-EXT-NORELAX-RELOC-NOT: R_RISCV
+# Code alignment of a byte size less than the size of a nop must be treated
+# as no alignment. This used to trigger a fatal error with relaxation enabled
+# as the calculation to emit the worst-case sequence of nops would overflow.
+	.p2align        1
+	add	a0, a0, a1
+	.p2align        0
+	add	a0, a0, a1
 # We only need to insert R_RISCV_ALIGN for code section
 # when the linker relaxation enabled.
         .data
Index: lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp
===================================================================
--- lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp
+++ lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp
@@ -317,8 +317,12 @@
   bool HasStdExtC = STI.getFeatureBits()[RISCV::FeatureStdExtC];
   unsigned MinNopLen = HasStdExtC ? 2 : 4;
 
-  Size = AF.getAlignment() - MinNopLen;
-  return true;
+  if (AF.getAlignment() <= MinNopLen) {
+    return false;
+  } else {
+    Size = AF.getAlignment() - MinNopLen;
+    return true;
+  }
 }
 
 // We need to insert R_RISCV_ALIGN relocation type to indicate the


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D63274.204576.patch
Type: text/x-patch
Size: 1293 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190613/a879eb8c/attachment.bin>