Comments (35)
Or perhaps we could have a "strict" mode:
#if defined(PORTABLE_VA_ARGS)
#define VM_Call(VM, ...) VM_Call_Impl(VM, &(int[13]){__VA_ARGS__})
#elif defined(CLANG_ABI)
#define VM_Call(VM, ...) VM_Call_Clang(VM, __VA_ARGS__)
#else
#define VM_Call(VM, ...) VM_Call_Hackish(VM, __VA_ARGS__)
#endif
That way you could remove all the compile-time branching from the individual functions, too.
from ioq3.
Invoking C99 makes ioquake3 and forks impossible to compile with MSVC < 2015.
from ioq3.
I understand, that would be a deal breaker.
Are you still interested in a conditionally-compiled version (like with PORTABLE_VA_ARGS
above), or should we just close this?
from ioq3.
I would be mildly surprised if you needed 2015 for VA_ARGS. In any case, we don't actually even officially support VS so that shouldn't really be a gate, and requiring 2015 is no bad thing anyway since it's the first compiler they've come up with since the mid 90s that is actually quite good.
from ioq3.
@timangus: No, sorry, to be clear here, the crucial feature from C99 is the compound literal:
&(int[13]){__VA_ARGS__}
// ^^^^^^^^^^
The __VA_ARGS__
aren't a big issue.
from ioq3.
Yeah, github seems to have stripped my leading and trailing underscores. Not very helpful.
from ioq3.
TEST_COMMENT "TEST_COMMENT"
from ioq3.
Well that's stupid.
from ioq3.
@timangus: That's markdown, baby :-)
from ioq3.
D'oh.
from ioq3.
By the way, there's a "preview" tab when you write a comment, so you can check before submitting what it will look like. And you can also edit comments if you discover a problem only after submission. :-)
from ioq3.
Yeah, but none of the subsequent comments would make any sense if I edited it now ;). Annnyway, if __VA_ARGS__
works, I don't have a problem with it (even if that means needing VS2015.)
from ioq3.
VS2015 community works? that's fine.
from ioq3.
So... would you be interested in a patch along those lines? The key features would be:
VM_Call
becomes a macro.- The default implementation of the macro would use a C99 compound literal.
We can also keep other code branches around that use the two existing versions of va_arg abuse, both as an alternative for non-C99 compilers and in case someone derives a genuine performance advantage from that.
from ioq3.
Opened the debugger- I observe that every invocations (in Tremulous at least) VM_Call()
results in leaking at least the callers stack frame, but often several frames to the vm
😈
Also, don't forget about VM_DllSyscall
in the fix.
from ioq3.
Another interesting tidbit: Every instance of vmMain
actually declares thirteen int
parameters, and never an ellipsis. So I'm very tempted to also change the definition of entryPoint
, because that's another abuse of va-args at the moment.
from ioq3.
Here is a tentative change: tkoeppe@9719411
This is just a rough skeleton; it has issues:
- Missing fall-back for non-C99 compilers
- Magic numbers 13 and 16 appear, which should really use the macros
MAX_VM{MAIN,SYSCALL}_ARGS
... - ... although those macros are a bit silly now given that we have fixed parameter lists and there's nothing to "max" here.
- There should probably be typedefs for some of the more arcane function types.
from ioq3.
@tkoeppe I've never seen &(int[13]){__VA_ARGS__}
before..
Let me know if I'm reading this correctly, __VA_ARGS__
populates elements of a temporary array which cast to an array of 13 integers. Is &
actually needed?
Assuming this is true have you verified with -O0
and -O3
that optimization doesn't break anything?
from ioq3.
@wtfbbqhax: ish. The compound literal is an lvalue, initialized from the given initializer (which means that array elements that aren't mentioned are zero-intialized. We then take the address of that array. Yes, I do want to take the address of the array rather than decay the array so as to retain a semblance of type safety here; that way we can at least check that the number of array elements is the same everywhere.
from ioq3.
I named MAX_VM{MAIN,SYSCALL}_ARGS
and I don't think this changes the meaning of them. It's not a "max" in the sense that arrays might have less elements, it's that less might actually be used and that more can't be used. It makes it easier and more obvious to increase the max number of syscall arguments compared to magic numbers and non-looped array access. My engine fork uses MAX_VMSYSCALL_ARGS
= 17. Historical reference: f7a2006, c5af65f.
from ioq3.
@zturtleman: OK, sure, my main issue with those macros in the light of this change is that you can't tie the macro value to the function signature easily. That is, I can't generate f(int, int, int)
from a macro #define NUM_ARGS 3
. (At least not without some heavy preprocessor lifting.) So we'd still need to keep the function signatures and forwarding calls in sync manually.
from ioq3.
Yeah, it requires additional code changes to actually get more args to (DLL) vmMain now as well. This isn't really much different.
from ioq3.
I updated the commit, it's now: tkoeppe@614848a
This one includes changes to the three *_syscall.c
DLL implementations. The changes follow the same pattern: the syscall function is replaced by a macro that creates an array as a compound literal, and passes the (decayed) array to the syscall function pointer. That function pointer now actually matches the function signature of the various syscall implementations (e.g. CL_CgameSystemCalls
).
from ioq3.
Can you please make the VM_Call require an actual call number i.e.,
#define VM_Call( VM, CALL, ... ) VM_Call_Impl( VM, CALL, &(int[12]){__VA_ARGS__})
intptr_t QDECL VM_Call_Impl( vm_t *vm, int callnum, int (*arg)[12] )
from ioq3.
This one includes changes to the three *_syscall.c DLL implementations.
Doesn't that break compatibility with existing DLLs?
from ioq3.
This one includes changes to the three *_syscall.c DLL implementations.
Doesn't that break compatibility with existing DLLs?
Hm, only if the name mangling includes the function signature. This isn't the case on Linux, but I don't know about Windows. I doubt it, though, because you don't include the signature in Sys_LoadFunction
, so I think it cannot be part of the mangling on any ABI. Moreover, the function signature changes from a function taking one parameter of one function pointer type to a function with one parameter taking a different function pointer type, so even if the size of the parameters is part of the mangling, it shouldn't change.
from ioq3.
Sorry, I should of been more specific. I meant when calling syscall
in a DLL as it use to be multiple arguments read using va_args in the engine and now it's takes a pointer to an array. So existing DLLs give engine a syscall number value which the engine with this patch thinks is a pointer. Engine will probably SEGFAULT when trying to read syscall number and arguments. Using new DLLs on old engine would likely error about unknown syscall number because it's actually passing a pointer address.
from ioq3.
@wtfbbqhax: Unfortunately no, because you cannot portably have zero variadic arguments in a variadic macro.
from ioq3.
@zturtleman: Yes, you're right. Previously compiled DLLs will pass the arguments as if to an ellipsis, but new client code would treat it as a pointer to the first element of an array, and that would break. So this change would require recompiling DLLs.
from ioq3.
Rocket Arena 3 uses a game DLL with no source release so it can't be recompiled. It seems like a bad idea to change the DLL-VM API in general.
from ioq3.
OK, so we should leave the DLLs alone then.
Are you still interested in the change to VM_Call
?
from ioq3.
(Note that DeepMind Lab uses the C99 compound literal: https://github.com/deepmind/lab/blob/master/engine/code/qcommon/qcommon.h#L369)
from ioq3.
Presumably they're using MSVC 2015/17 rather than mingw/make or extremely old msvc
from ioq3.
No, neither, we're using Clang and GCC only -- I just wanted to show how the proposed change looks in a real fork.
from ioq3.
Any progress on this? QuakeJS -s SAFE_HEAP=1 errors out on this kind of stuff. I don't mind only supporting mods with source/reverse engineering just to recompile if someone can point me towards a good implementation like Daemon or or one of the other off-shoots?
from ioq3.
Related Issues (20)
- OpenGL2: Lightmap as an environment map renders incorrect with r_mergeLightmaps 1 HOT 1
- OpenGL2: r_ext_multisample > 0 breaks r_drawSunRays 1 (without HDR or FB-MSAA) HOT 1
- OpenGL2: Flares in mirrors calculate wrong depth and have wrong visibility
- Failing to open x64 game in a path with special characters HOT 3
- OpenGL2: The game is too bright and auto-exposure doesn't match the original
- OpenGL2: Curved surfaces don't use dynamic level of detail
- OpenGL2: Stencil shadow volumes are broken (cg_shadows 2) HOT 1
- OpenGL2: r_shownormals 1 is not implemented
- [Warning] Area 51 (2005) HOT 1
- rendering improvements feedback
- OpenGL2: r_greyscale doesn't affect everything
- OpenGL2: r_anaglyphMode causes 2D and HUD models to be transparent
- opengl1: Skybox in OpenGL 1.1 displays black lines at edges
- Add OpenGLES2 renderer HOT 1
- Add pk3 file caching to emscripten port
- Make emscripten port default to 85 FPS, non-V-sync HOT 1
- cgame project is not used HOT 3
- Emscripten CD Key valid but still doesn't show levels HOT 4
- Add PBR support
- Unlagged: add option to trust clients
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from ioq3.