Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[REGRESSION] READ_CORE_RAM returns empty (-1) since 1.9.1 #12262

Closed
bmarwell opened this issue Apr 12, 2021 · 21 comments · Fixed by #12267
Closed

[REGRESSION] READ_CORE_RAM returns empty (-1) since 1.9.1 #12262

bmarwell opened this issue Apr 12, 2021 · 21 comments · Fixed by #12267

Comments

@bmarwell
Copy link

bmarwell commented Apr 12, 2021

First and foremost consider this:

  • Only RetroArch bugs should be filed here. Not core bugs or game bugs
  • This is not a forum or a help section, this is strictly developer oriented

Description

READ_CORE_RAM returns an empty list since 1.9.1. This did not happen in 1.9.0

Expected behavior

should not return READ_CORE_RAM 0 -1

Actual behavior

returns

READ_CORE_RAM 0 -1

Steps to reproduce the bug

  1. Upgrade from any to 1.9.1
  2. send `READ_CORE_RAM

Bisect Results

[Try to bisect and tell us when this started happening]

Version/Commit

You can find this information under Information/System Information

  • RetroArch: [version/commit]

Environment information

  • OS: Linux
  • Compiler: [In case you are running local builds]
@bmarwell bmarwell changed the title [REGRESSION] READ_CORE_MEMORY returns empty array since 1.9.1 [REGRESSION] READ_CORE_MEMORY returns empty (-1) since 1.9.1 Apr 12, 2021
@hizzlekizzle
Copy link
Contributor

Can you bisect and identify when it stopped working?

@bmarwell
Copy link
Author

As a user, keeping a history of windows builds is one thing, but you haven't got a history of Linux builds. I cannot revert to the old snap Version.

I will try to bisect the change soon. Cannot promise an ETA.

@bmarwell
Copy link
Author

According to the qusb2snes discord, all SNES cores are affected.

@hizzlekizzle
Copy link
Contributor

I can probably help bisect if there's an easy way to test it. Do you have a program that queries it that I can run?

@bmarwell
Copy link
Author

I am using qusb2snes (a Qt abstraction layer for various emulators, e.g. for a real (sd2)SNES, RA, snes9x-rr, etc.).
But you can just do echo "READ_CORE_RAM 0 1^D" | socat - udp:localhost:55355.

2021-04-13T08:46:46_0001_screenshot

@bmarwell
Copy link
Author

TBH looking at command.c, it can only be this commit: b830b33

The two later commits are only style nits, the previous commits are from 2019.

@davidgfnet
Copy link
Contributor

Hey there!
This is unrelated to b830b33, just tested it on the previous commit.
If it helps I can tell you what' ive seen. The command gets correctly to the cheevos interface, but the function rcheevos_memory_find returns null due to the fact that there's no memory regions defined. I'm not sure what's Cheevos and how it works, but I suspect some glue code fell out of sync and it's not getting the info it needs to be able to peek/poke emulator memory any more.
Sorry I cannot be of more help :)

@davidgfnet
Copy link
Contributor

Checked ~1st Jan and it is broken too, so it's been broken for a while :|

@Jamiras
Copy link
Contributor

Jamiras commented Apr 13, 2021

READ_CORE_RAM should be going through the rcheevos_memory_find path. READ_CORE_MEMORY should not. It was added to support cases where achievements aren't available. See #12105

The screenshot shows you calling READ_CORE_RAM, but the ticket title says READ_CORE_MEMORY. Are you just calling the wrong one?

@davidgfnet
Copy link
Contributor

Oh I see, however the other function has the same issue. Seems like command_read_memory does not have info on memory maps. This is also happening before my commit, so I suspect something must be going on.
READ_CORE_MEMORY sounds like a relatively recent addition, Im surprised it's already broken.

@Jamiras
Copy link
Contributor

Jamiras commented Apr 13, 2021

I'm not seeing the "-1" behavior, but there does appear to be a change in behavior associated to b830b33.

With the a989cfb commit, the command always returns a value:

$ echo -n "READ_CORE_RAM 0004ee 1" | nc -u -w1 localhost 55355
READ_CORE_RAM 4ee 05
$

With the b830b33 commit, the first command returns nothing, but later commands seem to work:

$ echo -n "READ_CORE_RAM 0004ee 1" | nc -u -w1 localhost 55355
$ echo -n "READ_CORE_RAM 0004ee 1" | nc -u -w1 localhost 55355
READ_CORE_RAM 4ee 05
$ echo -n "READ_CORE_RAM 0004ef 1" | nc -u -w1 localhost 55355
READ_CORE_RAM 4ee 00
$ echo -n "READ_CORE_MEMORY 7e04ee 1" | nc -u -w1 localhost 55355
READ_CORE_RAM 4ee 05
$ echo "READ_CORE_RAM 0 1^D" | nc -u -w1 localhost 55355
READ_CORE_RAM 0 48
$

I'm seeing the same behavior on current master (first command fails silently, later commands work). Tested with bsnes-mercury and snes9x cores on ubuntu 18.

The "-1" return value indicates the address could not be mapped to memory in the core (READ_CORE_MEMORY provides slightly more information in it's return value). It sounds like SET_MEMORY_MAPS isn't being called. Do you see a block like this in your log file?

[INFO] [Environ]: SET_MEMORY_MAPS.
[INFO]    ndx flags  ptr              offset   start    select   disconn  len      addrspace
[INFO]    001 M1A1bC (nil) 00000000 00802188 00C0FFE8 00000000 01000000 
[INFO]    002 M1A1bC (nil) 00000000 00002188 00C0FFE8 00000000 01000000 
[INFO]    003 M1A1bC (nil) 00000000 00802100 00C0FFC0 00000000 01000000 
[INFO]    004 M1A1bC (nil) 00000000 00002100 00C0FFC0 00000000 01000000 
[INFO]    005 M1A1bc 0x7f163ff85c7c 00000000 007E0000 00FE0000 00000000 00020000 
[INFO]    006 M1A1bc 0x7f163ff85c7c 00000000 00800000 00C0E000 00000000 00002000 
[INFO]    007 M1A1bc 0x7f163ff85c7c 00000000 00000000 00C0E000 00000000 00002000 
[INFO]    008 M1A1bC (nil) 00000000 00804300 00C0FF80 00000000 01000000 
[INFO]    009 M1A1bC (nil) 00000000 00004300 00C0FF80 00000000 01000000 
[INFO]    010 M1A1bC (nil) 00000000 00804200 00C0FFE0 00000000 01000000 
[INFO]    011 M1A1bC (nil) 00000000 00004200 00C0FFE0 00000000 01000000 
[INFO]    012 M1A1bC (nil) 00000000 00804016 00C0FFFE 00000000 01000000 
[INFO]    013 M1A1bC (nil) 00000000 00004016 00C0FFFE 00000000 01000000 
[INFO]    014 M1A1bc 0x555df3693d90 00000000 00F00000 00F00000 00000000 00002000 
[INFO]    015 M1A1bc 0x555df3693d90 00000000 00700000 00F00000 00000000 00002000 
[INFO]    016 M1A1bC 0x555df4152fa0 00000000 00808000 00808000 00008000 00200000 
[INFO]    017 M1A1bC 0x555df4152fa0 00000000 00008000 00808000 00008000 00200000 
[INFO]    018 M1A1bC (nil) 00000000 00000000 00000000 00000000 01000000 

@bmarwell
Copy link
Author

I am here of qusb2snes, and it is sending READ_CORE_RAM 0 1 to see if a game is running at all, on -1 it suspects no.

If there is another return value, it will then try to send READ_CORE_RAM 40FFC0 32.

However, as much as I tried your commands (eg echo -n "READ_CORE_RAM 0004ee 1" | nc -u -w1 localhost 55355), I always receive -1.

Here's a verbose output excerpt:

[INFO] [CONTENT LOAD]: Loading content file: $USER/git/alttp/MultiWorld-Utilities/output/BM_42126988336210426889_P1_noglitches_normal-normal-open-ganon_vanilla-balanced.sfc.
[INFO] Did not find a valid content patch.
[INFO] [Environ]: SET_INPUT_DESCRIPTORS:
[libretro INFO] BML map:
[libretro INFO] cartridge region=NTSC
[libretro INFO]   rom name=program.rom size=0x400000
[libretro INFO]   ram name=save.ram size=0x8000
[libretro INFO]   map id=rom address=00-7f,80-ff:8000-ffff mask=0x8000
[libretro INFO]   map id=ram address=70-7f,f0-ff:0000-7fff
[libretro INFO] [Memory]: ID 7, Request "manifest.bml".
[libretro INFO] Complete load request.
[libretro INFO] [Memory]: ID 8, Request "program.rom".
[libretro INFO] Load ROM.
[libretro INFO] Complete load request.
[libretro INFO] [Memory]: ID 9, Request "save.ram".
[libretro INFO] Complete load request.
[libretro INFO] [Memory]: ID 6, Request "".
[libretro INFO] Complete load request.
[INFO] [Environ]: SET_MEMORY_MAPS.
[INFO]    ndx flags  ptr              offset   start    select   disconn  len      addrspace
[INFO]    001 M1A1bC (nil) 00000000 00802188 00C0FFE8 00000000 01000000 
[INFO]    002 M1A1bC (nil) 00000000 00002188 00C0FFE8 00000000 01000000 
[INFO]    003 M1A1bC (nil) 00000000 00802100 00C0FFC0 00000000 01000000 
[INFO]    004 M1A1bC (nil) 00000000 00002100 00C0FFC0 00000000 01000000 
[INFO]    005 M1A1bc 0x7f1a1ff85c7c 00000000 007E0000 00FE0000 00000000 00020000 
[INFO]    006 M1A1bc 0x7f1a1ff85c7c 00000000 00800000 00C0E000 00000000 00002000 
[INFO]    007 M1A1bc 0x7f1a1ff85c7c 00000000 00000000 00C0E000 00000000 00002000 
[INFO]    008 M1A1bC (nil) 00000000 00804300 00C0FF80 00000000 01000000 
[INFO]    009 M1A1bC (nil) 00000000 00004300 00C0FF80 00000000 01000000 
[INFO]    010 M1A1bC (nil) 00000000 00804200 00C0FFE0 00000000 01000000 
[INFO]    011 M1A1bC (nil) 00000000 00004200 00C0FFE0 00000000 01000000 
[INFO]    012 M1A1bC (nil) 00000000 00804016 00C0FFFE 00000000 01000000 
[INFO]    013 M1A1bC (nil) 00000000 00004016 00C0FFFE 00000000 01000000 
[INFO]    014 M1A1bc 0x5639abc5b620 00000000 00F00000 00F08000 00000000 00008000 
[INFO]    015 M1A1bc 0x5639abc5b620 00000000 00700000 00F08000 00000000 00008000 
[INFO]    016 M1A1bC 0x5639ac55b720 00000000 00808000 00808000 00008000 00400000 
[INFO]    017 M1A1bC 0x5639ac55b720 00000000 00008000 00808000 00008000 00400000 
[INFO]    018 M1A1bC (nil) 00000000 00000000 00000000 00000000 01000000 
[libretro INFO] SRAM memory size: 32768.
[INFO] [SRAM]: Skipping SRAM load..
[INFO] Version of libretro API: 1
[INFO] Compiled against API: 1
[INFO] [Environ]: SET_PIXEL_FORMAT: XRGB8888.

@Jamiras
Copy link
Contributor

Jamiras commented Apr 13, 2021

READ_CORE_RAM does not function unless a game with achievements is loaded, which was the impetus for #12105.

What happens if you try READ_CORE_MEMORY 7E0000 1? That should access the system RAM identified in this line of the memory map:

[INFO]    005 M1A1bc 0x7f1a1ff85c7c 00000000 007E0000 00FE0000 00000000 00020000 

@bmarwell
Copy link
Author

Latest snap and flatpak version (both 1.9.1) give:

$ echo -n "READ_CORE_MEMORY 7E0000 1" | nc -u -w1 localhost 55355
READ_CORE_MEMORY 7e0000 00

I found older versions on flathub, let me quickly check.

@Jamiras
Copy link
Contributor

Jamiras commented Apr 13, 2021

Well, that seems to be working, so the problem seems to be that READ_CORE_RAM requires achievements to be active, and they aren't for whatever reason. Do you want to keep investigating on that end? Or does switching to READ_CORE_MEMORY address your concerns?

@bmarwell
Copy link
Author

    Commit: 96a53026110974a42f317b13cc1762ddf5a7cc8f17532c6089e18f526d3e1d71
   Subject: Fix update list (3039dea0)
      Date: 2020-08-10 00:45:14 +0000

$ sudo flatpak update  --commit=96a53026110974a42f317b13cc1762ddf5a7cc8f17532c6089e18f526d3e1d71 org.libretro.RetroArch
$ flatpak run org.libretro.RetroArch &
$ echo -n "READ_CORE_RAM 0 1" | nc -u -w1 localhost 55355
READ_CORE_RAM 0 00
$ echo -n "READ_CORE_MEMORY 7E0000 1" | nc -u -w1 localhost 55355
# no output

    Commit: b22f81ea375aa64aeb885268357d32df7df81d4da1f7ce498eb97dc91c53b6f8
   Subject: Add libusb (#167) (ff466df3)
      Date: 2021-02-28 19:47:52 +0000
$ sudo flatpak update  --commit=b22f81ea375aa64aeb885268357d32df7df81d4da1f7ce498eb97dc91c53b6f8 org.libretro.RetroArch
$ flatpak run org.libretro.RetroArch &
$ echo -n "READ_CORE_RAM 0 1" | nc -u -w1 localhost 55355
READ_CORE_RAM 0 00
$ echo -n "READ_CORE_MEMORY 7E0000 1" | nc -u -w1 localhost 55355
# no output

       Sdk: org.kde.Sdk/x86_64/5.14
    Commit: 6206dc3fed228d43cfe446afc3dad0d8a44a546632af3acd18444a803ce1cc50
    Parent: b22f81ea375aa64aeb885268357d32df7df81d4da1f7ce498eb97dc91c53b6f8
   Subject: v1.9.1 (#168) (1f357fde)
      Date: 2021-03-30 18:47:21 +0000
$ sudo flatpak update  --commit=6206dc3fed228d43cfe446afc3dad0d8a44a546632af3acd18444a803ce1cc50 org.libretro.RetroArch
$ flatpak run org.libretro.RetroArch &
$ echo -n "READ_CORE_RAM 0 1" | nc -u -w1 localhost 55355
READ_CORE_RAM 0 -1
$ echo -n "READ_CORE_MEMORY 7E0000 1" | nc -u -w1 localhost 55355
READ_CORE_MEMORY 7e0000 00

@bmarwell
Copy link
Author

Well, that seems to be working, so the problem seems to be that READ_CORE_RAM requires achievements to be active, and they aren't for whatever reason. Do you want to keep investigating on that end? Or does switching to READ_CORE_MEMORY address your concerns?

Sorry I was getting the above data while you created another post.
The API definitely changed, see above output. Does that help?

@Jamiras
Copy link
Contributor

Jamiras commented Apr 13, 2021

Ok, I think I understand the problem now. In 1.9.0 stable, you can call READ_CORE_RAM with achievements off and get back a valid achievement memory value. In 1.9.1 stable, calling READ_CORE_RAM with achievements off returns -1.

The change was all the way back in August via #11183, which pre-generates the achievement address->core mappings after the achievements are loaded, instead of on demand as they're looked up. The READ_CORE_RAM functionality you're expecting was relying on the dynamic population of lookup misses. And since no achievement information is loaded, it's using a generic mapping where the system ram starts at address 0. This just happens to match the scheme used by Super Nintendo, but will not work for all RetroAchievement supported systems.

So, there's basically two options to address this:

  1. call READ_CORE_MEMORY instead of READ_CORE_RAM (which is guaranteed to work with achievements off, with HAVE_CHEEVOS not defined, and for systems where achievements aren't available).
  2. modify READ_CORE_RAM to call the achievement address->core mapping initialization code. This will still be performed with a generic system for reasons mentioned above.

@bmarwell
Copy link
Author

For the sake of backwards compatibility (this is a public API), I vote for option 2. For a version 2.x of RetroArch you are free to do breaking changes (semver).
WDYT?

@Jamiras
Copy link
Contributor

Jamiras commented Apr 14, 2021

As the change was relatively simple, I've gone ahead an implemented it.

I still recommend that you should move to the READ_CORE_MEMORY API.

@bmarwell
Copy link
Author

Thanks @Jamiras I will forward it to the qusb2snes project for future versions!

@bmarwell bmarwell changed the title [REGRESSION] READ_CORE_MEMORY returns empty (-1) since 1.9.1 [REGRESSION] READ_CORE_RAM returns empty (-1) since 1.9.1 Apr 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants