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

Disassembly displays old source (caching issue?) and search is broken #577

Closed
GitMensch opened this issue Dec 20, 2023 · 8 comments · Fixed by #595 or #599
Closed

Disassembly displays old source (caching issue?) and search is broken #577

GitMensch opened this issue Dec 20, 2023 · 8 comments · Fixed by #595 or #599
Labels

Comments

@GitMensch
Copy link
Contributor

Describe the bug
Use of Disassembly shows partially (and not always) the source of a previous disassembly.
Furthermore the search function seems to be totally broken.

@lievenhey I guess you can reproduce and want to have a look at this. Search definitely worked in older appimages and "caching atifacts" also seem to be a new thing.

@milianw
Copy link
Member

milianw commented Jan 3, 2024

how do you trigger this? can you give more detailed information please?

@GitMensch
Copy link
Contributor Author

GitMensch commented Jan 15, 2024

just retested with today's appimage

  • open disassembly of a bigger function in source1
  • open disassembly of a function in source2

result: you mostly see the source of source1, instead of source2, but the title is set to and the syntax language is used according to source2 and the number of lines is limited to the lines of the function in source2

@lievenhey
Copy link
Contributor

lievenhey commented Jan 15, 2024

Just to make sure, you are using the correct version of your executable? Disassembler works with addresses not and dwarf information. In theory a compiler can generate two different binaries for the same source code. Depending if he reorders the binary or not (used in obfuscated code to make leaked debug infos unusable).

I sometimes encounter a similar problem after a compiler change / update.

@GitMensch
Copy link
Contributor Author

Sometimes the binaries are out of date, often just recompiled. If there are differences then the counters are off by some lines (or plain wrong when the code around it changed).

But this is about totally showing the wrong source, and this is "new" and happens since the last big Disassembly overhaul.

@milianw
Copy link
Member

milianw commented Jan 15, 2024

also: for the situation you describe @lievenhey we should ideally catch it and noisily show a warning to the user. we can detect it by comparing buildids.

@GitMensch
Copy link
Contributor Author

I guess you cannot reproduce that with the appimage?
Is that related to QT_QPA_PLATFORM as in #339?

Do we need something like an explicit update call to also change the content of the lines?

lievenhey added a commit that referenced this issue Jan 16, 2024
The search functions only works on a limited range, but the model
contained the complete source code. This patch runs the search function
on only a subset of that code.
fixes: #577
lievenhey added a commit that referenced this issue Jan 22, 2024
The search functions only works on a limited range, but the model
contained the complete source code. This patch runs the search function
on only a subset of that code.
fixes: #577
lievenhey added a commit that referenced this issue Jan 22, 2024
The search functions only works on a limited range, but the model
contained the complete source code. This patch runs the search function
on only a subset of that code.
fixes: #577
@lievenhey
Copy link
Contributor

Ah, found it (actually @milianw but in another context). In HighlightedText::setText the QVector is reset correctly. This causes the weird behaviour you are seeing.

lievenhey added a commit that referenced this issue Jan 22, 2024
The vectors containing the source code are not reset correctly. This
results one some old code being shown. This patch resets them correctly.

fixes: #577
milianw added a commit that referenced this issue Jan 23, 2024
Don't always append data, resize-to-fit and then transform.
Sadly, QStringList has no resize, so we need to copy+for_each
there instead.

Fixes: #577
milianw added a commit that referenced this issue Jan 24, 2024
Don't always append data, resize-to-fit and then transform.
Sadly, QStringList has no resize, so we need to copy+for_each
there instead.

Fixes: #577
@milianw
Copy link
Member

milianw commented Jan 26, 2024

@GitMensch wrote:

At least with the new approach-the problem persists.
I can now reproduce: disassembly shown, then showing a different disassembly - the code pane was adjusted to show other code, but the code lines shown mismatches the cycles and the disassembly.
If I restart hotspot and show the disassembly of that other function "directly" then there's a perfect match.

I suggest to reopen the originating issue....

@milianw milianw reopened this Jan 26, 2024
lievenhey added a commit that referenced this issue Apr 15, 2024
The search functions only works on a limited range, but the model
contained the complete source code. This patch runs the search function
on only a subset of that code.
fixes: #577
lievenhey added a commit that referenced this issue Apr 23, 2024
The search functions only works on a limited range, but the model
contained the complete source code. This patch runs the search function
on only a subset of that code.
fixes: #577
lievenhey added a commit to lievenhey/hotspot that referenced this issue Apr 26, 2024
The search functions only works on a limited range, but the model
contained the complete source code. This patch runs the search function
on only a subset of that code.
fixes: KDAB#577
lievenhey added a commit to lievenhey/hotspot that referenced this issue Apr 26, 2024
The search functions only works on a limited range, but the model
contained the complete source code. This patch runs the search function
on only a subset of that code.
fixes: KDAB#577
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants