38 featdiag add duplicate else if condition diagnostic ir level tests

CMakeLists.txt

@@ -30,9 +30,9 @@ if(DEFINED DEBUG_ASAN)
 endif()
 
 FetchContent_Declare(
-  cc
-  GIT_REPOSITORY https://github.com/CoreTrace/coretrace-compiler.git
-  GIT_TAG main
+    cc
+    GIT_REPOSITORY https://github.com/CoreTrace/coretrace-compiler.git
+    GIT_TAG main
 )
 
 FetchContent_MakeAvailable(cc)
@@ -43,6 +43,7 @@ message(STATUS "Using LLVMConfig.cmake in: ${LLVM_DIR}")
 # Options de build
 option(BUILD_CLI "Build stack_usage_analyzer CLI tool" ON)
 option(BUILD_SHARED_LIB "Build shared library variant" ON)
+option(ENABLE_STACK_USAGE "Emit per-function stack usage (.su) files" OFF)
 
 # ===========================
 #  Communs Sources
@@ -84,6 +85,10 @@ add_library(stack_usage_analyzer_lib STATIC
     ${STACK_ANALYZER_SOURCES}
 )
 
+if(ENABLE_STACK_USAGE)
+    target_compile_options(stack_usage_analyzer_lib PRIVATE -fstack-usage)
+endif()
+
 if(ENABLE_DEBUG_ASAN)
     target_compile_options(stack_usage_analyzer_lib PUBLIC -fsanitize=address -fno-omit-frame-pointer -g)
     target_link_options(stack_usage_analyzer_lib PUBLIC -fsanitize=address)
@@ -157,6 +162,10 @@ if(BUILD_CLI)
         # it is already linked into the library.
     )
 
+    if(ENABLE_STACK_USAGE)
+        target_compile_options(stack_usage_analyzer PRIVATE -fstack-usage)
+    endif()
+
     if(ENABLE_DEBUG_ASAN)
         target_compile_options(stack_usage_analyzer PRIVATE -fsanitize=address -fno-omit-frame-pointer -g)
         target_link_options(stack_usage_analyzer PRIVATE -fsanitize=address)

main.cpp

@@ -402,6 +402,9 @@ int main(int argc, char** argv)
     // cfg.mode = AnalysisMode::IR; -> already set by default constructor
     // cfg.stackLimit = 8ull * 1024ull * 1024ull; // 8 MiB -> already set by default constructor but needed to be set with args
 
+    cfg.extraCompileArgs.emplace_back("-O0");
+    cfg.extraCompileArgs.emplace_back("--ct-optnone");
+
     for (int i = 1; i < argc; ++i)
     {
         const char* arg = argv[i];

src/analysis/DuplicateIfCondition.cpp

@@ -11,10 +11,12 @@
 #include <llvm/Analysis/CaptureTracking.h>
 #include <llvm/IR/BasicBlock.h>
 #include <llvm/IR/CFG.h>
+#include <llvm/IR/Constants.h>
 #include <llvm/IR/Function.h>
 #include <llvm/IR/Instructions.h>
 #include <llvm/IR/IntrinsicInst.h>
 #include <llvm/IR/Module.h>
+#include <llvm/IR/Operator.h>
 #include <llvm/IR/Value.h>
 #include <llvm/IR/Dominators.h>
 
@@ -178,11 +180,20 @@ namespace ctrace::stack::analysis
             bool precise = false; // true if we can reason about direct stores only
         };
 
+        enum class ConditionKind
+        {
+            Invalid,
+            ICmp,
+            BoolValue
+        };
+
         struct ConditionKey
         {
+            ConditionKind kind = ConditionKind::Invalid;
             llvm::CmpInst::Predicate pred = llvm::CmpInst::BAD_ICMP_PREDICATE;
             llvm::Value* lhs = nullptr;
             llvm::Value* rhs = nullptr;
+            llvm::Value* boolValue = nullptr;
             bool valid = false;
             llvm::SmallVector<MemoryOperand, 2> memoryOperands;
         };
@@ -196,6 +207,68 @@ namespace ctrace::stack::analysis
             return v;
         }
 
+        static bool valuesEquivalent(const llvm::Value* a, const llvm::Value* b, int depth = 0)
+        {
+            if (a == b)
+                return true;
+            if (!a || !b)
+                return false;
+            if (depth > 6)
+                return false;
+
+            if (auto* la = llvm::dyn_cast<llvm::LoadInst>(a))
+            {
+                auto* lb = llvm::dyn_cast<llvm::LoadInst>(b);
+                if (!lb)
+                    return false;
+                return valuesEquivalent(la->getPointerOperand()->stripPointerCasts(),
+                                        lb->getPointerOperand()->stripPointerCasts(), depth + 1);
+            }
+
+            if (auto* ga = llvm::dyn_cast<llvm::GEPOperator>(a))
+            {
+                auto* gb = llvm::dyn_cast<llvm::GEPOperator>(b);
+                if (!gb)
+                    return false;
+                if (ga->getNumIndices() != gb->getNumIndices())
+                    return false;
+                if (!valuesEquivalent(ga->getPointerOperand()->stripPointerCasts(),
+                                      gb->getPointerOperand()->stripPointerCasts(), depth + 1))
+                    return false;
+
+                auto itA = ga->idx_begin();
+                auto itB = gb->idx_begin();
+                for (; itA != ga->idx_end(); ++itA, ++itB)
+                {
+                    auto* ca = llvm::dyn_cast<llvm::ConstantInt>(itA->get());
+                    auto* cb = llvm::dyn_cast<llvm::ConstantInt>(itB->get());
+                    if (!ca || !cb)
+                        return false;
+                    if (ca->getValue() != cb->getValue())
+                        return false;
+                }
+
+                return true;
+            }
+
+            if (auto* opA = llvm::dyn_cast<llvm::Operator>(a))
+            {
+                auto* opB = llvm::dyn_cast<llvm::Operator>(b);
+                if (!opB || opA->getOpcode() != opB->getOpcode())
+                    return false;
+                switch (opA->getOpcode())
+                {
+                case llvm::Instruction::BitCast:
+                case llvm::Instruction::AddrSpaceCast:
+                    return valuesEquivalent(opA->getOperand(0), opB->getOperand(0), depth + 1);
+                default:
+                    break;
+                }
+            }
+
+            return false;
+        }
+
         static bool isPrecisePointer(const llvm::Value* ptr)
         {
             using namespace llvm;
@@ -242,9 +315,20 @@ namespace ctrace::stack::analysis
             ConditionKey key;
             auto* cmp = llvm::dyn_cast<llvm::ICmpInst>(cond);
             if (!cmp)
+            {
+                llvm::Value* raw = stripCasts(cond);
+                if (raw && raw->getType()->isIntegerTy())
+                {
+                    key.valid = true;
+                    key.kind = ConditionKind::BoolValue;
+                    key.boolValue = canonicalizeOperand(raw, key);
+                    dedupeMemoryOperands(key);
+                }
                 return key;
+            }
 
             key.valid = true;
+            key.kind = ConditionKind::ICmp;
             key.pred = cmp->getPredicate();
             key.lhs = canonicalizeOperand(cmp->getOperand(0), key);
             key.rhs = canonicalizeOperand(cmp->getOperand(1), key);
@@ -263,7 +347,21 @@ namespace ctrace::stack::analysis
         {
             if (!a.valid || !b.valid)
                 return false;
-            return a.pred == b.pred && a.lhs == b.lhs && a.rhs == b.rhs;
+            if (a.kind != b.kind)
+                return false;
+            if (a.kind == ConditionKind::ICmp)
+            {
+                if (a.pred == b.pred && valuesEquivalent(a.lhs, b.lhs) &&
+                    valuesEquivalent(a.rhs, b.rhs))
+                    return true;
+                if (llvm::CmpInst::getSwappedPredicate(a.pred) == b.pred &&
+                    valuesEquivalent(a.lhs, b.rhs) && valuesEquivalent(a.rhs, b.lhs))
+                    return true;
+                return false;
+            }
+            if (a.kind == ConditionKind::BoolValue)
+                return valuesEquivalent(a.boolValue, b.boolValue);
+            return false;
         }
 
         static bool isInterferingWrite(const llvm::Instruction& I, const MemoryOperand& mem)
@@ -275,25 +373,25 @@ namespace ctrace::stack::analysis
             if (auto* store = dyn_cast<StoreInst>(&I))
             {
                 const Value* ptr = store->getPointerOperand()->stripPointerCasts();
-                return ptr == mem.ptr;
+                return valuesEquivalent(ptr, mem.ptr);
             }
 
             if (auto* rmw = dyn_cast<AtomicRMWInst>(&I))
             {
                 const Value* ptr = rmw->getPointerOperand()->stripPointerCasts();
-                return ptr == mem.ptr;
+                return valuesEquivalent(ptr, mem.ptr);
             }
 
             if (auto* cmpxchg = dyn_cast<AtomicCmpXchgInst>(&I))
             {
                 const Value* ptr = cmpxchg->getPointerOperand()->stripPointerCasts();
-                return ptr == mem.ptr;
+                return valuesEquivalent(ptr, mem.ptr);
             }
 
             if (auto* memIntrinsic = dyn_cast<MemIntrinsic>(&I))
             {
                 const Value* ptr = memIntrinsic->getRawDest()->stripPointerCasts();
-                return ptr == mem.ptr;
+                return valuesEquivalent(ptr, mem.ptr);
             }
 
             if (auto* call = dyn_cast<CallBase>(&I))
@@ -307,7 +405,7 @@ namespace ctrace::stack::analysis
                     const Value* argVal = arg.get();
                     if (!argVal || !argVal->getType()->isPointerTy())
                         continue;
-                    if (argVal->stripPointerCasts() == mem.ptr)
+                    if (valuesEquivalent(argVal->stripPointerCasts(), mem.ptr))
                         return true;
                 }
                 return false;
@@ -330,6 +428,8 @@ namespace ctrace::stack::analysis
             {
                 if (!DT.dominates(pathBlock, &BB))
                     continue;
+                if (&BB != atBlock && !DT.dominates(&BB, atBlock))
+                    continue;
 
                 for (const llvm::Instruction& I : BB)
                 {

src/analysis/InputPipeline.cpp

@@ -323,6 +323,14 @@ namespace ctrace::stack::analysis
                 result.error = "Compilation failed:\n" + res.diagnostics + '\n';
                 return result;
             }
+            if (!res.diagnostics.empty() && !config.quiet)
+            {
+                llvm::errs() << res.diagnostics;
+                if (res.diagnostics.back() != '\n')
+                {
+                    llvm::errs() << '\n';
+                }
+            }
 
             if (res.llvmIR.empty())
             {

test/diagnostics/duplicate-else-if-bool.c

@@ -0,0 +1,20 @@
+#include <stdbool.h>
+
+int main(int argc, char* argv[])
+{
+    bool hasFilter = argc > 1;
+
+    if (hasFilter)
+    {
+        return 0;
+    }
+    // at line 14, column 14
+    // [!] unreachable else-if branch: condition is equivalent to a previous 'if' condition
+    //     else branch implies previous condition is false
+    else if (hasFilter)
+    {
+        return 1;
+    }
+
+    return 2;
+}

test/diagnostics/duplicate-else-if-enum-class.cpp

@@ -0,0 +1,35 @@
+enum class DiagnosticSeverity
+{
+    Info = 0,
+    Warning = 1,
+    Error = 2
+};
+
+int main(int argc, char** argv)
+{
+    int iter = 13;
+    DiagnosticSeverity severity;
+
+    if (argc == 2)
+        severity = DiagnosticSeverity::Warning;
+    if (argc == 3)
+        severity = DiagnosticSeverity::Error;
+    else
+        severity = DiagnosticSeverity::Info;
+
+    for (int i = 0; i < iter; ++i)
+    {
+        if (severity != DiagnosticSeverity::Info)
+        {
+            return 1;
+        }
+        // at line 29, column 27
+        // [!] unreachable else-if branch: condition is equivalent to a previous 'if' condition
+        //     else branch implies previous condition is false
+        else if (severity != DiagnosticSeverity::Info)
+        {
+            return 1;
+        }
+    }
+    return 0;
+}

test/diagnostics/duplicate-else-if-enum.c

@@ -0,0 +1,35 @@
+typedef enum DiagnosticSeverity
+{
+    Info = 0,
+    Warning = 1,
+    Error = 2
+} DiagnosticSeverity;
+
+int main(int argc, char** argv)
+{
+    int iter = 13;
+    DiagnosticSeverity severity;
+
+    if (argc == 2)
+        severity = Warning;
+    if (argc == 3)
+        severity = Error;
+    else
+        severity = Info;
+
+    for (int i = 0; i < iter; ++i)
+    {
+        if (severity != Info)
+        {
+            return 1;
+        }
+        // at line 29, column 27
+        // [!] unreachable else-if branch: condition is equivalent to a previous 'if' condition
+        //     else branch implies previous condition is false
+        else if (severity != Info)
+        {
+            return 1;
+        }
+    }
+    return 0;
+}

test/diagnostics/duplicate-else-if-enum.cpp

@@ -0,0 +1,35 @@
+enum DiagnosticSeverity
+{
+    Info = 0,
+    Warning = 1,
+    Error = 2
+};
+
+int main(int argc, char** argv)
+{
+    int iter = 13;
+    DiagnosticSeverity severity;
+
+    if (argc == 2)
+        severity = DiagnosticSeverity::Warning;
+    if (argc == 3)
+        severity = DiagnosticSeverity::Error;
+    else
+        severity = DiagnosticSeverity::Info;
+
+    for (int i = 0; i < iter; ++i)
+    {
+        if (severity != DiagnosticSeverity::Info)
+        {
+            return 1;
+        }
+        // at line 29, column 27
+        // [!] unreachable else-if branch: condition is equivalent to a previous 'if' condition
+        //     else branch implies previous condition is false
+        else if (severity != DiagnosticSeverity::Info)
+        {
+            return 1;
+        }
+    }
+    return 0;
+}