unicorn-engine / unicorn

Unicorn CPU emulator framework (ARM, AArch64, M68K, Mips, Sparc, PowerPC, RiscV, S390x, TriCore, X86)
http://www.unicorn-engine.org
GNU General Public License v2.0
7.33k stars 1.31k forks source link

cmake: set C standard to GNU C due to inline assembly usage #1875

Closed magiruuvelvet closed 10 months ago

magiruuvelvet commented 10 months ago

Standard ISO C does not have inline assembly support, but is a compiler extension. For compilers which default to ISO C, specify the use of GNU extensions to enable inline assembly.

Fixes compiler errors like this:

error: use of undeclared identifier 'asm'

I'm opening this pull request as a package maintainer, rather than a user of this library. Please feel free to ignore this pull request if it is not fitting for this project. I just want to bring attention to this issue and hopefully safe someone else a couple of headaches. I'm not even sure if this is the proper location to add this flag.

For reference: I'm using clang with extensions disabled by default, when -std= is not specified on the command line.

EDIT: I should mention that this is a very niche use case, since you must actively change the C standard (when -std= is omitted from the command line) during the configure step when building your own copy of LLVM. No sane compiler distribution will be configured like this.

wtdcode commented 10 months ago

Thanks for the issue. Which package are you maintaining?

magiruuvelvet commented 10 months ago

Local packages for myself to run on my Steam Deck. I recently picked up Vita3K which makes use of unicorn. yuzu-emu also used unicorn in the past (removed by now), where I had the exact same problem during compilation.

wtdcode commented 10 months ago

This obviously conflicts with our:

set(CMAKE_C_STANDARD 11)

mlgiraud commented 10 months ago

The documentation says this:

When compiling in ISO C mode by GCC or Clang (e.g. with option -std=c11), __asm__ must be used instead of asm.

So replacing all asm usages by __asm__ should fix this i presume.

magiruuvelvet commented 10 months ago

I compiled the version in the master branch just fine with my toolchain. Downstream is simply outdated or may contain custom patches which break the passing of the -std=gnu... flag, so I guess this issue is invalid for this repository.

For that, I will just continue to provide my own patches for the packaging process. It's not that much of a hassle for me.

I found the CMAKE_C_EXTENSIONS variable in the CMake documentation, but it is supposed to be ON by default. I don't have custom patches integrated into my CMake which would overwrite the behavior of this variable.


Regarding the __asm__ comment. This snippet of code compiled just fine in ISO C mode.

int main() {
    __asm__("nop");
    return 0;
}