Navigation Menu

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

Jump to bottom

compiler error with icc on Linux #5096

Closed
aminiussi opened this issue Mar 13, 2020 · 14 comments
Closed

compiler error with icc on Linux #5096

aminiussi opened this issue Mar 13, 2020 · 14 comments
Labels
build needs-info

Comments

@aminiussi
Copy link

I did this

with CC=icc and:

[alainm@gemini curl-7.69.1]$ icc -v
icc version 19.1.0.166 (gcc version 8.3.0 compatibility)
[alainm@gemini curl-7.69.1]$ 

$CFLAGS=-axCOMMON-AVX512 ./configure --prefix=....
$make

I got:

[alainm@gemini curl-7.69.1]$ make
Making all in lib
make[1]: Entering directory `/beegfs/SCRATCH/alainm/install/curl-7.69.1/lib'
make  all-am
make[2]: Entering directory `/beegfs/SCRATCH/alainm/install/curl-7.69.1/lib'
  CC       libcurl_la-mime.lo
icc: command line remark #10148: option '-vec-report0' not supported
In file included from urldata.h(92),
                 from mime.c(30):
/usr/include/netinet/in.h(376): error: expected a type specifier
  extern uint16_t ntohs (uint16_t __netshort)
                  ^

In file included from urldata.h(92),
                 from mime.c(30):
/usr/include/netinet/in.h(380): error: expected a type specifier
  extern uint16_t htons (uint16_t __hostshort)
                  ^

compilation aborted for mime.c (code 2)
make[2]: *** [libcurl_la-mime.lo] Error 1
make[2]: Leaving directory `/beegfs/SCRATCH/alainm/install/curl-7.69.1/lib'
make[1]: *** [all] Error 2
make[1]: Leaving directory `/beegfs/SCRATCH/alainm/install/curl-7.69.1/lib'
make: *** [all-recursive] Error 1
[alainm@gemini curl-7.69.1]$

Once I preprocessed explicitly with -E, it appear that the compile code was:

[alainm@gemini lib]$ icc truc.c
In file included from urldata.h(91),
                 from mime.c(30):
/usr/include/netinet/in.h(376): error: expected a type specifier
  extern uint16_t curlx_ntohs((uint16_t __netshort))
                              ^

In file included from urldata.h(91),
                 from mime.c(30):
/usr/include/netinet/in.h(380): error: expected a type specifier
  extern uint16_t curlx_htons((uint16_t __hostshort))
                              ^

compilation aborted for truc.c (code 2)
[alainm@gemini lib]$

Which can be explained by:

[alainm@gemini curl-7.69.1]$ find . -type f -exec egrep "define.+ntohs" {} \; -print
#  define ntohs(a)      curlx_ntohs((a))
./lib/warnless.h

I expected the following

A nice build

curl/libcurl version

https://github.com/curl/curl/releases/download/curl-7_69_1/curl-7.69.1.tar.gz

operating system

Linux gemini 3.10.0-693.2.2.el7.x86_64 #1 SMP Tue Sep 12 22:26:13 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux
@bagder bagder changed the title BUILDING_WARNLESS_C add faulty () in /usr/include/netinet/in.h header compiler error with icc on Linux Mar 13, 2020
@bagder bagder added the build label Mar 13, 2020
@bagder
Copy link
Member

bagder commented Mar 13, 2020

That BUILDING_WARNLESS_C trick was added there in b735717 for the sake of icc it seems. Does it work if you simply not define it?

@aminiussi
Copy link
Author

Actually, I got it the other way around, it works if I define it.

@bagder
Copy link
Member

bagder commented Mar 13, 2020

Where do you define it then so that it works? Isn't the logic in warnless.c enough to get it defined for icc?

@bagder
Copy link
Member

bagder commented Mar 13, 2020

Ah, that define only works for warnless.c, while your error above is for mime.c...

@aminiussi
Copy link
Author

I define it through the CFLAGS in the configure script. Since the defines are in the warnless.h header, it looks ok so far.
Make check failed but that was a valgrind issue.

@bagder
Copy link
Member

bagder commented Mar 13, 2020

But why is that BUILDING_WARNLESS_C thing needed at all?

@aminiussi
Copy link
Author

Because without it, the

  extern uint16_t ntohs (uint16_t __netshort)

in '/usr/include/netinet/in.h' get rewritten into

  extern uint16_t curlx_htons((uint16_t __hostshort))

which is a syntax error.

Now, I've read that on some platforms. htons is a macro that triggers a problematic cast. So maybe rewriting it to curlx_htons mask the problematic cast ?

@bagder
Copy link
Member

bagder commented Mar 13, 2020

What happens if you remove those defines and build with this patch?

diff --git a/lib/warnless.h b/lib/warnless.h
index ea4c4395d..ab78f9448 100644
--- a/lib/warnless.h
+++ b/lib/warnless.h
@@ -92,21 +92,8 @@ void curlx_FD_ZERO(fd_set *fdset);
 
 unsigned short curlx_htons(unsigned short usnum);
 
 unsigned short curlx_ntohs(unsigned short usnum);
 
-#ifndef BUILDING_WARNLESS_C
-#  undef  FD_ISSET
-#  define FD_ISSET(a,b) curlx_FD_ISSET((a),(b))
-#  undef  FD_SET
-#  define FD_SET(a,b)   curlx_FD_SET((a),(b))
-#  undef  FD_ZERO
-#  define FD_ZERO(a)    curlx_FD_ZERO((a))
-#  undef  htons
-#  define htons(a)      curlx_htons((a))
-#  undef  ntohs
-#  define ntohs(a)      curlx_ntohs((a))
-#endif
-
 #endif /* __INTEL_COMPILER && __unix__ */
 
 #endif /* HEADER_CURL_WARNLESS_H */

@bagder
Copy link
Member

bagder commented Apr 5, 2020

@aminiussi we need your assistance with this issue or it will get closed. We don't have access to icc nor any CI or autobuilds using it.

@aminiussi
Copy link
Author

@bagder sorry, I'll test test the patch in the afternoon.

@aminiussi
Copy link
Author

Build with the patch wen ok on the last git pull.
There is just this recurring warning: icc: command line remark #10148: option '-vec-report0' not supported.

Not all test passes, but that is due to valgrind:

test 0117...sh: line 1: 130092 Illegal instruction     ../libtool --mode=execute /usr/bin/valgrind --tool=memcheck --quiet --leak-check=yes --suppressions=./valgrind.supp --num-callers=16 --log-file=log/valgrind117 ../src/curl --output log/curl117.out --include --trace-ascii log/trace117 --trace-time ftp://127.0.0.1:8992/117 > log/stdout117 2> log/stderr117
FAILED

Thx

@bagder
Copy link
Member

bagder commented Apr 6, 2020

-vec-report0 is added by configure with this comment "Disable vectorizer diagnostic information" and is noted "Only icc 10.0 or later". I suppose we should remove this too.

I'll cook up a proper PR with both these changes.

Test 117 has no problems with or without valgrind for others. Maybe a problem with icc and valgrind?

bagder added a commit that referenced this issue Apr 6, 2020
@bagder
warnless: remove code block for icc that didn't work
38fa610 
Reported-by: Alain Miniussi
Fixes #5096
bagder added a commit that referenced this issue Apr 6, 2020
@bagder
configure: remove use of -vec-report0 from CFLAGS with icc
36a36ca 
... as it apparently isn't (always) supported.
Reported-by: Alain Miniussi
Fixes #5096
@bagder bagder mentioned this issue Apr 6, 2020
Closed
@aminiussi
Copy link
Author

Test 117 has no problems with or without valgrind for others. Maybe a problem with icc and valgrind?

Yes, our centos distribution valgrind is not compatible with icc.

I just relaunched with locally compiled valgrind.

@aminiussi
Copy link
Author

Tests looks ok with a more recent valgrind.

bagder added a commit that referenced this issue Apr 6, 2020
@bagder
warnless: remove code block for icc that didn't work
ae57e27 
Reported-by: Alain Miniussi
Fixes #5096
bagder added a commit that referenced this issue Apr 6, 2020
@bagder
configure: remove use of -vec-report0 from CFLAGS with icc
efa830f 
... as it apparently isn't (always) supported.
Reported-by: Alain Miniussi
Fixes #5096
Closes #5191
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
build needs-info
Milestone
No milestone
Development

No branches or pull requests
2 participants
@bagder @aminiussi