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

Errors in plugin parameter strings - which are suppressed during autoloading - can lead to an Access Violation #422

Open
dartheditous opened this issue Jan 17, 2025 · 6 comments

Comments

@dartheditous
Copy link

dartheditous commented Jan 17, 2025

I'm not entirely sure what's going on with this one but I was having trouble working out why I was getting an Access Violation when trying to debug a new plugin.

It turned out to be due to calling AddFunction with a malformed parameter string, e.g.

env->AddFunction("_", "[fade_offset]f[fade_length]f[fade_offset_type]", Create_params, 0);

which is missing the final type specifier for the parameter fade_offset_type.

AddFunction calls IsValidParameterString to do a quick check of the string (which could be simplified and improved by replacing it with a regex, but that's by-the-by) and throws an exception if it doesn't pass - but during autoloading of plugins, the exception is caught and suppressed.

This is to avoid throwing exceptions on non-plugin DLLs which may be residing in plugin folders. This is a good reason, but it does lead to the above issue of plugins silently not loading properly and resulting in crashes. What I think might be happening is that any previous AddFunction calls in the faulty plugin's AvisynthPluginInit3 are still completed, so the functions exist in the AviSynth environment, but since plugin loading is then aborted by the suppressed exception, this then leads to an Access Violation when you try to call one of the previous added functions.

If I try to call the faulty function, I get the "There is no function named..." error. But if I try to call one of the plugin's other functions which came before the faulty one in AvisynthPluginInit3, I get the Access Violation.

Could the process be reworked to identify and ignore non-plugin DLLs in a more targeted way, and to not suppress errors from faulty plugin DLLs?

@Jamaika1
Copy link

Jamaika1 commented Jan 18, 2025

I encountered other difficulties. The last parameter was missing e.g. Jinc36Resize. The plugin compiled but didn't work.
Asd-g/AviSynth-JincResize#14
There was no possibility to add [param][[ops]i...]

@pinterf
Copy link

pinterf commented Jan 19, 2025

So far the only fatal error which immediately breaks everything is mixing 32-64 bit plugins. Perhaps I could make your case to act similarly. And the second issue is that the already successfully loaded functions from the plugin must be removed properly.
Regarding the parameter error handling, I encountered a problem, once I played with adding 64 bit long and double types to 64 bit versions, maybe I return to this idea, but I wouldn't like such functions with l and d in their parameter list to break the loading process, the nice way would be simply to ignore the functions with these future types. Obviously 32 bit versions must ignore such functions all, without throwing the all plugin away. Just thinking...

@Jamaika1
Copy link

So far the only fatal error which immediately breaks everything is mixing 32-64 bit plugins.

Maybe. Plugins whose time has passed and no one knows how to run them in GCC C++17 with ffmpeg 7.2.0 (2025).
https://github.com/Klimax/FFT3DFilter
https://github.com/pinterf/FFT3DFilter
https://github.com/HomeOfAviSynthPlusEvolution/neo_FFT3D

@pinterf
Copy link

pinterf commented Jan 19, 2025

Not really related to the issue, nevertheless they are external plugins with zillions of script examples, documentation and specific build needs and not part of ffmpeg.

@dartheditous
Copy link
Author

Although personally I think incompatible/malformed plugins should throw visible exceptions on autoloading, perhaps autoloading could just flag such a plugin as incompatible and fully load it anyway? Errors will be thrown during TypeMatch (e.g. if it uses l or d, or is malformed) if you try to call that particular function anyway.

@pinterf
Copy link

pinterf commented Jan 19, 2025

I think you are right, this type of fatal error (error in parameter string) must be treated the same way like 32/64 bit mismatch: resulting in visible exitus. Moreover, this only can occur during development; normal users won't experience it in live. All I have to solve is to be able loading a plugin despite the optional "l" and "d" parameter strings (looking in the future), omitting such functions with this "unknown" signature, but able to proceed with the others.

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

No branches or pull requests

3 participants