Skip to content

If backends fail it is possible to try using a null pointer #3313

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

peardox
Copy link
Contributor

@peardox peardox commented Jul 9, 2025

This is, hopefully, slighly cleaner than a core dump

Copy link
Member

@danbev danbev left a comment

Choose a reason for hiding this comment

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

I'm not sure if we should add the nullptr checks as mentioned in the comments but I don't feel strongly about it. But the other changes look good to me.

src/whisper.cpp Outdated
buft_list.emplace_back(cpu_dev, *extra_bufts);
++extra_bufts;
// Prevent null pointer being used in following code
if(cpu_dev != nullptr) {
Copy link
Member

Choose a reason for hiding this comment

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

Nit: Can you add a space after the if keyword.

Copy link
Contributor Author

@peardox peardox Jul 21, 2025

Choose a reason for hiding this comment

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

Added space after if *4 to fix nit

The most easiest way to trigger the state this avoids is by commenting out the ggml_backend_load_all() in whisper-bench and build shared all_cpu_variants. OK that error would be pretty stupid but at least with this it'll tell you there are no backends. The alternative is a core-dump which tells you less than zero

I know most perople will build static for the build-box but if you want to pass something around as a release shared all_cpu_variants is a must

Also if you pass a static release around you run into the issue of it not loading the right CPU on older kit (probably same result - not tested - yet, only just though of it)

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