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

Support Windows x64 compiling #109

Open
Xemorph opened this issue Aug 21, 2021 · 15 comments
Open

Support Windows x64 compiling #109

Xemorph opened this issue Aug 21, 2021 · 15 comments
Labels
enhancement New feature or request

Comments

@Xemorph
Copy link

Xemorph commented Aug 21, 2021

Hey,
the current version of the tool "cpufetch" currently only supports 32 bit compilation under Windows. The 32 bit compiled program runs under Windows 32 bit as well as 64 bit.

For me personally I think it makes sense to remove the dependency to 32 bit. The following sourcecode files prevent a 64 bit compilation under Windows:

  • src/x86/cpuid_asm.h
  • src/x86/cpuid_asm.c

The error is exactly in the inline assembler instruction:

void cpuid(uint32_t *eax, uint32_t *ebx, uint32_t *ecx, uint32_t *edx) {
        __asm volatile("cpuid"
            : "=a" (*eax),
              "=b" (*ebx),
              "=c" (*ecx),
              "=d" (*edx)
            : "0" (*eax), "2" (*ecx));
}

Microsoft says the following about inline assembly instructions (source is attached as a link)

Note

Programs with inline assembler code are not fully portable to other hardware platforms. If you are designing for portability, avoid using inline assembler.

Inline assembly is not supported on the ARM and x64 processors. The following topics explain how to use the Visual C/C++ inline assembler with x86 processors:

Source: Microsoft Inline Assembler

At least for Windows the provided methods __cpuid and __cpuidex should be used. According to Microsoft these functions generate the appropriate assembly set for the respective selected architecture. Documentation for the mentioned functions: __cpuid, __cpuidex

I hope I haven't overwhelmed you with all the information 😳 What is your opinion?
Best regards, Derik

@Dr-Noob
Copy link
Owner

Dr-Noob commented Aug 21, 2021

First of all, thanks for your report and your investigation around the issue.

However, I can compile the program under 64 bit windows.
screenshot

This suggests that there is a problem in cpufetch, but it does not appear under all compilers/flags. I think that when you report a problem like this, you should always detail your compiler (and version) and the exact error that you get. This way, I can understand better the problem. In my case I'm using the gcc that comes with mingw, which I believe it is actually a 32 bit compiler. Maybe that is why my build success, but yours not. I'm a bit unsure about if the issue is because Windows is 64 bit or because the compiler you used is 64 bit, but it looks like it is the second scenario.

Which compiler are you using and what is the exact errror?

@Xemorph
Copy link
Author

Xemorph commented Aug 21, 2021

Hi,
Thank you very much for the quick answer. I also use MinGW, but my GCC runs on 64 bit (see my photo).

image

I can compile the program without problems, but I can't run it, nothing happens.
image

I could compile the program again with the MSVC compiler, but for that I would have to rewrite the whole Makefile once ... Because Microsoft unfortunately uses completely different parameters for their compiler ...

@Xemorph
Copy link
Author

Xemorph commented Aug 21, 2021

Please don't hate me ... I have now built your program with the official Microsoft build tools & get an error message when compiling.

Error:

cpuid_asm.c
src/x86/cpuid_asm.c(4): error C4235: nonstandard extension used: '__asm' keyword not supported on this architecture
src/x86/cpuid_asm.c(4): warning C4431: missing type specifier - int assumed. Note: C no longer supports default-int
src/x86/cpuid_asm.c(4): error C2059: syntax error: 'string'

I also attach the whole output of the compiler as a file for you.
I built the program with the following parameters, please note that I added getopt.h and getopt.c manually, because Windows does not know these header files.

Compile command:

cl /Wall /std:c17 /TC /Od /DARCH_X86 /MT src/common/main.c src/common/cpu.c src/common/getopt.c src/common/udev.c src/common/printer.c src/common/args.c src/common/global.c src/x86/cpuid.c src/x86/apic.c src/x86/cpuid_asm.c src/x86/uarch.c /DEBUG /MACHINE:X64 /OUT:cpufetch_msvc.exe > msvc_output_cl.txt

msvc_output_cl.txt

@Dr-Noob
Copy link
Owner

Dr-Noob commented Aug 21, 2021

If the program builds but you get no output (as you showed in your screenshot), then cpufetch must have encountered an error and exited. It is pretty strange but may happen. You can run the program again with --verbose, which may print some additional messages that we can use to understand what is going on. I also had really bad experiences when compiling a C program with some compilers and I remember an issue with gcc in Windows which was very similar to this one. If you run the program with --verbose and you still dont get any output, I will try to install mingw64 and see the issue by myself.

About the second message, if it compiles with mingw and does not compile with mscv, I am not really interested in fixing it. I saw the log you attached and msvc reports lots and lots of warnings about things that (from my perspective) are totally ok and that no other compiler would ever complain. I'm not going to invest hours and hours to check what msvc likes or does not like.

Can you try again with --verbose and see what happens? Thanks!

@Dr-Noob Dr-Noob added the enhancement New feature or request label Aug 21, 2021
@Xemorph
Copy link
Author

Xemorph commented Aug 21, 2021

I can run the program again later with the --verbose option, I would build a separate environment for it again, so I don't collide with MSVC now.

The warnings from MSVC are normal, a lot is just Windows specific, like padding. I will create a fork of your project and take care of MSVC, maybe I can find a way to keep everything 🤷


Update:
I compiled the program again with Mingw64 & with GCC 64bit. Afterwards I executed the program with the option --verbose ... Unfortunately the console remains empty.

C:\Users\Benny Nystroem\Documents\AAA_Zielstruktur\AAA_Environment\cpufetcher.master.env\cpufetch>cpufetch.exe --verbose

C:\Users\Benny Nystroem\Documents\AAA_Zielstruktur\AAA_Environment\cpufetcher.master.env\cpufetch>

@Xemorph
Copy link
Author

Xemorph commented Aug 21, 2021

Okay, wow. I managed to make the program compatible with the MSVC compiler. And the program works too! A 64bit compiled program, without inline assembler instructions.
I hope the provided exe-file is portable, if not, just ping me.

I show here the changes to the files cpuid_asm.h & cpuid_asm.c - Attention: There are more changes needed to compile it successfully!

Content of cpuid_asm.h

#ifndef __CPUID_ASM__
#define __CPUID_ASM__

/// We're going for a cross-platform solution and yes,
/// Windows has it's own rules to deal with cpuid ... Thanks Microsoft!
#ifdef _WIN32
#include <limits.h>
#include <intrin.h>
typedef unsigned __int32  uint32_t;
#else
#include <stdint.h>
#endif

void cpuid(uint32_t *eax, uint32_t *ebx, uint32_t *ecx, uint32_t *edx);

#endif

Content of cpuid_asm.c

#include "cpuid_asm.h"

void cpuid(uint32_t *eax, uint32_t *ebx, uint32_t *ecx, uint32_t *edx) {
#ifdef _WIN32
  // Array as register (EAX, EBX, ECX, EDX)
  uint32_t regs[4];
  /// Call Windows-specific cpuid function, we're using the extended one
  __cpuidex((int *)regs, (int)*eax, (int)*ecx);
  /// Now let's debug our results
  printf("- CPUID dump: 0x%.8X\n", regs[0]); // regs[0] = eax
  /// Bind result to the arguments, I hope this works
  (*eax) = regs[0];
  (*ebx) = regs[1];
  (*ecx) = regs[2];
  (*edx) = regs[3];
#else
    __asm volatile("cpuid"
        : "=a" (*eax),
          "=b" (*ebx),
          "=c" (*ecx),
          "=d" (*edx),
        : "0" (*eax), "2" (*ecx));
#endif
}

Update:

OS: Windows 10 64bit
CPU: AMD Ryzen 9 (Zen2) 3900X @ 4.1 GHz (all cores) with 1.3v (Overclocked!!!)

image

Theoretically, the modified program code should compile without problems under Unix 🤔


cpufetch_x64_windows.zip

@Dr-Noob
Copy link
Owner

Dr-Noob commented Aug 22, 2021

Thanks for your effort and your results, Xemorph.

Before anything, I would like to investigate the first problem; why cpufetch compiled with mingw64 does not produce any output (and yes... this was the same problem I had some time ago). I just tried compiling and running it and I now I remember why I love windows so much.
screenshot2
The program segfaults even before starting. The first printf does not get printed. I searched on the internet and I found that it may be a problem with the stack. I tried increasing it (as indicated here) but the output is the same. I also tried to compile with your fixed code, but the same output again. Do you know what's going on?

Regarding your fixed code, I was thinking about compiling the program with mingw64 and trying your code to check if it works. I mean, if mingw64 works without your fix, maybe it is only useful for msvc? I will consider adding your fix to the master branch if I see it is useful.

@Xemorph
Copy link
Author

Xemorph commented Aug 22, 2021

UPDATE

Sorry, I made a mistake. I have split the Windows architecture again separately and destroyed the program. I have now fixed the errors and the changes work properly.

I will upload the whole changed source code in my fork, link follows!


UPDATE 2

But I am not happy with the changes at all, because the whole program code becomes completely invisible.
I will try to refactor the program code and find a better solution for the files cpuid_asm.h & cpuid_asm.c!

@Dr-Noob
Copy link
Owner

Dr-Noob commented Aug 22, 2021

UPDATE

Sorry, I made a mistake. I have split the Windows architecture again separately and destroyed the program. I have now fixed the errors and the changes work properly.

I will upload the whole changed source code in my fork, link follows!

UPDATE 2

But I am not happy with the changes at all, because the whole program code becomes completely invisible.
I will try to refactor the program code and find a better solution for the files cpuid_asm.h & cpuid_asm.c!

The changes that you made are not responsible for the segfault when you compiled with mingw64. I also had the segfault and the program itself works perfectly, so it is not our problem. Looks like it is something related to mingw64?

The summary right now is:

  • mingw (32 bits): Works
  • mingw (64 bits): Broken, probably due to mingw itself
  • msvc: Works with your patches

If you find anything new, please let me know! And also let's see how your patches go

@Xemorph
Copy link
Author

Xemorph commented Aug 22, 2021

@Dr-Noob , could you maybe test something for me? What happens if we take your source code v0.99 and take out the parameter -fstack-protector-all? Maybe the error is here ... Because the GCC under Mingw is only a port & not all functions were taken over completely.

Dr-Noob added a commit that referenced this issue Aug 22, 2021
@Dr-Noob
Copy link
Owner

Dr-Noob commented Aug 22, 2021

That was a great idea! Yeah, this was the problem in mingw64, removing -fstack-protector-all solves the issue. It looks like this issue comes from a long time (for example, see bitcoin/bitcoin#8732).

I have updated the Makefile to fix this problem. Now you should be able to compile with mingw64 and run it without problems.

Now we have:

  • mingw (32 bits): Works
  • mingw (64 bits): Works
  • msvc: Works with your patches

@Xemorph
Copy link
Author

Xemorph commented Aug 22, 2021

Nice 👍 Your hint that it could be the stack gave me the idea to throw out the stack protector.

Can we keep this issue open? Because I am still tinkering here to be able to provide people with an executable file for Windows again.

@Dr-Noob
Copy link
Owner

Dr-Noob commented Aug 22, 2021

Sure, I will keep this open, I'm still unsure if I will include the code needed to be able to compile with msvc.

By the way, you look like know a lot about Windows stuff. Are you interested in giving a helping hand in #95? Maybe you know a better approach than winget?

One last thing, do you have issues compiling with mingw with the default Makefile? At least in my case, it complains that cc command does not exist; The CC variable is set to cc but the command itself is not found, so I have to manually replace the CC ?= gcc by CC = gcc. I can fix it, but I don't know if it is a general problem or just mine.

@Xemorph
Copy link
Author

Xemorph commented Aug 22, 2021

One last thing, do you have issues compiling with mingw with the default Makefile? At least in my case, it complains that cc command does not exist; The CC variable is set to cc but the command itself is not found, so I have to manually replace the CC ?= gcc by CC = gcc. I can fix it, but I don't know if it is a general problem or just mine.

Ah, that problem. I have the problem too, but again this is apparently a bug in MinGW. Because the expression CC ?= gcc says: If the variable CC is not set, then initialize it with the value gcc.

I just changed the expression to CC = gcc, because I didn't want to get any bad surprises.


By the way, you look like know a lot about Windows stuff. Are you interested in giving a helping hand in #95? Maybe you know a better approach than winget?

winget is a completely new tool from Microsoft for the Windows 10 operating system. It is actually very handy because it works like a package manager under Unix.

I would put the issue on the back burner for now, because the Windows version is not really stable & the effort would be too big. That's why I'm currently writing a separate version of cpufetch that will be properly cross-platform (but that will take time 😆 ).

@Xemorph
Copy link
Author

Xemorph commented Aug 26, 2021

Hey,

I've done a bit of work on the cpufetch tool and changed a lot of things, please note that I'm still developing.
First of all I separated the assembler part properly & put it into a submodule, so I can realize changes faster. Internal assembler code in C-files is just a nogo.
Furthermore I changed the build system (make) completely, because I wanted to support all current compilers.

You can find all the changes in my fork under the branch: https://github.com/Xemorph/cpufetch/tree/playground/v2

If you have any questions, please don't hesitate to contact me.

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

No branches or pull requests

2 participants