sarah-walker-pcem / pcem

PCem
http://pcem-emulator.co.uk
GNU General Public License v2.0
1.47k stars 204 forks source link

vm settings are permanently reseted #173

Closed Peugeot205GTI closed 1 year ago

Peugeot205GTI commented 1 year ago

Describe the bug the vm settings are reseted each time i start a vm.

To Reproduce just create a machine and set some settings to non-default values.

Emulator configuration (also tried with other configs)

Host machine

kingilein commented 1 year ago

I can confirm that. it affects the newer compilations from the master and dev branches. Possibly even all branches are affected. No changes at all can be made in the settings of the VMs. If you leave the config menu and then go straight back in, the settings are back to default. PCem v17 does not have this problem.

eddmanx commented 1 year ago

@kingilein Seems to be working now with the latest Test Debug Builds 7 from the Actions builds.

https://github.com/sarah-walker-pcem/pcem/actions/runs/2930586339

I didn't notice that's the debug build.

The release build actually crashes when trying to create a machine. I copied an existing machine from v17, but it resets the settings to default when editing; still crashes when closing the config window.

P.S. "reseted" doesn't exist; it's simply "reset".

michael-manley commented 1 year ago

I did assign this to v18 as a major bug but might be irrelevant with the whole recode to Qt. I have yet to switch release/debug builds to Qt as it is rebuildable at this moment.

michael-manley commented 1 year ago

Can you all try the lastest vNext build (I will mention config files need to be moved to .pcem in your home directory like so) image

eddmanx commented 1 year ago

Can you all try the lastest vNext build (I will mention config files need to be moved to .pcem in your home directory like so))

I downloaded the latest release build from the Actions tab, copied over the rom files to the specified location, and it still crashes when trying to save a machine. There is also no pcem.cfg.

I also tried copying existing machine config files to the new locations, but when I open the machine editing window, the settings are reset to default, and closing the window with either cancel or ok buttons crashes PCem.

The debug build still works, but it's very slow; performance is half of v17's.

michael-manley commented 1 year ago

This seems very odd, def tested the build and am able to save and create machines.

I wonder if its an issue with ROMs. Are you using a full-blown pack you found somewhere or select ROMs.

eddmanx commented 1 year ago

I was testing with a full pack but since you mentioned it, I tested only with p55t2p4 and voodoo3_3000, and no change.

Why would it be rom related if the debug build works?

Unfortunately PCem creates no logs when this crash happens.

michael-manley commented 1 year ago

Release builds do some optimizations, so somthing could be optimized out that's causing the issue.

I will try to get a release build with Logging enabled so we can test whats causing the issue.

michael-manley commented 1 year ago

Try now with a Release build with debugging and logging enabled. it still optimizes like a Release build, but adds logging and gdb debugging symbols

eddmanx commented 1 year ago

pcem_release.log

I blanked the user name.

ruben-balea commented 1 year ago

Does your username have spaces or special characters? Mine does and it causes me problems with several other programs, I have not tried this release of PCem. I use Windows 10 but the same thing happens to me since XP... as a solution I use a different user account for those programs, with a short name and without special characters or spaces, literally: USERNAME

eddmanx commented 1 year ago

No, it's a simple one word name, with only english letters.

michael-manley commented 1 year ago

Cant tell where its failing :/

If you can somehow get gdb, run it with GDB so I can get a backtrace.

eddmanx commented 1 year ago

I don't know how to do that and the instructions on the internet are very varied and are confusing to me. If someone can post instructions for a simple method, I'll try it.

Derailedzack commented 1 year ago

Do you have GDB installed?

AITUS95 commented 1 year ago

@michael-manley Same problem for me, below I leave you a video with the gdb backtrace as you requested, for me it only happens when I modify the ram, cpu, mouse devices and gamepad / joystick, everything else is saved:

https://user-images.githubusercontent.com/48457684/194638000-a7276c1a-dd3e-4adb-995c-1c1bea163b2c.mp4

stefandd commented 1 year ago

The HDD image parameters (sectors, cylinders, etc) are lost every time on loading, not writing, preventing the mounting of any image for me

michael-manley commented 1 year ago

@AITUS95 Almost got it. When it crashes i think bt shows the backtrace. Your using the relwithdebug builds right?

AITUS95 commented 1 year ago

@michael-manley yes I'm using that

stefandd commented 1 year ago

@michael-manley With the relwithdebug build any config file is destroyed upon trying to load it -- for some reason it is also immediately overwritten???

As you can see small values disapper (replaced with 0) whereas the memory size of 16MB gets bitshifted to 4MB. Consider this fragment of a VM configuration file.

Original

gameblaster = 0
gus = 0
ssi2001 = 0
voodoo = 1
model = sis496
cpu_manufacturer = 0
cpu = 10
cpu_use_dynarec = 1
cpu_waitstates = 0
gfxcard = px_trio64
video_speed = -1
sndcard = sbawe32
cpu_speed = 6
has_fpu = 1
disc_a = 
disc_b = 
hdd_controller = ide
mem_size = 16384
cdrom_drive = 0
cdrom_channel = 2
cdrom_path = 
zip_channel = -1
hdc_sectors = 63
hdc_heads = 16
hdc_cylinders = 1024
hdc_fn = C:\PCEmInst\DRDOS7IMG.img
hdd_sectors = 63
hdd_heads = 16
hdd_cylinders = 4096

Overwritten immediately when trying to start/edit

gameblaster = 0
gus = 0
ssi2001 = 0
voodoo = 0
model = sis496
cpu_manufacturer = 0
cpu = 0
cpu_use_dynarec = 0
cpu_waitstates = 0
gfxcard = px_trio64
video_speed = -1
sndcard = sbawe32
cpu_speed = 0
has_fpu = 1
disc_a = 
disc_b = 
hdd_controller = ide
mem_size = 4096
cdrom_drive = 0
cdrom_channel = 2
cdrom_path = 
zip_channel = -1
hdc_sectors = 0
hdc_heads = 0
hdc_cylinders = 0
hdc_fn = C:\PCEmInst\DRDOS7IMG.img
hdd_sectors = 0
hdd_heads = 0
hdd_cylinders = 0
paulwratt commented 1 year ago

did this get fixed yet, or diagnosed. It seems the might be an extra element in added to a struct on DEBUG option

TomoshibiAkira commented 1 year ago

@paulwratt No it hasn't. The Debug build is fine, Release build is also fine on BSD (macOS). I can only reproduce this on Windows with the Release build. Could it possibly be a path thing?

TomoshibiAkira commented 1 year ago

I think I found the problem. https://github.com/sarah-walker-pcem/pcem/blob/abec5fdc17bd26ba5490e7dfe7d95aa83c74c134/src/plugin-api/config.c#L226-L242

Change char blank[] = ""; in L228 to char blank[256] = {0}; solves the problem. EDIT: Even without the initializer, char blank[256] = ""; works too, but you need to make sure this array is at least 256 bytes long.

I still don't know why though. It seems like the optimization during compiling did something evil and caused the strncmp in L236 to fail. Subsequently, in config_get_int(), it directly returns def, which is 4096 for mem_size. And that would be expanded to 8192 on a 486/Pentium-based machine.

Interestingly, if you change the size of the blank array to another value (say 128), the "default" config would change and the program might crash.


OK, I'm sure it's an optimization-related thing now.

More interesting founds: Under the RelWithDebInfo build and enable logging, if you add pclog ("sizeof blank %d, strlen blank %d\n", sizeof(blank), strlen(blank)); directly below L228, the problem will also go away, and the log will print a normal value (size=1 and strlen=0), even without the 256-byte size array. But the problem still persists under the Release build because pclog is basically an empty function there. The blank variable will be optimized by the compiler. Very sneaky stuff.

michael-manley commented 1 year ago

Can you guys test the latest build and see if the changes this issue. I really want to kill this bug lol.

And yes alot of issues have cropped up due to optimizations, I am going to look into alot of optimization issues in Windows soon.

eddmanx commented 1 year ago

Yes, both settings being reset and crashing issues are fixed, however, the release (non-debug) build is still considerably slower than v17, at least on my ryzen 2600.

Pentium MMX 233, ATI video expression (no voodoo), windows 98, sitting idle on the desktop:

v17: Emu speed: 100% CPU time: ~97% (~97%)

Release build 4619559: Emu speed: ~84% CPU time: 100% (~118%)

stefandd commented 1 year ago

It would be good to first establish whether this performance regression is due to the new UI engine (Qt instead of Wx) or not by compiling HEAD with -DPCEM_DISPLAY_ENGINE=wxWidgets.

michael-manley commented 1 year ago

The qt engine isnt built by default. And its not yet in the codebase yet. Still looking into keeping wxWidgets anyway.

The issue with speed is dynarec and it is in need of some speed improvements

stefandd commented 1 year ago

@michael-manley But the report of @eddlang is a significant performance regression that must have a cause in changes made post v17, right? (notwithstanding the fact that dynarec might need speed improvements)

unreal9010 commented 1 year ago

@michael-manley When regressions happen the CPU usage increases considerably and temperatures suddenly start to rise and may go past 90 degrees. CPU speed also becomes stuck between 4.5-4.7 Ghz (normally when the emulator is running at 100% the CPU speed usually is 5.1-5.2 Ghz on my 12900KF). Some of the regressions were already present in v17 and even before, however, many were introduced at later stages. I also noticed that running Voodoo 3 or Voodoo Banshee might help to alleviate some of these regression issues. For example, I tried running Moto Racer and Hardball 6 demos and noticed that under Voodoo 2 PCem may drop down to 25% when accessing settings menu in Moto Racer or attempting to open a drop-down menu with the right mouse button in Hardball 6. Yet, when running these two demos with either Voodoo Banshee or Voodoo 3 speed is always consistent at 100% on my emulated Pentium II 350. I suggest you to download Moto Racer and/or Hardball 6 demo and then compare the outputs when running them under Voodoo 2 and Voodoo 3, respectively. I think that might be the key to finally solve PCem's biggest issue.

michael-manley commented 1 year ago

@unreal9010 It might be due to how GCC optimizes in newer versions. Half the bugs i see now seem to be due to a change in GCC.

I know there is need for tons of refactoring on components. My biggest next project once i get things stable is dynarec optimizations which should help elevate speed slowdowns.

Im trying hard for a December release, as it's been years since v17.

unreal9010 commented 1 year ago

Thank you for your best efforts, @michael-manley. I keep my fingers crossed for a December release. Currently, is there any way to build PCem with another compiler suite? I'm just curious to see how this would affect the regressions.

michael-manley commented 1 year ago

@unreal9010 ATM, no, GCC is the only official compiler,

Though I will say, if you want to see if this works at all, https://github.com/MarekKnapek/pcem/tree/msvc does implement some changes to make it build with MSVC, but I have yet to look at the code changes and verify if it will still work on GCC as well as actually verify with MSVC if it works at all. I have Visual Studio on my PC but I dont really use MSVC, only have it more for C# Development lol.

Maybe even attempt to build with LLVM? CMake does make it possible to use different suites, I just never attempted building with other compilers.

unreal9010 commented 1 year ago

@michael-manley Would be an interesting thing to test and compare any possible differences. Too bad the builds have expired. Looks like I will have to commit it manually to my fork.

michael-manley commented 1 year ago

@unreal9010 I am gonna try myself maybe tomorrow but if you wanna try clang (LLVM) in MSYS2

pacman -Sy mingw-w64-clang-x86_64-ntldd-git mingw-w64-clang-x86_64-toolchain mingw-w64-clang-x86_64-SDL2 mingw-w64-clang-x86_64-openal mingw-w64-clang-x86_64-wxWidgets mingw-w64-clang-x86_64-libpcap mingw-w64-clang-x86_64-cmake mingw-w64-clang-x86_64-ninja

This will get all the LLVM stuff installed, then just use the clang64 prompt

unreal9010 commented 1 year ago

Thanks a lot, @michael-manley. Will try it later today and will let you know about the results.

AITUS95 commented 1 year ago

When I try to build the build with clang64, I get the following errors:

Cattura

unreal9010 commented 1 year ago

I get a simillar error too: C:/msys64/home/myusername/pcem/src/lpt/lpt_dss.c:104:17: error: incompatible pointer to integer conversion initializing 'uint32_t' (aka 'unsigned int') with an expression of type 'void *' [-Wint-conversion] ninja: build stopped: subcommand failed.

AITUS95 commented 1 year ago

I get a simillar error too: C:/msys64/home/myusername/pcem/src/lpt/lpt_dss.c:104:17: error: incompatible pointer to integer conversion initializing 'uint32_t' (aka 'unsigned int') with an expression of type 'void *' [-Wint-conversion] ninja: build stopped: subcommand failed.

maybe an older build could fix it? I try as soon as I can.

michael-manley commented 1 year ago

Try now, I did change the NULL issue with lpt.dss.c

AITUS95 commented 1 year ago

the build compiles, but with some errors, the exe file refuses to open complaining about libpcap.dll not found and "could not find procedure entry point _Unwind_Resume in dynamic link library libpcem-plugin-api.dll", in Attached the log while compiling.

log.txt

AITUS95 commented 1 year ago

if I run the command "-DUSE_PCAP_NETWORKING=OFF" I get "could not find procedure entry point _ZTV17wxMemoryFSHandler in dynamic link library pcem.exe" and "could not find procedure entry point _Unwind_Resume in dynamic link library libpcem-plugin-api.dll".

sorry for double post.