Merge pull request #10746 from ctiller/ubsan
Ubsan fixes
diff --git a/src/core/ext/transport/chttp2/transport/chttp2_transport.c b/src/core/ext/transport/chttp2/transport/chttp2_transport.c
index 5c5175f..a4cee39 100644
--- a/src/core/ext/transport/chttp2/transport/chttp2_transport.c
+++ b/src/core/ext/transport/chttp2/transport/chttp2_transport.c
@@ -2150,6 +2150,7 @@
(int)bdp);
}
push_setting(exec_ctx, t, GRPC_CHTTP2_SETTINGS_INITIAL_WINDOW_SIZE, bdp);
+ push_setting(exec_ctx, t, GRPC_CHTTP2_SETTINGS_MAX_FRAME_SIZE, bdp);
}
static grpc_error *try_http_parsing(grpc_exec_ctx *exec_ctx,
diff --git a/src/core/lib/iomgr/error.c b/src/core/lib/iomgr/error.c
index fbbca6b..5f2c989 100644
--- a/src/core/lib/iomgr/error.c
+++ b/src/core/lib/iomgr/error.c
@@ -217,8 +217,14 @@
if ((*err)->arena_size + slots > (*err)->arena_capacity) {
return UINT8_MAX;
}
+#ifdef GRPC_ERROR_REFCOUNT_DEBUG
+ grpc_error *orig = *err;
+#endif
*err = gpr_realloc(
*err, sizeof(grpc_error) + (*err)->arena_capacity * sizeof(intptr_t));
+#ifdef GRPC_ERROR_REFCOUNT_DEBUG
+ if (*err != orig) gpr_log(GPR_DEBUG, "realloc %p -> %p", orig, *err);
+#endif
}
uint8_t placement = (*err)->arena_size;
(*err)->arena_size = (uint8_t)((*err)->arena_size + slots);
@@ -313,7 +319,7 @@
// It is very common to include and extra int and string in an error
#define SURPLUS_CAPACITY (2 * SLOTS_PER_INT + SLOTS_PER_TIME)
-grpc_error *grpc_error_create(grpc_slice file, int line, grpc_slice desc,
+grpc_error *grpc_error_create(const char *file, int line, grpc_slice desc,
grpc_error **referencing,
size_t num_referencing) {
GPR_TIMER_BEGIN("grpc_error_create", 0);
@@ -339,7 +345,8 @@
memset(err->times, UINT8_MAX, GRPC_ERROR_TIME_MAX);
internal_set_int(&err, GRPC_ERROR_INT_FILE_LINE, line);
- internal_set_str(&err, GRPC_ERROR_STR_FILE, file);
+ internal_set_str(&err, GRPC_ERROR_STR_FILE,
+ grpc_slice_from_static_string(file));
internal_set_str(&err, GRPC_ERROR_STR_DESCRIPTION, desc);
for (size_t i = 0; i < num_referencing; ++i) {
@@ -756,7 +763,7 @@
return grpc_error_set_str(
grpc_error_set_str(
grpc_error_set_int(
- grpc_error_create(grpc_slice_from_static_string(file), line,
+ grpc_error_create(file, line,
grpc_slice_from_static_string("OS Error"), NULL,
0),
GRPC_ERROR_INT_ERRNO, err),
@@ -772,7 +779,7 @@
grpc_error *error = grpc_error_set_str(
grpc_error_set_str(
grpc_error_set_int(
- grpc_error_create(grpc_slice_from_static_string(file), line,
+ grpc_error_create(file, line,
grpc_slice_from_static_string("OS Error"), NULL,
0),
GRPC_ERROR_INT_WSA_ERROR, err),
diff --git a/src/core/lib/iomgr/error.h b/src/core/lib/iomgr/error.h
index 2a44fcf..34b24d9 100644
--- a/src/core/lib/iomgr/error.h
+++ b/src/core/lib/iomgr/error.h
@@ -138,7 +138,7 @@
const char *grpc_error_string(grpc_error *error);
/// Create an error - but use GRPC_ERROR_CREATE instead
-grpc_error *grpc_error_create(grpc_slice file, int line, grpc_slice desc,
+grpc_error *grpc_error_create(const char *file, int line, grpc_slice desc,
grpc_error **referencing, size_t num_referencing);
/// Create an error (this is the preferred way of generating an error that is
/// not due to a system call - for system calls, use GRPC_OS_ERROR or
@@ -148,21 +148,21 @@
/// err = grpc_error_create(x, y, z, r, nr) is equivalent to:
/// err = grpc_error_create(x, y, z, NULL, 0);
/// for (i=0; i<nr; i++) err = grpc_error_add_child(err, r[i]);
-#define GRPC_ERROR_CREATE_FROM_STATIC_STRING(desc) \
- grpc_error_create(grpc_slice_from_static_string(__FILE__), __LINE__, \
- grpc_slice_from_static_string(desc), NULL, 0)
-#define GRPC_ERROR_CREATE_FROM_COPIED_STRING(desc) \
- grpc_error_create(grpc_slice_from_static_string(__FILE__), __LINE__, \
- grpc_slice_from_copied_string(desc), NULL, 0)
+#define GRPC_ERROR_CREATE_FROM_STATIC_STRING(desc) \
+ grpc_error_create(__FILE__, __LINE__, grpc_slice_from_static_string(desc), \
+ NULL, 0)
+#define GRPC_ERROR_CREATE_FROM_COPIED_STRING(desc) \
+ grpc_error_create(__FILE__, __LINE__, grpc_slice_from_copied_string(desc), \
+ NULL, 0)
// Create an error that references some other errors. This function adds a
// reference to each error in errs - it does not consume an existing reference
-#define GRPC_ERROR_CREATE_REFERENCING_FROM_STATIC_STRING(desc, errs, count) \
- grpc_error_create(grpc_slice_from_static_string(__FILE__), __LINE__, \
- grpc_slice_from_static_string(desc), errs, count)
-#define GRPC_ERROR_CREATE_REFERENCING_FROM_COPIED_STRING(desc, errs, count) \
- grpc_error_create(grpc_slice_from_static_string(__FILE__), __LINE__, \
- grpc_slice_from_copied_string(desc), errs, count)
+#define GRPC_ERROR_CREATE_REFERENCING_FROM_STATIC_STRING(desc, errs, count) \
+ grpc_error_create(__FILE__, __LINE__, grpc_slice_from_static_string(desc), \
+ errs, count)
+#define GRPC_ERROR_CREATE_REFERENCING_FROM_COPIED_STRING(desc, errs, count) \
+ grpc_error_create(__FILE__, __LINE__, grpc_slice_from_copied_string(desc), \
+ errs, count)
//#define GRPC_ERROR_REFCOUNT_DEBUG
#ifdef GRPC_ERROR_REFCOUNT_DEBUG
diff --git a/src/core/lib/support/stack_lockfree.c b/src/core/lib/support/stack_lockfree.c
index 9d7c9e5..c481a3e 100644
--- a/src/core/lib/support/stack_lockfree.c
+++ b/src/core/lib/support/stack_lockfree.c
@@ -72,11 +72,6 @@
struct gpr_stack_lockfree {
lockfree_node *entries;
lockfree_node head; /* An atomic entry describing curr head */
-
-#ifndef NDEBUG
- /* Bitmap of pushed entries to check for double-push or pop */
- gpr_atm pushed[(INVALID_ENTRY_INDEX + 1) / (8 * sizeof(gpr_atm))];
-#endif
};
gpr_stack_lockfree *gpr_stack_lockfree_create(size_t entries) {
@@ -91,9 +86,6 @@
/* Clear out all entries */
memset(stack->entries, 0, entries * sizeof(stack->entries[0]));
memset(&stack->head, 0, sizeof(stack->head));
-#ifndef NDEBUG
- memset(&stack->pushed, 0, sizeof(stack->pushed));
-#endif
GPR_ASSERT(sizeof(stack->entries->atm) == sizeof(stack->entries->contents));
@@ -130,19 +122,6 @@
newhead.contents.aba_ctr = ++curent.contents.aba_ctr;
gpr_atm_no_barrier_store(&stack->entries[entry].atm, curent.atm);
-#ifndef NDEBUG
- /* Check for double push */
- {
- int pushed_index = entry / (int)(8 * sizeof(gpr_atm));
- int pushed_bit = entry % (int)(8 * sizeof(gpr_atm));
- gpr_atm old_val;
-
- old_val = gpr_atm_no_barrier_fetch_add(&stack->pushed[pushed_index],
- ((gpr_atm)1 << pushed_bit));
- GPR_ASSERT((old_val & (((gpr_atm)1) << pushed_bit)) == 0);
- }
-#endif
-
do {
/* Atomically get the existing head value for use */
head.atm = gpr_atm_no_barrier_load(&(stack->head.atm));
@@ -168,18 +147,6 @@
gpr_atm_no_barrier_load(&(stack->entries[head.contents.index].atm));
} while (!gpr_atm_no_barrier_cas(&(stack->head.atm), head.atm, newhead.atm));
-#ifndef NDEBUG
- /* Check for valid pop */
- {
- int pushed_index = head.contents.index / (8 * sizeof(gpr_atm));
- int pushed_bit = head.contents.index % (8 * sizeof(gpr_atm));
- gpr_atm old_val;
-
- old_val = gpr_atm_no_barrier_fetch_add(&stack->pushed[pushed_index],
- -((gpr_atm)1 << pushed_bit));
- GPR_ASSERT((old_val & (((gpr_atm)1) << pushed_bit)) != 0);
- }
-#endif
return head.contents.index;
}
diff --git a/src/core/lib/surface/call.c b/src/core/lib/surface/call.c
index 3e96d09..a42fe7e 100644
--- a/src/core/lib/surface/call.c
+++ b/src/core/lib/surface/call.c
@@ -346,6 +346,8 @@
gpr_timespec send_deadline =
gpr_convert_clock_type(args->send_deadline, GPR_CLOCK_MONOTONIC);
+ bool immediately_cancel = false;
+
if (args->parent_call != NULL) {
child_call *cc = call->child_call =
gpr_arena_alloc(arena, sizeof(child_call));
@@ -386,8 +388,7 @@
if (args->propagation_mask & GRPC_PROPAGATE_CANCELLATION) {
call->cancellation_is_inherited = 1;
if (gpr_atm_acq_load(&args->parent_call->received_final_op_atm)) {
- cancel_with_error(exec_ctx, call, STATUS_FROM_API_OVERRIDE,
- GRPC_ERROR_CANCELLED);
+ immediately_cancel = true;
}
}
@@ -422,6 +423,10 @@
cancel_with_error(exec_ctx, call, STATUS_FROM_SURFACE,
GRPC_ERROR_REF(error));
}
+ if (immediately_cancel) {
+ cancel_with_error(exec_ctx, call, STATUS_FROM_API_OVERRIDE,
+ GRPC_ERROR_CANCELLED);
+ }
if (args->cq != NULL) {
GPR_ASSERT(
args->pollset_set_alternative == NULL &&
diff --git a/tools/jenkins/run_c_cpp_test.sh b/tools/jenkins/run_c_cpp_test.sh
new file mode 100755
index 0000000..a7e5745
--- /dev/null
+++ b/tools/jenkins/run_c_cpp_test.sh
@@ -0,0 +1,47 @@
+#!/usr/bin/env bash
+# Copyright 2017, Google Inc.
+# All rights reserved.
+#
+# Redistribution and use in source and binary forms, with or without
+# modification, are permitted provided that the following conditions are
+# met:
+#
+# * Redistributions of source code must retain the above copyright
+# notice, this list of conditions and the following disclaimer.
+# * Redistributions in binary form must reproduce the above
+# copyright notice, this list of conditions and the following disclaimer
+# in the documentation and/or other materials provided with the
+# distribution.
+# * Neither the name of Google Inc. nor the names of its
+# contributors may be used to endorse or promote products derived from
+# this software without specific prior written permission.
+#
+# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+# A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+# OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+#
+# This script is invoked by a Jenkins pull request job and executes all
+# args passed to this script if the pull request affect C/C++ code
+set -ex
+
+# Enter the gRPC repo root
+cd $(dirname $0)/../..
+
+AFFECTS_C_CPP=`python -c 'import sys; \
+ sys.path.insert(0, "tools/run_tests/python_utils"); \
+ import filter_pull_request_tests as filter; \
+ print(filter.affects_c_cpp("origin/$ghprbTargetBranch"))'`
+
+if [ $AFFECTS_C_CPP == "False" ] ; then
+ echo "This pull request does not affect C/C++. Tests do not need to be run."
+else
+ $@
+fi
diff --git a/tools/run_tests/python_utils/filter_pull_request_tests.py b/tools/run_tests/python_utils/filter_pull_request_tests.py
index e013376..958eb56 100644
--- a/tools/run_tests/python_utils/filter_pull_request_tests.py
+++ b/tools/run_tests/python_utils/filter_pull_request_tests.py
@@ -127,6 +127,9 @@
'setup\.py$': [_PYTHON_TEST_SUITE]
}
+# Regex that combines all keys in _WHITELIST_DICT
+_ALL_TRIGGERS = "(" + ")|(".join(_WHITELIST_DICT.keys()) + ")"
+
# Add all triggers to their respective test suites
for trigger, test_suites in six.iteritems(_WHITELIST_DICT):
for test_suite in test_suites:
@@ -169,6 +172,21 @@
test.labels[2] not in skippable_labels]
+def affects_c_cpp(base_branch):
+ """
+ Determines if a pull request's changes affect C/C++. This function exists because
+ there are pull request tests that only test C/C++ code
+ :param base_branch: branch that a pull request is requesting to merge into
+ :return: boolean indicating whether C/C++ changes are made in pull request
+ """
+ changed_files = _get_changed_files(base_branch)
+ # Run all tests if any changed file is not in the whitelist dictionary
+ for changed_file in changed_files:
+ if not re.match(_ALL_TRIGGERS, changed_file):
+ return True
+ return not _can_skip_tests(changed_files, _CPP_TEST_SUITE.triggers + _CORE_TEST_SUITE.triggers)
+
+
def filter_tests(tests, base_branch):
"""
Filters out tests that are safe to ignore
@@ -181,11 +199,9 @@
print(' %s' % changed_file)
print('')
- # Regex that combines all keys in _WHITELIST_DICT
- all_triggers = "(" + ")|(".join(_WHITELIST_DICT.keys()) + ")"
- # Check if all tests have to be run
+ # Run all tests if any changed file is not in the whitelist dictionary
for changed_file in changed_files:
- if not re.match(all_triggers, changed_file):
+ if not re.match(_ALL_TRIGGERS, changed_file):
return(tests)
# Figure out which language and platform tests to run
skippable_labels = []
@@ -196,4 +212,3 @@
skippable_labels.append(label)
tests = _remove_irrelevant_tests(tests, skippable_labels)
return tests
-