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

qp-trie's count is incorrect and causes subtraction overflow in debug mode #661

Closed
kevinaboos opened this issue Oct 12, 2022 · 2 comments · Fixed by #841
Closed

qp-trie's count is incorrect and causes subtraction overflow in debug mode #661

kevinaboos opened this issue Oct 12, 2022 · 2 comments · Fixed by #841
Assignees
Labels
bug upstream a problem with an upstream repo (dependency of Theseus)

Comments

@kevinaboos
Copy link
Member

This weird error is revealed when trying to run any application on the shell in a debug build of Theseus.

I changed the impl Drop for AppCrateRef drop handler to print a bit more info:

impl Drop for AppCrateRef {
    fn drop(&mut self) {
        // trace!("### Dropping AppCrateRef {:?} from namespace {:?}", self.crate_ref, self.namespace.name());
        let crate_locked = self.crate_ref.lock_as_ref();
        // First, remove the actual crate from the namespace.
        if let Some(_removed_app_crate) = self.namespace.crate_tree().lock().remove(&crate_locked.crate_name) {
            // Second, remove all of the crate's global symbols from the namespace's symbol map.
            let mut symbol_map = self.namespace.symbol_map().lock();
            for sec_to_remove in crate_locked.global_sections_iter() {
                trace!("Removing sec_to_remove {:?}", sec_to_remove.deref());
                trace!("symbol_map count: {}\n{:?}", symbol_map.count(), symbol_map.deref());
                trace!("symbol_map had weak sec: {:?}", symbol_map.get(&sec_to_remove.name));
                trace!("symbol_map had strong sec: {:?}", symbol_map.get(&sec_to_remove.name).and_then(|w| w.upgrade()));

                match symbol_map.remove(&sec_to_remove.name) {
                    Some(_removed) => {
                        trace!("Removed symbol {}: {:?}", sec_to_remove.name, _removed.upgrade());
                    }
                    None => {
                        error!("NOTE: couldn't find old symbol {:?} in the old crate {:?} to remove from namespace {:?}.", sec_to_remove.name, crate_locked.crate_name, self.namespace.name());
                    }
                }
            }
        } else {
            error!("BUG: the dropped AppCrateRef {:?} could not be removed from namespace {:?}", self.crate_ref, self.namespace.name());
        }
    }
}

and the log shows some strange behavior surrounding Trie::count(): the count is 0 but there are two elements in the map (?)...

[D] kernel/mod_mgmt/src/lib.rs:816: load_crate_as_application(): trying to load application crate at "/namespaces/_applications/hello-2bf26f3c151abb0a.o"
[I] kernel/mod_mgmt/src/lib.rs:824: loaded new application crate: "hello-2bf26f3c151abb0a", num sections: 111, added 1 new symbols
[D] kernel/runqueue_round_robin/src/lib.rs:204: Adding task to runqueue_round_robin 1, TaskRef(Task { name: "hello-2bf26f3c151abb0a", id: 15, running_on: None, runstate: Blocked, pinned: None })
[D] kernel/spawn/src/lib.rs:610: task_wrapper [1]: "hello-2bf26f3c151abb0a" about to call task entry func 0x72c7790 {fn(alloc::vec::Vec<alloc::string::String>) -> isize} with arg []
[D] kernel/spawn/src/lib.rs:687: task_cleanup_success: "hello-2bf26f3c151abb0a" successfully exited with return value 0
[D] kernel/runqueue_round_robin/src/lib.rs:225: Removing task from runqueue_round_robin 1, TaskRef(Task { name: "hello-2bf26f3c151abb0a", id: 15, running_on: Some(1), runstate: Exited, pinned: None })
[I] applications/shell/src/lib.rs:1056: terminal: task [15] returned exit value: Some(0)
[T] kernel/task/src/lib.rs:970: [CPU 2] Task::drop(): hello-2bf26f3c151abb0a{15}
[T] kernel/mod_mgmt/src/lib.rs:429: Removing sec_to_remove LoadedSection { name: "hello::main::h8bbf4df10c7ebacc", typ: Text, parent: "<locked>", vaddr: v0x72C7790, size: 211, .. }
[T] kernel/mod_mgmt/src/lib.rs:430: symbol_map count: 0
{"shell::main::h0a3c693b3c290b52": (Weak), "hello::main::h8bbf4df10c7ebacc": (Weak)}
[T] kernel/mod_mgmt/src/lib.rs:431: symbol_map had weak sec: Some((Weak))
[T] kernel/mod_mgmt/src/lib.rs:432: symbol_map had strong sec: Some(LoadedSection { name: "hello::main::h8bbf4df10c7ebacc", typ: Text, parent: "<locked>", vaddr: v0x72C7790, size: 211, .. })
[T] kernel/panic_wrapper/src/lib.rs:32: at top of panic_wrapper: PanicInfo { payload: Any { .. }, message: Some(attempt to subtract with overflow), location: Location { file: "/home/kevin/.cargo/registry/src/github.com-1ecc6299db9ec823/qp-trie-0.8.0/src/trie.rs", line: 323, col: 13 }, can_unwind: true }

I'm not sure if this is a bug in qp-trie or an issue with how our new StrRef type implements Borrow<[u8]>, so this issue is a placeholder/TODO item until I can spend more time debugging it.

@kevinaboos
Copy link
Member Author

Upon further examination of the qp-trie code, it seems like inserting or removing key-value pairs via the entry API does not update the Trie's inner count at all. I will verify this with a few test programs and then likely submit an issue on that repo.

@kevinaboos kevinaboos added upstream a problem with an upstream repo (dependency of Theseus) bug and removed bug minor labels Oct 12, 2022
@kevinaboos
Copy link
Member Author

I filed sdleffler/qp-trie-rs#31 to address this.

kevinaboos added a commit that referenced this issue Feb 18, 2023
github-actions bot pushed a commit that referenced this issue Feb 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug upstream a problem with an upstream repo (dependency of Theseus)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant