[compiler-rt] r355236 - [sanitizers] Don't use Windows Trace Logging on MinGW
Evgenii Stepanov via llvm-commits
llvm-commits at lists.llvm.org
Fri Mar 1 17:33:37 PST 2019
r355231 reverted.
On Fri, Mar 1, 2019 at 5:28 PM Evgenii Stepanov <eugenis at google.com> wrote:
>
> Well, realistically, nothing we do at this point can make this code any uglier.
> Might as well add an argument to COMMON_INTERCEPTOR_ENTER that would
> evaluate to "return" for interceptors with a non-void return value,
> and to "" otherwise :(
>
> I'll revert for now.
>
> On Fri, Mar 1, 2019 at 5:11 PM Volodymyr Sapsai <vsapsai at apple.com> wrote:
> >
> > And the error is
> >
> > In file included from /Users/buildslave/jenkins/workspace/clang-stage1-configure-RA/llvm/projects/compiler-rt/lib/tsan/rtl/tsan_interceptors.cc:2351:
> >
> > /Users/buildslave/jenkins/workspace/clang-stage1-configure-RA/llvm/projects/compiler-rt/lib/tsan/../sanitizer_common/sanitizer_common_interceptors.inc:5538:3: error: void function 'wrap___bzero' should not return a value [-Wreturn-type]
> >
> > COMMON_INTERCEPTOR_MEMSET_IMPL(ctx, block, 0, size);
> > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > /Users/buildslave/jenkins/workspace/clang-stage1-configure-RA/llvm/projects/compiler-rt/lib/tsan/../sanitizer_common/sanitizer_common_interceptors.inc:246:5: note: expanded from macro 'COMMON_INTERCEPTOR_MEMSET_IMPL'
> > COMMON_INTERCEPTOR_ENTER(ctx, memset, dst, v, size); \
> > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > /Users/buildslave/jenkins/workspace/clang-stage1-configure-RA/llvm/projects/compiler-rt/lib/tsan/rtl/tsan_interceptors.cc:2241:3: note: expanded from macro 'COMMON_INTERCEPTOR_ENTER'
> > SCOPED_TSAN_INTERCEPTOR(func, __VA_ARGS__); \
> > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > /Users/buildslave/jenkins/workspace/clang-stage1-configure-RA/llvm/projects/compiler-rt/lib/tsan/rtl/tsan_interceptors.h:49:7: note: expanded from macro 'SCOPED_TSAN_INTERCEPTOR'
> > return REAL(func)(__VA_ARGS__); \
> > ^ ~~~~~~~~~~~~~~~~~~~~~~~
> >
> >
> >
> > On Mar 1, 2019, at 17:07, Volodymyr Sapsai via llvm-commits <llvm-commits at lists.llvm.org> wrote:
> >
> > There are 2 failing bots:
> > http://green.lab.llvm.org/green/job/clang-stage1-configure-RA/
> > http://green.lab.llvm.org/green/job/clang-stage1-cmake-RA-expensive/
> > The first one is more useful as the second one is much slower.
> >
> > Compiler diagnostic seems to be correct but unfortunately I don’t see a simple way to fix tsan interceptors.
> >
> > On Mar 1, 2019, at 17:02, Vlad Tsyrklevich <vlad at tsyrklevich.net> wrote:
> >
> > Volodymyr, your change fixed the build on the Linux sanitizer bots. I originally misread the buildbot data and thought it hadn't. I have no idea about macOS. I've pinged Evgenii to take a look at this thread, is there a broken bot you can point him to?
> >
> > On Fri, Mar 1, 2019 at 4:46 PM Volodymyr Sapsai <vsapsai at apple.com> wrote:
> >>
> >> Does your revert help with the build? Because we are still seeing compilation errors on macOS, presumably caused by r355231 https://github.com/llvm/llvm-project/commit/ddc4b7c1d6d4d4ffd8538b40c00c3fb16b1b1ab0
> >>
> >>
> >> On Mar 1, 2019, at 16:41, Vlad Tsyrklevich <vlad at tsyrklevich.net> wrote:
> >>
> >> I've reverted both of these changes in r355250. The sanitizer build fails on Linux and I have other failures to investigate. (For what it's worth Volodymyr, I took a brief look and it was unclear to me if either were correct. The Linux build was failing before and after your change.)
> >>
> >> On Fri, Mar 1, 2019 at 4:07 PM Volodymyr Sapsai via llvm-commits <llvm-commits at lists.llvm.org> wrote:
> >>>
> >>> Martin, I’ve made a small fix in r355244. See inline for more details
> >>>
> >>> > On Mar 1, 2019, at 14:30, Martin Storsjo via llvm-commits <llvm-commits at lists.llvm.org> wrote:
> >>> >
> >>> > Author: mstorsjo
> >>> > Date: Fri Mar 1 14:30:14 2019
> >>> > New Revision: 355236
> >>> >
> >>> > URL: http://llvm.org/viewvc/llvm-project?rev=355236&view=rev
> >>> > Log:
> >>> > [sanitizers] Don't use Windows Trace Logging on MinGW
> >>> >
> >>> > mingw-w64 currently is lacking the headers for this feature.
> >>> >
> >>> > Make the include lowercase at the same time. We consistently
> >>> > use lowercase for windows header includes, as windows itself is
> >>> > case insensitive, the SDK headers (in general, not necessarily
> >>> > considering this particular header) aren't consistent among themselves
> >>> > about what the proper canonical capitalization for headers are,
> >>> > and MinGW uses all lowercase names for the headers (as it is often
> >>> > used on case sensitive filesystems).
> >>> >
> >>> > In case mingw-w64 later gets this header, we can revert this
> >>> > (but keep the include lowercased).
> >>> >
> >>> > Differential Revision: https://reviews.llvm.org/D58765
> >>> >
> >>> > Modified:
> >>> > compiler-rt/trunk/lib/sanitizer_common/sanitizer_common.h
> >>> > compiler-rt/trunk/lib/sanitizer_common/sanitizer_win.cc
> >>> >
> >>> > Modified: compiler-rt/trunk/lib/sanitizer_common/sanitizer_common.h
> >>> > URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/sanitizer_common.h?rev=355236&r1=355235&r2=355236&view=diff
> >>> > ==============================================================================
> >>> > --- compiler-rt/trunk/lib/sanitizer_common/sanitizer_common.h (original)
> >>> > +++ compiler-rt/trunk/lib/sanitizer_common/sanitizer_common.h Fri Mar 1 14:30:14 2019
> >>> > @@ -804,7 +804,13 @@ enum AndroidApiLevel {
> >>> >
> >>> > void WriteToSyslog(const char *buffer);
> >>> >
> >>> > -#if SANITIZER_MAC || SANITIZER_WINDOWS
> >>> > +#if defined(SANITIZER_WINDOWS) && defined(_MSC_VER)
> >>> > +#define SANITIZER_WIN_TRACE 1
> >>> > +#else
> >>> > +#define SANITIZER_WIN_TRACE 0
> >>> > +#endif
> >>> > +
> >>> > +#if SANITIZER_LINUX || SANITIZER_WIN_TRACE
> >>> I think changing SANITIZER_MAC to SANITIZER_LINUX was a mistake and was copy-pasted from the part of the file below. But I can be wrong and maybe we need #if SANITIZER_MAC || SANITIZER_LINUX || SANITIZER_WIN_TRACE
> >>>
> >>> > void LogFullErrorReport(const char *buffer);
> >>> > #else
> >>> > INLINE void LogFullErrorReport(const char *buffer) {}
> >>> > @@ -818,7 +824,7 @@ INLINE void WriteOneLineToSyslog(const c
> >>> > INLINE void LogMessageOnPrintf(const char *str) {}
> >>> > #endif
> >>> >
> >>> > -#if SANITIZER_LINUX || SANITIZER_WINDOWS
> >>> > +#if SANITIZER_LINUX || SANITIZER_WIN_TRACE
> >>> > // Initialize Android logging. Any writes before this are silently lost.
> >>> > void AndroidLogInit();
> >>> > void SetAbortMessage(const char *);
> >>> >
> >>> > Modified: compiler-rt/trunk/lib/sanitizer_common/sanitizer_win.cc
> >>> > URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/sanitizer_win.cc?rev=355236&r1=355235&r2=355236&view=diff
> >>> > ==============================================================================
> >>> > --- compiler-rt/trunk/lib/sanitizer_common/sanitizer_win.cc (original)
> >>> > +++ compiler-rt/trunk/lib/sanitizer_common/sanitizer_win.cc Fri Mar 1 14:30:14 2019
> >>> > @@ -20,7 +20,6 @@
> >>> > #include <io.h>
> >>> > #include <psapi.h>
> >>> > #include <stdlib.h>
> >>> > -#include <TraceLoggingProvider.h>
> >>> >
> >>> > #include "sanitizer_common.h"
> >>> > #include "sanitizer_file.h"
> >>> > @@ -32,6 +31,8 @@
> >>> > #if defined(PSAPI_VERSION) && PSAPI_VERSION == 1
> >>> > #pragma comment(lib, "psapi")
> >>> > #endif
> >>> > +#if SANITIZER_WIN_TRACE
> >>> > +#include <traceloggingprovider.h>
> >>> > // Windows trace logging provider init
> >>> > #pragma comment(lib, "advapi32.lib")
> >>> > TRACELOGGING_DECLARE_PROVIDER(g_asan_provider);
> >>> > @@ -39,6 +40,9 @@ TRACELOGGING_DECLARE_PROVIDER(g_asan_pro
> >>> > TRACELOGGING_DEFINE_PROVIDER(g_asan_provider, "AddressSanitizerLoggingProvider",
> >>> > (0x6c6c766d, 0x3846, 0x4e6a, 0xa4, 0xfb, 0x5b,
> >>> > 0x53, 0x0b, 0xd0, 0xf3, 0xfa));
> >>> > +#else
> >>> > +#define TraceLoggingUnregister(x)
> >>> > +#endif
> >>> >
> >>> > // A macro to tell the compiler that this part of the code cannot be reached,
> >>> > // if the compiler supports this feature. Since we're using this in
> >>> > @@ -1080,6 +1084,7 @@ u32 GetNumberOfCPUs() {
> >>> > return sysinfo.dwNumberOfProcessors;
> >>> > }
> >>> >
> >>> > +#if SANITIZER_WIN_TRACE
> >>> > // TODO(mcgov): Rename this project-wide to PlatformLogInit
> >>> > void AndroidLogInit(void) {
> >>> > HRESULT hr = TraceLoggingRegister(g_asan_provider);
> >>> > @@ -1103,6 +1108,7 @@ void LogFullErrorReport(const char *buff
> >>> > TraceLoggingValue(buffer, "AsanReportContents"));
> >>> > }
> >>> > }
> >>> > +#endif // SANITIZER_WIN_TRACE
> >>> >
> >>> > } // namespace __sanitizer
> >>> >
> >>> >
> >>> >
> >>> > _______________________________________________
> >>> > llvm-commits mailing list
> >>> > llvm-commits at lists.llvm.org
> >>> > https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
> >>>
> >>> _______________________________________________
> >>> llvm-commits mailing list
> >>> llvm-commits at lists.llvm.org
> >>> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
> >>
> >>
> >
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at lists.llvm.org
> > https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
> >
> >
More information about the llvm-commits
mailing list