On Samstag 08 November 2008, Dylan Smith wrote:
(Ensured compiler independence by modifying byte code templates)
The initial tests added simply call CreateTextServices, and query for the ITextServices interface, then cleans up.
Since CreateTextServices needs an implementation of ITextHost to be given, this patch needed a stub implementation. This required wrappers for converting thiscall to stdcall which are now implementing by adapting a byte code wrapper template. Although byte code isn't portable across processors, this is only needed for x86 platforms so it is only meant to be compiler independent.
In order to investigate what is happening in the tests, it is helpful to trace the calls to ITextHost methods by the richedit controls. I realized it would be too verbose to trace all these calls by default, so I wrapped them all with a check for winedebug > 1, since normally winedebug is 0 or 1.
dlls/riched20/tests/Makefile.in | 3 +- dlls/riched20/tests/txtsrv.c | 593 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 595 insertions(+), 1 deletions(-) create mode 100644 dlls/riched20/tests/txtsrv.c
Hi,
This page [1] seems to suggest memory returned from HeapAlloc is marked as not executable. Other tests (ntdll exception) are using VirtualAlloc() for dynamically created code.
patch 1
+/* The following x86 code string converts between the thiscall and stdcall
- calling convetions. The thiscall calling convention places the This
- pointer in ecx on the x86 platform, and the stdcall calling convention
- pushes the This pointer on the stack as the first argument.
- The wrapper's code finishes by jumping to the real function.
- Byte codes are used so that a copy of it can be modified to use
- for each method in ITextHost. */
patch 2
-/* The following x86 code string converts between the thiscall and stdcall +/* The following x86 code strings convert between the thiscall and stdcall
- calling convetions. The thiscall calling convention places the This
Here you introduce comments that you seem to change in the second patch. Maybe use the right text in the first patch? Additionally the term code string sounds odd. Maybe replace that with "The following x86 assembler code..." or something similar?
#define ITextServices_QueryInterface(p,a,b) (p)->lpVtbl->QueryInterface(p,a,b) #define ITextServices_AddRef(p) (p)->lpVtbl->AddRef(p) #define ITextServices_Release(p) (p)->lpVtbl->Release(p) -/*** ITextServices methods ***/ -#define ITextServices_TxSendMessage(p,a,b,c,d) (p)->lpVtbl->TxSendMessage(p,a,b,c,d) -#define ITextServices_TxDraw(p,a,b,c,d,e,f,g,h,i,j,k,l) (p)->lpVtbl->TxDraw(p,a,b,c,d,e,f,g,h,i,j,k,l)
Why do you only move some of those #defines part of ITextServices and not the one part of IUnknown ? I do not know much about those #defines but this seems odd
Maybe i am missing something but why do you need those thiscall wrappers? Wouldn't it be possible to use the DEFINE_THISCALL_WRAPPER and THISCALL macros like in txtserv.c?
Greetings Peter
On Sat, Nov 8, 2008 at 10:40 AM, Peter Oberndorfer kumbayo84@arcor.dewrote:
This page [1] seems to suggest memory returned from HeapAlloc is marked as not executable. Other tests (ntdll exception) are using VirtualAlloc() for dynamically created code.
patch 1
+/* The following x86 code string converts between the thiscall and
stdcall
- calling convetions. The thiscall calling convention places the This
- pointer in ecx on the x86 platform, and the stdcall calling
convention
- pushes the This pointer on the stack as the first argument.
- The wrapper's code finishes by jumping to the real function.
- Byte codes are used so that a copy of it can be modified to use
- for each method in ITextHost. */
Thanks, I forgot to address that issue because I was too busy trying to get it to run correctly. I used HeapAlloc initially because I wasn't familiar with VirtualAlloc, so thanks for letting me know what to use.
patch 2
-/* The following x86 code string converts between the thiscall and
stdcall
+/* The following x86 code strings convert between the thiscall and
stdcall
- calling convetions. The thiscall calling convention places the This
Here you introduce comments that you seem to change in the second patch. Maybe use the right text in the first patch? Additionally the term code string sounds odd. Maybe replace that with "The following x86 assembler code..." or something similar?
All I changed in the comments between the patches was singular to plural word changes. Using the same comments for both patches would make the comment confusing if only one of the patches were accepted.
I could change code string, but probably to "The following x86 byte code...", since it is just the comments that are assembly.
#define ITextServices_QueryInterface(p,a,b)
(p)->lpVtbl->QueryInterface(p,a,b)
#define ITextServices_AddRef(p) (p)->lpVtbl->AddRef(p) #define ITextServices_Release(p) (p)->lpVtbl->Release(p) -/*** ITextServices methods ***/ -#define ITextServices_TxSendMessage(p,a,b,c,d)
(p)->lpVtbl->TxSendMessage(p,a,b,c,d)
-#define ITextServices_TxDraw(p,a,b,c,d,e,f,g,h,i,j,k,l)
(p)->lpVtbl->TxDraw(p,a,b,c,d,e,f,g,h,i,j,k,l)
Why do you only move some of those #defines part of ITextServices and not the one part of IUnknown ? I do not know much about those #defines but this seems odd
The QueryInterface, AddRef, and Release methods were actually defined using the stdcall calling convention, so it doesn't have the same problems as the other methods which were defined using the C++ thiscall calling convention.
Maybe i am missing something but why do you need those thiscall wrappers? Wouldn't it be possible to use the DEFINE_THISCALL_WRAPPER and THISCALL macros like in txtserv.c?
I can't use DEFINE_THISCALL_WRAPPER because it uses __ASM_GLOBAL_FUNC, __ASM_FUNC, and __ASM_NAME, which are defined in include/wine/port.h and include/config.h. These cannot be included in the tests since the tests must be compiled and crosscompiled with the same configuration (and include/config.h is generated by the configure script). Previously I tried to do it that way, but it wasn't commited I tried to define these macros in the test without making it portable enough to other compilers.
Greetings Peter
Thanks for all the helpful comments Peter!
-
Dylan Smith -
Peter Oberndorfer