No doubt the new code is a huge advantage over the old code. Still there are some issues in my opinion.
Currently the trap handlers are generated inside C code by a macro called KiTrap(), located in trap_x.h. This macro creates a C function, that uses KiTrapStub inline function to create the actual entry stub. KiTrapStub is 90% pure gcc inline assembly. At the end it does a jmp to the real C handler.
Most C handlers will start processing with an inline function KiEnterTrap, which is C code with inline assembly functions like KiSetSaneSegments, Ke386GetFs. KiTrap06Handler/Trap0DHandler are exceptions, they use KiIsV8086TrapSafe to determine in inline assembly if we have a v86 trap. In this case KiEnterV86Trap is used instead of KiEnterTrap. KiTrap02 is also an exception, it's a pure C handler
Now what's wrong?
1.) The gcc inline assembly is very compiler specific and MSVC unfriendly. The old method using GAS specific asm files was at least linkable with MSVC code. The new code needs porting.
2.) The current C code makes assumptions about what the compiler will do, ignoring the ABI, which requires that you do not change ds/es in C code, not even temporarily and that the direction flag is clear on function entry. The compiler might at any place use ds/es for whatever it likes. It must be a valid flat segment. Only using stack variables might work on the current gcc, but might fail on other compilers. A good example would be MSVC's use of __security_cookie, which is referenced on function entry through ds. I remember that not having cleared the direction flag in a trap handler caused strange random bugs in the past, we should avoid making this mistake again. KiTrapStub even changes esp inside C code (in cases where the function is entered without valid esp)
Suggested modifications:
Implement the trap entry functions in a pure assembly file, using assembly macros which would combine KiTrapStub and the segment handling part of KiEnterTrap. Trap exit could possibly be done this way as well. On the amd64 branch the C handlers return a status value back to the assembly stub, but a jump or call could be done as well. It would be possible to use ML for these assembly files as well. This works fine in the amd64 branch. Also strictly stick to C ABI instead of using compiler extensions and making assumptions about how the compiler works. It would improve portability, make the code safe and also more readable. And we would not really add more assembly, just move the current inline assembly to a pure assembly file. Possible performance disadvantages: I don't see any. We might end up with a jmp or call instruction more, but we can possibly safe this somewhere else.
And maybe finally someone deletes the disgusting Ke386SetDs()... ;-)
Regards, Timo
MSVC supports Intel syntax inline assembly, doesn't it? Why not use GAS with Intel syntax mode, so that inline assembly would be compatible with both?
On Sat, Feb 6, 2010 at 9:41 AM, Timo Kreuzer timo.kreuzer@web.de wrote:
No doubt the new code is a huge advantage over the old code. Still there are some issues in my opinion.
Currently the trap handlers are generated inside C code by a macro called KiTrap(), located in trap_x.h. This macro creates a C function, that uses KiTrapStub inline function to create the actual entry stub. KiTrapStub is 90% pure gcc inline assembly. At the end it does a jmp to the real C handler.
Most C handlers will start processing with an inline function KiEnterTrap, which is C code with inline assembly functions like KiSetSaneSegments, Ke386GetFs. KiTrap06Handler/Trap0DHandler are exceptions, they use KiIsV8086TrapSafe to determine in inline assembly if we have a v86 trap. In this case KiEnterV86Trap is used instead of KiEnterTrap. KiTrap02 is also an exception, it's a pure C handler
Now what's wrong?
1.) The gcc inline assembly is very compiler specific and MSVC unfriendly. The old method using GAS specific asm files was at least linkable with MSVC code. The new code needs porting.
2.) The current C code makes assumptions about what the compiler will do, ignoring the ABI, which requires that you do not change ds/es in C code, not even temporarily and that the direction flag is clear on function entry. The compiler might at any place use ds/es for whatever it likes. It must be a valid flat segment. Only using stack variables might work on the current gcc, but might fail on other compilers. A good example would be MSVC's use of __security_cookie, which is referenced on function entry through ds. I remember that not having cleared the direction flag in a trap handler caused strange random bugs in the past, we should avoid making this mistake again. KiTrapStub even changes esp inside C code (in cases where the function is entered without valid esp)
Suggested modifications:
Implement the trap entry functions in a pure assembly file, using assembly macros which would combine KiTrapStub and the segment handling part of KiEnterTrap. Trap exit could possibly be done this way as well. On the amd64 branch the C handlers return a status value back to the assembly stub, but a jump or call could be done as well. It would be possible to use ML for these assembly files as well. This works fine in the amd64 branch. Also strictly stick to C ABI instead of using compiler extensions and making assumptions about how the compiler works. It would improve portability, make the code safe and also more readable. And we would not really add more assembly, just move the current inline assembly to a pure assembly file. Possible performance disadvantages: I don't see any. We might end up with a jmp or call instruction more, but we can possibly safe this somewhere else.
And maybe finally someone deletes the disgusting Ke386SetDs()... ;-)
Regards, Timo
Ros-dev mailing list Ros-dev@reactos.org http://www.reactos.org/mailman/listinfo/ros-dev
-
Sir Gallantmon -
Timo Kreuzer