[Bug middle-end/79647] bogus warning when building openssl
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79647 Jakub Jelinek changed: What|Removed |Added CC||jakub at gcc dot gnu.org --- Comment #8 from Jakub Jelinek --- Not complaining about unrecognized -Wno-* unless something other has been reported is a feature.
[Bug middle-end/79647] bogus warning when building openssl
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79647 --- Comment #7 from Bernd Edlinger --- (In reply to Andrew Pinski from comment #1) > >The second warning is also bogus, the -Wno-parentheses-equality was accepted > before. > > Well this error message about an unknown warning option only happens if > there was another error message. is this a bug or a feature?
[Bug middle-end/79647] bogus warning when building openssl
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79647
--- Comment #6 from Martin Sebor ---
I'm not entirely sure. But the trouble with the test case is that it converts
between signed and unsigned integers with different sizes. The async_jobs
variable's type is int but memset takes size_t (which is unsigned long in
LP64). Constraining a signed variable to an unsigned range should work but in
complex code the unsigned range can easily change and become signed. The best
way to guarantee this doesn't happen is to make the variable unsigned. In the
test case, change async_jobs to unsigned and add the hunk below. That has the
same effect and avoids the warning.
}
+if (async_jobs > 9) {
+BIO_printf(bio_err,
+ "%s: too many async_jobs\n",
+ prog);
+goto opterr;
+}
Btw., there's another hidden instance of the same issue lurking here.
app_malloc is declared to take size as a signed int, and there's nothing to let
GCC know it's an allocation function. The function would be better declared
like so to help GCC find buffer overflows and detect excessive allocations.
void* app_malloc(size_t sz, const char *what) __attribute__ ((alloc_size (1)));
With this declaration (and without the int -> unsigned change above), GCC also
warns for the call to it, for the same reason:
apps/speed.c: In function ‘speed_main’:
apps/speed.c:1514:14: warning: argument 1 range [18446742819579101184,
18446744073709551032] exceeds maximum object size 9223372036854775807
[-Walloc-size-larger-than=]
In file included from apps/speed.c:36:0:
apps/apps.h:473:7: note: in a call to allocation function ‘app_malloc’ declared
here
[Bug middle-end/79647] bogus warning when building openssl
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79647
--- Comment #5 from Bernd Edlinger ---
When I tried to limit the memory allocation to reasonable size,
I triggered the warning again, with this modified check:
@@ -1393,6 +1393,12 @@ int speed_main(int argc, char **argv)
prog);
goto opterr;
}
+if (async_jobs < 0 || async_jobs > 9) {
+BIO_printf(bio_err,
+ "%s: too many async_jobs\n",
+ prog);
+goto opterr;
+}
#endif
break;
case OPT_MISALIGN:
Why is it not clear that async_jobs can't be negative?
[Bug middle-end/79647] bogus warning when building openssl
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79647
--- Comment #4 from Bernd Edlinger ---
(In reply to Martin Sebor from comment #3)
> @@ -19021,6 +19021,13 @@
> case OPT_ASYNCJOBS:
>
> async_jobs = atoi(opt_arg());
> + if (async_jobs < 0) {
> + BIO_printf(bio_err,
> + "%s: async_jobs must be non-negative\n",
> + prog);
> + goto opterr;
> + }
> +
> if (!ASYNC_is_capable()) {
> BIO_printf(bio_err,
> "%s: async_jobs specified but async not
Thanks for your suggestion, I took it verbatim and made
a pull request: https://github.com/openssl/openssl/pull/2693
[Bug middle-end/79647] bogus warning when building openssl
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79647
Martin Sebor changed:
What|Removed |Added
Status|UNCONFIRMED |RESOLVED
CC||msebor at gcc dot gnu.org
Resolution|--- |INVALID
Assignee|unassigned at gcc dot gnu.org |msebor at gcc dot
gnu.org
--- Comment #3 from Martin Sebor ---
I can reproduce the warning. The VRP dump shows the following, indicating the
warning is correctly interpreting the range information passed to it.
_2724: [18446744071562067968, +INF]
_1184: [18446742819579101184, 18446744073709551032]
...
speed_main (int argc, char * * argv)
...
long unsigned int _2724;
long unsigned int _1184;
...
[0.06%]:
if (async_jobs_550 != 0)
goto ; [25.00%]
...
[0.02%]:
loopargs_len.29_1198 = (unsigned int) async_jobs_550;
_1196 = loopargs_len.29_1198 * 584;
_2726 = (int) _1196;
loopargs_1193 = app_malloc (_2726, "array of loopargs");
_2724 = (long unsigned int) async_jobs_550;
_1184 = _2724 * 584;
memset (loopargs_1193, 0, _1184);
The call to memset above is introduced by jump threading from this one:
memset(loopargs, 0, loopargs_len * sizeof(loopargs_t));
for negative values of async_jobs (its type is int and its value is returned
from atoi()). I think the warning is justified (if not exactly clear(*)) and
indicative of a possible bug in the code. To avoid the warning, prevent
negative async_jobs values from reaching the memset, e.g., like so:
@@ -19021,6 +19021,13 @@
case OPT_ASYNCJOBS:
async_jobs = atoi(opt_arg());
+ if (async_jobs < 0) {
+ BIO_printf(bio_err,
+"%s: async_jobs must be non-negative\n",
+prog);
+ goto opterr;
+ }
+
if (!ASYNC_is_capable()) {
BIO_printf(bio_err,
"%s: async_jobs specified but async not
supported\n",
I'm resolving this report as invalid on that basis.
[*] The warning is unfortunately missing the inlining context which can make
some its instances hard to debug. A patch with a fix for that was submitted
last month but deferred until GCC 8
(https://gcc.gnu.org/ml/gcc-patches/2017-01/msg01994.html).
[Bug middle-end/79647] bogus warning when building openssl
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79647 --- Comment #2 from Andrew Pinski --- int async_jobs = 0; ... async_jobs = atoi(opt_arg()); loopargs_len = (async_jobs == 0 ? 1 : async_jobs); loopargs = app_malloc(loopargs_len * sizeof(loopargs_t), "array of loopargs"); memset(loopargs, 0, loopargs_len * sizeof(loopargs_t)); Actually in my mind there is a security hole here as the async_jobs is not checked and it is an user input.
