Skip to content
Open
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
4 changes: 4 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -31,3 +31,7 @@ pest_derive = "2.8.1"
llvm-sys = "201.0.1"
docopt = "1.1.1"
signal-hook = "0.3.18"

[features]
default = []
nightly = []
27 changes: 21 additions & 6 deletions Containerfile
Original file line number Diff line number Diff line change
@@ -1,10 +1,8 @@
FROM registry.fedoraproject.org/fedora:43 as builder

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Minor Dockerfile lint fixes flagged by pipeline.

as should match FROM casing (AS) on line 1, and ENTRYPOINT should use JSON/exec form on line 62.

🧹 Proposed fix
-FROM registry.fedoraproject.org/fedora:43 as builder
+FROM registry.fedoraproject.org/fedora:43 AS builder
-ENTRYPOINT berserker
+ENTRYPOINT ["berserker"]

Also applies to: 62-62

🧰 Tools
🪛 GitHub Actions: Main berserker CI / 0_build-and-push.txt

[warning] 1-1: Dockerfile lint warning: FromAsCasing - 'as' and 'FROM' keywords' casing do not match.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@Containerfile` at line 1, Fix the Dockerfile lint issues by updating the
stage alias in the top-level FROM instruction to use matching uppercase AS, and
change the ENTRYPOINT in the Containerfile to JSON/exec form instead of shell
form. Use the FROM and ENTRYPOINT instructions in this file as the targets for
the cleanup.

Source: Pipeline failures


ARG RUST_VERSION=stable

RUN dnf install -y \
rust \
cargo \
clippy \
rustfmt \
# for stub \
nasm \
# for script jit \
Expand All @@ -15,7 +13,19 @@ RUN dnf install -y \
# for bpf \
clang \
kernel-devel \
libbpf-devel
libbpf-devel && \
curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | \
sh -s -- -y --default-toolchain $RUST_VERSION --profile minimal

# $HOME is not evaluated here, any better way of getting the toolchain
# directory?
ENV PATH="${PATH}:/root/.cargo/bin"

RUN rustup component add rustfmt clippy

RUN if [ "${RUST_VERSION}" == "nightly" ]; then \
rustup component add rust-src --toolchain nightly; \
fi

ADD ./ /berserker/

Expand All @@ -32,7 +42,12 @@ RUN cargo build -r
# Test will require stub binary to be available
ENV PATH="${PATH}:/berserker:/berserker/target/release"

RUN cargo test
RUN if [ "${RUST_VERSION}" == "nightly" ]; then \
TARGET=$(rustc --version --verbose | grep host | cut -d" " -f2) && \
RUSTFLAGS="-Z sanitizer=address" cargo +nightly test -Z build-std --target "$TARGET"; \
else \
cargo test; \
fi
Comment on lines +45 to +50

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Nightly ASAN run never enables the crate's nightly Cargo feature.

This branch runs cargo +nightly test -Z build-std ... but doesn't pass --features nightly, so #![cfg_attr(feature = "nightly", feature(sanitize))] in src/lib.rs stays disabled during the ASAN run, defeating the purpose of the new feature gate.

🔧 Proposed fix
 RUN if [ "${RUST_VERSION}" == "nightly" ]; then \
         TARGET=$(rustc --version --verbose | grep host | cut -d" " -f2) && \
-        RUSTFLAGS="-Z sanitizer=address" cargo +nightly test -Z build-std --target "$TARGET"; \
+        RUSTFLAGS="-Z sanitizer=address" cargo +nightly test --features nightly -Z build-std --target "$TARGET"; \
     else \
         cargo test; \
     fi
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
RUN if [ "${RUST_VERSION}" == "nightly" ]; then \
TARGET=$(rustc --version --verbose | grep host | cut -d" " -f2) && \
RUSTFLAGS="-Z sanitizer=address" cargo +nightly test -Z build-std --target "$TARGET"; \
else \
cargo test; \
fi
RUN if [ "${RUST_VERSION}" == "nightly" ]; then \
TARGET=$(rustc --version --verbose | grep host | cut -d" " -f2) && \
RUSTFLAGS="-Z sanitizer=address" cargo +nightly test --features nightly -Z build-std --target "$TARGET"; \
else \
cargo test; \
fi
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@Containerfile` around lines 45 - 50, The nightly ASAN test branch in the
Containerfile is missing the crate’s `nightly` Cargo feature, so the `sanitize`
feature gate in `src/lib.rs` never turns on. Update the `cargo +nightly test`
invocation in this RUN block to include the crate’s `nightly` feature alongside
the existing `-Z build-std` and `RUSTFLAGS="-Z sanitizer=address"` settings, so
the ASAN path actually exercises the intended nightly-only code.


FROM registry.fedoraproject.org/fedora:43

Expand Down
1 change: 1 addition & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ endif
.PHONY: all
all:
docker build -t berserker -f Containerfile .
docker build -t berserker -f Containerfile --build-arg=RUST_VERSION=nightly .
Comment on lines 9 to +10

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n== Files ==\n'
git ls-files | rg '(^|/)(Makefile|Containerfile|Dockerfile|README.md)$'

printf '\n== Makefile (mapped) ==\n'
ast-grep outline Makefile --view expanded || true

printf '\n== Makefile with line numbers ==\n'
cat -n Makefile | sed -n '1,120p'

printf '\n== Containerfile with line numbers ==\n'
cat -n Containerfile | sed -n '1,220p'

printf '\n== Search for berserker tag usage ==\n'
rg -n 'berserker|RUST_VERSION=nightly|build-and-push|docker build' -S .

Repository: stackrox/berserker

Length of output: 8595


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n== README build / image references ==\n'
rg -n 'make |docker build|docker run|latest|BERSERKER_TAG|tag:' README.md Makefile Containerfile Containerfile.test scripts -S

printf '\n== README excerpt around usage ==\n'
cat -n README.md | sed -n '1,140p'

Repository: stackrox/berserker

Length of output: 5722


Makefile:9-10 — Use a separate tag for the nightly build. The second docker build -t berserker ... --build-arg=RUST_VERSION=nightly overwrites the first image, so berserker:latest ends up pointing at the nightly build. That means the later test/network images consume the nightly toolchain image, and the stable build is never used.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@Makefile` around lines 9 - 10, The Makefile’s two docker build steps
currently reuse the same berserker tag, so the nightly build overwrites the
stable image. Update the second docker build invocation to use a distinct tag
for the nightly variant while keeping the first build as the default berserker
image, so later steps consume the intended stable image; make this change in the
build commands near the two docker build lines.

docker build -t berserker-test -f Containerfile.test .
docker run --privileged berserker-test

Expand Down
12 changes: 8 additions & 4 deletions src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
#![cfg_attr(feature = "nightly", feature(sanitize))]

use core_affinity::CoreId;
use serde::{Deserialize, Deserializer};
use std::{collections::HashMap, fmt::Display, net::Ipv4Addr, str::FromStr};
Expand Down Expand Up @@ -28,7 +30,8 @@ pub struct WorkloadConfig {
/// Custom workload configuration.
pub workload: Workload,

/// For how long to run the worker. Default value is zero, meaning no limit.
/// For how long to run the worker. Default value is zero, meaning no
/// limit.
#[serde(default = "default_duration")]
pub duration: u64,
}
Expand Down Expand Up @@ -194,8 +197,8 @@ pub enum Workload {
/// Maximum number of dynamic connections
connections_dyn_max: u32,

// How many connections to make to the same server address and port with
// different client ports
/// How many connections to make to the same server address and port
/// with different client ports
#[serde(default = "default_conns_per_addr")]
conns_per_addr: u16,

Expand All @@ -211,7 +214,8 @@ pub enum Workload {
/// Whether or not to wait for a connection to be removed before adding
/// a new one, when the dynamic connection limit is reached.
/// if true: an old connection will be forcibly removed
/// if false: wait for a connection to naturally age-off before adding a new one
/// if false: wait for a connection to naturally age-off before adding
/// a new one
preempt: bool,
},

Expand Down
6 changes: 4 additions & 2 deletions src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,8 @@ fn main() {
.required(false),
)
// Add in settings from the environment (with a prefix of APP)
// Eg.. `BERSERKER__WORKLOAD__ARRIVAL_RATE=1` would set the `arrival_rate` key
// Eg.. `BERSERKER__WORKLOAD__ARRIVAL_RATE=1` would set the
// `arrival_rate` key
.add_source(
config::Environment::with_prefix("BERSERKER")
.try_parsing(true)
Expand All @@ -230,7 +231,8 @@ fn main() {
let elapsed = duration_timer.elapsed().unwrap().as_secs();

// Ignore processes without specified duration -- we don't want
// neither terminate them, nor count against processes to compare.
// neither terminate them, nor count against processes to
// compare.
let watched_processes = processes
.iter()
.filter(|(_, duration)| *duration > 0)
Expand Down
94 changes: 68 additions & 26 deletions src/worker/script.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
extern crate llvm_sys as llvm;

use std::{
cell::RefCell,
collections::HashMap,
fmt::Display,
fs::OpenOptions,
Expand All @@ -23,7 +24,6 @@ use llvm::execution_engine::*;
use llvm::target::*;
use llvm_sys::LLVMType;
use llvm_sys::prelude::*;
use std::cell::RefCell;
use std::ffi::{CStr, CString, c_void};
use std::mem;

Expand Down Expand Up @@ -132,8 +132,7 @@ pub unsafe extern "C" fn task(name: *const i8, random: bool) -> u64 {
}

thread_local! {
static LAST_RANDOM_PATH: RefCell<CString> =
RefCell::new(CString::new("").unwrap());
static POINTERS: RefCell<Vec<*mut i8>> = const { RefCell::new(vec![]) };
}

/// Return a randomly generated path.
Expand All @@ -151,11 +150,24 @@ pub unsafe extern "C" fn random_path(base: *const i8) -> *const i8 {
.map(char::from)
.collect();

LAST_RANDOM_PATH.with(|last| {
let mut last = last.borrow_mut();
*last = CString::new(format!("{base}/{uniq}")).unwrap();
last.as_ptr()
})
let result = CString::new(format!("{base}/{uniq}")).unwrap().into_raw();
POINTERS.with(|ps| ps.borrow_mut().push(result));
result
}

/// # Safety
#[unsafe(no_mangle)]
pub unsafe extern "C" fn cleanup(_: *const i8) -> u64 {
POINTERS.with(|ps| {
let mut vec = ps.borrow_mut();
for p in vec.as_slice() {
let _ = unsafe { CString::from_raw(*p) };
}

vec.clear();
});

0
}

#[derive(Debug, Clone)]
Expand Down Expand Up @@ -217,14 +229,27 @@ pub static RUNTIME: LazyLock<HashMap<String, RuntimeFunc>> =
return_type: RuntimeType::Pointer,
},
),
// utils
(
"cleanup".to_string(),
RuntimeFunc {
func: cleanup as *const () as usize,
param_count: 1,
param_types: &[RuntimeType::Pointer],
return_type: RuntimeType::Int,
},
),
])
});

impl ScriptWorker {
fn jit_instruction(name: String, arg: Arg, ctx: &BuildContext) {
fn jit_instruction(name: &CStr, arg: Arg, ctx: &BuildContext) {
let mut arg_ptr = Self::get_arg_value(arg, ctx);

let (func, func_type) = ctx.module_runtime.get(&name).unwrap();
let (func, func_type) = ctx
.module_runtime
.get(name.to_str().expect("Couldn't convert name to string"))
.unwrap();

unsafe {
LLVMBuildCall2(
Expand All @@ -233,7 +258,7 @@ impl ScriptWorker {
*func,
&mut arg_ptr,
1,
name.as_str().as_ptr() as *const _,
name.as_ptr() as *const _,
);
}
}
Expand Down Expand Up @@ -331,7 +356,8 @@ impl ScriptWorker {
&mut err,
) != 0
{
// In case of error, we must avoid using the uninitialized ExecutionEngineRef.
// In case of error, we must avoid using the uninitialized
// ExecutionEngineRef.
assert!(!err.is_null());
panic!(
"Failed to create execution engine: {:?}",
Expand Down Expand Up @@ -396,8 +422,8 @@ impl ScriptWorker {
function_type,
);

// Create a basic block in the function and set our builder to generate
// code in it.
// Create a basic block in the function and set our builder to
// generate code in it.
let bb = LLVMAppendBasicBlockInContext(
context,
function,
Expand Down Expand Up @@ -436,30 +462,22 @@ impl ScriptWorker {
// JIT the instruction and collect it's name
let name = match instr.clone() {
Instruction::Task { name, args: _ } => {
Self::jit_instruction(String::from("task"), name, &ctx);
Self::jit_instruction(c"task", name, &ctx);
"task"
}

Instruction::Open { path } => {
Self::jit_instruction(String::from("open"), path, &ctx);
Self::jit_instruction(c"open", path, &ctx);
"open"
}

Instruction::Ping { server } => {
Self::jit_instruction(
String::from("ping"),
server,
&ctx,
);
Self::jit_instruction(c"ping", server, &ctx);
"ping"
}

Instruction::Debug { text } => {
Self::jit_instruction(
String::from("debug"),
text,
&ctx,
);
Self::jit_instruction(c"debug", text, &ctx);
"debug"
}
};
Expand All @@ -483,6 +501,30 @@ impl ScriptWorker {
);
}

// Final instruction to clear dangling pointers
Self::jit_instruction(
c"cleanup",
Arg::Const {
text: "".to_string(),
},
&ctx,
);

let module_func = LLVMGetNamedFunction(
module,
c"cleanup".to_bytes().as_ptr() as *const _,
);

let runtime_func = &RUNTIME
.get("cleanup")
.expect("No runtime function with the name");

LLVMAddGlobalMapping(
ee,
module_func,
runtime_func.func as *mut c_void,
);

// Emit a `ret i64` into the function to return the computed sum.
let ret = LLVMConstInt(i64t, 0, 0);
LLVMBuildRet(builder, ret);
Expand Down
4 changes: 2 additions & 2 deletions src/worker/syscalls/ioctl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@ pub struct IoctlCall {

impl IoctlCall {
pub fn new(_: &ArgsMap) -> Self {
// Zero initialize all fields, fd will be initialized in `Syscaller::init`.
// All other fields can be overridden as needed
// Zero initialize all fields, fd will be initialized in
// `Syscaller::init`. All other fields can be overridden as needed
Default::default()
}
}
Expand Down
Loading