Skip to content

Conversation

@max-charlamb
Copy link
Member

@max-charlamb max-charlamb commented Oct 3, 2024

Tightens cdac_data friend relationships to only expose internals to exact specialization.

Previously classes exposed internals to all cdac_data specializations. By changing the friend declarations to the exact specialization which needs will access the internals, we can better verify where the internals are accessed and ensure cdac_data<T> accesses the correct class.

// Example:
class Module : public ModuleBase
{
  // previous, access given to all specializations
  template<typename T> friend struct ::cdac_data
  
  // new, only cdac_data<Module> can access private class members
  friend struct ::cdac_data<Module>;
}

Note there are two special cases.

  1. Some classes A are purposefully used as part of a different cdac_data<B> which may hold an instance of them. For example cdac_data<Thread> reads ThreadExceptionState offsets. In this case class A (ThreadExceptionState) declares friend to cdac_data<B> (cdac_data<Thread>).

  2. Given SList is a generic implementation of a singly linked list, it seems to make sense for all cdac_data specializations to be able to access it, therefore the declaration was not changed.

@dotnet-policy-service
Copy link
Contributor

Tagging subscribers to this area: @tommcdon
See info in area-owners.md if you want to be subscribed.

@max-charlamb
Copy link
Member Author

@dotnet-policy-service agree company="Microsoft"

Copy link
Member

@lambdageek lambdageek left a comment

Choose a reason for hiding this comment

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

Thanks!

@max-charlamb max-charlamb marked this pull request as ready for review October 4, 2024 15:23
Copy link
Member

@lambdageek lambdageek left a comment

Choose a reason for hiding this comment

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

One more round of bikeshedding this comment ;-)

max-charlamb and others added 4 commits October 4, 2024 11:27
Co-authored-by: Aleksey Kliger (λgeek) <[email protected]>
Co-authored-by: Aleksey Kliger (λgeek) <[email protected]>
Co-authored-by: Aleksey Kliger (λgeek) <[email protected]>
@max-charlamb max-charlamb merged commit fd4d0a9 into dotnet:main Oct 4, 2024
88 of 90 checks passed
sirntar pushed a commit to sirntar/runtime that referenced this pull request Oct 8, 2024
tighten cdac_data friend declarations

Previously classes exposed internals to all cdac_data specializations. By changing the
friend declarations to the exact specialization which needs will access the internals,
we can better verify where the internals are accessed and ensure cdac_data<T> accesses
the correct class.
@github-actions github-actions bot locked and limited conversation to collaborators Nov 4, 2024
@max-charlamb max-charlamb deleted the dev/maxcharlamb/friend-spec branch June 10, 2025 16:37
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants