From 219905c6bcecfda2ae9724a44653831ccb6f6271 Mon Sep 17 00:00:00 2001 From: Greg King Date: Thu, 9 Jul 2015 10:28:38 -0400 Subject: [PATCH 1/7] Fix two string output functions' handling of their buffer-size parameter. That parameter's type is unsigned; but, the functions return an int. If the size is too big for a signed integer, then return an error code. If the size is zero, then don't write anything into a buffer (the buffer pointer may be NULL). But, do format and count the arguments. --- libsrc/common/vsnprintf.s | 47 +++++++++++++++++++++------------------ 1 file changed, 25 insertions(+), 22 deletions(-) diff --git a/libsrc/common/vsnprintf.s b/libsrc/common/vsnprintf.s index a8ed50e06..94ad072ca 100644 --- a/libsrc/common/vsnprintf.s +++ b/libsrc/common/vsnprintf.s @@ -1,7 +1,8 @@ ; ; int __fastcall__ vsnprintf (char* Buf, size_t size, const char* Format, va_list ap); ; -; Ullrich von Bassewitz, 2009-09-26 +; 2009-09-26, Ullrich von Bassewitz +; 2015-07-09, Greg King ; .export _vsnprintf, vsnprintf @@ -9,6 +10,8 @@ .import _memcpy, __printf .importzp sp, ptr1 + .include "errno.inc" + .macpack generic .data @@ -46,8 +49,10 @@ vsnprintf: sta ccount+1 ; Clear ccount ; Get the size parameter and replace it by a pointer to outdesc. This is to -; build a stack frame for the call to _printf. -; If size is zero, there's nothing to do. +; build a stack frame for the call to _printf. The size must not be greater +; than INT_MAX because the return type is int. If the size is zero, +; then nothing will be written into the buffer; but, the arguments still will +; be formatted and counted. ldy #2 lda (sp),y @@ -58,15 +63,13 @@ vsnprintf: iny lda (sp),y + bmi L9 ; More than $7FFF sta ptr1+1 - ora ptr1 - beq L9 - lda #>outdesc sta (sp),y -; Write size-1 to outdesc.uns +; Write size-1 to outdesc.uns. It will be -1 if there is no buffer. ldy ptr1+1 ldx ptr1 @@ -90,17 +93,18 @@ L1: dex pla jsr __printf -; Terminate the string. The last char is either at bufptr+ccount or -; bufptr+bufsize, whichever is smaller. +; Terminate the string if there is a buffer. The last char. is at either +; bufptr+bufsize or bufptr+ccount, whichever is smaller. + ldx bufsize+1 + bmi L4 ; -1 -- No buffer + lda bufsize+0 + cpx ccount+1 + bne L2 + cmp ccount+0 +L2: bcc L3 lda ccount+0 ldx ccount+1 - cpx bufsize+1 - bne L2 - cmp bufsize+0 -L2: bcc L3 - lda bufsize+0 - ldx bufsize+1 clc L3: adc bufptr+0 sta ptr1 @@ -114,16 +118,14 @@ L3: adc bufptr+0 ; Return the number of bytes written and drop buf - lda ccount+0 +L4: lda ccount+0 ldx ccount+1 jmp incsp2 -; Bail out if size is zero. +; Bail out if size is too high. -L9: pla - pla ; Discard ap - lda #0 - tax +L9: lda #ERANGE + jsr __directerrno ; Return -1 jmp incsp6 ; Drop parameters @@ -146,10 +148,11 @@ out: sbc ccount+0 ; Low byte of bytes already written sta ptr1 lda bufsize+1 + bmi @L9 ; -1 -- No buffer sbc ccount+1 sta ptr1+1 bcs @L0 ; Branch if space left - lda #$00 +@L9: lda #$0000 sta ptr1 sta ptr1+1 ; No space left From 146daa1d0a43883bfa22ddfbb571b3d649c6465e Mon Sep 17 00:00:00 2001 From: Greg King Date: Thu, 9 Jul 2015 14:46:28 -0400 Subject: [PATCH 2/7] Made some string output functions reject an invalid NULL buffer pointer. --- libsrc/common/vsnprintf.s | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/libsrc/common/vsnprintf.s b/libsrc/common/vsnprintf.s index 94ad072ca..01bcd6406 100644 --- a/libsrc/common/vsnprintf.s +++ b/libsrc/common/vsnprintf.s @@ -86,9 +86,16 @@ L1: dex sta bufptr+0 stx bufptr+1 +; There must be a buffer if its size is non-zero. + + bit bufsize+1 + bmi L5 + ora bufptr+1 + bze L0 ; The pointer shouldn't be NULL + ; Restore ap and call _printf - pla +L5: pla tax pla jsr __printf @@ -125,6 +132,11 @@ L4: lda ccount+0 ; Bail out if size is too high. L9: lda #ERANGE + .byte $2C ;(bit $xxxx) + +; NULL buffer pointers usually are invalid. + +L0: lda #EINVAL jsr __directerrno ; Return -1 jmp incsp6 ; Drop parameters From 25cf239d806efaa92ef5b3ef218f1c4ec4e101b1 Mon Sep 17 00:00:00 2001 From: Greg King Date: Thu, 16 Jul 2015 15:31:35 -0400 Subject: [PATCH 3/7] Added make rules that build the overlay sample programs. Fixes half of bug issue 178 (on GitHub). --- samples/Makefile | 23 +++++++++++++++++------ samples/README | 1 + 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/samples/Makefile b/samples/Makefile index 79988ea70..951706ce6 100644 --- a/samples/Makefile +++ b/samples/Makefile @@ -44,7 +44,7 @@ endif C1541 = c1541 # -------------------------------------------------------------------------- -# System dependent settings +# System-dependent settings # The Apple machines need the start address adjusted when using TGI LDFLAGS_mandelbrot_apple2 = --start-addr 0x4000 @@ -81,10 +81,10 @@ LDFLAGS_tgidemo_atari = -D __RESERVED_MEMORY__=0x2000 .PRECIOUS: %.o .o: - @$(LD) $(LDFLAGS_$(basename $@)_$(SYS)) -o $@ -t $(SYS) -m $@.map $^ $(CLIB) + @$(LD) $(LDFLAGS_$(@F)_$(SYS)) -o $@ -t $(SYS) -m $@.map $^ $(CLIB) # -------------------------------------------------------------------------- -# List of executables. This list could be made target dependent by checking +# List of executables. This list could be made target-dependent by checking # $(SYS). EXELIST = ascii \ @@ -103,13 +103,23 @@ EXELIST = ascii \ tgidemo # -------------------------------------------------------------------------- -# Rules how to make each one of the binaries +# Rules to make the binaries .PHONY: all all: $(EXELIST) # -------------------------------------------------------------------------- -# Rule to make a disk with all samples. Needs the c1541 program that comes +# Overlay rules. Overlays need special ld65 configuration files. Also, the +# overlay file-names are shortenned to fit the Atari's 8.3-character limit. + +multdemo: multidemo.o + @$(LD) -o $@ -C $(SYS)-overlay.cfg -m $@.map $^ $(CLIB) + +ovrldemo: overlaydemo.o + @$(LD) -o $@ -C $(SYS)-overlay.cfg -m $@.map $^ $(CLIB) + +# -------------------------------------------------------------------------- +# Rule to make a CBM disk with all samples. Needs the c1541 program that comes # with the VICE emulator. .PHONY: disk @@ -125,7 +135,7 @@ samples.d64: all done # -------------------------------------------------------------------------- -# Cleanup rules +# Clean-up rules .PHONY: clean clean: @@ -134,3 +144,4 @@ clean: .PHONY: zap zap: clean $(RM) $(EXELIST) samples.d64 + $(RM) multdemo.? ovrldemo.? diff --git a/samples/README b/samples/README index 5997fc8d0..edd06ff02 100644 --- a/samples/README +++ b/samples/README @@ -11,6 +11,7 @@ Please note: the programs manually. * The makefile specifies the C64 as the default target platform, because all + but one of the programs run on this platform. When compiling for another platform, you will have to change the line that specifies the target system at the top of the makefile. From dd7e55820cfcdfa33ae113412d899df8ba0727aa Mon Sep 17 00:00:00 2001 From: Greg King Date: Fri, 17 Jul 2015 20:33:17 -0400 Subject: [PATCH 4/7] Added a test program for the special features of snprintf(). --- testcode/lib/snprintf-test.c | 144 +++++++++++++++++++++++++++++++++++ 1 file changed, 144 insertions(+) create mode 100644 testcode/lib/snprintf-test.c diff --git a/testcode/lib/snprintf-test.c b/testcode/lib/snprintf-test.c new file mode 100644 index 000000000..d3af47d78 --- /dev/null +++ b/testcode/lib/snprintf-test.c @@ -0,0 +1,144 @@ +/* +** Test a function that formats and writes characters into a string buffer. +** This program does not test formatting. It tests some behaviors that are +** specific to the buffer. It tests that certain conditions are handled +** properly. +** +** 2015-07-17, Greg King +*/ + +#include +#include +#include +#include + +static const char format[] = "1234567890\nabcdefghijklmnopqrstuvwxyz\n%u\n%s\n\n"; +#define FORMAT_SIZE (sizeof format - 2u - 2u - 1u) + +#define arg1 12345u +#define ARG1_SIZE (5u) + +static const char arg2[] = "!@#$%^&*()-+"; +#define ARG2_SIZE (sizeof arg2 - 1u) + +#define STRING_SIZE (FORMAT_SIZE + ARG1_SIZE + ARG2_SIZE) + +static char buf[256]; +static int size; + + +static void fillbuf(void) +{ + memset(buf, 0xFF, sizeof buf - 1u); + buf[sizeof buf - 1u] = '\0'; +} + + +unsigned char main(void) +{ + static unsigned char failures = 0; + + /* Show what sprintf() should create. */ + + if ((size = printf(format, arg1, arg2)) != STRING_SIZE) { + ++failures; + printf("printf() gave the wrong size: %d.\n", size); + } + + /* Test the normal behavior of sprintf(). */ + + fillbuf(); + size = sprintf(buf, format, arg1, arg2); + fputs(buf, stdout); + if (size != STRING_SIZE) { + ++failures; + printf("sprintf() gave the wrong size: %d.\n", size); + } + + /* Test the normal behavior of snprintf(). */ + + fillbuf(); + size = snprintf(buf, sizeof buf, format, arg1, arg2); + fputs(buf, stdout); + if (size != STRING_SIZE) { + ++failures; + printf("snprintf(sizeof buf) gave the wrong size:\n %d.\n", size); + } + + /* Does snprintf() return the full-formatted size even when the buffer + ** is short? Does it write beyond the end of that buffer? + */ + + fillbuf(); + size = snprintf(buf, STRING_SIZE - 5u, format, arg1, arg2); + if (size != STRING_SIZE) { + ++failures; + printf("snprintf(STRING_SIZE-5) gave the wrong size:\n %d.\n", size); + } + if (buf[STRING_SIZE - 5u - 1u] != '\0' || buf[STRING_SIZE - 5u] != 0xFF) { + ++failures; + printf("snprintf(STRING_SIZE-5) wrote beyond\n the end of the buffer.\n"); + } + + /* Does snprintf() detect a buffer size that is too big? */ + + fillbuf(); + errno = 0; + size = snprintf(buf, 0x8000, format, arg1, arg2); + if (size >= 0) { + ++failures; + printf("snprintf(0x8000) didn't give an error:\n %d; errno=%d.\n", size, errno); + } else { + printf("snprintf(0x8000) did give an error:\n errno=%d.\n", errno); + } + if (buf[0] != 0xFF) { + ++failures; + printf("snprintf(0x8000) wrote into the buffer.\n"); + } + + /* snprintf() must measure the length of the formatted output even when the + ** buffer size is zero. But, it must not touch the buffer. + */ + + fillbuf(); + size = snprintf(buf, 0, format, arg1, arg2); + if (size != STRING_SIZE) { + ++failures; + printf("snprintf(0) gave the wrong size:\n %d.\n", size); + } + if (buf[0] != 0xFF) { + ++failures; + printf("snprintf(0) wrote into the buffer.\n"); + } + + /* Does sprintf() detect a zero buffer-pointer? */ + + errno = 0; + size = sprintf(NULL, format, arg1, arg2); + if (size >= 0) { + ++failures; + printf("sprintf(NULL) didn't give an error:\n %d; errno=%d.\n", size, errno); + } else { + printf("sprintf(NULL) did give an error:\n errno=%d.\n", errno); + } + + /* snprintf() must measure the length of the formatted output even when the + ** buffer size is zero. A zero pointer is not an error, in that case. + */ + + size = snprintf(NULL, 0, format, arg1, arg2); + if (size != STRING_SIZE) { + ++failures; + printf("snprintf(NULL,0) gave the wrong size:\n %d.\n", size); + } + + if (failures != 0) { + printf("There were %u", failures); + } else { + printf("There were no"); + } + printf(" failures.\nTap a key. "); + cgetc(); + + return failures; +} From 0b6bcb565e7f514c09bcd1c7b44d3e1bc8791e32 Mon Sep 17 00:00:00 2001 From: Greg King Date: Fri, 17 Jul 2015 20:36:56 -0400 Subject: [PATCH 5/7] Fixed a hardware-stack leak. --- libsrc/common/vsnprintf.s | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/libsrc/common/vsnprintf.s b/libsrc/common/vsnprintf.s index 01bcd6406..228e531d0 100644 --- a/libsrc/common/vsnprintf.s +++ b/libsrc/common/vsnprintf.s @@ -2,7 +2,7 @@ ; int __fastcall__ vsnprintf (char* Buf, size_t size, const char* Format, va_list ap); ; ; 2009-09-26, Ullrich von Bassewitz -; 2015-07-09, Greg King +; 2015-07-17, Greg King ; .export _vsnprintf, vsnprintf @@ -131,12 +131,15 @@ L4: lda ccount+0 ; Bail out if size is too high. -L9: lda #ERANGE +L9: ldy #ERANGE .byte $2C ;(bit $xxxx) ; NULL buffer pointers usually are invalid. -L0: lda #EINVAL +L0: ldy #EINVAL + pla ; Drop ap + pla + tya jsr __directerrno ; Return -1 jmp incsp6 ; Drop parameters From a9982de475a04568542683b197539a075dee9384 Mon Sep 17 00:00:00 2001 From: Greg King Date: Sat, 18 Jul 2015 18:23:08 -0400 Subject: [PATCH 6/7] Added _directerrno() to the sim6502/sim65c02 libraries. --- libsrc/sim6502/errno.s | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/libsrc/sim6502/errno.s b/libsrc/sim6502/errno.s index e6c2422c1..d9f7e397e 100644 --- a/libsrc/sim6502/errno.s +++ b/libsrc/sim6502/errno.s @@ -1,11 +1,29 @@ ; -; Oliver Schmidt, 2013-05-16 +; 2013-05-16, Oliver Schmidt +; 2015-07-18, Greg King ; -; extern int errno; +; Helper functions for several high-level functions. ; .include "errno.inc" +; ---------------------------------------------------------------------------- +; int __fastcall__ _directerrno (unsigned char code); +; /* Set errno to a specific error code; and, return -1. Used +; ** by the library. +; */ + +__directerrno: + jsr __seterrno ; Save in errno +fail: lda #$FF ; Return -1 + tax +ok: rts + + +; ---------------------------------------------------------------------------- +; +; extern int _errno; +; .bss __errno: From b79687da2b85d4759651ec8fbb024ee96b4e77ca Mon Sep 17 00:00:00 2001 From: JT Date: Sun, 19 Jul 2015 18:56:42 -0400 Subject: [PATCH 7/7] Fix base 10 bug (ca65 allows 'a' or 'A' in base10 value) --- src/ca65/scanner.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ca65/scanner.c b/src/ca65/scanner.c index 97ed2c9d9..20053e7e6 100644 --- a/src/ca65/scanner.c +++ b/src/ca65/scanner.c @@ -1012,7 +1012,7 @@ Again: break; } DVal = DigitVal (Buf[I]); - if (DVal > Base) { + if (DVal >= Base) { Error ("Invalid digits in number"); CurTok.IVal = 0; break;