Skip to content

Commit

Permalink
option to treat exception constructors as strict
Browse files Browse the repository at this point in the history
Summary:
We also don't want to inline constructors of throwable (exception, error) classes, as they capture the current stack trace in a way that is sensitive to inlining.

This new behaviour is introduced via a new flag that is off by default. So this is in effect a behavior-preserving change.

TODO: Explore alternative approach that changes symbolication/positions.

Reviewed By: ssj933

Differential Revision: D63564560

fbshipit-source-id: f520533191c841ec0125b31a89c99e71fb4aa2e9
  • Loading branch information
Nikolai Tillmann authored and facebook-github-bot committed Oct 4, 2024
1 parent f77efcd commit 928029e
Show file tree
Hide file tree
Showing 4 changed files with 18 additions and 5 deletions.
2 changes: 2 additions & 0 deletions libredex/ConfigFiles.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -490,6 +490,8 @@ void ConfigFiles::load_inliner_config(inliner::InlinerConfig* inliner_config) {
jw.get("virtual", true, inliner_config->virtual_inline);
jw.get("true_virtual_inline", false, inliner_config->true_virtual_inline);
jw.get("relaxed_init_inline", false, inliner_config->relaxed_init_inline);
jw.get("strict_throwable_init_inline", false,
inliner_config->strict_throwable_init_inline);
jw.get("throws", false, inliner_config->throws_inline);
jw.get("throw_after_no_return", false, inliner_config->throw_after_no_return);
jw.get("max_cost_for_constant_propagation",
Expand Down
2 changes: 2 additions & 0 deletions libredex/GlobalConfig.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ void InlinerConfig::bind_config() {
bind("delete_non_virtuals", delete_non_virtuals, delete_non_virtuals);
bind("true_virtual_inline", true_virtual_inline, true_virtual_inline);
bind("relaxed_init_inline", relaxed_init_inline, relaxed_init_inline);
bind("strict_throwable_init_inline", strict_throwable_init_inline,
strict_throwable_init_inline);
bind("intermediate_shrinking", intermediate_shrinking,
intermediate_shrinking);
bind("enforce_method_size_limit", enforce_method_size_limit,
Expand Down
1 change: 1 addition & 0 deletions libredex/InlinerConfig.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ struct InlinerConfig {
bool virtual_inline{true};
bool true_virtual_inline{false};
bool relaxed_init_inline{false};
bool strict_throwable_init_inline{false};
bool throws_inline{false};
bool throw_after_no_return{false};
bool enforce_method_size_limit{true};
Expand Down
18 changes: 13 additions & 5 deletions service/method-inliner/Inliner.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1680,11 +1680,19 @@ bool MultiMethodInliner::can_inline_init(const DexMethod* init_method) {
// TODO T184662680: While this is not a correctness issue,
// we should fully support relaxed init methods in
// class-merging.
bool relaxed = m_config.relaxed_init_inline &&
m_shrinker.min_sdk() >= 21 &&
!klass::maybe_anonymous_class(
type_class(init_method->get_class())) &&
!is_finalizable(init_method->get_class());

// We also don't want to inline constructors of throwable
// (exception, error) classes, as they capture the current
// stack trace in a way that is sensitive to inlining.
bool relaxed =
m_config.relaxed_init_inline &&
m_shrinker.min_sdk() >= 21 &&
!klass::maybe_anonymous_class(
type_class(init_method->get_class())) &&
!is_finalizable(init_method->get_class()) &&
(!m_config.strict_throwable_init_inline ||
!type::check_cast(init_method->get_class(),
type::java_lang_Throwable()));
return constructor_analysis::can_inline_init(
init_method, finalizable_fields, relaxed);
})
Expand Down

0 comments on commit 928029e

Please sign in to comment.