intel/numatop

Doesn't build on 32 bits platforms

Closed this issue · 21 comments

Hi,

I'm currently packaging numatop for Fedora:
https://bugzilla.redhat.com/show_bug.cgi?id=991221#c10

During the review it appeared that the build fails for the i686 architecture. Since this is a primary architecture, we have to try to support it (in this case even if numatop ends up saying "CPU is not supported!").

If it doesn't make any sense, we'll simply exclude this architecture.

Best Regards,
Dridi

yaoj commented

Hi Dridi,

Could you provide more information for what you said “the build fails for the i686 architecture”?

Do you mean you have changed the current Makefile to satisfy the build for i686 architecture, but it’s failed?

Thanks
Jin Yao

From: Dridi Boukelmoune [mailto:notifications@github.com]
Sent: Friday, September 06, 2013 3:26 PM
To: 01org/numatop
Subject: [numatop] Doesn't build on 32 bits platforms (#3)

Hi,

I'm currently packaging numatop for Fedora:
https://bugzilla.redhat.com/show_bug.cgi?id=991221#c10

During the review it appeared that the build fails for the i686 architecture. Since this is a primary architecture, we have to try to support it (in this case even if numatop ends up saying "CPU is not supported!").

If it doesn't make any sense, we'll simply exclude this architecture.

Best Regards,
Dridi


Reply to this email directly or view it on GitHubhttps://github.com//issues/3.

You can see a build log here:
http://kojipkgs.fedoraproject.org//work/tasks/5678/5865678/build.log

I haven't touched the makefile, but I do change some variables at build and install times:
https://bitbucket.org/dridi/fedora_packages/src/dabc4cbd938b2d6730216707635f1164473bf3af/numatop.spec

It expands the build command to:

make 'CFLAGS=-O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -grecord-gcc-switches -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -m32 -march=i686 -mtune=atom -fasynchronous-unwind-tables' -j5

Dridi

The CFLAGS override definitely breaks the build.

Without it it builds just fine:
http://koji.fedoraproject.org/koji/taskinfo?taskID=5905869

The reason I'm doing this is described here:
https://fedoraproject.org/wiki/Packaging:Guidelines#Compiler_flags

yaoj commented

The build log said:
common/util.c:322:2: error: inconsistent operand constraints in an 'asm'
__asm volatile(

The old code is:
cpuid(unsigned int _eax, unsigned int *ebx, unsigned int *ecx,
unsigned int *edx)
{
__asm volatile(
"cpuid\n\t"
:"=a" (_eax),
"=b" (_ebx),
"=c" (_ecx),
"=d" (_edx)
:"a" (_eax));
}

The problem I see is that it has a "=a" in the output list and an "a" in the input list. Maybe it’s not allowed in your packaging case. Now I substitute "a" in the input list by "0".

cpuid(unsigned int _eax, unsigned int *ebx, unsigned int *ecx,
unsigned int *edx)
{
__asm volatile(
"cpuid\n\t"
:"=a" (_eax),
"=b" (_ebx),
"=c" (_ecx),
"=d" (_edx)
:"0" (_eax));
}

I have updated the code in github. Would you like to try the new code to build package for fedora?

Thanks
Jin Yao

From: Dridi Boukelmoune [mailto:notifications@github.com]
Sent: Friday, September 06, 2013 4:40 PM
To: 01org/numatop
Cc: Jin, Yao
Subject: Re: [numatop] Doesn't build on 32 bits platforms (#3)

You can see a build log here:
http://kojipkgs.fedoraproject.org//work/tasks/5678/5865678/build.loghttp://kojipkgs.fedoraproject.org/work/tasks/5678/5865678/build.log

I haven't touched the makefile, but I do change some variables at build and install times:
https://bitbucket.org/dridi/fedora_packages/src/dabc4cbd938b2d6730216707635f1164473bf3af/numatop.spec

It expands the build command to:

make 'CFLAGS=-O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -grecord-gcc-switches -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -m32 -march=i686 -mtune=atom -fasynchronous-unwind-tables' -j5

Dridi


Reply to this email directly or view it on GitHubhttps://github.com//issues/3#issuecomment-23926453.

Hi,

Thanks, I've added the following patches in this build:
https://github.com/01org/numatop/commit/8e96874e8edc0ef3fd70498e95393a2888f2057e.diff
https://github.com/01org/numatop/commit/28e24ed9b0ef4a19c3ce91f766e1fd7926b0fa14.diff

But it's still failing :(
http://kojipkgs.fedoraproject.org//work/tasks/1414/5911414/build.log

However I can easily reproduce the failure on my machine by overriding the CFLAGS. I think you've missed my previous message.

yaoj commented

I see your previous message now. So does it mean the issue has been solved?

Thanks
Jin Yao

From: Dridi Boukelmoune [mailto:notifications@github.com]
Sent: Monday, September 09, 2013 2:03 PM
To: 01org/numatop
Cc: Jin, Yao
Subject: Re: [numatop] Doesn't build on 32 bits platforms (#3)

Hi,

Thanks, I've added the following patches in this build:
https://github.com/01org/numatop/commit/8e96874e8edc0ef3fd70498e95393a2888f2057e.diff
https://github.com/01org/numatop/commit/28e24ed9b0ef4a19c3ce91f766e1fd7926b0fa14.diff

But it's still failing :(
http://kojipkgs.fedoraproject.org//work/tasks/1414/5911414/build.loghttp://kojipkgs.fedoraproject.org/work/tasks/1414/5911414/build.log

However I can easily reproduce the failure on my machine by overriding the CFLAGS. I think you've missed my previous message.


Reply to this email directly or view it on GitHubhttps://github.com//issues/3#issuecomment-24044757.

No, I have to either use those flags or drop the i686 architecture. But the rule of thumb is to do my best as a packager to support all primary architectures.

yaoj commented

Now I use the __cpuid() in <cpuid.h> and update the code in github.

Could you try the package-build again?

Thanks
Jin Yao

From: Dridi Boukelmoune [mailto:notifications@github.com]
Sent: Monday, September 09, 2013 2:28 PM
To: 01org/numatop
Cc: Jin, Yao
Subject: Re: [numatop] Doesn't build on 32 bits platforms (#3)

No, I have to either use those flags or drop the i686 architecture. But the rule of thumb is to do my best as a packager to support all primary architectures.


Reply to this email directly or view it on GitHubhttps://github.com//issues/3#issuecomment-24046311.

yaoj commented

Does it work?

From: Jin, Yao
Sent: Monday, September 09, 2013 3:23 PM
To: '01org/numatop'; 01org/numatop
Subject: RE: [numatop] Doesn't build on 32 bits platforms (#3)

Now I use the __cpuid() in <cpuid.h> and update the code in github.

Could you try the package-build again?

Thanks
Jin Yao

From: Dridi Boukelmoune [mailto:notifications@github.com]
Sent: Monday, September 09, 2013 2:28 PM
To: 01org/numatop
Cc: Jin, Yao
Subject: Re: [numatop] Doesn't build on 32 bits platforms (#3)

No, I have to either use those flags or drop the i686 architecture. But the rule of thumb is to do my best as a packager to support all primary architectures.


Reply to this email directly or view it on GitHubhttps://github.com//issues/3#issuecomment-24046311.

Sorry, haven't tested it yet I'll tell you as soon as I find time to do it.

I've just noticed you're not using the CFLAGS for the numatop build (only for the individual object files). I suspect the C/asm code is just fine as is and it's only a build issue!
https://github.com/01org/numatop/blob/3d9caf75783b3165df319670827b14fabcf7343d/Makefile#L21

I'll try that and tell you whether it fixes the issue.

My assumption was wrong unfortunately, and even with your last commit it fails.

I have tested all the CFLAGS that are added during the build for Fedora, and I have found the single gcc arg that fails the build:

-specs=/usr/lib/rpm/redhat/redhat-hardened-cc1

I'll investigate this further.

yaoj commented

With the new patch, does the same build error still exist? I mean the following build error.

gcc -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -grecord-gcc-switches -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -m32 -march=i686 -mtune=atom -fasynchronous-unwind-tables -o os_perf.o -c common/os/os_perf.c
common/util.c: In function 'cpu_type_get':
common/util.c:322:2: error: inconsistent operand constraints in an 'asm'
__asm volatile(
^
common/util.c:322:2: error: inconsistent operand constraints in an 'asm'
__asm volatile(
^
make: *** [util.o] Error 1

I guess this build error is disappear, is it?

For why the arg “ -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 “ fails the build, sorry, I’m no idea.

Thanks
Jin Yao

From: Dridi Boukelmoune [mailto:notifications@github.com]
Sent: Wednesday, September 11, 2013 6:06 AM
To: 01org/numatop
Cc: Jin, Yao
Subject: Re: [numatop] Doesn't build on 32 bits platforms (#3)

My assumption was wrong unfortunately, and even with your last commit it fails.

I have tested all the CFLAGS that are added during the build for Fedora, and I have found the single gcc arg that fails the build:

-specs=/usr/lib/rpm/redhat/redhat-hardened-cc1

I'll investigate this further.


Reply to this email directly or view it on GitHubhttps://github.com//issues/3#issuecomment-24198679.

Hi,
I can confirm that the i686 build issue seems to be fixed. Thanks.

http://koji.fedoraproject.org/koji/taskinfo?taskID=5929748

I've run the 32bit version of numatop (on a 64 bit host), with the fixes in #4, and it seems to work fine.

yaoj commented

Hi,

I merged all of your commits. After that I used the “B_FALSE” to replace the “false” to initialize the variable “locked” in main() and remove the stdbool.h. That’s for removing the duplicated definition for boolean.

Thank you very much for your contribution.

Thanks
Jin Yao

From: keszybz [mailto:notifications@github.com]
Sent: Friday, September 13, 2013 5:39 AM
To: 01org/numatop
Cc: Jin, Yao
Subject: Re: [numatop] Doesn't build on 32 bits platforms (#3)

Hi,
I can confirm that the i686 build issue seems to be fixed. Thanks.

http://koji.fedoraproject.org/koji/taskinfo?taskID=5929748


Reply to this email directly or view it on GitHubhttps://github.com//issues/3#issuecomment-24358131.

That's good news !

When is the next release planned ?
In the mean time I can patch the current release in the package.

yaoj commented

Actually I have defined the features of next release. The new feature will be very useful for analyzing the NUMA performance for shared memory type application (e.g. oracle database). We will develop the next release in Q4 this year and hope it will be ready at the early of next year.

Thanks
Jin Yao

From: Dridi Boukelmoune [mailto:notifications@github.com]
Sent: Friday, September 13, 2013 4:47 PM
To: 01org/numatop
Cc: Jin, Yao
Subject: Re: [numatop] Doesn't build on 32 bits platforms (#3)

That's good news !

When is the next release planned ?
In the mean time I can patch the current release in the package.


Reply to this email directly or view it on GitHubhttps://github.com//issues/3#issuecomment-24381030.

Great, and thank you for your support.
Numatop should land in Fedora within two or three weeks !

yaoj commented

Great, thank you very much!

Thanks
Jin Yao

From: Dridi Boukelmoune [mailto:notifications@github.com]
Sent: Friday, September 13, 2013 10:25 PM
To: 01org/numatop
Cc: Jin, Yao
Subject: Re: [numatop] Doesn't build on 32 bits platforms (#3)

Great, and thank you for your support.
Numatop should land in Fedora within two or three weeks !


Reply to this email directly or view it on GitHubhttps://github.com//issues/3#issuecomment-24397983.

Like Dridi noticed, indeed I built a fedora scratch package, but for numactl, not numatop. And actually it still doesn't build out of the box with the hardening flags. I had it built locally, but I added -D__pic__ to CFLAGS, but the I thought that numatop built fine in koji, so I didn't mention that.

Why -D__pic__ is needed? We are compiling in PIE mode, but the gcc headers where cpuid is defined checks for __pic, but not pie. So this might be a bug in gcc, I don't know. So you might either consider taking pull request #5, or adding '#define pic 2' somewhere before the necessary includes, or prodding gcc maintainers to make __cpuid work with -fpie. Dunno.

yaoj commented

Hi,

I have merged this pull request to master. Thank you for your contribution.

Thanks
Jin Yao

From: keszybz [mailto:notifications@github.com]
Sent: Sunday, September 15, 2013 8:44 AM
To: 01org/numatop
Cc: Jin, Yao
Subject: Re: [numatop] Doesn't build on 32 bits platforms (#3)

Like Dridi noticed, indeed I built a fedora scratch package, but for numactl, not numatop. And actually it still doesn't build out of the box with the hardening flags. I had it built locally, but I added -D__pic__ to CFLAGS, but the I thought that numatop built fine in koji, so I didn't mention that.

Why -D__pic__ is needed? We are compiling in PIE mode, but the gcc headers where cpuid is defined checks for __pic, but not pie. So this might be a bug in gcc, I don't know. So you might either consider taking pull request #5#5, or adding '#define pic 2' somewhere before the necessary includes, or prodding gcc maintainers to make __cpuid work with -fpie. Dunno.


Reply to this email directly or view it on GitHubhttps://github.com//issues/3#issuecomment-24461990.