Re: [PATCH 1/5] Libsanitizer: merge from trunk with merge.sh.
Eric Gallager wrote: > On 11/5/19, Jakub Jelinek wrote: >> On Mon, Nov 04, 2019 at 04:10:27PM +0100, Martin Liska wrote: >>> libsanitizer/ChangeLog: >>> >>> 2019-11-05 Martin Liska >>> >>> * all source files: Merge from upstream r375507. >>> --- >>> libsanitizer/BlocksRuntime/Block.h| 59 + >>> libsanitizer/BlocksRuntime/Block_private.h| 179 ++ >> >> Do we really need this? probably not, I’ve been tied up with WG21, so not properly reviewed the libsanitizer merge yet.. However, I have a note to myself to check the *existing* interface that GCC is emitting for the facility on Darwin, since it differs from the one emitted by clang (that might/might not be imporant, it’s not yet analysed). > So, maybe we don't need this for the sanitizer itself, but if the > sanitizers now come with their own copy of the Blocks Runtime... > couldn't that be the solution as to where to take our blocks support > library for GCC proper from? No, this issue is not the absence of a blocks runtime (at least, not on Darwin) the issue is that the compiler support is missing. >> I mean, couldn't we wrap this Block.h include with #ifdef __BLOCKS__ or so >> as a local patch (at least for now)? > > This is bug 78352 btw: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78352 I haven’t got anything mature enough to post for inclusion in GCC-10, the last 6 months have been “playing catch up” and just didn’t leave enough time for this, sadly. thanks Iain
Re: [PATCH 1/5] Libsanitizer: merge from trunk with merge.sh.
On 11/5/19, Jakub Jelinek wrote:
> On Mon, Nov 04, 2019 at 04:10:27PM +0100, Martin Liska wrote:
>>
>> libsanitizer/ChangeLog:
>>
>> 2019-11-05 Martin Liska
>>
>> * all source files: Merge from upstream r375507.
>> ---
>> libsanitizer/BlocksRuntime/Block.h| 59 +
>> libsanitizer/BlocksRuntime/Block_private.h| 179 ++
>
> Do we really need this?
So, maybe we don't need this for the sanitizer itself, but if the
sanitizers now come with their own copy of the Blocks Runtime...
couldn't that be the solution as to where to take our blocks support
library for GCC proper from? So many previous discussions about adding
blocks support to GCC got derailed by the issue of licensing of the
Blocks Runtime, but if it's okay to include it as part of
libsanitizer, I'd say that should apply to the rest of GCC too...
>
>> --- a/libsanitizer/tsan/tsan_libdispatch.cpp
>> +++ b/libsanitizer/tsan/tsan_interceptors_libdispatch.cpp
>> @@ -1,4 +1,4 @@
>> -//===-- tsan_libdispatch.cpp
>> --===//
>> +//===-- tsan_interceptors_libdispatch.cpp
>> -===//
>> //
>> // Part of the LLVM Project, under the Apache License v2.0 with LLVM
>> Exceptions.
>> // See https://llvm.org/LICENSE.txt for license information.
>> @@ -16,6 +16,7 @@
>> #include "tsan_interceptors.h"
>> #include "tsan_rtl.h"
>>
>> +#include "BlocksRuntime/Block.h"
>> #include "tsan_dispatch_defs.h"
>>
>> namespace __tsan {
>
> I mean, couldn't we wrap this Block.h include with #ifdef __BLOCKS__ or so
> as a local patch (at least for now)?
This is bug 78352 btw: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78352
>
> Otherwise the patch series LGTM.
>
> Jakub
>
>
Re: [PATCH 1/5] Libsanitizer: merge from trunk with merge.sh.
On 11/5/19 1:23 PM, Jakub Jelinek wrote:
On Mon, Nov 04, 2019 at 04:10:27PM +0100, Martin Liska wrote:
libsanitizer/ChangeLog:
2019-11-05 Martin Liska
* all source files: Merge from upstream r375507.
---
libsanitizer/BlocksRuntime/Block.h| 59 +
libsanitizer/BlocksRuntime/Block_private.h| 179 ++
Do we really need this?
No, as we do not use tsan_interceptors_libdispatch.cpp file.
Originally I included the file so that I needed libsanitizer/BlocksRuntime/*
files. That is resolved now.
Martin
--- a/libsanitizer/tsan/tsan_libdispatch.cpp
+++ b/libsanitizer/tsan/tsan_interceptors_libdispatch.cpp
@@ -1,4 +1,4 @@
-//===-- tsan_libdispatch.cpp
--===//
+//===-- tsan_interceptors_libdispatch.cpp
-===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
@@ -16,6 +16,7 @@
#include "tsan_interceptors.h"
#include "tsan_rtl.h"
+#include "BlocksRuntime/Block.h"
#include "tsan_dispatch_defs.h"
namespace __tsan {
I mean, couldn't we wrap this Block.h include with #ifdef __BLOCKS__ or so
as a local patch (at least for now)?
Otherwise the patch series LGTM.
Jakub
Re: [PATCH 1/5] Libsanitizer: merge from trunk with merge.sh.
On Mon, Nov 04, 2019 at 04:10:27PM +0100, Martin Liska wrote:
>
> libsanitizer/ChangeLog:
>
> 2019-11-05 Martin Liska
>
> * all source files: Merge from upstream r375507.
> ---
> libsanitizer/BlocksRuntime/Block.h| 59 +
> libsanitizer/BlocksRuntime/Block_private.h| 179 ++
Do we really need this?
> --- a/libsanitizer/tsan/tsan_libdispatch.cpp
> +++ b/libsanitizer/tsan/tsan_interceptors_libdispatch.cpp
> @@ -1,4 +1,4 @@
> -//===-- tsan_libdispatch.cpp
> --===//
> +//===-- tsan_interceptors_libdispatch.cpp
> -===//
> //
> // Part of the LLVM Project, under the Apache License v2.0 with LLVM
> Exceptions.
> // See https://llvm.org/LICENSE.txt for license information.
> @@ -16,6 +16,7 @@
> #include "tsan_interceptors.h"
> #include "tsan_rtl.h"
>
> +#include "BlocksRuntime/Block.h"
> #include "tsan_dispatch_defs.h"
>
> namespace __tsan {
I mean, couldn't we wrap this Block.h include with #ifdef __BLOCKS__ or so
as a local patch (at least for now)?
Otherwise the patch series LGTM.
Jakub
