Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove debug feature from fuel-vm and use debugger by default. #554

Merged
merged 7 commits into from
Aug 29, 2023
Merged
Show file tree
Hide file tree
Changes from 5 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
2 changes: 1 addition & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ concurrency:

env:
CARGO_TERM_COLOR: always
RUST_VERSION: 1.69.0
RUST_VERSION: 1.71.0
NIGHTLY_RUST_VERSION: nightly-2023-05-23

jobs:
Expand Down
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).

#### Breaking

- [#554](https://github.com/FuelLabs/fuel-vm/pull/554): Removed `debug` feature from teh `fuel-vm`. The debugger is always available and becomes active after calling any `set_*` method.
xgreenx marked this conversation as resolved.
Show resolved Hide resolved
- [#537](https://github.com/FuelLabs/fuel-vm/pull/537): Use dependent cost for `k256`, `s256`, `mcpi`, `scwq`, `swwq` opcodes.
These opcodes charged inadequately low costs in comparison to the amount of work.
This change should make all transactions that used these opcodes much more expensive than before.
Expand Down
2 changes: 1 addition & 1 deletion ci_checks.sh
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
# The script runs almost all CI checks locally.
#
# Requires installed:
# - Rust `1.69.0`
# - Rust `1.71.0`
# - Nightly rust formatter
# - `rustup target add thumbv6m-none-eabi`
# - `rustup target add wasm32-unknown-unknown`
Expand Down
1 change: 0 additions & 1 deletion fuel-vm/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,6 @@ tokio-rayon = "2.1.0"
[features]
default = []
arbitrary = ["fuel-asm/arbitrary"]
debug = []
optimized = []
profile-gas = ["profile-any"]
profile-coverage = ["profile-any"]
Expand Down
2 changes: 0 additions & 2 deletions fuel-vm/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ pub enum InterpreterError {
#[error("Unrecoverable error: {0}")]
Io(#[from] io::Error),

#[cfg(feature = "debug")]
#[error("Execution error")]
/// The debug state is not initialized; debug routines can't be called.
DebugStateNotInitialized,
Expand Down Expand Up @@ -118,7 +117,6 @@ impl PartialEq for InterpreterError {
(Self::NoTransactionInitialized, Self::NoTransactionInitialized) => true,
(Self::Io(s), Self::Io(o)) => s.kind() == o.kind(),

#[cfg(feature = "debug")]
(Self::DebugStateNotInitialized, Self::DebugStateNotInitialized) => true,

_ => false,
Expand Down
1 change: 0 additions & 1 deletion fuel-vm/src/interpreter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,6 @@ mod metadata;
mod post_execution;
mod receipts;

#[cfg(feature = "debug")]
mod debug;

use crate::profiler::Profiler;
Expand Down
2 changes: 1 addition & 1 deletion fuel-vm/src/interpreter/balances.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ impl TryFrom<InitialBalances> for RuntimeBalances {
.checked_add(*retryable_amount)
.ok_or(CheckError::ArithmeticOverflow)?;
}
Self::try_from_iter(balances.into_iter())
Self::try_from_iter(balances)
}
}

Expand Down
1 change: 0 additions & 1 deletion fuel-vm/src/interpreter/executors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,4 @@ mod instruction;
mod main;
mod predicate;

#[cfg(feature = "debug")]
mod debug;
3 changes: 1 addition & 2 deletions fuel-vm/src/interpreter/executors/instruction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,7 @@ where
&mut self,
raw: R,
) -> Result<ExecuteState, InterpreterError> {
#[cfg(feature = "debug")]
{
if self.debugger.is_active() {
let debug = self.eval_debugger_state();
if !debug.should_continue() {
return Ok(debug.into())
Expand Down
2 changes: 0 additions & 2 deletions fuel-vm/src/interpreter/executors/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -569,7 +569,6 @@ where

self.append_receipt(receipt);

#[cfg(feature = "debug")]
if program.is_debug() {
self.debugger_set_last_state(program);
}
Expand Down Expand Up @@ -620,7 +619,6 @@ where

ExecuteState::Proceed => (),

#[cfg(feature = "debug")]
ExecuteState::DebugEvent(d) => return Ok(ProgramState::RunProgram(d)),
}
}
Expand Down
1 change: 0 additions & 1 deletion fuel-vm/src/interpreter/executors/predicate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@ where

ExecuteState::Proceed => (),

#[cfg(feature = "debug")]
ExecuteState::DebugEvent(d) => {
return Ok(ProgramState::VerifyPredicate(d))
}
Expand Down
1 change: 0 additions & 1 deletion fuel-vm/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,6 @@ pub mod prelude {
transactor::Transactor,
};

#[cfg(feature = "debug")]
pub use crate::state::{
Breakpoint,
DebugEval,
Expand Down
24 changes: 1 addition & 23 deletions fuel-vm/src/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,25 +6,17 @@ use fuel_types::{
Word,
};

#[cfg(feature = "debug")]
mod debug;

#[cfg(feature = "debug")]
mod debugger;

#[cfg(feature = "debug")]
pub use debug::{
Breakpoint,
DebugEval,
};

#[cfg(feature = "debug")]
pub use debugger::Debugger;

#[cfg(not(feature = "debug"))]
/// Fallback functionless implementation if `debug` feature isn't enabled.
pub type Debugger = ();

#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
/// Resulting state of an instruction set execution.
pub enum ExecuteState {
Expand All @@ -37,23 +29,14 @@ pub enum ExecuteState {
/// The set execution resulted in a `RVRT` instruction.
Revert(Word),

#[cfg(feature = "debug")]
/// A debug event was reached.
DebugEvent(DebugEval),
}

impl ExecuteState {
/// Return true if the VM execution should continue.
pub const fn should_continue(&self) -> bool {
#[cfg(not(feature = "debug"))]
{
matches!(self, Self::Proceed)
}

#[cfg(feature = "debug")]
{
matches!(self, Self::Proceed | Self::DebugEvent(DebugEval::Continue))
}
matches!(self, Self::Proceed | Self::DebugEvent(DebugEval::Continue))
}
}

Expand All @@ -63,7 +46,6 @@ impl Default for ExecuteState {
}
}

#[cfg(feature = "debug")]
impl From<DebugEval> for ExecuteState {
fn from(d: DebugEval) -> Self {
Self::DebugEvent(d)
Expand All @@ -81,17 +63,14 @@ pub enum ProgramState {
/// The transaction execution resulted in a `RVRT` instruction.
Revert(Word),

#[cfg(feature = "debug")]
/// A debug event was reached for the transaction. The VM is suspended.
RunProgram(DebugEval),

#[cfg(feature = "debug")]
/// A debug event was reached for a predicate verification. The VM is
/// suspended.
VerifyPredicate(DebugEval),
}

#[cfg(feature = "debug")]
impl PartialEq<Breakpoint> for ProgramState {
fn eq(&self, other: &Breakpoint) -> bool {
match self.debug_ref() {
Expand All @@ -101,7 +80,6 @@ impl PartialEq<Breakpoint> for ProgramState {
}
}

#[cfg(feature = "debug")]
impl ProgramState {
/// Debug event representation.
///
Expand Down
16 changes: 12 additions & 4 deletions fuel-vm/src/state/debugger.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,31 +14,37 @@ use std::collections::{
HashSet,
};

#[derive(Debug, Default, Clone)]
/// Debugger implementation for the VM.
///
/// Required features:
/// - `debug`
#[derive(Debug, Default, Clone)]
pub struct Debugger {
/// Debugger is active and used.
is_active: bool,
/// Single-stepping mode triggers a breakpoint after each instruction
single_stepping: bool,
breakpoints: HashMap<ContractId, HashSet<Word>>,
last_state: Option<ProgramState>,
}

impl Debugger {
/// Returns `true` if the `Debugger` is active and used.
pub const fn is_active(&self) -> bool {
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it be useful to expose an API to activate the debugger explicitly, without calling a set_* function?

Copy link
Contributor

@bvrooman bvrooman Aug 28, 2023

Choose a reason for hiding this comment

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

Is there any way to turn off debugging once it's on? In case you need to switch between debugging and performance modes

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We don't provide this kind of functionality on the API side, so we don't need to worry about on VM side right now. Plus, in the future, we plan to re-work how the debugger works, and it is more to support that already exists.

self.is_active
}

/// Get single-stepping mode
pub const fn single_stepping(&self) -> bool {
self.single_stepping
}

/// Set single-stepping mode
pub fn set_single_stepping(&mut self, single_stepping: bool) {
self.is_active = true;
self.single_stepping = single_stepping;
}

/// Set a new breakpoint in the provided location.
pub fn set_breakpoint(&mut self, breakpoint: Breakpoint) {
self.is_active = true;
let contract = *breakpoint.contract();
let pc = breakpoint.pc();

Expand All @@ -57,6 +63,7 @@ impl Debugger {

/// Remove a breakpoint, if existent.
pub fn remove_breakpoint(&mut self, breakpoint: &Breakpoint) {
self.is_active = true;
self.breakpoints
.get_mut(breakpoint.contract())
.map(|set| set.remove(&breakpoint.pc()));
Expand Down Expand Up @@ -90,6 +97,7 @@ impl Debugger {

/// Overwrite the last known state of the VM.
pub fn set_last_state(&mut self, state: ProgramState) {
self.is_active = true;
self.last_state.replace(state);
}

Expand Down