kanjitalk755 / macemu

Basilisk II and SheepShaver Macintosh emulators
349 stars 53 forks source link

BasiliskII needs `-fwrapv` to be set #197

Closed agoode closed 6 months ago

agoode commented 6 months ago

I have been debugging a mysterious freeze that occurs with a specific program that I am porting to classic mac os (https://github.com/standardml/twelf).

The freeze occurs only on Basilisk II when compiled with optimizations. It doesn't happen with -O0 or when some sanitizers are configured. This is reproducible on infinitemac.org as well as on aarch64. QEMU works fine. It might happen only with clang.

After running ubsan, I realized there are lots of places in the uae code that assume that signed overflow works ok, but it is undefined behavior (https://www.gnu.org/software/c-intro-and-ref/manual/html_node/Signed-Overflow.html). When compiled with optimizations, this can cause broken behavior.

Compiling with -fwrapv (which works in gcc and clang to define signed overflow as expected) fixes the problem on Mac and in wasm. I recommend adding this to the default CFLAGS. As an alternative, the cpu generation code could be fixed, but that is a big job.

kanjitalk755 commented 6 months ago

Fixed. I'm interested in the code that actually triggered the problem. If possible, could you please tell me the specific point of the UAE where the problem occurred?

agoode commented 6 months ago

Thank you for looking at this!

The code is somewhat hard to build but some of it is documented at https://github.com/jcreedcmu/macemu/blob/main/docker/Dockerfile.

For xcode at least, the fix does not yet work. The CFLAGS changes have to go into uae_cpu_2021.xcodeproj/project.pbxproj and uae_cpu.xcodeproj/project.pbxproj.

I've included a sample file that can be used to trigger the problem and test that the fix is successful: twelf-server.bin.zip.

The main source of the sample looks like this:

#include <stdio.h>
#include <stdlib.h>
#include <stdbool.h>
#include <Memory.h>

#define PART_OF_TWELF_SERVER
#include "twelf_server.h"

int main (void) {
  MaxApplZone();
  const char *argv[] = {"twelf-server", "@MLton", "gc-messages", "--"};

  printf("Press any key to start twelf.\n");
  getchar();
  printf("Starting twelf...\n");
  /* The next line will fail to complete if -fwrapv is not specified. */
  twelf_server_open(4, argv);
  printf("twelf startup successful! Press any key to exit.\n");
  getchar();
  twelf_server_close();
  return 0;
}

The rest of the build comes from the mlton compiler and twelf code.

agoode commented 6 months ago

It does look like the fix is good for the unix build, just not the xcode build. Thank you again.

kanjitalk755 commented 6 months ago

Ah, my mistake. Added the compile flag to UAE subprojects.

I was be able to confirm that this change was effective. Thank you.