Re: Base - color pparsing bug - Was: [Re: GNUstep releases this month?]
> Am 20.01.2021 um 20:13 schrieb Riccardo Mottola : > > Hi, > > Fred Kiefer wrote: >>> I will try to bisect - but I'd liek to track this down before a release, >>> especially knowing it is something done these last months. >> If this is really a recent bug, not something that went unnoticed for years >> as nobody uses SPARC processors any more, than the only possible change >> would be the float parsing change that Richard just made. No idea why this >> would fail on something as simple as „1“ or „0“ but this is SPARC specific >> so you probably should start of by using the test suit of base on that >> machine. > > > Yes... I confirm it is a recent commit and I tracked it down to this commit: > > > r23416 | richard.frith.macdonald | 2020-12-30 12:58:19 +0100 (Wed, 30 > Dec 2020) | 2 lines > > Remove internal GSScanInt and GSScanDouble functions after moving > functionality into the -scanDouble: method. Add private class method to > support scanning a string for a double without having to create a new > scanner each time (use a shared lock-protected, instance with resetting > of the string being scanned). > > before this, all color look fine. > > I'm running the tests to see if some specific failed tests can be > identified ... but apparently they run sequentially on only one CPU and > it takes hours > > Anyway having an exact commit is also a good start. OK, so it is this code in [NSColor colorFromString:] double r, g, b; NSScanner *scanner = [[NSScanner alloc] initWithString: str]; if ([scanner scanDouble: &r] && [scanner scanDouble: &g] && [scanner scanDouble: &b] && [scanner isAtEnd]) In a previous mail you stated that str is something like „1 1 1“ or „0 0 0“. The next step would be to write a short test program that feeds these strings to the code above and outputs the values for r, g and b and of course the result of isAtEnd. That should make it easy to track down the issue. But you are the only one with a SPARC processor.
Re: Base - color pparsing bug - Was: [Re: GNUstep releases this month?]
> On 20 Jan 2021, at 19:48, Fred Kiefer wrote: > > > >> Am 20.01.2021 um 20:13 schrieb Riccardo Mottola : >> >> Hi, >> >> Fred Kiefer wrote: I will try to bisect - but I'd liek to track this down before a release, especially knowing it is something done these last months. >>> If this is really a recent bug, not something that went unnoticed for years >>> as nobody uses SPARC processors any more, than the only possible change >>> would be the float parsing change that Richard just made. No idea why this >>> would fail on something as simple as „1“ or „0“ but this is SPARC specific >>> so you probably should start of by using the test suit of base on that >>> machine. >> >> >> Yes... I confirm it is a recent commit and I tracked it down to this commit: >> >> >> r23416 | richard.frith.macdonald | 2020-12-30 12:58:19 +0100 (Wed, 30 >> Dec 2020) | 2 lines >> >> Remove internal GSScanInt and GSScanDouble functions after moving >> functionality into the -scanDouble: method. Add private class method to >> support scanning a string for a double without having to create a new >> scanner each time (use a shared lock-protected, instance with resetting >> of the string being scanned). >> >> before this, all color look fine. >> >> I'm running the tests to see if some specific failed tests can be >> identified ... but apparently they run sequentially on only one CPU and >> it takes hours >> >> Anyway having an exact commit is also a good start. > > > OK, so it is this code in [NSColor colorFromString:] > > > doubler, g, b; > NSScanner *scanner = [[NSScanner alloc] initWithString: str]; > > if ([scanner scanDouble: &r] && > [scanner scanDouble: &g] && > [scanner scanDouble: &b] && > [scanner isAtEnd]) > > In a previous mail you stated that str is something like „1 1 1“ or „0 0 0“. > The next step would be to write a short test program that feeds these strings > to the code above and outputs the values for r, g and b and of course the > result of isAtEnd. That should make it easy to track down the issue. But you > are the only one with a SPARC processor. Looks interesting ... do we have the data this is failing on anywhere? If I can get a testcase to debug I can see what's going on.
Re: Base - color pparsing bug - Was: [Re: GNUstep releases this month?]
> On 21 Jan 2021, at 14:10, Richard Frith-Macdonald > wrote: > > > >> On 20 Jan 2021, at 19:48, Fred Kiefer wrote: >> >> >> >>> Am 20.01.2021 um 20:13 schrieb Riccardo Mottola >>> : >>> >>> Hi, >>> >>> Fred Kiefer wrote: > I will try to bisect - but I'd liek to track this down before a release, > especially knowing it is something done these last months. If this is really a recent bug, not something that went unnoticed for years as nobody uses SPARC processors any more, than the only possible change would be the float parsing change that Richard just made. No idea why this would fail on something as simple as „1“ or „0“ but this is SPARC specific so you probably should start of by using the test suit of base on that machine. >>> >>> >>> Yes... I confirm it is a recent commit and I tracked it down to this commit: >>> >>> >>> r23416 | richard.frith.macdonald | 2020-12-30 12:58:19 +0100 (Wed, 30 >>> Dec 2020) | 2 lines >>> >>> Remove internal GSScanInt and GSScanDouble functions after moving >>> functionality into the -scanDouble: method. Add private class method to >>> support scanning a string for a double without having to create a new >>> scanner each time (use a shared lock-protected, instance with resetting >>> of the string being scanned). >>> >>> before this, all color look fine. >>> >>> I'm running the tests to see if some specific failed tests can be >>> identified ... but apparently they run sequentially on only one CPU and >>> it takes hours >>> >>> Anyway having an exact commit is also a good start. >> >> >> OK, so it is this code in [NSColor colorFromString:] >> >> >> double r, g, b; >> NSScanner *scanner = [[NSScanner alloc] initWithString: str]; >> >> if ([scanner scanDouble: &r] && >>[scanner scanDouble: &g] && >>[scanner scanDouble: &b] && >>[scanner isAtEnd]) >> >> In a previous mail you stated that str is something like „1 1 1“ or „0 0 0“. >> The next step would be to write a short test program that feeds these >> strings to the code above and outputs the values for r, g and b and of >> course the result of isAtEnd. That should make it easy to track down the >> issue. But you are the only one with a SPARC processor. > > Looks interesting ... do we have the data this is failing on anywhere? > If I can get a testcase to debug I can see what's going on. Never mind, I found/fixed it. Rather than waiting to find out what 'something like' meant exactly, I just tried testing with 1 1 1 and with 0 0 0 The problem was the second case ... I had placed a check in the code intended to avoid assigning the result of the scan via a nul pointer, but rather than checking the pointer i checked the result. That would mean that, in the case where the scanned value was zero, the result was not returned and the memory pointed to was undefined.
Re: Base - color pparsing bug - Was: [Re: GNUstep releases this month?]
Hi, Fred Kiefer wrote: > In a previous mail you stated that str is something like „1 1 1“ or „0 0 0“. > The next step would be to write a short test program that feeds these strings > to the code above and outputs the values for r, g and b and of course the > result of isAtEnd. That should make it easy to track down the issue. But you > are the only one with a SPARC processor I prepared a test containing this: str = @"1 1 1"; scanner = [[NSScanner alloc] initWithString: str]; [scanner scanDouble: &r]; [scanner scanDouble: &g]; [scanner scanDouble: &b]; isAtEnd = [scanner isAtEnd]; NSLog(@"parsed: %lf, %lf, %lf - isAtEnd? %d", r, g, b, isAtEnd); [scanner release]; str = @"0 0 0"; scanner = [[NSScanner alloc] initWithString: str]; [scanner scanDouble: &r]; [scanner scanDouble: &g]; [scanner scanDouble: &b]; isAtEnd = [scanner isAtEnd]; NSLog(@"parsed: %lf, %lf, %lf - isAtEnd? %d", r, g, b, isAtEnd); [scanner release]; does this look reasonable? it looks to me. On intel I get an unexpected result: 2021-01-21 14:09:20.967 ScannerTest[26541:26541] parsed: 1.00, 1.00, 1.00 - isAtEnd? 1 2021-01-21 14:09:20.968 ScannerTest[26541:26541] parsed: 1.00, 1.00, 1.00 - isAtEnd? 1 which is consistent with SPARC: 2021-01-21 16:02:32.466 ScannerTest[5278:3930190648] parsed: 1.00, 1.00, 1.00 - isAtEnd? 1 2021-01-21 16:02:32.555 ScannerTest[5278:3930190648] parsed: 1.00, 1.00, 1.00 - isAtEnd? 1 but why not 0.0 the second time? Riccardo
Re: Base - color pparsing bug - Was: [Re: GNUstep releases this month?]
Hi! Richard Frith-Macdonald wrote: > Never mind, I found/fixed it. Rather than waiting to find out what > 'something like' meant exactly, I just tried testing with 1 1 1 and with 0 0 0 > The problem was the second case ... I had placed a check in the code intended > to avoid assigning the result of the scan via a nul pointer, but rather than > checking the pointer i checked the result. > That would mean that, in the case where the scanned value was zero, the > result was not returned and the memory pointed to was undefined. oh.. I was still testing while you fixed it! You are one of the best ones! Tests on Intel: 2021-01-21 15:17:27.353 ScannerTest[3312:3312] parsed: 1.00, 1.00, 1.00 - isAtEnd? 1 2021-01-21 15:17:27.354 ScannerTest[3312:3312] parsed: 0.00, 0.00, 0.00 - isAtEnd? 1 Tests on Sparc: 2021-01-21 16:19:47.859 ScannerTest[20968:3930190728] parsed: 1.00, 1.00, 1.00 - isAtEnd? 1 2021-01-21 16:19:47.913 ScannerTest[20968:3930190728] parsed: 0.00, 0.00, 0.00 - isAtEnd? 1 once again.. the code was general, but SPARC was more sensitive I was proposing to add it to the test suite, but I saw you did with a later commit. Does it make sense to have both 1 & 0 as tests? I also confirm that now interface colors come up correctly on both SPARC32-bit and SPARC64bit. With Fred's fixes in GUI... SPARC64 is working fine again, I can launch several complex applications like GWorkspace and PRICE without spotting gross errors. Riccardo
