[vm] Do not leak string values of flags

Previously repetetively parsing some string valued
flags would cause their values to leak.

This change makes sure that we delete the previous
value when we assign a new one.

vm/cc/ParseFlags is extended to catch this when running
under ASAN.

TEST=tools/test.py -n dartk-asan-linux-release-x64 vm/cc/ParseFlags

Change-Id: I7478cdb48063dcae35d4129a4c9a2829dddae729
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/267821
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Commit-Queue: Slava Egorov <vegorov@google.com>
This commit is contained in:
Vyacheslav Egorov 2022-11-03 22:41:53 +00:00 committed by Commit Queue
parent d65a0c2de5
commit 8de4432f68
2 changed files with 18 additions and 18 deletions

View file

@ -9,14 +9,6 @@
#include "vm/json_stream.h"
#include "vm/os.h"
#if defined(DART_USE_ABSL)
#include "third_party/absl/debugging/leak_check.h"
#define ABSL_IGNORE_LEAK(expr) absl::IgnoreLeak(expr)
#else
#define ABSL_IGNORE_LEAK(expr) (expr)
#endif // defined(DART_USE_ABSL)
namespace dart {
DEFINE_FLAG(bool, print_flags, false, "Print flags as they are being parsed.");
@ -104,13 +96,11 @@ class Flag {
Flag(const char* name, const char* comment, FlagHandler handler)
: name_(name),
comment_(comment),
string_value_("false"),
flag_handler_(handler),
type_(kFlagHandler) {}
Flag(const char* name, const char* comment, OptionHandler handler)
: name_(name),
comment_(comment),
string_value_(nullptr),
option_handler_(handler),
type_(kOptionHandler) {}
@ -158,7 +148,11 @@ class Flag {
const char* name_;
const char* comment_;
const char* string_value_;
// For kString, kOptionHandler, kFlagHandler flags this stores the copy
// of the original flag value passed to SetFlagFromString
Utils::CStringUniquePtr string_value_ =
Utils::CreateCStringUniquePtr(nullptr);
union {
void* addr_;
bool* bool_ptr_;
@ -168,7 +162,7 @@ class Flag {
FlagHandler flag_handler_;
OptionHandler option_handler_;
};
FlagType type_;
const FlagType type_;
bool changed_ = false;
};
@ -302,8 +296,9 @@ bool Flags::SetFlagFromString(Flag* flag, const char* argument) {
break;
}
case Flag::kString: {
*flag->charp_ptr_ =
argument == NULL ? NULL : ABSL_IGNORE_LEAK(Utils::StrDup(argument));
flag->string_value_.reset(argument == nullptr ? nullptr
: Utils::StrDup(argument));
*flag->charp_ptr_ = flag->string_value_.get();
break;
}
case Flag::kInteger: {
@ -344,11 +339,11 @@ bool Flags::SetFlagFromString(Flag* flag, const char* argument) {
} else {
return false;
}
flag->string_value_ = argument;
flag->string_value_.reset(Utils::StrDup(argument));
break;
}
case Flag::kOptionHandler: {
flag->string_value_ = argument;
flag->string_value_.reset(Utils::StrDup(argument));
(flag->option_handler_)(argument);
break;
}
@ -536,13 +531,14 @@ void Flags::PrintFlagToJSONArray(JSONArray* jsarr, const Flag* flag) {
}
case Flag::kFlagHandler: {
jsflag.AddProperty("_flagType", "Bool");
jsflag.AddProperty("valueAsString", flag->string_value_);
const char* value = flag->string_value_.get();
jsflag.AddProperty("valueAsString", value == nullptr ? "false" : value);
break;
}
case Flag::kOptionHandler: {
jsflag.AddProperty("_flagType", "String");
if (flag->string_value_ != nullptr) {
jsflag.AddProperty("valueAsString", flag->string_value_);
jsflag.AddProperty("valueAsString", flag->string_value_.get());
} else {
// valueAsString missing means NULL.
}

View file

@ -39,6 +39,10 @@ VM_UNIT_TEST_CASE(ParseFlags) {
Flags::Parse("string_opt_test=doobidoo");
EXPECT_EQ(true, FLAG_string_opt_test != NULL);
EXPECT_EQ(0, strcmp(FLAG_string_opt_test, "doobidoo"));
FLAG_string_opt_test = reinterpret_cast<charp>(0xDEADBEEF);
Flags::Parse("string_opt_test=foofoo");
EXPECT_EQ(true, FLAG_string_opt_test != NULL);
EXPECT_EQ(0, strcmp(FLAG_string_opt_test, "foofoo"));
EXPECT_EQ(true, FLAG_entrypoint_test != NULL);
EXPECT_EQ(0, strcmp(FLAG_entrypoint_test, "main"));