Fix UB in DwarfExpression::emitLegacyZExt()

[aprantl]

Summary

A shift-left > 63 triggers a UBSAN failure. This patch kicks the can down the road (to the consumer) by emitting a more compact representation of the shift computation in DWARF expressions.

[aprantl]

Apologies, I accidentally pushed this commit instead of a different one. Reverted in 3efa016d4c1a, this one still needs to be reviewed!

[thakis]

Could this have caused http://45.33.8.238/linux/66347/step_7.txt ?

[thakis]

Looks like https://reviews.llvm.org/D117965 caused that. Sorry for the noise.

[JDevlieghere]

llvm/lib/CodeGen/AsmPrinter/DwarfExpression.cpp

687

I had some trouble figuring out what the 1+1+1+1+1 was supposed to represent. In case anyone is wondering it's 1 byte per DWARF opcode and 1 byte for the small ULEB128 in the alternative encoding below. Maybe we should make that more clear in the comment or used a named temporary/

Also, should we use (FromBits+6)/7 to ensure we always round up?

[aprantl]

Address review feedback!

[aprantl]

Patch test.

[JDevlieghere]

Thanks Adrian. LGTM

[thakis]

Looks like this broke check-llvm: http://45.33.8.238/linux/66479/step_12.txt

[dyung]

Hi Adrian, the test you updated in your commit is failing on the PS4 linux bot, can you take a look?

https://lab.llvm.org/buildbot/#/builders/139/builds/16906

[thakis]

On the main llvm waterfall too: https://lab.llvm.org/buildbot/#/builders/57/builds/14335

[xbolva00]

llvm/lib/CodeGen/AsmPrinter/DwarfExpression.cpp

687

Agree with you. This should be reworked /commented more.

llvm/lib/CodeGen/AsmPrinter/DwarfExpression.cpp

 #include "DwarfCompileUnit.h"
 #include "llvm/ADT/APInt.h"
 #include "llvm/ADT/SmallBitVector.h"
 #include "llvm/BinaryFormat/Dwarf.h"
 #include "llvm/CodeGen/Register.h"
 #include "llvm/CodeGen/TargetRegisterInfo.h"
 #include "llvm/IR/DataLayout.h"
 #include "llvm/Support/ErrorHandling.h"
+#include "llvm/Support/MathExtras.h"
 #include <algorithm>
 
 using namespace llvm;
 
 #define DEBUG_TYPE "dwarfdebug"
 
 void DwarfExpression::emitConstu(uint64_t Value) {
   if (Value < 32)
void DwarfExpression::emitLegacySExt(unsigned FromBits) {
   emitOp(dwarf::DW_OP_mul);
   emitOp(dwarf::DW_OP_constu);
   emitUnsigned(FromBits);
   emitOp(dwarf::DW_OP_shl);
   emitOp(dwarf::DW_OP_or);
 }
 
 void DwarfExpression::emitLegacyZExt(unsigned FromBits) {
+  // Heuristic to decide the most efficient encoding.  A LEB128 can
+  // encode 7 bits per byte. A mask for FromBits bits encoded in
+  // LEB128 this takes FromBits / 7 bytes and 1 byte for the OP_constu.
+  if (divideCeil(FromBits, 7) + 1 < 1 /* lit1, ...*/ + 1 + 1 + 1 + 1 + 1) {
     // (X & (1 << FromBits - 1))
     emitOp(dwarf::DW_OP_constu);
     emitUnsigned((1ULL << FromBits) - 1);
+  } else {
+    // Note that the DWARF 4 stack consists of pointer-sized elements,
+    // so technically it doesn't make sense to shift left more than 64
+    // bits. We leave that for the consumer to decide though. LLDB for
+    // example uses APInt for the stack elements and can still deal
+    // with this.
+    emitOp(dwarf::DW_OP_lit1);
+    emitOp(dwarf::DW_OP_constu);
+    emitUnsigned(FromBits);
+    emitOp(dwarf::DW_OP_shl);
+    emitOp(dwarf::DW_OP_lit1);
+    emitOp(dwarf::DW_OP_minus);
+  }
   emitOp(dwarf::DW_OP_and);
 }
 
 void DwarfExpression::addWasmLocation(unsigned Index, uint64_t Offset) {
   emitOp(dwarf::DW_OP_WASM_location);
   emitUnsigned(Index == 4/*TI_LOCAL_INDIRECT*/ ? 0/*TI_LOCAL*/ : Index);
   emitUnsigned(Offset);
   if (Index == 4 /*TI_LOCAL_INDIRECT*/) {
     assert(LocationKind == Unknown);
     LocationKind = Memory;
   } else {
     assert(LocationKind == Implicit || LocationKind == Unknown);
     LocationKind = Implicit;
   }
 }

llvm/test/DebugInfo/X86/convert-debugloc.ll

 ; Test DW_OP_convert + Split DWARF
 ; RUN: llc -mtriple=x86_64-pc-linux-gnu -dwarf-version=5 -filetype=obj -O0 < %s -debugger-tune=lldb -dwarf-op-convert=Enable -split-dwarf-file=baz.dwo | llvm-dwarfdump - \
 ; RUN:   | FileCheck %s --check-prefix=CONV --check-prefix=SPLITCONV --check-prefix=SPLIT "--implicit-check-not={{DW_TAG|NULL}}"
 
 ; RUN: llc -mtriple=x86_64 -dwarf-version=5 -filetype=obj -O0 < %s -debugger-tune=gdb  | llvm-dwarfdump -v -debug-info - \
 ; RUN:   | FileCheck %s --check-prefix=VERBOSE --check-prefix=CONV "--implicit-check-not={{DW_TAG|NULL}}"
 
 
-; SPLITCONV: Compile Unit:{{.*}} DWO_id = 0x62f17241069b1fa3
+; SPLITCONV: Compile Unit:{{.*}} DWO_id = 0x24191746f389535f
 ; SPLIT: DW_TAG_skeleton_unit
 
 ; CONV: DW_TAG_compile_unit
 ; CONV:[[SIG8:.*]]:   DW_TAG_base_type
 ; CONV-NEXT:DW_AT_name {{.*}}"DW_ATE_signed_8")
 ; CONV-NEXT:DW_AT_encoding {{.*}}DW_ATE_signed)
 ; CONV-NEXT:DW_AT_byte_size {{.*}}0x01)
 ; CONV-NOT: DW_AT
 ; CONV:[[SIG32:.*]]:   DW_TAG_base_type
 ; CONV-NEXT:DW_AT_name {{.*}}"DW_ATE_signed_32")
 ; CONV-NEXT:DW_AT_encoding {{.*}}DW_ATE_signed)
 ; CONV-NEXT:DW_AT_byte_size {{.*}}0x04)
 ; CONV-NOT: DW_AT
+; CONV:   DW_TAG_base_type
+; CONV:   DW_TAG_base_type
 ; CONV:   DW_TAG_subprogram
 ; CONV:     DW_TAG_formal_parameter
 ; CONV:     DW_TAG_variable
 ; CONV:     DW_AT_location {{.*}}DW_OP_constu 0x20, DW_OP_lit0, DW_OP_plus, DW_OP_convert (
 ; VERBOSE-SAME: [[SIG8]] ->
 ; CONV-SAME: [[SIG8]]) "DW_ATE_signed_8", DW_OP_convert (
 ; VERBOSE-SAME: [[SIG32]] ->
 ; CONV-SAME: [[SIG32]]) "DW_ATE_signed_32", DW_OP_stack_value)
 ; CONV:       DW_AT_name {{.*}}"y")
+; CONV:     DW_TAG_variable
 ; CONV:     NULL
 ; CONV:   DW_TAG_base_type
 ; CONV:     DW_AT_name {{.*}}"signed char")
 ; CONV:   DW_TAG_base_type
 ; CONV:     DW_AT_name {{.*}}"int")
+; CONV:   DW_TAG_base_type
+; CONV:     DW_AT_name {{.*}}"unsigned long long")
 ; CONV:   NULL
 
 ; NOCONV: DW_TAG_compile_unit
 ; NOCONV:   DW_TAG_subprogram
 ; NOCONV:     DW_TAG_formal_parameter
 ; NOCONV:     DW_TAG_variable
 ; NOCONV:       DW_AT_location (
 ; NOCONV:         {{.*}}, DW_OP_dup, DW_OP_constu 0x7, DW_OP_shr, DW_OP_lit0, DW_OP_not, DW_OP_mul, DW_OP_constu 0x8, DW_OP_shl, DW_OP_or, DW_OP_stack_value)
 ; NOCONV:       DW_AT_name ("y")
+; NOCONV:     DW_TAG_variable
+; NOCONV:       DW_AT_location (
+; NOCONV:         DW_OP_constu 0x40, DW_OP_lit0, DW_OP_plus, DW_OP_lit1, DW_OP_constu 0x40, DW_OP_shl, DW_OP_lit1, DW_OP_minus, DW_OP_and, DW_OP_stack_value)
+; NOCONV:       DW_AT_name ("z")
 ; NOCONV:     NULL
 ; NOCONV:   DW_TAG_base_type
 ; NOCONV:     DW_AT_name ("signed char")
 ; NOCONV:   DW_TAG_base_type
 ; NOCONV:     DW_AT_name ("int")
+; NOCONV:   DW_TAG_base_type
+; NOCONV:     DW_AT_name ("unsigned long long")
 ; NOCONV:   NULL
 
 
 ; Function Attrs: noinline nounwind uwtable
 define dso_local signext i8 @foo(i8 signext %x) !dbg !7 {
 entry:
   call void @llvm.dbg.value(metadata i8 %x, metadata !11, metadata !DIExpression()), !dbg !12
 ;; This test depends on "convert" surviving all the way to the final object.
 ;; So, insert something before DW_OP_LLVM_convert that the expression folder
 ;; will not attempt to eliminate.  At the moment, only "convert" ops are folded.
 ;; If you have to change the expression, the expected DWO_id also changes.
   call void @llvm.dbg.value(metadata i8 32, metadata !13, metadata !DIExpression(DW_OP_lit0, DW_OP_plus, DW_OP_LLVM_convert, 8, DW_ATE_signed, DW_OP_LLVM_convert, 32, DW_ATE_signed, DW_OP_stack_value)), !dbg !15
+  call void @llvm.dbg.value(metadata i8 64, metadata !17, metadata !DIExpression(DW_OP_lit0, DW_OP_plus, DW_OP_LLVM_convert, 64, DW_ATE_unsigned, DW_OP_LLVM_convert, 128, DW_ATE_unsigned, DW_OP_LLVM_convert, 64, DW_ATE_unsigned, DW_OP_stack_value)), !dbg !15
   ret i8 %x, !dbg !16
 }
 
 ; Function Attrs: nounwind readnone speculatable
 declare void @llvm.dbg.declare(metadata, metadata, metadata)
 
 ; Function Attrs: nounwind readnone speculatable
 declare void @llvm.dbg.value(metadata, metadata, metadata)

 !9 = !{!10, !10}
 !10 = !DIBasicType(name: "signed char", size: 8, encoding: DW_ATE_signed_char)
 !11 = !DILocalVariable(name: "x", arg: 1, scope: !7, file: !1, line: 1, type: !10)
 !12 = !DILocation(line: 1, column: 29, scope: !7)
 !13 = !DILocalVariable(name: "y", scope: !7, file: !1, line: 3, type: !14)
 !14 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
 !15 = !DILocation(line: 3, column: 14, scope: !7)
 !16 = !DILocation(line: 4, column: 3, scope: !7)
+!17 = !DILocalVariable(name: "z", scope: !7, file: !1, line: 3, type: !18)
+!18 = !DIBasicType(name: "unsigned long long", size: 64, encoding: DW_ATE_unsigned)