native patches for os4

kas1e
2013-05-02
2014-04-29
1 2 3 4 > >> (Page 1 of 4)
  • kas1e

    kas1e - 2013-05-02

    As we share all the info about that subject in all the other topics i assume it better to have one single topic about that problem.

    So, problem is: originally dopus5 use SetFunction() for patch some libraryes functions in workbench.library. SetFunction() is used to patch libs with jump-table. But on os4 there is also "interface vectors", so when we use native os4 programs, we can't use SetFunction because it will only patch jump-table, while all the native os4 bins works over iface vectors and patching expected to be done via SetMethod(). In other words code which do patching via SetFunction() will not crashes by itself, but native os4 binaryes which done to use patched data will not works at all, as they expect patching to be done in iface vectors.

    Ikka says that to convert patches from SetFunction() to SetMethod() only macros need to be changed. Or at least this was his intention when he introduced PATCHED and LIBCALL macros.

    PATCH macro:
    - EmuTrap can be removed.

    PATCHED macros:
    Remove stubs. I.e. just use plain call without 68k register usage.

    LIBCALL macros:
    - Remove EmulateTags() calls but call functions directly.

    Ikka dont think actual patches need changes, but just how they are inserted to the system, their entrance and how they call the original OS function.

    But now there is just pure design/etc question:

    How we can do all of this now ? I mean in wb.c these is 2 places where SetFunction called: in wb.c:L_WB_Install_Patch() and wb.c:L_WB_Remove_Patch().

    SetFunctions() args are:
    oldFunc = SetFunction(library, funcOffset, funcEntry)

    While SetMethod() args are:
    old_func = SetMethod(interface, funcOffset, newFunc);

    I thinking just to do in wb.c something like this:

    For install patch:

    // Patch this function
    #if defined(__amigaos4__)
    wb_data->old_function[patch]=SetMethod(libiface,wb_patches[patch].offset,(APTR)&wb_patches[patch].trap);
    #endif
    
    #if defined(__MORPHOS__)
    wb_data->old_function[patch]=SetFunction(lib,wb_patches[patch].offset,(APTR)&wb_patches[patch].trap);
    #endif
    
    #if define(__amigaos3__)
    wb_data->old_function[patch]=SetFunction(lib,wb_patches[patch].offset,wb_patches[patch].function);
    #endif 
    

    For remove patch:

    ~~~~~
    // Restore old value
    old_patch_val[patch]=

    ifdef amigaos4

    SetMethod(libiface,wb_patches[patch].offset,wb_data->old_function[patch]);

    else

    SetFunction(lib,wb_patches[patch].offset,wb_data->old_function[patch]);

    endif

    
    

    // Reinstall patch

    ifdef amigaos4

    SetMethod(libiface,wb_patches[patch].offset,wb_data->old_function[patch]);

    else

    SetFunction(lib,wb_patches[patch].offset,wb_data->old_function[patch]);

    endif

    ~~~~~~

    As for macroses in wb.h, they looks a bit scare, but by logic all he ifdefs/etc remain the same, we only change in ifdef amigaos4 parts that what itix says.

     
    Last edit: kas1e 2013-05-02
  • kas1e

    kas1e - 2013-05-02

    Seems wb_get_patchbase() in wb.c also not enough, and will need to be replaced/expand it or just make a new function for os4 which will "get" ifaces, and maybe even in end it should be called from code like this:

    // Get library
    if ((lib=wb_get_patchbase(wb_patches[patch].type,(struct LibData *)libbase->ml_UserData)))
    {
    // Patch this function
    #if defined(__amigaos4__)
    wb_data->old_function[patch]=SetMethod(libiface,wb_patches[patch].offset,(APTR)&wb_patches[patch].trap);
    #endif
    

    On:

     // Get library
    if ((lib=wb_get_patchbase(wb_patches[patch].type,(struct LibData *)libbase->ml_UserData))
    #ifdef __amigaos4__
    || libiface = wb_get_ifaces(wb_patches[patch].type,(struct LibData *)libbase->ml_UserData))
    #endif
    )
    {
    // Patch this function
    #if defined(__amigaos4__)
    wb_data->old_function[patch]=SetMethod(libiface,wb_patches[patch].offset,(APTR)&wb_patches[patch].trap);
    #endif
    

    But of course better to have done all in one function and just return and lib and iface at one time from one function (if possible).

     
    Last edit: kas1e 2013-05-02
  • kas1e

    kas1e - 2013-05-02

    @all

    Ok, thats what i have do so far:

    wb.c:

    Search for amigaos4 ifdefs in wb_get_patchbase(), L_WB_Install_Patch() and L_WB_Remove_Patch().

    Through one moment which of course wrong, is adding to the wb_get_patchbase() return and of lib, and of libiface (check at end of function). And in code later we do only:

    if ((lib=wb_get_patchbase(wb_patches[patch].type,(struct LibData *)libbase->ml_UserData)))
    

    So need to sort that. Of course we can create another wb_get_patchbase() which will return only ifaces , but that will be a bit sucky imho and if it possible somehow to just expand current wb_get_patchbase() that will be imho cleaner and better.

    wb.h:

    I not so understand all those macroses at all, but i just remove 68k-related parts. For sure wrong, but to start with.

    Also what to do with that structure:

    typedef struct
    {
        long    offset;
    #if defined(__amigaos4__)
        struct EmuTrap trap;
    #elif defined(__MORPHOS__)
        struct EmulLibEntry trap;
    #else
        APTR    function;
    #endif
        short   type;
    } PatchList;
    

    Because i assume it can make a difference in how offsets will counts or so..

     
    Last edit: kas1e 2013-05-02
  • Ilkka Lehtoranta

    Yes you are on the right track. If you are using SetMethod() then:

    • remove traps
    • remove stubs
    • add LIBCALL in to function declaration
    • fix offset

    68k jump table offset wont work. You must check new offset from interface definition.

     
    Last edit: Ilkka Lehtoranta 2013-05-02
  • kas1e

    kas1e - 2013-05-02

    @Ikka
    What i mean is that can i modify wb.c/wb.h to not create more "os4 only" functions ? I just want to avoid mess when all our code will full of all kind macroses and co, and in end when bugs will come up we will understand nothing..

    remove traps
    remove stubs

    You mean wb.h only ? If so, then maybe just copy + paste in wb.h everything from os3 part to os4 ?

    add LIBCALL to function declaration

    You mean wb.h again ?

    fix offset

    Yep, that was another bit about which i think. That mean that there should be wb_patches[WB_PATCH_COUNT] for os4 (like wb_patches_os4[WB_PATCH_COUNT], or ,what can be even better , make some ifdefs like:

    // Patch functions
    static PatchList
        wb_patches[WB_PATCH_COUNT]={
    #ifdef __amigaos4__
            PATCH(SOME,L_WB_AddAppWindow,WB_PATCH_WORKBENCH),
            PATCH(SOME,L_WB_RemoveAppWindow,WB_PATCH_WORKBENCH),
            PATCH(SOME,L_WB_AddAppIcon,WB_PATCH_WORKBENCH),
                    .....
    #else
            PATCH(-0x30,L_WB_AddAppWindow,WB_PATCH_WORKBENCH),
            PATCH(-0x36,L_WB_RemoveAppWindow,WB_PATCH_WORKBENCH),
            PATCH(-0x3c,L_WB_AddAppIcon,WB_PATCH_WORKBENCH),
                    ....
    #endif
    

    68k jump table offset wont work. You must check new offset from interface definition

    If i remember right, there is done by the same offsets in interfaces too, but need to check.

     
  • Ilkka Lehtoranta

    I dont think offsets are same in interface definitions because they include vararg calls and some calls were removed.

    PATCH macro and its usage must be changed:

    Now:

    PATCH(-0x30,L_WB_AddAppWindow,WB_PATCH_WORKBENCH),
    

    With OS4 interface offsets:

    PATCH(-0x30, <interface offset here>, L_WB_AddAppWindow,WB_PATCH_WORKBENCH),
    

    So same lines can be used.

    You mean wb.h only ? If so, then maybe just copy + paste in wb.h everything from os3 part to os4 ?

    Yes, only wb.h should be changed (apart from PATCH() macro which needs OS4 interface offsets).

    And of course little changes in actual code installing patches, too.

     
  • kas1e

    kas1e - 2013-05-02

    @Ikka

    I dont think offsets are same in interface definitions because they include vararg calls and some calls were removed.

    I am not so understand, but did those offsets are offsets in workbench.library's jump-table ? Because if for SetMethod() we need only interface's offsets, why we need to keep -0x30 as in example which you show which is offsets in jump-tables which is not need it for SetMethod() ?

    I just checked some code of patches, and thats how they looks like.

    warp3dpatch example:

        PtrW3D_DrawPoint    =SetMethod((struct Interface *)IWarp3D, offsetof(struct Warp3DIFace, W3D_DrawPoint)  ,(APTR)LibW3D_DrawPoint);
    

    ccdapatch:

    device->oldbeginio = IExec->SetMethod(device->interface,offsetof(struct DeviceManagerInterface, BeginIO), device->newbeginio); 
    

    So i think about something like this in wb.c:

    #ifdef __amigaos4__
    static PatchList
        wb_patches[WB_PATCH_COUNT]={
            PATCH(offsetof(struct WorkbenchIFace, AddAppWindow), L_WB_AddAppWindow,WB_PATCH_WORKBENCH),
            PATCH(offsetof(struct WorkbenchIFace, RemoveAppWindow), L_WB_RemoveAppWindow,WB_PATCH_WORKBENCH),
            PATCH(offsetof(struct WorkbenchIFace, AddAppIcon), L_WB_AddAppIcon,WB_PATCH_WORKBENCH),
            PATCH(offsetof(struct WorkbenchIFace, RemoveAppIcon), L_WB_RemoveAppIcon,WB_PATCH_WORKBENCH),
    
    .....
    

    What i also didn't get, it is: if we have in PatchList about 28 lines for 28 patches, then why we only for 12 of them do those externs ?

    Maybe you can go through macroses and add them , because i for sure will spend a lot of time to just understand what them all do :) At least when all will be ready, we can have another bunch of proper stacktraces, etc, so can compare beetwen 2 oses and find out roots of those module-crashes.

    @Xenic

    Maybe you can help us with it too ? I just fear to fuck such necessary and low-level parts, as well as i do not understand those new macroses and stop on them quite fast.

     
    Last edit: kas1e 2013-05-02
  • kas1e

    kas1e - 2013-05-02

    @all
    I do some more work on wb.c :

    1. Now i add to wb_get_patchbase() to scan for libbase only for os3/mos, and if os4, then the same call will grab ifaces only (check in new attach wb_get_patchbase() ). That should works because we not open there libs/ifaces, but just use global values to have right one when will do patching. So ifaces without libs should be ok.

    2. Also do proper ifdefs in 4 places where SetFunction is called. Dunno if in all of them its allright, but looks ok imho.

    3. Add ifdef for PatchList struct, which will be one for os4 and another for os3/mos. Through, that one still dont fills correctly as i dunno how to make a PATCH macro normally for os4 now, and then will by correct stuff that PatchList struct.

    I attach new wb.c.

    Wb.h is what i can't understand normally, and what need to fix in amigaos4 part like Ikka says. Also need to do something with PatchList strucutre in wb.h i assume, because now that struct have some "struct EmuTrap trap" which we do not need anymore, so maybe that struct should be also now "new" and specific for os4 (or if possible to change it as it now, also will be ok).

     
    Last edit: kas1e 2013-05-02
  • Ilkka Lehtoranta

    http://itix.amiga-projects.net/dopus5/wb.c
    http://itix.amiga-projects.net/dopus5/wb.h

    Btw for OS4 interface stuff you can use this kind of macros:

    #define PATCHED_1(ret,name,r1,t1,n1) ret name (APTR __local_iface, REG(r1, t1 n1)) {
    
    #define LIBCALL_1(ret,func,callbase,r1,n1) ((ret ASM (*) (APTR __local_iface, REG(r1, ULONG)))func) (iface, (ULONG)n1)
    

    (Compare to OS3 macros to see difference.)

    No idea if it works, though.

    PatchList structure:

    typedef struct
    {
        long    offset;
    #if defined(__MORPHOS__)
        struct EmulLibEntry trap;
    #else
        APTR    function;
    #endif
        short   type;
    } PatchList;
    

    And patching:

                        wb_data->old_function[patch]=SetMethod(interface,wb_patches[patch].offset,wb_patches[patch].function);
    

    Or something like that...

     
    Last edit: Ilkka Lehtoranta 2013-05-02
  • kas1e

    kas1e - 2013-05-02

    @Ikka

    Btw, i currently tryes to make my wb.c, but notice there is new ones. But just in case, check plz my new wb.c and how i create static PatchList wb_patches[WB_PATCH_COUNT].

     
  • kas1e

    kas1e - 2013-05-02

    @Ikka
    Tryed your wb.c and wb.h, but seems they just originals ? And i still can't get why there is 2 offsets need it for os4, as first one its in jump table which we will not use by SetMethod

     
    Last edit: kas1e 2013-05-02
  • Ilkka Lehtoranta

    They are not the same. Idea is to use reduce number of ifdefs in our code so it looks cleaner.

    To reduce number of ifdefs you could also consider replacing...

                    #ifndef __amigaos4__
                    struct Library *lib;
                    #else
                    struct Interface *libiface;
                    #endif
    
                    // Get library
                    #if defined (__amigaos3__) || defined (__MORPHOS__)
                    if ((lib=wb_get_patchbase(wb_patches[patch].type,(struct LibData *)libbase->ml_UserData)))
                    #else
                    if ((libiface=wb_get_patchbase(wb_patches[patch].type,(struct LibData *)libbase->ml_UserData)))
                    #endif
    

    to

                    APTR libptr;
    
                    // Get library
                    if ((libptr=wb_get_patchbase(wb_patches[patch].type,(struct LibData *)libbase->ml_UserData)))
    

    (I.e. use void pointer that casts to anything.)

     
  • Ilkka Lehtoranta

    Anyway your way to construct pathlist is fine, of course. I just tried to reduce ifdefs from the code and define both 68k jump table and os4 jump table offsets on the same line.

     
  • kas1e

    kas1e - 2013-05-02

    Anyway your way to construct pathlist is fine, of course. I just tried to reduce ifdefs from the code and define both 68k jump table and os4 jump table offsets on the same lin

    Do we need that on os4 ?

    I mean, if we now use SetMethod for patching, then as i understand that mean that we do not need to worry about 68k jump table offsets at all ?

    Or you just to do it at one time to reduce all those ifdefs ? But then PathList structure should be changed somehow so it will ok for all oses and for both SetFunction and SetMethod ?

    Also do we need those TRAPINST and TRAPTYPE in the PATCH macro for os4 ?

     
    Last edit: kas1e 2013-05-02
  • Ilkka Lehtoranta

    No, we dont need 68k jump offsets on OS4.

    Or you just to do it at one time to reduce all those ifdefs ? But then PathList structure should be changed somehow so it
    will ok for all oses and for both SetFunction and SetMethod ?

    No need to. Just in case, redownload my wb.h (I fixed some errors I made there) and take a look at PATCHED() macros. On OS4 builds 68k offset is discarded and in OS3 and MOS builds OS4 interface offset is discarded. The offset is stored to same location in PatchList structure.

     
  • xenic

    xenic - 2013-05-02

    @kas1e
    I no sure what exactly you guys are doing. It does seem to me though, that the OS4 library will need to have 68k jump table and Interface patched if the modules are still 68k or if other 68k program versions (like viewfont) are expected to work. This is all getting more complicated than I can understand :-)

     
  • kas1e

    kas1e - 2013-05-02

    @Ikka

    redownload my wb.h

    Got your latest wb.c/wb.h , compiles for os4 well, but those damn macroses just killing me :)) I understand here nothing. All those () ## REG68K TRAPINST and so on and soon. Maybe you can just cut that all so it will be faster for all of us :) I will later just include to the PatchList struct all the right entryes and so on.

    I no sure what exactly you guys are doing. It does seem to me though, that the OS4 library will need to have 68k jump table and Interface patched if the modules are still 68k or if other 68k program versions (like viewfont) are expected to work. This is all getting more complicated than I can understand :-)

    :)) All what i undestand for myself currently, is that we should use SetMethod to make patching works when we do it from os4 binary. But then, and as far as i understand, no 68k app will works with that library. Or i am wrong ?

     
    Last edit: kas1e 2013-05-02
  • Ilkka Lehtoranta

    No, 68k apps will work. I am off to bed now but can take a look at it tomorrow.

     
  • kas1e

    kas1e - 2013-05-02

    @Ikka

    No, 68k apps will work.

    That good, so no worry about ..

    I am off to bed now but can take a look at it tomorrow.

    Yeah, it will be much faster if you will cut all unnecessary stuff for few minutes i assume, and so i can just insert those PatchList entryes recompile and test. Then we can concentrate at last on those module-crashes.

     
  • kas1e

    kas1e - 2013-05-03

    @Ikka

    I commit your wb.c/wb.h, just with fully added PatchList list with proper comment and so on. Let's works on those ones from SVN now.

    Btw for OS4 interface stuff you can use this kind of macros:

    #define PATCHED_1(ret,name,r1,t1,n1) ret name (APTR __local_iface, REG(r1, t1 n1)) {
    
    #define LIBCALL_1(ret,func,callbase,r1,n1) ((ret ASM (*) (APTR __local_iface, REG(r1, ULONG)))func) (iface, (ULONG)n1)
    

    wb.h is ok with new LIBCALL_x macroses, but wb.c scream that "iface" udeclared in all the places where we do LIBCALL_x. Seems we also need to "expand" macroses for iface for all the macroses, and skip ifaces for os3/mos, but use for os4 (i.e. like we do with PATCH macros) ?

    As for PATCHED_x macro, once i change it like this, wb.c screams for all PATCHED_x entrys something like:

    wb.c:322: error: conflicting types for 'L_WB_RemoveAppWindow'
    dopuslib.h:1578: note: previous declaration of 'L_WB_RemoveAppWindow' was here
    
     
    Last edit: kas1e 2013-05-03
  • Ilkka Lehtoranta

    We must change LIBCALL macros to include interface parameter...

    #define LIBCALL_1(ret,func,callbase,callface,r1,n1) ((ret ASM (*) (APTR __local_iface, REG(r1, ULONG)))func) (callface, (ULONG)n1)
    

    OS3 and MorphOS LIBCALL_x macros must be changed accordingly to take dummy interface parameter....

     
  • Ilkka Lehtoranta

    wb.c:322: error: conflicting types for 'L_WB_RemoveAppWindow'
    dopuslib.h:1578: note: previous declaration of 'L_WB_RemoveAppWindow' was here
    

    This gets even trickier now... This L_WB_RemoveAppWindow is a function call you can call from Magellan program and we must keep it as is.

     
  • Ilkka Lehtoranta

    Ok, maybe try this one...

    ~~~~

    define PATCHED_1(ret,name,r1,t1,n1) \

    ret name##_stubs(APTR __local_iface, REG(r1, t1 n1)) \ { \ extern name(REG(r1, t1 n1); \ return (ret)name(n1); \ } \ ret name(t1 n1) { \ ~~~~ 

    and PATCH macro would be something like

    ~~~~ 

    define PATCH(offset,off2,func,type) { off2, (APTR)&func##_stubs, type }

    ~~~~ 

     
  • kas1e

    kas1e - 2013-05-03

    @Ikka

    Ok, maybe try this one...

    I assume at end there should be no \ ? Anyway with and without it bring a lot of

    error: expected declaration specifiers or '...' before 'return'
    error: expected declaration specifiers or '...' before '}' token
    error: expected ';', ',' or ')' before '}' token
    

    Seems one ) is missed after n1), but after i add that , its still throw:

    ~~~~~~
    wb.c:322: warning: type defaults to 'int' in declaration of 'L_WB_RemoveAppWindow'
    wb.c:322: error: conflicting types for 'L_WB_RemoveAppWindow'
    dopuslib.h:1578: note: previous declaration of 'L_WB_RemoveAppWindow' was here
    ~~~~~

     
    Last edit: kas1e 2013-05-03
  • kas1e

    kas1e - 2013-05-03

    PATH macro is ok, through bring a lot of :

    initialization makes integer from pointer without a cast
    
     
1 2 3 4 > >> (Page 1 of 4)

Log in to post a comment.