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

Avoid core dump from some bad pointer math. #1522

Merged
merged 1 commit into from
May 7, 2024

Conversation

linas
Copy link
Member

@linas linas commented May 7, 2024

This is a temporary patch, till I figure out something better.

This is a temporary patch, till I figure out something better.
@linas linas merged commit ffb21ec into opencog:master May 7, 2024
1 check passed
@linas linas deleted the null-pointer-crash branch May 7, 2024 16:08
@ampli
Copy link
Member

ampli commented May 8, 2024

Instead, why not:

if (0 != dict->num_categories)
{
   prt_error("Warning: Dictionary %s has no categories\n";
   return NULL;
}

(Not tested.)

@linas
Copy link
Member Author

linas commented May 9, 2024

I tried that. Returning null here causes a crash later on, when no expressions are found. I figured that the earlier the catch, the better.

@ampli
Copy link
Member

ampli commented May 9, 2024

I proposed a warning because I try to keep assert() for logic failures (as opposed to bad external data). Otherwise (in the general case), a program may crash when it could just continue to perform fine.

In this specific case, I could think about two such scenarios (to demonstrate the idea of not crashing on bad data):

  1. A thread with a bad dictionary (maybe created on the fly) is added, crashing a long-running program with other good threads.
  2. A hypothetical interactive program in which the user creates dictionaries and accidentally creates a dictionary with no categories. Crashing may abort other interactive work that the user has done.

However, if the program encounters bad logic, it means there is a bug that may, e.g., clobber memory and there may be no point in continuing. BTW, even in such a case, there may be a benefit to not immediately aborting, e.g., if an editor program uses the library and may want to save data before aborting. To that end, I created lg_library_failure_hook (not tested).

@linas
Copy link
Member Author

linas commented May 9, 2024

This was meant to be temporary. The failure_hook thing seems too complicated; I would not want to use it, at least, not right now.

The easiest solution would be to just look at the number of categories in the dict, when the dict is opened, and print "can't use this for generation", and then immediately return zero results. (and then the assert can be removed) I was in too much of a hurry to figure out where that would need to be done.

The assert leaves me with an error message, which is still better than having to run gdb to find out what happened.

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.

2 participants