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

Fix const pointer conversion error for RISC-V #16027

Merged
merged 1 commit into from Sep 15, 2022
Merged

Fix const pointer conversion error for RISC-V #16027

merged 1 commit into from Sep 15, 2022

Conversation

SpriteOvO
Copy link
Contributor

@SpriteOvO SpriteOvO commented Sep 15, 2022

Fixed errors:

[14/601] Building CXX object CMakeFiles/Common.dir/Common/RiscVEmitter.cpp.o
FAILED: CMakeFiles/Common.dir/Common/RiscVEmitter.cpp.o 
/usr/bin/c++ -DASSETS_DIR=\"/usr/local/share/ppsspp/assets/\" -DGLEW_NO_GLU -DMINIUPNPC_SET_SOCKET_TIMEOUT -DMINIUPNP_STATICLIB -DPNG_ARM_NEON_OPT=0 -DSHARED_LIBZIP -DSHARED_ZLIB -DVK_USE_PLATFORM_XLIB_KHR -DWITH_UPNP -D_BSD_SOURCE -D_DEFAULT_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE64_SOURCE=1 -D_POSIX_C_SOURCE=200112L -D_XOPEN_SOURCE=700 -D_XOPEN_SOURCE_EXTENDED -D__BSD_VISIBLE=1 -D__LIBRETRO__ -D__STDC_CONSTANT_MACROS -I/build/libretro-ppsspp/src/libretro-ppsspp/ext/glslang -I/build/libretro-ppsspp/src/libretro-ppsspp/ext/glew -I/build/libretro-ppsspp/src/libretro-ppsspp -I/build/libretro-ppsspp/src/libretro-ppsspp/Common -I/build/libretro-ppsspp/src/libretro-ppsspp/ext/cityhash -I/build/libretro-ppsspp/src/libretro-ppsspp/ext/libkirk -I/build/libretro-ppsspp/src/libretro-ppsspp/ext/sfmt19937 -I/build/libretro-ppsspp/src/libretro-ppsspp/ext/xbrz -I/build/libretro-ppsspp/src/libretro-ppsspp/ext/xxhash -I/build/libretro-ppsspp/src/build -march=rv64gc -mabi=lp64d -O2 -pipe -fexceptions         -Wp,-D_FORTIFY_SOURCE=2 -Wformat -Werror=format-security         -fstack-clash-protection -Wp,-D_GLIBCXX_ASSERTIONS -flto=auto   -Wformat -Wno-multichar -Wno-psabi -fPIC -fPIC -fno-strict-aliasing -std=gnu++11 -MD -MT CMakeFiles/Common.dir/Common/RiscVEmitter.cpp.o -MF CMakeFiles/Common.dir/Common/RiscVEmitter.cpp.o.d -o CMakeFiles/Common.dir/Common/RiscVEmitter.cpp.o -c /build/libretro-ppsspp/src/libretro-ppsspp/Common/RiscVEmitter.cpp
/build/libretro-ppsspp/src/libretro-ppsspp/Common/RiscVEmitter.cpp: In member function ‘void RiscVGen::RiscVEmitter::FlushIcacheSection(const u8*, const u8*)’:
/build/libretro-ppsspp/src/libretro-ppsspp/Common/RiscVEmitter.cpp:631:33: error: invalid conversion from ‘const void*’ to ‘void*’ [-fpermissive]
  631 |         __builtin___clear_cache(start, end);
      |                                 ^~~~~
      |                                 |
      |                                 const void*
<built-in>: note:   initializing argument 1 of ‘void __builtin___clear_cache(void*, void*)’
/build/libretro-ppsspp/src/libretro-ppsspp/Common/RiscVEmitter.cpp:631:40: error: invalid conversion from ‘const void*’ to ‘void*’ [-fpermissive]
  631 |         __builtin___clear_cache(start, end);
      |                                        ^~~
      |                                        |
      |                                        const void*
<built-in>: note:   initializing argument 2 of ‘void __builtin___clear_cache(void*, void*)’

The modifications are referenced from other architecture classes. But ARM64XEmitter still accepts const pointers and converts them internally to void*, IIRC this is a UB. I'm willing to fix it in the same way if you wish. (See comments for updates)

@hrydgard
Copy link
Owner

Hm. I would argue that these should be const pointers, and that __builtin___clear_cache should take const pointers, since c++-semantically the memory is not changed... right? Hm again. Hard to reason about.

While UB to cast from const here, since no actual writing is performed I kinda feel that it's not :P

In short, don't know what I want to do about this...

@hrydgard hrydgard added this to the v1.14.0 milestone Sep 15, 2022
@SpriteOvO
Copy link
Contributor Author

SpriteOvO commented Sep 15, 2022

Interesting point! "Flushing" is not exactly "modifying", and I also can't find any more information to argue either one is more correct.

Should I update this PR now just converting the const pointers to (void *)?

@hrydgard
Copy link
Owner

Yeah, do that.

@SpriteOvO SpriteOvO marked this pull request as draft September 15, 2022 15:51
@SpriteOvO SpriteOvO marked this pull request as ready for review September 15, 2022 16:14
@SpriteOvO
Copy link
Contributor Author

Compile passed on Arch Linux RISC-V, ready for review now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants