ARM and -Wextra fixups (#19141)

* Make generated code compile with -Wunused-parameters

-Wunused-parameters is turned on by -Werror -Wextra.  gRPC code
generation creates header files with unused parameters.  So let's modify
the generator to not generate code which will make -Wunused-parameters
unhappy.

* Fix unsigned vs signed comparisons and 32 bit string formats

Fix unsigned vs signed comparison warnings.

For 64 bit numbers printed in gRPC, the string formats assume that you
are running on a 64 bit machine.  Use inttypes.h to make it portable.
Also, use size_t format strings for the same reason.

* Fix unaligned memory access

cost_entry_ptr has no alignment guarentees that ubsan can find.  So it
fails the test with an alignment problem.  Use memcopy to read the data
from the pointer to fix this.

* Fix undefined behavior with memcpy and memcmp

Passing in a 0 length piece of data and a null pointer is undefined
behavior.  If the length is 0, don't pass it in.  This fixes ubsan
failures.

* Clang-format

* Automated change: Fix sanity tests
pull/27488/head
Austin Schuh 3 years ago committed by GitHub
parent 0c82838860
commit d30404868f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
  1. 9
      src/core/ext/filters/load_reporting/server_load_reporting_filter.cc
  2. 4
      src/core/tsi/alts/crypt/aes_gcm.cc
  3. 5
      src/cpp/server/load_reporter/get_cpu_stats_linux.cc
  4. 36
      test/core/tsi/alts/crypt/aes_gcm_test.cc
  5. 4
      test/core/tsi/alts/crypt/gsec_test_util.cc
  6. 4
      test/cpp/qps/client.h
  7. 2
      test/cpp/server/load_reporter/load_reporter_test.cc
  8. 1
      third_party/address_sorting/address_sorting_posix.c

@ -200,8 +200,9 @@ void ServerLoadReportingCallData::StoreClientIpAndLrToken(const char* lr_token,
if (lr_token_len != 0) {
strncpy(cur_pos, lr_token, lr_token_len);
}
GPR_ASSERT(cur_pos + lr_token_len - client_ip_and_lr_token_ ==
long(client_ip_and_lr_token_len_));
GPR_ASSERT(
static_cast<size_t>(cur_pos + lr_token_len - client_ip_and_lr_token_) ==
client_ip_and_lr_token_len_);
}
grpc_filtered_mdelem ServerLoadReportingCallData::RecvInitialMetadataFilter(
@ -292,7 +293,9 @@ grpc_filtered_mdelem ServerLoadReportingCallData::SendTrailingMetadataFilter(
}
const double* cost_entry_ptr =
reinterpret_cast<const double*>(GRPC_SLICE_START_PTR(value));
double cost_value = *cost_entry_ptr++;
double cost_value;
memcpy(&cost_value, cost_entry_ptr, sizeof(double));
cost_entry_ptr++;
const char* cost_name = reinterpret_cast<const char*>(cost_entry_ptr);
const size_t cost_name_len = cost_entry_size - sizeof(double);
opencensus::stats::Record(

@ -559,7 +559,9 @@ static grpc_status_code gsec_aes_gcm_aead_crypter_decrypt_iovec(
if (!EVP_DecryptFinal_ex(aes_gcm_crypter->ctx, nullptr,
&bytes_written_temp)) {
aes_gcm_format_errors("Checking tag failed.", error_details);
memset(plaintext_vec.iov_base, 0x00, plaintext_vec.iov_len);
if (plaintext_vec.iov_base != nullptr) {
memset(plaintext_vec.iov_base, 0x00, plaintext_vec.iov_len);
}
return GRPC_STATUS_FAILED_PRECONDITION;
}
if (bytes_written_temp != 0) {

@ -20,6 +20,8 @@
#ifdef GPR_LINUX
#include <inttypes.h>
#include <cstdio>
#include "src/cpp/server/load_reporter/get_cpu_stats.h"
@ -32,7 +34,8 @@ std::pair<uint64_t, uint64_t> GetCpuStatsImpl() {
FILE* fp;
fp = fopen("/proc/stat", "r");
uint64_t user, nice, system, idle;
if (fscanf(fp, "cpu %lu %lu %lu %lu", &user, &nice, &system, &idle) != 4) {
if (fscanf(fp, "cpu %" PRIu64 " %" PRIu64 " %" PRIu64 " %" PRIu64, &user,
&nice, &system, &idle) != 4) {
// Something bad happened with the information, so assume it's all invalid
user = nice = system = idle = 0;
}

@ -120,7 +120,9 @@ static void gsec_test_random_encrypt_decrypt(gsec_aead_crypter* crypter,
GPR_ASSERT(status == GRPC_STATUS_OK);
GPR_ASSERT(message_length == plaintext_bytes_written);
GPR_ASSERT(memcmp(message, plaintext, message_length) == 0);
if (message_length != 0) {
GPR_ASSERT(memcmp(message, plaintext, message_length) == 0);
}
/**
* The returned plaintext will be zeroed if there was an authentication error.
@ -142,7 +144,9 @@ static void gsec_test_random_encrypt_decrypt(gsec_aead_crypter* crypter,
status, GRPC_STATUS_FAILED_PRECONDITION, "Checking tag failed.",
error_message));
GPR_ASSERT(plaintext_bytes_written == 0);
GPR_ASSERT(memcmp(zero_message, plaintext, plaintext_length) == 0);
if (plaintext_length != 0) {
GPR_ASSERT(memcmp(zero_message, plaintext, plaintext_length) == 0);
}
gpr_free(corrupt_nonce);
gpr_free(error_message);
}
@ -162,7 +166,9 @@ static void gsec_test_random_encrypt_decrypt(gsec_aead_crypter* crypter,
status, GRPC_STATUS_FAILED_PRECONDITION, error_message,
"Checking tag failed"));
GPR_ASSERT(plaintext_bytes_written == 0);
GPR_ASSERT(memcmp(zero_message, plaintext, plaintext_length) == 0);
if (plaintext_length != 0) {
GPR_ASSERT(memcmp(zero_message, plaintext, plaintext_length) == 0);
}
gpr_free(error_message);
gpr_free(corrupt_ciphertext_and_tag);
@ -176,7 +182,9 @@ static void gsec_test_random_encrypt_decrypt(gsec_aead_crypter* crypter,
corrupt_ciphertext_and_tag, ciphertext_bytes_written, plaintext,
plaintext_length, &plaintext_bytes_written, &error_message);
GPR_ASSERT(plaintext_bytes_written == 0);
GPR_ASSERT(memcmp(zero_message, plaintext, plaintext_length) == 0);
if (plaintext_length != 0) {
GPR_ASSERT(memcmp(zero_message, plaintext, plaintext_length) == 0);
}
GPR_ASSERT(gsec_test_expect_compare_code_and_substr(
status, GRPC_STATUS_FAILED_PRECONDITION, error_message,
"Checking tag failed"));
@ -198,7 +206,9 @@ static void gsec_test_random_encrypt_decrypt(gsec_aead_crypter* crypter,
status, GRPC_STATUS_FAILED_PRECONDITION, error_message,
"Checking tag failed"));
GPR_ASSERT(plaintext_bytes_written == 0);
GPR_ASSERT(memcmp(zero_message, plaintext, plaintext_length) == 0);
if (plaintext_length != 0) {
GPR_ASSERT(memcmp(zero_message, plaintext, plaintext_length) == 0);
}
gpr_free(error_message);
gpr_free(corrupt_ciphertext_and_tag);
}
@ -292,7 +302,9 @@ static void gsec_test_multiple_random_encrypt_decrypt(
&(plaintext_bytes_writtens[ind]), nullptr);
GPR_ASSERT(status == GRPC_STATUS_OK);
GPR_ASSERT(message_length == plaintext_bytes_writtens[ind]);
GPR_ASSERT(memcmp(messages[ind], plaintexts[ind], message_length) == 0);
if (message_length != 0) {
GPR_ASSERT(memcmp(messages[ind], plaintexts[ind], message_length) == 0);
}
}
/* Slice the plaintext and encrypt with iovecs */
@ -358,8 +370,10 @@ static void gsec_test_multiple_random_encrypt_decrypt(
decrypted_vec, &decrypted_length, &error_details),
error_details);
GPR_ASSERT(decrypted_vec.iov_len == message_length);
GPR_ASSERT(memcmp(decrypted_vec.iov_base, messages[ind], message_length) ==
0);
if (message_length != 0) {
GPR_ASSERT(
memcmp(decrypted_vec.iov_base, messages[ind], message_length) == 0);
}
free(decrypted);
free(aad_vecs);
free(ciphertext_vecs);
@ -713,8 +727,10 @@ static void gsec_test_encrypt_decrypt_test_vector(
test_vector->ciphertext_and_tag_length, plaintext_bytes, plaintext_length,
&plaintext_bytes_written, nullptr);
GPR_ASSERT(status == GRPC_STATUS_OK);
GPR_ASSERT(memcmp(test_vector->plaintext, plaintext_bytes,
plaintext_bytes_written) == 0);
if (plaintext_bytes_written != 0) {
GPR_ASSERT(memcmp(test_vector->plaintext, plaintext_bytes,
plaintext_bytes_written) == 0);
}
gpr_free(ciphertext_and_tag_bytes);
gpr_free(plaintext_bytes);

@ -49,7 +49,9 @@ uint32_t gsec_test_bias_random_uint32(uint32_t max_length) {
void gsec_test_copy(const uint8_t* src, uint8_t** des, size_t source_len) {
if (src != nullptr && des != nullptr) {
*des = static_cast<uint8_t*>(gpr_malloc(source_len));
memcpy(*des, src, source_len);
if (*des != nullptr) {
memcpy(*des, src, source_len);
}
} else {
fprintf(stderr, "Either src or des buffer is nullptr in gsec_test_copy().");
abort();

@ -191,8 +191,8 @@ class Client {
if (median_latency_collection_interval_seconds_ > 0) {
std::vector<double> medians_per_interval =
threads_[0]->GetMedianPerIntervalList();
gpr_log(GPR_INFO, "Num threads: %ld", threads_.size());
gpr_log(GPR_INFO, "Number of medians: %ld", medians_per_interval.size());
gpr_log(GPR_INFO, "Num threads: %zu", threads_.size());
gpr_log(GPR_INFO, "Number of medians: %zu", medians_per_interval.size());
for (size_t j = 0; j < medians_per_interval.size(); j++) {
gpr_log(GPR_INFO, "%f", medians_per_interval[j]);
}

@ -176,7 +176,7 @@ class LbFeedbackTest : public LoadReporterTest {
ASSERT_THAT(static_cast<double>(lb_feedback.errors_per_second()),
DoubleNear(expected_eps, expected_eps * 0.3));
gpr_log(GPR_INFO,
"Verified LB feedback matches the samples of index [%lu, %lu).",
"Verified LB feedback matches the samples of index [%zu, %zu).",
start, start + count);
}

@ -54,6 +54,7 @@ static bool posix_source_addr_factory_get_source_addr(
address_sorting_source_addr_factory* factory,
const address_sorting_address* dest_addr,
address_sorting_address* source_addr) {
(void)factory;
bool source_addr_exists = false;
// Android sets SOCK_CLOEXEC. Don't set this here for portability.
int s = socket(((struct sockaddr*)dest_addr)->sa_family, SOCK_DGRAM, 0);

Loading…
Cancel
Save