search

adventuregamestudio / ags

AGS editor and engine source code
Other
675 stars 159 forks source link

AGS 4: make plugin API 64-bit safe #590

Open ghost opened 5 years ago

ghost commented 5 years ago

There's a number of issues related to the fact that plugin API is working with 32-bit ints and passes pointers converted to 32-bit ints as well under certain circumstances. Since we break compatibilities in ags4 this is an opportunity to fix these.

[SOLVED] ~1. IAGSEngine::FRead and FWrite accept 32-bit "handles" and convert them to FILE* pointer. Not sure why these functions were needed there in the first place and what was their practical use case. I'd say AGS needs a proper stream API instead, but that's probably a separate problem.~

  1. get_translation() calls pl_run_plugin_hooks() which returns 32-bit int and in turn converted to char pointer (should be const char actually) in attempt to use as a translated string.

  2. IAGSEngine::CallGameScriptFunction and IAGSEngine::QueueGameScriptFunction are passing some arguments into the interpreter as 32-bit integers, but in theory these could be pointers too. You may see how these are dealt with by searching for kScValPluginArg type. Unfortunately I do not remember when this was the case in practice (but it definitely was once). Maybe searching for commit descriptions when kScValPluginArg type was added will shed some light. That may hint possible solution too.

Note that one of the possible ways of solving the latter may be to redesign the "ScriptValue" struct. Right now it holds a real pointer Ptr and 32-bit integer IValue (coupled with FValue in a union). This IValue may be interpreted as an offset to Ptr. I remember trying to make IValue 64-bit instead very long ago, but that was causing some value conversion issues so I decided to keep it 32-bit. Perhaps there are ways to improve this.

ghost commented 4 years ago

Update: IAGSEngine::FRead/FWrite problem was solved by changing the meaning of argument from FILE* pointer to abstract handle (id). Two other issues remain though.

ericoporto commented 1 year ago

While the mentioned functions exists in the plugin API, is anyone familiar with any plugin using these? I have a feeling these are safe for modifications in the API.

Maybe searching for commit descriptions when kScValPluginArg type was added will shed some light. That may hint possible solution too.

It was added (in 2013!) in the commit https://github.com/adventuregamestudio/ags/commit/90310e5fb0f7822a53d792ffea10a155b3bb76da