[compiler-rt] [asan][win][msvc] override new and delete and seperate TUs (PR #68754)

Charlie Barto via llvm-commits llvm-commits at lists.llvm.org
Mon Nov 20 17:12:01 PST 2023


barcharcraz wrote:

> > For the declarations of the dllexported memory functions I don't understand how moving them would reduce duplicated code at all. Each fallback works through the operator, not the associated `__asan_*` function, so the declaration only happens once.
> 
> Declaration is not about duplication. It's to make sure that all involved *cpp files, implementation and users, see the same signature.
> 
> Deduplication is about diagrams, they look very similar to me. And on another look they are quite redundant, will be a chore of counting spaces if change is needed.
> 
> e.g. compiler-rt/lib/asan/asan_win_delete_array_align_nothrow_thunk.cpp
> 
> if I look at
> 
> ```
> // Avoid tailcall optimization to preserve stack frame.
> #pragma optimize("", off)
> void operator delete[](void* ptr, std::align_val_t align,
>                        std::nothrow_t const&) noexcept {
>   // nothrow version is identical to throwing version
>   operator delete[](ptr, align);
> }
> ```
> 
> It's not very obvious why the first half of the file is there and how edit the file to preserve this patch intention. Seems like full tree in the common header, with instruction for the tree, will work better.
> 
> Also There are more compact and easier to maintain tree visualizations. E.g. `tree` utility looks nicer to me
> 
> ```
> delete_scalar_align
> ├──delete_array_align
> │   ├── delete_array_size_align
> │   └── DELETE_ARRAY_ALIGN_NOTHROW
> ├── delete_scalar_size_align
> └── delete_scalar_align_nothrow
> ```

OK, There's already a full tree in asan_win_new_delete_thunk_common.h, so I'll change each comment in the individual cpp files to reference that

https://github.com/llvm/llvm-project/pull/68754


More information about the llvm-commits mailing list