Re: Bug in Cygwin strtod()

[Corinna Vinschen <vinschen at redhat dot com>]

On Dec 19 12:44, Christian Bruel wrote:
> Hello Corinna,
> 
> The current strtod patchlet doesn't look the posted patch. For instance
> in newlib 1.20, strtod.c:339 I see
> 
> if (nd + i < 10)
> 
> while in my patch it was
> 
> if (nd + 1 < 10)

It doesn't matter if the line contains nd + i or nd + 1.  The result
is always wrong.  Compare with the orig code:

Orig:
	for(i = 1; i < nz; i++) {
		if (nd++ <= DBL_DIG + 1) {
			if (nd < 10)
				[...]
	}
	if (nd++ <= DBL_DIG + 1) {
		if (nd < 10)
			[...]
	}

Yours:
	for(i = 1; i < nz; i++) {
		if (nd <= DBL_DIG + 1) {
			if (nd + i < 10)
				[...]
	}
	if (nd <= DBL_DIG + 1) {
		if (nd + 1 < 10)


Your code is doing very different tests.
Assuming DBL_DIG is 4, nd is 0 and nz is 3 when entering the loop:

		Orig:				Yours:

First loop:	if (0 <= 4 + 1) {		if (0 <= 4 + 1) {
			if (1 < 10)			if (1 < 10)

Second loop:	if (1 <= 4 + 1) {		if (0 <= 4 + 1) {
			if (2 < 10)			if (2 < 10)

Third loop:	if (2 <= 4 + 1) {		if (0 <= 4 + 1) {
			if (3 < 10)			if (3 < 10)

After leaving the loop:
		if (3 <= 4 + 1) {		if (0 <= 4 + 1) {
			if (4 < 10)			if (1 < 10)
						if (0 <= 4 + 1) {
		nd = 3;					nd = 3;

Assuming DBL_DIG is 4, nd is 4 and nz is 3 when entering the loop:

First loop:	if (4 <= 4 + 1) {		if (4 <= 4 + 1) {
			if (5 < 10)			if (5 < 10)

Second loop:	if (5 <= 4 + 1) {		if (4 <= 4 + 1) {
			if (6 < 10)			if (6 < 10)

Third loop:	if (6 <= 4 + 1) {		if (4 <= 4 + 1) {
			SKIP IF				if (7 < 10)

After leaving the loop:
		if (7 <= 4 + 1) {		if (4 <= 4 + 1) {
			SKIP IF				if (5 < 10)
						if (4 <= 4 + 1) {
		nd = 7;					nd = 7;

Something IS fishy here, and it's not really target dependent.

> For now on sh4, I can't notice the problem (but I use my version of the
> patch). Do you know if this is reproducible only on Cygwin ? It is
> easier for me to check on Linux. I'll try to reproduce on any target and
> give a status.

I can only test on Cygwin right now, but the code is target independent.
It's supposed to work on all targets.


Corinna

-- 
Corinna Vinschen
Cygwin Project Co-Leader
Red Hat