Date: Sat, 16 Feb 2013 00:04:30 +0100 From: deeptech71 <deeptech71@gmail.com> To: freebsd-current@freebsd.org Subject: Re: the latest version of Clang/LLVM for the world and kernel Message-ID: <511EBEFE.5010406@gmail.com> In-Reply-To: <511E5B05.5050402@FreeBSD.org> References: <511E378E.3090200@gmail.com> <511E5B05.5050402@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On 02/15/2013 16:57, Dimitry Andric wrote:
> On 2013-02-15 14:26, deeptech71 wrote:
>> [...] an effort to test out the latest version of Clang [...]
>
> Sorry, but you are doing something that is totally unsupported.
> Therefore, any broken pieces are yours to keep. :-)
Another purpose is development. Developers will have to import the next version of Clang/LLVM sooner or later. Let's hedge against the developer-time requirements in case the next version of Clang gets released just before a particular version of FreeBSD, and someone decides to update the base Clang, with only a couple of days of time at hand. :P
>> 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.
First of all, you should understand that these are compilation errors (under an up-to-date version of Clang, at least), and that they should be fixed ("sooner or later", that is), whether by altering the program code or the makefiles.
Second of all, you should understand that these are real errors; in this case, the compiler may omit generating code to check for a boolean condition, if the value of that condition is known at compile time, and thus generating a program with an undesired mode of operation.
Finally, of course personally I can ignore these "warnings" (with whatever hack). However, as I said, I am not asking for support (ie., I do not say "help me get this compiler working !" -- I understand that there is little "consumer support" for what I'm doing); instead, I am simply reporting issues to the committers.
>> * /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.
Even excluding the fact that this is a compilation error (which can, for example, be silenced by passing appropriate flags to Clang), I doubt this should be "just ignored". If a parameter is interpreted as an int inside the function definition, then all source files using the relevant function should also pass an int, not a bool (even if there is no difference on a particular architecture). So, for example, bool should be changed to int in the prototype/declaration; or bool should be typedefed/preprocessed to int; or something like that.
>> 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/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.
In the 1st case, size_t is always unsigned, end of story. You may be right about the 2nd case; in fact, I would prefer to have a new compiler-specific type attribute: __may_have_different_signedness_on_other_platforms. But again, these are compilation errors, not warnings.
>> * /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.
Defining NO_COUNTRY is the way to go. Otherwise, as-is, if we cast mt->country->code to an int, then the result may never be 65535.
>> [header incompatibilities]
>
> 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.
I did not use a port, by the way; I checked out the Subversion repository at llvm.org.
My next "compilation run" will consist of first removing some of Clang's headers.
>> 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.
There is: -Wno-format, -Wno-format-invalid-specifier, and -Wno-format-extra-args.
> Absolute paths in ${CC} and ${CXX} will currently not work, for various
> reasons. Don't count on it getting fixed very soon.
s/Absolute/Relative/
I did not.
>> * /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...
>
> this is just a very common error.
Which is exactly why it is reasonable to do intentionally -- even if/when found, you will always be able to argue that the error was done by accident, so you will not be held accountable for intentionally writing malicious code.
> 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. :)
I assume you intentionally got the 2nd one WRONG.
>> * /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.
Then use strcpy()?
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?511EBEFE.5010406>