[PATCH] D36970: [sanitizer] Do not over-dup string flags

Kostya Kortchinsky via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Aug 21 09:38:09 PDT 2017


cryptoad created this revision.
Herald added a subscriber: kubamracek.

String flags values appear to be duped twice. Once in `FlagParser::parse_flag`
using the `LowLevelAllocator` via `ll_strndup`, once in
`FlagHandler<const char *>::Parse` using the `InternalAllocator` via
`internal_strdup`. It looks like the second one is redundant, as the memory
for the first one is never freed and not used for anything else.

Assigning the value to the flag instead of duping it has a few advantages:

- if it was the only use of the `InternalAllocator` (which is the case for Scudo), then the related code will not be compiled it, which saves us a whole instantiation of the CombinedAllocator worth of extra code;
- in the event a string flag is parsed, the `InternalAllocator` would have created a whole SizeClassAllocator32 region for a single allocation, which is kind of wasteful.
- also, the string is dup'ed twice for the whole lifetime of a process.

I tested check-{sanitizer,asan,tsan,ubsan,scudo} successfully, so as far as I
can tell this doesn't appear to have bad side effects.


https://reviews.llvm.org/D36970

Files:
  lib/sanitizer_common/sanitizer_flag_parser.h


Index: lib/sanitizer_common/sanitizer_flag_parser.h
===================================================================
--- lib/sanitizer_common/sanitizer_flag_parser.h
+++ lib/sanitizer_common/sanitizer_flag_parser.h
@@ -75,7 +75,7 @@
 
 template <>
 inline bool FlagHandler<const char *>::Parse(const char *value) {
-  *t_ = internal_strdup(value);
+  *t_ = value;
   return true;
 }
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D36970.112000.patch
Type: text/x-patch
Size: 388 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170821/de2e3b2a/attachment.bin>


More information about the llvm-commits mailing list