From 754b9ad3009f1ce5f04ec3fe84324c6dd88fdf27 Mon Sep 17 00:00:00 2001 From: David Blaikie Date: Tue, 4 Feb 2020 19:25:47 -0800 Subject: [PATCH] DebugInfo: Hash DW_OP_convert in loclists when using Split DWARF Originally committed in: 1ced28cbe75ff81f35ac2c71e941041eb3afcd00 Reverted in: f75301d16d444d8cb6810d679290df744bc79ec7 (reverted due to tests failing on non-linux/x86 targets, tests have since been generalized and specialized... since Split DWARF isn't supported on non-elf targets anyway and we have no way to run on "whatever elf target is available" so they fail on MacOS without an explicit target triple) This code was incorrectly emitting extra bytes into arbitrary parts of the object file when it was meant to be hashing them to compute the DWO ID. Follow-up patch(es) will refactor this API somewhat to make such bugs harder to introduce, hopefully. --- lib/CodeGen/AsmPrinter/ByteStreamer.h | 2 +- lib/CodeGen/AsmPrinter/DwarfDebug.cpp | 2 +- test/DebugInfo/X86/convert-loclist.ll | 19 ++++++++++++++++++- 3 files changed, 20 insertions(+), 3 deletions(-) diff --git a/lib/CodeGen/AsmPrinter/ByteStreamer.h b/lib/CodeGen/AsmPrinter/ByteStreamer.h index 09f7496cd4e..b60f68ca59b 100644 --- a/lib/CodeGen/AsmPrinter/ByteStreamer.h +++ b/lib/CodeGen/AsmPrinter/ByteStreamer.h @@ -50,7 +50,7 @@ public: } void EmitULEB128(uint64_t DWord, const Twine &Comment, unsigned PadTo) override { AP.OutStreamer->AddComment(Comment); - AP.EmitULEB128(DWord); + AP.EmitULEB128(DWord, nullptr, PadTo); } }; diff --git a/lib/CodeGen/AsmPrinter/DwarfDebug.cpp b/lib/CodeGen/AsmPrinter/DwarfDebug.cpp index 0923394ceb6..447d3fa3e20 100644 --- a/lib/CodeGen/AsmPrinter/DwarfDebug.cpp +++ b/lib/CodeGen/AsmPrinter/DwarfDebug.cpp @@ -2241,7 +2241,7 @@ void DwarfDebug::emitDebugLocEntry(ByteStreamer &Streamer, uint64_t Offset = CU->ExprRefedBaseTypes[Op.getRawOperand(I)].Die->getOffset(); assert(Offset < (1ULL << (ULEB128PadSize * 7)) && "Offset wont fit"); - Asm->EmitULEB128(Offset, nullptr, ULEB128PadSize); + Streamer.EmitULEB128(Offset, "", ULEB128PadSize); // Make sure comments stay aligned. for (unsigned J = 0; J < ULEB128PadSize; ++J) if (Comment != End) diff --git a/test/DebugInfo/X86/convert-loclist.ll b/test/DebugInfo/X86/convert-loclist.ll index a12ce5c766a..2421ee1f5e0 100644 --- a/test/DebugInfo/X86/convert-loclist.ll +++ b/test/DebugInfo/X86/convert-loclist.ll @@ -1,7 +1,24 @@ ; RUN: %llc_dwarf -filetype=obj < %s \ ; RUN: | llvm-dwarfdump -debug-info -debug-loclists - | FileCheck %s ; RUN: llc -mtriple x86_64-pc-linux -split-dwarf-file=foo.dwo -filetype=obj < %s \ -; RUN: | llvm-dwarfdump -debug-info -debug-loclists - | FileCheck %s +; RUN: | llvm-dwarfdump -debug-info -debug-loclists - | FileCheck --check-prefix=SPLIT --check-prefix=CHECK %s +; RUN: llc -mtriple x86_64-pc-linux -split-dwarf-file=foo.dwo -filetype=asm < %s \ +; RUN: | FileCheck --check-prefix=ASM %s + +; A bit of a brittle test - this is testing the specific DWO_id. The +; alternative would be to test two files with different DW_OP_convert values & +; ensuring the DWO IDs differ when the DW_OP_convert parameter differs. + +; So if this test ends up being a brittle pain to maintain, updating the DWO ID +; often - add another IR file with a different DW_OP_convert that's otherwise +; identical and demonstrate that they have different DWO IDs. + +; SPLIT: 0x00000000: Compile Unit: {{.*}} DWO_id = 0xafd73565c68bc661 + +; Regression testing a fairly quirky bug where instead of hashing (see above), +; extra bytes would be emitted into the output assembly in no +; particular/intentional section - so let's check they don't show up at all: +; ASM-NOT: .asciz "\200\200\200" ; CHECK: 0x{{0*}}[[TYPE:.*]]: DW_TAG_base_type ; CHECK-NEXT: DW_AT_name ("DW_ATE_unsigned_32")