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

[edward-jones]

Summary

RISCVAsmBackend::shouldInsertExtraNopBytesForCodeAlign() assumed that the align specified would be greater than or equal to the minimum nop length, but that is not always the case - for example if a user specifies ".align 0" in assembly.

[lewis-revill]

lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp

320 ↗

(On Diff #204541)

Shouldn't make any functional difference but can we return false instead? Seems more prudent to say we 'shouldn't emit nops' rather than saying we should but the size should be 0?

[edward-jones]

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)

[asb]

LGTM, thanks!

[edward-jones]

Rebased

llvm/trunk/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp

bool RISCVAsmBackend::shouldInsertExtraNopBytesForCodeAlign(
     const MCAlignFragment &AF, unsigned &Size) {
   // Calculate Nops Size only when linker relaxation enabled.
   if (!STI.getFeatureBits()[RISCV::FeatureRelax])
     return false;
 
   bool HasStdExtC = STI.getFeatureBits()[RISCV::FeatureStdExtC];
   unsigned MinNopLen = HasStdExtC ? 2 : 4;
 
+  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
 // position of Nops and the total bytes of the Nops have been inserted
 // when linker relaxation enabled.
 // The function insert fixup_riscv_align fixup which eventually will
 // transfer to R_RISCV_ALIGN relocation type.
 bool RISCVAsmBackend::shouldInsertFixupForCodeAlign(MCAssembler &Asm,
                                                     const MCAsmLayout &Layout,

llvm/trunk/test/MC/RISCV/align.s

 # RELAX-INST:  01 01
 # RELAX-INST:  01 01
 # C-EXT-RELAX-RELOC-NOT: R_RISCV_ALIGN - 0xE
 # C-EXT-RELAX-INST:  01 01
 # C-EXT-INST:  01 01
   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
   .p2align        3
 # RELAX-RELOC-NOT: R_RISCV_ALIGN
 # C-EXT-RELAX-RELOC-NOT: R_RISCV_ALIGN
 data1:
   .word 7
   .p2align        4
 # RELAX-RELOC-NOT: R_RISCV_ALIGN
 # C-EXT-RELAX-RELOC-NOT: R_RISCV_ALIGN
 data2:
   .word 9