Howdy all,
Several months ago, a patch from Sebastian: commit 44fbc018eda12bdee5c2c1e2e40dbdc6a81b27fd Author: Sebastian Lackner sebastian@fds-team.de Date: Thu Feb 12 11:09:34 2015 +0100
winebuild: Do not access memory below ESP when restoring thread contexts.
Based on a patch by John Reiser.
for https://bugs.winehq.org/show_bug.cgi?id=14367
Caused problems for running Wine under Valgrind: https://bugs.kde.org/show_bug.cgi?id=344139
There are patches floating around for both Wine and Valgrind (see previous link), but from what I've seen so far, both projects aren't a fan of the current patches.
Given that Wine developers like using Valgrind to check for problems, and I know Julian would like to use Wine with Valgrind, I feel like some solution to this problem can be found. Since my efforts at trying to run messages between the two projects hasn't solved it yet, I figured an email thread would work better ;).
On Dec 5, 2016 2:23 AM, "Austin English" austinenglish@gmail.com wrote:
Howdy all,
Several months ago, a patch from Sebastian: commit 44fbc018eda12bdee5c2c1e2e40dbdc6a81b27fd Author: Sebastian Lackner sebastian@fds-team.de Date: Thu Feb 12 11:09:34 2015 +0100
winebuild: Do not access memory below ESP when restoring thread contexts.
Based on a patch by John Reiser.
for https://bugs.winehq.org/show_bug.cgi?id=14367
Caused problems for running Wine under Valgrind: https://bugs.kde.org/show_bug.cgi?id=344139
There are patches floating around for both Wine and Valgrind (see previous link), but from what I've seen so far, both projects aren't a fan of the current patches.
Given that Wine developers like using Valgrind to check for problems, and I know Julian would like to use Wine with Valgrind, I feel like some solution to this problem can be found. Since my efforts at trying to run messages between the two projects hasn't solved it yet, I figured an email thread would work better ;).
-- -Austin GPG: 14FB D7EA A041 937B
Ping.
-- -Austin GPG: 14FB D7EA A041 937B
On 16.01.2017 04:57, Austin English wrote:
On Dec 5, 2016 2:23 AM, "Austin English" austinenglish@gmail.com wrote:
Howdy all,
Several months ago, a patch from Sebastian: commit 44fbc018eda12bdee5c2c1e2e40dbdc6a81b27fd Author: Sebastian Lackner sebastian@fds-team.de Date: Thu Feb 12 11:09:34 2015 +0100
winebuild: Do not access memory below ESP when restoring threadcontexts.
Based on a patch by John Reiser.for https://bugs.winehq.org/show_bug.cgi?id=14367
Caused problems for running Wine under Valgrind: https://bugs.kde.org/show_bug.cgi?id=344139
There are patches floating around for both Wine and Valgrind (see previous link), but from what I've seen so far, both projects aren't a fan of the current patches.
Given that Wine developers like using Valgrind to check for problems, and I know Julian would like to use Wine with Valgrind, I feel like some solution to this problem can be found. Since my efforts at trying to run messages between the two projects hasn't solved it yet, I figured an email thread would work better ;).
-- -Austin GPG: 14FB D7EA A041 937B
Ping.
-- -Austin GPG: 14FB D7EA A041 937B
Hi all,
I'm not sure what the best way is to get this finally fixed. If this bug is considered very critical for Wine, I can certainly write a patch to replace this code with a slower version, which uses a different segment registers. The SIGILL workaround is definitely too hacky for Wine sources imho.
When I wrote the Wine patch back then, my goal was to keep this code as efficient as possible. Wine uses this specific piece of code quite a lot, so even a couple of instructions could make a difference for applications with lots of exceptions or while single-stepping.
It is important to keep in mind that Wine isn't doing anything special here - those are all valid x86 assembly instructions. While we can certainly workaround this bug on the Wine side, implementing this missing feature in Valgrind would be the better option and also help other applications.
The Valgrind bugtracker also contains three patches I proposed back then. I was hoping that this would speed up the process of getting this fixed, but unfortunately it didn't help much. At many places Valgrind does not yet handle signal registers correctly and doesn't complain - except here, which is really unfortunate because even ignoring would be sufficient to make Wine happy.
@Julian Seward: Could you please revisit this bug, and check if any of the proposed ideas is acceptable for Valgrind?
Regards, Sebastian
Hi Sebastian,
The Valgrind bugtracker also contains three patches I proposed back then. I was hoping that this would speed up the process of getting this fixed, but unfortunately it didn't help much. At many places Valgrind does not yet handle signal registers correctly and doesn't complain - except here, which is really unfortunate because even ignoring would be sufficient to make Wine happy.
Sorry this fell through the cracks. One underlying problem is that, some time back in 2003, I learnt just enough about x86 segment stuff to hack up what's currently in Valgrind, and then promptly forgot about it. So I'm not in much of a position to make an informed judgement now.
That said .. I would be OK with a partial fix which improves Valgrind's SS handling enough to make Wine work, so long as it doesn't create a situation where other cases are silently handled incorrectly. That is, if the fix only moves forwards on the correctness scale, and doesn't introduce any regressions.
On rereading https://bugs.kde.org/show_bug.cgi?id=344139#c1 I see that you have a candidate patch (http://ix.io/gKt, "Correctly handle the SS prefix when explicitly specified.") which appears to have the abovementioned properties. Is that correct?
J
On 19.01.2017 20:30, Julian Seward wrote:
Hi Sebastian,
The Valgrind bugtracker also contains three patches I proposed back then. I was hoping that this would speed up the process of getting this fixed, but unfortunately it didn't help much. At many places Valgrind does not yet handle signal registers correctly and doesn't complain - except here, which is really unfortunate because even ignoring would be sufficient to make Wine happy.
Sorry this fell through the cracks. One underlying problem is that, some time back in 2003, I learnt just enough about x86 segment stuff to hack up what's currently in Valgrind, and then promptly forgot about it. So I'm not in much of a position to make an informed judgement now.
That said .. I would be OK with a partial fix which improves Valgrind's SS handling enough to make Wine work, so long as it doesn't create a situation where other cases are silently handled incorrectly. That is, if the fix only moves forwards on the correctness scale, and doesn't introduce any regressions.
On rereading https://bugs.kde.org/show_bug.cgi?id=344139#c1 I see that you have a candidate patch (http://ix.io/gKt, "Correctly handle the SS prefix when explicitly specified.") which appears to have the abovementioned properties. Is that correct?
J
As discussed on IRC, this direction probably makes most sense. In contrast to the other approaches, no special handling for the SS segment is required - nevertheless, we have to initialize some GDT entries to make this work.
I have attached a new series which is hopefully less hacky:
https://bugs.kde.org/show_bug.cgi?id=344139#c3 (*) https://bugs.kde.org/show_bug.cgi?id=344139#c4
(*) On some systems the segment for DS and SS is the same, but initializing twice shouldn't hurt.
For me it fixes the Wine tests (@Austin: Could you confirm?). Please let me know if you have any other suggestions for improvement (either here or on the bugtracker). Thanks!
Best regards, Sebastian
On Thu, Jan 19, 2017 at 9:40 PM, Sebastian Lackner sebastian@fds-team.de wrote:
On 19.01.2017 20:30, Julian Seward wrote:
Hi Sebastian,
The Valgrind bugtracker also contains three patches I proposed back then. I was hoping that this would speed up the process of getting this fixed, but unfortunately it didn't help much. At many places Valgrind does not yet handle signal registers correctly and doesn't complain - except here, which is really unfortunate because even ignoring would be sufficient to make Wine happy.
Sorry this fell through the cracks. One underlying problem is that, some time back in 2003, I learnt just enough about x86 segment stuff to hack up what's currently in Valgrind, and then promptly forgot about it. So I'm not in much of a position to make an informed judgement now.
That said .. I would be OK with a partial fix which improves Valgrind's SS handling enough to make Wine work, so long as it doesn't create a situation where other cases are silently handled incorrectly. That is, if the fix only moves forwards on the correctness scale, and doesn't introduce any regressions.
On rereading https://bugs.kde.org/show_bug.cgi?id=344139#c1 I see that you have a candidate patch (http://ix.io/gKt, "Correctly handle the SS prefix when explicitly specified.") which appears to have the abovementioned properties. Is that correct?
J
As discussed on IRC, this direction probably makes most sense. In contrast to the other approaches, no special handling for the SS segment is required - nevertheless, we have to initialize some GDT entries to make this work.
I have attached a new series which is hopefully less hacky:
https://bugs.kde.org/show_bug.cgi?id=344139#c3 (*) https://bugs.kde.org/show_bug.cgi?id=344139#c4
(*) On some systems the segment for DS and SS is the same, but initializing twice shouldn't hurt.
For me it fixes the Wine tests (@Austin: Could you confirm?). Please let me know if you have any other suggestions for improvement (either here or on the bugtracker). Thanks!
Best regards, Sebastian
With those two patches on top of r16203, I can run dlls/advapi32/tests/service.c again without the trap 0 occurring
My wine/valgrind branch has one other patch, for https://bugs.winehq.org/show_bug.cgi?id=39097. So I also tried reapplying 4a1629c4117fda9eca63b6f56ea45771dc9734ac, and rerunning dlls/comdlg32/tests/filedlg.c with valgrind, but it still hangs.
So the patches are good for me, but there's still at least one wine and valgrind regression that needs to be investigated.
On 20/01/17 09:12, Austin English wrote:
I have attached a new series which is hopefully less hacky:
https://bugs.kde.org/show_bug.cgi?id=344139#c3 (*) https://bugs.kde.org/show_bug.cgi?id=344139#c4
Landed, r16204, r3299.
My wine/valgrind branch has one other patch, for https://bugs.winehq.org/show_bug.cgi?id=39097. So I also tried reapplying 4a1629c4117fda9eca63b6f56ea45771dc9734ac, and rerunning dlls/comdlg32/tests/filedlg.c with valgrind, but it still hangs.
What's the Valgrind patch there? I can't see any patch attached to https://bugs.winehq.org/show_bug.cgi?id=39097.
J
On Fri, Jan 20, 2017 at 4:15 AM, Julian Seward jseward@acm.org wrote:
On 20/01/17 09:12, Austin English wrote:
I have attached a new series which is hopefully less hacky:
https://bugs.kde.org/show_bug.cgi?id=344139#c3 (*) https://bugs.kde.org/show_bug.cgi?id=344139#c4
Landed, r16204, r3299.
Great, thanks!
My wine/valgrind branch has one other patch, for https://bugs.winehq.org/show_bug.cgi?id=39097. So I also tried reapplying 4a1629c4117fda9eca63b6f56ea45771dc9734ac, and rerunning dlls/comdlg32/tests/filedlg.c with valgrind, but it still hangs.
What's the Valgrind patch there? I can't see any patch attached to https://bugs.winehq.org/show_bug.cgi?id=39097.
Sorry if I was unclear, there's not one. It was a wine change that causes problems for me running under valgrind, but apparently not others.
-
Austin English -
Julian Seward -
Sebastian Lackner