Commit ff0f1535 authored by Kenton Varda's avatar Kenton Varda

Fix uninitialized memory usage found by valgrind.

parent dec098cc
...@@ -565,7 +565,7 @@ class ListBuilder: public kj::DisallowConstCopy { ...@@ -565,7 +565,7 @@ class ListBuilder: public kj::DisallowConstCopy {
public: public:
inline ListBuilder() inline ListBuilder()
: segment(nullptr), ptr(nullptr), elementCount(0 * ELEMENTS), : segment(nullptr), ptr(nullptr), elementCount(0 * ELEMENTS),
step(0 * BITS / ELEMENTS) {} step(0 * BITS / ELEMENTS), elementSize(ElementSize::VOID) {}
MSVC_DEFAULT_ASSIGNMENT_WORKAROUND(, ListBuilder) MSVC_DEFAULT_ASSIGNMENT_WORKAROUND(, ListBuilder)
......
...@@ -1261,21 +1261,24 @@ _::RawSchema* SchemaLoader::Impl::load(const schema::Node::Reader& reader, bool ...@@ -1261,21 +1261,24 @@ _::RawSchema* SchemaLoader::Impl::load(const schema::Node::Reader& reader, bool
// Check if we already have a schema for this ID. // Check if we already have a schema for this ID.
_::RawSchema*& slot = schemas[validatedReader.getId()]; _::RawSchema*& slot = schemas[validatedReader.getId()];
bool shouldReplace; bool shouldReplace;
bool shouldClearInitializer;
if (slot == nullptr) { if (slot == nullptr) {
// Nope, allocate a new RawSchema. // Nope, allocate a new RawSchema.
slot = &arena.allocate<_::RawSchema>(); slot = &arena.allocate<_::RawSchema>();
memset(&slot->defaultBrand, 0, sizeof(slot->defaultBrand));
slot->id = validatedReader.getId(); slot->id = validatedReader.getId();
slot->canCastTo = nullptr; slot->canCastTo = nullptr;
slot->defaultBrand.generic = slot; slot->defaultBrand.generic = slot;
slot->lazyInitializer = isPlaceholder ? &initializer : nullptr;
slot->defaultBrand.lazyInitializer = isPlaceholder ? &brandedInitializer : nullptr;
shouldReplace = true; shouldReplace = true;
shouldClearInitializer = false;
} else { } else {
// Yes, check if it is compatible and figure out which schema is newer. // Yes, check if it is compatible and figure out which schema is newer.
if (slot->lazyInitializer == nullptr) { // If the existing slot is a placeholder, but we're upgrading it to a non-placeholder, we
// The existing slot is not a placeholder, so whether we overwrite it or not, we cannot // need to clear the initializer later.
// end up with a placeholder. shouldClearInitializer = slot->lazyInitializer != nullptr && !isPlaceholder;
isPlaceholder = false;
}
auto existing = readMessageUnchecked<schema::Node>(slot->encodedNode); auto existing = readMessageUnchecked<schema::Node>(slot->encodedNode);
CompatibilityChecker checker(*this); CompatibilityChecker checker(*this);
...@@ -1301,10 +1304,7 @@ _::RawSchema* SchemaLoader::Impl::load(const schema::Node::Reader& reader, bool ...@@ -1301,10 +1304,7 @@ _::RawSchema* SchemaLoader::Impl::load(const schema::Node::Reader& reader, bool
slot->defaultBrand.dependencyCount = deps.size(); slot->defaultBrand.dependencyCount = deps.size();
} }
if (isPlaceholder) { if (shouldClearInitializer) {
slot->lazyInitializer = &initializer;
slot->defaultBrand.lazyInitializer = &brandedInitializer;
} else {
// If this schema is not newly-allocated, it may already be in the wild, specifically in the // If this schema is not newly-allocated, it may already be in the wild, specifically in the
// dependency list of other schemas. Once the initializer is null, it is live, so we must do // dependency list of other schemas. Once the initializer is null, it is live, so we must do
// a release-store here. // a release-store here.
...@@ -1318,10 +1318,15 @@ _::RawSchema* SchemaLoader::Impl::load(const schema::Node::Reader& reader, bool ...@@ -1318,10 +1318,15 @@ _::RawSchema* SchemaLoader::Impl::load(const schema::Node::Reader& reader, bool
_::RawSchema* SchemaLoader::Impl::loadNative(const _::RawSchema* nativeSchema) { _::RawSchema* SchemaLoader::Impl::loadNative(const _::RawSchema* nativeSchema) {
_::RawSchema*& slot = schemas[nativeSchema->id]; _::RawSchema*& slot = schemas[nativeSchema->id];
bool shouldReplace; bool shouldReplace;
bool shouldClearInitializer;
if (slot == nullptr) { if (slot == nullptr) {
slot = &arena.allocate<_::RawSchema>(); slot = &arena.allocate<_::RawSchema>();
memset(&slot->defaultBrand, 0, sizeof(slot->defaultBrand));
slot->defaultBrand.generic = slot; slot->defaultBrand.generic = slot;
slot->lazyInitializer = nullptr;
slot->defaultBrand.lazyInitializer = nullptr;
shouldReplace = true; shouldReplace = true;
shouldClearInitializer = false; // already cleared above
} else if (slot->canCastTo != nullptr) { } else if (slot->canCastTo != nullptr) {
// Already loaded natively, or we're currently in the process of loading natively and there // Already loaded natively, or we're currently in the process of loading natively and there
// was a dependency cycle. // was a dependency cycle.
...@@ -1336,6 +1341,7 @@ _::RawSchema* SchemaLoader::Impl::loadNative(const _::RawSchema* nativeSchema) { ...@@ -1336,6 +1341,7 @@ _::RawSchema* SchemaLoader::Impl::loadNative(const _::RawSchema* nativeSchema) {
auto native = readMessageUnchecked<schema::Node>(nativeSchema->encodedNode); auto native = readMessageUnchecked<schema::Node>(nativeSchema->encodedNode);
CompatibilityChecker checker(*this); CompatibilityChecker checker(*this);
shouldReplace = checker.shouldReplace(existing, native, true); shouldReplace = checker.shouldReplace(existing, native, true);
shouldClearInitializer = slot->lazyInitializer != nullptr;
} }
// Since we recurse below, the slot in the hash map could move around. Copy out the pointer // Since we recurse below, the slot in the hash map could move around. Copy out the pointer
...@@ -1389,11 +1395,13 @@ _::RawSchema* SchemaLoader::Impl::loadNative(const _::RawSchema* nativeSchema) { ...@@ -1389,11 +1395,13 @@ _::RawSchema* SchemaLoader::Impl::loadNative(const _::RawSchema* nativeSchema) {
} }
} }
// If this schema is not newly-allocated, it may already be in the wild, specifically in the if (shouldClearInitializer) {
// dependency list of other schemas. Once the initializer is null, it is live, so we must do // If this schema is not newly-allocated, it may already be in the wild, specifically in the
// a release-store here. // dependency list of other schemas. Once the initializer is null, it is live, so we must do
__atomic_store_n(&result->lazyInitializer, nullptr, __ATOMIC_RELEASE); // a release-store here.
__atomic_store_n(&result->defaultBrand.lazyInitializer, nullptr, __ATOMIC_RELEASE); __atomic_store_n(&result->lazyInitializer, nullptr, __ATOMIC_RELEASE);
__atomic_store_n(&result->defaultBrand.lazyInitializer, nullptr, __ATOMIC_RELEASE);
}
return result; return result;
} }
...@@ -1513,6 +1521,7 @@ const _::RawBrandedSchema* SchemaLoader::Impl::makeBranded( ...@@ -1513,6 +1521,7 @@ const _::RawBrandedSchema* SchemaLoader::Impl::makeBranded(
if (slot == nullptr) { if (slot == nullptr) {
auto& brand = arena.allocate<_::RawBrandedSchema>(); auto& brand = arena.allocate<_::RawBrandedSchema>();
memset(&brand, 0, sizeof(brand));
slot = &brand; slot = &brand;
brand.generic = schema; brand.generic = schema;
...@@ -1537,10 +1546,11 @@ SchemaLoader::Impl::makeBrandedDependencies( ...@@ -1537,10 +1546,11 @@ SchemaLoader::Impl::makeBrandedDependencies(
#define ADD_ENTRY(kind, index, make) \ #define ADD_ENTRY(kind, index, make) \
if (const _::RawBrandedSchema* dep = make) { \ if (const _::RawBrandedSchema* dep = make) { \
deps.add(_::RawBrandedSchema::Dependency {\ auto& slot = deps.add(); \
_::RawBrandedSchema::makeDepLocation(_::RawBrandedSchema::DepKind::kind, index), \ memset(&slot, 0, sizeof(slot)); \
dep \ slot.location = _::RawBrandedSchema::makeDepLocation( \
}); \ _::RawBrandedSchema::DepKind::kind, index); \
slot.schema = dep; \
} }
switch (node.which()) { switch (node.which()) {
...@@ -1788,6 +1798,7 @@ const _::RawBrandedSchema* SchemaLoader::Impl::getUnbound(const _::RawSchema* sc ...@@ -1788,6 +1798,7 @@ const _::RawBrandedSchema* SchemaLoader::Impl::getUnbound(const _::RawSchema* sc
auto& slot = unboundBrands[schema]; auto& slot = unboundBrands[schema];
if (slot == nullptr) { if (slot == nullptr) {
slot = &arena.allocate<_::RawBrandedSchema>(); slot = &arena.allocate<_::RawBrandedSchema>();
memset(slot, 0, sizeof(*slot));
slot->generic = schema; slot->generic = schema;
auto deps = makeBrandedDependencies(schema, nullptr); auto deps = makeBrandedDependencies(schema, nullptr);
slot->dependencies = deps.begin(); slot->dependencies = deps.begin();
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment