diff --git a/src/internal/analysis/data_race_checker.cpp b/src/internal/analysis/data_race_checker.cpp index b562431..864614e 100644 --- a/src/internal/analysis/data_race_checker.cpp +++ b/src/internal/analysis/data_race_checker.cpp @@ -158,6 +158,11 @@ namespace ctrace::concurrency::internal::analysis std::tie(rhs.file, rhs.line, rhs.column, rhs.function); } + bool hasDistinctLoweredLocation(const AccessFact& access) + { + return !sameSourceLocation(access.userLocation, access.loweredLocation); + } + bool shareSelfConcurrentEntry(const EntrySet& lhsEntries, const EntrySet& rhsEntries, const TUFacts& facts) { @@ -193,7 +198,7 @@ namespace ctrace::concurrency::internal::analysis std::set conflictKinds; conflictKinds.insert(conflictKindLabel(lhs.kind, rhs.kind)); - if (sameSourceLocation(lhs.location, rhs.location) && + if (sameSourceLocation(lhs.loweredLocation, rhs.loweredLocation) && shareSelfConcurrentEntry(lhsEntries, rhsEntries, facts) && (lhs.kind == AccessKind::Write || rhs.kind == AccessKind::Write)) { @@ -207,7 +212,7 @@ namespace ctrace::concurrency::internal::analysis const std::vector& entries) { std::ostringstream stream; - stream << toString(access.kind) << " at " << formatLocation(access.location); + stream << toString(access.kind) << " at " << formatLocation(access.userLocation); if (!entries.empty()) stream << " (thread entries: " << joinValues(entries) << ")"; @@ -227,9 +232,9 @@ namespace ctrace::concurrency::internal::analysis const std::vector conflictKinds = collectConflictKinds(lhs, rhs, lhsEntries, rhsEntries, facts); - DiagnosticBuilder(report, RuleId::DataRaceGlobal) - .primaryLocation(lhs.location) - .relatedLocation("Conflicting access", rhs.location) + DiagnosticBuilder builder(report, RuleId::DataRaceGlobal); + builder.primaryLocation(lhs.userLocation) + .relatedLocation("Conflicting access", rhs.userLocation) .message("unsynchronized concurrent access to global '" + lhs.symbol + "'") .note("first access: " + describeAccess(lhs, orderedLhsEntries)) .note("conflicting access: " + describeAccess(rhs, orderedRhsEntries)) @@ -243,8 +248,14 @@ namespace ctrace::concurrency::internal::analysis .property("firstThreadEntries", orderedLhsEntries) .property("secondThreadEntries", orderedRhsEntries) .property("conflictKinds", conflictKinds) - .property("variableAliasing", std::vector{}) - .emit(); + .property("variableAliasing", std::vector{}); + + if (hasDistinctLoweredLocation(lhs)) + builder.relatedLocation("Lowered first access", lhs.loweredLocation); + if (hasDistinctLoweredLocation(rhs)) + builder.relatedLocation("Lowered conflicting access", rhs.loweredLocation); + + builder.emit(); } void emitSelfConcurrentDiagnostic(DiagnosticReport& report, const AccessFact& access, @@ -255,9 +266,9 @@ namespace ctrace::concurrency::internal::analysis orderedEntries.empty() ? access.functionId : joinValues(orderedEntries); const std::vector conflictKinds = {"write/write"}; - DiagnosticBuilder(report, RuleId::DataRaceGlobal) - .primaryLocation(access.location) - .relatedLocation("Concurrent invocation", access.location) + DiagnosticBuilder builder(report, RuleId::DataRaceGlobal); + builder.primaryLocation(access.userLocation) + .relatedLocation("Concurrent invocation", access.userLocation) .message("unsynchronized concurrent access to global '" + access.symbol + "'") .note("access: " + describeAccess(access, orderedEntries)) .note("conflicts with another concurrent invocation reachable from thread entry " @@ -273,8 +284,12 @@ namespace ctrace::concurrency::internal::analysis .property("firstThreadEntries", orderedEntries) .property("secondThreadEntries", orderedEntries) .property("conflictKinds", conflictKinds) - .property("variableAliasing", std::vector{}) - .emit(); + .property("variableAliasing", std::vector{}); + + if (hasDistinctLoweredLocation(access)) + builder.relatedLocation("Lowered access", access.loweredLocation); + + builder.emit(); } DiagnosticSummary computeSummary(const std::vector& diagnostics) @@ -316,13 +331,13 @@ namespace ctrace::concurrency::internal::analysis for (const AccessFact& access : facts.accesses) { FunctionSummary& summary = ensureSummary(access.functionId); - if (summary.name.empty() && !access.location.function.empty()) - summary.name = access.location.function; - else if (!access.location.function.empty()) - summary.name = access.location.function; + if (summary.name.empty() && !access.userLocation.function.empty()) + summary.name = access.userLocation.function; + else if (!access.userLocation.function.empty()) + summary.name = access.userLocation.function; - if (summary.file.empty() && !access.location.file.empty()) - summary.file = access.location.file; + if (summary.file.empty() && !access.userLocation.file.empty()) + summary.file = access.userLocation.file; ++summary.sharedAccessCount; if (!access.heldLocks.empty()) diff --git a/src/internal/analysis/facts.hpp b/src/internal/analysis/facts.hpp index e4a557a..1cb63df 100644 --- a/src/internal/analysis/facts.hpp +++ b/src/internal/analysis/facts.hpp @@ -6,6 +6,7 @@ #include #include #include +#include #include namespace llvm @@ -34,12 +35,43 @@ namespace ctrace::concurrency::internal::analysis bool insideLoop = false; }; + enum class RootBindingKind + { + Global, + Argument, + }; + + struct RootBinding + { + RootBindingKind kind = RootBindingKind::Global; + std::string symbol; + unsigned argumentIndex = 0; + + [[nodiscard]] static RootBinding global(std::string globalSymbol) + { + return RootBinding{ + .kind = RootBindingKind::Global, + .symbol = std::move(globalSymbol), + }; + } + + [[nodiscard]] static RootBinding argument(unsigned index) + { + return RootBinding{ + .kind = RootBindingKind::Argument, + .argumentIndex = index, + }; + } + }; + struct AccessFact { std::string symbol; std::string functionId; AccessKind kind = AccessKind::Read; - SourceLocation location; + SourceLocation loweredLocation; + SourceLocation userLocation; + bool allowCallsiteProjection = false; std::set heldLocks; }; @@ -47,6 +79,7 @@ namespace ctrace::concurrency::internal::analysis { const llvm::Function* function = nullptr; const llvm::Instruction* instruction = nullptr; + RootBinding root; AccessFact fact; }; diff --git a/src/internal/analysis/ir_utils.cpp b/src/internal/analysis/ir_utils.cpp index ebd0414..80f8ba4 100644 --- a/src/internal/analysis/ir_utils.cpp +++ b/src/internal/analysis/ir_utils.cpp @@ -2,11 +2,13 @@ #include "ir_utils.hpp" #include +#include #include #include #include #include #include +#include #include @@ -14,32 +16,155 @@ namespace ctrace::concurrency::internal::analysis { namespace { - std::string normalizeFunctionName(llvm::StringRef name) + std::string normalizeValueName(llvm::StringRef name) { if (name.starts_with("\x01")) name = name.drop_front(); return name.str(); } - } // namespace - const llvm::GlobalVariable* resolveBaseGlobal(const llvm::Value& value) - { - const llvm::Value* current = value.stripPointerCastsAndAliases(); - while (current != nullptr) + bool canIgnoreLocalSlotUser(const llvm::User& user) { - if (const auto* global = llvm::dyn_cast(current)) - return global; + if (llvm::isa(user)) + return true; + + const auto* intrinsic = llvm::dyn_cast(&user); + if (intrinsic == nullptr) + return false; + + switch (intrinsic->getIntrinsicID()) + { + case llvm::Intrinsic::lifetime_start: + case llvm::Intrinsic::lifetime_end: + case llvm::Intrinsic::dbg_declare: + case llvm::Intrinsic::dbg_value: + case llvm::Intrinsic::dbg_assign: + return true; + default: + return false; + } + } + + const llvm::Value* followLocalPointerCopy(const llvm::LoadInst& load, + llvm::SmallPtrSetImpl& seen); + + SourceLocation sourceLocationFromDebugLocation(const llvm::DILocation& debugLocation, + std::string_view fallbackFunction) + { + SourceLocation location; + location.function = std::string(fallbackFunction); + + if (const llvm::DISubprogram* subprogram = debugLocation.getScope()->getSubprogram()) + { + if (!subprogram->getName().empty()) + location.function = subprogram->getName().str(); + } + + location.line = debugLocation.getLine(); + location.column = debugLocation.getColumn(); + location.endLine = location.line; + location.endColumn = location.column; + + const std::string filename = debugLocation.getFilename().str(); + const std::string directory = debugLocation.getDirectory().str(); + if (filename.empty()) + return location; + + std::filesystem::path filePath(filename); + if (!directory.empty() && filePath.is_relative()) + filePath = std::filesystem::path(directory) / filePath; + location.file = filePath.lexically_normal().string(); + return location; + } - if (const auto* gep = llvm::dyn_cast(current)) + const llvm::Value* resolveCopiedValue(const llvm::Value& value, + llvm::SmallPtrSetImpl& seen) + { + const llvm::Value* current = value.stripPointerCastsAndAliases(); + while (current != nullptr) { - current = gep->getPointerOperand()->stripPointerCastsAndAliases(); - continue; + if (!seen.insert(current).second) + return nullptr; + + if (llvm::isa(current) || + llvm::isa(current) || llvm::isa(current)) + return current; + + if (const auto* gepInstruction = llvm::dyn_cast(current)) + { + current = gepInstruction->getPointerOperand()->stripPointerCastsAndAliases(); + continue; + } + + if (const auto* gep = llvm::dyn_cast(current)) + { + current = gep->getPointerOperand()->stripPointerCastsAndAliases(); + continue; + } + + if (const auto* load = llvm::dyn_cast(current)) + return followLocalPointerCopy(*load, seen); + + return nullptr; } return nullptr; } - return nullptr; + const llvm::Value* followLocalPointerCopy(const llvm::LoadInst& load, + llvm::SmallPtrSetImpl& seen) + { + const llvm::Value* slot = load.getPointerOperand()->stripPointerCastsAndAliases(); + const auto* alloca = llvm::dyn_cast(slot); + if (alloca == nullptr || !seen.insert(alloca).second) + return nullptr; + + const llvm::Value* storedValue = nullptr; + for (const llvm::User* user : alloca->users()) + { + if (const auto* store = llvm::dyn_cast(user)) + { + if (store->getPointerOperand()->stripPointerCastsAndAliases() != alloca) + return nullptr; + + const llvm::Value* candidate = + store->getValueOperand()->stripPointerCastsAndAliases(); + if (storedValue == nullptr) + storedValue = candidate; + else if (storedValue != candidate) + return nullptr; + continue; + } + + if (const auto* localLoad = llvm::dyn_cast(user)) + { + if (localLoad->getPointerOperand()->stripPointerCastsAndAliases() != alloca) + return nullptr; + continue; + } + + if (canIgnoreLocalSlotUser(*user)) + continue; + + return nullptr; + } + + if (storedValue == nullptr) + return nullptr; + + return resolveCopiedValue(*storedValue, seen); + } + } // namespace + + bool shouldTrackSharedGlobal(const llvm::GlobalVariable& global) + { + return !global.isDeclaration() && !global.isConstant() && !global.isThreadLocal(); + } + + const llvm::GlobalVariable* resolveBaseGlobal(const llvm::Value& value) + { + llvm::SmallPtrSet seen; + return llvm::dyn_cast_or_null(resolveCopiedValue(value, seen)); } std::optional canonicalGlobalId(const llvm::Value& value) @@ -48,12 +173,58 @@ namespace ctrace::concurrency::internal::analysis if (global == nullptr) return std::nullopt; - return normalizeFunctionName(global->getName()); + return normalizeValueName(global->getName()); + } + + std::optional resolveTrackedRoot(const llvm::Value& value) + { + llvm::SmallPtrSet seen; + const llvm::Value* root = resolveCopiedValue(value, seen); + if (root == nullptr) + return std::nullopt; + + if (const auto* global = llvm::dyn_cast(root)) + { + if (!shouldTrackSharedGlobal(*global)) + return std::nullopt; + + return RootBinding::global(normalizeValueName(global->getName())); + } + + if (const auto* argument = llvm::dyn_cast(root)) + return RootBinding::argument(argument->getArgNo()); + + return std::nullopt; + } + + std::optional resolveFunctionBinding(const llvm::Value& value) + { + llvm::SmallPtrSet seen; + const llvm::Value* root = resolveCopiedValue(value, seen); + if (root == nullptr) + return std::nullopt; + + if (const auto* function = llvm::dyn_cast(root)) + return FunctionBinding{.function = function}; + + if (const auto* argument = llvm::dyn_cast(root)) + return FunctionBinding{.argumentIndex = argument->getArgNo()}; + + return std::nullopt; + } + + const llvm::Function* resolveFunctionValue(const llvm::Value& value) + { + const std::optional binding = resolveFunctionBinding(value); + if (!binding.has_value()) + return nullptr; + + return binding->function; } std::string functionId(const llvm::Function& function) { - return normalizeFunctionName(function.getName()); + return normalizeValueName(function.getName()); } std::string functionDisplayName(const llvm::Function& function) @@ -67,29 +238,39 @@ namespace ctrace::concurrency::internal::analysis return functionId(function); } - SourceLocation makeSourceLocation(const llvm::Instruction& instruction) + ResolvedSourceLocations resolveSourceLocations(const llvm::Instruction& instruction) { - SourceLocation location; - location.function = functionDisplayName(*instruction.getFunction()); + const std::string fallbackFunction = functionDisplayName(*instruction.getFunction()); + ResolvedSourceLocations locations; + locations.loweredLocation.function = fallbackFunction; + locations.userLocation.function = fallbackFunction; const llvm::DebugLoc debugLocation = instruction.getDebugLoc(); if (!debugLocation) - return location; + return locations; - location.line = debugLocation.getLine(); - location.column = debugLocation.getCol(); - location.endLine = location.line; - location.endColumn = location.column; + locations.loweredLocation = + sourceLocationFromDebugLocation(*debugLocation, fallbackFunction); + locations.userLocation = locations.loweredLocation; - const std::string filename = debugLocation->getFilename().str(); - const std::string directory = debugLocation->getDirectory().str(); - if (filename.empty()) - return location; + const llvm::DILocation* outermostInlineLocation = debugLocation.get(); + while (outermostInlineLocation != nullptr && + outermostInlineLocation->getInlinedAt() != nullptr) + outermostInlineLocation = outermostInlineLocation->getInlinedAt(); + + if (outermostInlineLocation != nullptr) + { + const SourceLocation candidate = + sourceLocationFromDebugLocation(*outermostInlineLocation, fallbackFunction); + if (candidate.line != 0 || candidate.column != 0 || !candidate.file.empty()) + locations.userLocation = candidate; + } - std::filesystem::path filePath(filename); - if (!directory.empty() && filePath.is_relative()) - filePath = std::filesystem::path(directory) / filePath; - location.file = filePath.lexically_normal().string(); - return location; + return locations; + } + + SourceLocation makeSourceLocation(const llvm::Instruction& instruction) + { + return resolveSourceLocations(instruction).loweredLocation; } } // namespace ctrace::concurrency::internal::analysis diff --git a/src/internal/analysis/ir_utils.hpp b/src/internal/analysis/ir_utils.hpp index 59de2a3..f11f0c1 100644 --- a/src/internal/analysis/ir_utils.hpp +++ b/src/internal/analysis/ir_utils.hpp @@ -1,24 +1,44 @@ // SPDX-License-Identifier: Apache-2.0 #pragma once -#include "coretrace_concurrency_analysis.hpp" +#include "facts.hpp" #include #include namespace llvm { - class Function; class GlobalVariable; + class Function; + class Argument; class Instruction; class Value; } // namespace llvm namespace ctrace::concurrency::internal::analysis { + struct ResolvedSourceLocations + { + SourceLocation loweredLocation; + SourceLocation userLocation; + }; + + [[nodiscard]] bool shouldTrackSharedGlobal(const llvm::GlobalVariable& global); + + struct FunctionBinding + { + const llvm::Function* function = nullptr; + std::optional argumentIndex; + }; + [[nodiscard]] const llvm::GlobalVariable* resolveBaseGlobal(const llvm::Value& value); [[nodiscard]] std::optional canonicalGlobalId(const llvm::Value& value); + [[nodiscard]] std::optional resolveTrackedRoot(const llvm::Value& value); + [[nodiscard]] std::optional resolveFunctionBinding(const llvm::Value& value); + [[nodiscard]] const llvm::Function* resolveFunctionValue(const llvm::Value& value); [[nodiscard]] std::string functionId(const llvm::Function& function); [[nodiscard]] std::string functionDisplayName(const llvm::Function& function); + [[nodiscard]] ResolvedSourceLocations + resolveSourceLocations(const llvm::Instruction& instruction); [[nodiscard]] SourceLocation makeSourceLocation(const llvm::Instruction& instruction); } // namespace ctrace::concurrency::internal::analysis diff --git a/src/internal/analysis/shared_access_collector.cpp b/src/internal/analysis/shared_access_collector.cpp index 4458f9c..1294b16 100644 --- a/src/internal/analysis/shared_access_collector.cpp +++ b/src/internal/analysis/shared_access_collector.cpp @@ -10,14 +10,6 @@ namespace ctrace::concurrency::internal::analysis { - namespace - { - bool shouldTrackGlobal(const llvm::GlobalVariable& global) - { - return global.hasExternalLinkage() && !global.isConstant() && !global.isThreadLocal(); - } - } // namespace - std::vector SharedAccessCollector::collect(const llvm::Module& module) const { std::vector accesses; @@ -52,17 +44,19 @@ namespace ctrace::concurrency::internal::analysis if (pointerOperand == nullptr) continue; - const llvm::GlobalVariable* global = resolveBaseGlobal(*pointerOperand); - if (global == nullptr || !shouldTrackGlobal(*global)) + const std::optional root = resolveTrackedRoot(*pointerOperand); + if (!root.has_value()) continue; PendingAccess access; access.function = &function; access.instruction = &instruction; - access.fact.symbol = global->getName().str(); + access.root = *root; access.fact.functionId = functionId(function); access.fact.kind = kind; - access.fact.location = makeSourceLocation(instruction); + const ResolvedSourceLocations locations = resolveSourceLocations(instruction); + access.fact.loweredLocation = locations.loweredLocation; + access.fact.userLocation = locations.userLocation; accesses.push_back(std::move(access)); } } diff --git a/src/internal/analysis/thread_spawn_detector.cpp b/src/internal/analysis/thread_spawn_detector.cpp index 77baeae..46fa893 100644 --- a/src/internal/analysis/thread_spawn_detector.cpp +++ b/src/internal/analysis/thread_spawn_detector.cpp @@ -11,11 +11,92 @@ #include #include +#include +#include + namespace ctrace::concurrency::internal::analysis { namespace { - const llvm::Function* threadEntryFromCall(const llvm::CallBase& call, CallKind kind) + struct ParameterizedSpawn + { + unsigned argumentIndex = 0; + SourceLocation location; + bool insideLoop = false; + }; + + struct DirectFunctionCallBinding + { + std::string callerFunctionId; + std::string calleeFunctionId; + std::unordered_map argumentBindings; + SourceLocation location; + bool insideLoop = false; + }; + + struct ThreadEntryBinding + { + const llvm::Function* function = nullptr; + std::optional argumentIndex; + }; + + std::string parameterizedSpawnKey(const ParameterizedSpawn& spawn) + { + std::ostringstream stream; + stream << spawn.argumentIndex << "|" << spawn.location.file << "|" + << spawn.location.line << "|" << spawn.location.column << "|" + << spawn.location.function << "|" << spawn.insideLoop; + return stream.str(); + } + + std::string concreteSpawnKey(const llvm::Function& entry, const SourceLocation& location, + bool insideLoop) + { + std::ostringstream stream; + stream << functionId(entry) << "|" << location.file << "|" << location.line << "|" + << location.column << "|" << location.function << "|" << insideLoop; + return stream.str(); + } + + bool addParameterizedSpawn( + std::unordered_map>& summariesByFunction, + std::unordered_map>& summaryKeysByFunction, + const std::string& functionId, ParameterizedSpawn spawn) + { + const std::string key = parameterizedSpawnKey(spawn); + if (!summaryKeysByFunction[functionId].insert(key).second) + return false; + + summariesByFunction[functionId].push_back(std::move(spawn)); + return true; + } + + bool addConcreteSpawn(ThreadSpawnCollection& collection, + std::unordered_set& spawnKeys, + const llvm::Function& entry, const SourceLocation& location, + bool insideLoop) + { + if (entry.isDeclaration()) + return false; + + const std::string key = concreteSpawnKey(entry, location, insideLoop); + if (!spawnKeys.insert(key).second) + return false; + + SpawnFact fact; + fact.entryFunctionId = functionId(entry); + fact.location = location; + fact.insideLoop = insideLoop; + collection.spawns.push_back(fact); + + EntryConcurrencyInfo& concurrency = collection.entryConcurrency[fact.entryFunctionId]; + ++concurrency.staticSpawnCount; + concurrency.hasSpawnInLoop = concurrency.hasSpawnInLoop || insideLoop; + return true; + } + + std::optional threadEntryBindingFromCall(const llvm::CallBase& call, + CallKind kind) { const llvm::Value* entryValue = nullptr; switch (kind) @@ -33,10 +114,70 @@ namespace ctrace::concurrency::internal::analysis } if (entryValue == nullptr) - return nullptr; + return std::nullopt; - entryValue = entryValue->stripPointerCasts(); - return llvm::dyn_cast(entryValue); + const std::optional binding = resolveFunctionBinding(*entryValue); + if (!binding.has_value()) + return std::nullopt; + + return ThreadEntryBinding{ + .function = binding->function, + .argumentIndex = binding->argumentIndex, + }; + } + + std::vector + collectDirectCallBindings(const llvm::Module& module, + const ConcurrencySymbolClassifier& classifier) + { + std::vector bindings; + + for (const llvm::Function& function : module) + { + if (function.isDeclaration()) + continue; + + llvm::Function& mutableFunction = const_cast(function); + llvm::DominatorTree dominatorTree(mutableFunction); + llvm::LoopInfo loopInfo(dominatorTree); + + for (const llvm::BasicBlock& block : function) + { + if (!dominatorTree.isReachableFromEntry(&block)) + continue; + + for (const llvm::Instruction& instruction : block) + { + const auto* call = llvm::dyn_cast(&instruction); + if (call == nullptr) + continue; + + const llvm::Function* callee = classifier.directCallee(*call); + if (callee == nullptr || callee->isDeclaration()) + continue; + + DirectFunctionCallBinding binding; + binding.callerFunctionId = functionId(function); + binding.calleeFunctionId = functionId(*callee); + binding.location = makeSourceLocation(instruction); + binding.insideLoop = loopInfo.getLoopFor(call->getParent()) != nullptr; + + for (unsigned argumentIndex = 0; argumentIndex < call->arg_size(); + ++argumentIndex) + { + const std::optional argumentBinding = + resolveFunctionBinding(*call->getArgOperand(argumentIndex)); + if (argumentBinding.has_value()) + binding.argumentBindings.emplace(argumentIndex, *argumentBinding); + } + + if (!binding.argumentBindings.empty()) + bindings.push_back(std::move(binding)); + } + } + } + + return bindings; } } // namespace @@ -48,6 +189,9 @@ namespace ctrace::concurrency::internal::analysis ThreadSpawnCollection ThreadSpawnDetector::collect(const llvm::Module& module) const { ThreadSpawnCollection collection; + std::unordered_set concreteSpawnKeys; + std::unordered_map> summariesByFunction; + std::unordered_map> summaryKeysByFunction; for (const llvm::Function& function : module) { @@ -73,21 +217,75 @@ namespace ctrace::concurrency::internal::analysis if (kind != CallKind::PThreadCreate && kind != CallKind::StdThreadCtor) continue; - const llvm::Function* entry = threadEntryFromCall(*call, kind); - if (entry == nullptr || entry->isDeclaration()) + const bool insideLoop = loopInfo.getLoopFor(call->getParent()) != nullptr; + const SourceLocation location = makeSourceLocation(instruction); + const std::optional entryBinding = + threadEntryBindingFromCall(*call, kind); + if (!entryBinding.has_value()) continue; - const bool insideLoop = loopInfo.getLoopFor(call->getParent()) != nullptr; - SpawnFact fact; - fact.entryFunctionId = functionId(*entry); - fact.location = makeSourceLocation(instruction); - fact.insideLoop = insideLoop; - collection.spawns.push_back(fact); - - EntryConcurrencyInfo& concurrency = - collection.entryConcurrency[fact.entryFunctionId]; - ++concurrency.staticSpawnCount; - concurrency.hasSpawnInLoop = concurrency.hasSpawnInLoop || insideLoop; + if (entryBinding->function != nullptr) + { + addConcreteSpawn(collection, concreteSpawnKeys, *entryBinding->function, + location, insideLoop); + continue; + } + + if (!entryBinding->argumentIndex.has_value()) + continue; + + addParameterizedSpawn(summariesByFunction, summaryKeysByFunction, + functionId(function), + ParameterizedSpawn{ + .argumentIndex = *entryBinding->argumentIndex, + .location = location, + .insideLoop = insideLoop, + }); + } + } + } + + const std::vector directCallBindings = + collectDirectCallBindings(module, classifier_); + + bool changed = true; + while (changed) + { + changed = false; + + for (const DirectFunctionCallBinding& callBinding : directCallBindings) + { + const auto summaryIt = summariesByFunction.find(callBinding.calleeFunctionId); + if (summaryIt == summariesByFunction.end()) + continue; + + const std::vector calleeSummaries = summaryIt->second; + for (const ParameterizedSpawn& spawn : calleeSummaries) + { + const auto bindingIt = callBinding.argumentBindings.find(spawn.argumentIndex); + if (bindingIt == callBinding.argumentBindings.end()) + continue; + + const bool insideLoop = spawn.insideLoop || callBinding.insideLoop; + if (bindingIt->second.function != nullptr) + { + addConcreteSpawn(collection, concreteSpawnKeys, *bindingIt->second.function, + callBinding.location, insideLoop); + continue; + } + + if (!bindingIt->second.argumentIndex.has_value()) + continue; + + changed = + addParameterizedSpawn(summariesByFunction, summaryKeysByFunction, + callBinding.callerFunctionId, + ParameterizedSpawn{ + .argumentIndex = *bindingIt->second.argumentIndex, + .location = callBinding.location, + .insideLoop = insideLoop, + }) || + changed; } } } diff --git a/src/internal/analysis/tu_facts_builder.cpp b/src/internal/analysis/tu_facts_builder.cpp index 8d329b1..9f85276 100644 --- a/src/internal/analysis/tu_facts_builder.cpp +++ b/src/internal/analysis/tu_facts_builder.cpp @@ -2,18 +2,213 @@ #include "tu_facts_builder.hpp" #include "concurrency_symbol_classifier.hpp" +#include "ir_utils.hpp" #include "lock_scope_tracker.hpp" #include "shared_access_collector.hpp" #include "thread_spawn_detector.hpp" +#include #include +#include #include +#include #include #include namespace ctrace::concurrency::internal::analysis { + namespace + { + struct ParameterizedAccess + { + RootBinding root; + AccessFact fact; + }; + + struct DirectCallBinding + { + std::string callerFunctionId; + std::string calleeFunctionId; + std::unordered_map argumentBindings; + SourceLocation callsiteLocation; + }; + + std::string rootBindingKey(const RootBinding& binding) + { + if (binding.kind == RootBindingKind::Global) + return "global:" + binding.symbol; + + return "argument:" + std::to_string(binding.argumentIndex); + } + + std::string accessFactKey(const AccessFact& fact) + { + std::ostringstream stream; + stream << fact.symbol << "|" << fact.functionId << "|" << toString(fact.kind) << "|" + << fact.loweredLocation.file << "|" << fact.loweredLocation.line << "|" + << fact.loweredLocation.column << "|" << fact.loweredLocation.function; + + for (const std::string& lock : fact.heldLocks) + stream << "|lock:" << lock; + + return stream.str(); + } + + std::string parameterizedAccessKey(const ParameterizedAccess& access) + { + return rootBindingKey(access.root) + "|" + accessFactKey(access.fact); + } + + bool sameSourceLocation(const SourceLocation& lhs, const SourceLocation& rhs) + { + return std::tie(lhs.file, lhs.line, lhs.column, lhs.function) == + std::tie(rhs.file, rhs.line, rhs.column, rhs.function); + } + + bool hasDistinctUserLocation(const AccessFact& access) + { + return !sameSourceLocation(access.userLocation, access.loweredLocation); + } + + bool addConcreteAccess(std::vector& accesses, + std::unordered_set& accessKeys, AccessFact fact) + { + const std::string key = accessFactKey(fact); + if (!accessKeys.insert(key).second) + return false; + + accesses.push_back(std::move(fact)); + return true; + } + + std::string projectedAccessPreferenceKey(const AccessFact& fact) + { + std::ostringstream stream; + stream << fact.symbol << "|" << toString(fact.kind) << "|" << fact.loweredLocation.file + << "|" << fact.loweredLocation.line << "|" << fact.loweredLocation.column << "|" + << fact.loweredLocation.function; + + for (const std::string& lock : fact.heldLocks) + stream << "|lock:" << lock; + + return stream.str(); + } + + std::vector filterProjectedConcreteAccesses(std::vector accesses) + { + std::unordered_set projectedKeys; + for (const AccessFact& access : accesses) + { + if (hasDistinctUserLocation(access)) + projectedKeys.insert(projectedAccessPreferenceKey(access)); + } + + std::vector filtered; + filtered.reserve(accesses.size()); + for (AccessFact& access : accesses) + { + const bool hasProjectedVariant = + projectedKeys.contains(projectedAccessPreferenceKey(access)); + if (hasProjectedVariant && !hasDistinctUserLocation(access)) + continue; + + filtered.push_back(std::move(access)); + } + + return filtered; + } + + bool shouldRemapAccessToCallsite(const AccessFact& access, const SourceLocation& callsite) + { + if (callsite.file.empty() && callsite.line == 0) + return false; + + if (access.userLocation.file.empty()) + return true; + + return access.userLocation.file != callsite.file; + } + + bool shouldProjectConcreteAccessToCallsite(const AccessFact& access, + const SourceLocation& callsite) + { + if (!access.allowCallsiteProjection) + return false; + + if (callsite.file.empty() && callsite.line == 0) + return false; + + if (access.userLocation.file.empty()) + return true; + + if (access.userLocation.file != callsite.file) + return true; + + return !hasDistinctUserLocation(access) && + !sameSourceLocation(access.userLocation, callsite); + } + + bool addParameterizedAccess( + std::unordered_map>& summariesByFunction, + std::unordered_map>& summaryKeysByFunction, + const std::string& functionId, ParameterizedAccess access) + { + const std::string key = parameterizedAccessKey(access); + if (!summaryKeysByFunction[functionId].insert(key).second) + return false; + + summariesByFunction[functionId].push_back(std::move(access)); + return true; + } + + std::vector + collectDirectCallBindings(const llvm::Module& module, + const ConcurrencySymbolClassifier& classifier) + { + std::vector bindings; + + for (const llvm::Function& function : module) + { + if (function.isDeclaration()) + continue; + + for (const llvm::BasicBlock& block : function) + { + for (const llvm::Instruction& instruction : block) + { + const auto* call = llvm::dyn_cast(&instruction); + if (call == nullptr) + continue; + + const llvm::Function* callee = classifier.directCallee(*call); + if (callee == nullptr || callee->isDeclaration()) + continue; + + DirectCallBinding binding; + binding.callerFunctionId = functionId(function); + binding.calleeFunctionId = functionId(*callee); + binding.callsiteLocation = resolveSourceLocations(instruction).userLocation; + + for (unsigned argumentIndex = 0; argumentIndex < call->arg_size(); + ++argumentIndex) + { + const std::optional root = + resolveTrackedRoot(*call->getArgOperand(argumentIndex)); + if (root.has_value()) + binding.argumentBindings.emplace(argumentIndex, *root); + } + + if (!binding.argumentBindings.empty()) + bindings.push_back(std::move(binding)); + } + } + } + + return bindings; + } + } // namespace + TUFacts TUFactsBuilder::build(const llvm::Module& module) const { const ConcurrencySymbolClassifier classifier; @@ -51,14 +246,124 @@ namespace ctrace::concurrency::internal::analysis facts.spawns = std::move(spawnFacts.spawns); facts.entryConcurrency = std::move(spawnFacts.entryConcurrency); + std::vector concreteAccesses; + std::unordered_set concreteAccessKeys; + std::unordered_map> summariesByFunction; + std::unordered_map> summaryKeysByFunction; + for (PendingAccess& pendingAccess : pendingAccesses) { const auto heldLocksIt = heldLocksByAccess.find(pendingAccess.instruction); if (heldLocksIt != heldLocksByAccess.end()) pendingAccess.fact.heldLocks = heldLocksIt->second; - facts.accesses.push_back(std::move(pendingAccess.fact)); + + if (pendingAccess.root.kind == RootBindingKind::Global) + { + pendingAccess.fact.symbol = pendingAccess.root.symbol; + addConcreteAccess(concreteAccesses, concreteAccessKeys, + std::move(pendingAccess.fact)); + continue; + } + + const std::string functionKey = pendingAccess.fact.functionId; + pendingAccess.fact.allowCallsiteProjection = true; + addParameterizedAccess(summariesByFunction, summaryKeysByFunction, functionKey, + ParameterizedAccess{ + .root = pendingAccess.root, + .fact = std::move(pendingAccess.fact), + }); + } + + const std::vector directCallBindings = + collectDirectCallBindings(module, classifier); + + bool changed = true; + while (changed) + { + changed = false; + + for (const DirectCallBinding& callBinding : directCallBindings) + { + const auto summaryIt = summariesByFunction.find(callBinding.calleeFunctionId); + if (summaryIt == summariesByFunction.end()) + continue; + + const std::vector calleeSummary = summaryIt->second; + for (const ParameterizedAccess& access : calleeSummary) + { + if (access.root.kind != RootBindingKind::Argument) + continue; + + const auto bindingIt = + callBinding.argumentBindings.find(access.root.argumentIndex); + if (bindingIt == callBinding.argumentBindings.end()) + continue; + + if (bindingIt->second.kind == RootBindingKind::Global) + { + AccessFact concrete = access.fact; + concrete.functionId = callBinding.callerFunctionId; + concrete.symbol = bindingIt->second.symbol; + concrete.allowCallsiteProjection = true; + if (shouldRemapAccessToCallsite(concrete, callBinding.callsiteLocation)) + { + concrete.userLocation = callBinding.callsiteLocation; + concrete.allowCallsiteProjection = false; + } + addConcreteAccess(concreteAccesses, concreteAccessKeys, + std::move(concrete)); + continue; + } + + ParameterizedAccess propagatedAccess{ + .root = bindingIt->second, + .fact = access.fact, + }; + propagatedAccess.fact.functionId = callBinding.callerFunctionId; + if (shouldRemapAccessToCallsite(propagatedAccess.fact, + callBinding.callsiteLocation)) + { + propagatedAccess.fact.userLocation = callBinding.callsiteLocation; + } + + changed = addParameterizedAccess(summariesByFunction, summaryKeysByFunction, + callBinding.callerFunctionId, + std::move(propagatedAccess)) || + changed; + } + } + } + + changed = true; + while (changed) + { + changed = false; + + const std::vector currentConcreteAccesses = concreteAccesses; + for (const DirectCallBinding& callBinding : directCallBindings) + { + for (const AccessFact& access : currentConcreteAccesses) + { + if (access.functionId != callBinding.calleeFunctionId) + continue; + + if (!shouldProjectConcreteAccessToCallsite(access, + callBinding.callsiteLocation)) + continue; + + AccessFact remapped = access; + remapped.functionId = callBinding.callerFunctionId; + remapped.userLocation = callBinding.callsiteLocation; + if (remapped.userLocation.file != remapped.loweredLocation.file) + remapped.allowCallsiteProjection = false; + changed = addConcreteAccess(concreteAccesses, concreteAccessKeys, + std::move(remapped)) || + changed; + } + } } + facts.accesses = filterProjectedConcreteAccesses(std::move(concreteAccesses)); return facts; } } // namespace ctrace::concurrency::internal::analysis diff --git a/src/internal/reporting/report_renderer.cpp b/src/internal/reporting/report_renderer.cpp index 5be5983..4193f0f 100644 --- a/src/internal/reporting/report_renderer.cpp +++ b/src/internal/reporting/report_renderer.cpp @@ -109,6 +109,15 @@ namespace ctrace::concurrency::internal::reporting return ""; } + std::string formatDetailedLocation(const SourceLocation& location, + const RenderContext& context) + { + std::string rendered = formatLocationHeader(location, context); + if (!location.function.empty()) + rendered += " in " + location.function; + return rendered; + } + std::string renderDiagnosticBody(const Diagnostic& diagnostic) { std::string rendered; @@ -291,6 +300,12 @@ namespace ctrace::concurrency::internal::reporting stream << "\t" << body.substr(0, body_size - 1) << "\n"; else stream << "\t" << body << "\n"; + + for (const RelatedLocation& related : diagnostic.relatedLocations) + { + stream << "\trelated: " << related.label << " -> " + << formatDetailedLocation(related.location, context) << "\n"; + } } stream << "\nDiagnostics summary: info=" << report.diagnosticsSummary.info diff --git a/tests/fixtures/concurrency/README.md b/tests/fixtures/concurrency/README.md index ea7bf3a..4c1e748 100644 --- a/tests/fixtures/concurrency/README.md +++ b/tests/fixtures/concurrency/README.md @@ -21,6 +21,8 @@ tests/fixtures/concurrency/ - **data_race_mixed_access.c**: Lectures et écritures mélangées sans synchronisation - **race_condition_check_then_use.c**: Pattern TOCTOU (check-then-use) - **cpp_data_race_class.cpp**: Data race dans une classe C++ non thread-safe +- **cpp_shared_object_by_ref.cpp**: Propagation d'un objet global partagé via une référence +- **cpp_thread_local_class.cpp**: Classe locale à chaque thread, ne doit pas être reportée - **cpp_race_std_async.cpp**: Data race avec std::async et shared state - **cpp_atomic_vs_non_atomic.cpp**: Mélange dangereux d'opérations atomiques et non-atomiques - **cpp_move_semantics_race.cpp**: Data race avec move semantics et unique_ptr diff --git a/tests/fixtures/concurrency/data-race/cpp_atomic_vs_non_atomic.cpp b/tests/fixtures/concurrency/data-race/cpp_atomic_vs_non_atomic.cpp index fb8057b..734cb8e 100644 --- a/tests/fixtures/concurrency/data-race/cpp_atomic_vs_non_atomic.cpp +++ b/tests/fixtures/concurrency/data-race/cpp_atomic_vs_non_atomic.cpp @@ -22,14 +22,14 @@ int main() { std::thread t1(worker); std::thread t2(worker); std::thread t3(worker); - + t1.join(); t2.join(); t3.join(); - + std::cout << "Atomic counter: " << state.counter << std::endl; - std::cout << "Non-atomic total: " << state.total + std::cout << "Non-atomic total: " << state.total << " (expected: 3000)" << std::endl; - + return 0; } diff --git a/tests/fixtures/concurrency/data-race/cpp_data_race_class.cpp b/tests/fixtures/concurrency/data-race/cpp_data_race_class.cpp index 9b70da7..af328d6 100644 --- a/tests/fixtures/concurrency/data-race/cpp_data_race_class.cpp +++ b/tests/fixtures/concurrency/data-race/cpp_data_race_class.cpp @@ -7,12 +7,12 @@ class Counter { private: int value = 0; // Non protégé - + public: void increment() { value++; // DATA RACE: non thread-safe } - + int get() const { return value; // DATA RACE: lecture concurrente } @@ -28,17 +28,17 @@ void worker(int iterations) { int main() { std::vector threads; - + for (int i = 0; i < 4; i++) { threads.emplace_back(worker, 10000); } - + for (auto& t : threads) { t.join(); } - - std::cout << "Final value: " << global_counter.get() + + std::cout << "Final value: " << global_counter.get() << " (expected: 40000)" << std::endl; - + return 0; } diff --git a/tests/fixtures/concurrency/data-race/cpp_shared_object_by_ref.cpp b/tests/fixtures/concurrency/data-race/cpp_shared_object_by_ref.cpp new file mode 100644 index 0000000..1e4b5b6 --- /dev/null +++ b/tests/fixtures/concurrency/data-race/cpp_shared_object_by_ref.cpp @@ -0,0 +1,42 @@ +// SPDX-License-Identifier: Apache-2.0 +// Test M2: shared global object propagated through a reference helper +#include +#include + +class Counter +{ + public: + void increment() + { + value++; + } + + private: + int value = 0; +}; + +Counter global_counter; + +void increment_shared(Counter& counter) +{ + counter.increment(); +} + +void worker(int iterations) +{ + for (int i = 0; i < iterations; ++i) + increment_shared(global_counter); +} + +int main() +{ + std::vector threads; + + for (int i = 0; i < 3; ++i) + threads.emplace_back(worker, 1000); + + for (auto& thread : threads) + thread.join(); + + return 0; +} diff --git a/tests/fixtures/concurrency/data-race/cpp_thread_local_class.cpp b/tests/fixtures/concurrency/data-race/cpp_thread_local_class.cpp new file mode 100644 index 0000000..a1f5366 --- /dev/null +++ b/tests/fixtures/concurrency/data-race/cpp_thread_local_class.cpp @@ -0,0 +1,32 @@ +// SPDX-License-Identifier: Apache-2.0 +// Test M2 negative: class instance remains thread-local in each worker +#include + +class Counter +{ + public: + void increment() + { + value++; + } + + private: + int value = 0; +}; + +void worker(int iterations) +{ + Counter local_counter; + for (int i = 0; i < iterations; ++i) + local_counter.increment(); +} + +int main() +{ + std::thread first(worker, 1000); + std::thread second(worker, 1000); + + first.join(); + second.join(); + return 0; +} diff --git a/tests/fixtures/concurrency/data-race/data_race_basic.c b/tests/fixtures/concurrency/data-race/data_race_basic.c index 1671e4e..db4e90e 100644 --- a/tests/fixtures/concurrency/data-race/data_race_basic.c +++ b/tests/fixtures/concurrency/data-race/data_race_basic.c @@ -37,4 +37,5 @@ int main() { // ↳ conflicting access: write at ${REPO_ROOT}/tests/fixtures/concurrency/data-race/data_race_basic.c:10:23 in increment (thread entries: increment) // ↳ possible conflict kinds: read/write, write/write // ↳ no common recognized lock protects the conflicting accesses +// related: Conflicting access -> ${REPO_ROOT}/tests/fixtures/concurrency/data-race/data_race_basic.c:10:23 in increment // EXPECT-HUMAN-DIAGNOSTICS-END diff --git a/tests/fixtures/concurrency/missing-join/missing_join_multiple.c b/tests/fixtures/concurrency/missing-join/missing_join_multiple.c index d06a9d6..0ca131b 100644 --- a/tests/fixtures/concurrency/missing-join/missing_join_multiple.c +++ b/tests/fixtures/concurrency/missing-join/missing_join_multiple.c @@ -53,4 +53,5 @@ int main() { // ↳ conflicts with another concurrent invocation reachable from thread entry 'compute' // ↳ possible conflict kinds: write/write // ↳ no common recognized lock protects the conflicting accesses +// related: Concurrent invocation -> ${REPO_ROOT}/tests/fixtures/concurrency/missing-join/missing_join_multiple.c:12:17 in compute // EXPECT-HUMAN-DIAGNOSTICS-END diff --git a/tests/integration/cli/__pycache__/human_output_expectations.cpython-314.pyc b/tests/integration/cli/__pycache__/human_output_expectations.cpython-314.pyc new file mode 100644 index 0000000..2dbc1ef Binary files /dev/null and b/tests/integration/cli/__pycache__/human_output_expectations.cpython-314.pyc differ diff --git a/tests/integration/cli/__pycache__/test_human_output_golden.cpython-314-pytest-9.0.2.pyc b/tests/integration/cli/__pycache__/test_human_output_golden.cpython-314-pytest-9.0.2.pyc new file mode 100644 index 0000000..f9f82a3 Binary files /dev/null and b/tests/integration/cli/__pycache__/test_human_output_golden.cpython-314-pytest-9.0.2.pyc differ diff --git a/tests/integration/cli/test_cli_cpp.cpp b/tests/integration/cli/test_cli_cpp.cpp index 720622e..2f586d5 100644 --- a/tests/integration/cli/test_cli_cpp.cpp +++ b/tests/integration/cli/test_cli_cpp.cpp @@ -262,6 +262,24 @@ namespace ok; } + { + const RunResult result = + runAnalyzer({fixturePath("concurrency/data-race/cpp_data_race_class.cpp").string(), + "--analyze"}); + ok = assertTrue(result.exitCode == 0, + "--analyze on cpp_data_race_class should not fail the CLI") && + ok; + ok = assertContains(result.output, "ruleId: DataRaceGlobal", + "cpp_data_race_class analyze output") && + ok; + ok = assertContains(result.output, "symbol: global_counter", + "cpp_data_race_class analyze output") && + ok; + ok = assertContains(result.output, "Function: increment", + "cpp_data_race_class analyze output") && + ok; + } + { const RunResult result = runAnalyzer( {fixturePath("concurrency/data-race/data_race_mutex_protected.c").string(), @@ -289,6 +307,30 @@ namespace ok; } + { + const RunResult result = runAnalyzer( + {fixturePath("concurrency/data-race/cpp_move_semantics_race.cpp").string(), + "--analyze"}); + ok = assertTrue(result.exitCode == 0, + "--analyze on cpp_move_semantics_race should succeed") && + ok; + ok = assertContains(result.output, "symbol: shared_resource", + "cpp_move_semantics_race analyze output") && + ok; + ok = assertNotContains(result.output, "symbol: _ZNSt3__14coutE", + "cpp_move_semantics_race analyze output") && + ok; + ok = assertContains(result.output, "Function: producer", + "cpp_move_semantics_race analyze output") && + ok; + ok = assertContains(result.output, "related: Lowered first access ->", + "cpp_move_semantics_race analyze output") && + ok; + ok = assertContains(result.output, "cpp_move_semantics_race.cpp:18:21 in producer", + "cpp_move_semantics_race analyze output") && + ok; + } + { const RunResult result = runAnalyzer({fixturePath("concurrency/data-race/data_race_basic.c").string(), @@ -309,6 +351,30 @@ namespace ok = assertContains(result.output, "\"startColumn\": 23", "json analyze output") && ok; } + { + const RunResult result = runAnalyzer( + {fixturePath("concurrency/data-race/cpp_move_semantics_race.cpp").string(), + "--analyze", "--format=json"}); + ok = assertTrue(result.exitCode == 0, + "--format=json on cpp_move_semantics_race should succeed") && + ok; + ok = assertContains(result.output, "\"symbol\": \"shared_resource\"", + "cpp_move_semantics_race json output") && + ok; + ok = assertNotContains(result.output, "_ZNSt3__14coutE", + "cpp_move_semantics_race json output") && + ok; + ok = assertContains(result.output, "\"relatedLocations\": [", + "cpp_move_semantics_race json output") && + ok; + ok = assertContains(result.output, "\"label\": \"Lowered first access\"", + "cpp_move_semantics_race json output") && + ok; + ok = assertContains(result.output, "cpp_move_semantics_race.cpp", + "cpp_move_semantics_race json output") && + ok; + } + { const RunResult result = runAnalyzer({fixturePath("concurrency/data-race/data_race_basic.c").string(), diff --git a/tests/unit/test_architecture.cpp b/tests/unit/test_architecture.cpp index f60a04c..f69759c 100644 --- a/tests/unit/test_architecture.cpp +++ b/tests/unit/test_architecture.cpp @@ -81,8 +81,13 @@ define i32 @main() { bool hasOutputFlagVariant(const std::vector& args) { - return std::find_if(args.begin(), args.end(), [](const std::string& arg) - { return isOutputFlagVariant(arg); }) != args.end(); + for (const std::string& arg : args) + { + if (isOutputFlagVariant(arg)) + return true; + } + + return false; } std::optional findOutputPath(const std::vector& args) @@ -811,9 +816,7 @@ define i32 @main() { const std::filesystem::path outputPath = fixturePath("build/output-attached.bc"); const std::vector bcArgs = CompileCommandBuilder::buildBC(request, outputPath); - const bool hasAttachedOutputFlag = std::any_of( - bcArgs.begin(), bcArgs.end(), [](const std::string& arg) - { return arg.rfind("-o=", 0) == 0 || (arg.size() > 2 && arg.rfind("-o", 0) == 0); }); + const bool hasAttachedOutputFlag = hasOutputFlagVariant(bcArgs); const bool bcOk = assertTrue(countToken(bcArgs, "-o") == 1, "BC args should contain exactly one -o") && assertTrue(hasOutputPair(bcArgs, outputPath.string()), diff --git a/tests/unit/test_concurrency_analysis.cpp b/tests/unit/test_concurrency_analysis.cpp index 18a431b..4a9e4cd 100644 --- a/tests/unit/test_concurrency_analysis.cpp +++ b/tests/unit/test_concurrency_analysis.cpp @@ -68,6 +68,12 @@ namespace return std::nullopt; } + bool locationReferencesFixture(const ctrace::concurrency::SourceLocation& location, + std::string_view fixtureName) + { + return location.file.find(fixtureName) != std::string::npos; + } + bool hasDiagnosticForSymbol(const DiagnosticReport& report, std::string_view symbol) { return std::any_of(report.diagnostics.begin(), report.diagnostics.end(), @@ -113,6 +119,36 @@ namespace "cpp_atomic_vs_non_atomic should report state"); } + bool testClassDataRaceReportsGlobalCounter() + { + const std::optional report = + analyzeFixture("tests/fixtures/concurrency/data-race/cpp_data_race_class.cpp"); + if (!report.has_value()) + return false; + + return assertTrue(!report->diagnostics.empty(), + "cpp_data_race_class should report a race") && + assertTrue(hasDiagnosticForSymbol(*report, "global_counter"), + "cpp_data_race_class should report global_counter") && + assertTrue(report->diagnostics.front().location.function == "increment", + "cpp_data_race_class should point to increment") && + assertTrue(report->diagnostics.front().location.line == 13, + "cpp_data_race_class should report line 13"); + } + + bool testSharedObjectByRefReportsGlobalCounter() + { + const std::optional report = + analyzeFixture("tests/fixtures/concurrency/data-race/cpp_shared_object_by_ref.cpp"); + if (!report.has_value()) + return false; + + return assertTrue(!report->diagnostics.empty(), + "cpp_shared_object_by_ref should report a race") && + assertTrue(hasDiagnosticForSymbol(*report, "global_counter"), + "cpp_shared_object_by_ref should report global_counter"); + } + bool testMutexProtectedFixtureHasNoDiagnostics() { const std::optional report = @@ -140,6 +176,61 @@ namespace assertTrue(!hasDiagnosticForSymbol(*report, "safe_counter"), "split-symbol fixture should not report safe_counter"); } + + bool testThreadLocalClassHasNoDiagnostics() + { + const std::optional report = + analyzeFixture("tests/fixtures/concurrency/data-race/cpp_thread_local_class.cpp"); + if (!report.has_value()) + return false; + + return assertTrue(report->diagnostics.empty(), + "thread-local class fixture should not report a race") && + assertTrue(report->diagnosticsSummary.error == 0, + "thread-local class fixture should not count error diagnostics"); + } + + bool testMoveSemanticsRaceUsesUserLocations() + { + const std::optional report = + analyzeFixture("tests/fixtures/concurrency/data-race/cpp_move_semantics_race.cpp"); + if (!report.has_value()) + return false; + + bool allSharedResource = true; + bool allPrimaryLocationsInFixture = true; + bool hasLoweredRelatedLocation = false; + + for (const auto& diagnostic : report->diagnostics) + { + const std::optional symbol = symbolOf(diagnostic); + if (!symbol.has_value() || *symbol != "shared_resource") + allSharedResource = false; + + if (!locationReferencesFixture(diagnostic.location, "cpp_move_semantics_race.cpp")) + allPrimaryLocationsInFixture = false; + + for (const auto& related : diagnostic.relatedLocations) + { + if (related.label.starts_with("Lowered ") && + !locationReferencesFixture(related.location, "cpp_move_semantics_race.cpp")) + { + hasLoweredRelatedLocation = true; + } + } + } + + return assertTrue(!report->diagnostics.empty(), + "cpp_move_semantics_race should report races") && + assertTrue(allSharedResource, + "cpp_move_semantics_race should only report shared_resource") && + assertTrue(!hasDiagnosticForSymbol(*report, "_ZNSt3__14coutE"), + "cpp_move_semantics_race should not report std::cout") && + assertTrue(allPrimaryLocationsInFixture, + "cpp_move_semantics_race should use fixture locations as primaries") && + assertTrue(hasLoweredRelatedLocation, + "cpp_move_semantics_race should preserve lowered related locations"); + } } // namespace int main() @@ -148,8 +239,12 @@ int main() ok = testDataRaceBasicIsReported() && ok; ok = testAtomicVsNonAtomicReportsSharedState() && ok; + ok = testClassDataRaceReportsGlobalCounter() && ok; + ok = testSharedObjectByRefReportsGlobalCounter() && ok; ok = testMutexProtectedFixtureHasNoDiagnostics() && ok; ok = testTwoGlobalFixtureOnlyReportsRacySymbol() && ok; + ok = testThreadLocalClassHasNoDiagnostics() && ok; + ok = testMoveSemanticsRaceUsesUserLocations() && ok; if (!ok) return 1;