fread cpm no no

Discussion about other targets
Post Reply
DJ Sures
Member
Posts: 67
Joined: Sun Dec 11, 2022 12:41 pm

fread cpm no no

Post by DJ Sures »

Hey everyone - Cloud CP/M and all the z88dk on Nabu have been amazing - thanks to Dom and his hardwork. Today, I worked on a file manager utility that will allow copying files between CPM and the Internet Adapter. I am having difficulty with fread(). Can you think of any reason why this function should not work? I used constants of the filenames for testing...

Code: Select all

#define IN_BUF_LEN 128
uint8_t inBuf[IN_BUF_LEN] = { 0 };

void CPM2CPM() {

  FILE *fsrc;
  FILE *fdst;

  fsrc = fopen("0/a:test.txt", "r");
    
  fdst = fopen("0/a:new.txt", "w");

  while (true) {

    int read = fread(inBuf, 1, IN_BUF_LEN, fsrc);

    printf("read: %d\n", read);

    if (read == 0)
      break;
    
    fwrite(inBuf, 1, read, fdst);
  };

  fclose(fsrc);
  fclose(fdst);

  puts("Done");
}
This is my compile-line

Code: Select all

zcc +cpm -subtype=nabubdos main.c -compiler=sdcc -create-app -O3 --opt-code-speed -o shell
The result of this code is that it causes the program to crash. And by the crash, I mean it jumps to an arbitrary program address and repeats the function call. I can't even get the print() or anything below the read() to run. Any ideas?
stefano
Well known member
Posts: 2137
Joined: Mon Jul 16, 2007 7:39 pm

Re: fread cpm no no

Post by stefano »

I suggest to proceed by steps. Try building a short program opening a file (no drive or path specifier), tell the result, and exit immediately.
Then try adding the drivespec, then the user specifier, etc..
User avatar
dom
Well known member
Posts: 2076
Joined: Sun Jul 15, 2007 10:01 pm

Re: fread cpm no no

Post by dom »

My hunch is that fopen() is returning NULL and then the memory at 0 is being interpreted as a funopen() style file which jumps to somewhere random.

So, as Stefano suggests, kill the user + drive specifiers, check for NULL and then add the features back in.
DJ Sures
Member
Posts: 67
Joined: Sun Dec 11, 2022 12:41 pm

Re: fread cpm no no

Post by DJ Sures »

I tried removing the drive & user spec, and the original version checked for null. I stripped it down to show the basic functionality of what I'm doing. Here's a more detailed test version of the same thing.

Code: Select all

void CPM2CPM() {

  FILE *fsrc;

  puts("Gonna open");

  fsrc = fopen("TEST.TXT", "r");

  if (fsrc == 0) {

    puts("fopen returned null");

    exit(0);
  }

  printf("opened %u\n", fsrc);

  while (true) {

    puts("gonna read");
    int read = fread(inBuf, 1, IN_BUF_LEN, fsrc);
    printf("read: %d\n", read);

    if (read == 0)
      break;    
  };

  puts("gonna close");
  fclose(fsrc);

  puts("Done");
}
When adding more code, the behavior is similar in that it jumps to an arbitrary location and repeats.
Capture.PNG

I also tried it in run cpm. In run CPM, fopen returned a null and behaved differently even though the file was present.
Capture.PNG
You do not have the required permissions to view the files attached to this post.
User avatar
dom
Well known member
Posts: 2076
Joined: Sun Jul 15, 2007 10:01 pm

Re: fread cpm no no

Post by dom »

I think there's two issues here, so lets separate them.

1. I suspect the user areas aren't fully supported by the file io routines - I'm noting that in RunCPM you changed to a user area. Let's leave that to one side for the moment.

2. A Cloud CP/M issue.

I can reproduce this problem only when running Cloud CP/M. I've tried with zxcc and the same binary behaves properly:

Code: Select all

~/emu/zxcc-0.5.7/bin/zxcc a.com
Gonna open
opened 6785
gonna read
read: 128
gonna read
read: 0
gonna close
Done
And I've also tried with the disc version of CP/M:
nabu_file.jpg
I've not managed to track down what the problem is, but it seems to occur after we do a F_READRAND. By default, we do rely on ix not being disturbed over a bdos call which if it is changed might cause some weird behaviour.
You do not have the required permissions to view the files attached to this post.
DJ Sures
Member
Posts: 67
Joined: Sun Dec 11, 2022 12:41 pm

Re: fread cpm no no

Post by DJ Sures »

I checked my bios functions to see the stack that's being saved. IX is indeed in there, saved and restored at the beginning and end of each bios function.
#define SAVE_STACK_ASM push hl; \
ld hl, sp; \
ld sp, 0xffff; \
push hl; \
push bc; \
push de; \
push af; \
push iy; \
push ix;

#define RESTORE_STACK_ASM pop ix; \
pop iy; \
pop af; \
pop de; \
pop bc; \
pop hl; \
ld sp, hl; \
pop hl;
Since z88dk's file io would be using bdos, it shouldn't matter the DPB configuration, but here is cloud cpm DPB.

Code: Select all

#define DSM_MAX_ALLOCATION_BLOCK_NUMBER_ABC 2047		
#define DSM_AREA_SIZE_ABC ( ( DSM_MAX_ALLOCATION_BLOCK_NUMBER_ABC / 8 ) + 1)

  // BLS = 4096
  4,                                   // _DPB_ABC.SPT Sectors per track
  5,                                   // _DPB_ABC.BSH Data allocation "Block Shift Factor"
  31,                                  // _DPB_ABC.BLM Data allocation Block Mask
  1,                                   // _DPB_ABC.EXM Extent Mask
  DSM_MAX_ALLOCATION_BLOCK_NUMBER_ABC, // _DPB_ABC.DSM Total storage capacity of the disk drive. Number of the last Allocation Block ( Number of entries per disk -1 )
  511,                                 // _DPB_ABC.DRM Number of the last directory entry ( Number of entries per disk -1 ). Total number of directory entries that can be stored on this drive. ( AL0, AL1 determine reserved directory blocks )
  0xff,                                // _DPB_ABC.AL0 Starting value of the first two bytes of the allocation table.
  0x00,                                // _DPB_ABC.AL1
  0,                                   // _DPB_ABC.CKS "Check area Size" Size of the directory check vector. Number of directory entries to check for disk change. Zero for a HDD.
  0,                                   // _DPB_ABC.OFF Number of system reserved tracks at the beginning of the ( logical ) disk
If user areas aren't fully supported, I'll have to look into using bdos directly. I'm making a utility to copy files between the IA and Cloud CPM.

Is there any compiler pragma that I should be using for file io? Or allocating a heap size? Also, cloud cpm is 2.2 whereas nabu cpm is 3. I'm not sure if that is making a difference.
User avatar
dom
Well known member
Posts: 2076
Joined: Sun Jul 15, 2007 10:01 pm

Re: fread cpm no no

Post by dom »

If user areas aren't fully supported, I'll have to look into using bdos directly. I'm making a utility to copy files between the IA and Cloud CPM.
I just think it's missing a call to setup the user area in the fcb when opening - for some reason the bit is commented out.

Yeah, I've already updated our naburn config to cope with the extent size increasing to 4k from 2k.

So, it looks like there's a routine in the bios that's writing over our program, in particular the code that handles the cache.

The offending routine is here which is being called from 0xed87 which I think is BIOS read_sector - I'm not sure where the calling code is loading up ix since I've not got the source code.

Code: Select all

                    push    ix                              ;[feba] dd e5
                    ex      (sp),hl                         ;[febc] e3
                    add     hl,bc                           ;[febd] 09
                    ld      (hl),d                          ;[febe] 72
                    dec     hl                              ;[febf] 2b
                    ld      (hl),e                          ;[fec0] 73
                    dec     hl                              ;[fec1] 2b
                    pop     bc                              ;[fec2] c1
                    ld      (hl),b                          ;[fec3] 70
                    dec     hl                              ;[fec4] 2b
                    ld      (hl),c                          ;[fec5] 71
                    ld      l,c                             ;[fec6] 69
                    ld      h,b                             ;[fec7] 60
                    ret                                     ;[fec8] c9
and here's a mame trace on the watchpoint:
watchpoint.jpg
With the program overwritten there ends up being a jp $0012 which restarts the program, hence the continual looping.
You do not have the required permissions to view the files attached to this post.
DJ Sures
Member
Posts: 67
Joined: Sun Dec 11, 2022 12:41 pm

Re: fread cpm no no

Post by DJ Sures »

Interesting, I can't imagine how that's happening because IX is pushed to my save stack at the beginning of the function and restored at the end.

The calling origin from the code below is...
7920 2887 cd0000 call ____sdcc_store_dehl_bcix

Which calls (this is the portion of code that you traced to)
____sdcc_store_dehl_bcix = $FEBA ; addr, public, , ____sdcc_store_dehl_bcix, code_l_sdcc, l/sdcc/____sdcc_store_dehl_bcix.asm:7

But at the time of sdcc_store_dehl_bcix mangling IX, it's already been saved and will be restored at the end of the function. Something with my observation and tracing are wrong if you think that's where the IX mangle occurs.

These memory addresses need to have 0xC500 added to them

Code: Select all

  7876                          ;	---------------------------------
  7877                          ; Function doRead
  7878                          ; ---------------------------------
  7879                          _doRead:
  7880  283b  f3                	di
  7881  283c  e5                	push	hl
  7882  283d  21000039          	ld hl, sp
  7883  2841  31ffff            	ld sp, 0xffff
  7884  2844  e5                	push hl
  7885  2845  c5                	push bc
  7886  2846  d5                	push de
  7887  2847  f5                	push af
  7888  2848  fde5              	push iy
  7889  284a  dde5              	push ix                            <--------------------------- IX SAVED
  7890  284c  3e26              	ld	a,0x26
  7891  284e  d3a1              	out	(_IO_VDPLATCH), a
  7892  2850  3e48              	ld	a,0x48
  7893  2852  d3a1              	out	(_IO_VDPLATCH), a
  7894  2854  3eb3              	ld	a,0xb3
  7895  2856  d3a0              	out	(_IO_VDPDATA), a
  7896  2858  3a0400            	ld	a,(_CPM_USER_DRIVE)
  7897  285b  e60f              	and	a,0x0f
  7898  285d  c6a2              	add	a,0xa2
  7899  285f  d3a0              	out	(_IO_VDPDATA), a
  7900  2861  3e0f              	ld	a,0x0f
  7901  2863  d300              	out	(_IO_CONTROL), a
  7902  2865  2114ff            	ld	hl,__TEMP
  7903  2868  3600              	ld	(hl),0x00
  7904  286a  218e07            	ld	hl,__FILEID
  7905  286d  7e                	ld	a, (hl)
  7906  286e  3c                	inc	a
  7907  286f  23                	inc	hl
  7908  2870  b6                	or	a, (hl)
  7909  2871  2864              	jr	Z,l_doRead_00104
  7910  2873  2a1bff            	ld	hl, (__TRACK)
  7911  2876  110000            	ld	de,0x0000
  7912  2879  53                	ld	d, e
  7913  287a  5c                	ld	e, h
  7914  287b  65                	ld	h, l
  7915  287c  2e00              	ld	l,0x00
  7916  287e  cb24              	sla	h
  7917  2880  cb13              	rl	e
  7918  2882  cb12              	rl	d
  7919  2884  01ffff            	ld	bc,-1
  7920  2887  cd0000            	call	____sdcc_store_dehl_bcix   <---------- CALLING ORIGIN
  7921  288a  2a1dff            	ld	hl, (__SECTOR)
  7922  288d  110000            	ld	de,0x0000
  7923  2890  0607              	ld	b,0x07
  7924                          l_doRead_00123:
  7925  2892  29                	add	hl, hl
  7926  2893  cb13              	rl	e
  7927  2895  cb12              	rl	d
  7928  2897  10f9              	djnz	l_doRead_00123
  7929  2899  dd7efc            	ld	a,(ix-4)
  7930  289c  85                	add	a, l
  7931  289d  4f                	ld	c, a
  7932  289e  dd7efd            	ld	a,(ix-3)
  7933  28a1  8c                	adc	a, h
  7934  28a2  47                	ld	b, a
  7935  28a3  dd7efe            	ld	a,(ix-2)
  7936  28a6  8b                	adc	a, e
  7937  28a7  5f                	ld	e, a
  7938  28a8  dd7eff            	ld	a,(ix-1)
  7939  28ab  8a                	adc	a, d
  7940  28ac  57                	ld	d, a
  7941  28ad  2a19ff            	ld	hl, (__DMA)
  7942  28b0  e5                	push	hl
  7943  28b1  3a8e07            	ld	a,(__FILEID)
  7944  28b4  218000            	ld	hl,0x0080
  7945  28b7  e3                	ex	(sp), hl
  7946  28b8  d5                	push	de
  7947  28b9  c5                	push	bc
  7948  28ba  110000            	ld	de,0x0000
  7949  28bd  d5                	push	de
  7950  28be  e5                	push	hl
  7951  28bf  f5                	push	af
  7952  28c0  33                	inc	sp
  7953  28c1  cdc320            	call	_rn_fileHandleRead
  7954  28c4  f1                	pop	af
  7955  28c5  f1                	pop	af
  7956  28c6  f1                	pop	af
  7957  28c7  f1                	pop	af
  7958  28c8  f1                	pop	af
  7959  28c9  33                	inc	sp
  7960  28ca  7d                	ld	a, l
  7961  28cb  d680              	sub	a,0x80
  7962  28cd  b4                	or	a, h
  7963  28ce  280c              	jr	Z,l_doRead_00105
  7964  28d0  2114ff            	ld	hl,__TEMP
  7965  28d3  3601              	ld	(hl),0x01
  7966  28d5  1805              	jr	l_doRead_00105
  7967                          l_doRead_00104:
  7968  28d7  2114ff            	ld	hl,__TEMP
  7969  28da  36ff              	ld	(hl),0xff
  7970                          l_doRead_00105:
  7971  28dc  3e26              	ld	a,0x26
  7972  28de  d3a1              	out	(_IO_VDPLATCH), a
  7973  28e0  3e48              	ld	a,0x48
  7974  28e2  d3a1              	out	(_IO_VDPLATCH), a
  7975  28e4  010000            	ld	bc,__vdp_textBuffer+0
  7976  28e7  218007            	ld	hl,__vdp_viewPortStart
  7977  28ea  5e                	ld	e, (hl)
  7978  28eb  1600              	ld	d,0x00
  7979  28ed  212600            	ld	hl,0x0026
  7980  28f0  19                	add	hl, de
  7981  28f1  09                	add	hl, bc
  7982  28f2  7e                	ld	a, (hl)
  7983  28f3  d3a0              	out	(_IO_VDPDATA), a
  7984  28f5  010000            	ld	bc,__vdp_textBuffer+0
  7985  28f8  218007            	ld	hl,__vdp_viewPortStart
  7986  28fb  5e                	ld	e, (hl)
  7987  28fc  1600              	ld	d,0x00
  7988  28fe  212700            	ld	hl,0x0027
  7989  2901  19                	add	hl, de
  7990  2902  09                	add	hl, bc
  7991  2903  7e                	ld	a, (hl)
  7992  2904  d3a0              	out	(_IO_VDPDATA), a
  7993  2906  3e07              	ld	a,0x07
  7994  2908  d300              	out	(_IO_CONTROL), a
  7995  290a  3e0e              	ld	a, 0x0e
  7996  290c  d341              	out	(_IO_AYLATCH), a
  7997  290e  3e20              	ld	a, 0x20
  7998  2910  d340              	out	(_IO_AYDATA), a
  7999  2912  dde1              	pop	ix                                  <----------------- IX RESTORED
  8000  2914  fde1              	pop iy
  8001  2916  f1                	pop af
  8002  2917  d1                	pop de
  8003  2918  c1                	pop bc
  8004  2919  e1                	pop hl
  8005  291a  f9                	ld sp, hl
  8006  291b  e1                	pop hl
  8007  291c  3a14ff            	ld	a, (__TEMP)
  8008  291f  fb                	ei
  8009  2920  c9                	ret
User avatar
dom
Well known member
Posts: 2076
Joined: Sun Jul 15, 2007 10:01 pm

Re: fread cpm no no

Post by dom »

So, it looks like we're trying to store at dehl at (ix + bc).

I would expect, by the time we get to here, that ix has been setup as a pointer to the stack frame using something like this:

Code: Select all

ld ix,-4
add ix,sp
ld sp,hl
However, looking at the entry to this function that bit of code seems to be missing, so it ends up using ix value that the function was entered with. In this case whatever value user code was using.

What compiler options are you using/what does the C function look like?

Oh, user area stuff should be working now - the parsing had a couple of bugs/oddities in it.
DJ Sures
Member
Posts: 67
Joined: Sun Dec 11, 2022 12:41 pm

Re: fread cpm no no

Post by DJ Sures »

Compiler options are

Code: Select all

zcc +z80 -mz80 --no-crt -compiler=sdcc --list -m -O3 --opt-code-size --no-peep -lm main.c -o "BIOS_CPM22.BIN"
The only reason it seems to use IX in the read function is due to the 32 bit int for dealing with the file size.

Code: Select all

/// **************************************************************************************************
// 13 Read 128 bytes
/// **************************************************************************************************
void doRead() __naked {

  __asm

    di;

    SAVE_STACK_ASM
    
  __endasm;

  vdp_setWriteAddress(VDP_NAME_TABLE + 38);
  IO_VDPDATA = 97 + 'R';
  IO_VDPDATA = 97 + 65 + (CPM_USER_DRIVE & 0x0f);
  
  IO_CONTROL = CONTROL_ROMSEL | CONTROL_VDOBUF | CONTROL_STROBE | CONTROL_LED_CHECK;

  _TEMP = 0x00;

  if (_FILEID != 0xff) {

    uint32_t offset = ((uint32_t)_TRACK * 128 * 4) + ((uint32_t)_SECTOR * 128);

    uint16_t read = rn_fileHandleRead(_FILEID, (void *)_DMA, 0, offset, 128);
    
    if (read != 128)
      _TEMP = 0x01;
      
  } else {

    _TEMP = 0xff;
  }

  vdp_setWriteAddress(VDP_NAME_TABLE + 38);
  IO_VDPDATA = _vdp_textBuffer[38 + _vdp_viewPortStart];
  IO_VDPDATA = _vdp_textBuffer[39 + _vdp_viewPortStart];

  IO_CONTROL = CONTROL_ROMSEL | CONTROL_VDOBUF | CONTROL_STROBE;

  __asm

    ld	a, IOPORTA;
    out	(_IO_AYLATCH), a;

    ld	a, INT_MASK_KEYBOARD;
    out	(_IO_AYDATA), a;
    
    RESTORE_STACK_ASM

    ld a, (__TEMP);

    ei;

    ret;

  __endasm;
}
User avatar
dom
Well known member
Posts: 2076
Joined: Sun Jul 15, 2007 10:01 pm

Re: fread cpm no no

Post by dom »

Code: Select all

void doRead() __naked {
So I think __naked is intended for pure assembly functions and as a result it removes the generated function pro- and epilogue. And unfortunately for some reason, sdcc has hoisted the creation of offset+read from inside the scope of the if to the top level so the stack space can't be reserved.

The prologue for this function without __naked is:

Code: Select all

_doRead:
        push    ix
        ld      ix,0
        add     ix,sp
        push    af
        push    af
        di
Which makes the stack space available.

I suspect the easiest solution is to ensure that __naked functions only contain assembly, and just call the real implementation as follows:

Code: Select all


void doRead() __naked {

  __asm
    di;
    SAVE_STACK_ASM
    call _doread_impl
    ......
    RESTORE_STACK_ASM
    ld a, (__TEMP);
    ei;
    ret;
  __endasm;
}

void doread_impl() {
 vdp_setWriteAddress(VDP_NAME_TABLE + 38);
  IO_VDPDATA = 97 + 'R';
  IO_VDPDATA = 97 + 65 + (CPM_USER_DRIVE & 0x0f);
....
}
[code]
DJ Sures
Member
Posts: 67
Joined: Sun Dec 11, 2022 12:41 pm

Re: fread cpm no no

Post by DJ Sures »

As always, you're insight set me on the right track - thanks again. Boy, I'd like to buy you a beer (or ten). So what you said about IX made sense, so actually, all I needed to do was save IX and ld IX, 0 in my SAVE and RESTORE stack functions. I also realized I should be using exx.

So now my stack is saved and restored with...

Code: Select all

#define COMMON_STACK_ADDR     0xffff
#define INT_STACK_ADDR        0xff40

#define SAVE_STACK_ASM        push hl;       \
                              ld hl, sp;     \
                              ld sp, COMMON_STACK_ADDR; \
                              exx;           \
                              push  af;      \
                              push  iy;      \
                              push  ix;      \
                              ld ix, 0;

#define RESTORE_STACK_ASM     pop ix;      \
                              pop iy;      \
                              pop af;      \
                              exx;         \
                              ld sp, hl;   \
                              pop hl;      
And it works...
Capture.PNG
You do not have the required permissions to view the files attached to this post.
DJ Sures
Member
Posts: 67
Joined: Sun Dec 11, 2022 12:41 pm

Re: fread cpm no no

Post by DJ Sures »

Okay the Cloud CPM BIOS online has been updated - so next time you load cloud cpm, the new bios will automatically download, and the z88dk CPM file IO will work for you. Thank you

The next question is - how can I read/write to different user areas? You mentioned the code for parsing the user/drive spec was commented out. Is it due to a bug, or was it incomplete?

The fopen() header documents the filename to be [userarea]/[drive]:[file] i.e. 2/a:test.txt

Would you advise that I implement my file io with bdos? Or is the user/drive spec of fopen() actively being worked on?
User avatar
dom
Well known member
Posts: 2076
Joined: Sun Jul 15, 2007 10:01 pm

Re: fread cpm no no

Post by dom »

That's cool, glad to help - pretty luck that it hit the code that we're testing because random corruption isn't fun to find. I'm not sure there's much benefit to using exx though since the registers are saved.

I fixed up the parsing of the filename last night, so the fcb is correctly populated with the user area. I didn't get a chance to test it all the way through though. The code looks like it should handle it, so if there's any failures I suspect it'll just be a few missing calls to F_USERNUM
DJ Sures
Member
Posts: 67
Joined: Sun Dec 11, 2022 12:41 pm

Re: fread cpm no no

Post by DJ Sures »

Wonderful - and I learned a lot as well about using ix as a stack. Something I never considered reseting after saving.

I only choose to exx thinking it’ll be faster than 4 separate push’s and pop’s *shrug* I didn’t look to see the cycle difference. But it looks cleaner anyway :)

I wasn’t expecting you to jump on fixing the user area and drive stuff so quick - so I built my own using bdos. But using yours would be more portable. So I’ll go with yours and throw mine away.
DJ Sures
Member
Posts: 67
Joined: Sun Dec 11, 2022 12:41 pm

Re: fread cpm no no

Post by DJ Sures »

Thanks to DOM, there is RNCMD in Cloud CP/M - for copying files between the PC, CPM and User Areas! Wooot
Capture.PNG
Capture.PNG
You do not have the required permissions to view the files attached to this post.
Post Reply