llvm dev - Jul 2021 - Using C++ and sanitizer_common in profile runtime

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/20210714/59a61414/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/pkcs7-signature
Size: 3996 bytes
Desc: S/MIME Cryptographic Signature
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20210714/59a61414/attachment.bin>

Can we introduce C wrappers to sanitizer_common and use them in profile
runtime? Of course 'sanitizer_common' needs to become
'profile_common'.

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/20210714/52c481fa/attachment.html>

Hi Petr,
> On 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.
These strike me as compelling reasons to use C++ and sanitizer_common in the
profile runtime, and the transition plan you've outlined sounds reasonable
to me.

For Apple's use cases, it's important that it remain possible to use the
profile runtime in bare metal contexts. Chiefly this means that it should be
possible to build a freestanding subset of the objects in libprofile as a static
archive (see `darwin_add_builtin_libraries` in
cmake/Modules/CompilerRTDarwinUtils.cmake for the specific subset), s.t.
kernels/firmware can link it. The transition to C++/sanitizer_common doesn't
pose a risk to freestanding support so long as files in that subset don't
pick up new dependencies on libc, libc++abi, etc.

thanks,
vedant