From 56175f63dfe326bb24b9dba3d3692b87153f08cb Mon Sep 17 00:00:00 2001 From: Zhang Junyang Date: Mon, 22 Apr 2024 22:42:14 +0800 Subject: [PATCH] Re-implement OSDK debugging fixes and other facilities upon the refactor This commit bring back the features introduced from d28292c to a52e432: - Disable KVM when using GDB; - Update docs about the GDB server address; - Add `config` option for `CargoArgs` in OSDK; - Ensure debug info added when debugging in the release profile. --- .gitignore | 1 + Makefile | 8 +++--- docs/src/kernel/advanced-instructions.md | 4 +++ docs/src/osdk/reference/commands/run.md | 2 +- osdk/src/cli.rs | 7 ++++++ osdk/src/commands/build/mod.rs | 19 ++++++++------ osdk/src/commands/debug.rs | 12 +++++---- osdk/src/commands/run.rs | 27 ++++++++++++++++++-- osdk/src/commands/util.rs | 2 +- osdk/src/config/mod.rs | 21 ++-------------- osdk/src/config/scheme/action.rs | 27 +++++++++++++++++++- osdk/src/config/scheme/qemu.rs | 32 ++---------------------- osdk/src/config/unix_args.rs | 30 ++++++++++++++++++++++ 13 files changed, 121 insertions(+), 71 deletions(-) diff --git a/.gitignore b/.gitignore index f6dd2427f..0ed877a55 100644 --- a/.gitignore +++ b/.gitignore @@ -20,3 +20,4 @@ virtio-net.pcap # vscode launch config file .vscode/launch.json +.vscode/launch.bak diff --git a/Makefile b/Makefile index c078b918a..097f199fc 100644 --- a/Makefile +++ b/Makefile @@ -6,6 +6,7 @@ BOOT_METHOD ?= grub-rescue-iso BOOT_PROTOCOL ?= multiboot2 BUILD_SYSCALL_TEST ?= 0 ENABLE_KVM ?= 1 +GDB_TCP_PORT ?= 1234 INTEL_TDX ?= 0 RELEASE_MODE ?= 0 SCHEME ?= "" @@ -140,13 +141,12 @@ else ifeq ($(AUTO_TEST), boot) @tail --lines 100 qemu.log | grep -q "^Successfully booted." || (echo "Boot test failed" && exit 1) endif -.PHONY: gdb_server -gdb_server: build - @cd kernel && cargo osdk run $(CARGO_OSDK_ARGS) -G --vsc --gdb-server-addr :1234 +gdb_server: initramfs $(CARGO_OSDK) + @cargo osdk run $(CARGO_OSDK_ARGS) -G --vsc --gdb-server-addr :$(GDB_TCP_PORT) .PHONY: gdb_client gdb_client: $(CARGO_OSDK) - @cd kernel && cargo osdk debug $(CARGO_OSDK_ARGS) --remote :1234 + @cd kernel && cargo osdk debug $(CARGO_OSDK_ARGS) --remote :$(GDB_TCP_PORT) .PHONY: test test: diff --git a/docs/src/kernel/advanced-instructions.md b/docs/src/kernel/advanced-instructions.md index 949ccff99..4c0695c04 100644 --- a/docs/src/kernel/advanced-instructions.md +++ b/docs/src/kernel/advanced-instructions.md @@ -74,6 +74,10 @@ Start a GDB-enabled VM of Asterinas with OSDK and wait for debugging connection: make gdb_server ``` +The server will listen at the default address specified in `Makefile`, i.e., a local TCP port `:1234`. +Change the address in `Makefile` for your convenience, +and check `cargo osdk run -h` for more details about the address. + Two options are provided to interact with the debug server. - A GDB client: start a GDB client in another terminal. diff --git a/docs/src/osdk/reference/commands/run.md b/docs/src/osdk/reference/commands/run.md index e7296b9a1..049ac5984 100644 --- a/docs/src/osdk/reference/commands/run.md +++ b/docs/src/osdk/reference/commands/run.md @@ -23,7 +23,7 @@ Options related with debugging: Requires [CodeLLDB](https://marketplace.visualstudio.com/items?itemName=vadimcn.vscode-lldb). - `--gdb-server-addr `: The network address on which the GDB server listens, it can be either a path for the UNIX domain socket or a TCP port on an IP address. -[default: .aster-gdb-socket] +[default: `.aster-gdb-socket`(a local UNIX socket)] See [Debug Command](debug.md) to interact with the GDB server in terminal. diff --git a/osdk/src/cli.rs b/osdk/src/cli.rs index 84cefb982..b457f8129 100644 --- a/osdk/src/cli.rs +++ b/osdk/src/cli.rs @@ -235,6 +235,13 @@ pub struct CargoArgs { pub features: Vec, #[arg(long, help = "Do not activate the `default` features", global = true)] pub no_default_features: bool, + #[arg( + long = "config", + help = "Override a configuration value", + value_name = "KEY=VALUE", + global = true + )] + pub override_configs: Vec, } impl CargoArgs { diff --git a/osdk/src/commands/build/mod.rs b/osdk/src/commands/build/mod.rs index 66a11456e..08e13f88f 100644 --- a/osdk/src/commands/build/mod.rs +++ b/osdk/src/commands/build/mod.rs @@ -12,7 +12,7 @@ use std::{ use bin::strip_elf_for_qemu; -use super::util::{cargo, COMMON_CARGO_ARGS, DEFAULT_TARGET_RELPATH}; +use super::util::{cargo, profile_name_adapter, COMMON_CARGO_ARGS, DEFAULT_TARGET_RELPATH}; use crate::{ arch::Arch, base_crate::new_base_crate, @@ -153,6 +153,7 @@ pub fn do_build( &build.profile, &build.features[..], build.no_default_features, + &build.override_configs[..], &cargo_target_directory, rustflags, ); @@ -186,6 +187,7 @@ fn build_kernel_elf( profile: &str, features: &[String], no_default_features: bool, + override_configs: &[String], cargo_target_directory: impl AsRef, rustflags: &[&str], ) -> AsterBin { @@ -219,6 +221,9 @@ fn build_kernel_elf( .arg(cargo_target_directory.as_ref()); command.args(COMMON_CARGO_ARGS); command.arg("--profile=".to_string() + profile); + for override_config in override_configs { + command.arg("--config").arg(override_config); + } info!("Building kernel ELF using command: {:#?}", command); @@ -228,13 +233,11 @@ fn build_kernel_elf( process::exit(Errno::ExecuteCommand as _); } - let aster_bin_path = cargo_target_directory.as_ref().join(&target_os_string); - let aster_bin_path = if profile == "dev" { - aster_bin_path.join("debug") - } else { - aster_bin_path.join(profile) - } - .join(get_current_crate_info().name); + let aster_bin_path = cargo_target_directory + .as_ref() + .join(&target_os_string) + .join(profile_name_adapter(profile)) + .join(get_current_crate_info().name); AsterBin::new( aster_bin_path, diff --git a/osdk/src/commands/debug.rs b/osdk/src/commands/debug.rs index 6268f952e..6cb207d9b 100644 --- a/osdk/src/commands/debug.rs +++ b/osdk/src/commands/debug.rs @@ -1,16 +1,18 @@ // SPDX-License-Identifier: MPL-2.0 -use crate::commands::util::bin_file_name; - -use crate::{cli::DebugArgs, util::get_target_directory}; +use crate::{ + cli::DebugArgs, + commands::util::{bin_file_name, profile_name_adapter}, + util::get_target_directory, +}; use std::process::Command; -pub fn execute_debug_command(profile: &String, args: &DebugArgs) { +pub fn execute_debug_command(profile: &str, args: &DebugArgs) { let remote = &args.remote; let file_path = get_target_directory() .join("x86_64-unknown-none") - .join(profile) + .join(profile_name_adapter(profile)) .join(bin_file_name()); println!("Debugging {}", file_path.display()); diff --git a/osdk/src/commands/run.rs b/osdk/src/commands/run.rs index fe7f13ed2..6a44f2ca4 100644 --- a/osdk/src/commands/run.rs +++ b/osdk/src/commands/run.rs @@ -3,7 +3,7 @@ use super::{build::create_base_and_cached_build, util::DEFAULT_TARGET_RELPATH}; use crate::{ cli::GdbServerArgs, - config::{scheme::ActionChoice, Config}, + config::{scheme::ActionChoice, unix_args::split_to_kv_array, Config}, util::{get_current_crate_info, get_target_directory}, }; @@ -40,10 +40,33 @@ pub fn execute_run_command(config: &Config, gdb_server_args: &GdbServerArgs) { } }; config.run.qemu.args += &qemu_gdb_args; + + // FIXME: Disable KVM from QEMU args in debug mode. + // Currently, the QEMU GDB server does not work properly with KVM enabled. + let mut splitted = split_to_kv_array(&config.run.qemu.args); + let args_num = splitted.len(); + splitted.retain(|x| !x.contains("kvm")); + if splitted.len() != args_num { + println!( + "[WARNING] KVM is forced to be disabled in GDB server currently. \ + Options related with KVM are ignored." + ); + } + + config.run.qemu.args = splitted.join(" "); + + // Ensure debug info added when debugging in the release profile. + if config.run.build.profile == "release" { + config + .run + .build + .override_configs + .push("profile.release.debug=true".to_owned()); + } } let _vsc_launch_file = gdb_server_args.vsc_launch_file.then(|| { vsc::check_gdb_config(gdb_server_args); - let profile = super::util::profile_adapter(&config.build.profile); + let profile = super::util::profile_name_adapter(&config.build.profile); vsc::VscLaunchConfig::new(profile, &gdb_server_args.gdb_server_addr) }); diff --git a/osdk/src/commands/util.rs b/osdk/src/commands/util.rs index d6f2647ff..036998be0 100644 --- a/osdk/src/commands/util.rs +++ b/osdk/src/commands/util.rs @@ -15,7 +15,7 @@ pub fn cargo() -> Command { Command::new("cargo") } -pub fn profile_adapter(profile: &str) -> &str { +pub fn profile_name_adapter(profile: &str) -> &str { match profile { "dev" => "debug", _ => profile, diff --git a/osdk/src/config/mod.rs b/osdk/src/config/mod.rs index 8eb21d053..e37c9c2f3 100644 --- a/osdk/src/config/mod.rs +++ b/osdk/src/config/mod.rs @@ -14,9 +14,7 @@ pub mod unix_args; #[cfg(test)] mod test; -use scheme::{ - Action, ActionScheme, BootScheme, Build, BuildScheme, GrubScheme, QemuScheme, Scheme, -}; +use scheme::{Action, ActionScheme, BootScheme, Build, GrubScheme, QemuScheme, Scheme}; use crate::{ arch::{get_default_arch, Arch}, @@ -34,22 +32,6 @@ pub struct Config { } fn apply_args_before_finalize(action_scheme: &mut ActionScheme, args: &CommonArgs) { - if action_scheme.build.is_none() { - action_scheme.build = Some(BuildScheme::default()); - } - if let Some(ref mut build) = action_scheme.build { - if let Some(profile) = &args.build_args.profile() { - build.profile = Some(profile.clone()); - } - build.features.extend(args.build_args.features.clone()); - if args.build_args.no_default_features { - build.no_default_features = true; - } - if args.linux_x86_legacy_boot { - build.linux_x86_legacy_boot = true; - } - } - if action_scheme.grub.is_none() { action_scheme.grub = Some(GrubScheme::default()); } @@ -91,6 +73,7 @@ fn apply_args_before_finalize(action_scheme: &mut ActionScheme, args: &CommonArg } fn apply_args_after_finalize(action: &mut Action, args: &CommonArgs) { + action.build.apply_common_args(args); action.qemu.apply_qemu_args(&args.qemu_args); if args.display_grub_menu { action.grub.display_grub_menu = true; diff --git a/osdk/src/config/scheme/action.rs b/osdk/src/config/scheme/action.rs index ee97dcc93..63a5ff70b 100644 --- a/osdk/src/config/scheme/action.rs +++ b/osdk/src/config/scheme/action.rs @@ -2,7 +2,10 @@ use super::{inherit_optional, Boot, BootScheme, Grub, GrubScheme, Qemu, QemuScheme}; -use crate::config::{scheme::Vars, Arch}; +use crate::{ + cli::CommonArgs, + config::{scheme::Vars, Arch}, +}; #[derive(Debug, Copy, Clone, PartialEq, Eq, Serialize, Deserialize)] pub enum ActionChoice { @@ -28,6 +31,8 @@ pub struct Build { pub features: Vec, #[serde(default)] pub no_default_features: bool, + // The cargo `--config` values. + pub override_configs: Vec, #[serde(default)] pub linux_x86_legacy_boot: bool, } @@ -38,11 +43,30 @@ impl Default for Build { profile: "dev".to_string(), features: Vec::new(), no_default_features: false, + override_configs: Vec::new(), linux_x86_legacy_boot: false, } } } +impl Build { + pub fn apply_common_args(&mut self, common_args: &CommonArgs) { + let build_args = &common_args.build_args; + if let Some(profile) = build_args.profile() { + self.profile = profile.clone(); + } + self.features.extend(build_args.features.clone()); + self.override_configs + .extend(build_args.override_configs.clone()); + if common_args.build_args.no_default_features { + self.no_default_features = true; + } + if common_args.linux_x86_legacy_boot { + self.linux_x86_legacy_boot = true; + } + } +} + impl BuildScheme { pub fn inherit(&mut self, parent: &Self) { if parent.profile.is_some() { @@ -64,6 +88,7 @@ impl BuildScheme { profile: self.profile.unwrap_or_else(|| "dev".to_string()), features: self.features, no_default_features: self.no_default_features, + override_configs: Vec::new(), linux_x86_legacy_boot: self.linux_x86_legacy_boot, } } diff --git a/osdk/src/config/scheme/qemu.rs b/osdk/src/config/scheme/qemu.rs index c86632cf6..3ae20e156 100644 --- a/osdk/src/config/scheme/qemu.rs +++ b/osdk/src/config/scheme/qemu.rs @@ -8,7 +8,7 @@ use crate::{ arch::{get_default_arch, Arch}, config::{ eval::{eval, Vars}, - unix_args::{apply_kv_array, get_key}, + unix_args::{apply_kv_array, get_key, split_to_kv_array}, }, error::Errno, error_msg, @@ -40,40 +40,12 @@ impl Default for Qemu { impl Qemu { pub fn apply_qemu_args(&mut self, args: &Vec) { - let target = match shlex::split(&self.args) { - Some(v) => v, - None => { - error_msg!("Failed to parse qemu args: {:#?}", &self.args); - process::exit(Errno::ParseMetadata as _); - } - }; - - // Join the key value arguments as a single element - let mut joined = Vec::new(); - let mut consumed = false; - for (first, second) in target.iter().zip(target.iter().skip(1)) { - if consumed { - consumed = false; - continue; - } - if first.starts_with('-') && !first.starts_with("--") && !second.starts_with('-') { - joined.push(format!("{} {}", first, second)); - consumed = true; - } else { - joined.push(first.clone()); - } - } - if !consumed { - joined.push(target.last().unwrap().clone()); - } + let mut joined = split_to_kv_array(&self.args); // Check the soundness of qemu arguments for arg in joined.iter() { check_qemu_arg(arg); } - for arg in joined.iter() { - check_qemu_arg(arg); - } apply_kv_array(&mut joined, args, " ", MULTI_VALUE_KEYS); diff --git a/osdk/src/config/unix_args.rs b/osdk/src/config/unix_args.rs index 5d8eef479..38b65bafb 100644 --- a/osdk/src/config/unix_args.rs +++ b/osdk/src/config/unix_args.rs @@ -8,6 +8,36 @@ use indexmap::{IndexMap, IndexSet}; use crate::{error::Errno, error_msg}; +/// Split a string of Unix arguments into an array of key-value strings or switches. +/// Positional arguments are not supported. +pub fn split_to_kv_array(args: &str) -> Vec { + let target = match shlex::split(args) { + Some(v) => v, + None => { + error_msg!("Failed to parse unix args: {:#?}", args); + process::exit(Errno::ParseMetadata as _); + } + }; + + // Join the key value arguments as a single element + let mut joined = Vec::::new(); + let mut last_has_value = false; + for elem in target { + if !elem.starts_with('-') && !last_has_value { + if let Some(last) = joined.last_mut() { + last.push(' '); + last.push_str(&elem); + last_has_value = true; + continue; + } + } + joined.push(elem); + last_has_value = false; + } + + joined +} + /// Apply key-value pairs to an array of strings. /// /// The provided arguments will be appended to the array if the key is not already present or if the key is a multi-value key.