From 0bed2255a15f23fea8d5e49e323db21a4bae0309 Mon Sep 17 00:00:00 2001 From: "Jonathan Pallant (Ferrous Systems)" Date: Wed, 8 Mar 2023 17:32:57 +0000 Subject: [PATCH 1/4] Start a GDB server with --gdblisten 127.0.0.1:1337 TODO: Turn down some warning logs TODO: Stop the GDB server when probe-run exits. --- CHANGELOG.md | 2 + Cargo.lock | 55 +++++++++++++++++++++ Cargo.toml | 1 + src/cli.rs | 4 ++ src/main.rs | 134 ++++++++++++++++++++++++++++++++++++++++++--------- 5 files changed, 172 insertions(+), 24 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 46d7b000..ab571b23 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,12 +11,14 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/) and this p - [#380] Add `--erase-all` flag - [#379] Update to `probe-rs v0.17` - [#378] Update to `probe-rs v0.16` +- [#86] Add GDB server support [#384]: https://github.com/knurling-rs/probe-run/pull/384 [#381]: https://github.com/knurling-rs/probe-run/pull/381 [#380]: https://github.com/knurling-rs/probe-run/pull/380 [#379]: https://github.com/knurling-rs/probe-run/pull/379 [#378]: https://github.com/knurling-rs/probe-run/pull/378 +[#86]: https://github.com/knurling-rs/probe-run/pull/386 ## [v0.3.6] - 2023-01-23 diff --git a/Cargo.lock b/Cargo.lock index 1d0fc71e..973821ac 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -271,6 +271,12 @@ version = "1.0.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "210ec60ae7d710bed8683e333e9d2855a8a56a3e9892b38bad3bb0d4d29b0d5e" +[[package]] +name = "either" +version = "1.8.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7fcaabb2fef8c910e7f4c7ce9f67a1283a1715879a7c230ca9d6d1ae31f16d91" + [[package]] name = "encode_unicode" version = "0.3.6" @@ -331,6 +337,33 @@ version = "2.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e6d5a32815ae3f33302d95fdcb2ce17862f8c65363dcfd29360480ba1001fc9c" +[[package]] +name = "gdb-server" +version = "0.17.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e59631a29fea7216c07dfb791245631ee338e507b0e092a7220bdb2b4332d45e" +dependencies = [ + "anyhow", + "gdbstub", + "itertools", + "log", + "probe-rs", +] + +[[package]] +name = "gdbstub" +version = "0.6.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ba4fddc6f9d12cbef29e395d9a6b48c128f513c8a2ded7048c97ed5c484e53e7" +dependencies = [ + "bitflags", + "cfg-if", + "log", + "managed", + "num-traits", + "paste", +] + [[package]] name = "getrandom" version = "0.2.8" @@ -468,6 +501,15 @@ dependencies = [ "windows-sys", ] +[[package]] +name = "itertools" +version = "0.10.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b0fd2260e829bddf4cb6ea802289de2f86d6a7a690192fbe91b3f46e0f2c8473" +dependencies = [ + "either", +] + [[package]] name = "itoa" version = "1.0.5" @@ -569,6 +611,12 @@ dependencies = [ "serde", ] +[[package]] +name = "managed" +version = "0.8.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0ca88d725a0a943b096803bd34e73a4437208b6077654cc4ecb2947a5f91618d" + [[package]] name = "memchr" version = "2.5.0" @@ -680,6 +728,12 @@ dependencies = [ "windows-sys", ] +[[package]] +name = "paste" +version = "1.0.12" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9f746c4065a8fa3fe23974dd82f15431cc8d40779821001404d10d2e79ca7d79" + [[package]] name = "pin-project-lite" version = "0.2.9" @@ -764,6 +818,7 @@ dependencies = [ "colored", "defmt-decoder", "dirs", + "gdb-server", "gimli", "git-version", "insta", diff --git a/Cargo.toml b/Cargo.toml index 226ba757..00620b6c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -27,6 +27,7 @@ log = "0.4" object = { version = "0.30", default-features = false } probe-rs = "0.17" signal-hook = "0.3" +gdb-server = "0.17" [dev-dependencies] dirs = "4" diff --git a/src/cli.rs b/src/cli.rs index 144d61fc..7463e122 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -91,6 +91,10 @@ pub struct Opts { #[arg(long)] pub verify: bool, + /// Starts a GDB server on this port + #[arg(long)] + pub gdblisten: Option, + /// Prints version information #[arg(short = 'V', long)] version: bool, diff --git a/src/main.rs b/src/main.rs index e5e22328..6712f17d 100644 --- a/src/main.rs +++ b/src/main.rs @@ -15,7 +15,7 @@ use std::{ path::Path, process, sync::atomic::{AtomicBool, Ordering}, - sync::Arc, + sync::{Arc, Mutex}, time::Duration, }; @@ -87,7 +87,7 @@ fn run_target_program(elf_path: &Path, chip_name: &str, opts: &cli::Opts) -> any } probe_attach? }; - log::debug!("started session"); + log::warn!("started session"); if opts.no_flash { log::info!("skipped flashing"); @@ -112,18 +112,36 @@ fn run_target_program(elf_path: &Path, chip_name: &str, opts: &cli::Opts) -> any if opts.measure_stack && canary.is_none() { bail!("failed to set up stack measurement"); } - start_program(&mut sess, elf)?; + + let sess = Arc::new(Mutex::new(sess)); + + let gdb_handle = start_program(sess.clone(), elf, opts.gdblisten.as_deref())?; let current_dir = &env::current_dir()?; - let memory_map = sess.target().memory_map.clone(); - let mut core = sess.core(0)?; + log::warn!("Getting memory map"); + + let memory_map = sess.lock().unwrap().target().memory_map.clone(); + + log::warn!("Getting core"); + + log::warn!("Getting logs"); + + let halted_due_to_signal = extract_and_print_logs( + elf, + sess.clone(), + &memory_map, + opts, + current_dir, + opts.gdblisten.is_some(), + )?; - let halted_due_to_signal = - extract_and_print_logs(elf, &mut core, &memory_map, opts, current_dir)?; + log::warn!("Got logs, result = {}", halted_due_to_signal); print_separator()?; + let mut locked_session = sess.lock().unwrap(); + let mut core = locked_session.core(0)?; let canary_touched = canary .map(|canary| canary.touched(&mut core, elf)) .transpose()? @@ -155,31 +173,84 @@ fn run_target_program(elf_path: &Path, chip_name: &str, opts: &cli::Opts) -> any core.reset_and_halt(TIMEOUT)?; + drop(core); + drop(locked_session); + outcome.log(); + if let Some(gdb_handle) = gdb_handle { + log::warn!("Waiting for GDB to stop"); + let _ = gdb_handle.join(); + } + Ok(outcome.into()) } -fn start_program(sess: &mut Session, elf: &Elf) -> anyhow::Result<()> { - let mut core = sess.core(0)?; +fn start_program( + sess: Arc>, + elf: &Elf, + gdbport: Option<&str>, +) -> anyhow::Result>> { + { + let mut locked_session = sess.lock().unwrap(); + let mut core = locked_session.core(0)?; + + log::debug!("starting device"); + if core.available_breakpoint_units()? == 0 { + if elf.rtt_buffer_address().is_some() { + bail!("RTT not supported on device without HW breakpoints"); + } else { + log::warn!("device doesn't support HW breakpoints; HardFault will NOT make `probe-run` exit with an error code"); + } + } - log::debug!("starting device"); - if core.available_breakpoint_units()? == 0 { - if elf.rtt_buffer_address().is_some() { - bail!("RTT not supported on device without HW breakpoints"); - } else { - log::warn!("device doesn't support HW breakpoints; HardFault will NOT make `probe-run` exit with an error code"); + if let Some(rtt_buffer_address) = elf.rtt_buffer_address() { + set_rtt_to_blocking(&mut core, elf.main_fn_address(), rtt_buffer_address)? } + + core.set_hw_breakpoint(cortexm::clear_thumb_bit(elf.vector_table.hard_fault).into())?; } - if let Some(rtt_buffer_address) = elf.rtt_buffer_address() { - set_rtt_to_blocking(&mut core, elf.main_fn_address(), rtt_buffer_address)? + let mut gdb_thread_handle = None; + + if let Some(port) = gdbport { + log::info!("Starting gdbserver on {:?}", port); + let gdb_connection_string = port.to_owned(); + let session = sess.clone(); + + gdb_thread_handle = Some(std::thread::spawn(move || { + log::info!( + " {} listening at {}", + "GDB stub".green().bold(), + gdb_connection_string + ); + + let instances = { + let session = session.lock().unwrap(); + probe_rs_gdb_server::GdbInstanceConfiguration::from_session( + &session, + Some(gdb_connection_string), + ) + }; + + if let Err(e) = probe_rs_gdb_server::run(&session, instances.iter()) { + log::warn!("During the execution of GDB an error was encountered:"); + log::warn!("{:?}", e); + } + })); } - core.set_hw_breakpoint(cortexm::clear_thumb_bit(elf.vector_table.hard_fault).into())?; - core.run()?; + if gdb_thread_handle.is_none() { + // Only start core if there is no GDB server + log::warn!("Starting core..."); + let mut locked_session = sess.lock().unwrap(); + let mut core = locked_session.core(0)?; + core.run()?; + } - Ok(()) + log::warn!("Core started..."); + + Ok(gdb_thread_handle) } /// Set rtt to blocking mode @@ -215,20 +286,26 @@ fn set_rtt_to_blocking( fn extract_and_print_logs( elf: &Elf, - core: &mut probe_rs::Core, + sess: Arc>, memory_map: &[MemoryRegion], opts: &cli::Opts, current_dir: &Path, + using_gdb: bool, ) -> anyhow::Result { let exit = Arc::new(AtomicBool::new(false)); let sig_id = signal_hook::flag::register(signal::SIGINT, exit.clone())?; + let mut locked_session = sess.lock().unwrap(); + let mut core = locked_session.core(0)?; + let mut logging_channel = if let Some(address) = elf.rtt_buffer_address() { - Some(setup_logging_channel(address, core, memory_map)?) + Some(setup_logging_channel(address, &mut core, memory_map)?) } else { eprintln!("RTT logs not available; blocking until the device halts.."); None }; + drop(core); + drop(locked_session); let use_defmt = logging_channel .as_ref() @@ -256,8 +333,15 @@ fn extract_and_print_logs( let mut read_buf = [0; 1024]; let mut was_halted = false; while !exit.load(Ordering::Relaxed) { + if using_gdb { + // Give GDB a chance. This is the same value used in probe-rs-cli. + std::thread::sleep(std::time::Duration::from_millis(10)); + } + let mut locked_session = sess.lock().unwrap(); + let mut core = locked_session.core(0)?; + if let Some(logging_channel) = &mut logging_channel { - let num_bytes_read = match logging_channel.read(core, &mut read_buf) { + let num_bytes_read = match logging_channel.read(&mut core, &mut read_buf) { Ok(n) => n, Err(e) => { eprintln!("RTT error: {e}"); @@ -289,7 +373,7 @@ fn extract_and_print_logs( let is_halted = core.core_halted()?; - if is_halted && was_halted { + if is_halted && was_halted && !using_gdb { break; } was_halted = is_halted; @@ -303,6 +387,8 @@ fn extract_and_print_logs( // TODO refactor: a printing fucntion shouldn't stop the MC as a side effect // Ctrl-C was pressed; stop the microcontroller. if exit.load(Ordering::Relaxed) { + let mut locked_session = sess.lock().unwrap(); + let mut core = locked_session.core(0)?; core.halt(TIMEOUT)?; } From 53270c43026208e8548c50e84ba591a450384e99 Mon Sep 17 00:00:00 2001 From: "Jonathan Pallant (Ferrous Systems)" Date: Wed, 8 Mar 2023 17:33:30 +0000 Subject: [PATCH 2/4] Fix PR number in changelog. --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ab571b23..1d949f1e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,7 +18,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/) and this p [#380]: https://github.com/knurling-rs/probe-run/pull/380 [#379]: https://github.com/knurling-rs/probe-run/pull/379 [#378]: https://github.com/knurling-rs/probe-run/pull/378 -[#86]: https://github.com/knurling-rs/probe-run/pull/386 +[#86]: https://github.com/knurling-rs/probe-run/pull/388 ## [v0.3.6] - 2023-01-23 From 71e03b7ed3adb94294bbdd372b4f8e05d540e959 Mon Sep 17 00:00:00 2001 From: "Jonathan Pallant (Ferrous Systems)" Date: Thu, 9 Mar 2023 11:50:27 +0000 Subject: [PATCH 3/4] Reduce log levels. --- src/main.rs | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/src/main.rs b/src/main.rs index 6712f17d..8b9902aa 100644 --- a/src/main.rs +++ b/src/main.rs @@ -87,7 +87,7 @@ fn run_target_program(elf_path: &Path, chip_name: &str, opts: &cli::Opts) -> any } probe_attach? }; - log::warn!("started session"); + log::debug!("started session"); if opts.no_flash { log::info!("skipped flashing"); @@ -119,13 +119,11 @@ fn run_target_program(elf_path: &Path, chip_name: &str, opts: &cli::Opts) -> any let current_dir = &env::current_dir()?; - log::warn!("Getting memory map"); + log::debug!("Getting memory map"); let memory_map = sess.lock().unwrap().target().memory_map.clone(); - log::warn!("Getting core"); - - log::warn!("Getting logs"); + log::debug!("Getting logs"); let halted_due_to_signal = extract_and_print_logs( elf, @@ -136,7 +134,7 @@ fn run_target_program(elf_path: &Path, chip_name: &str, opts: &cli::Opts) -> any opts.gdblisten.is_some(), )?; - log::warn!("Got logs, result = {}", halted_due_to_signal); + log::debug!("Got logs, result = {}", halted_due_to_signal); print_separator()?; @@ -214,14 +212,13 @@ fn start_program( let mut gdb_thread_handle = None; if let Some(port) = gdbport { - log::info!("Starting gdbserver on {:?}", port); let gdb_connection_string = port.to_owned(); let session = sess.clone(); gdb_thread_handle = Some(std::thread::spawn(move || { log::info!( - " {} listening at {}", - "GDB stub".green().bold(), + "{} to {}", + "Now connect GDB".green().bold(), gdb_connection_string ); @@ -242,14 +239,13 @@ fn start_program( if gdb_thread_handle.is_none() { // Only start core if there is no GDB server - log::warn!("Starting core..."); + log::debug!("Starting core..."); let mut locked_session = sess.lock().unwrap(); let mut core = locked_session.core(0)?; core.run()?; + log::info!("Core started..."); } - log::warn!("Core started..."); - Ok(gdb_thread_handle) } From 0827c281b86a59ac081783eb2893af0ec45aea69 Mon Sep 17 00:00:00 2001 From: "Jonathan Pallant (Ferrous Systems)" Date: Thu, 9 Mar 2023 11:50:41 +0000 Subject: [PATCH 4/4] Support GDB on Startup and GDB on Fault. Adds separate argument/env var for GDB Server port. The default is 127.0.0.1:3333. --- src/cli.rs | 19 ++++++++++++--- src/main.rs | 68 ++++++++++++++++++++++++++++++++++++++++++++++------- 2 files changed, 76 insertions(+), 11 deletions(-) diff --git a/src/cli.rs b/src/cli.rs index 7463e122..802a1ab2 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -91,9 +91,22 @@ pub struct Opts { #[arg(long)] pub verify: bool, - /// Starts a GDB server on this port - #[arg(long)] - pub gdblisten: Option, + /// Sets which port we bind the GDB Server to. + /// + /// Note: doesn't necessarily start the GDB Server. See `--gdb-on-start` and + /// `--gdb-on-crash`. + #[arg(long, env = "PROBE_RUN_GDB_PORT")] + pub gdb_port: Option, + + /// Starts the GDB Server right after flashing. + /// + /// You must use GDB to start the processor or it won't run. + #[arg(long, conflicts_with = "gdb_on_fault")] + pub gdb_on_start: bool, + + /// Starts the GDB Server only if the chip faults. + #[arg(long, conflicts_with = "gdb_on_start")] + pub gdb_on_fault: bool, /// Prints version information #[arg(short = 'V', long)] diff --git a/src/main.rs b/src/main.rs index 8b9902aa..06ec41d6 100644 --- a/src/main.rs +++ b/src/main.rs @@ -36,6 +36,15 @@ use crate::{backtrace::Outcome, canary::Canary, elf::Elf, target_info::TargetInf const TIMEOUT: Duration = Duration::from_secs(1); +/// The default TCP port for the GDB Server +/// +/// Port 3333 is the OpenOCD default, so we use that. +/// +/// We only bind to localhost for security reasons, and we force IPv4 because +/// using `localhost` might bind to the IPv6 address `::1`, which is non-obvious +/// and will cause issues if people use `target remote 127.0.0.1:3333` in GDB. +const GDB_SERVER_DEFAULT_PORT: &str = "127.0.0.1:3333"; + fn main() -> anyhow::Result<()> { configure_terminal_colorization(); @@ -115,7 +124,14 @@ fn run_target_program(elf_path: &Path, chip_name: &str, opts: &cli::Opts) -> any let sess = Arc::new(Mutex::new(sess)); - let gdb_handle = start_program(sess.clone(), elf, opts.gdblisten.as_deref())?; + let gdb_port = opts.gdb_port.as_deref().unwrap_or(GDB_SERVER_DEFAULT_PORT); + let startup_gdb_port = if opts.gdb_on_start { + Some(gdb_port) + } else { + None + }; + + let _gdb_handle = start_program(sess.clone(), elf, startup_gdb_port)?; let current_dir = &env::current_dir()?; @@ -131,7 +147,7 @@ fn run_target_program(elf_path: &Path, chip_name: &str, opts: &cli::Opts) -> any &memory_map, opts, current_dir, - opts.gdblisten.is_some(), + opts.gdb_on_start, )?; log::debug!("Got logs, result = {}", halted_due_to_signal); @@ -169,16 +185,52 @@ fn run_target_program(elf_path: &Path, chip_name: &str, opts: &cli::Opts) -> any outcome = Outcome::CtrlC } - core.reset_and_halt(TIMEOUT)?; - drop(core); drop(locked_session); - outcome.log(); + if opts.gdb_on_fault && outcome == Outcome::HardFault { + outcome.log(); + + let exit = Arc::new(AtomicBool::new(false)); + signal_hook::flag::register(signal::SIGINT, Arc::clone(&exit))?; + + log::info!("Starting gdbserver on {:?}", gdb_port); + let gdb_connection_string = gdb_port.to_owned(); + let session = sess.clone(); + + let _gdb_thread_handle = Some(std::thread::spawn(move || { + log::info!( + "{} to {}", + "Now connect GDB".green().bold(), + gdb_connection_string + ); + log::info!("Press Ctrl-C to exit..."); + + let instances = { + let session = session.lock().unwrap(); + probe_rs_gdb_server::GdbInstanceConfiguration::from_session( + &session, + Some(gdb_connection_string), + ) + }; + + if let Err(e) = probe_rs_gdb_server::run(&session, instances.iter()) { + log::warn!("During the execution of GDB an error was encountered:"); + log::warn!("{:?}", e); + } + })); + + while !exit.load(Ordering::Relaxed) { + std::thread::sleep(Duration::from_millis(100)); + } - if let Some(gdb_handle) = gdb_handle { - log::warn!("Waiting for GDB to stop"); - let _ = gdb_handle.join(); + // FIXME there's no clean way to terminate the `run` thread. in the current implementation, + // the process will `exit` (see `main` function) and `Session`'s destructor will not run + } else { + let mut locked_session = sess.lock().unwrap(); + let mut core = locked_session.core(0)?; + core.reset_and_halt(TIMEOUT)?; + outcome.log(); } Ok(outcome.into())