Enable MSVC warning C4701, use of potentially uninitialized variable.
C4701 is "potentially uninitialized local variable 'buf' used". It
sometimes results in false positives, which can now be suppressed
using the macro OPENSSL_SUPPRESS_POTENTIALLY_UNINITIALIZED_WARNINGS.
Change-Id: I15068b5a48e1c704702e7752982b9ead855e7633
Reviewed-on: https://boringssl-review.googlesource.com/3160
Reviewed-by: Adam Langley <agl@google.com>
diff --git a/CMakeLists.txt b/CMakeLists.txt
index 0a63dbf..f55765c 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -45,7 +45,6 @@
# copy constructor is inaccessible or deleted
"C4626" # assignment operator could not be generated because a base class
# assignment operator is inaccessible or deleted
- "C4701" # potentially uninitialized local variable 'mdlen' used
"C4706" # assignment within conditional expression
"C4710" # 'function': function not inlined
"C4711" # function 'function' selected for inline expansion
diff --git a/crypto/asn1/tasn_dec.c b/crypto/asn1/tasn_dec.c
index d5a26ee..73d3bb3 100644
--- a/crypto/asn1/tasn_dec.c
+++ b/crypto/asn1/tasn_dec.c
@@ -63,6 +63,8 @@
#include <openssl/err.h>
#include <openssl/mem.h>
+#include "../internal.h"
+
static int asn1_check_eoc(const unsigned char **in, long len);
static int asn1_find_end(const unsigned char **in, long len, char inf);
@@ -762,6 +764,7 @@
const unsigned char **in, long inlen,
const ASN1_ITEM *it,
int tag, int aclass, char opt, ASN1_TLC *ctx)
+ OPENSSL_SUPPRESS_POTENTIALLY_UNINITIALIZED_WARNINGS
{
int ret = 0, utype;
long plen;
diff --git a/crypto/evp/digestsign.c b/crypto/evp/digestsign.c
index ced2d7f..c163d40 100644
--- a/crypto/evp/digestsign.c
+++ b/crypto/evp/digestsign.c
@@ -162,12 +162,12 @@
r = tmp_ctx.pctx->pmeth->signctx(tmp_ctx.pctx, out_sig, out_sig_len, &tmp_ctx);
} else {
r = EVP_DigestFinal_ex(&tmp_ctx, md, &mdlen);
+ if (r) {
+ r = EVP_PKEY_sign(ctx->pctx, out_sig, out_sig_len, md, mdlen);
+ }
}
EVP_MD_CTX_cleanup(&tmp_ctx);
- if (has_signctx || !r) {
- return r;
- }
- return EVP_PKEY_sign(ctx->pctx, out_sig, out_sig_len, md, mdlen);
+ return r;
} else {
if (has_signctx) {
return ctx->pctx->pmeth->signctx(ctx->pctx, out_sig, out_sig_len, ctx);
@@ -184,21 +184,21 @@
uint8_t md[EVP_MAX_MD_SIZE];
int r;
unsigned int mdlen;
- const int has_verifyctx = ctx->pctx->pmeth->verifyctx != NULL;
EVP_MD_CTX_init(&tmp_ctx);
if (!EVP_MD_CTX_copy_ex(&tmp_ctx, ctx)) {
return 0;
}
- if (has_verifyctx) {
+ if (ctx->pctx->pmeth->verifyctx) {
r = tmp_ctx.pctx->pmeth->verifyctx(tmp_ctx.pctx, sig, sig_len, &tmp_ctx);
} else {
r = EVP_DigestFinal_ex(&tmp_ctx, md, &mdlen);
+ if (r) {
+ r = EVP_PKEY_verify(ctx->pctx, sig, sig_len, md, mdlen);
+ }
}
EVP_MD_CTX_cleanup(&tmp_ctx);
- if (has_verifyctx || !r) {
- return r;
- }
- return EVP_PKEY_verify(ctx->pctx, sig, sig_len, md, mdlen);
+
+ return r;
}
diff --git a/crypto/internal.h b/crypto/internal.h
index 38f9820..e8f4ef7 100644
--- a/crypto/internal.h
+++ b/crypto/internal.h
@@ -120,6 +120,27 @@
#endif
+/* MSVC's C4701 warning about the use of *potentially*--as opposed to
+ * *definitely*--uninitialized values sometimes has false positives. Usually
+ * the false positives can and should be worked around by simplifying the
+ * control flow. When that is not practical, annotate the function containing
+ * the code that triggers the warning with
+ * OPENSSL_SUPPRESS_POTENTIALLY_UNINITIALIZED_WARNINGS after its parameters:
+ *
+ * void f() OPENSSL_SUPPRESS_POTENTIALLY_UNINITIALIZED_WARNINGS {
+ * ...
+ * }
+ *
+ * Note that MSVC's control flow analysis seems to operate on a whole-function
+ * basis, so the annotation must be placed on the entire function, not just a
+ * block within the function. */
+#if defined(_MSC_VER)
+#define OPENSSL_SUPPRESS_POTENTIALLY_UNINITIALIZED_WARNINGS \
+ __pragma(warning(suppress:4701))
+#else
+#define OPENSSL_SUPPRESS_POTENTIALLY_UNINITIALIZED_WARNINGS
+#endif
+
/* MSVC will sometimes correctly detect unreachable code and issue a warning,
* which breaks the build since we treat errors as warnings, in some rare cases
* where we want to allow the dead code to continue to exist. In these
diff --git a/crypto/pem/pem_lib.c b/crypto/pem/pem_lib.c
index ff72c44..48e3297 100644
--- a/crypto/pem/pem_lib.c
+++ b/crypto/pem/pem_lib.c
@@ -363,10 +363,11 @@
|| !EVP_EncryptUpdate(&ctx,data,&j,data,i)
|| !EVP_EncryptFinal_ex(&ctx,&(data[j]),&i))
ret = 0;
+ else
+ i += j;
EVP_CIPHER_CTX_cleanup(&ctx);
if (ret == 0)
goto err;
- i+=j;
}
else
{
diff --git a/crypto/rsa/rsa.c b/crypto/rsa/rsa.c
index 0823929..2de2add 100644
--- a/crypto/rsa/rsa.c
+++ b/crypto/rsa/rsa.c
@@ -361,10 +361,6 @@
int *is_alloced, int hash_nid, const uint8_t *msg,
size_t msg_len) {
unsigned i;
- const uint8_t* prefix = NULL;
- unsigned prefix_len;
- uint8_t *signed_msg;
- unsigned signed_msg_len;
if (hash_nid == NID_md5_sha1) {
/* Special case: SSL signature, just check the length. */
@@ -381,38 +377,39 @@
for (i = 0; kPKCS1SigPrefixes[i].nid != NID_undef; i++) {
const struct pkcs1_sig_prefix *sig_prefix = &kPKCS1SigPrefixes[i];
- if (sig_prefix->nid == hash_nid) {
- prefix = sig_prefix->bytes;
- prefix_len = sig_prefix->len;
- break;
+ if (sig_prefix->nid != hash_nid) {
+ continue;
}
+
+ const uint8_t* prefix = sig_prefix->bytes;
+ unsigned prefix_len = sig_prefix->len;
+ unsigned signed_msg_len;
+ uint8_t *signed_msg;
+
+ signed_msg_len = prefix_len + msg_len;
+ if (signed_msg_len < prefix_len) {
+ OPENSSL_PUT_ERROR(RSA, pkcs1_prefixed_msg, RSA_R_TOO_LONG);
+ return 0;
+ }
+
+ signed_msg = OPENSSL_malloc(signed_msg_len);
+ if (!signed_msg) {
+ OPENSSL_PUT_ERROR(RSA, pkcs1_prefixed_msg, ERR_R_MALLOC_FAILURE);
+ return 0;
+ }
+
+ memcpy(signed_msg, prefix, prefix_len);
+ memcpy(signed_msg + prefix_len, msg, msg_len);
+
+ *out_msg = signed_msg;
+ *out_msg_len = signed_msg_len;
+ *is_alloced = 1;
+
+ return 1;
}
- if (prefix == NULL) {
- OPENSSL_PUT_ERROR(RSA, pkcs1_prefixed_msg, RSA_R_UNKNOWN_ALGORITHM_TYPE);
- return 0;
- }
-
- signed_msg_len = prefix_len + msg_len;
- if (signed_msg_len < prefix_len) {
- OPENSSL_PUT_ERROR(RSA, pkcs1_prefixed_msg, RSA_R_TOO_LONG);
- return 0;
- }
-
- signed_msg = OPENSSL_malloc(signed_msg_len);
- if (!signed_msg) {
- OPENSSL_PUT_ERROR(RSA, pkcs1_prefixed_msg, ERR_R_MALLOC_FAILURE);
- return 0;
- }
-
- memcpy(signed_msg, prefix, prefix_len);
- memcpy(signed_msg + prefix_len, msg, msg_len);
-
- *out_msg = signed_msg;
- *out_msg_len = signed_msg_len;
- *is_alloced = 1;
-
- return 1;
+ OPENSSL_PUT_ERROR(RSA, pkcs1_prefixed_msg, RSA_R_UNKNOWN_ALGORITHM_TYPE);
+ return 0;
}
int RSA_sign(int hash_nid, const uint8_t *in, unsigned in_len, uint8_t *out,
diff --git a/crypto/x509/asn1_gen.c b/crypto/x509/asn1_gen.c
index e4ed22c..d4d1ee6 100644
--- a/crypto/x509/asn1_gen.c
+++ b/crypto/x509/asn1_gen.c
@@ -64,6 +64,8 @@
#include <openssl/obj.h>
#include <openssl/x509v3.h>
+#include "../internal.h"
+
/* Although this file is in crypto/x509 for layering purposes, it emits errors
* from the ASN.1 module for OpenSSL compatibility. */
@@ -141,6 +143,7 @@
}
ASN1_TYPE *ASN1_generate_v3(char *str, X509V3_CTX *cnf)
+ OPENSSL_SUPPRESS_POTENTIALLY_UNINITIALIZED_WARNINGS
{
ASN1_TYPE *ret;
tag_exp_arg asn1_tags;
diff --git a/crypto/x509/x509_vfy.c b/crypto/x509/x509_vfy.c
index 78e70a6..e556fa4 100644
--- a/crypto/x509/x509_vfy.c
+++ b/crypto/x509/x509_vfy.c
@@ -68,6 +68,8 @@
#include <openssl/x509v3.h>
#include "vpm_int.h"
+#include "../internal.h"
+
/* CRL score values */
@@ -813,6 +815,7 @@
}
static int check_cert(X509_STORE_CTX *ctx)
+ OPENSSL_SUPPRESS_POTENTIALLY_UNINITIALIZED_WARNINGS
{
X509_CRL *crl = NULL, *dcrl = NULL;
X509 *x;
diff --git a/crypto/x509v3/v3_conf.c b/crypto/x509v3/v3_conf.c
index 7606ac1..e3afaf1 100644
--- a/crypto/x509v3/v3_conf.c
+++ b/crypto/x509v3/v3_conf.c
@@ -67,6 +67,8 @@
#include <openssl/x509.h>
#include <openssl/x509v3.h>
+#include "../internal.h"
+
static int v3_check_critical(char **value);
static int v3_check_generic(char **value);
@@ -260,6 +262,7 @@
static X509_EXTENSION *v3_generic_extension(const char *ext, char *value,
int crit, int gen_type,
X509V3_CTX *ctx)
+ OPENSSL_SUPPRESS_POTENTIALLY_UNINITIALIZED_WARNINGS
{
unsigned char *ext_der=NULL;
long ext_len;
