[scudo] Provide allocator declaration

[ddcc]

Summary

Ensure that extern allocator declaration is visible before definition

[vitalybuka]

Can you please split #include cleanup and the rest?

compiler-rt/lib/scudo/standalone/string_utils.cpp

13 ↗

(On Diff #415970)

there is memset on :50

compiler-rt/lib/scudo/standalone/vector.h

14 ↗

(On Diff #415970)

it's used on :46

compiler-rt/lib/scudo/standalone/wrappers_c.h

22

Why do we need this?

[ddcc]

Do you split want the non-#include changes split out?

compiler-rt/lib/scudo/standalone/string_utils.cpp

13 ↗

(On Diff #415970)

Same thing, common.h already provides it.

compiler-rt/lib/scudo/standalone/vector.h

14 ↗

(On Diff #415970)

common.h includes string.h already, so this additional include is unnecessary. We have some custom library paths in our build system, so I needed to go through each include.

compiler-rt/lib/scudo/standalone/wrappers_c.h

22

-Wmissing-variable-declarations complains about no previous extern declaration for Allocator in wrappers_c.cpp, so the intention was to centralize the duplicate extern declarations between wrappers_c.cpp and wrapper_cpp.cpp, but I think that got lost while splitting the commits, and I missed that there's a SCUDO_PREFIX macro here, so I'll just add a declaration to wrappers_c.cpp instead.

[ddcc]

Fix extern declaration of SCUDO_ALLOCATOR

[vitalybuka]

compiler-rt/lib/scudo/standalone/vector.h

14 ↗

(On Diff #415970)

Unless there is a strong reason to do so, I am for IWYU.

[ddcc]

Revert header file removal

compiler-rt/lib/scudo/standalone/vector.h

14 ↗

(On Diff #415970)

Ok, I've dropped the header removals from this patch

[vitalybuka]

compiler-rt/lib/scudo/standalone/wrappers_c.h

22

Cab we move extern to the patch where actually it's necessary?

[ddcc]

compiler-rt/lib/scudo/standalone/wrappers_c.cpp

24

@vitalybuka Are you saying to split this change off into another patch?

[vitalybuka]

compiler-rt/lib/scudo/standalone/common.h

120 ↗

(On Diff #416584)

There is the same in cpp file.
I believe the point was to hide declaration from the header to avoid use by mistake.

compiler-rt/lib/scudo/standalone/wrappers_c.cpp

27–28

if we need extern, I assumed there is a some new user outside this cpp file.
But I don't see benefits of having extern just before the actual var declaration.

I believe correct approach is how it's done in wrappers_c_bionic.cpp.

Separate patch for unrelated stuff will be nice. Actually each of this fixes better to have as a separate patch with "why" in description.

[ddcc]

compiler-rt/lib/scudo/standalone/wrappers_c.cpp

27–28

The comment above this line seems to suggest that it's not feasible to have separate allocators for C and C++. I did try it and it seemed to work fine locally, but I suspect there may be other issues that haven't yet appeared. For example, what if a heap allocation from the C++ scudo allocator is freed by C code or vice versa?

[vitalybuka]

compiler-rt/lib/scudo/standalone/wrappers_c.cpp

27–28

I see, I missed cpp file

then probably in wrappers_c.h
#if !SCUDO_ANDROID || !_BIONIC
extern HIDDEN scudo::Allocator<scudo::Config, malloc_postinit> Allocator;
#endif

and remove extern in cpp file

[ddcc]

Move extern declarations to common header

compiler-rt/lib/scudo/standalone/wrappers_c.h

 
 // Bionic's struct mallinfo consists of size_t (mallinfo(3) uses int).
 #if SCUDO_ANDROID
 typedef size_t __scudo_mallinfo_data_t;
 #else
 typedef int __scudo_mallinfo_data_t;
 #endif
 
 struct __scudo_mallinfo {
   __scudo_mallinfo_data_t arena;
   __scudo_mallinfo_data_t ordblks;
   __scudo_mallinfo_data_t smblks;
   __scudo_mallinfo_data_t hblks;
   __scudo_mallinfo_data_t hblkhd;
   __scudo_mallinfo_data_t usmblks;
   __scudo_mallinfo_data_t fsmblks;
   __scudo_mallinfo_data_t uordblks;

 // conflicting return types for mallinfo() if we use our own structure. So if
 // struct mallinfo is declared (#define courtesy of malloc.h), use it directly.
 #if STRUCT_MALLINFO_DECLARED
 #define SCUDO_MALLINFO mallinfo
 #else
 #define SCUDO_MALLINFO __scudo_mallinfo
 #endif
 
+#if !SCUDO_ANDROID || !_BIONIC
+extern "C" void malloc_postinit();
+extern HIDDEN scudo::Allocator<scudo::Config, malloc_postinit> Allocator;
+#endif
+
 #endif // SCUDO_WRAPPERS_C_H_

compiler-rt/lib/scudo/standalone/wrappers_c.cpp

 #include "wrappers_c_checks.h"
 
 #include <stdint.h>
 #include <stdio.h>
 
 #define SCUDO_PREFIX(name) name
 #define SCUDO_ALLOCATOR Allocator
 
-extern "C" void SCUDO_PREFIX(malloc_postinit)();
-
 // Export the static allocator so that the C++ wrappers can access it.
 // Technically we could have a completely separated heap for C & C++ but in
 // reality the amount of cross pollination between the two is staggering.
 SCUDO_REQUIRE_CONSTANT_INITIALIZATION
 scudo::Allocator<scudo::Config, SCUDO_PREFIX(malloc_postinit)> SCUDO_ALLOCATOR;
 
 #include "wrappers_c.inc"
 
 #undef SCUDO_ALLOCATOR
 #undef SCUDO_PREFIX
 
 extern "C" INTERFACE void __scudo_print_stats(void) { Allocator.printStats(); }
 
 #endif // !SCUDO_ANDROID || !_BIONIC

compiler-rt/lib/scudo/standalone/wrappers_cpp.cpp

 //===-- wrappers_cpp.cpp ----------------------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "platform.h"
 
 // Skip this compilation unit if compiled as part of Bionic.
 #if !SCUDO_ANDROID || !_BIONIC
 
 #include "allocator_config.h"
+#include "wrappers_c.h"
 
 #include <stdint.h>
 
-extern "C" void malloc_postinit();
-extern HIDDEN scudo::Allocator<scudo::Config, malloc_postinit> Allocator;
-
 namespace std {
 struct nothrow_t {};
 enum class align_val_t : size_t {};
 } // namespace std
 
 INTERFACE WEAK void *operator new(size_t size) {
   return Allocator.allocate(size, scudo::Chunk::Origin::New);
 }