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

add check for nulled list #1528

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from
Open

add check for nulled list #1528

wants to merge 1 commit into from

Conversation

lacraig2
Copy link
Member

@lacraig2 lacraig2 commented Sep 2, 2024

This PR fixes #1527 by adding checks to see if the list is empty on each iteration.

FIXES: #1527

@@ -18,7 +18,9 @@ PANDAENDCOMMENT */
void HELPER(panda_insn_exec)(target_ulong pc) {
// PANDA instrumentation: before basic block
panda_cb_list *plist;
for(plist = panda_cbs[PANDA_CB_INSN_EXEC]; plist != NULL; plist = panda_cb_list_next(plist)) {
for(plist = panda_cbs[PANDA_CB_INSN_EXEC];
(plist != NULL && panda_cbs[PANDA_CB_INSN_EXEC] != NULL) ;
Copy link
Member

Choose a reason for hiding this comment

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

From my reading of this, it seems like some of these changes might not be meaningful (though I'm sure there's something in this PR that's fixing the underlying bug)

Since plist is set to panda_cbs[PANDA_CB_INSN_EXEC] I don't see how plist could ever be NULL while the panda_cbs[PANDA_CB_INSN_EXEC] object is non-NULL - since they're the same.

Though, if there are multiple threads causing race conditions, then that could mean timing is changed by the redundant check (if it's not optimized out) and perhaps improve things - though at that point I'd think the solution would be to do better thread safety instead of duplicating checks.

Copy link
Member

Choose a reason for hiding this comment

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

Hang on - staring at this closer and seeing how they could differ - after the initial iteration of the loop the panda_cb_next updates plist and perhaps after the first iteration the entire list could become NULL?

Seems like an easier fix might be to ensure the various callback lists only ever become empty, not null?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SEGFAULT in some plugin deletion contexts
2 participants