Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Function name auto-insertion does not work #1987

Open
Mis-hat opened this Issue Jul 15, 2018 · 11 comments

Comments

Projects
None yet
6 participants
@Mis-hat
Copy link

Mis-hat commented Jul 15, 2018

Function name auto-insertion in latest releases does not work (hotkey CTRL+G in debugger).

snapshot_2018-07-14_20-58:
snapshot_2018-07-14_20-58
No drop-down list!

But in snapshot_2018-06-19_19-02 it's work:
snapshot_2018-06-19_19-02

I have tried on Windows 7 x32, x64 operating systems, and Windows 8.1 x64.
It's a new feature or a bug?

I have no desire to keep in mind all WinAPI...

@Mis-hat

This comment has been minimized.

Copy link
Author

Mis-hat commented Jul 16, 2018

My settings:
settings

But it still does not work, regardless of whether a check is ticked or not.

@mrexodia mrexodia added the bug label Jul 16, 2018

@mrexodia

This comment has been minimized.

Copy link
Member

mrexodia commented Jul 16, 2018

This is likely because of the new export table parsing.

@IssuehuntBot

This comment has been minimized.

Copy link

IssuehuntBot commented Jul 20, 2018

@0maxxam0 funded this issue with $7. Visit this issue on Issuehunt

@Mattiwatti

This comment has been minimized.

Copy link
Contributor

Mattiwatti commented Jul 31, 2018

The behaviour I observe is this:

  • If you have the PDB for user32.dll, everything works as before.
  • If you don't have the PDB for user32.dll, some entries do get filled in the dropdown list, but not all. E.g. I get MessageBoxW but not MessageBoxA.
  • Even if you don't have the PDB, if you correctly type the full name (so MessageBoxA and not MessageBox like in the screenshot), the dialog will accept the input even if it doesn't show the function in the dropdown.

So it doesn't seem like some fundamental issue with the export name parsing but to me looks like some minor bug with filling in the autocomplete dropdown list entries. I don't know where that happens though...

@mrexodia

This comment has been minimized.

Copy link
Member

mrexodia commented Jul 31, 2018

Yeah I forgot to add the exports as a source for the autocomplete is all. Will try to find some time tomorrow:)

@torusrxxx

This comment has been minimized.

Copy link
Member

torusrxxx commented Oct 10, 2018

Export and other "x64dbg-defined" exports ("OptionalHeader.EntryPoint") is not added to symbol cache, causing the issue. The solution is to add them regardless of whether PDB has been loaded or not.

@mrexodia

This comment has been minimized.

Copy link
Member

mrexodia commented Oct 10, 2018

Yes, I have a prepared cache for exports sorted by name, just have to query it when doing the autocomplete... currently my focus is stability, a new source view and swapping TitanEngine with GleeBug. There is slow progress, but this time things are actually working properly.

@torusrxxx if I don’t catch you on the chat, I found a buffer overflow when tracing “nop;fxsave [esp]”. I assume this is because fxsave has a memory operand bigger than the trace buffer? Another issue I found is when you have the bytes “90FFFF” (instruction->invalid instruction) and it will crash because IsNop requires a valid disassembly, I’m working on both, but if you have an easy fix I’m interested:)

@balintf

This comment has been minimized.

Copy link
Contributor

balintf commented Oct 30, 2018

I have worked on this issue a bit, and might have a PR ready by tomorrow. I can add the export names to the list in SymAutoComplete but there are a few open questions that we need to figure out.
Should it only add the exports to the list when there are no symbols loaded for the module, or should it add them always?
On my local build using VS 2017 _MessageBoxA@16 doesn't get undecorated to MessageBoxA, so adding the exports always is useful in my case.

If we are adding the exports always, should we try to handle ordering issues e.g for prefix 'A' eypoers Ax1, Ax2, ... will always come before Aa1, Aa2 symbols? This already happens more or less due to concatenating lists from different modules.

@torusrxxx

This comment has been minimized.

Copy link
Member

torusrxxx commented Oct 30, 2018

mrexodia's intent is to add these export names into the symbol cache, not just into SymAutoComplete. I would like to always add these exports, if there is not a same symbol at the same address already.

@mrexodia

This comment has been minimized.

Copy link
Member

mrexodia commented Oct 30, 2018

I already have stored all symbols+exports both sorted by rva and by name. Currently only the symbols are being used, the exports need a similar function to query autocomplete for a prefix.

Because the limit is 20 we can just sort the list in the gui before displaying it to not care about these problems.

In the long term my goal is to completely refactor the system and have proper autocompletion with addr,name pairs and a dialog like ollydbg2.

@balintf

This comment has been minimized.

Copy link
Contributor

balintf commented Oct 31, 2018

As far as I can see there is no simple way to truly merge the exports into the symbol cache with the current architecture. It would require either implementing a SymbolSourceBase derived class that is capable of merging different symbol sources, or some redesign of the symbol handling architecture. Both seem to be too big of a task and changeset for simply adding exports to the autocomplete list.

I agree with @mrexodia that this fix should be a relatively simple prefix search, similar to the one already implemented in SymbolSourceDIA::findSymbolsByPrefix, and that we should do a refactor later, making the autocomplete smarter in the process.

I already have a working implementation in my local repo, but it only uses linear search yet. To fully take advantage of the presorted vector<size_t> exportsByName, and use a binary search I might need to modify it to store const char*-s or name + index pairs, so that I can use std::lower_bound to search for a prefix that does not have an index in exports.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.