Paint crashes when using Brush tool
Pages: 1 2
Andy S (2979) 504 posts |
Nice work! I really appreciate your help, thanks Rick. First, it’s probably the LDR two instructions earlier that is choking Ah, that explains a lot. I didn’t know it does that. perhaps knowing the values of the registers may help? Do you have any backtrace, to know where in Paint it is failing? (more specifically than “the brush tool”)? Do you have the value of any registers at the point of failure? As mentioned previously, I don’t remember ever personally seeing Paint crash in SharedCLibrary, so I’m relying on whatever dumps other people may post in this thread. I have had various other crashes but to date the only repeatable ones have turned out (AFAICR) to be things I’d done wrong (I have recently been getting Illegal Window Handle apparently relating to a RISC_OSLib dbox I’ve added but perversely the errors went away as soon as I added more debug output; grrr). If it helps, I created a simple backtrace handler that dumps its output to file or screen (depending if file handle given). Yeah, that could be exceedingly useful, thank you. I’m only tinkering around the edges of debugging these crashes at the moment in between working on the other features. There’s a build option XTRACE in Paint that I leave off most of the time (because it spams megs and megs of output to the logs) but that does a lot of extra checks on the integrity of the memory (de)allocations so that might shed some more light on any problems as well. At some point, maybe at the end when I stop changing the rest of the code, I can hopefully pay full attention to figuring it out. |
Rick Murray (539) 13742 posts |
Exactly what I saw with Manga (with the added benefit of when it failed, it was a complete machine freeze!). You probably have something that’s writing beyond its buffer/allocation, and is trampling on something else. I’ll describe more later this evening, when I’m back home. |
Colin Ferris (399) 1797 posts |
Something like *MemoryI PC -24 +40 In a taskwindow Plus *Showregs After a abort. Also the utility Prog Addr Back pedals from an abort address to a Function name. Written by Justin Fletcher 0.12 [Updated] Any chance for updates to Addr Justin/Ian :-) |
Rick Murray (539) 13742 posts |
Unfortunately it is getting stuck in ROM. It would be better to try to understand how it ended up there. This is probably what you want: https://www.riscosopen.org/wiki/documentation/show/Debugger%20Annotated%20Exception%20Dumps Oh, and ensure things are built with function names embedded. There’s no excuse not to, these days. |
Andy S (2979) 504 posts |
but perversely the errors went away as soon as I added more debug output; grrr And when I undid the debugging changes, the errors still didn’t come back. You probably have something that’s writing beyond its buffer/allocation, and is trampling on something else. Probably. The hard part is spotting what it is and that’s assuming it’s even something in Paint’s code and not the OS or libraries. *MemoryI PC -24 +40 Regarding the new “Invalid window handle” error, I think “*where” just returned &00000000. |
Andy S (2979) 504 posts |
Also the utility Prog Was that bundled with RISC OS 4? |
Colin Ferris (399) 1797 posts |
No – but the author has been on this website under another name – Ian? |
David Pitt (3386) 1248 posts |
I have found a 32bit *addr 8f00 Area Free pool (inaccessible) has start within Zero page It looks more useful on OS4.02. Address &8f00 Application space, at &8000 offset &f00 (size &1bf8000) * I even found its help!!! Addr 0.12 (06 May 2003) *Addr is a debugging tool, used to describe an address in memory. It is intended for developers to locate problems and isolate faults. Syntax: *Addr <address> | -A <filename> | -a | -e | -m | -h Switches: -A Absolute file to search for address in -a Display address of last abort -e Display list of known entities (entry points, known data points) -m Display entire known memory -h Display help |
Rick Murray (539) 13742 posts |
As promised: the fun, frolics, and freaky headaches of a one byte overrun in reading a string. https://heyrick.eu/blog/index.php?diary=20200908 1 For those who don’t read my blog, Anna is a kitten. |
Colin Ferris (399) 1797 posts |
Interestingly to read about Gerf’s involvement with DDT. Perhaps he could explain what can and cannot be done with the Prog. DDT seems to written in ‘C’ what’s the latest version number? The version I have seems to query zero page. |
Chris Mahoney (1684) 2160 posts |
I believe 1.98 is the latest. |
Andy S (2979) 504 posts |
I have had various other crashes but to date the only repeatable ones have turned out (AFAICR) to be things I’d done wrong (I have recently been getting Illegal Window Handle apparently relating to a RISC_OSLib dbox I’ve added but perversely the errors went away as soon as I added more debug output; grrr). Turns out that particular Illegal Window Handle was my fault as well. I wasn’t checking whether the tool window had been closed before trying to disable the opacity slider. |
Andy S (2979) 504 posts |
I’m hesitant to say this, but we may, just maybe, be on the verge of a breakthrough with this brush bug! ROOL very kindly saved me some crash dumps from when Paint crashed using the brush tool while they were working with some screenshots. Luckily, this time with their help we were able to narrow down the location of the crash to near the start of psprite_drop_translation(). This function is responsible for deleting translation tables both for the sprite and the brush tool.
I believe the crash is occurring on the line There are two translation tables that could be relevant here, Well, the toolspace is an array of integers that the code for each of Paint’s tools uses as a sort of scratch space. Significantly, this means the values in the toolspace have different meanings depending on which tool is selected. In the case of the brush tool, Annoyingly I have still never, ever been able to reproduce this crash on my systems, but my current best guess by inspecting the code is that psprite_drop_translation() is being called with an invalid pointer in Ordinarily, when the brush tool’s selected, The way the code is supposed to work is that if a sprite has no windows open, its toolspace is left alone. As soon as you open a window onto that sprite, the toolspace is then supposed to be initialised appropriately for the current tool. The fact that each sprite has its own toolspace whereas the current tool itself is global for all of Paint makes things more complicated, especially if you’re switching between multiple sprites and using multiple tools. It means there are likely some edge cases that can trigger the crash. I’m thinking this sort of thing might cause a crash sometimes:
1. Select the camera tool and maybe use it a bit. Deleting the sprite may cause a crash because even though the brush tool is selected, the sprite never got the translation table pointer assigned in its toolspace, because it had no sprite windows open. Here’s why the deletion of a sprite in those circumstances can cause a crash, in the code for psprite_delete():
That code doesn’t check whether “sprite” has any windows open. If it doesn’t, Now obviously the above steps describe more of a “crash on sprite deletion” rather than a “crash on brush selection” and psprite_delete() isn’t called by the brush code but it illustrates the dangers of leaving There’s another situation where My plan of action is to make sure that That’s enough for now, methinks. |
Rick Murray (539) 13742 posts |
[…]
What sort of RISC OS are you using? High vector or low? If high vector, do you have the compatibility page enabled? In other words, does this work in BASIC: PRINT !0 If it works, and you have a high vector build, then: SYS "OS_Memory", 20, 0 will turn it off so zero page addresses cause a crash. Together with either ZeroPain or the Debugger annotated dump, it can help you see what’s been going on. It’s
Of course, that code is a particular sort of awful. I mean, it’s saying “if tb isn’t a null pointer than let’s pre-decrement it and then go look at an array element of something else”. WTF? |
Andy S (2979) 504 posts |
Of course, that code is a particular sort of awful. I mean, it’s saying “if tb isn’t a null pointer than let’s pre-decrement it and then go look at an array element of something else”. Is it??? I’ll go and revise operator precedence. I can tell you what the programmer intended it to do. They intended it to pre-decrement the refcount integer variable and then test if it’s 0. If, as you say, it’s decrementing the “*tb” pointer, then that in itself is a terrible bug! EDIT: My understanding is the programmer was correct as the SYS “OS_Memory”, 20, 0 On my RPCEmu, PRINT !0 still works after running the above call. Also, where can I download ZeroPain? |
Stuart Painting (5389) 707 posts |
It’s in the Bonus binaries package. |
Dave Higton (1515) 3477 posts |
I think that’s an example of trying to do too much in one line. |
Chris Mahoney (1684) 2160 posts |
If my memory’s correct (and it may not be!), the zero page relocation only occurs on newer hardware (where the Risc PC and therefore RPCEmu are “old” and the Raspberry Pi is “new”). |
Andy S (2979) 504 posts |
Ah thanks Chris, because I just checked RiscOS/BuildSys/-/blob/master/Components/ROOL/IOMD32 and there’s no kernel option specified for HiProcVecs specified there (I’ve just been reading Rick’s blog about all this). ; ) I’ll try it again on my Pi later, though that’s running RISC OS Direct at the moment which may be built vector low – I don’t know. I never get the brush crashes on my Pi either though FWIW but I’m beginning to suspect it’s something to do with the way other people are using Paint, in a way that never occurs to me! |
Rick Murray (539) 13742 posts |
Ah, that’s too old, it won’t support vector relocation. As far as I’m aware, VBAR was introduced in ARMv7?
Now that the compatibility page exists and shonky stuff doesn’t randomly drop dead, there’s no reason to use low vector any more. I’ve been running high vector builds for about two years now. |
Andy S (2979) 504 posts |
Anyway, exciting news: I believe I have found the smoking g__ – erm, let’s just call it the smoking non-violent defence accessory – you have to watch what you say these days! I had another look at a couple of ROOL’s crash dumps where it crashed in psprite_drop_translation() and I saw they still had a return address in R14. I subtracted the address of the crash from that to try to find the offset from there to the function that had called psprite_drop_translation(). I looked at that offset in a dump of the ROM I had saved and from my calculations that puts it as psprite_free_brush_blocks(). There was a slight oddity in that the BL that actually calls psprite_drop_translation() seemed to be about eight instructions further down, so I don’t know if my arithmetic was a bit off or some oddity with the ROM, or what? I don’t think it matters too much though because the good news is that psprite_free_brush_blocks() gets called a lot. Most notably, it’s called when a brush is first selected (in the code from the old OK button) and since I added the Shape / Stamp / Tint options with the transfer function, it even gets called during the brush plot code, for example when the colour used with Shape or Tint is changed. And, more importantly,
What that’s doing is it’s looping through all the sprite file windows and for each one looping through every sprite in that file and trying to drop what it assumes is a translation table at I hope that’s what’s been going wrong all these years because if so, I should be able to get a fix together. For completeness I still want to see the crash happening on one of my systems. I’ll have a go on the Pi but otherwise it may come to popping up a message box if the ttab pointer is less than &8000! |
Rick Murray (539) 13742 posts |
😋 Different languages have different precedence rules, and it’s a pain to re-enter all of it. Brackets are your friends! |
Andy S (2979) 504 posts |
For completeness I still want to see the crash happening on one of my systems. I’ll have a go on the Pi but otherwise it may come to popping up a message box if the ttab pointer is less than &8000! For testing on RPCEmu I’ve just coded that check for 1. Run Paint and make a new (smallish) sprite. If anyone’s a little bored and running a high vector RISC OS, would you like to try the above steps and tell us if it explodes? Sometimes using the scissor tool isn’t even needed but it’s a guaranteed way to write a coordinate into the first sprite’s |
Stuart Painting (5389) 707 posts |
I got as far as item 5 before getting "Unrecoverable internal error (abort on data transfer at &FC43C468). I’ll try again as I didn’t perform exactly the commands you specified. Second attempt (being careful to do the exact things you specified) gave “abort on data transfer at &FC43C460”, again at item 5. Raspberry Pi 4B, RISC OS 5.28 (16 December 2020). |
Andy S (2979) 504 posts |
I got as far as item 5 Thanks. I’d call that a repro. I’m not 100% sure why it might try to drop translation tables at step 5. I think it would do it if you were painting in a colour other than black or if you changed any of the brush settings, chose a different brush sprite or also if you swapped from the brush tool to another tool and back. |
Pages: 1 2