Commit bebb5c66 authored by Emily Andrews's avatar Emily Andrews Committed by Commit Bot

Modify mojo::internal::Validate* functions for 48KB disk space savings

mojo::internal::ValidateStruct is not able to fold properly even though
almost 90% of the disassembly of the defined template specializations is
the same because the call to T::Validate is unique.

This change slightly modifies the internals of mojo::internal::Validate*
to call a new helper function which pulls out most of the similar
functionality. While the helper function is also templated, because the
compiled specializations are identical, the compiler is able to fold all
the calls to the same function.

This reduces the overall size of chrome.dll and chrome_child.dll by
24 KB each for a total of 48 KB in disk space savings. If there are
other dlls that compile in mojo, these will experience similar benefits.

Other approaches considered included:
- creating an interface which defined helper functions within
T:Validate. While this did allow all the functions to fold for 400 KB
savings, this also opened us to a security risk because the *_Data
objects are sent via IPC and could be coming from anywhere.
- Including the ScopedDepthTracker call in the helper. This resulted in
bugs since the ScopedDepthTracker takes advantage of deconstructors to
keep track of how far down we've recursed through Validate function.
- Generalize the header validation code generated in T:Validate. This
resulted in a size increase. The reason is that we know the number of
loops at compile time, so the compiler is able to take advantage of loop
unrolling.

Bug: 988151
Change-Id: I80e8d35005dc3ec1a8c09762bf14b0823af7c3bd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1723023Reviewed-by: default avatarKen Rockot <rockot@google.com>
Reviewed-by: default avatarDaniel Cheng <dcheng@chromium.org>
Commit-Queue: Ken Rockot <rockot@google.com>
Cr-Commit-Position: refs/heads/master@{#682339}
parent 79fab97a
...@@ -147,16 +147,24 @@ bool ValidateHandleOrInterfaceNonNullable( ...@@ -147,16 +147,24 @@ bool ValidateHandleOrInterfaceNonNullable(
ValidationContext* validation_context); ValidationContext* validation_context);
template <typename T> template <typename T>
bool ValidateContainer(const Pointer<T>& input, bool ValidateParams(const Pointer<T>& input,
ValidationContext* validation_context, ValidationContext* validation_context) {
const ContainerValidateParams* validate_params) {
ValidationContext::ScopedDepthTracker depth_tracker(validation_context);
if (validation_context->ExceedsMaxDepth()) { if (validation_context->ExceedsMaxDepth()) {
ReportValidationError(validation_context, ReportValidationError(validation_context,
VALIDATION_ERROR_MAX_RECURSION_DEPTH); VALIDATION_ERROR_MAX_RECURSION_DEPTH);
return false; return false;
} }
return ValidatePointer(input, validation_context) &&
return ValidatePointer(input, validation_context);
}
template <typename T>
bool ValidateContainer(const Pointer<T>& input,
ValidationContext* validation_context,
const ContainerValidateParams* validate_params) {
ValidationContext::ScopedDepthTracker depth_tracker(validation_context);
return ValidateParams(input, validation_context) &&
T::Validate(input.Get(), validation_context, validate_params); T::Validate(input.Get(), validation_context, validate_params);
} }
...@@ -164,12 +172,8 @@ template <typename T> ...@@ -164,12 +172,8 @@ template <typename T>
bool ValidateStruct(const Pointer<T>& input, bool ValidateStruct(const Pointer<T>& input,
ValidationContext* validation_context) { ValidationContext* validation_context) {
ValidationContext::ScopedDepthTracker depth_tracker(validation_context); ValidationContext::ScopedDepthTracker depth_tracker(validation_context);
if (validation_context->ExceedsMaxDepth()) {
ReportValidationError(validation_context, return ValidateParams(input, validation_context) &&
VALIDATION_ERROR_MAX_RECURSION_DEPTH);
return false;
}
return ValidatePointer(input, validation_context) &&
T::Validate(input.Get(), validation_context); T::Validate(input.Get(), validation_context);
} }
...@@ -189,12 +193,8 @@ template <typename T> ...@@ -189,12 +193,8 @@ template <typename T>
bool ValidateNonInlinedUnion(const Pointer<T>& input, bool ValidateNonInlinedUnion(const Pointer<T>& input,
ValidationContext* validation_context) { ValidationContext* validation_context) {
ValidationContext::ScopedDepthTracker depth_tracker(validation_context); ValidationContext::ScopedDepthTracker depth_tracker(validation_context);
if (validation_context->ExceedsMaxDepth()) {
ReportValidationError(validation_context, return ValidateParams(input, validation_context) &&
VALIDATION_ERROR_MAX_RECURSION_DEPTH);
return false;
}
return ValidatePointer(input, validation_context) &&
T::Validate(input.Get(), validation_context, false); T::Validate(input.Get(), validation_context, false);
} }
......
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