dreamlayers / em-dosbox

An Emscripten port of DOSBox
www.dosbox.com
GNU General Public License v2.0
1.22k stars 154 forks source link

EmDosBox does not work on 32 bit version of Chrome #36

Closed MartySVK closed 8 years ago

MartySVK commented 8 years ago

I tried games from archive.org on different computers, but I can't run these games on 32bit version of Chrome on Windows 10 and 7. But 64bit version works well. Do you have this problem too ?

dreamlayers commented 8 years ago

What happens on 32 bit Chrome?

Problems with DOS programs crashing on Chrome 51 but not 52 or 53 or other browsers were reported recently by @textfiles. I just installed Chrome 51.0.2704.103 m in 32-bit Windows 7 and got a stack overflow in DOS for https://archive.org/details/msdos_Prince_of_Persia_1990 .

So, I can confirm the problem exists. If it only happens in one version of one browser, that seems like a browser bug. The Chrome Beta channel is now version 52, so you could try that: https://www.google.com/chrome/browser/beta.html

Edit: https://archive.org/details/msdos_Prince_of_Persia_1990 and https://archive.org/details/msdos_Wolfenstein_3D_1992 both crash on startup in 32-bit versions of 51.0.2704.103 m, 52.0.2743.41 beta-m and 53.0.2767.4 dev-m, but work with 64-bit versions.

MartySVK commented 8 years ago

I can confirm, that I have also problem with 32-bit of Chrome 52 beta

textfiles commented 8 years ago

Same here. Chrome Canary crashes as well.

On Tue, Jun 21, 2016 at 4:35 AM, MartySVK notifications@github.com wrote:

I can confirm, that I have also problem with 32-bit of Chrome 52 beta

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/dreamlayers/em-dosbox/issues/36#issuecomment-227375686, or mute the thread https://github.com/notifications/unsubscribe/ABk5zOrOPCHvBgmVpLO6Y72xOYGu17Wqks5qN6JCgaJpZM4I6O4C .

MartySVK commented 8 years ago

I started topic on google forum few days ago - https://productforums.google.com/forum/#!topic/chrome/iIuzadNzB-8 it would be better, if more users will confirm this issue.

dreamlayers commented 8 years ago

I reported the issue at https://bugs.chromium.org/p/chromium/issues/detail?id=621926 I realize it may not be useful to just say something this complex doesn't work, but reporting the bug is probably better than not reporting. Since it works in one version of Chrome and doesn't work in the next, doing a git bisect on Chrome might help.

BTW Here is another open issue, with Emscripten code giving wrong results in 64-bit Chrome: https://bugs.chromium.org/p/chromium/issues/detail?id=551746

bengarrett commented 8 years ago

So to add to this puzzle. Most of the demos hosted on defacto2.net fail in Chrome 51 32-bit, some work but not always correctly.

Revolt demo works correctly in 64-bit but in 32-bit suffers major graphic issues. https://defacto2.net/file/detail/ad21858

64-bit ad21858

32-bit ad21858 1

A couple like Cronologia complain about a lack of memory in 32-bit. http://defacto2.net/file/detail/ab1a980 ab1a980

While FC's Unreal just crashes the Chrome 32-bit tab but works fine in 64-bit. http://defacto2.net/file/detail/b32bc68?dosmachine=vga snap

Yet if you change the machine to SVGA from VGA, the emulated Unreal program fails but at least the Chrome tab doesn't crash. https://defacto2.net/file/detail/b32bc68?dosmachine=svga b32bc68

dreamlayers commented 8 years ago

@bengarrett, thank you for finding that http://defacto2.net/file/detail/b32bc68?dosmachine=vga&dosspeed=auto crashes Chrome. For me it crashes 51.0.2704.84 32-bit but works in 50.0.2661.75 32-bit and 51.0.2704.84 64-bit. That means it behaves like the other problems.

For me this further reinforces the suspicion that this is a Chrome problem. JavaScript should not be able to crash the browser, regardless of bugs in the JavaScript web application. That might even have security implications, if specially crafted JavaScript could use the bug to access things JavaScript shouldn't be able to access.

I have passed on this information in the Chromium issue: https://bugs.chromium.org/p/chromium/issues/detail?id=621926#c2

Edit: A very small DOS program which fails is QEMU's pi_10.com test. The same thing affects http://www.qemu-advent-calendar.org/ Day 3 - Bootable Pi. It doesn't simply output the wrong number; it outputs letters. Here's a screenshot in chrome32_51.0.2704.84. Different runs give the same answer. The correct output is Pi: 00031415...

pi_10

textfiles commented 8 years ago

Can you look through it again and add it more? I got the attention of a lot of Google people and have really pushed this is so important it could stand a retroactive hot patch.

On Tue, Jun 21, 2016 at 8:05 PM, Boris Gjenero notifications@github.com wrote:

@bengarrett https://github.com/bengarrett, thank you for finding that http://defacto2.net/file/detail/b32bc68?dosmachine=vga&dosspeed=auto crashes Chrome. For me it crashes 51.0.2704.84 32-bit but works in 50.0.2661.75 32-bit and 51.0.2704.84 64-bit. That means it behaves like the other problems.

For me this further reinforces the suspicion that this is a Chrome problem. JavaScript should not be able to crash the browser, regardless of bugs in the JavaScript web application. That might even have security implications, if specially crafted JavaScript could use the bug to access things JavaScript shouldn't be able to access.

I have passed on this information in the Chromium issue: https://bugs.chromium.org/p/chromium/issues/detail?id=621926#c2

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/dreamlayers/em-dosbox/issues/36#issuecomment-227607998, or mute the thread https://github.com/notifications/unsubscribe/ABk5zB03MRhpVqeLJIw7XD3FRMpqCZkIks5qOHw7gaJpZM4I6O4C .

dreamlayers commented 8 years ago

So that's why there was so much activity at that bug! Thank you for drawing attention to this bug, @textfiles .

I've tried to narrow it down to an instruction. It seems like adc dx, bp at offset 0x24 in pi_10.com is triggering the problem. That was my first guess because of how a bisect found a change affecting compares of 8 and 16 bit values. Replacing the instruction with just add dx, bp by changing the byte at offset 0x24 from 0x13 to 0x03 causes the program to output something much more like Pi. It starts off with the proper 000314159, which is much better than C4E31I1H0E. Though a simple adc dx, bp test case works fine, so I'm not certain that it is due to ADC. If I can narrow it down to an instruction it may be possible to find a short bit of JavaScript code which triggers the problem.

The pi_10.com source is in: http://web.archive.org/web/20160305205129/http://boo.net/~jasonp/bf3pi.zip From this page: http://web.archive.org/web/20160305205129/http://boo.net/~jasonp/pipage.html

Edit: I added a comment with a stripped down pi_10.com test case: https://bugs.chromium.org/p/chromium/issues/detail?id=621926#c12

Edit 2: Code fragment which breaks, with explanation:

start:
; Number of iterations must be high enough.
; Maybe needed to cause Chrome to compile frequently used code to x86?
        mov     cx, 32760

; Initialize variables for loop below
        mov     ax, 0
; bp must be non-zero. Does not have to be large enough to cause carry.
        mov     bp, 1

inner_loop:
        add     ax, bp
; The carry flag from the add needs to affect the adc.
; It's okay to preserve ax via push and pop around the add.
; Don't interfere with flags, even pushf/popf or cmc prevents bug.
        adc     ax, bp
        loop    inner_loop
dreamlayers commented 8 years ago

A fix has been found. 32-bit binaries from http://chromium.woolyss.com/ now work. The fix is not in currently released Canary yet.

Edit: It got Merge-Request-52 and Merge-Request-51 labels.

dreamlayers commented 8 years ago

32-bit Opera developer 40.0.2273.0 is affected the same way. I have reported a bug and got the identifier DNAWIZ-4263. There is no link. It seems their bug tracker isn't accessible to everyone, like with Chromium.

bengarrett commented 8 years ago

Would it be safe to assume most browsers using the 32-bit Blink engine would be effected? I just tested in Vivaldi and it crashed with http://defacto2.net/file/detail/b32bc68?dosmachine=vga&dosspeed=auto

Vivaldi 1.2.490.43 () (32-bit) Revision aa7c8d23c098e96a388ffedf6698230bda650bb3 OS Windows Blink 537.36 JavaScript V8 5.1.281.65

I wonder if there are any mobile browsers effected? I did try on a Samsung S6 6.0.1 with its Internet app which is based on Blink but it worked fine.

dreamlayers commented 8 years ago

I'm disappointed with how long it's taking for this fix to make it into Chrome releases. It still fails in stable and beta, but works in dev. So, whoever really wants to use Em-DOSBox in 32-bit Chrome in Windows can install from the dev channel as a workaround, but that's still a barrier for people. (Mac and Linux builds of Chrome are 64-bit. You'd need a 32-bit build of Chromium to see the bug.)

So, if you want things to work in Chrome, test in Canary or at least dev channel so there will be enough time for a fix before the bug makes it into stable. That might also help avoid the bug in other browsers using 32-bit Blink.

MartySVK commented 8 years ago

They fixed it again, but I hope, that it won't take another month to release it.

MartySVK commented 8 years ago

It should be working

dreamlayers commented 8 years ago

Yes, works for me in Chrome 53.0.2785.116 m in 32-bit Windows 7. I'm going to close this now.