From 216002c4bb708e6d6fd1895c8ea636470961f824 Mon Sep 17 00:00:00 2001 From: Adrian Prantl Date: Tue, 25 Jan 2022 13:29:22 -0800 Subject: [PATCH] Fix UB in DwarfExpression::emitLegacyZExt() 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. Differential Revision: https://reviews.llvm.org/D118183 --- .../CodeGen/AsmPrinter/DwarfExpression.cpp | 22 ++++++++++++++++--- llvm/test/DebugInfo/X86/convert-debugloc.ll | 9 ++++++++ 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/llvm/lib/CodeGen/AsmPrinter/DwarfExpression.cpp b/llvm/lib/CodeGen/AsmPrinter/DwarfExpression.cpp index 37407c98e75f..ee932d105107 100644 --- a/llvm/lib/CodeGen/AsmPrinter/DwarfExpression.cpp +++ b/llvm/lib/CodeGen/AsmPrinter/DwarfExpression.cpp @@ -681,9 +681,25 @@ void DwarfExpression::emitLegacySExt(unsigned FromBits) { } void DwarfExpression::emitLegacyZExt(unsigned FromBits) { - // (X & (1 << FromBits - 1)) - emitOp(dwarf::DW_OP_constu); - emitUnsigned((1ULL << FromBits) - 1); + // Heuristic to decide the most efficient encoding. + // A ULEB can encode 7 1-bits per byte. + if (FromBits / 7 < 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); } diff --git a/llvm/test/DebugInfo/X86/convert-debugloc.ll b/llvm/test/DebugInfo/X86/convert-debugloc.ll index 21e41dcc4c2a..2ff6e9662190 100644 --- a/llvm/test/DebugInfo/X86/convert-debugloc.ll +++ b/llvm/test/DebugInfo/X86/convert-debugloc.ll @@ -64,11 +64,17 @@ ; 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 @@ -81,6 +87,7 @@ entry: ;; 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 } @@ -111,3 +118,5 @@ declare void @llvm.dbg.value(metadata, metadata, metadata) !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)