[llvm-dev] Using C++ and sanitizer_common in profile runtime

[Xinliang David Li via llvm-dev]

Sounds good to me too.   +kcc@ to comment on the reuse/refactoring of
sanitizer_common library for common use.

David

On Thu, Jul 15, 2021 at 10:14 AM Vedant Kumar <vsk at apple.com> wrote:

> On Jul 14, 2021, at 5:50 PM, Petr Hosek <phosek at google.com> wrote:
>
>
> That renaming makes sense given that sanitizer_common is slowly becoming a
> common runtime library. At the risk of more bikeshedding, we could also
> consider naming it just 'common'. We might also consider breaking up
> sanitizer_common into multiple libraries like 'common', 'libcdep',
> 'symbolizer', 'sancov', matching the existing object libraries (see
> https://github.com/llvm/llvm-project/blob/0da172b/compiler-rt/lib/sanitizer_common/CMakeLists.txt
> ).
>
> I want to avoid introducing external dependencies (either direct or
> transitive via sanitizer_common), in fact my goal is the opposite, I want
> to eliminate the existing dependency on libc. This is important for us
> since we use the profile runtime in our kernel and our libc. The use of C++
> as an implementation language shouldn't be a problem, we have plenty of
> experience using C++ in bare metal contexts.
>
>
> Sounds good, I think we're on the same page.
>
> From the user perspective this change should be invisible and the
> libclang_rt.profile ABI should ideally remain the same. We can introduce
> tooling to enforce that by comparing libclang_rt.profile ABI against a list
> that's checked into the repository, which is an approach that other
> runtimes use as well.
>
>
> This would be great to have. At the moment, we only have profile runtime
> tests that certain symbols *aren't* used/exported
> (instrprof-{darwin-exports,without-libc}.c).
>
> thanks,
> vedant
>
>
> On Wed, Jul 14, 2021 at 5:19 PM Xinliang David Li <davidxl at google.com>
> wrote:
>
>> I actually meant 'runtime_common'.
>>
>> David
>>
>> On Wed, Jul 14, 2021 at 5:07 PM Vedant Kumar <vsk at apple.com> wrote:
>>
>>> On Jul 14, 2021, at 4:56 PM, Xinliang David Li <davidxl at google.com>
>>> wrote:
>>>
>>>
>>> Can we introduce C wrappers to sanitizer_common and use them in profile
>>> runtime? Of course 'sanitizer_common' needs to become 'profile_common'.
>>>
>>>
>>> I like the sound of 'runtime_common' a bit better (sorry for the
>>> bikeshed :).
>>>
>>> vedant
>>>
>>>
>>>
>>> David
>>>
>>> On Wed, Jul 14, 2021 at 4:14 PM Petr Hosek <phosek at google.com> wrote:
>>>
>>>> What are your thoughts on using C++ and sanitizer_common in profile
>>>> runtime?
>>>>
>>>> First, to clarify, I'm not suggesting using the standard C++ library.
>>>> What I'm suggesting is just using the C++ language akin to sanitizer
>>>> runtimes or memprof.
>>>>
>>>> profile runtime is the last major runtime in compiler-rt that's
>>>> implemented in C and switching to C++ could make the compiler-rt codebase
>>>> more uniform. Beyond that, I think this could be beneficial for several
>>>> reasons:
>>>>
>>>> * _Reducing the duplication between profile runtime and
>>>> sanitizer_common._ Both of these implement various polyfills for different
>>>> platforms (for example the mmap like interface) and it would be great if we
>>>> could avoid duplicating the effort and share a common implementation.
>>>>
>>>> * _Avoiding potential cyclic dependencies in profile runtime._ The
>>>> implementation currently uses various libc functions, but if libc is
>>>> profile instrumented there is a risk of potentially entering an infinite
>>>> recursion. This could be avoided by using internal implementations of these
>>>> libc functions which sanitizer_common does, but if we wanted to take the
>>>> same approach for profile runtime, we would need to re-implement these
>>>> functions again further increasing duplication.
>>>>
>>>> * _Simplifying the profile runtime implementation._ Some parts of the
>>>> runtime already use object-oriented style interfaces, that is structs with
>>>> function pointers, replacing these with classes would make the code
>>>> cleaner. Furthermore, we could use classes to abstract away some of the
>>>> platform implementation which currently relies on ifdefs. We could also
>>>> take the advantage of RAII for resource management.
>>>>
>>>> If we decided to go ahead with this change, I'd suggest an incremental
>>>> transition rather than doing a major rewrite. We would start by ensuring
>>>> that all source files compile as C++. Then we would introduce the
>>>> sanitizer_common dependency and see which functions could be replaced by
>>>> sanitizer_common counterparts. Finally, we could refactor the
>>>> implementation and take advantage of C++ constructs where appropriate.
>>>>
>>>
>>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20210715/862cdcb2/attachment.html>