This is an archive of the discontinued LLVM Phabricator instance.

Differential D38515

Fix ISO C++ fnptr -> objptr errors.
AbandonedPublic

Authored by hctim on Oct 3 2017, 2:14 PM.

Details

Reviewers
pcc
Summary

Fixes -Wpedantic errors associated with casting between pointer-to-function and pointer-to-object present in some sanitizer DLL lookups.

windows.h GetProcAddress returns pointer-to-function through type FARPROC (https://msdn.microsoft.com/en-us/library/windows/desktop/ms683212(v=vs.85).aspx). Casting this to a void* is invalid in ISO C++.

Changed return value of LookupSymbolFromMain to a basic function pointer that returns void, that can then be reinterpreted during the call. The problem is that dlsym [3] returns a void*. This is a known problem with this function, as any use of this function must do a pointer-to-object -> pointer-to-function cast before using it.

Possible solutions to this problem are casting via an integral type (the chosen method) or a union cast. As a union cast is undefined behaviour, the best method is to cast through the integral type, uint64_t in this case.

See https://web.archive.org/web/20061214100447/http://www.trilithium.com/johan/2004/12/problem-with-dlsym/ for more information (archive.org linked as the original article is no longer available).

Diff Detail

Event Timeline

hctim created this revision.Oct 3 2017, 2:14 PM
Herald added a subscriber: kubamracek. · View Herald TranscriptOct 3 2017, 2:14 PM
pcc edited edge metadata.Oct 3 2017, 2:34 PM
pcc added a subscriber: llvm-commits.

What's the motivation for this patch? Are you trying to make LLVM compile with -Wpedantic? Is this the only issue, or are there others?

hctim added a comment.Oct 3 2017, 3:51 PM
In D38515#887573, @pcc wrote:

What's the motivation for this patch? Are you trying to make LLVM compile with -Wpedantic? Is this the only issue, or are there others?

Currently, if you build compiler-rt as part of llvm, -DLLVM_ENABLE_PEDANTIC is default-set to true. This results in warnings being shown on the default compile.

There are currently 17 warnings produced by compiling llvm with default parameters. compiler-rt is responsible for all 17. Most of them are from -Wall or -Wextra and I'm working through them sequentially. This patch fixes 6/17 warnings, making -Wpedantic no longer cause errors.

hctim added a comment.Oct 3 2017, 3:59 PM

FYI: current default LLVM build errors list:

[565/4353] Building CXX object projects/compiler-rt/...les/clang_rt.stats_client-i386.dir/stats_client.cc.o
<REDACTED>/llvm/projects/compiler-rt/lib/stats/stats_client.cc: In constructor ‘{anonymous}::RegisterSanStats::RegisterSanStats()’:
<REDACTED>/llvm/projects/compiler-rt/lib/stats/stats_client.cc:50:59: warning: ISO C++ forbids casting between pointer-to-function and pointer-to-object [-Wpedantic]
         LookupSymbolFromMain("__sanitizer_stats_register"));
                                                           ^
<REDACTED>/llvm/projects/compiler-rt/lib/stats/stats_client.cc: In destructor ‘{anonymous}::RegisterSanStats::~RegisterSanStats()’:
<REDACTED>/llvm/projects/compiler-rt/lib/stats/stats_client.cc:58:61: warning: ISO C++ forbids casting between pointer-to-function and pointer-to-object [-Wpedantic]
         LookupSymbolFromMain("__sanitizer_stats_unregister"));
                                                             ^
[619/4353] Building CXX object projects/compiler-rt/...ibc.i386.dir/sanitizer_symbolizer_posix_libcdep.cc.o
<REDACTED>/llvm/projects/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_posix_libcdep.cc: In function ‘void __sanitizer::InitializeSwiftDemangler()’:
<REDACTED>/llvm/projects/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_posix_libcdep.cc:79:77: warning: ISO C++ forbids casting between pointer-to-function and pointer-to-object [-Wpedantic]
   swift_demangle_f = (swift_demangle_ft)dlsym(RTLD_DEFAULT, "swift_demangle");
                                                                             ^
[641/4353] Building CXX object projects/compiler-rt/...s/clang_rt.stats_client-x86_64.dir/stats_client.cc.o
<REDACTED>/llvm/projects/compiler-rt/lib/stats/stats_client.cc: In constructor ‘{anonymous}::RegisterSanStats::RegisterSanStats()’:
<REDACTED>/llvm/projects/compiler-rt/lib/stats/stats_client.cc:50:59: warning: ISO C++ forbids casting between pointer-to-function and pointer-to-object [-Wpedantic]
         LookupSymbolFromMain("__sanitizer_stats_register"));
                                                           ^
<REDACTED>/llvm/projects/compiler-rt/lib/stats/stats_client.cc: In destructor ‘{anonymous}::RegisterSanStats::~RegisterSanStats()’:
<REDACTED>/llvm/projects/compiler-rt/lib/stats/stats_client.cc:58:61: warning: ISO C++ forbids casting between pointer-to-function and pointer-to-object [-Wpedantic]
         LookupSymbolFromMain("__sanitizer_stats_unregister"));
                                                             ^
[672/4353] Building CXX object projects/compiler-rt/...c.x86_64.dir/sanitizer_symbolizer_posix_libcdep.cc.o
<REDACTED>/llvm/projects/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_posix_libcdep.cc: In function ‘void __sanitizer::InitializeSwiftDemangler()’:
<REDACTED>/llvm/projects/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_posix_libcdep.cc:79:77: warning: ISO C++ forbids casting between pointer-to-function and pointer-to-object [-Wpedantic]
   swift_demangle_f = (swift_demangle_ft)dlsym(RTLD_DEFAULT, "swift_demangle");
                                                                             ^
[713/4353] Building C object projects/compiler-rt/li...ins/CMakeFiles/clang_rt.builtins-i386.dir/divtc3.c.o
<REDACTED>/llvm/projects/compiler-rt/lib/builtins/divtc3.c:21:1: warning: conflicting types for built-in function ‘__divtc3’ [enabled by default]
 __divtc3(long double __a, long double __b, long double __c, long double __d)
 ^
[2352/4353] Building C object projects/compiler-rt/l...s/CMakeFiles/clang_rt.builtins-x86_64.dir/divtc3.c.o
<REDACTED>/llvm/projects/compiler-rt/lib/builtins/divtc3.c:21:1: warning: conflicting types for built-in function ‘__divtc3’ [enabled by default]
 __divtc3(long double __a, long double __b, long double __c, long double __d)
 ^
[2522/4353] Building CXX object projects/compiler-rt...b/msan/CMakeFiles/clang_rt.msan-x86_64.dir/msan.cc.o
<REDACTED>/llvm/projects/compiler-rt/lib/msan/msan.cc: In function ‘__sanitizer::u16 __sanitizer_unaligned_load16(const uu16*)’:
<REDACTED>/llvm/projects/compiler-rt/lib/msan/msan.cc:586:32: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing]
   *(uu16 *)&__msan_retval_tls[0] = *(uu16 *)MEM_TO_SHADOW((uptr)p);
                                ^
<REDACTED>/llvm/projects/compiler-rt/lib/msan/msan.cc: In function ‘__sanitizer::u32 __sanitizer_unaligned_load32(const uu32*)’:
<REDACTED>/llvm/projects/compiler-rt/lib/msan/msan.cc:592:32: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing]
   *(uu32 *)&__msan_retval_tls[0] = *(uu32 *)MEM_TO_SHADOW((uptr)p);
                                ^
<REDACTED>/llvm/projects/compiler-rt/lib/msan/msan.cc: In function ‘void __sanitizer_unaligned_store16(__sanitizer::uu16*, __sanitizer::u16)’:
<REDACTED>/llvm/projects/compiler-rt/lib/msan/msan.cc:604:39: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing]
   u16 s = *(uu16 *)&__msan_param_tls[1];
                                       ^
<REDACTED>/llvm/projects/compiler-rt/lib/msan/msan.cc: In function ‘void __sanitizer_unaligned_store32(__sanitizer::uu32*, __sanitizer::u32)’:
<REDACTED>/llvm/projects/compiler-rt/lib/msan/msan.cc:612:39: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing]
   u32 s = *(uu32 *)&__msan_param_tls[1];
                                       ^
[2792/4353] Building CXX object projects/compiler-rt...Ubsan_minimal.x86_64.dir/ubsan_minimal_handlers.cc.o
<REDACTED>/llvm/projects/compiler-rt/lib/ubsan_minimal/ubsan_minimal_handlers.cc: In function ‘void message(const char*)’:
<REDACTED>/llvm/projects/compiler-rt/lib/ubsan_minimal/ubsan_minimal_handlers.cc:7:29: warning: ignoring return value of ‘ssize_t write(int, const void*, size_t)’, declared with attribute warn_unused_result [-Wunused-result]
   write(2, msg, strlen(msg));
                             ^
[2797/4353] Building CXX object projects/compiler-rt...RTUbsan_minimal.i386.dir/ubsan_minimal_handlers.cc.o
<REDACTED>/llvm/projects/compiler-rt/lib/ubsan_minimal/ubsan_minimal_handlers.cc: In function ‘void message(const char*)’:
<REDACTED>/llvm/projects/compiler-rt/lib/ubsan_minimal/ubsan_minimal_handlers.cc:7:29: warning: ignoring return value of ‘ssize_t write(int, const void*, size_t)’, declared with attribute warn_unused_result [-Wunused-result]
   write(2, msg, strlen(msg));
                             ^
[2862/4353] Building CXX object projects/compiler-rt...r/CMakeFiles/RTfuzzer.x86_64.dir/FuzzerIOPosix.cpp.o
<REDACTED>/llvm/projects/compiler-rt/lib/fuzzer/FuzzerIOPosix.cpp: In function ‘void fuzzer::RawPrint(const char*)’:
<REDACTED>/llvm/projects/compiler-rt/lib/fuzzer/FuzzerIOPosix.cpp:125:29: warning: ignoring return value of ‘ssize_t write(int, const void*, size_t)’, declared with attribute warn_unused_result [-Wunused-result]
   write(2, Str, strlen(Str));
                             ^
[3207/4353] Generating SVNVersion.inc
-- Found Subversion: /usr/bin/svn (found version "1.8.8") 
[3289/4353] Building CXX object projects/compiler-rt.../clang_rt.tsan-x86_64.dir/rtl/tsan_interceptors.cc.o
<REDACTED>/llvm/projects/compiler-rt/lib/tsan/rtl/tsan_interceptors.cc:374:28: warning: ISO C99 requires rest arguments to be used [enabled by default]
 TSAN_INTERCEPTOR(int, pause) {
                            ^
<REDACTED>/llvm/projects/compiler-rt/lib/tsan/rtl/tsan_interceptors.cc:375:32: warning: ISO C99 requires rest arguments to be used [enabled by default]
   SCOPED_TSAN_INTERCEPTOR(pause);
                                ^
hctim abandoned this revision.Oct 3 2017, 4:25 PM

Revision Contents

PathSize
lib/
sanitizer_common/
3 lines
stats/
9 lines

Diff 117583

lib/sanitizer_common/sanitizer_symbolizer_posix_libcdep.cc

Show First 20 Lines • Show All 70 Lines • ▼ Show 20 Linestypedef char *(*swift_demangle_ft)(const char *mangledName,
size_t mangledNameLength, char *outputBuffer, size_t mangledNameLength, char *outputBuffer,
size_t *outputBufferSize, uint32_t flags); size_t *outputBufferSize, uint32_t flags);
static swift_demangle_ft swift_demangle_f; static swift_demangle_ft swift_demangle_f;
// This must not happen lazily at symbolication time, because dlsym uses // This must not happen lazily at symbolication time, because dlsym uses
// malloc and thread-local storage, which is not a good thing to do during // malloc and thread-local storage, which is not a good thing to do during
// symbolication. // symbolication.
static void InitializeSwiftDemangler() { static void InitializeSwiftDemangler() {
swift_demangle_f = (swift_demangle_ft)dlsym(RTLD_DEFAULT, "swift_demangle"); uint64_t ptr = (uint64_t) dlsym(RTLD_DEFAULT, "swift_demangle");
swift_demangle_f = (swift_demangle_ft) ptr;
} }
// Attempts to demangle a Swift name. The demangler will return nullptr if a // Attempts to demangle a Swift name. The demangler will return nullptr if a
// non-Swift name is passed in. // non-Swift name is passed in.
const char *DemangleSwift(const char *name) { const char *DemangleSwift(const char *name) {
if (!name) return nullptr; if (!name) return nullptr;
// Check if we are dealing with a Swift mangled name first. // Check if we are dealing with a Swift mangled name first.
▲ Show 20 Lines • Show All 453 Lines • Show Last 20 Lines

lib/stats/stats_client.cc

Show All 25 Lines
#include "sanitizer_common/sanitizer_internal_defs.h" #include "sanitizer_common/sanitizer_internal_defs.h"
#include "stats/stats.h" #include "stats/stats.h"
using namespace __sanitizer; using namespace __sanitizer;
namespace { namespace {
void *LookupSymbolFromMain(const char *name) { typedef void (*void_fnptr)();
void_fnptr LookupSymbolFromMain(const char *name) {
#ifdef _WIN32 #ifdef _WIN32
return reinterpret_cast<void *>(GetProcAddress(GetModuleHandle(0), name)); return (void_fnptr) GetProcAddress(GetModuleHandle(0), name);
#else #else
return dlsym(RTLD_DEFAULT, name); uint64_t ptr = (uint64_t) dlsym(RTLD_DEFAULT, name);
return (void_fnptr) ptr;
#endif #endif
} }
StatModule *list; StatModule *list;
struct RegisterSanStats { struct RegisterSanStats {
unsigned module_id; unsigned module_id;
Show All 38 Lines