ZeroPain in FileSwitch

Got the following ZeroPain after clicking on “add Game…” in s!ScummVM RPi 3 (OS 5.25 15 Aug 2018)

Time: Thu Aug 23 19:07:27 2018
Location: Offset 00002374 in module FileSwitch
Current Wimp task: scummvm
Last app to start: Fat32fs::Fatty.$.Diversions.Games.ScummVM.!ScummVM.scummvm >Boot:Choices.ScummVM.stdout 2>Boot:Choices.ScummVM.stderr

R0 = 0227b33c R1 = 0227b33c R2 = 00000001 R3 = 00000000
R4 = 00000580 R5 = 00000000 R6 = 00000000 R7 = fc053c2c
R8 = 0227af10 R9 = 00000001 R10 = 300ef57c R11 = 00000000
R12 = 200001d4 R13 = fa207f34 R14 = fc14969c R15 = fc04a414
DFAR = 00000001 Mode SVC32 Flags nzCv if PSR = 20000113

fc04a3cc : 1354001f : CMPNE R4,#&1F ; =31
fc04a3d0 : 93a04000 : MOVLS R4,#0
fc04a3d4 : e4d25001 : LDRB R5,[R2],#1
fc04a3d8 : e33e0101 : TEQ R14,#&40000000
fc04a3dc : 17de5005 : LDRNEB R5,[R14,R5]
fc04a3e0 : e355007f : CMP R5,#&7F ; =127
fc04a3e4 : 1355001f : CMPNE R5,#&1F ; =31
fc04a3e8 : 93a05000 : MOVLS R5,#0
fc04a3ec : e1540005 : CMP R4,R5
fc04a3f0 : 18bd803e : LDMNEIA R13!,{R1-R5,PC}
fc04a3f4 : e3540000 : CMP R4,#0
fc04a3f8 : 1affffec : BNE &FC04A3B0
fc04a3fc : e8bd803e : LDMIA R13!,{R1-R5,PC}
fc04a400 : e92d4c1e : STMDB R13!,{R1-R4,R10,R11,R14}
fc04a404 : e59ce044 : LDR R14,[R12,#68]
fc04a408 : e3a0b000 : MOV R11,#0
fc04a40c * e4d23001 * LDRB R3,[R2],#1
fc04a410 : e333002a : TEQ R3,#&2A ; =“*”
fc04a414 : 0a000011 : BEQ &FC04A460
fc04a418 : e4d14001 : LDRB R4,[R1],#1
fc04a41c : e3340000 : TEQ R4,#0
fc04a420 : 0a00000b : BEQ &FC04A454
fc04a424 : e33e0101 : TEQ R14,#&40000000
fc04a428 : 17de3003 : LDRNEB R3,[R14,R3]
fc04a42c : e33e0101 : TEQ R14,#&40000000
fc04a430 : 17de4004 : LDRNEB R4,[R14,R4]
fc04a434 : e1330004 : TEQ R3,R4
fc04a438 : 13330023 : TEQNE R3,#&23 ; =“#”
fc04a43c : 0afffff2 : BEQ &FC04A40C
fc04a440 : e1a0100a : MOV R1,R10
fc04a444 : e1b0200b : MOVS R2,R11
fc04a448 : 1a000004 : BNE &FC04A460

fc04a400 : e92d4c1e : STMDB R13!,{R1-R4,R10,R11,R14}
fc04a404 : e59ce044 : LDR R14,[R12,#68]
fc04a408 : e3a0b000 : MOV R11,#0
fc04a40c * e4d23001 * LDRB R3,[R2],#1

This is function WildMatch with a null pointer for the wildcarded name. It’s most likely given you pressed a button to add a game, which could involve enumerating a directory, that ScummVM is the origin of the error. What FS are you using? The ResolveWildcardBySteam function is the only caller of WildMatch I can see that doesn’t carefully check R2 for NULL before calling.

The FS is irrelevant, I was trying to set a different default path, it is just that I had not set it correctly. It should be /::.$/folder/subfolder/… while I used . as folder seperator.

The ResolveWildcardBySteam function is the only caller of WildMatch I can see that doesn’t carefully check R2 for NULL before calling.

Which I think is similar to the cause of the errors being reported when SyncDiscs does a compare and people report a zeropain error from FilerAction. As I said in the SyncDiscs thread I think SyncDiscs is feeding incorrect values to the OS in certain circumstances.

The real question on this is should the OS check and refuse if the values are incorrect or should we insist that application authors have to fix all their code?
The former would force correct use by not working when the API is incorrectly used but then people might throw the proverbial teddy that the OS “change” broke their software.

The real question on this is should the OS check and refuse if the values are incorrect or should we insist that application authors have to fix all their code?

Why not both?

If the program is passing in a bogus wildcard / path then regardless of whether the SWI returns an error, I doubt the SWI is going to be doing what the programmer intended.

The real question on this is should the OS check and refuse if the values are incorrect

Yes, of course. Especially if it would result in an illegal memory access.

or should we insist that application authors have to fix all their code?

The code cannot do anything sensible anyway, so it needs o be fixed anyway.

Ah, Jeffrey says the same. Good :-)

the proverbial teddy that the OS “change” broke their software

I’m still trying to work out what OS change (in 5.25) caused the HidePtr module to go nuts. It’s a pretty moot point (HidePtr isn’t exactly essential), but it does hidelight how changes can have…consequences in unexpected places.

The former would force correct use by not working when the API is incorrectly used

Define “API”.

What the nearly thirty year old documentation says even if it doesn’t match what the firmware actually does,
What the firmware actually does (and has done for nearly thirty years) even if this doesn’t match the documention of what it’s supposed to do,
Some random mixture of the above two choices that’ll probably land you in a recursive loop if you think about it too hard.

[and this will be the moment nemo thinks “oh, Rick finally gets it” ;-) ]

Especially if it would result in an illegal memory access.

And all the other SWIs within RISC OS that take pointers? ;-)

I’m not sure the OS needs to be changed. My understanding of this is that FileSwitch is causing a zeropain issue because something is providing it with a rubbish pointer. In keeping with RISC OS in general, it tries to do something with the bad pointer, and obviously fails. But the fault is neither RISC OS nor FileSwitch. It’s the software providing the bad pointer. Fix that, and the problem is resolved. Fix FileSwitch, then it will cope better with bad pointers but the broken program will still be broken.

Which I think is similar to the cause of the errors being reported when SyncDiscs does a compare and people report a zeropain error from FilerAction. As I said in the SyncDiscs thread I think SyncDiscs is feeding incorrect values to the OS in certain circumstances.

Could I reinterate that as far as I know, SyncDiscs does not raise zero pain errors when comparing directories or synchronising directories. I have done a lot of testing recently trying to provoke such errors, and have seen no entries in the log. If people are seeing zero pain log entries involving SyncDiscs, then I hope they will (a) tell me and (b) provide a copy of the log.

I have also added some code to deal with situations where a network filing system is either not present or the disc is not mounted, etc. This is mainly to prevent SyncDiscs stalling when a job is initiated at some specified time when the machine is unattended, rather than for zero pain. I will put the latest version up on my site.

And all the other SWIs within RISC OS that take pointers? ;-)

There’s this guy called Rick Murray that has in the past suggested the OS should be more robust…

It’s the software providing the bad pointer. Fix that, and the problem is resolved. Fix FileSwitch, then it will cope better with bad pointers but the broken program will still be broken.

The point is while the OS accepts the incorrect input, albeit with a bad reaction of varying degree in different instances, authors don’t fix the software.
Didn’t someone called Rick Murray argue for an OS version where zero page accesses didn’t routinely produce an error log?

Why not both?

If the program is passing in a bogus wildcard / path then regardless of whether the SWI returns an error, I doubt the SWI is going to be doing what the programmer intended.

Well, that would be ideal if you can persuade all authors to accept that when the change results in their software not working it’s not the fault of the OS. Given the reaction I’ve seen in recent years to (medical) software with a web front end breaking because MS fixed security holes in IE that their¹ software relied on I’d say there may be a few “spirited discussions”. This is a smaller pond.

¹ GE Medical first among others.

suggested the OS should be more robust…

Indeed it should be, but I’d be first concentrating on design flaws (such as code and data strewn across the RMA).

authors don’t fix the software.

That requires complaints from users that the software doesn’t work correctly? Some zero pain problems are innocuous – for example in a certain DTP package it liked to check if an array was null or if an element of that array was null… leading to an unnecessary check of an offset of a null pointer. That’s innocuous.
Passing null pointers to system SWIs…that’s less innocuous in a “how did that ever work” sense. But then maybe it did work and something else is tripping it up?

Let’s wait until this one is unwound a little more. What we know is that FileSwitch is getting a null pointer for a wildcarded name…

Didn’t someone called Rick Murray argue for an OS version where zero page accesses didn’t routinely produce an error log?

Yup. A fake zero page. Which, these days, is one of the options for people who just want to run the machine like before and have their software run like before (even if it’s a creeping cesspit of bugs) because, as you said:

if you can persuade all authors to accept that when the change results in their software not working it’s not the fault of the OS.

End users, too, if it’s something unlikely to be further developed.

GE Medical first among others.

GE? As in the makers of the minigun? The oil for food thing? The “let’s dump radioactive waste all over the place and see what the effect is on civilians”? The “let’s make your balls glow in the dark” (Walla Walla prison)? The let’s dump toxic waste in the river (multiple locations)? If that’s the GE we’re talking about, it’s probably a good thing if their software doesn’t work…

Which I think is similar to the cause of the errors being reported when SyncDiscs does a compare and people report a zeropain error from FilerAction. As I said in the SyncDiscs thread I think SyncDiscs is feeding incorrect values to the OS in certain circumstances.

I rechecked the thread, and was a bit puzzled why the thread sort of fizzled out without any mention of a fix. However, checking back in my archived emails with the user who had the problems reminded me a bit more about the resolution. To cut a long story short, it was due to power supply problems to the board and accessories. The supply was loaded ‘to the limit’. Once this was addressed, the zero pain problems involving SyncDiscs immediately disappeared. It would appear that both USB and SSDs draw extra current when being worked hard. Applications such as SyncDiscs obviously exercise the drive(s) continuously so power supply problems are more likely to show up.

I am not sure why the resolution of that particular problem was never posted back to the forum.

Some zero pain problems are innocuous – for example in a certain DTP package it liked to check if an array was null or if an element of that array was null… leading to an unnecessary check of an offset of a null pointer.

You’re kind of supposed to do that, are you not…? In a sensible language (ie. not BASIC), that kind of OR won’t even bother testing the second “index of” clause if the initial “is the pointer NULL?” test fails.

I thought the problem was that a certain DTP package had the clauses the wrong way around: test the index, then test the pointer? The overall result is the same, but the pointer is always dereferenced first – even if the second test then shows it to be NULL and causes the subsequent code to be skipped.

What we know is that FileSwitch is getting a null pointer for a wildcarded name…

Might be interesting (and possibly useful) if Fileswitch (for one) were rewritten to check the input for naff values and available as a softload to see what software breaks.

If that’s the GE we’re talking about

Yes. The GE with fingers in so many pies that apparently employees are provided with a catalogue of goods and services they can get from the different sections.

In a sensible language (ie. not BASIC), that kind of OR won’t even bother testing the second “index of” clause if the initial “is the pointer NULL?” test fails.

Ah, but I define “sensible language” as one to provide the developer with an easy way to either enforce full evaluation or else as short-circuit.

Unfortunately, this reduces the number of sensible languages available a lot…hint: one of them uses “or else”, another “or_else” :-)

BASIC isn’t the only language that doesn’t short circuit; Pascal is similar. ISTR the same applies to Fortran.

Don’t know what languages Steffen is thinking of, but Visual Basic has OrElse (and AndAlso), as well as And and Or; that makes it a sensible language!

Visual Basic has OrElse (and AndAlso), as well as And and Or

A syntax that might be implemented in BBC BASIC is OR ELSE with AND IF perhaps. AND WHEN and AND WHILE are altenatives that occur to me. None of these are possible the way Basalt works, but you can have LOGOR() and LOGAND() functions to do the same. (Logical or/and).

However, it is quite simple to implement the functionality of these shortcuts in BBC BASIC for branching, so does that make it “a sensible language” or not? ;-)

Don’t know what languages Steffen is thinking of

Ada uses “and then” and “or else”, DEC Pascal (which was internally a kind of Ada with a Pascal parser bolted on) uses “and_then” and “or_else” to keep ISO Pascal compatibility.

Of course both “full” and “short circuit” variants are easily “simulated” with other language elements if not directly available, so the “sensible language” comment is more tongue-in-cheek. I was often surprised how many seasoned developers using a specific language did not know about those semantics of the language they were using! And if you are switching languages often, you really have to think about it every time to avoid easy mistakes.