On 2013-02-15 14:26, deeptech71 wrote: > In an effort to test out the latest version of Clang -- for supposedly increased performance and less crashes --, I've checked out and compiled a recent version Clang/LLVM (recompiled it with itself), compiled a new kernel and world using WITHOUT_GCC=1 and WITHOUT_CLANG=1, installed them, removed (almost) all remnants of the base GCC and base Clang binaries (/usr/{libexec/cc1{|plus}|bin/c{c|pp|++|++filt|89|99}} -- that ``make delete-old'' did not remove!) (keeping only the latest Clang installation), and recompiled the kernel and world again. This whole process was very annoying, not only due to compilation errors, but also due to makefile-related workings that I learned via the hard way. > > I'll describe the errors I've encountered, along with some workarounds I used to get the compiling process to move on. I've also looked at the compiler warnings, and noticed some frightening/valid ones. I expect committers to do something about both the errors and, where appropriate, the warnings (which perhaps means telling me what I did WRONG). Sorry, but you are doing something that is totally unsupported. Therefore, any broken pieces are yours to keep. :-) > First come the compilation errors. > > * /usr/src/lib/libc/net/nss_compat.c:154:18: error: comparison of constant 16 with expression of type 'enum nss_status' is always true [-Werror,-Wtautological-constant-out-of-range-compare] > * else if (status != NS_RETURN) > * ~~~~~~ ^ ~~~~~~~~~ > * /usr/src/lib/libc/net/nss_compat.c:255:18: error: comparison of constant 16 with expression of type 'enum nss_status' is always true [-Werror,-Wtautological-constant-out-of-range-compare] > * else if (status != NS_RETURN) > * ~~~~~~ ^ ~~~~~~~~~ These can safely be ignored. They are the result of a compability shim. It might be neater to just cast 'status' to int there, to silence the warning. > * /usr/src/lib/libsm/../../contrib/sendmail/libsm/test.c:110:7: error: promoted type 'int' of K&R function parameter is not compatible with the parameter type 'bool' declared in a previous prototype [-Werror,-Wknr-promoted-parameter] > * bool success; > * ^ > * /usr/src/lib/libsm/../../contrib/sendmail/include/sm/test.h:38:7: note: previous declaration is here > * bool _success, > * ^ > * /usr/include/sys/cdefs.h:136:21: note: expanded from macro '__P' > * #define __P(protos) protos /* full-blown ANSI C */ > * ^ > In short, the last error above also applies to: > /usr/src/lib/libsm/../../contrib/sendmail/libsm/signal.c:264:7 > /usr/src/lib/libsm/../../contrib/sendmail/libsm/sem.c:40:7 > /usr/src/lib/libsm/../../contrib/sendmail/libsm/sem.c:224:9 > /usr/src/lib/libsm/../../contrib/sendmail/libsm/sem.c:40:7 > /usr/src/lib/libsm/../../contrib/sendmail/libsm/shm.c:45:7 > /usr/src/lib/libsm/../../contrib/sendmail/include/sm/shm.h > /usr/src/lib/libsm/../../contrib/sendmail/libsm/shm.c:96:7 > /usr/src/lib/libsm/../../contrib/sendmail/libsm/shm.c:127:9 > /usr/src/libexec/mail.local/../../contrib/sendmail/mail.local/mail.local.c:439:7 > /usr/src/libexec/smrsh/../../contrib/sendmail/smrsh/smrsh.c:117:7 These can also be safely ignored. This is a side effect of the K&R declarations still used in sendmail. It is every unlikely these will ever go away, as sendmail is probably still supported on systems with very old compilers which require K&R. > * /usr/src/lib/libugidfw/ugidfw.c:74:10: error: comparison of unsigned expression < 0 is always false [-Werror,-Wtautological-compare] > * if (len < 0 || len > left) > * ~~~ ^ ~ > In short, the last error above is repeated numerous times for ``len < 0'' expressions. > > * /usr/src/sbin/fsck_ffs/pass1.c:141:17: error: comparison of unsigned expression > * < 0 is always false [-Werror,-Wtautological-compare] > * if (inosused < 0) > * ~~~~~~~~ ^ ~ Again, these can be safely ignored. These warnings really depend on the type of variable, and if you cannot predict in advance whether the type is signed or unsigned, it is too costly (too much churn) to fix them all. > * /usr/src/sbin/ifconfig/regdomain.c:350:25: error: comparison of constant 65535 with expression of type 'enum ISOCountryCode' is always false [-Werror,-Wtautological-constant-out-of-range-compare] > * if (mt->country->code == NO_COUNTRY) { > * ~~~~~~~~~~~~~~~~~ ^ ~~~~~~~~~~ This could be fixed by defining NO_COUNTRY as value in enum ISOCountryCode, but that up to the maintainer. Another solution is to cast mt->country->code to int. > * /usr/src/sbin/routed/rtquery/rtquery.c:388:4: error: array index 1 is past the end of the array (which contains 1 element) [-Werror,-Warray-bounds] > * OMSG.rip_nets[1] = OMSG.rip_nets[0]; > * ^ ~ > * /usr/src/sbin/routed/rtquery/rtquery.c:101:14: note: expanded from macro 'OMSG' > * #define OMSG omsg_buf.rip > * ^ > * /usr/include/protocols/routed.h:110:6: note: array 'ru_nets' declared here > * struct netinfo ru_nets[1]; > * ^ > * /usr/src/sbin/routed/rtquery/rtquery.c:395:4: error: array index 1 is past the end of the array (which contains 1 element) [-Werror,-Warray-bounds] > * OMSG.rip_nets[1] = OMSG.rip_nets[0]; > * ^ ~ > * /usr/src/sbin/routed/rtquery/rtquery.c:101:14: note: expanded from macro 'OMSG' > * #define OMSG omsg_buf.rip > * ^ > * /usr/include/protocols/routed.h:110:6: note: array 'ru_nets' declared here > * struct netinfo ru_nets[1]; > * ^ These are all due to old-style flexible array members in these programs. If somebody wants to spend the time to convert these to C99 flexible array members, these warnings would go away. Until then, they can be safely ignored. > * /usr/src/sbin/ipf/ipfs/../../../contrib/ipfilter/tools/ipfs.c:201:16: error: array index 2 is past the end of the array (which contains 2 elements) [-Werror,-Warray-bounds] > * if (!strncmp(nat->nat_ifnames[2], ifs, olen + 1)) { > * ^ ~ > * /usr/src/sbin/ipf/ipfs/../../../sys/contrib/ipfilter/netinet/ip_nat.h:125:2: note: array 'nat_ifnames' declared here > * char nat_ifnames[2][LIFNAMSIZ]; > * ^ > * /usr/src/sbin/ipf/ipfs/../../../contrib/ipfilter/tools/ipfs.c:202:11: error: array index 2 is past the end of the array (which contains 2 elements) [-Werror,-Warray-bounds] > * strcpy(nat->nat_ifnames[2], s); > * ^ ~ > * /usr/src/sbin/ipf/ipfs/../../../sys/contrib/ipfilter/netinet/ip_nat.h:125:2: note: array 'nat_ifnames' declared here > * char nat_ifnames[2][LIFNAMSIZ]; > * ^ > * /usr/src/sbin/ipf/ipfs/../../../contrib/ipfilter/tools/ipfs.c:205:16: error: array index 3 is past the end of the array (which contains 2 elements) [-Werror,-Warray-bounds] > * if (!strncmp(nat->nat_ifnames[3], ifs, olen + 1)) { > * ^ ~ > * /usr/src/sbin/ipf/ipfs/../../../sys/contrib/ipfilter/netinet/ip_nat.h:125:2: note: array 'nat_ifnames' declared here > * char nat_ifnames[2][LIFNAMSIZ]; > * ^ > * /usr/src/sbin/ipf/ipfs/../../../contrib/ipfilter/tools/ipfs.c:206:11: error: array index 3 is past the end of the array (which contains 2 elements) [-Werror,-Warray-bounds] > * strcpy(nat->nat_ifnames[3], s); > * ^ ~ > * /usr/src/sbin/ipf/ipfs/../../../sys/contrib/ipfilter/netinet/ip_nat.h:125:2: note: array 'nat_ifnames' declared here > * char nat_ifnames[2][LIFNAMSIZ]; > * ^ These could very well be real bugs in ipfilter, but I doubt anybody will ever fix them. In my opinion, ipfilter should be removed from the tree, as there has been no active maintenance for years. > Clang's standard headers conflicted somewhat with the system ones. > > * In file included from /usr/src/lib/libc/gen/err.c:39: > * /path/to/LLVM/installation/bin/../lib/clang/3.3/include/stdarg.h:47:9: error: '__GNUC_VA_LIST' macro redefined [-Werror] > * #define __GNUC_VA_LIST 1 > * ^ > * /usr/include/x86/_types.h:160:9: note: previous definition is here > * #define __GNUC_VA_LIST > * ^ > * In file included from /usr/src/lib/libc/gen/err.c:39: > * /path/to/LLVM/installation/bin/../lib/clang/3.3/include/stdarg.h:48:27: error: redefinition of typedef '__gnuc_va_list' is a C11 feature [-Werror,-Wtypedef-redefinition] > * typedef __builtin_va_list __gnuc_va_list; > * ^ > * /usr/include/x86/_types.h:161:20: note: previous definition is here > * typedef __va_list __gnuc_va_list; /* compatibility w/GNU headers*/ > * ^ > * In file included from /usr/src/lib/libc/gen/err.c:40: > * /usr/src/lib/libc/../../include/stdio.h:63:19: error: redefinition of typedef 'va_list' is a C11 feature [-Werror,-Wtypedef-redefinition] > * typedef __va_list va_list; > * ^ > * /path/to/LLVM/installation/bin/../lib/clang/3.3/include/stdarg.h:30:27: note: previous definition is here > * typedef __builtin_va_list va_list; > * ^ > > * In file included from /usr/src/lib/atf/libatf-c/../../../contrib/atf/atf-c/build.c:35: > * In file included from /usr/src/lib/atf/libatf-c/../../../contrib/atf/atf-c/error.h:34: > * /path/to/LLVM/installation/bin/../lib/clang/3.3/include/stddef.h:35:23: error: redefinition of typedef 'size_t' is a C11 feature [-Werror,-Wtypedef-redefinition] > * typedef __SIZE_TYPE__ size_t; > * ^ > * /usr/include/stdlib.h:48:18: note: previous definition is here > * typedef __size_t size_t; > * ^ > Also: redefinition of typedef wchar_t. > > * In file included from /usr/src/lib/libcam/scsi_cmdparse.c:51: > * In file included from /usr/src/lib/libcam/../../sys/cam/cam_ccb.h:42: > * In file included from /usr/src/lib/libcam/../../sys/cam/scsi/scsi_all.h:28: > * In file included from /usr/include/machine/stdarg.h:6: > * /usr/include/x86/stdarg.h:44:9: error: 'va_start' macro redefined [-Werror] > * #define va_start(ap, last) \ > * ^ > * /path/to/LLVM/installation/bin/../lib/clang/3.3/include/stdarg.h:34:9: note: previous definition is here > * #define va_start(ap, param) __builtin_va_start(ap, param) > * ^ > Also: redefinition of macros va_arg, __va_copy, and va_copy. Yes, this is precisely why we do not install those clang internal headers. They should either be removed from the port, or upstream should stop installing them on systems where these headers already exist. > Allow me to introduce -Wunsequenced, a recent addition to Clang/LLVM! > * /usr/src/usr.bin/mail/util.c:240:8: error: unsequenced modification and access to 'dest' [-Werror,-Wunsequenced] > * *dest++ = tolower((unsigned char)*dest); > * ^ ~~~~ Yes, this is real undefined behaviour. I will commit a fix later today. > (Including invocation information...) > * lint -cghapbx -Cposix /usr/src/usr.bin/xlint/llib/llib-lposix > * llib-lposix: > * lint: cannot exec /usr/obj/usr/src/tmp/usr/bin/cc: No such file or directory > This can be fixed by adding a CC environment variable: In usr.bin/xlint/llib/Makefile, change > ${LINT} ${LINTFLAGS} -Cposix ${.ALLSRC} > to > CC=${CC:Q} ${LINT} ${LINTFLAGS} -Cposix ${.ALLSRC} > With this, linting proceeds: > * stdarg.h(30): syntax error [249] > * llib-lposix(307): syntax error [249] > * llib-lposix(308): syntax error [249] > * llib-lposix(309): syntax error [249] > Also, the same execution error, fix, and linting error applies to llib-lstdc instead of llib-lposix as well. This might be nice to fix, indeed. But it is not critical at this point; you can either do buildworld WITHOUT_GCC or WITHOUT_CLANG, but not both yet. > A crappy workaround patch for the incompatibilities (or whatever) is at the end of this e-mail. > > * clang: error: argument unused during compilation: '-fformat-extensions' [-Werror,-Wunused-command-line-argument] > However, with -fformat-extensions plainly removed from file sys/conf/kern.mk, compilation bumps into -Wformat, -Wformat-invalid-specifier, and -Wformat-extra-args. There is no easy workaround for this issue. Either the FreeBSD specific format string extensions should be submitted upstream (which requires them to be greatly cleaned up), or any non-standard format strings should be removed from the kernel (which is a lot of work and churn, for not too much gain). > * /usr/src/sys/modules/wlan/../../net80211/ieee80211_mesh.c:2628:25: error: 'memset' call operates on objects of type 'struct ieee80211_meshgann_ie' while the size is based on a different type 'struct ieee80211_meshgann_ie *' [-Werror,-Wsizeof-pointer-memaccess] > * memset(ie, 0, sizeof(ie)); > * ~~ ^~ > * /usr/src/sys/modules/wlan/../../net80211/ieee80211_mesh.c:2628:25: note: did you mean to dereference the argument to 'sizeof' (and multiply it by the number of elements)? > * memset(ie, 0, sizeof(ie)); > * ^~ That is a real bug, and should be fixed. I will check with the maintainer. > During ``make installworld'': > * btxld: Command not found. > I had to append not only ``btxld'', but also ``ls dd cp'', to the ITOOLS variable in Makefile.inc1. There are apparently several things that can trigger that btxld error, but the usual one is that your system clock is out of whack. I have never researched it too deeply; maybe somebody else can chip in here. > On a side-note, around this point I found out that the CC setting in /etc/make.conf should be absolute, instead of having the compiler in the PATH of the user's shell. The user's PATH is ignored during buildworld, as the first thing the top-level Makefile does is reset it to "/sbin:/bin:/usr/sbin:/usr/bin". Absolute paths in ${CC} and ${CXX} will currently not work, for various reasons. Don't count on it getting fixed very soon. > I also find it WRONG that there is some compilation going on during ``make installworld''. Normally, there should not be any compilation during installworld. It is probably because you are using non-standard settings, and certain components which are required for installworld are not there, because you turned them off. > And now, here come the warnings. Some (all?) of these should have been errors, but Clang was instructed to ignore these. > > * /usr/src/lib/libbsnmp/libbsnmp/../../../contrib/bsnmp/lib/snmpagent.c:964:23: warning: variable 'len' is uninitialized when used here [-Wuninitialized] > * if (pdu_b->asn_len < len) > * ^~~ That seems to be a real bug. I am not sure at this moment what the correct fix is. > K&R incompatibility warnings happen at: > /usr/src/lib/libmilter/../../contrib/sendmail/libmilter/main.c:117:7 > /usr/src/lib/libmilter/../../contrib/sendmail/libmilter/listener.c:63:7: > /usr/src/lib/libmilter/../../contrib/sendmail/libmilter/listener.c:125:7: > /usr/src/usr.bin/vacation/../../contrib/sendmail/vacation/vacation.c:503:7: > 110 other K&R warnings from source files in /usr/src/usr.sbin/sendmail/../../contrib/sendmail/src/ Can be ignored. > * /usr/src/gnu/usr.bin/gdb/libgdb/../../../../contrib/gdb/gdb/dwarf2-frame.c:1364:42: warning: unsequenced modification and access to 'buf' [-Wunsequenced] > * buf += size_of_encoded_value (*buf++); > * ~~ ^ > > * /usr/src/usr.bin/vi/../../contrib/nvi/ex/ex_txt.c:402:10: warning: unsequenced modification and access to 'scno' [-Wunsequenced] > * scno -= --scno % sw; > * ~~ ^ > > * /usr/src/usr.bin/vi/../../contrib/nvi/vi/v_txt.c:1960:13: warning: unsequenced modification and access to 'target' [-Wunsequenced] > * target -= --target % sw; > * ~~ ^ These are all bugs, and should be fixed. Contacting maintainer(s)... > * /usr/src/kerberos5/libexec/kcm/../../../crypto/heimdal/kcm/cache.c:105:34: warning: sizeof on array function parameter will return size of 'unsigned char *' instead of 'kcmuuid_t' (aka 'unsigned char [16]') [-Wsizeof-array-argument] > * if (memcmp(p->uuid, uuid, sizeof(uuid)) == 0) { > * ^ > * /usr/src/kerberos5/libexec/kcm/../../../crypto/heimdal/kcm/cache.c:90:17: note: declared here > * kcmuuid_t uuid, > * ^ > * /usr/src/kerberos5/libexec/kcm/../../../crypto/heimdal/kcm/cache.c:105:35: warning: 'memcmp' call operates on objects of type 'unsigned char' while the size is based on a different type 'unsigned char *' [-Wsizeof-pointer-memaccess] > * if (memcmp(p->uuid, uuid, sizeof(uuid)) == 0) { > * ~~~~ ^~~~ > * /usr/src/kerberos5/libexec/kcm/../../../crypto/heimdal/kcm/cache.c:105:35: note: did you mean to * provide an explicit length? > * if (memcmp(p->uuid, uuid, sizeof(uuid)) == 0) { > * ^~~~ > ZOMG ! a bug in an encryption system ! Yes, that is also a bug, and should be fixed. Contacting maintainer. > * /usr/src/usr.bin/opiekey/../../contrib/opie/opiekey.c:112:32: warning: 'memset' call operates on objects of type 'char' while the size is based on a different type 'char *' [-Wsizeof-pointer-memaccess] > * memset(secret, 0, sizeof(secret)); > * ~~~~~~ ^~~~~~ > * /usr/src/usr.bin/opiekey/../../contrib/opie/opiekey.c:112:32: note: did you mean to provide an explicit length? > * memset(secret, 0, sizeof(secret)); > * ^~~~~~ > * /usr/src/usr.bin/opiekey/../../contrib/opie/opiekey.c:118:32: warning: 'memset' call operates on objects of type 'char' while the size is based on a different type 'char *' [-Wsizeof-pointer-memaccess] > * memset(secret, 0, sizeof(secret)); > * ~~~~~~ ^~~~~~ > * /usr/src/usr.bin/opiekey/../../contrib/opie/opiekey.c:118:32: note: did you mean to provide an explicit length? > * memset(secret, 0, sizeof(secret)); > * ^~~~~~ > * /usr/src/usr.bin/opiekey/../../contrib/opie/opiekey.c:124:30: warning: 'memset' call operates on objects of type 'char' while the size is based on a different type 'char *' [-Wsizeof-pointer-memaccess] > * memset(secret, 0, sizeof(secret)); > * ~~~~~~ ^~~~~~ > * /usr/src/usr.bin/opiekey/../../contrib/opie/opiekey.c:124:30: note: did you mean to provide an explicit length? > * memset(secret, 0, sizeof(secret)); > * ^~~~~~ > ZOMG ! another one ! Yes, another one. The size should probably be OPIE_SECRET_MAX+1 instead. > * /usr/src/usr.sbin/wpa/wpa_passphrase/../../../contrib/wpa//src/crypto/md5-internal.c:191:30: warning: 'os_memset' call operates on objects of type 'struct MD5Context' while the size is based on a different type 'struct MD5Context *' [-Wsizeof-pointer-memaccess] > * os_memset(ctx, 0, sizeof(ctx)); /* In case it's sensitive */ > * ~~~ ^~~ > * /usr/src/usr.sbin/wpa/wpa_passphrase/../../../contrib/wpa//src/crypto/md5-internal.c:191:30: note: did you mean to dereference the argument to 'sizeof' (and multiply it by the number of elements)? > * os_memset(ctx, 0, sizeof(ctx)); /* In case it's sensitive */ > * ^~~ > There just has to be an organization behind these backdoors... No, this is just a very common error. The idiom is either: memset(&object, 0, sizeof object); or memset(&pointer, 0, sizeof *pointer); but apparently it is difficult to choose the right one. :) > * /usr/src/sys/boot/zfs/zfsimpl.c:2077:19: warning: array index 264 is past the end of the array (which contains 192 elements) [-Warray-bounds] > * memcpy(path, &dn.dn_bonus[sizeof(znode_phys_t)], > * ^ ~~~~~~~~~~~~~~~~~~~~ > * /usr/src/sys/boot/zfs/../../cddl/boot/zfs/zfsimpl.h:792:2: note: array 'dn_bonus' declared here > * uint8_t dn_bonus[DN_MAX_BONUSLEN - sizeof (blkptr_t)]; > * ^ I have seen this one before, but I am not sure if it is a real problem. This should be inspected by the ZFS maintainers. > * /usr/src/usr.bin/kdump/kdump.c:1669:19: warning: format specifies type 'uintmax_t' (aka 'unsigned long long') but the argument has type 'vm_offset_t' (aka 'unsigned int') [-Wformat] > * printf("0x%jx ", ktr->vaddr); > * ~~~ ^~~~~~~~~~ > * %x For the %j format, one usually casts the argument explicitly to uintmax_t. I think this will be harmless here. > * /usr/src/usr.sbin/bsnmpd/tools/libbsnmptools/bsnmpimport.c:778:34: warning: size argument in 'strlcpy' call appears to be size of the source; expected the size of the destination [-Wstrlcpy-strlcat-size] > * strlcpy(string, nexttok, strlen(nexttok) + 1); > * ~~~~~~~^~~~~~~~~~~~ > In short, the last error above applies to: > /usr/src/usr.sbin/bsnmpd/tools/libbsnmptools/bsnmpmap.c:286:44: > /usr/src/usr.sbin/bsnmpd/tools/libbsnmptools/bsnmpmap.c:572:38: > /usr/src/usr.sbin/bsnmpd/tools/libbsnmptools/bsnmptools.c:269:36: > /usr/src/usr.sbin/bsnmpd/tools/libbsnmptools/bsnmpimport.c:778:34: > /usr/src/usr.sbin/bsnmpd/tools/libbsnmptools/bsnmpmap.c:286:44: > /usr/src/usr.sbin/bsnmpd/tools/libbsnmptools/bsnmpmap.c:572:38: > /usr/src/usr.sbin/bsnmpd/tools/libbsnmptools/bsnmptools.c:269:36: > /usr/src/usr.sbin/bsnmpd/tools/libbsnmptools/bsnmpimport.c:778:34: > /usr/src/usr.sbin/bsnmpd/tools/libbsnmptools/bsnmpmap.c:286:44: > /usr/src/usr.sbin/bsnmpd/tools/libbsnmptools/bsnmpmap.c:572:38: > /usr/src/usr.sbin/bsnmpd/tools/libbsnmptools/bsnmptools.c:269:36: This seems to be a false positive. The destination string is malloc'd with a size of strlen(nexttok) + 1, so the length is correct. I think clang just triggers on the source token in the length argument. > And here is the crappy workaround patch (I hope I'm including every bit here): > > --- /path/to/LLVM/installation/lib/clang/3.3/include/limits.h > +++ /path/to/LLVM/installation/lib/clang/3.3/include/limits.h > _at__at_ -30,6 +30,7 _at__at_ > #if defined __GNUC__ && !defined _GCC_LIMITS_H_ > #define _GCC_LIMITS_H_ > #endif > +#include <x86/_limits.h> > > /* System headers include a number of constants from POSIX in <limits.h>. > Include it if we're hosted. */ > _at__at_ -114,4 +115,9 _at__at_ > #define ULONG_LONG_MAX (__LONG_LONG_MAX__*2ULL+1ULL) > #endif > > +#if __XSI_VISIBLE || __POSIX_VISIBLE >= 200809 > +#define LONG_BIT __LONG_BIT > +#define WORD_BIT __WORD_BIT > +#endif > + > #endif /* __CLANG_LIMITS_H */ This cannot be accepted by upstream, <x86/> is a FreeBSD specific include directory. Again, these headers should only be installed by llvm/clang if they are not already available on the system. -DimitryReceived on Fri Feb 15 2013 - 14:57:58 UTC
This archive was generated by hypermail 2.4.0 : Wed May 19 2021 - 11:40:34 UTC