Xinliang David Li via llvm-dev
2021-Jul-15 00:19 UTC
[llvm-dev] Using C++ and sanitizer_common in profile runtime
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/20210714/0e83ac13/attachment.html>
Petr Hosek via llvm-dev
2021-Jul-15 00:50 UTC
[llvm-dev] Using C++ and sanitizer_common in profile runtime
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.>From the user perspective this change should be invisible and thelibclang_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. 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/20210714/06712292/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/06712292/attachment.bin>