From 184d72a7c6a7f40e752a044eb0336cbd4c43d004 Mon Sep 17 00:00:00 2001 From: stozer Date: Fri, 22 Nov 2019 16:40:32 +0000 Subject: [PATCH] [DebugInfo] Disallow fragmenting DIExpressions with shift operators DIExpressions with shift operators should not be fragmented for the same reason as arithmetic operators: carry over cannot be expressed from one fragment to the other, so an invalid result would be produced. Differential Revision: https://reviews.llvm.org/D70601 --- llvm/lib/IR/DebugInfoMetadata.cpp | 8 +++++-- llvm/unittests/IR/MetadataTest.cpp | 43 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 49 insertions(+), 2 deletions(-) diff --git a/llvm/lib/IR/DebugInfoMetadata.cpp b/llvm/lib/IR/DebugInfoMetadata.cpp index e4036ee..b010227 100644 --- a/llvm/lib/IR/DebugInfoMetadata.cpp +++ b/llvm/lib/IR/DebugInfoMetadata.cpp @@ -1148,10 +1148,14 @@ Optional DIExpression::createFragmentExpression( for (auto Op : Expr->expr_ops()) { switch (Op.getOp()) { default: break; + case dwarf::DW_OP_shr: + case dwarf::DW_OP_shra: + case dwarf::DW_OP_shl: case dwarf::DW_OP_plus: + case dwarf::DW_OP_plus_uconst: case dwarf::DW_OP_minus: - // We can't safely split arithmetic into multiple fragments because we - // can't express carry-over between fragments. + // We can't safely split arithmetic or shift operations into multiple + // fragments because we can't express carry-over between fragments. // // FIXME: We *could* preserve the lowest fragment of a constant offset // operation if the offset fits into SizeInBits. diff --git a/llvm/unittests/IR/MetadataTest.cpp b/llvm/unittests/IR/MetadataTest.cpp index e6c7a50..9929568 100644 --- a/llvm/unittests/IR/MetadataTest.cpp +++ b/llvm/unittests/IR/MetadataTest.cpp @@ -2394,6 +2394,49 @@ TEST_F(DIExpressionTest, isValid) { #undef EXPECT_INVALID } +TEST_F(DIExpressionTest, createFragmentExpression) { +#define EXPECT_VALID_FRAGMENT(Offset, Size, ...) \ + do { \ + uint64_t Elements[] = {__VA_ARGS__}; \ + DIExpression* Expression = DIExpression::get(Context, Elements); \ + EXPECT_TRUE(DIExpression::createFragmentExpression( \ + Expression, Offset, Size).hasValue()); \ + } while (false) +#define EXPECT_INVALID_FRAGMENT(Offset, Size, ...) \ + do { \ + uint64_t Elements[] = {__VA_ARGS__}; \ + DIExpression* Expression = DIExpression::get(Context, Elements); \ + EXPECT_FALSE(DIExpression::createFragmentExpression( \ + Expression, Offset, Size).hasValue()); \ + } while (false) + + // createFragmentExpression adds correct ops. + Optional R = DIExpression::createFragmentExpression( + DIExpression::get(Context, {}), 0, 32); + EXPECT_EQ(R.hasValue(), true); + EXPECT_EQ(3u, (*R)->getNumElements()); + EXPECT_EQ(dwarf::DW_OP_LLVM_fragment, (*R)->getElement(0)); + EXPECT_EQ(0u, (*R)->getElement(1)); + EXPECT_EQ(32u, (*R)->getElement(2)); + + // Valid fragment expressions. + EXPECT_VALID_FRAGMENT(0, 32, {}); + EXPECT_VALID_FRAGMENT(0, 32, dwarf::DW_OP_deref); + EXPECT_VALID_FRAGMENT(0, 32, dwarf::DW_OP_LLVM_fragment, 0, 32); + EXPECT_VALID_FRAGMENT(16, 16, dwarf::DW_OP_LLVM_fragment, 0, 32); + + // Invalid fragment expressions (incompatible ops). + EXPECT_INVALID_FRAGMENT(0, 32, dwarf::DW_OP_constu, 6, dwarf::DW_OP_plus); + EXPECT_INVALID_FRAGMENT(0, 32, dwarf::DW_OP_constu, 14, dwarf::DW_OP_minus); + EXPECT_INVALID_FRAGMENT(0, 32, dwarf::DW_OP_constu, 16, dwarf::DW_OP_shr); + EXPECT_INVALID_FRAGMENT(0, 32, dwarf::DW_OP_constu, 16, dwarf::DW_OP_shl); + EXPECT_INVALID_FRAGMENT(0, 32, dwarf::DW_OP_constu, 16, dwarf::DW_OP_shra); + EXPECT_INVALID_FRAGMENT(0, 32, dwarf::DW_OP_plus_uconst, 6); + +#undef EXPECT_VALID_FRAGMENT +#undef EXPECT_INVALID_FRAGMENT +} + typedef MetadataTest DIObjCPropertyTest; TEST_F(DIObjCPropertyTest, get) { -- 2.7.4