This is an archive of the discontinued LLVM Phabricator instance.

scudo: Make it thread-safe to set some runtime configuration flags.
ClosedPublic

Authored by pcc on Sep 29 2020, 5:10 PM.

Details

Summary

Move some of the flags previously in Options, as well as the
UseMemoryTagging flag previously in the primary allocator, into an
atomic variable so that it can be updated while other threads are
running. Relaxed accesses are used because we only have the requirement
that the other threads see the new value eventually.

The code is set up so that the variable is generally loaded once per
allocation function call with the exception of some rarely used code
such as error handlers. The flag bits can generally stay in a register
during the execution of the allocation function which means that they
can be branched on with minimal overhead (e.g. TBZ on aarch64).

Diff Detail

Event Timeline

pcc created this revision.Sep 29 2020, 5:10 PM
Herald added a project: Restricted Project. · View Herald TranscriptSep 29 2020, 5:10 PM
Herald added subscribers: Restricted Project, jfb, kristof.beyls. · View Herald Transcript
pcc requested review of this revision.Sep 29 2020, 5:10 PM
cryptoad accepted this revision.Sep 30 2020, 8:34 AM

LGTM with comment

compiler-rt/lib/scudo/standalone/options.h
1

LLVM header is missing
Also I used a #ifdef construct in all the other .h, so maybe keep that or consistency? Or eventually move them all to a pragma if it works everywhere.

This revision is now accepted and ready to land.Sep 30 2020, 8:34 AM
This revision was landed with ongoing or failed builds.Sep 30 2020, 9:43 AM
This revision was automatically updated to reflect the committed changes.
pcc added inline comments.Sep 30 2020, 9:43 AM
compiler-rt/lib/scudo/standalone/options.h
1

Oops, this was meant to start like all the other files (I used pragma once to start with out of laziness, then forgot to fix it before uploading). Fixed now.

I'm not actually sure why we don't use pragma once in LLVM since it's supported by all of the compilers that we support. I guess it may be something like "don't make it gratuitously difficult to build with a compiler that doesn't support it". At any rate I'll leave the situation as is for now.

eugenis added inline comments.Oct 1 2020, 1:44 PM
compiler-rt/lib/scudo/standalone/options.h
67

This could be more readable in the form of

do { ... } while (!atomic_compare_exchange_strong

Also, there is no need to reload Opts on each iteration - compare_exchange does it for you.

pcc added inline comments.Oct 2 2020, 10:31 AM
compiler-rt/lib/scudo/standalone/options.h
67