diff --git a/CMakeLists.txt b/CMakeLists.txt index f1b0da658..ce59b1b85 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -218,6 +218,16 @@ if(MSVC) set(CMAKE_ASM_MASM_FLAGS "${CMAKE_ASM_MASM_FLAGS} /safeseh") endif() + # using `/Wall` is not feasible, as it spews tons of warnings from windows headers + target_compile_options(sentry PRIVATE $) + # ignore all warnings for mpack + set_source_files_properties( + "${PROJECT_SOURCE_DIR}/vendor/mpack.c" + PROPERTIES + COMPILE_FLAGS + "/W0" + ) + # set static runtime if enabled if(SENTRY_BUILD_RUNTIMESTATIC) set_property(TARGET sentry PROPERTY MSVC_RUNTIME_LIBRARY "MultiThreaded$<$:Debug>") diff --git a/azure-pipelines.yml b/azure-pipelines.yml index 3acad0d9b..5210d0ece 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -29,8 +29,12 @@ stages: vmImage: "ubuntu-latest" CC: clang-9 CXX: clang++-9 + ERROR_ON_WARNINGS: 1 + # TODO: install `clang-tools` + #SCAN_BUILD: 1 macOS: vmImage: "macOs-latest" + ERROR_ON_WARNINGS: 1 Windows (VS2017, 32bit): vmImage: "vs2017-win2016" TEST_X86: 1 @@ -51,7 +55,7 @@ stages: Android (API 29 NDK 21): vmImage: "macOs-latest" ANDROID_API: 29 - ANDROID_NDK: 21.0.6113669 + ANDROID_NDK: 21.2.6472646 pool: vmImage: $(vmImage) steps: diff --git a/examples/example.c b/examples/example.c index 18a493365..de2a17c61 100644 --- a/examples/example.c +++ b/examples/example.c @@ -33,6 +33,16 @@ has_arg(int argc, char **argv, const char *arg) return false; } +static void *invalid_mem = (void *)1; + +static void +trigger_crash() +{ + // Triggers a segfault by writing to `NULL`. We actually do a `1 - 1` to + // defeat static analyzers which would warn for the trivial case. + memset((char *)invalid_mem - 1, 1, 100); +} + int main(int argc, char **argv) { @@ -131,7 +141,7 @@ main(int argc, char **argv) } if (has_arg(argc, argv, "crash")) { - memset((char *)0x0, 1, 100); + trigger_crash(); } if (has_arg(argc, argv, "capture-event")) { diff --git a/src/backends/sentry_backend_crashpad.cpp b/src/backends/sentry_backend_crashpad.cpp index d3ebe1ebf..5d45356e2 100644 --- a/src/backends/sentry_backend_crashpad.cpp +++ b/src/backends/sentry_backend_crashpad.cpp @@ -13,12 +13,15 @@ extern "C" { #include #include -#ifdef __GNUC__ +#if defined(__GNUC__) # pragma GCC diagnostic push # pragma GCC diagnostic ignored "-Wunused-parameter" # pragma GCC diagnostic ignored "-Wgnu-zero-variadic-macro-arguments" # pragma GCC diagnostic ignored "-Wfour-char-constants" # pragma GCC diagnostic ignored "-Wgnu-include-next" +#elif defined(_MSC_VER) +# pragma warning(push) +# pragma warning(disable : 4100) // unreferenced formal parameter #endif #include "client/crash_report_database.h" @@ -26,8 +29,10 @@ extern "C" { #include "client/crashpad_info.h" #include "client/settings.h" -#ifdef __GNUC__ +#if defined(__GNUC__) # pragma GCC diagnostic pop +#elif defined(_MSC_VER) +# pragma warning(pop) #endif extern "C" { diff --git a/src/modulefinder/sentry_modulefinder_linux.c b/src/modulefinder/sentry_modulefinder_linux.c index fd6470267..dddff53b4 100644 --- a/src/modulefinder/sentry_modulefinder_linux.c +++ b/src/modulefinder/sentry_modulefinder_linux.c @@ -310,8 +310,8 @@ sentry__procmaps_module_to_value(const sentry_module_t *module) { sentry_value_t mod_val = sentry_value_new_object(); sentry_value_set_by_key(mod_val, "type", sentry_value_new_string("elf")); - sentry_value_set_by_key( - mod_val, "image_addr", sentry__value_new_addr((uint64_t)module->start)); + sentry_value_set_by_key(mod_val, "image_addr", + sentry__value_new_addr((uint64_t)(size_t)module->start)); sentry_value_set_by_key(mod_val, "image_size", sentry_value_new_int32( (int32_t)((size_t)module->end - (size_t)module->start))); diff --git a/src/path/sentry_path_windows.c b/src/path/sentry_path_windows.c index ab8b9afdb..8ec8cc9c4 100644 --- a/src/path/sentry_path_windows.c +++ b/src/path/sentry_path_windows.c @@ -340,7 +340,6 @@ sentry__path_remove(const sentry_path_t *path) } return 1; } - return 1; } int diff --git a/src/sentry_core.h b/src/sentry_core.h index baffb9cfc..df7c70cd5 100644 --- a/src/sentry_core.h +++ b/src/sentry_core.h @@ -14,8 +14,10 @@ # define MUST_USE #endif -#ifdef __GNUC__ +#if defined(__GNUC__) # define UNUSED(x) UNUSED_##x __attribute__((__unused__)) +#elif defined(_MSC_VER) +# define UNUSED(x) UNUSED_##x __pragma(warning(suppress : 4100)) #else # define UNUSED(x) UNUSED_##x #endif diff --git a/src/sentry_database.c b/src/sentry_database.c index ba8edea54..48acd066c 100644 --- a/src/sentry_database.c +++ b/src/sentry_database.c @@ -156,8 +156,11 @@ sentry__process_old_runs(const sentry_options_t *options) } sentry_path_t *lockfile = sentry__path_append_str(run_dir, ".lock"); - sentry_filelock_t *lock; - if (!lockfile || !(lock = sentry__filelock_new(lockfile))) { + if (!lockfile) { + continue; + } + sentry_filelock_t *lock = sentry__filelock_new(lockfile); + if (!lock) { continue; } bool did_lock = sentry__filelock_try_lock(lock); diff --git a/src/sentry_json.c b/src/sentry_json.c index 11211b1d0..7b5ecb07e 100644 --- a/src/sentry_json.c +++ b/src/sentry_json.c @@ -360,7 +360,7 @@ decode_string_inplace(char *buf) input += 4; if (sentry__is_lead_surrogate(uchar)) { - uint16_t lead = uchar; + uint16_t lead = (uint16_t)uchar; if (input[0] != '\\' || input[1] != 'u') { return false; } diff --git a/src/sentry_logger.c b/src/sentry_logger.c index c9a52335c..4fcfc1212 100644 --- a/src/sentry_logger.c +++ b/src/sentry_logger.c @@ -1,9 +1,9 @@ -#include -#include - #include "sentry_logger.h" #include "sentry_options.h" +#include +#include + #if defined(SENTRY_PLATFORM_ANDROID) # include @@ -43,11 +43,19 @@ sentry__logger_defaultlogger( const char *prefix = "[sentry] "; const char *priority = sentry__logger_describe(level); - char *format = sentry_malloc( - strlen(prefix) + strlen(priority) + strlen(message) + 1); - strcpy(format, prefix); - strcat(format, priority); - strcat(format, message); + size_t len = strlen(prefix) + strlen(priority) + strlen(message) + 1; + char *format = sentry_malloc(len); + + // Some compilers/tools, such as MSVC warn for using `strcpy` and friends. + // However, there are not really any good alternatives: + // * `strcpy_s` is only available on Windows + // * `strlcpy` is apparently a BSD thing + // * `strscpy` is a linux kernel thing + // * `strncpy` is not really a good choice either, but tools do not mark it + // as "unsafe". + strncpy(format, prefix, strlen(prefix)); + strncat(format, priority, strlen(priority)); + strncat(format, message, strlen(message)); vfprintf(stderr, format, args); diff --git a/src/sentry_logger.h b/src/sentry_logger.h index d3a48dd6a..e714e5d08 100644 --- a/src/sentry_logger.h +++ b/src/sentry_logger.h @@ -1,4 +1,4 @@ -#include "sentry.h" +#include "sentry_boot.h" void sentry__logger_defaultlogger( sentry_level_t level, const char *message, va_list args); diff --git a/src/sentry_scope.c b/src/sentry_scope.c index b541d7f4b..9613a9837 100644 --- a/src/sentry_scope.c +++ b/src/sentry_scope.c @@ -175,13 +175,13 @@ sentry__symbolize_frame(const sentry_frame_info_t *info, void *data) && sentry_value_is_null( sentry_value_get_by_key(frame, "symbol_addr"))) { sentry_value_set_by_key(frame, "symbol_addr", - sentry__value_new_addr((uint64_t)info->symbol_addr)); + sentry__value_new_addr((uint64_t)(size_t)info->symbol_addr)); } if (info->load_addr && sentry_value_is_null(sentry_value_get_by_key(frame, "image_addr"))) { sentry_value_set_by_key(frame, "image_addr", - sentry__value_new_addr((uint64_t)info->load_addr)); + sentry__value_new_addr((uint64_t)(size_t)info->load_addr)); } } @@ -204,8 +204,8 @@ sentry__symbolize_stacktrace(sentry_value_t stacktrace) } // The addr is saved as a hex-number inside the value. - uint64_t addr - = (uint64_t)strtoll(sentry_value_as_string(addr_value), NULL, 0); + size_t addr + = (size_t)strtoll(sentry_value_as_string(addr_value), NULL, 0); if (!addr) { continue; } diff --git a/src/sentry_string.c b/src/sentry_string.c index 7a383f6bc..d9809b594 100644 --- a/src/sentry_string.c +++ b/src/sentry_string.c @@ -152,8 +152,8 @@ sentry__string_to_wstr(const char *s) size_t sentry__unichar_to_utf8(uint32_t c, char *buf) { - size_t i, len; - int first; + size_t i, len = 1; + uint32_t first; if (c < 0x80) { first = 0; @@ -172,9 +172,9 @@ sentry__unichar_to_utf8(uint32_t c, char *buf) } for (i = len - 1; i > 0; --i) { - buf[i] = (c & 0x3f) | 0x80; + buf[i] = (char)(c & 0x3f) | 0x80; c >>= 6; } - buf[0] = c | first; + buf[0] = (char)(c | first); return len; } diff --git a/src/sentry_sync.c b/src/sentry_sync.c index 7569d5623..fdd5883d5 100644 --- a/src/sentry_sync.c +++ b/src/sentry_sync.c @@ -64,7 +64,12 @@ shutdown_task(void *data) # define UNSIGNED_MINGW #endif -static UNSIGNED_MINGW int THREAD_FUNCTION_API +// pthreads use `void *` return types, whereas windows uses `DWORD` +#ifdef SENTRY_PLATFORM_WINDOWS +static UNSIGNED_MINGW DWORD THREAD_FUNCTION_API +#else +static void * +#endif worker_thread(void *data) { sentry_bgworker_t *bgw = data; diff --git a/src/sentry_sync.h b/src/sentry_sync.h index 871a3a292..085b38a37 100644 --- a/src/sentry_sync.h +++ b/src/sentry_sync.h @@ -2,6 +2,7 @@ #define SENTRY_SYNC_H_INCLUDED #include "sentry_boot.h" +#include "sentry_core.h" #include #include @@ -121,7 +122,8 @@ struct sentry__winmutex_s { }; static inline BOOL CALLBACK -sentry__winmutex_init(PINIT_ONCE InitOnce, PVOID cs, PVOID *lpContext) +sentry__winmutex_init( + PINIT_ONCE UNUSED(InitOnce), PVOID cs, PVOID *UNUSED(lpContext)) { InitializeCriticalSection(cs); return TRUE; @@ -226,9 +228,7 @@ typedef pthread_cond_t sentry_cond_t; } while (0) # define sentry__cond_wake pthread_cond_signal # define sentry__thread_spawn(ThreadId, Func, Data) \ - (pthread_create(ThreadId, NULL, (void *(*)(void *))Func, Data) == 0 \ - ? 0 \ - : 1) + (pthread_create(ThreadId, NULL, Func, Data) == 0 ? 0 : 1) # define sentry__thread_join(ThreadId) pthread_join(ThreadId, NULL) # define sentry__threadid_equal pthread_equal # define sentry__current_thread pthread_self @@ -254,8 +254,8 @@ sentry__cond_wait_timeout( *(Mutex) = tmp; \ } while (0) -static inline int -sentry__atomic_fetch_and_add(volatile int *val, int diff) +static inline long +sentry__atomic_fetch_and_add(volatile long *val, long diff) { #ifdef SENTRY_PLATFORM_WINDOWS return InterlockedExchangeAdd(val, diff); @@ -264,8 +264,8 @@ sentry__atomic_fetch_and_add(volatile int *val, int diff) #endif } -static inline int -sentry__atomic_fetch(volatile int *val) +static inline long +sentry__atomic_fetch(volatile long *val) { return sentry__atomic_fetch_and_add(val, 0); } diff --git a/src/sentry_unix_pageallocator.c b/src/sentry_unix_pageallocator.c index 9dbac6d96..264475de4 100644 --- a/src/sentry_unix_pageallocator.c +++ b/src/sentry_unix_pageallocator.c @@ -10,6 +10,8 @@ # define MAP_ANONYMOUS MAP_ANON #endif +#define ALIGN 8 + struct page_header; struct page_header { struct page_header *next; @@ -81,10 +83,15 @@ sentry__page_allocator_alloc(size_t size) return NULL; } + // make sure the requested size is correctly aligned + size_t diff = size % ALIGN; + size = size + ALIGN - diff; + char *rv = NULL; sentry__spinlock_lock(&g_lock); + // current page is large enough if (g_alloc->current_page && g_alloc->page_size - g_alloc->page_offset >= size) { rv = g_alloc->current_page + g_alloc->page_offset; @@ -93,21 +100,25 @@ sentry__page_allocator_alloc(size_t size) g_alloc->page_offset = 0; g_alloc->current_page = NULL; } - } - - size_t pages = (size + sizeof(struct page_header) + g_alloc->page_size - 1) - / g_alloc->page_size; - rv = get_pages(pages); - - if (rv) { - g_alloc->page_offset = (g_alloc->page_size - - (g_alloc->page_size * pages - - (size + sizeof(struct page_header)))) - % g_alloc->page_size; - g_alloc->current_page = g_alloc->page_offset - ? rv + g_alloc->page_size * (pages - 1) - : NULL; - rv += sizeof(struct page_header); + } else { + // allocate new pages + + size_t requested_size = size + sizeof(struct page_header); + size_t pages + = (requested_size + g_alloc->page_size - 1) / g_alloc->page_size; + size_t actual_size = g_alloc->page_size * pages; + + rv = get_pages(pages); + + if (rv) { + size_t diff = actual_size - requested_size; + g_alloc->page_offset + = (g_alloc->page_size - diff) % g_alloc->page_size; + g_alloc->current_page = g_alloc->page_offset + ? rv + g_alloc->page_size * (pages - 1) + : NULL; + rv += sizeof(struct page_header); + } } sentry__spinlock_unlock(&g_lock); diff --git a/src/sentry_utils.c b/src/sentry_utils.c index e0ad1ba4d..4c0182406 100644 --- a/src/sentry_utils.c +++ b/src/sentry_utils.c @@ -161,7 +161,6 @@ sentry__url_parse(sentry_url_t *url_out, const char *url) tmp = ptr; SKIP_WHILE_NOT(tmp, 0); url_out->fragment = sentry__string_clonen(ptr, tmp - ptr); - ptr = tmp; } if (url_out->port == 0) { diff --git a/src/sentry_uuid.c b/src/sentry_uuid.c index 325af3999..1ebcd929b 100644 --- a/src/sentry_uuid.c +++ b/src/sentry_uuid.c @@ -33,7 +33,7 @@ sentry_uuid_from_string(const char *str) size_t len = strlen(str); size_t pos = 0; bool is_nibble = true; - char nibble; + char nibble = 0; for (i = 0; i < len && pos < 16; i++) { char c = str[i]; diff --git a/src/sentry_value.c b/src/sentry_value.c index 78348467f..0bdc41e8b 100644 --- a/src/sentry_value.c +++ b/src/sentry_value.c @@ -6,13 +6,15 @@ #include #include -#ifdef __GNUC__ -# pragma GCC diagnostic push -# pragma GCC diagnostic ignored "-Wstatic-in-inline" +#if defined(_MSC_VER) +# pragma warning(push) +# pragma warning(disable : 4127) // conditional expression is constant #endif + #include "../vendor/mpack.h" -#ifdef __GNUC__ -# pragma GCC diagnostic pop + +#if defined(_MSC_VER) +# pragma warning(pop) #endif #include "sentry_alloc.h" @@ -38,7 +40,7 @@ typedef struct { void *payload; - int refcount; + long refcount; char type; } thing_t; @@ -185,8 +187,8 @@ new_thing_value(void *ptr, int thing_type) thing->payload = ptr; thing->refcount = 1; - thing->type = thing_type; - rv._bits = (((uint64_t)thing) >> 2) | TAG_THING; + thing->type = (char)thing_type; + rv._bits = (((uint64_t)(size_t)thing) >> 2) | TAG_THING; return rv; } @@ -196,7 +198,7 @@ value_as_thing(sentry_value_t value) if (value._bits <= MAX_DOUBLE) { return NULL; } else if ((value._bits & TAG_THING) == TAG_THING) { - return (thing_t *)((value._bits << 2) & ~TAG_THING); + return (thing_t *)(size_t)((value._bits << 2) & ~TAG_THING); } else { return NULL; } @@ -960,7 +962,7 @@ sentry_event_value_add_stacktrace(sentry_value_t event, void **ips, size_t len) for (size_t i = 0; i < len; i++) { sentry_value_t frame = sentry_value_new_object(); sentry_value_set_by_key(frame, "instruction_addr", - sentry__value_new_addr((uint64_t)ips[len - i - 1])); + sentry__value_new_addr((uint64_t)(size_t)ips[len - i - 1])); sentry_value_append(frames, frame); } diff --git a/src/symbolizer/sentry_symbolizer_windows.c b/src/symbolizer/sentry_symbolizer_windows.c index 382669cb2..4a22242ed 100644 --- a/src/symbolizer/sentry_symbolizer_windows.c +++ b/src/symbolizer/sentry_symbolizer_windows.c @@ -24,13 +24,14 @@ sentry__symbolize( } char mod_name[MAX_PATH]; - GetModuleFileNameA((HMODULE)sym->ModBase, mod_name, sizeof(mod_name)); + GetModuleFileNameA( + (HMODULE)(size_t)sym->ModBase, mod_name, sizeof(mod_name)); sentry_frame_info_t frame_info; memset(&frame_info, 0, sizeof(sentry_frame_info_t)); - frame_info.load_addr = (void *)sym->ModBase; + frame_info.load_addr = (void *)(size_t)sym->ModBase; frame_info.instruction_addr = addr; - frame_info.symbol_addr = (void *)sym->Address; + frame_info.symbol_addr = (void *)(size_t)sym->Address; frame_info.symbol = sym->Name; frame_info.object_name = mod_name; func(&frame_info, data); diff --git a/src/transports/sentry_transport_winhttp.c b/src/transports/sentry_transport_winhttp.c index 0a8e9e10c..116a8210e 100644 --- a/src/transports/sentry_transport_winhttp.c +++ b/src/transports/sentry_transport_winhttp.c @@ -159,14 +159,14 @@ task_exec_func(void *data) sentry__stringbuilder_append(&sb, "\r\n"); } - char *buf = sentry__stringbuilder_into_string(&sb); - wchar_t *headers = sentry__string_to_wstr(buf); - sentry_free(buf); + char *headers_buf = sentry__stringbuilder_into_string(&sb); + wchar_t *headers = sentry__string_to_wstr(headers_buf); + sentry_free(headers_buf); SENTRY_TRACEF( "sending request using winhttp to \"%s\":\n%S", req->url, headers); - if (WinHttpSendRequest(request, headers, -1, (LPVOID)req->body, + if (WinHttpSendRequest(request, headers, (DWORD)-1, (LPVOID)req->body, (DWORD)req->body_len, (DWORD)req->body_len, 0)) { WinHttpReceiveResponse(request, NULL); diff --git a/src/unwinder/sentry_unwinder_dbghelp.c b/src/unwinder/sentry_unwinder_dbghelp.c index d57c93228..c6d6dc508 100644 --- a/src/unwinder/sentry_unwinder_dbghelp.c +++ b/src/unwinder/sentry_unwinder_dbghelp.c @@ -66,7 +66,7 @@ sentry__unwind_stack_dbghelp( &stack_frame, &ctx, NULL, SymFunctionTableAccess64, SymGetModuleBase64, NULL) && size < max_frames) { - ptrs[size++] = (void *)stack_frame.AddrPC.Offset; + ptrs[size++] = (void *)(size_t)stack_frame.AddrPC.Offset; } return size; diff --git a/tests/__init__.py b/tests/__init__.py index bef1063fa..8d2500e61 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -94,7 +94,10 @@ def cmake(cwd, targets, options=None): "ANDROID_NATIVE_API_LEVEL": os.environ["ANDROID_API"], } ) - configcmd = ["cmake"] + cmake = ["cmake"] + if os.environ.get("SCAN_BUILD"): + cmake = ["scan-build", "cmake"] + configcmd = [*cmake] for key, value in options.items(): configcmd.append("-D{}={}".format(key, value)) if sys.platform == "win32" and os.environ.get("TEST_X86"): @@ -103,10 +106,20 @@ def cmake(cwd, targets, options=None): cmakelists_dir = os.path.dirname(os.path.dirname(os.path.realpath(__file__))) configcmd.append(cmakelists_dir) + # we have to set `-Werror` for this cmake invocation only, otherwise + # completely unrelated things will break + env = dict(os.environ) + if os.environ.get("ERROR_ON_WARNINGS"): + env["CFLAGS"] = env["CXXFLAGS"] = "-Werror" + if sys.platform == "win32": + # MP = object level parallelism, WX = warnings as errors + cpus = os.cpu_count() + env["CFLAGS"] = env["CXXFLAGS"] = "/WX /MP{}".format(cpus) + print("\n{} > {}".format(cwd, " ".join(configcmd)), flush=True) - subprocess.run(configcmd, cwd=cwd, check=True) + subprocess.run(configcmd, cwd=cwd, env=env, check=True) - buildcmd = ["cmake", "--build", ".", "--parallel"] + buildcmd = [*cmake, "--build", ".", "--parallel"] for target in targets: buildcmd.extend(["--target", target]) print("{} > {}".format(cwd, " ".join(buildcmd)), flush=True)