From 705910094d07ddfc5a3b7a4baab58b0a94bcc691 Mon Sep 17 00:00:00 2001 From: George Burgess IV Date: Tue, 27 Jun 2017 16:23:45 -0700 Subject: [PATCH] bionic: fix assorted static analyzer warnings Warnings: bionic/libc/bionic/fts.c:722:5: warning: Null passed to a callee that requires a non-null 1st parameter bionic/libc/bionic/sched_cpualloc.c:34:25: warning: Result of 'malloc' is converted to a pointer of type 'cpu_set_t', which is incompatible with sizeof operand type 'unsigned long' bionic/linker/linker_main.cpp:315:7: warning: Access to field 'e_type' results in a dereference of a null pointer (loaded from variable 'elf_hdr') bionic/linker/linker_main.cpp:493:66: warning: Access to field 'e_phoff' results in a dereference of a null pointer (loaded from variable 'elf_hdr') bionic/linker/linker_main.cpp:90:14: warning: Access to field 'next' results in a dereference of a null pointer (loaded from variable 'prev') Bug: None Test: mma; analyzer warnings are gone. CtsBionicTestCases pass. Change-Id: I699a60c2c6f64c50b9ea06848a680c98a8abb44a --- benchmarks/pthread_benchmark.cpp | 2 +- libc/bionic/fts.c | 4 +++- libc/bionic/sched_cpualloc.c | 5 ++++- linker/linker_main.cpp | 15 +++++++++++++++ 4 files changed, 23 insertions(+), 3 deletions(-) diff --git a/benchmarks/pthread_benchmark.cpp b/benchmarks/pthread_benchmark.cpp index bf4d6cb6e..d3c2de865 100644 --- a/benchmarks/pthread_benchmark.cpp +++ b/benchmarks/pthread_benchmark.cpp @@ -56,7 +56,7 @@ static void DummyPthreadOnceInitFunction() { } static void BM_pthread_once(benchmark::State& state) { - pthread_once_t once = PTHREAD_ONCE_INIT; + static pthread_once_t once = PTHREAD_ONCE_INIT; pthread_once(&once, DummyPthreadOnceInitFunction); while (state.KeepRunning()) { diff --git a/libc/bionic/fts.c b/libc/bionic/fts.c index 31a4b2ff5..a43c8c91d 100644 --- a/libc/bionic/fts.c +++ b/libc/bionic/fts.c @@ -32,6 +32,7 @@ #include #include +#include #include #include #include @@ -719,7 +720,8 @@ mem1: saved_errno = errno; /* Build a file name for fts_stat to stat. */ if (ISSET(FTS_NOCHDIR)) { p->fts_accpath = p->fts_path; - memmove(cp, p->fts_name, p->fts_namelen + 1); + assert(cp && "cp should be non-null if FTS_NOCHDIR is set"); + memmove(cp, p->fts_name, p->fts_namelen + 1); // NOLINT p->fts_info = fts_stat(sp, p, 0, dirfd(dirp)); } else { p->fts_accpath = p->fts_name; diff --git a/libc/bionic/sched_cpualloc.c b/libc/bionic/sched_cpualloc.c index 30964bca7..345de9149 100644 --- a/libc/bionic/sched_cpualloc.c +++ b/libc/bionic/sched_cpualloc.c @@ -31,7 +31,10 @@ cpu_set_t* __sched_cpualloc(size_t count) { - return (cpu_set_t*) malloc(CPU_ALLOC_SIZE(count)); + // The static analyzer complains that CPU_ALLOC_SIZE eventually expands to + // N * sizeof(unsigned long), which is incompatible with cpu_set_t. This is + // on purpose. + return (cpu_set_t*) malloc(CPU_ALLOC_SIZE(count)); // NOLINT } void __sched_cpufree(cpu_set_t* set) diff --git a/linker/linker_main.cpp b/linker/linker_main.cpp index 5dc215f9d..db3697618 100644 --- a/linker/linker_main.cpp +++ b/linker/linker_main.cpp @@ -87,6 +87,7 @@ bool solist_remove_soinfo(soinfo* si) { // prev will never be null, because the first entry in solist is // always the static libdl_info. + CHECK(prev != nullptr); prev->next = si->next; if (si == sonext) { sonext = prev; @@ -307,6 +308,11 @@ static ElfW(Addr) __linker_init_post_relocation(KernelArgumentBlock& args) { break; } } + + if (si->base == 0) { + async_safe_fatal("Could not find a PHDR: broken executable?"); + } + si->dynamic = nullptr; ElfW(Ehdr)* elf_hdr = reinterpret_cast(si->base); @@ -488,6 +494,15 @@ extern "C" ElfW(Addr) __linker_init(void* raw_args) { static uintptr_t linktime_addr = reinterpret_cast(&linktime_addr); ElfW(Addr) linker_addr = reinterpret_cast(&linktime_addr) - linktime_addr; +#if defined(__clang_analyzer__) + // The analyzer assumes that linker_addr will always be null. Make it an + // unknown value so we don't have to mark N places with NOLINTs. + // + // (`+=`, rather than `=`, allows us to sidestep a potential "unused store" + // complaint) + linker_addr += reinterpret_cast(raw_args); +#endif + ElfW(Addr) entry_point = args.getauxval(AT_ENTRY); ElfW(Ehdr)* elf_hdr = reinterpret_cast(linker_addr); ElfW(Phdr)* phdr = reinterpret_cast(linker_addr + elf_hdr->e_phoff);