This is an archive of the discontinued LLVM Phabricator instance.

[scudo][standalone] Introduce the C & C++ wrappers
ClosedPublic

Authored by cryptoad on Jun 20 2019, 9:53 AM.

Details

Summary

This CL adds C & C++ wrappers and associated tests. Those use default
configurations for a Scudo combined allocator that will likely be
tweaked in the future.

This is the final CL required to have a functional C & C++ allocator
based on Scudo.

The structure I have chosen is to define the core C allocation
primitives in an .inc file that can be customized through defines.
This allows to easily have 2 (or more) sets of wrappers backed by
different combined allocators, as demonstrated by the Bionic
wrappers: one set for the "default" allocator, one set for the "svelte"
allocator.

Currently all the tests added have been gtests, but I am planning to
add some more lit tests as well.

Event Timeline

cryptoad created this revision.Jun 20 2019, 9:53 AM
Herald added projects: Restricted Project, Restricted Project. · View Herald Transcript
Herald added subscribers: Restricted Project, jfb, delcypher and 2 others. · View Herald Transcript
cryptoad updated this revision to Diff 206001.Jun 21 2019, 8:07 AM

Fixing a couple of errors that came up in testing.

morehouse accepted this revision.Jun 24 2019, 11:14 AM
morehouse added inline comments.
lib/scudo/standalone/tests/wrappers_c_test.cc
79

Why do we only check errno on Android? If calloc returns nullptr, I'd expect errno to be set.

105

Just curious -- why?

lib/scudo/standalone/tests/wrappers_cpp_test.cc
70

Maybe test a class type as well?

lib/scudo/standalone/wrappers.inc
2

Should this be wrappers_c.inc?

This revision is now accepted and ready to land.Jun 24 2019, 11:14 AM
cryptoad marked 7 inline comments as done.Jun 24 2019, 1:39 PM
cryptoad added inline comments.
lib/scudo/standalone/tests/wrappers_c_test.cc
79

You are right. I somehow thought there was nothing specified in the standard for an errno for int overflow, but I found the following: "The UNIX 98 standard requires malloc(), calloc(), and realloc() to set errno to ENOMEM upon failure.", which I assume covers that case as well. I updated the wrappers accordingly. Thanks!

105

Backward compatibility with badly written code probably. There is a unit test in Bionic for it, so I have to support the use case, but I do not want it to bleed to other platforms.

lib/scudo/standalone/tests/wrappers_cpp_test.cc
70

Will add some.

cryptoad updated this revision to Diff 206295.Jun 24 2019, 1:40 PM
cryptoad marked 3 inline comments as done.

Changing some errno behaviors to be more consistent.
Renaming wrappers.inc to wrappers_c.inc and updated associated strings.

cryptoad updated this revision to Diff 206456.Jun 25 2019, 8:12 AM

Adding a testcase for a Class to the New tests.

This revision was automatically updated to reflect the committed changes.