putc/fputc incorrect return?

putc/fputc incorrect return?

[tlaronde]

Hello,

I have been investigating a failure to compile METAFONT fonts on present
version of 9front and found the culprit.

When writing a binary file, putc is used and the macro invoking it was
testing the return not against EOF but against the byte supposed to be
written.

The problem is that, apparently now, putc is returning a signed char
promoted to a signed int, meaning that when the byte written has the
higher bit set, all the higher bits are set in the int returned:

I write, say 0xaa, it returns 0xffffffaa. So when comparing, this
failed.

But the problem is that if I write 0xff it returns 0xffffffff that is
EOF...

Isn't putc/fputc supposed to return a "char" as a value that is
0x000000nn so precisely one can differenciate from EOF error?

Best regards,
-- 
        Thierry Laronde <tlaronde +AT+ polynum +dot+ com>
                     http://www.kergis.com/
                       http://www.sbfa.fr/
Key fingerprint = 0FF7 E906 FBAF FE95 FD89  250D 52B1 AE95 6006 F40C

Re: [9front] putc/fputc incorrect return?

[ori]

> I write, say 0xaa, it returns 0xffffffaa. So when comparing, this
> failed.
> 
> But the problem is that if I write 0xff it returns 0xffffffff that is
> EOF...
> 
> Isn't putc/fputc supposed to return a "char" as a value that is
> 0x000000nn so precisely one can differenciate from EOF error?

Hey -- thanks for the report, but I can't reproduce your issue.
Here's my test code:

#include <stdio.h>

int
main(int argc, char **argv)
{
	printf("\n'%x'\n", putc(0xaa, stdout));
	return 0;
}

It prints
<junk>
'aa'

I'm running on 9front tip, on amd64.

Re: [9front] putc/fputc incorrect return?

[tlaronde]

Hello,

On Thu, Jul 09, 2020 at 07:27:13AM -0700, ori@eigenstate.org wrote:
> 
> > I write, say 0xaa, it returns 0xffffffaa. So when comparing, this
> > failed.
> > 
> > But the problem is that if I write 0xff it returns 0xffffffff that is
> > EOF...
> > 
> > Isn't putc/fputc supposed to return a "char" as a value that is
> > 0x000000nn so precisely one can differenciate from EOF error?
> 
> Hey -- thanks for the report, but I can't reproduce your issue.
> Here's my test code:
> 
> #include <stdio.h>
> 
> int
> main(int argc, char **argv)
> {
> 	printf("\n'%x'\n", putc(0xaa, stdout));
> 	return 0;
> }
> 
> It prints
> <junk>
> 'aa'
> 
> I'm running on 9front tip, on amd64.

Yep, if I compile just this, I get the same and the correct result.

But if I print the returned value in the kerTeX code, I get the
promotion ("0xffffffaa") (whether calling putc or fputc)...

I will have to try to reduce the code to the strict minimum to
understand what the hell is going on (it works everywhere and it used to
work under 9front too).

But there is a fair amount of cascading cpp substitutions due to the
pseudo-pascal to raw C dance plus system patching and it triggers something...

And (last minute memory), it tries to use registers too (I will need to
test this too to see if that makes a difference).

Best,
-- 
        Thierry Laronde <tlaronde +AT+ polynum +dot+ com>
                     http://www.kergis.com/
                       http://www.sbfa.fr/
Key fingerprint = 0FF7 E906 FBAF FE95 FD89  250D 52B1 AE95 6006 F40C

Re: [9front] putc/fputc incorrect return?

[ori]

> Hello,
> 
> On Thu, Jul 09, 2020 at 07:27:13AM -0700, ori@eigenstate.org wrote:
>> 
>> > I write, say 0xaa, it returns 0xffffffaa. So when comparing, this
>> > failed.
>> > 
>> > But the problem is that if I write 0xff it returns 0xffffffff that is
>> > EOF...
>> > 
>> > Isn't putc/fputc supposed to return a "char" as a value that is
>> > 0x000000nn so precisely one can differenciate from EOF error?
>> 
>> Hey -- thanks for the report, but I can't reproduce your issue.
>> Here's my test code:
>> 
>> #include <stdio.h>
>> 
>> int
>> main(int argc, char **argv)
>> {
>> 	printf("\n'%x'\n", putc(0xaa, stdout));
>> 	return 0;
>> }
>> 
>> It prints
>> <junk>
>> 'aa'
>> 
>> I'm running on 9front tip, on amd64.
> 
> Yep, if I compile just this, I get the same and the correct result.
> 
> But if I print the returned value in the kerTeX code, I get the
> promotion ("0xffffffaa") (whether calling putc or fputc)...
> 
> I will have to try to reduce the code to the strict minimum to
> understand what the hell is going on (it works everywhere and it used to
> work under 9front too).
> 
> But there is a fair amount of cascading cpp substitutions due to the
> pseudo-pascal to raw C dance plus system patching and it triggers something...
> 
> And (last minute memory), it tries to use registers too (I will need to
> test this too to see if that makes a difference).

If you can post the code somewhere, I can also take a look.

Re: [9front] putc/fputc incorrect return?

[tlaronde]

On Thu, Jul 09, 2020 at 09:55:08AM -0700, ori@eigenstate.org wrote:
> > Hello,
> > 
> > On Thu, Jul 09, 2020 at 07:27:13AM -0700, ori@eigenstate.org wrote:
> >> 
> >> > I write, say 0xaa, it returns 0xffffffaa. So when comparing, this
> >> > failed.
> >> > 
> >> > But the problem is that if I write 0xff it returns 0xffffffff that is
> >> > EOF...
> >> > 
> >> > Isn't putc/fputc supposed to return a "char" as a value that is
> >> > 0x000000nn so precisely one can differenciate from EOF error?
> >> 
> >> Hey -- thanks for the report, but I can't reproduce your issue.
> >> Here's my test code:
> >> 
> >> #include <stdio.h>
> >> 
> >> int
> >> main(int argc, char **argv)
> >> {
> >> 	printf("\n'%x'\n", putc(0xaa, stdout));
> >> 	return 0;
> >> }
> >> 
> >> It prints
> >> <junk>
> >> 'aa'
> >> 
> >> I'm running on 9front tip, on amd64.
> > 
> > Yep, if I compile just this, I get the same and the correct result.
> > 
> > But if I print the returned value in the kerTeX code, I get the
> > promotion ("0xffffffaa") (whether calling putc or fputc)...
> > 
> > I will have to try to reduce the code to the strict minimum to
> > understand what the hell is going on (it works everywhere and it used to
> > work under 9front too).
> > 
> > But there is a fair amount of cascading cpp substitutions due to the
> > pseudo-pascal to raw C dance plus system patching and it triggers something...
> > 
> > And (last minute memory), it tries to use registers too (I will need to
> > test this too to see if that makes a difference).
> 
> If you can post the code somewhere, I can also take a look.

Thank you, but I will have to reduce it because with the pecularities of
RISK (the compilation framework) and the pecularities of the TeX web-to-raw-C
translation process, you will waste a significant amount of time just to get
your way around before succeeding to tackle the problem.

BTW, I tried to truss (with the acid program) the binary, but it gives
absolutely no clue. I hit the problem with classic printf bisection on
the resulting C files.

I will return back to it this week-end and will post an update (whether
the hit on the core or some simplified code exhibiting the problem) at
the begining of next week.

Best,
-- 
        Thierry Laronde <tlaronde +AT+ polynum +dot+ com>
                     http://www.kergis.com/
                       http://www.sbfa.fr/
Key fingerprint = 0FF7 E906 FBAF FE95 FD89  250D 52B1 AE95 6006 F40C

Re: [9front] putc/fputc incorrect return?

[ori]

>> If you can post the code somewhere, I can also take a look.
> 
> Thank you, but I will have to reduce it because with the pecularities of
> RISK (the compilation framework) and the pecularities of the TeX web-to-raw-C
> translation process, you will waste a significant amount of time just to get
> your way around before succeeding to tackle the problem.
> 
> BTW, I tried to truss (with the acid program) the binary, but it gives
> absolutely no clue. I hit the problem with classic printf bisection on
> the resulting C files.
> 
> I will return back to it this week-end and will post an update (whether
> the hit on the core or some simplified code exhibiting the problem) at
> the begining of next week.
> 
> Best,

Sounds good, thanks for taking the time.

Re: [FOUND] [9front] putc/fputc incorrect return

[tlaronde]

Here is the simple code exhibiting the problem. There are three
components:

1) int is negative;
2) there are at least two successive calls;
3) the stream is neither stdout nor stderr.

#include <u.h>
#include <stdio.h>

int
main(int argc, char *argv[])
{
	FILE *fp;
	short w;

	if ( (fp = fopen("/tmp/bug.tfm", "wb")) == NULL ) {
		(void)fprintf(stderr, "Unable to open '/tmp/bug.tfm' for writing.\n");
		return 1;
	}

	/* Writing successively negative word on the file stream: bug.
	 */
	w = 0xffaa;
	(void)fprintf(stderr, "\nWriting big endian '%#hx' on file stream returned"
		" -> %#x %#x\n", w, putc(w >> 8, fp), putc(w, fp));

	/* On stdout, no problem.
	 */
	(void)fprintf(stderr, "\nWriting big endian '%#hx' on stdout returned"
		" -> %#x %#x\n", w, putc(w >> 8, stdout), putc(w, stdout));

	return 0;
}

Best,
-- 
        Thierry Laronde <tlaronde +AT+ polynum +dot+ com>
                     http://www.kergis.com/
                       http://www.sbfa.fr/
Key fingerprint = 0FF7 E906 FBAF FE95 FD89  250D 52B1 AE95 6006 F40C

Re: [FOUND] [9front] putc/fputc incorrect return

[ori]

> #include <u.h>
> #include <stdio.h>
> 
> int
> main(int argc, char *argv[])
> {
> 	FILE *fp;
> 	short w;
> 
> 	if ( (fp = fopen("/tmp/bug.tfm", "wb")) == NULL ) {
> 		(void)fprintf(stderr, "Unable to open '/tmp/bug.tfm' for writing.\n");
> 		return 1;
> 	}
> 
> 	/* Writing successively negative word on the file stream: bug.
> 	 */
> 	w = 0xffaa;
> 	(void)fprintf(stderr, "\nWriting big endian '%#hx' on file stream returned"
> 		" -> %#x %#x\n", w, putc(w >> 8, fp), putc(w, fp));
> 
> 	/* On stdout, no problem.
> 	 */
> 	(void)fprintf(stderr, "\nWriting big endian '%#hx' on stdout returned"
> 		" -> %#x %#x\n", w, putc(w >> 8, stdout), putc(w, stdout));
> 
> 	return 0;
> }

The type of the assignment ends up as char, so it
gets sign extended. Masking after the assignment
fixes it for me.

Here's a patch.

diff -r 7bd3f3b9dc76 sys/include/ape/stdio.h
--- a/sys/include/ape/stdio.h	Sun Jul 05 22:15:02 2020 +0200
+++ b/sys/include/ape/stdio.h	Sat Jul 11 08:44:06 2020 -0700
@@ -106,7 +106,7 @@
 #define	getchar()	getc(stdin)
 extern char *gets(char *);
 extern int putc(int, FILE *);
-#define	putc(c, f) ((f)->wp>=(f)->rp?_IO_putc(c, f):(*(f)->wp++=(c)&_IO_CHMASK))
+#define	putc(c, f) ((f)->wp>=(f)->rp?_IO_putc(c, f):((*(f)->wp++=(c))&_IO_CHMASK))
 extern int _IO_putc(int, FILE *);
 extern int putchar(int);
 #define	putchar(c)	putc(c, stdout)
diff -r 7bd3f3b9dc76 sys/include/stdio.h
--- a/sys/include/stdio.h	Sun Jul 05 22:15:02 2020 +0200
+++ b/sys/include/stdio.h	Sat Jul 11 08:44:06 2020 -0700
@@ -92,7 +92,7 @@
 #define	getchar()	getc(stdin)
 char *gets(char *);
 int putc(int, FILE *);
-#define	putc(c, f) ((f)->wp>=(f)->rp?_IO_putc(c, f):(*(f)->wp++=(c)&_IO_CHMASK))
+#define	putc(c, f) ((f)->wp>=(f)->rp?_IO_putc(c, f):((*(f)->wp++=(c))&_IO_CHMASK))
 int _IO_putc(int, FILE *);
 int putchar(int);
 #define	putchar(c)	putc(c, stdout)

Re: [FOUND] [9front] putc/fputc incorrect return

[tlaronde]

Hello,

On Sat, Jul 11, 2020 at 08:45:26AM -0700, ori@eigenstate.org wrote:
> > #include <u.h>
> > #include <stdio.h>
> > 
> > int
> > main(int argc, char *argv[])
> > {
> > 	FILE *fp;
> > 	short w;
> > 
> > 	if ( (fp = fopen("/tmp/bug.tfm", "wb")) == NULL ) {
> > 		(void)fprintf(stderr, "Unable to open '/tmp/bug.tfm' for writing.\n");
> > 		return 1;
> > 	}
> > 
> > 	/* Writing successively negative word on the file stream: bug.
> > 	 */
> > 	w = 0xffaa;
> > 	(void)fprintf(stderr, "\nWriting big endian '%#hx' on file stream returned"
> > 		" -> %#x %#x\n", w, putc(w >> 8, fp), putc(w, fp));
> > 
> > 	/* On stdout, no problem.
> > 	 */
> > 	(void)fprintf(stderr, "\nWriting big endian '%#hx' on stdout returned"
> > 		" -> %#x %#x\n", w, putc(w >> 8, stdout), putc(w, stdout));
> > 
> > 	return 0;
> > }
> 
> The type of the assignment ends up as char, so it
> gets sign extended. Masking after the assignment
> fixes it for me.
> 
> Here's a patch.
> 
> diff -r 7bd3f3b9dc76 sys/include/ape/stdio.h
> --- a/sys/include/ape/stdio.h	Sun Jul 05 22:15:02 2020 +0200
> +++ b/sys/include/ape/stdio.h	Sat Jul 11 08:44:06 2020 -0700
> @@ -106,7 +106,7 @@
>  #define	getchar()	getc(stdin)
>  extern char *gets(char *);
>  extern int putc(int, FILE *);
> -#define	putc(c, f) ((f)->wp>=(f)->rp?_IO_putc(c, f):(*(f)->wp++=(c)&_IO_CHMASK))
> +#define	putc(c, f) ((f)->wp>=(f)->rp?_IO_putc(c, f):((*(f)->wp++=(c))&_IO_CHMASK))
>  extern int _IO_putc(int, FILE *);
>  extern int putchar(int);
>  #define	putchar(c)	putc(c, stdout)
> diff -r 7bd3f3b9dc76 sys/include/stdio.h
> --- a/sys/include/stdio.h	Sun Jul 05 22:15:02 2020 +0200
> +++ b/sys/include/stdio.h	Sat Jul 11 08:44:06 2020 -0700
> @@ -92,7 +92,7 @@
>  #define	getchar()	getc(stdin)
>  char *gets(char *);
>  int putc(int, FILE *);
> -#define	putc(c, f) ((f)->wp>=(f)->rp?_IO_putc(c, f):(*(f)->wp++=(c)&_IO_CHMASK))
> +#define	putc(c, f) ((f)->wp>=(f)->rp?_IO_putc(c, f):((*(f)->wp++=(c))&_IO_CHMASK))
>  int _IO_putc(int, FILE *);
>  int putchar(int);
>  #define	putchar(c)	putc(c, stdout)

That was fast! Yes, this solves the problem for me.

I don't see what problem it could bring, so can you push the patch to
the source so that users can update?

Thank you for the work!

Best regards,
-- 
        Thierry Laronde <tlaronde +AT+ polynum +dot+ com>
                     http://www.kergis.com/
                       http://www.sbfa.fr/
Key fingerprint = 0FF7 E906 FBAF FE95 FD89  250D 52B1 AE95 6006 F40C

Re: [FOUND] [9front] putc/fputc incorrect return

[tlaronde]

Hello,

On Sat, Jul 11, 2020 at 08:45:26AM -0700, ori@eigenstate.org wrote:
> 
> The type of the assignment ends up as char, so it
> gets sign extended. Masking after the assignment
> fixes it for me.
> 
> Here's a patch.
> 
> diff -r 7bd3f3b9dc76 sys/include/ape/stdio.h
> --- a/sys/include/ape/stdio.h	Sun Jul 05 22:15:02 2020 +0200
> +++ b/sys/include/ape/stdio.h	Sat Jul 11 08:44:06 2020 -0700
> @@ -106,7 +106,7 @@
>  #define	getchar()	getc(stdin)
>  extern char *gets(char *);
>  extern int putc(int, FILE *);
> -#define	putc(c, f) ((f)->wp>=(f)->rp?_IO_putc(c, f):(*(f)->wp++=(c)&_IO_CHMASK))
> +#define	putc(c, f) ((f)->wp>=(f)->rp?_IO_putc(c, f):((*(f)->wp++=(c))&_IO_CHMASK))
>  extern int _IO_putc(int, FILE *);
>  extern int putchar(int);
>  #define	putchar(c)	putc(c, stdout)

On second thought, I think that the fix is not correct due to C
specification: we assign a potentially negative integer value (if the
call of putc doesn't reduce the int first to [0-255]) to a signed char,
so the result is "implementation defined"---it works now perhaps for
all arch but could cease to work later when modifying a compiler.

The MASK is here precisely to ensure the [0-255] value.

So I think: 

#define	putc(c, f) ((f)->wp>=(f)->rp?_IO_putc(c, f):(unsigned char)(*(f)->wp++=(c)&_IO_CHMASK))

would be better, letting the promotion to int be defined from unsigned
char and in fact documenting what is done.

What do you think?

Best regards,
-- 
        Thierry Laronde <tlaronde +AT+ polynum +dot+ com>
                     http://www.kergis.com/
                       http://www.sbfa.fr/
Key fingerprint = 0FF7 E906 FBAF FE95 FD89  250D 52B1 AE95 6006 F40C