Skip to content

Fix delegation def path hash collision#153955

Draft
aerooneqq wants to merge 1 commit intorust-lang:mainfrom
aerooneqq:def-path-hash-collision
Draft

Fix delegation def path hash collision#153955
aerooneqq wants to merge 1 commit intorust-lang:mainfrom
aerooneqq:def-path-hash-collision

Conversation

@aerooneqq
Copy link
Contributor

@aerooneqq aerooneqq commented Mar 16, 2026

This PR addresses the following delegation issues:

Fixes #153410. Fixes #143498. Part of #118212.

r? @petrochenkov

DUMMY_SP related ICE
thread 'rustc' (11124) panicked at compiler\rustc_errors\src\diagnostic.rs:946:9:
Span must not be empty and have no suggestion
stack backtrace:
   0: std::panicking::panic_handler
             at /rustc/ad726b5063362ec9897ef3d67452fc5606ee70fa/library\std\src\panicking.rs:689
   1: core::panicking::panic_fmt
             at /rustc/ad726b5063362ec9897ef3d67452fc5606ee70fa/library\core\src\panicking.rs:80
   2: rustc_errors::diagnostic::Diag::span_suggestion_with_style<rustc_span::ErrorGuaranteed,alloc::string::String,ref$<str$> >
             at .\compiler\rustc_errors\src\diagnostic.rs:946
   3: rustc_errors::diagnostic::Diag::span_suggestion<rustc_span::ErrorGuaranteed,alloc::string::String,ref$<str$> >
             at .\compiler\rustc_errors\src\diagnostic.rs:926
   4: rustc_hir_analysis::errors::wrong_number_of_generic_args::impl$0::suggest_removing_args_or_generics::closure$1<rustc_span::ErrorGuaranteed>
             at .\compiler\rustc_hir_analysis\src\errors\wrong_number_of_generic_args.rs:1007
   5: rustc_hir_analysis::errors::wrong_number_of_generic_args::impl$1::into_diag<rustc_span::ErrorGuaranteed>
             at .\compiler\rustc_hir_analysis\src\errors\wrong_number_of_generic_args.rs:1149
   6: rustc_errors::DiagCtxtHandle::create_err<rustc_hir_analysis::errors::wrong_number_of_generic_args::WrongNumberOfGenericArgs>
             at .\compiler\rustc_errors\src\lib.rs:1023
   7: rustc_hir_analysis::hir_ty_lowering::generics::check_generic_arg_count::closure$2::closure$6
             at .\compiler\rustc_hir_analysis\src\hir_ty_lowering\generics.rs:577
   8: core::option::Option::unwrap_or_else
             at /rustc/ad726b5063362ec9897ef3d67452fc5606ee70fa/library\core\src\option.rs:1067
   9: rustc_hir_analysis::hir_ty_lowering::generics::check_generic_arg_count::closure$2
             at .\compiler\rustc_hir_analysis\src\hir_ty_lowering\generics.rs:575
  10: rustc_hir_analysis::hir_ty_lowering::generics::check_generic_arg_count
             at .\compiler\rustc_hir_analysis\src\hir_ty_lowering\generics.rs:605
  11: rustc_hir_analysis::hir_ty_lowering::generics::check_generic_arg_count_for_call
             at .\compiler\rustc_hir_analysis\src\hir_ty_lowering\generics.rs:399
  12: rustc_hir_typeck::fn_ctxt::FnCtxt::instantiate_value_path
             at .\compiler\rustc_hir_typeck\src\fn_ctxt\_impl.rs:1100
  13: rustc_hir_typeck::fn_ctxt::FnCtxt::check_expr_path
             at .\compiler\rustc_hir_typeck\src\expr.rs:623
  14: rustc_hir_typeck::expr::impl$0::check_expr_with_expectation_and_args::closure$0
             at .\compiler\rustc_hir_typeck\src\expr.rs:290
  15: stacker::maybe_grow
             at C:\Users\user\.cargo\registry\src\index.crates.io-1949cf8c6b5b557f\stacker-0.1.21\src\lib.rs:57
  16: rustc_data_structures::stack::ensure_sufficient_stack
             at .\compiler\rustc_data_structures\src\stack.rs:21
  17: rustc_hir_typeck::fn_ctxt::FnCtxt::check_expr_with_expectation_and_args
             at .\compiler\rustc_hir_typeck\src\expr.rs:286
  18: rustc_hir_typeck::fn_ctxt::FnCtxt::check_expr_call
             at .\compiler\rustc_hir_typeck\src\callee.rs:74
  19: rustc_hir_typeck::fn_ctxt::FnCtxt::check_expr_kind
             at .\compiler\rustc_hir_typeck\src\expr.rs:388
  20: rustc_hir_typeck::expr::impl$0::check_expr_with_expectation_and_args::closure$0
             at .\compiler\rustc_hir_typeck\src\expr.rs:291
  21: stacker::maybe_grow
             at C:\Users\user\.cargo\registry\src\index.crates.io-1949cf8c6b5b557f\stacker-0.1.21\src\lib.rs:57
  22: rustc_data_structures::stack::ensure_sufficient_stack
             at .\compiler\rustc_data_structures\src\stack.rs:21
  23: rustc_hir_typeck::fn_ctxt::FnCtxt::check_expr_with_expectation_and_args
             at .\compiler\rustc_hir_typeck\src\expr.rs:286
  24: rustc_hir_typeck::fn_ctxt::FnCtxt::check_expr_with_expectation
             at .\compiler\rustc_hir_typeck\src\expr.rs:231
  25: rustc_hir_typeck::fn_ctxt::checks::impl$0::check_expr_block::closure$0::closure$0
             at .\compiler\rustc_hir_typeck\src\fn_ctxt\checks.rs:1029
  26: core::option::Option::map
             at /rustc/ad726b5063362ec9897ef3d67452fc5606ee70fa/library\core\src\option.rs:1165
  27: rustc_hir_typeck::fn_ctxt::checks::impl$0::check_expr_block::closure$0
             at .\compiler\rustc_hir_typeck\src\fn_ctxt\checks.rs:1029
  28: rustc_hir_typeck::fn_ctxt::FnCtxt::with_breakable_ctxt<rustc_hir_typeck::fn_ctxt::checks::impl$0::check_expr_block::closure_env$0,tuple$<> >
             at .\compiler\rustc_hir_typeck\src\fn_ctxt\_impl.rs:1563
  29: rustc_hir_typeck::fn_ctxt::FnCtxt::check_expr_block
             at .\compiler\rustc_hir_typeck\src\fn_ctxt\checks.rs:1021
  30: rustc_hir_typeck::fn_ctxt::FnCtxt::check_expr_kind
             at .\compiler\rustc_hir_typeck\src\expr.rs:387
  31: rustc_hir_typeck::expr::impl$0::check_expr_with_expectation_and_args::closure$0
             at .\compiler\rustc_hir_typeck\src\expr.rs:291
  32: stacker::maybe_grow
             at C:\Users\user\.cargo\registry\src\index.crates.io-1949cf8c6b5b557f\stacker-0.1.21\src\lib.rs:57
  33: rustc_data_structures::stack::ensure_sufficient_stack
             at .\compiler\rustc_data_structures\src\stack.rs:21
  34: rustc_hir_typeck::fn_ctxt::FnCtxt::check_expr_with_expectation_and_args
             at .\compiler\rustc_hir_typeck\src\expr.rs:286
  35: rustc_hir_typeck::fn_ctxt::FnCtxt::check_expr_with_expectation
             at .\compiler\rustc_hir_typeck\src\expr.rs:231
  36: rustc_hir_typeck::fn_ctxt::FnCtxt::check_expr_with_hint
             at .\compiler\rustc_hir_typeck\src\expr.rs:186
  37: rustc_hir_typeck::fn_ctxt::FnCtxt::check_return_or_body_tail
             at .\compiler\rustc_hir_typeck\src\expr.rs:968
  38: rustc_hir_typeck::check::check_fn
             at .\compiler\rustc_hir_typeck\src\check.rs:130
  39: rustc_hir_typeck::typeck_with_inspect::closure$0
             at .\compiler\rustc_hir_typeck\src\lib.rs:176
  40: rustc_hir_typeck::typeck_with_inspect
             at .\compiler\rustc_hir_typeck\src\lib.rs:101
      [... omitted 13 frames ...]
  41: rustc_middle::query::inner::query_ensure_ok_or_done
             at .\compiler\rustc_middle\src\query\inner.rs:64
  42: rustc_middle::query::plumbing::TyCtxtEnsureOk::typeck
             at .\compiler\rustc_middle\src\query\plumbing.rs:414
  43: rustc_hir_analysis::check_crate::closure$2
             at .\compiler\rustc_hir_analysis\src\lib.rs:246
  44: rustc_middle::hir::map::impl$3::par_hir_body_owners::closure$0<rustc_hir_analysis::check_crate::closure_env$2>
             at .\compiler\rustc_middle\src\hir\map.rs:340
  45: rustc_data_structures::sync::parallel::par_for_each_in::closure$0::closure$1::closure$0
             at .\compiler\rustc_data_structures\src\sync\parallel.rs:158
  46: core::panic::unwind_safe::impl$25::call_once
             at /rustc/ad726b5063362ec9897ef3d67452fc5606ee70fa/library\core\src\panic\unwind_safe.rs:274
  47: std::panicking::catch_unwind::do_call
             at /rustc/ad726b5063362ec9897ef3d67452fc5606ee70fa/library\std\src\panicking.rs:581
  48: std::panicking::catch_unwind
             at /rustc/ad726b5063362ec9897ef3d67452fc5606ee70fa/library\std\src\panicking.rs:544
  49: std::panic::catch_unwind
             at /rustc/ad726b5063362ec9897ef3d67452fc5606ee70fa/library\std\src\panic.rs:359
  50: rustc_data_structures::sync::parallel::ParallelGuard::run
             at .\compiler\rustc_data_structures\src\sync\parallel.rs:23
  51: rustc_data_structures::sync::parallel::par_for_each_in::closure$0::closure$1
             at .\compiler\rustc_data_structures\src\sync\parallel.rs:158
  52: core::slice::iter::impl$171::for_each
             at /rustc/ad726b5063362ec9897ef3d67452fc5606ee70fa/library\core\src\slice\iter\macros.rs:301
  53: rustc_data_structures::sync::parallel::par_for_each_in::closure$0
             at .\compiler\rustc_data_structures\src\sync\parallel.rs:157
  54: rustc_data_structures::sync::parallel::parallel_guard
             at .\compiler\rustc_data_structures\src\sync\parallel.rs:39
  55: rustc_data_structures::sync::parallel::par_for_each_in<ref$<rustc_span::def_id::LocalDefId>,ref$<slice2$<rustc_span::def_id::LocalDefId> >,rustc_middle::hir::map::impl$3::par_hir_body_owners::closure_env$0<rustc_hir_analysis::check_crate::closure_env$2> >        
             at .\compiler\rustc_data_structures\src\sync\parallel.rs:152
  56: rustc_middle::ty::context::TyCtxt::par_hir_body_owners
             at .\compiler\rustc_middle\src\hir\map.rs:340
  57: rustc_hir_analysis::check_crate
             at .\compiler\rustc_hir_analysis\src\lib.rs:221
  58: rustc_interface::passes::run_required_analyses
             at .\compiler\rustc_interface\src\passes.rs:1073
  59: rustc_interface::passes::analysis
             at .\compiler\rustc_interface\src\passes.rs:1125
      [... omitted 13 frames ...]
  60: rustc_middle::query::inner::query_ensure_ok_or_done
             at .\compiler\rustc_middle\src\query\inner.rs:64
  61: rustc_middle::query::plumbing::TyCtxtEnsureOk::analysis
             at .\compiler\rustc_middle\src\query\plumbing.rs:414
  62: rustc_driver_impl::run_compiler::closure$0::closure$2
             at .\compiler\rustc_driver_impl\src\lib.rs:325
  63: rustc_interface::passes::create_and_enter_global_ctxt::closure$2
             at .\compiler\rustc_interface\src\passes.rs:1015
  64: rustc_middle::ty::context::impl$19::enter::closure$1
             at .\compiler\rustc_middle\src\ty\context.rs:859
  65: rustc_middle::ty::context::tls::enter_context::closure$0
             at .\compiler\rustc_middle\src\ty\context\tls.rs:56
  66: std::thread::local::LocalKey::try_with
             at /rustc/ad726b5063362ec9897ef3d67452fc5606ee70fa/library\std\src\thread\local.rs:513
  67: std::thread::local::LocalKey::with<core::cell::Cell<ptr_const$<tuple$<> > >,rustc_middle::ty::context::tls::enter_context::closure_env$0<rustc_middle::ty::context::impl$19::enter::closure_env$1<rustc_interface::passes::create_and_enter_global_ctxt::closure       
             at /rustc/ad726b5063362ec9897ef3d67452fc5606ee70fa/library\std\src\thread\local.rs:477
  68: rustc_middle::ty::context::tls::enter_context
             at .\compiler\rustc_middle\src\ty\context\tls.rs:53
  69: rustc_middle::ty::context::GlobalCtxt::enter
             at .\compiler\rustc_middle\src\ty\context.rs:859
  70: rustc_middle::ty::context::TyCtxt::create_global_ctxt<enum2$<core::option::Option<rustc_interface::queries::Linker> >,rustc_interface::passes::create_and_enter_global_ctxt::closure_env$2<enum2$<core::option::Option<rustc_interface::queries::Linker> >,rustc       
             at .\compiler\rustc_middle\src\ty\context.rs:1068
  71: rustc_interface::passes::create_and_enter_global_ctxt<enum2$<core::option::Option<rustc_interface::queries::Linker> >,rustc_driver_impl::run_compiler::closure$0::closure_env$2>
             at .\compiler\rustc_interface\src\passes.rs:982
  72: rustc_driver_impl::run_compiler::closure$0
             at .\compiler\rustc_driver_impl\src\lib.rs:298
  73: rustc_interface::interface::run_compiler::closure$1::closure$0
             at .\compiler\rustc_interface\src\interface.rs:500
  74: core::panic::unwind_safe::impl$25::call_once
             at /rustc/ad726b5063362ec9897ef3d67452fc5606ee70fa/library\core\src\panic\unwind_safe.rs:274
  75: std::panicking::catch_unwind::do_call
             at /rustc/ad726b5063362ec9897ef3d67452fc5606ee70fa/library\std\src\panicking.rs:581
  76: std::panicking::catch_unwind
             at /rustc/ad726b5063362ec9897ef3d67452fc5606ee70fa/library\std\src\panicking.rs:544
  77: std::panic::catch_unwind
             at /rustc/ad726b5063362ec9897ef3d67452fc5606ee70fa/library\std\src\panic.rs:359
  78: rustc_interface::interface::run_compiler::closure$1
             at .\compiler\rustc_interface\src\interface.rs:500
  79: rustc_interface::util::run_in_thread_pool_with_globals::closure$0
             at .\compiler\rustc_interface\src\util.rs:204
  80: rustc_interface::util::run_in_thread_with_globals::closure$0::closure$0::closure$0
             at .\compiler\rustc_interface\src\util.rs:159
  81: scoped_tls::ScopedKey::set<rustc_span::SessionGlobals,rustc_interface::util::run_in_thread_with_globals::closure$0::closure$0::closure_env$0<rustc_interface::util::run_in_thread_pool_with_globals::closure_env$0<rustc_interface::interface::run_compiler::clo       
             at C:\Users\user\.cargo\registry\src\index.crates.io-1949cf8c6b5b557f\scoped-tls-1.0.1\src\lib.rs:137
  82: rustc_span::create_session_globals_then<tuple$<>,rustc_interface::util::run_in_thread_with_globals::closure$0::closure$0::closure_env$0<rustc_interface::util::run_in_thread_pool_with_globals::closure_env$0<rustc_interface::interface::run_compiler::closure_       
             at .\compiler\rustc_span\src\lib.rs:153
  83: rustc_interface::util::run_in_thread_with_globals::closure$0::closure$0
             at .\compiler\rustc_interface\src\util.rs:155
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Mar 16, 2026
| ^^^
| |
| expected at most 2 generic arguments
| help: remove the unnecessary generic argument
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new span for this help is a regression. It no longer points to any generic argument anymore.

#![allow(incomplete_features)]

impl X {
//~^ ERROR: cannot find type `X` in this scope
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: an off-topic error, if it can removed by adding struct X; without harming the reproduction, then it's better to do it.

@petrochenkov
Copy link
Contributor

petrochenkov commented Mar 16, 2026

This is all pretty awful.
A number of other def ids / def paths are also created during AST lowering, but they don't require keeping per-node disambiguator tables around. Why is delegation so special in this regard?

What exactly definitions create collisions like this? Generic parameters?

As I understand the small per-node disambiguator tables partially duplicate content from the big table.
What prevents us from looking up the current disambiguators in the big table instead?

@petrochenkov petrochenkov added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 16, 2026
@aerooneqq
Copy link
Contributor Author

aerooneqq commented Mar 17, 2026

A number of other def ids / def paths are also created during AST lowering, but they don't require keeping per-node disambiguator tables around. Why is delegation so special in this regard?

We generate new DefIds for each delegation generic param, as creation of DefId requires a name, we want to use names that are defined in a function being reused, for example for proper error messages. It is possible to create DefIds with, for example, sym::dummy name, but in this case error messages will be terrible. If we prefer to use names of generic params of the delegee, then we use names that are created by user, so they can clash with already used names in delegation.

The difference between delegation and other DefIds creation is that latter don't use user-defined names, as other cases of DefIds creation are DefPathData::DesugaredAnonymousLifetime which uses Some(kw::UnderscoreLifetime) as name and DefPathData::LateAnonConst which uses None as name.

Maybe we can create our own DefPathData variant and use it during creation of DefIds, but it still will need to hold the name of the param, as it is required for example in hir_ty_param_name. I am not sure that this is a correct option.

What exactly definitions create collisions like this? Generic parameters?

From our side yes, when we create generic params, their def path data can be either DefPathData::TypeNs(symbol) for DefKind::TyParam, DefPathData::ValueNs(symbol) for DefKind::ConstParam or DefPathData::LifetimeNs(symbol) for DefKind::LifetimeParam, so this def path data can have a collision with anything that have same def path created in scope of delegation LocalDefId during resolve stage.

What prevents us from looking up the current disambiguators in the big table instead?

Big table which is Resolver::disambiguator is not passed anywhere, as DefIds which are created during AST -> HIR lowering before this PR do not need it. Passing the whole disambiguator to lowering stage seems to be not the best option as it contains many unneeded information and it is not passed from Resolver to ResolverAstLowering now.

Internally DisambiguatorState uses UnordMap<(LocalDefId, DefPathData), u32> which prevents from fast lookups by LocalDefId only and changing it (for example to UnordMap<LocalDefId, UnordMap<DefPathData, u32>>) seemed to be out of scope of this PR.

Given this underlying structure I also didn't want to iterate over all map to find delegation-related key-values, and I also didn't want to expose the possibility of traversal of whole DisambiguatorState.

This left me with an option that I implemented in this PR: for each delegation we create additional disambiguator which follows the main one and then stored in delegation_infos map, for this purpose I exposed next operation on DisambiguatorState, which is also not that good, beacuase it was inlined in create_def function, thus only create_def could update it, but in the given conditions I think it is more-or-less OK solution.

@aerooneqq
Copy link
Contributor Author

@rustbot ready

To get feedback on the comments.

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 17, 2026
@rust-bors

This comment has been minimized.

@aerooneqq aerooneqq force-pushed the def-path-hash-collision branch from fbb4d60 to b9ef605 Compare March 19, 2026 09:51
@rustbot
Copy link
Collaborator

rustbot commented Mar 19, 2026

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

@rust-log-analyzer

This comment has been minimized.

@petrochenkov
Copy link
Contributor

petrochenkov commented Mar 19, 2026

This PR may be relevant:

It solved the collision problem by adding new DefPathData variants.

@petrochenkov
Copy link
Contributor

Blocked on #154049.
Can try the approach with new DefPathData variants in the meantime.
@rustbot blocked

@rustbot rustbot added S-blocked Status: Blocked on something else such as an RFC or other implementation work. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 19, 2026
@aerooneqq aerooneqq force-pushed the def-path-hash-collision branch from 5c0e8e9 to ef583eb Compare March 20, 2026 13:17
@rustbot
Copy link
Collaborator

rustbot commented Mar 20, 2026

Some changes occurred in compiler/rustc_sanitizers

cc @rcvalle

@rustbot rustbot added the PG-exploit-mitigations Project group: Exploit mitigations label Mar 20, 2026
@aerooneqq aerooneqq changed the title Fix delegation def path hash collision, fix ICE connected to dummy spans Fix delegation def path hash collision Mar 20, 2026
@aerooneqq aerooneqq marked this pull request as draft March 20, 2026 14:31
@rustbot rustbot added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Mar 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

PG-exploit-mitigations Project group: Exploit mitigations S-blocked Status: Blocked on something else such as an RFC or other implementation work. S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[ICE]: found DefPathHash collision between DefPath ICE: delegation: DefId::expect_local DefId(..) isn't local

5 participants