Skip to content

Conversation

@quanzhuo
Copy link
Contributor

Cache register names

@quanzhuo
Copy link
Contributor Author

quanzhuo commented Jun 5, 2025

Sorry that two unrelated commits appear in the same PR.
The first commit is for caching register names to avoid requesting them every time.
The second commit removes the stopAtEntry property (only for attach requests), as the stopAtEntry property is meaningless for attach-type requests.
@WebFreak001 Hi, boss could you please take a while and look at this ?

@GitMensch
Copy link
Collaborator

stopAtEntry should still work and lead to "stop at attach" (but that may be automatic) and if set to an entry lead to a temporary breakpoint at the given function name, no? It does look like the code you've adjusted would do that...

// but for now we just retrieve them every time to keep it simple.
const names = await this.getRegisterNames();
if (!this.regNames) {
this.regNames = await this.getRegisterNames();
Copy link
Collaborator

Choose a reason for hiding this comment

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

the idea is good, but we need to store the regnames per inferior (you may switch between multiple ones via UI), not globally, as the inferiors can have different registers (example: server in 64 bit and client in 32 bit (or vice versa), attached to both)

Copy link
Collaborator

@GitMensch GitMensch left a comment

Choose a reason for hiding this comment

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

please check the scope of the caching and add a ChangeLog entry as well (one for each commit, but I'm not convinced yet that the second commit should be merged - maybe focus on the register caching here and discuss dropping the setting [which also needs a ChangeLog entry] separately)

@WebFreak001
Copy link
Owner

rest LGTM, just GitMensch's comment

@brownts
Copy link
Collaborator

brownts commented Jun 5, 2025

The second commit removes the stopAtEntry property (only for attach requests), as the stopAtEntry property is meaningless for attach-type requests.

Please don't remove this functionality, it is not meaningless. I've used this for many years and others have as well. Was there some problem you were trying to solve by removing this?

In the original PR thread (see here), I described how this works for an "attach" when used in combination with gdbserver where the application does not start running immediately. In that context it is valid to run the application to the entry point.

@quanzhuo
Copy link
Contributor Author

quanzhuo commented Jun 6, 2025

Close this PR first. When I'm ready, I'll submit a new one.
Thanks all.

@quanzhuo quanzhuo closed this Jun 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants