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

BUG: compile error on MIPS architecture #274

Closed
pudh4418 opened this issue Jul 26, 2020 · 11 comments · Fixed by #279
Closed

BUG: compile error on MIPS architecture #274

pudh4418 opened this issue Jul 26, 2020 · 11 comments · Fixed by #279
Assignees
Milestone

Comments

@pudh4418
Copy link

Hi,

Compiling libseccomp v2.5.0 on MIPS will bring errors like this:

libtool: compile:  mips64el-unknown-linux-gnuabi64-gcc -DHAVE_CONFIG_H -I. -I.. -I../include -I../include -Wall -g -O2 -fPIC -DPIC -fvisibility=hidden -g -O2 -c arch-x86.c  -fPIC -DPIC -o .libs/libseccomp_la-arch-x86.o
syscalls.h:44:6: error: expected identifier or '(' before numeric constant
   44 |  int mips;
      |      ^~~~

This error will also appear when cross compiling MIPS lib from x86-64. A preliminary analysis shows that GCC defines mips for MIPS architecture, thus the code becomes int 1; .
Maybe we can change it to a different name?

Thanks.

@pcmoore pcmoore added this to the v2.5.1 milestone Jul 26, 2020
@pcmoore
Copy link
Member

pcmoore commented Jul 26, 2020

Thanks for the report @pudh4418, I'll take a look.

@pcmoore pcmoore self-assigned this Jul 26, 2020
pcmoore added a commit to pcmoore/misc-libseccomp that referenced this issue Jul 26, 2020
It turns out that the MIPS GCC compiler defines the "mips" symbol
which was resulting in build failures on MIPS so we need to
namespace the arch/ABI fields in the arch_syscall_table struct.

This was reported in the following GH issue:
 * seccomp#274

Signed-off-by: Paul Moore <paul@paul-moore.com>
@pcmoore
Copy link
Member

pcmoore commented Jul 26, 2020

Hi @pudh4418, can you check the commit below? I think it should fix your problem:

Also, I'd like to give you credit for reporting the bug in the commit via the Reported-by: tag, but in order to do that I need your name and email. If you don't want to share the information, that's fine too, I'll leave the commit as it is now.

@pudh4418
Copy link
Author

It seems the fix is not that easy. After applying your patch, error messages turn to:

libtool: compile:  mips64el-unknown-linux-gnuabi64-gcc -DHAVE_CONFIG_H -I. -I.. -I../include -I../include -Wall -g -O2 -fPIC -DPIC -fvisibility=hidden -g -O2 -c arch-s390x.c  -fPIC -DPIC -o libseccomp_la-arch-s390x.o >/dev/null 2>&1
In file included from arch.h:26,
                 from syscalls.c:25:
syscalls.c: In function 'mips_syscall_resolve_name':
syscalls.h:55:63: error: 'struct arch_syscall_table' has no member named 'arch_1'; did you mean 'arch_x86'?
   55 | #define OFFSET_ARCH(NAME) offsetof(struct arch_syscall_table, arch_##NAME)
      |                                                               ^~~~~
syscalls.c:31:37: note: in expansion of macro 'OFFSET_ARCH'
   31 |   return syscall_resolve_name(name, OFFSET_ARCH(NAME)); \
      |                                     ^~~~~~~~~~~
syscalls.c:47:1: note: in expansion of macro 'ARCH_DEF'
   47 | ARCH_DEF(mips)
      | ^~~~~~~~
syscalls.c: In function 'mips_syscall_resolve_num':
syscalls.h:55:63: error: 'struct arch_syscall_table' has no member named 'arch_1'; did you mean 'arch_x86'?
   55 | #define OFFSET_ARCH(NAME) offsetof(struct arch_syscall_table, arch_##NAME)
      |                                                               ^~~~~
syscalls.c:35:35: note: in expansion of macro 'OFFSET_ARCH'
   35 |   return syscall_resolve_num(num, OFFSET_ARCH(NAME)); \
      |                                   ^~~~~~~~~~~
syscalls.c:47:1: note: in expansion of macro 'ARCH_DEF'
   47 | ARCH_DEF(mips)
      | ^~~~~~~~
syscalls.c: In function 'mips_syscall_iterate':
syscalls.h:55:63: error: 'struct arch_syscall_table' has no member named 'arch_1'; did you mean 'arch_x86'?
   55 | #define OFFSET_ARCH(NAME) offsetof(struct arch_syscall_table, arch_##NAME)
      |                                                               ^~~~~
syscalls.c:39:32: note: in expansion of macro 'OFFSET_ARCH'
   39 |   return syscall_iterate(spot, OFFSET_ARCH(NAME)); \
      |                                ^~~~~~~~~~~
syscalls.c:47:1: note: in expansion of macro 'ARCH_DEF'
   47 | ARCH_DEF(mips)
      | ^~~~~~~~
syscalls.c: In function 'mips_syscall_resolve_name':
syscalls.c:32:2: warning: control reaches end of non-void function [-Wreturn-type]
   32 |  } \
      |  ^
syscalls.c:47:1: note: in expansion of macro 'ARCH_DEF'
   47 | ARCH_DEF(mips)
      | ^~~~~~~~
syscalls.c: In function 'mips_syscall_resolve_num':
syscalls.c:36:2: warning: control reaches end of non-void function [-Wreturn-type]
   36 |  } \
      |  ^
syscalls.c:47:1: note: in expansion of macro 'ARCH_DEF'
   47 | ARCH_DEF(mips)
      | ^~~~~~~~
syscalls.c: In function 'mips_syscall_iterate':
syscalls.c:40:2: warning: control reaches end of non-void function [-Wreturn-type]
   40 |  }
      |  ^
syscalls.c:47:1: note: in expansion of macro 'ARCH_DEF'
   47 | ARCH_DEF(mips)
      | ^~~~~~~~
make[3]: *** [Makefile:971: libseccomp_la-syscalls.lo] Error 1

mips has already been substituted to 1 when ARCH_DEF was called.

You may put Reported-by: Rongwei Zhang <pudh4418@gmail.com> in the commit. Thanks.

@pcmoore
Copy link
Member

pcmoore commented Jul 26, 2020

Ungh, that's pretty awful.

Thanks for testing that out, let me see what I can do.

@pcmoore
Copy link
Member

pcmoore commented Jul 26, 2020

A preliminary analysis shows that GCC defines mips for MIPS architecture ...

As I'm working on this I just noticed that we use the "mips" string in a lot of places in the libseccomp code; I'm growing skeptical that GCC is defining a "mips" macro as that would mean this code never compiled (?!).

Would you mind checking you application and build environment @pudh4418 to make sure that something else isn't defining a "mips" macro somewhere?

pcmoore added a commit to pcmoore/misc-libseccomp that referenced this issue Jul 26, 2020
It turns out that the MIPS GCC compiler defines the "mips" symbol
which was resulting in build failures on MIPS so we need to
namespace the arch/ABI fields in the arch_syscall_table struct.

This was reported in the following GH issue:
 * seccomp#274

Reported-by: Rongwei Zhang <pudh4418@gmail.com>
Signed-off-by: Paul Moore <paul@paul-moore.com>
@pcmoore
Copy link
Member

pcmoore commented Jul 26, 2020

Some quick googling found this:

@pudh4418 can you try compiling libseccomp with --ansi or another --std option?

If this proves to be the case, this may be something we simply want to note in the README as obfuscating ever instance of the "mips" string in the code is going to be very annoying.

@pudh4418
Copy link
Author

Oh, I didn't find that post.

I tried compiling with --ansi, but the code uses lots of bool which only exists in C99 or above, it won't compile in strict ANSI mode (C89). I also tried compiling with --std=c99 or --std=c11, but it turns out the code uses endian conversion functions like htobe16() which are GNU extensions. These functions are only defined when compiling in GNU mode and that would bring mips into our scope, sadly.

Fortunately, it seems adding -Umips solves the problem. I think it's better to tweak autotools sutffs than to obfuscate every mips string in the code.

@pcmoore
Copy link
Member

pcmoore commented Jul 28, 2020

Fortunately, it seems adding -Umips solves the problem. I think it's better to tweak autotools sutffs than to obfuscate every mips string in the code.

Good find on -Umips, I agree that passing a compiler flag is the better option. I'll see about working up a fix for that.

pcmoore added a commit to pcmoore/misc-libseccomp that referenced this issue Aug 2, 2020
It turns out that the MIPS GCC compiler defines a "mips" cpp macro
which was resulting in build failures on MIPS so we need to
undefine the "mips" macro during build.  As this should be safe
to do in all architectures, just add it to the compiler flags by
default.

This was reported in the following GH issue:
* seccomp#274

Reported-by: Rongwei Zhang <pudh4418@gmail.com>
Suggested-by: Rongwei Zhang <pudh4418@gmail.com>
Signed-off-by: Paul Moore <paul@paul-moore.com>
@pcmoore
Copy link
Member

pcmoore commented Aug 2, 2020

Sorry about the delay, but I put together a proper PR to fix this with PR #279.

pcmoore added a commit to pcmoore/misc-libseccomp that referenced this issue Aug 4, 2020
It turns out that the MIPS GCC compiler defines a "mips" cpp macro
which was resulting in build failures on MIPS so we need to
undefine the "mips" macro during build.  As this should be safe
to do in all architectures, just add it to the compiler flags by
default.

This was reported in the following GH issue:
* seccomp#274

Reported-by: Rongwei Zhang <pudh4418@gmail.com>
Suggested-by: Rongwei Zhang <pudh4418@gmail.com>
Acked-by: Tom Hromatka <tom.hromatka@oracle.com>
Signed-off-by: Paul Moore <paul@paul-moore.com>
@debfx
Copy link
Contributor

debfx commented Oct 25, 2020

Hi @pcmoore, I noticed that this issue is marked for the 2.5.1 milestone but the fix is not part of the release-2.5 branch.

pcmoore added a commit that referenced this issue Oct 25, 2020
It turns out that the MIPS GCC compiler defines a "mips" cpp macro
which was resulting in build failures on MIPS so we need to
undefine the "mips" macro during build.  As this should be safe
to do in all architectures, just add it to the compiler flags by
default.

This was reported in the following GH issue:
* #274

Reported-by: Rongwei Zhang <pudh4418@gmail.com>
Suggested-by: Rongwei Zhang <pudh4418@gmail.com>
Acked-by: Tom Hromatka <tom.hromatka@oracle.com>
Signed-off-by: Paul Moore <paul@paul-moore.com>
(imported from commit 5cd9059)
@pcmoore
Copy link
Member

pcmoore commented Oct 25, 2020

Sorry about that @debfx, thanks for catching that! I just pushed the backport via 3e1a828.

LeSpocky pushed a commit to LeSpocky/ptxdist that referenced this issue Mar 2, 2021
This includes a fix for compiling on MIPS platforms:
<seccomp/libseccomp#274>

Signed-off-by: Roland Hieber <rhi@pengutronix.de>
Message-Id: <20210228231043.10783-1-rhi@pengutronix.de>
Signed-off-by: Michael Olbrich <m.olbrich@pengutronix.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants