Bug de LibsExec - Page 1

yop,

C'est la première fois que j'utilise cette fonction, je crois. D'après la doc :

	It relocs the library, calls the function and unreallocs the library.
	The parameters pushed on the stack are corrupted.
	If there is an error, the parameter 'lib_name' equals zero after the call.

Et dans le code (sld.asm, ligne 1226) :\next subq.l #8,a7 ; Fix stack ptr. movem.l d0-d7/a0-a7,-(a7) ; Pop a7 to increase Stack Frame GET_DATA_PTR ; Get a6 subq.l #4,a7 ; Create Stack Frame \UnrelocLib: move.l (LibsExecList-Ref)(a6),a3 move.l (a3),a5 ; Read Lib Ptr bsr kernel::unrelocation ; Unreloc & delete. \FreeNode: move.l (LibsExecList-Ref)(a6),a3 ; Kernel::relocation may destroy all registers if an error occured ! (execpt a6 & a5) move.l -(a3),(4*16)(a7) ; Set Return Address move.l -(a3),(LibsExecList-Ref)(a6) ; Set new head move.l a3,(a7) ROM_THROW HeapFreePtr ; Free Node moveq #1,d7 ; Set no error \AllocError addq.l #4,a7 ; Fix stack move.l d7,(4*16)(a7) ; Set error \end: movem.l (a7)+,d0-d7/a0-a6 rts
Le \next est le point de "réentrance" de LibsExec après que la fonction de la lib désirée ait été exécuté. Comme on voit, le moveq #1,d7 est exécuté après le \FreeNode. C'est le code de retour de LibsExec, qui se retrouve à (sp) au retour au programme appelant. Mais si l'initialisation de la librairie échoue, LibsExec saute à \Freenode, ce qui fait que dans tous les cas, on se retrouve avec un code de retour de 1, même en cas d'erreur (dans mon cas, la lib n'est pas présente sur la calc).

Au passage, il y a une petite erreur dans l'exemple de la doc : Example: pea arg2function ; Arg2 of the function move.w #arg1function,-(a7) ; Arg1 of the function move.b #1,-(a7) ; Version 1 move.w #3,-(a7) ; Function 3 pea LibsName ; Libs Name move.w #145,d0 ; D0 = 145 for the function ! jsr kernel::LibsExec tst.l (a7) lea (2+4+2+2+2+4)(a7),a7 ; Does not affect the flag
Le premier 2 est manifestement en trop, t'as dû compter par mégarde l'argument de d0.

Voilà, si tu veux, je mets mon bureau d'étude sur un patch afin de corriger cette gravissime faille de sécurité


(edit -> post Zeph-syntax-colorized-powered)

1) Ok pour le bug. Je ne vois pas trop corriger sans augmenter de manièree déraisonnable la taille de la fonction Comme c'est un code retour, ce n'est pas critique.
2) J'ai essayé de faire cette fonction avec un LibsBegin + LibsCall. LibsCall + LibsEnd, mais vu les contraintes (pile + registres non détruits), je n'ai finalement pas trouvé cela plus simple que faire le reloc à la main.
3) doc : ok corrigé.

1. Erf, pareil, j'ai tripatouillé ça, j'arrive pas à faire quelque chose de bien, j'espérais que t'aies l'idée qui casse tout mais je vois que tu butes sur le même problème :/
2. Ok, je m'en doutais que c'était la sauvegardes des registres qui faisait mal. C'est le prix de la puissance de la fonction cela-dit. ^^

Bon, je crois avoir une solution qui tient en 4 octets. Pas compilé, pas testé, mais le linker est complexe, donc tu pourrais déjà me dire si c'est pas complètement déconnant.

Le patch (attention, j'ai ouvert l'original je sais pas comment, Kate l'a modifié sans prévenir, donc le patch n'est pas applicable tel quel (il me sort un fichier de 600 lignes pour les quelques modifs que j'ai faites :/). Mais au moins, tu localises les modifs avec ça) :

 kernel::LibsExec:
	movem.l	d0-d7/a0-a6,-(a7)			;
 	GET_DATA_PTR					; Get Preos Data Ptr
-	moveq	#0,d7					; Set Error
 	pea	(12).w					; Create a list (Next, RealReturnAddr, LibPtr)
@@ -1189,17 +1189,18 @@
 	move.l	a0,(a7)					; Ok ?
 	beq.s	\AllocError
 	move.l	a0,a3					; Ptr to node
	move.l	(LibsExecList-Ref)(a6),(a3)+		; Save Next node in List
 	move.l	(4*16)(a7),(a3)+			; Save Real Return Address
 	move.l	a3,(LibsExecList-Ref)(a6)		; Update list head
 	move.l	(4*17)(a7),a2				; Get Library name
 	move.b	(4*18+2)(a7),d3				; Minimum Library version
 	bsr	kernel::FindLib				; Find the library
+	clr.l	4*17(sp)				; Set error
 	move.l	a0,(a3)					; Save the lib & Check for success
 	beq.s	\FreeNode
 	move.l	a0,a5					; Relocation of the library.
 	bsr	kernel::relocation			; If it failed the library is free!
 	tst.w	d0					;
 	bne.s	\FreeNode
 	move.l	a5,(a3)					; Update Library Code section
 	move.l	a5,a0
@@ -1216,30 +1217,33 @@
 	rts						; Jump to the function and return \next

 \next	subq.l	#8,a7					; Fix stack ptr.
+	st.b	(sp)					; Set success
 	movem.l	d0-d7/a0-a7,-(a7)			; Pop a7 to increase Stack Frame
	GET_DATA_PTR					; Get a6
 	subq.l	#4,a7					; Create Stack Frame
 \UnrelocLib:
 	move.l	(LibsExecList-Ref)(a6),a3
 	move.l	(a3),a5					; Read Lib Ptr
	bsr	kernel::unrelocation			; Unreloc & delete.
 \FreeNode:
 	move.l	(LibsExecList-Ref)(a6),a3		; Kernel::relocation may destroy all registers if an error occured ! (execpt a6 & a5)
 	move.l	-(a3),(4*16)(a7)			; Set Return Address
 	move.l	-(a3),(LibsExecList-Ref)(a6)		; Set new head
 	move.l	a3,(a7)
 	ROM_THROW HeapFreePtr				; Free Node
-	moveq	#1,d7					; Set no error
-\AllocError
+\AllocError2:
 	addq.l	#4,a7					; Fix stack
-	move.l	d7,(4*16)(a7)				; Set error
 \end:	movem.l	(a7)+,d0-d7/a0-a6
 	rts

+\AllocError:
+	clr.l	4*17(sp)					; Set error
+	bra.s	\AllocError2

Ce qui donne :; It calls the function without modifying the registers, and it pops its argument ; during the call (LIB_DESCRIPTOR, function, and version). ; kernel::LibsExec(char name[], WORD function, BYTE version, ...) kernel::LibsExec: movem.l d0-d7/a0-a6,-(a7) ; GET_DATA_PTR ; Get Preos Data Ptr moveq #0,d7 ; Set Error pea (12).w ; Create a list (Next, RealReturnAddr, LibPtr) ROM_THROW HeapAllocPtr ; Alloc Node move.l a0,(a7) ; Ok ? beq.s \AllocError move.l a0,a3 ; Ptr to node move.l (LibsExecList-Ref)(a6),(a3)+ ; Save Next node in List move.l (4*16)(a7),(a3)+ ; Save Real Return Address move.l a3,(LibsExecList-Ref)(a6) ; Update list head move.l (4*17)(a7),a2 ; Get Library name move.b (4*18+2)(a7),d3 ; Minimum Library version bsr kernel::FindLib ; Find the library clr.l 4*17(sp) ; Set error move.l a0,(a3) ; Save the lib & Check for success beq.s \FreeNode move.l a0,a5 ; Relocation of the library. bsr kernel::relocation ; If it failed the library is free! tst.w d0 ; bne.s \FreeNode move.l a5,(a3) ; Update Library Code section move.l a5,a0 move.w (4*18)(a7),d0 ; Function # bsr.s kernel::LibsPtr ; Get the ptr to the required func move.l a0,d0 beq.s \UnrelocLib move.l a0,(4*17)(a7) ; New return address lea \next(pc),a0 move.l a0,(4*18)(a7) ; After it return here addq.l #4,a7 ; Pop Stack Frame movem.l (a7)+,d0-d7/a0-a6 addq.l #4,a7 ; Pop First Return Address rts ; Jump to the function and return \next \next subq.l #8,a7 ; Fix stack ptr. st.b (sp) ; Set success movem.l d0-d7/a0-a7,-(a7) ; Pop a7 to increase Stack Frame GET_DATA_PTR ; Get a6 subq.l #4,a7 ; Create Stack Frame \UnrelocLib: move.l (LibsExecList-Ref)(a6),a3 move.l (a3),a5 ; Read Lib Ptr bsr kernel::unrelocation ; Unreloc & delete. \FreeNode: move.l (LibsExecList-Ref)(a6),a3 ; Kernel::relocation may destroy all registers if an error occured ! (execpt a6 & a5) move.l -(a3),(4*16)(a7) ; Set Return Address move.l -(a3),(LibsExecList-Ref)(a6) ; Set new head move.l a3,(a7) ROM_THROW HeapFreePtr ; Free Node moveq #1,d7 ; Set no error \AllocError2: addq.l #4,a7 ; Fix stack \end: movem.l (a7)+,d0-d7/a0-a6 rts \AllocError: clr.l 4*17(sp) ; Set error bra.s \AllocError2

Je crois que j'ai une solution avec -2 octets

diff --git a/src/sld.asm b/src/sld.asm
index d477fff..66e7344 100644
--- a/src/sld.asm
+++ b/src/sld.asm
@@ -1183,7 +1183,9 @@ kernel::ExtractFromPack:
 kernel::LibsExec:
        movem.l d0-d7/a0-a6,-(a7)                       ; 
        GET_DATA_PTR                                    ; Get Preos Data Ptr
-       moveq   #0,d7                                   ; Set Error
+       move.l  (4*17)(a7),a2                           ; Get Library name^M
+       move.b  (4*18+2)(a7),d3                         ; Minimum Library version^M
+       clr.l   (4*17)(a7)                              ; Set Error^M
        pea     (12).w                                  ; Create a list (Next, RealReturnAddr, LibPtr)
        ROM_THROW HeapAllocPtr                          ; Alloc Node
        move.l  a0,(a7)                                 ; Ok ?
@@ -1192,8 +1194,6 @@ kernel::LibsExec:
        move.l  (LibsExecList-Ref)(a6),(a3)+            ; Save Next node in List 
        move.l  (4*16)(a7),(a3)+                        ; Save Real Return Address
        move.l  a3,(LibsExecList-Ref)(a6)               ; Update list head
-       move.l  (4*17)(a7),a2                           ; Get Library name
-       move.b  (4*18+2)(a7),d3                         ; Minimum Library version
        bsr     kernel::FindLib                         ; Find the library
        move.l  a0,(a3)                                 ; Save the lib & Check for success
        beq.s   \FreeNode
@@ -1216,6 +1216,7 @@ kernel::LibsExec:
        rts                                             ; Jump to the function and return \next
 
 \next  subq.l  #8,a7                                   ; Fix stack ptr.
+       st.b    (a7)                                    ; Set success^M
        movem.l d0-d7/a0-a7,-(a7)                       ; Pop a7 to increase Stack Frame
        GET_DATA_PTR                                    ; Get a6 
        subq.l  #4,a7                                   ; Create Stack Frame
@@ -1229,10 +1230,8 @@ kernel::LibsExec:
        move.l  -(a3),(LibsExecList-Ref)(a6)            ; Set new head
        move.l  a3,(a7)
        ROM_THROW HeapFreePtr                           ; Free Node
-       moveq   #1,d7                                   ; Set no error
 \AllocError
        addq.l  #4,a7                                   ; Fix stack
-       move.l  d7,(4*16)(a7)                           ; Set error
 \end:  movem.l (a7)+,d0-d7/a0-a6
        rts

Superbe. Heureux de t'avoir donné de l'inspiration en tout cas.

Un bugfix qui est plus petit que l'original, chapô #chapeau#

C'est toujours pareil avec PpHd, j'ai jamais le dernier mot en optimisation

Bon, maintenant, je veux faire ça :

Dans main :
LibRef* desc = kernel::LibsExec (malib, CreateTrampoline, 0, <args>);
Et dans malib:

LibRef* malib::CreateTrampoline (<args>)
{
    LibRef* desc = kernel::LibsBegin (malib, 0);

    while (!functions_table_end)
    {
        void* ptr = kernel::LibsPtr (desc, funcX);
        ...
    }

    return desc;
}

On va voir si PreOS a les reins solides

Folco (./9) :
C'est toujours pareil avec PpHd, j'ai jamais le dernier mot en optimisation

Tiens grâce à toi je viens de comprendre un truc : si les posts de PpHd sont aussi courts, ce n'est pas parce qu'il est laconique, c'est parce qu'il les optimise en taille

Y'a un bug dans ma solution proposée, et vous ne l'avez pas vu

Folco (./8) :
C'est toujours pareil avec PpHd, j'ai jamais le dernier mot en optimisation

Bon, maintenant, je veux faire ça :

Dans main :
LibRef* desc = kernel::LibsExec (malib, CreateTrampoline, 0, <args>);
Et dans malib:

LibRef* malib::CreateTrampoline (<args>)
{
    LibRef* desc = kernel::LibsBegin (malib, 0);

    while (!functions_table_end)
    {
        void* ptr = kernel::LibsPtr (desc, funcX);
        ...
    }

    return desc;
}

On va voir si PreOS a les reins solides

Oh ! *******

Zerosquare (./9) :

Folco (./9) :
C'est toujours pareil avec PpHd, j'ai jamais le dernier mot en optimisation

Tiens grâce à toi je viens de comprendre un truc : si les posts de PpHd sont aussi courts, ce n'est pas parce qu'il est laconique, c'est parce qu'il les optimise en taille

Plutôt parce que je poste allongé sur le canapé, et que ce n'est pas simple de taper avec le doc courbé

Wow, on découvre des trucs mine de rien

PpHd (./11) :
Y'a un bug dans ma solution proposée, et vous ne l'avez pas vu

Oui, faut remonter le pea 12 de 3 lignes

(edit -> voir post suivant pour gagner 2+6 octets)

Voici le diff unifié qui s'applique au sld.asm de PreOS 1.0.7 :

--- sld.asm.org	2013-04-26 19:09:04.882258000 +0200
+++ sld.asm	2013-04-27 14:04:37.533079000 +0200
@@ -1183,17 +1183,18 @@
 kernel::LibsExec:
 	movem.l	d0-d7/a0-a6,-(a7)			; 
 	GET_DATA_PTR					; Get Preos Data Ptr
-	moveq	#0,d7					; Set Error
+	lea	(4*16)(a7),a4
+	move.l	(a4),a2
+	move.b	(4*17+2)(a7),d3				; Minimum Library version
+	clr.l	(a4)+
 	pea	(12).w					; Create a list (Next, RealReturnAddr, LibPtr)
 	ROM_THROW HeapAllocPtr				; Alloc Node
 	move.l	a0,(a7)					; Ok ?
 	beq.s	\AllocError
 	move.l	a0,a3					; Ptr to node
 	move.l	(LibsExecList-Ref)(a6),(a3)+		; Save Next node in List 
-	move.l	(4*16)(a7),(a3)+			; Save Real Return Address
+	move.l	(a4)+,(a3)+
 	move.l	a3,(LibsExecList-Ref)(a6)		; Update list head
-	move.l	(4*17)(a7),a2				; Get Library name
-	move.b	(4*18+2)(a7),d3				; Minimum Library version
 	bsr	kernel::FindLib				; Find the library
 	move.l	a0,(a3)					; Save the lib & Check for success
 	beq.s	\FreeNode
@@ -1207,16 +1208,17 @@
 	bsr.s	kernel::LibsPtr				; Get the ptr to the required func
 	move.l	a0,d0
 	beq.s	\UnrelocLib
-	move.l	a0,(4*17)(a7)				; New return address
+	move.l	a0,(a4)+
 	lea	\next(pc),a0
-	move.l	a0,(4*18)(a7)				; After it return here
+	move.l	a0,(a4)
 	addq.l	#4,a7					; Pop Stack Frame
 	movem.l	(a7)+,d0-d7/a0-a6
 	addq.l	#4,a7					; Pop First Return Address
 	rts						; Jump to the function and return \next
 
 \next	subq.l	#8,a7					; Fix stack ptr.
-	movem.l	d0-d7/a0-a7,-(a7)			; Pop a7 to increase Stack Frame
+	st.b	(a7)
+	movem.l	d0-d7/a0-a7,-(a7)			; Push a7 to increase Stack Frame
 	GET_DATA_PTR					; Get a6 
 	subq.l	#4,a7					; Create Stack Frame
 \UnrelocLib:
@@ -1229,10 +1231,8 @@
 	move.l	-(a3),(LibsExecList-Ref)(a6)		; Set new head
 	move.l	a3,(a7)
 	ROM_THROW HeapFreePtr				; Free Node
-	moveq	#1,d7					; Set no error
 \AllocError
 	addq.l	#4,a7					; Fix stack
-	move.l	d7,(4*16)(a7)				; Set error
 \end:	movem.l	(a7)+,d0-d7/a0-a6
 	rts
 

Tu as testé ?

Non. Je m'y mets, sauf si mon chef de gouvernement m'envoie faire des courses

Bon, j'avais un bug. On est à -6 octets par rapport à la version initiale.

1. Le patch (s'applique à sld.asm de la dernière release officielle) :

--- sld.asm.org	2013-04-26 19:09:04.882258000 +0200
+++ sld.asm	2013-04-27 20:19:43.530160000 +0200
@@ -1183,17 +1183,19 @@
 kernel::LibsExec:
 	movem.l	d0-d7/a0-a6,-(a7)			; 
 	GET_DATA_PTR					; Get Preos Data Ptr
-	moveq	#0,d7					; Set Error
+	lea	(4*16)(a7),a4
+	move.l	(a4),a2					; Get Library name
+	move.b	(4*17+2)(a7),d3				; Minimum Library version
+	clr.l	(a4)					; Set error
 	pea	(12).w					; Create a list (Next, RealReturnAddr, LibPtr)
 	ROM_THROW HeapAllocPtr				; Alloc Node
 	move.l	a0,(a7)					; Ok ?
 	beq.s	\AllocError
 	move.l	a0,a3					; Ptr to node
 	move.l	(LibsExecList-Ref)(a6),(a3)+		; Save Next node in List 
-	move.l	(4*16)(a7),(a3)+			; Save Real Return Address
+	move.l	-(a4),(a3)+				; Save Real Return Address
+	addq.l	#4,a4
 	move.l	a3,(LibsExecList-Ref)(a6)		; Update list head
-	move.l	(4*17)(a7),a2				; Get Library name
-	move.b	(4*18+2)(a7),d3				; Minimum Library version
 	bsr	kernel::FindLib				; Find the library
 	move.l	a0,(a3)					; Save the lib & Check for success
 	beq.s	\FreeNode
@@ -1207,16 +1209,17 @@
 	bsr.s	kernel::LibsPtr				; Get the ptr to the required func
 	move.l	a0,d0
 	beq.s	\UnrelocLib
-	move.l	a0,(4*17)(a7)				; New return address
+	move.l	a0,(a4)+				; New return address
 	lea	\next(pc),a0
-	move.l	a0,(4*18)(a7)				; After it return here
+	move.l	a0,(a4)					; After it return here
 	addq.l	#4,a7					; Pop Stack Frame
 	movem.l	(a7)+,d0-d7/a0-a6
 	addq.l	#4,a7					; Pop First Return Address
 	rts						; Jump to the function and return \next
 
 \next	subq.l	#8,a7					; Fix stack ptr.
-	movem.l	d0-d7/a0-a7,-(a7)			; Pop a7 to increase Stack Frame
+	st.b	(a7)					; Set success
+	movem.l	d0-d7/a0-a7,-(a7)			; Push a7 to increase Stack Frame
 	GET_DATA_PTR					; Get a6 
 	subq.l	#4,a7					; Create Stack Frame
 \UnrelocLib:
@@ -1229,10 +1232,8 @@
 	move.l	-(a3),(LibsExecList-Ref)(a6)		; Set new head
 	move.l	a3,(a7)
 	ROM_THROW HeapFreePtr				; Free Node
-	moveq	#1,d7					; Set no error
 \AllocError
 	addq.l	#4,a7					; Fix stack
-	move.l	d7,(4*16)(a7)				; Set error
 \end:	movem.l	(a7)+,d0-d7/a0-a6
 	rts
 

2. h220xtsr n'est pas construit dans le makefile de PreOS : tigcc -O2 -Wall -W h220xtsr -ar h220xtsr.a

3. J'ai un truc étrange, particulièrement quand j'ai un souci de lib non trouvé. Le problème semble purement graphique, et à ce moment du programme, je n'ai même pas ouvert dynamiquement la libc de PedroM, donc a priori j'ai pas touché aux streams :

et

4. PreOS a les reins solides
;----------------------------------------------- ; Open pdtlib ;----------------------------------------------- lea PdtlibFilename(pc),a0 lea PdtlibTrampolinesTable(pc),a1 lea PdtlibTrampolinesOffets(pc),a2 movea.l fp,a3 moveq.l #0,d1 move.b d1,-(sp) move.w #PDTLIB_INSTALL_TRAMPOLINES,-(sp) pea (a0) RAMT RAM_kernel::LibsExec move.l a0,PDTLIB_DESCRIPTOR(fp) tst.l (sp) bne.s \PdtlibLoaded lea STACK_FRAME_SIZE(fp),sp lea StrErrorPdtlib(pc),a0 bra.s \BootFail \PdtlibLoaded:
et DEFINE pdtlib@0004 RAMT RAM_kernel::LibsBegin move.l a0,d0 beq.s \Fail movem.l a0-a2,-(sp) \Loop: movea.l (sp),a0 move.w (a1)+,d0 bmi.s \EOT RAMT RAM_kernel::LibsPtr move.w (a2)+,d0 move.w #$4EF9,0(a3,d0.w) ; jmp ... move.l a0,2(a3,d0.w) ; ... imm.l bra.s \Loop movea.l (sp),a0 ; Function not found RAMT RAM_kernel::LibsEnd clr.l (sp) \EOT: movem.l (sp)+,a0-a2 \Fail: rts
FEEL THE POWER !!!

(edit, j'avais oublié les commentaires, le patch est maintenant complet)

Au fait, un LibsBegin sur une lib qui n'a pas de table d'export fonctionne, il faut attendre un LibsPtr pour se rendre compte du problème. C'est pas très cohérent, mais au pire c'est pas bien grave. (edit -> ok, c'est le comportement ajouté par PreOS 1.0.6, pas la peine d'y toucher)

En tout cas, cette version de LibsExec se comporte comme attendu avec une lib sans export, avec une lib valide, ou sans la lib attendue. J'ai pas testé sous condition de mémoire basse.

1. Ok, je vais voir.
2. déjà corrigé je crois
3. Heu, je sais plus comment ca marche
4. je crois l'avoir testé dans des cas encore plus tordu. Et le linker de pedrom est encore plus compliqué que celui de preos
5. Ben on peut faire un libsbegin pour réallouer la librairie puis après l’exécuter par un kernel::exec par exemple... ou pour la décompresser pour récupérer des infos.

3. J'ai utilisé DlgMessage pour signaler l'absence de la lib (ce qui fait doublon avec PedroM, je devrais le virer). C'est la première fois que j'utilise ce romcall avec PedroM, et c'est la première fois que je constate ce comportement. J'irai jeter un oeil.
5. En effet.

yop,

Il y a une version de PerdoM à jour avec ce patch, ou il faut que je recompile avec ?

Merci bien.