From 2485cd2c8ba1d7d0923159428a0fd0733ef31cea Mon Sep 17 00:00:00 2001 From: Pietro Fezzardi Date: Tue, 5 Jan 2021 00:15:55 +0100 Subject: [PATCH] DLAHelpers: relax assumptions on ExtractValueInst Before this commit, the DLA code made very strong assumptions about Functions that returned struct types. In particular, calls to such Functions were expected to have at most a number of uses equal to the number of fields of the returned struct. Moreover, such uses were only expected to be ExtractValueInst. Now, we still assume that such uses are ExtractValueInst, but we don't make any strong assumption on their number anymore. This makes the DLA code less reliant on specific form of LLVM IR, so we can also drop -gvn-hoist from the decompilation test pipeline. --- CMakeLists.txt | 1 - .../DLACreateIntraProceduralTypes.cpp | 23 +++--- lib/Decompiler/DLAHelpers.cpp | 24 +++--- lib/Decompiler/DLAHelpers.h | 7 +- lib/Decompiler/DLATypeSystem.cpp | 81 ++++++++++++++----- 5 files changed, 93 insertions(+), 43 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 0e1b4c97a..85f1c2a94 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -106,7 +106,6 @@ macro(artifact_handler CATEGORY INPUT_FILE CONFIGURATION OUTPUT TARGET_NAME) -instcombine -early-cse -simplifycfg - -gvn-hoist # apply type shrinking -type-shrinking --min-width 8 diff --git a/lib/Decompiler/DLACreateIntraProceduralTypes.cpp b/lib/Decompiler/DLACreateIntraProceduralTypes.cpp index b97392e3f..bf1dab125 100644 --- a/lib/Decompiler/DLACreateIntraProceduralTypes.cpp +++ b/lib/Decompiler/DLACreateIntraProceduralTypes.cpp @@ -364,20 +364,21 @@ public: for (const auto &[Ext, RetTy] : llvm::zip(ExtractedVals, FormalRetTys)) { - if (nullptr == Ext) + if (Ext.empty()) continue; - revng_assert(isa(Ext)); + for (llvm::ExtractValueInst *E : Ext) { + revng_assert(E); + llvm::Type *ExtTy = E->getType(); + revng_assert(isa(ExtTy) + or isa(ExtTy)); - llvm::Type *ExtTy = Ext->getType(); - revng_assert(isa(ExtTy) - or isa(ExtTy)); - - const auto &[ExtLayout, New] = TS.getOrCreateLayoutType(Ext); - Changed |= New; - Changed |= TS.addEqualityLink(RetTy, ExtLayout).second; - const SCEV *S = SE->getSCEV(Ext); - SCEVToLayoutType.insert(std::make_pair(S, ExtLayout)); + const auto &[ExtLayout, New] = TS.getOrCreateLayoutType(E); + Changed |= New; + Changed |= TS.addEqualityLink(RetTy, ExtLayout).second; + const SCEV *S = SE->getSCEV(E); + SCEVToLayoutType.insert(std::make_pair(S, ExtLayout)); + } } } else { // Type representing the return type diff --git a/lib/Decompiler/DLAHelpers.cpp b/lib/Decompiler/DLAHelpers.cpp index e5fc29f2c..42b3c4464 100644 --- a/lib/Decompiler/DLAHelpers.cpp +++ b/lib/Decompiler/DLAHelpers.cpp @@ -78,38 +78,42 @@ getInsertValueLeafOperands(const llvm::InsertValueInst *Ins) { return getConstQualifiedInsertValueLeafOperands(Ins); } +template +using ExtractValueT = conditional_t, + const llvm::ExtractValueInst, + llvm::ExtractValueInst>; + +template +using ExtractValuePtrSet = llvm::SmallPtrSet *, 2>; + template std::enable_if_t, llvm::CallInst>, - llvm::SmallVector *, 2>> + llvm::SmallVector, 2>> getConstQualifiedExtractedValuesFromCall(T *Call) { - using ValueT = LLVMValueT; - llvm::SmallVector Results; + llvm::SmallVector, 2> Results; llvm::SmallSet FoundIds; auto *StructTy = llvm::cast(Call->getType()); unsigned NumFields = StructTy->getNumElements(); - Results.resize(NumFields, nullptr); - revng_assert(Call->getNumUses() <= NumFields); + Results.resize(NumFields, {}); for (auto *Extract : Call->users()) { auto *E = cast(Extract); revng_assert(E->getNumIndices() == 1); unsigned FieldId = E->getIndices()[0]; revng_assert(FieldId < NumFields); - revng_assert(FoundIds.count(FieldId) == 0); FoundIds.insert(FieldId); revng_assert(isa(E->getType()) or isa(E->getType())); - revng_assert(Results[FieldId] == nullptr); - Results[FieldId] = E; + Results[FieldId].insert(E); } return Results; }; -llvm::SmallVector +llvm::SmallVector, 2> getExtractedValuesFromCall(llvm::CallInst *Call) { return getConstQualifiedExtractedValuesFromCall(Call); } -llvm::SmallVector +llvm::SmallVector, 2> getExtractedValuesFromCall(const llvm::CallInst *Call) { return getConstQualifiedExtractedValuesFromCall(Call); } diff --git a/lib/Decompiler/DLAHelpers.h b/lib/Decompiler/DLAHelpers.h index 9f4b732a6..350e49b75 100644 --- a/lib/Decompiler/DLAHelpers.h +++ b/lib/Decompiler/DLAHelpers.h @@ -4,12 +4,14 @@ // Copyright (c) rev.ng Srls. See LICENSE.md for details. // +#include "llvm/ADT/SmallPtrSet.h" #include "llvm/ADT/SmallVector.h" namespace llvm { class InsertValueInst; class CallInst; +class ExtractValueInst; class Use; class Value; @@ -21,10 +23,11 @@ getInsertValueLeafOperands(llvm::InsertValueInst *); extern llvm::SmallVector getInsertValueLeafOperands(const llvm::InsertValueInst *); -extern llvm::SmallVector +extern llvm::SmallVector, 2> getExtractedValuesFromCall(llvm::CallInst *); -extern llvm::SmallVector +extern llvm::SmallVector, + 2> getExtractedValuesFromCall(const llvm::CallInst *); namespace dla { diff --git a/lib/Decompiler/DLATypeSystem.cpp b/lib/Decompiler/DLATypeSystem.cpp index d33d30c52..dc3f65e6a 100644 --- a/lib/Decompiler/DLATypeSystem.cpp +++ b/lib/Decompiler/DLATypeSystem.cpp @@ -299,17 +299,38 @@ LayoutTypeSystem::getLayoutTypes(const Value &V) { } } else if (auto *StructTy = dyn_cast(VTy)) { revng_assert(not isa(V)); - SmallVector LeafVals; - if (auto *Ins = dyn_cast(&V)) - LeafVals = getInsertValueLeafOperands(Ins); - else if (auto *Call = dyn_cast(&V)) - LeafVals = getExtractedValuesFromCall(Call); - else - LeafVals.resize(StructTy->getNumElements(), nullptr); + if (auto *Call = dyn_cast(&V)) { - for (const Value *LeafVal : LeafVals) - Results.push_back(getLayoutType(LeafVal)); + auto ExtractedValues = getExtractedValuesFromCall(Call); + + for (const auto &ExtractedSet : ExtractedValues) { + // Inside here we're working on a single field of the struct. + // ExtractedSet contains all the ExtractValueInst that extract the same + // field of the struct. + + // We get or create a layout type for each of them, but they should all + // be the same. + SmallVector FieldResults; + for (const llvm::ExtractValueInst *E : ExtractedSet) { + FieldResults.push_back(getLayoutType(E)); + revng_assert(FieldResults.front() == FieldResults.back()); + } + + Results.push_back(std::move(FieldResults.front())); + } + + } else { + + SmallVector LeafVals; + if (auto *Ins = dyn_cast(&V)) + LeafVals = getInsertValueLeafOperands(Ins); + else + LeafVals.resize(StructTy->getNumElements(), nullptr); + + for (const Value *LeafVal : LeafVals) + Results.push_back(getLayoutType(LeafVal)); + } } else { // For non-struct and non-function types we only add a LayoutTypeSystemNode Results.push_back(getLayoutType(&V)); @@ -320,7 +341,8 @@ LayoutTypeSystem::getLayoutTypes(const Value &V) { SmallVector, 2> LayoutTypeSystem::getOrCreateLayoutTypes(const Value &V) { assertGetLayoutTypePreConditions(V); - SmallVector, 2> Results; + using GetOrCreateResult = std::pair; + SmallVector Results; const Type *VTy = V.getType(); if (const auto *F = dyn_cast(&V)) { auto *RetTy = F->getReturnType(); @@ -338,17 +360,38 @@ LayoutTypeSystem::getOrCreateLayoutTypes(const Value &V) { } } else if (auto *StructTy = dyn_cast(VTy)) { revng_assert(not isa(V)); - SmallVector LeafVals; - if (auto *Ins = dyn_cast(&V)) - LeafVals = getInsertValueLeafOperands(Ins); - else if (auto *Call = dyn_cast(&V)) - LeafVals = getExtractedValuesFromCall(Call); - else - LeafVals.resize(StructTy->getNumElements(), nullptr); + if (auto *Call = dyn_cast(&V)) { - for (const Value *LeafVal : LeafVals) - Results.push_back(getOrCreateLayoutType(LeafVal)); + const auto &ExtractedValues = getExtractedValuesFromCall(Call); + + for (const auto &ExtractedSet : ExtractedValues) { + // Inside here we're working on a single field of the struct. + // ExtractedSet contains all the ExtractValueInst that extract the same + // field of the struct. + + // We get or create a layout type for each of them, but they should all + // be the same. + SmallVector FieldResults; + for (const llvm::ExtractValueInst *E : ExtractedSet) { + FieldResults.push_back(getOrCreateLayoutType(E)); + revng_assert(FieldResults.front().first == FieldResults.back().first); + } + + Results.push_back(std::move(FieldResults.front())); + } + + } else { + + SmallVector LeafVals; + if (auto *Ins = dyn_cast(&V)) + LeafVals = getInsertValueLeafOperands(Ins); + else + LeafVals.resize(StructTy->getNumElements(), nullptr); + + for (const Value *LeafVal : LeafVals) + Results.push_back(getOrCreateLayoutType(LeafVal)); + } } else { // For non-struct and non-function types we only add a LayoutTypeSystemNode Results.push_back(getOrCreateLayoutType(&V));