scanf/printf is/are broken

[mheyer32]

Likely scanf is the offender as ScummVM seems not to be able to parse the ini file correctly,

This was introduced with commit 396a8e85029ecfd158a3ef0f1af492cb3a958bae

Maybe I can dig a bit deeper tomorrow.

[bebbo]

These are the current results for libc-test:

Test	newlib	libnix	clib2	gcc/cygwin
sscanf	3	0	-(39)	3
snprintf	0	0	-(39)	1

[mheyer32]

Have you tried running the test with (winuae)enforcer? Maybe the functions themselves are mostly ok but write past buffer ends?

[mheyer32]

Its still not working. I'm looking at a suspect here: https://github.com/bebbo/libnix/commit/396a8e85029ecfd158a3ef0f1af492cb3a958bae#r32536252

There's also a similar minor change that would limit the exponent in a floating point number to 100 - but I don't think this is relevant.

[bebbo]

I'm still missing the info what's broken...

• scummvm reads the ini and lists the game [sq1sci].
• SQ1 loads and seems to playable (I'm not good at playing it yet)
• no enforcer hits
• no winuaeenforcer hits
• 0x with an invalid numer must not continue parsing, the break exits the scanf loop, whilest a missing x is valid.
• the exponent is not limited to 99, it's valid to provide any exponent.

[mheyer32]

So what happens here: DiscWorld will not load, claiming there's no game data SC1 will start, load for a while and then crash with a std::bad_alloc(), followed by the TimerManager thread crashing.

(The TimerManager crash is probably just a bad side effect, not an actual problem. I believe its because the main program aborts(), its code and data goes out the window and this pulls the rug under the TimerManager process. Maybe I can install some form of abort() handler and try tearing down the extra processes there?).

Try the following. Configure to 'full debug' ./configure --host=m68k-amigaos --disable-all-engines --enable-engine=tinsel,sci --disable-mt32emu --enable-debug --disable-optimizations --disable-hq-scalers --with-amiga-prefix=/media/sf_Amiga/ScummVM --disable-translation --enable-c++11 --disable-updates

Force the debug output level to something high:

OSystem_AmigaOS3::OSystem_AmigaOS3() {
     gDebugLevel = 11;

Add some debug output to Scumm's vformat()

String String::vformat(const char *fmt, va_list args) {

...
            // vsnprintf didn't have enough space, so grow buffer
        printf("String::vformat allocates %d bytes for format \"%s\"\n", len, fmt);
        output.ensureCapacity(len, false);
}

Then ScummVM will not even start properly and immediately abort with bad_alloc :-)

[mheyer32]

It looks like vsnprintf() doesn't like it when there's no actual format thingies in the format string:

The below program prints a pretty large number :-)

static void myPrint(const char* fmt) {
   char str[32];
   int len = vsnprintf(str, sizeof(str), fmt, NULL);
   printf("%d", len); fflush(stdout);
}

int main(int argc, char ** argv) {
    myPrint("hjjjjjjjjjjjashjashjkahsjkahslajhsjkahsjlkhslajhsajklhsajklhs");
  return 0;
}

[bebbo]

followed by the TimerManager thread crashing.

guess there is no atexit() handling which shuts it properly down

[bebbo]

It looks like vsnprintf() doesn't like it when there's no actual format thingies in the format string: The below program prints a pretty large number :-)

61 is not that large!?

[bebbo]

do you still use features like -fshort-enums to build your lib or so?

[bebbo]

followed by the TimerManager thread crashing.

guess there is no atexit() handling which shuts it properly down

diff --git a/engines/sci/graphics/palette.cpp b/engines/sci/graphics/palette.cpp
index bc0b8485d2..821b5c6c29 100644
:
+++ b/backends/platform/amigaos3/amigaos3-main.cpp
@@ -171,6 +171,18 @@ static void load_libraries(void) {
        }
 }

+void cleanup() {
+       // Delete OSystem
+       if (g_system) {
+               delete (OSystem_AmigaOS3 *)g_system;
+       }
+
+       if (wbClosed) {
+               OpenWorkBench();
+       }
+
+}
+
 __stdargs int main(int argcWb, char const * argvWb[]) {
        load_libraries();

@@ -182,10 +194,12 @@ __stdargs int main(int argcWb, char const * argvWb[]) {
        int audioThreadPriority = DEFAULT_AUDIO_THREAD_PRIORITY;
        int closeWb = 0;

-       struct Process* proc = (struct Process*)FindTask(NULL);
-       printf("Process stack size is %ld bytes.\n", proc->pr_StackSize);
-       if (proc->pr_StackSize < 100000) {
-               printf("Process stack size is smaller than 100000. Exiting.\n");
+       struct Task * task = FindTask(NULL);
+       unsigned ss = (char*)task->tc_SPUpper - (char*)task->tc_SPLower;
+       struct Process* proc = (struct Process*)task;
+//     printf("Process stack size is %ld bytes.\n", ss);
+       if (ss < 100000) {
+               printf("Process stack size is smaller than 100000. Exiting. %d\n", ss);
                exit(EXIT_FAILURE);
        }

@@ -218,6 +232,7 @@ __stdargs int main(int argcWb, char const * argvWb[]) {
        OSystem_AmigaOS3 *amigaOsSystem = new OSystem_AmigaOS3();
        g_system = amigaOsSystem;
        assert(g_system);
+       atexit(cleanup);

        // Pre initialize the backend
        amigaOsSystem->init(audioThreadPriority);
@@ -226,17 +241,9 @@ __stdargs int main(int argcWb, char const * argvWb[]) {
                wbClosed = CloseWorkBench();
        }

+
        // Invoke the actual ScummVM main entry point:
        int res = scummvm_main(argc, argv);

-       // Delete OSystem
-       if (g_system) {
-               delete amigaOsSystem;
-       }
-
-       if (wbClosed) {
-               OpenWorkBench();
-       }
-
        return res;
 }

[mheyer32]

do you still use features like -fshort-enums to build your lib or so?

No, I tried with and without, both result in the same output. So for you above example did produce correct results?

I just did a clean, clean-prefix, all, all-sdk rebuild and the example code above is still producing a weird number.

[bebbo]

I only added the missing #include to your code

> m68k-amigaos-gcc -Os vpr.c -o vpr -noixemul
>  vamos vpr
61

also compiled it with the downloadable setup-amiga-gcc.exe package => 61

... will test running Linux soon

[bebbo]

Linux compiled binary also prints 61... Please attach your binary.

[mheyer32]

Not sure if it makes a difference, but try compiling with g++ as a cpp file. Thats what I did and ScummVM is also compiled as C++ project.

Test.zip

[bebbo]

perdon? you compiled this vpr.zip with m68k-amigaos-g++ -Os vpr.c -o vpr -noixemul?

[bebbo]

printf isn't broken - please tell me your CFLAGS_FOR_TARGET.

[mheyer32]

I can't do it right now, I'll follow up tomorrow.

Can you try compiling vpr.c as .cpp file?

I'll triple-check that I'm not messing with CFLAGS_FOR_TARGET... Or better yet, delete and rebuilt the whole toochain directory.

[bebbo]

check your .bashrc etc.p.p

I rather suspect a further bug in my optimizer.

[bebbo]

my guess: you are using -fomit-frame-pointer -Ofast

[bebbo]

You found something very valuable, you found a gem: Sebum!

It happens that this code:

.L185:
    move.l a6,d2
.L1:

gets converted to

.L185:
    move.l a6,a6
.L1:

and thus the statement is dropped:

.L185:
.L1:

that's all ok. But it run into the bug that collecting the registers to rename stopped at .L1 since the previous insn .L185 does not use the searched register (label's don't use registers at all). Thus a false rename of a register occurs which is used elsewhere...

[mheyer32]

Ha, why am I always the lucky one to find the gems? :-D

[mheyer32]

(I'm still in the process of rebuilding the toolchain and Aminet is not helping)

[bebbo]

Ha, why am I always the lucky one to find the gems? :-D

You are the only user...

[bebbo]

(I'm still in the process of rebuilding the toolchain and Aminet is not helping)

it's ok to copy the download folder from other build attempts

[mheyer32]

Time's out for today. More testing tomorrow night. Hopefully Aminet is back up by then - I didn't keep a copy of my download directory. Are there working mirrors?

[bebbo]

http://de.aminet.net/aminet/

[mheyer32]

Woohoo! vsnprintf() seems to work again. Now I need to find out why ScummVM jumps to 0x00000000 during the intro of DiscWorld... it didn't do that before. I'll file a new issue once I know more.

Btw, could we make the aminet mirror adjustable via an env variable? I helped myself with a small sed script for now, but it would be cool to have a switch in case the main mirror is down again (which seems to happen rather often).