Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
93 changes: 93 additions & 0 deletions score/evaluation/BUILD
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@

load("@rules_cc//cc:defs.bzl", "cc_binary")

alias(
name = "solid_violations",
actual = "//score/evaluation/solid:solid_violations",
)

alias(
name = "memory_issues",
actual = "//score/evaluation/memory:memory_issues",
)

alias(
name = "non_idiomatic_cpp",
actual = "//score/evaluation/idiomatic:non_idiomatic_cpp",
)

alias(
name = "modern_cpp_syntax",
actual = "//score/evaluation/modern_cpp:modern_cpp_syntax",
)

alias(
name = "design_patterns",
actual = "//score/evaluation/patterns:design_patterns",
)

alias(
name = "code_optimization",
actual = "//score/evaluation/optimization:code_optimization",
)

alias(
name = "safe_cpp",
actual = "//score/evaluation/safety:safe_cpp",
)

alias(
name = "security_issues",
actual = "//score/evaluation/security:security_issues",
)

alias(
name = "design_faults",
actual = "//score/evaluation/design:design_faults",
)

alias(
name = "concurrency_issues",
actual = "//score/evaluation/concurrency:concurrency_issues",
)

alias(
name = "exception_error_handling",
actual = "//score/evaluation/exception_handling:exception_error_handling",
)

alias(
name = "unit_test_quality",
actual = "//score/evaluation/unit_test_quality:unit_test_quality",
)

alias(
name = "template_metaprogramming_issues",
actual = "//score/evaluation/template_metaprogramming:template_metaprogramming_issues",
)

alias(
name = "unit_test_quality_test",
actual = "//score/evaluation/unit_test_quality:unit_test_quality_test",
)

cc_binary(
name = "evaluation_all",
srcs = ["evaluation_main.cpp"],
visibility = ["//visibility:public"],
deps = [
"//score/evaluation/concurrency:concurrency_issues",
"//score/evaluation/design:design_faults",
"//score/evaluation/exception_handling:exception_error_handling",
"//score/evaluation/idiomatic:non_idiomatic_cpp",
"//score/evaluation/memory:memory_issues",
"//score/evaluation/modern_cpp:modern_cpp_syntax",
"//score/evaluation/optimization:code_optimization",
"//score/evaluation/patterns:design_patterns",
"//score/evaluation/security:security_issues",
"//score/evaluation/safety:safe_cpp",
"//score/evaluation/solid:solid_violations",
"//score/evaluation/template_metaprogramming:template_metaprogramming_issues",
"//score/evaluation/unit_test_quality:unit_test_quality",
],
)
8 changes: 8 additions & 0 deletions score/evaluation/concurrency/BUILD
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@

load("@rules_cc//cc:defs.bzl", "cc_library")

cc_library(
name = "concurrency_issues",
hdrs = ["concurrency_issues.h"],
visibility = ["//visibility:public"],
)
148 changes: 148 additions & 0 deletions score/evaluation/concurrency/concurrency_issues.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,148 @@


#ifndef SCORE_EVALUATION_CONCURRENCY_ISSUES_H
#define SCORE_EVALUATION_CONCURRENCY_ISSUES_H

#include <atomic>
#include <condition_variable>
#include <mutex>
#include <string>
#include <thread>

namespace score
{
namespace evaluation
{

class UnsafeCounter
{
public:
void Increment() { ++count_; }
int Get() const { return count_; }

private:
int count_{0};
};

class ManualLockUser
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Advisory

[Unused Code] The class ManualLockUser is defined but never used anywhere else in this PR (searched all files). Please remove it or add callers.

Context for Agents
The class `ManualLockUser` is defined but never used anywhere else in this PR (searched all files). Please remove it or add callers.

File: score/evaluation/concurrency/concurrency_issues.h
Line: 27

{
public:
void Write(const std::string& data)
{
mtx_.lock();
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Critical

[Reliability] Manual use of mtx_.lock() and mtx_.unlock() is not exception-safe. If ProcessData(data) throws an exception, mtx_.unlock() will never be called, leading to a permanent deadlock for any other thread trying to acquire the mutex.

Suggested change
mtx_.lock();
void Write(const std::string& data)
{
std::lock_guard<std::mutex> lock(mtx_);
ProcessData(data);
}
Context for Agents
[**Bug**] Manual use of `mtx_.lock()` and `mtx_.unlock()` is not exception-safe. If `ProcessData(data)` throws an exception, `mtx_.unlock()` will never be called, leading to a permanent deadlock for any other thread trying to acquire the mutex.

```suggestion
    void Write(const std::string& data)
    {
        std::lock_guard<std::mutex> lock(mtx_);
        ProcessData(data);
    }
```

File: score/evaluation/concurrency/concurrency_issues.h
Line: 32

ProcessData(data);
mtx_.unlock();
}

private:
static void ProcessData(const std::string& /*data*/)
{
}

std::mutex mtx_;
};

class BrokenLazyInit
{
public:
void EnsureInitialised()
{
if (!initialised_)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Critical

[Logic] The initialised_ flag is read outside of a lock, while it may be written by another thread inside a lock. This constitutes a data race and can lead to undefined behavior. Both reads and writes to shared data must be synchronized.

Context for Agents
The `initialised_` flag is read outside of a lock, while it may be written by another thread inside a lock. This constitutes a data race and can lead to undefined behavior. Both reads and writes to shared data must be synchronized.

File: score/evaluation/concurrency/concurrency_issues.h
Line: 50

{
std::lock_guard<std::mutex> lk(mtx_);
if (!initialised_)
{
resource_ = 42;
initialised_ = true;
}
}
}

int GetResource() const { return resource_; }

private:
bool initialised_{false};
int resource_{0};
std::mutex mtx_;
};

class DeadlockProne
{
public:
void TransferAB(int amount)
{
std::lock_guard<std::mutex> la(mtx_a_);
std::lock_guard<std::mutex> lb(mtx_b_);
balance_a_ -= amount;
balance_b_ += amount;
}

void TransferBA(int amount)
{
std::lock_guard<std::mutex> lb(mtx_b_);
std::lock_guard<std::mutex> la(mtx_a_);
balance_b_ -= amount;
balance_a_ += amount;
}

private:
std::mutex mtx_a_;
std::mutex mtx_b_;
int balance_a_{1000};
int balance_b_{1000};
};

class SpuriousWakeupBug
{
public:
void Wait()
{
std::unique_lock<std::mutex> lk(mtx_);
if (!ready_) cv_.wait(lk);
}

void Signal()
{
std::lock_guard<std::mutex> lk(mtx_);
ready_ = true;
cv_.notify_one();
}

private:
std::mutex mtx_;
std::condition_variable cv_;
bool ready_{false};
};

inline void StartDetachedThread()
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Advisory

[Unused Code] The helper StartDetachedThread is defined but never invoked in this PR (confirmed by searching all files). Consider deleting it if it isn’t needed.

Context for Agents
The helper `StartDetachedThread` is defined but never invoked in this PR (confirmed by searching all files). Consider deleting it if it isn’t needed.

File: score/evaluation/concurrency/concurrency_issues.h
Line: 117

{
std::string local_config = "config_data";

std::thread t([&local_config]()
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Critical

[Reliability] The lambda captures local_config by reference and is executed in a detached thread. Since local_config is a local variable that goes out of scope when StartDetachedThread returns, the thread will attempt to access a destroyed object, resulting in a Use-After-Free (UAF).

Context for Agents
[**Bug**] The lambda captures `local_config` by reference and is executed in a detached thread. Since `local_config` is a local variable that goes out of scope when `StartDetachedThread` returns, the thread will attempt to access a destroyed object, resulting in a Use-After-Free (UAF).

File: score/evaluation/concurrency/concurrency_issues.h
Line: 121

{
(void)local_config;
});
t.detach();
}

class TocTouAtomic
{
public:
void ConsumeIfAvailable()
{
if (count_.load() > 0)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Important

[Reliability] ConsumeIfAvailable has a Check-Then-Act race condition. Even though count_ is atomic, another thread could decrement count_ to 0 between the check count_.load() > 0 and the decrement --count_, causing the count to wrap around to a very large positive value (if unsigned) or become negative.

Context for Agents
[**Bug**] `ConsumeIfAvailable` has a Check-Then-Act race condition. Even though `count_` is atomic, another thread could decrement `count_` to 0 between the check `count_.load() > 0` and the decrement `--count_`, causing the count to wrap around to a very large positive value (if unsigned) or become negative.

File: score/evaluation/concurrency/concurrency_issues.h
Line: 133

{
--count_;
}
}

void Produce() { ++count_; }

private:
std::atomic<int> count_{0};
};

}
}

#endif // SCORE_EVALUATION_CONCURRENCY_ISSUES_H
8 changes: 8 additions & 0 deletions score/evaluation/design/BUILD
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@

load("@rules_cc//cc:defs.bzl", "cc_library")

cc_library(
name = "design_faults",
hdrs = ["design_faults.h"],
visibility = ["//visibility:public"],
)
Loading
Loading