Skip to content

Commit

Permalink
<@:(<&f) bug when f returns a pyx
Browse files Browse the repository at this point in the history
  • Loading branch information
HenryHRich committed Nov 23, 2024
1 parent 72e0ca6 commit e62eafe
Show file tree
Hide file tree
Showing 6 changed files with 36 additions and 21 deletions.
2 changes: 1 addition & 1 deletion jsrc/j.h
Original file line number Diff line number Diff line change
Expand Up @@ -738,7 +738,7 @@ struct jtimespec jmtfclk(void); //'fast clock'; maybe less inaccurate; intended

// Use MEMAUDIT to sniff out errant memory alloc/free
#ifndef MEMAUDIT
#define MEMAUDIT 0x0 // Bitmask for memory audits:
#define MEMAUDIT 0x00 // Bitmask for memory audits:
// 1: make sure chains are valid (check headers)
// 2: full audit of tpush/tpop
// detect double-frees before they happen,
Expand Down
8 changes: 4 additions & 4 deletions jsrc/m.c
Original file line number Diff line number Diff line change
Expand Up @@ -216,17 +216,17 @@ B jtmeminitt(JJ jt){I k;
R 1;}

// Audit all memory chains to detect overrun
#if SY_64
#define AUDITFILL ||((MEMAUDIT&0x4)?AC(Wx)!=(I)0xdeadbeefdeadbeefLL:0)
#if SY_64 // define optional audits.
#define AUDITFILL // obsolete ||((MEMAUDIT&0x4)?AM(Wx)!=(I)0xdeadbeefdeadbeefLL:0)
#else
#define AUDITFILL ||((MEMAUDIT&0x4)?AC(Wx)!=(I)0xdeadbeefL:0)
#define AUDITFILL // obsolete ||((MEMAUDIT&0x4)?AM(Wx)!=(I)0xdeadbeefL:0)
#endif
// your code for which the warning gets suppressed
void jtauditmemchains(J jt){
#if MEMAUDIT&0x30
F1PREFIP; I Wi,Wj;A Wx,prevWx=0; forcetomemory(&prevWx); if((MEMAUDITPCALLENABLE)&&((MEMAUDIT&0x20)||JT(jt,peekdata))){
for(Wi=PMINL;Wi<=PLIML;++Wi){Wj=0; Wx=(jt->mempool[-PMINL+Wi]);
NOUNROLL while(Wx){if(FHRHPOOLBIN(AFHRH(Wx))!=(Wi-PMINL)AUDITFILL||Wj>0x10000000)SEGFAULT; prevWx=Wx; Wx=AFCHAIN(Wx); ++Wj;}}
NOUNROLL while(Wx){if(Wx->origin!=THREADID1(jt)||FHRHPOOLBIN(AFHRH(Wx))!=(Wi-PMINL)AUDITFILL||Wj>0x10000000)SEGFAULT; prevWx=Wx; Wx=AFCHAIN(Wx); ++Wj;}}
}
#endif
}
Expand Down
26 changes: 13 additions & 13 deletions jsrc/result.h
Original file line number Diff line number Diff line change
Expand Up @@ -252,15 +252,15 @@ do{
}while(1);
}
}else{
// forced-boxed result. Must not be sparse. The result box is recursive to begin with, unless WILLBEOPENED is set
// forced-boxed result. Must not be sparse. The result box is recursive to begin with, unless WILLBEOPENED is set. Wrecks are impossible
ASSERT(!ISSPARSE(AT(z)),EVNONCE);
// If z is DIRECT inplaceable, it must be unique and we can inherit them into a pristine result. Otherwise clear pristinity
ZZFLAGWORD&=((AC(z)>>(BW-AFPRISTINEX))&(-(AT(z)&DIRECT)))|~ZZFLAGPRISTINE;
if(likely(ZZWILLBEOPENEDNEVER||!(ZZFLAGWORD&ZZFLAGWILLBEOPENED))) {
// normal case where we are creating the result box. Must incorp the result. Can't see an advantage is storing the virtual temporarily, and that would require testing for UNINCORP block
realizeifvirtual(z); razaptstackend(z); // Since we are moving the result into a recursive box, we must ra() it. This plus rifv plus pristine flagging (above) =INCORPRA. We trouble to see if we can shorten tstack, since that is likely
*zzboxp=z; // install the new box. zzboxp is ALWAYS a pointer to a box when force-boxed result
} else {
}else{
// The result of this verb will be opened next, so we can take some liberties with it. We don't need to realize any virtual block EXCEPT one that we might
// be reusing in this loop. The user flags those UNINCORPABLE. Rather than realize it we just make a virtual clone, since realizing might be expensive.
// That is, if z is one of the virtual blocks we use to track subarrays, we mustn't incorporate it, so we clone it. These subarrays can be inputs to functions
Expand All @@ -273,21 +273,20 @@ do{
// box code all over assumes that contents are never inplaceable, and since we go through here only when we are going through box code next, we honor that
ACIPNO(z); *zzboxp=z; // install the new box. zzboxp is ALWAYS a pointer to a box when force-boxed result
if(unlikely((ZZFLAGWORD&ZZFLAGCOUNTITEMS)!=0)){
// if the result will be razed next, we will count the items and store that in AM. We will also ensure that the result boxes' contents have the same type
// the result will be razed next. We will count the items and store that in AM. We will also ensure that the result boxes' contents have the same type
// and item-shape. If one does not, we turn off special raze processing. It is safe to take over the AM field in this case, because we know this is WILLBEOPENED and
// (1) will never assemble or epilog; (2) will feed directly into a verb that will discard it without doing any usecount modification
I diff; // Will be set to 0 if we are unable to report the # items
I diff; // Will be set to non0 if we are unable to report the # items
#if PYXES
// If the returned result is a pyx, we can't look into it to get its type/len. We could see if the pyx has been resolved, but we don't
if(unlikely(diff=(AT(z)&PYX))){ZZFLAGWORD&=~ZZFLAGCOUNTITEMS; // if the result is a pyx, which can't be inspected, skip it, which makes the item count invalid
}else{
#else
{
if(likely((diff=(AT(z)&PYX))==0)) // if the result is a pyx, which can't be inspected, set diff to non0 (which makes the item count invalid) and skip the shape test
#endif
{ // this brace may be part of the previous line!
// not a pyx - we can count the items
#if !ZZSTARTATEND // going forwards
A result0=AAV(zz)[0]; // fetch pointer to the first
A result0=AAV(zz)[0]; // fetch pointer to the first
#else
A result0=AAV(zz)[AN(zz)-1]; // fetch pointer to first value stored, which is in the last position
A result0=AAV(zz)[AN(zz)-1]; // fetch pointer to first value stored, which is in the last position
#endif
// see if the items of the new match the old, and increment the number of items
I* zs=AS(z); I* ress=AS(result0); I zr=AR(z); I resr=AR(result0); //fetch info
Expand All @@ -296,7 +295,7 @@ do{
I nitems=zs[0]; nitems=(zr==0)?1:nitems; zzcounteditems+=nitems; // add new items to count in zz. zs[0] will never segfault, even if z is empty
#endif
}
ZZFLAGWORD^=(diff!=0)<<ZZFLAGCOUNTITEMSX; // turn off bit if we can't say the items are homogeneous
ZZFLAGWORD^=(diff!=0)<<ZZFLAGCOUNTITEMSX; // turn off bit (which must be set now) if we can't say the items are homogeneous
}
// Note: by checking COUNTITEMS inside WILLBEOPENED we suppress support for COUNTITEMS in \. which sets WILLBEOPENEDNEVER. It would be safe to
// count then, because no virtual contents would be allowed. But we are not sure that the EPILOG is safe, and this path is now off to the side
Expand All @@ -316,8 +315,9 @@ do{
// Processing the first cell. Allocate the result area now that we know the shape/type of the result.
// Get the rank/type to allocate for the presumed result
// Get the type to allocate
I natoms=AN(z); // number of atoms per result cell
I zzt=AT(z); I zzr=AR(z); zzt=(ZZASSUMEBOXATOP||ZZFLAGWORD&ZZFLAGBOXATOP)?BOX:zzt; zzr=(ZZASSUMEBOXATOP||ZZFLAGWORD&ZZFLAGBOXATOP)?0:zzr; natoms=(ZZASSUMEBOXATOP||ZZFLAGWORD&ZZFLAGBOXATOP)?1:natoms;
I natoms=AN(z); I zzt=AT(z); I zzr=AR(z); // number of atoms per result cell; type and rank of first result cell
// Type of zz is that of z, except for BOXATOP: then it's BOX. The value can be a pyx only if BOXATOP is set, so we know zz can't itself be a pyx
zzt=(ZZASSUMEBOXATOP||ZZFLAGWORD&ZZFLAGBOXATOP)?BOX:zzt; zzr=(ZZASSUMEBOXATOP||ZZFLAGWORD&ZZFLAGBOXATOP)?0:zzr; natoms=(ZZASSUMEBOXATOP||ZZFLAGWORD&ZZFLAGBOXATOP)?1:natoms;
zzcelllen=natoms<<bplg(zzt); // number of bytes in one cell.
JMCSETMASK(zzendmask,zzcelllen,ZZSTARTATEND) // set mask for JMCR

Expand Down
2 changes: 1 addition & 1 deletion jsrc/viavx.c
Original file line number Diff line number Diff line change
Expand Up @@ -1128,7 +1128,7 @@ A jtindexofsub(J jt,I mode,A a,A w){F2PREFIP;PROLOG(0079);A h=0;fauxblockINT(zfa
RZ(z=EPILOGNORET(z));
// Since EPILOG may have rewritten AM, and IFORKEY never returns to the parser, we can store the FORKEY result in AM.
*((mode&IIOPMSK)==IFORKEY?(I*)&AM(z):(I*)&jt->shapesink)=forkeyresult;
RETF(z);
R z; // Not RETF, which may audit the AM field which we just modified
} /* a i."r w main control */

// verb to vector combine@e. compounds. The i. code is in the self
Expand Down
15 changes: 15 additions & 0 deletions test/gtdot4.ijs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,22 @@ stime =: 6!:1''
stime =: 6!:1''
1 = > 6!:3 t. 1"0 ] 6 # 1
(5-granularity) < stime -~ 6!:1''
delth''

NB. Test mixtures of pyxes and non-pyxes
1: 0 T. ''
1: 0 T. ''
1: 0 T. ''
1: 0 T. '' NB. create threads

a =: i.&.> (20 ?@$ 10)
(;a) -: ; (0.30 > (#a) ?@$ 0) >@] t.'worker'^:["0 a
(;a) -: ; >@] t.'worker'"0 a
(;a) -: ;@:((0.30 > (#a) ?@$ 0) >@] t.'worker'^:["0 ]) a
(;a) -: ;@:(>@] t.'worker'"0) a


delth''
4!:55 ;:'delth granularity stime'

epilog''
Expand Down
4 changes: 2 additions & 2 deletions test/tsu.ijs
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ blacklist=: ((<testpath),each 'gmbx.ijs';'gfft.ijs';'glapack.ijs'),testfiles 'gm
blacklist=: blacklist, (-. (<UNAME)e.<'Darwin')#(<testpath),each <'gcip1.ijs'
blacklist=: blacklist, (IFRASPI<(IF64<UNAME-:'Linux')+.(IFWIN>IF64)+.IFIOS+.(UNAME-:'Wasm'))#(<testpath),each <'gregex.ijs' NB. require libjpcre2 binary
blacklist=: blacklist, (-.IF64)#(<testpath),each <'g6x14.ijs' NB. require 64-bit
blacklist=: blacklist, (1=1 { 8 T. '')#(<testpath),each 'gtdot.ijs';'gtdot1.ijs';'gtdot2.ijs';'gtdot3.ijs';'gtdot4.ijs' NB. require multithreading
blacklist=: blacklist, (1=1 { 8 T. '')#(<testpath),each 'gtdot.ijs';'gtdot1.ijs';'gtdot2.ijs';'gtdot3.ijs';'gtdot4.ijs';'gtdot5.ijs' NB. require multithreading
blacklist=: blacklist, (-.15!:23'')#(<testpath),each 'g15x.ijs';'g7x5.ijs';'gdll.ijs';'gdll_df.ijs';'gmmf.ijs';'gmmf1s.ijs';'gmmf1u.ijs';'gmmf1w.ijs' NB. 15!:0 unavailable
blacklist=: blacklist, ('Wasm'-:UNAME)#(<testpath),each <'gstack.ijs' NB. crash
blacklist=: blacklist, (IFQT*.'Wasm'-:UNAME)#(<testpath),each 'g331ps.ijs';'gsp422.ijs';'gsp432.ijs' NB. crash
Expand Down Expand Up @@ -475,7 +475,7 @@ allorcmdline=: 3 :0
NB. fail
end.
else.
ddall-.((testpath,'g') , ,&'.ijs')&.>;:'131 cip 520 sp 7x tdot1 3x tdot2 tdot3 tdot4 tdot t' NB. temporarily ignore threading
ddall-.((testpath,'g') , ,&'.ijs')&.>;:'131 cip 520 sp 7x tdot1 3x tdot2 tdot3 tdot4 tdot5 tdot t' NB. temporarily ignore threading
end.
)

Expand Down

0 comments on commit e62eafe

Please sign in to comment.