| Summary: | wine's unit tests enter a signal delivery loop under valgrind on armv7l when SIGSEGV is used | ||
|---|---|---|---|
| Product: | [Developer tools] valgrind | Reporter: | Austin English <austinenglish> |
| Component: | general | Assignee: | Julian Seward <jseward> |
| Status: | RESOLVED FIXED | ||
| Severity: | normal | ||
| Priority: | NOR | ||
| Version First Reported In: | 3.14 SVN | ||
| Target Milestone: | --- | ||
| Platform: | Compiled Sources | ||
| OS: | Linux | ||
| Latest Commit: | Version Fixed/Implemented In: | ||
| Sentry Crash Report: | |||
| Attachments: |
A possible fix
valgrind output |
||
|
Description
Austin English
2018-06-29 07:47:05 UTC
Using valgrind-3.14.0.GIT-90daa486e8-20180620 4:49:22 AM austin_laptop if someone with decent understanding of kernel32/heap.c has a few minutes, I'd appreciate if someone could help me answer Julian's questions from https://bugs.kde.org/show_bug.cgi?id=395991 (I'm happy to forward your answers if you don't have a KDE bz account) 5:01:50 AM _Marcus_ austin_laptop: so if we take a segfault, the handler would run into the expcetion chain 5:02:02 AM _Marcus_ austin_laptop: but in general, globalalloc should not fault in my opinion 5:02:54 AM _Marcus_ SetLastError(MAGIC_DEAD); 5:02:54 AM _Marcus_ hsecond = GlobalFree(LongToHandle(0xdeadbeef)); /* bogus handle */ 5:03:01 AM _Marcus_ hmm, but this code does 5:03:07 AM austin_laptop thanks _Marcus_. Yeah, from a quick read of the code, I didn't see why it would segfault, but wasn't completely sure 5:03:29 AM _Marcus_ so basically we are running into the TRY / __EXCEPT_PAGE_FAULT block in GlobalFree 5:03:37 AM _Marcus_ the testcase causes it to fault 5:03:47 AM _Marcus_ but this is our generic exceptipon handling 5:04:10 AM austin_laptop ok 5:06:40 AM _Marcus_ something in the arm signal handling is not fully correct I would think :/ 5:07:11 AM austin_laptop yeah, that's likely 5:07:36 AM austin_laptop this is armv7l rather than arm64 (and at least on VG side, is not as complete) 5:07:37 AM _Marcus_ winedebug +heap should print the ERR("invalid handle %p\n", hmem); 5:07:48 AM _Marcus_ from GlobalFree, if it does not ... we are not getting there for some reason Note: I ran with +heap, but do not get that message, so something may be broke in wine's armv7l exception handling as well. Created attachment 115037 [details]
A possible fix
Austin, I think this might fix it. But I can only test it on
my tiny test case. Can you cause it to be tested on Wine?
Fixed (I think!), 43115c8058052fdb0e1c5dd76ec286519e2bfe78. Created attachment 115058 [details]
valgrind output
While the situation has changed, it still differs from what I see on amd64. Log attached.
(In reply to Austin English from comment #5) > While the situation has changed, it still differs from what I see on amd64. > Log attached. Hmm. If I had to guess, I'd say that the signal frame that V creates doesn't have enough details in it to keep the Wine signal handler happy. Austin, can you show me the source of the signal handler involved? (In reply to Julian Seward from comment #6) > (In reply to Austin English from comment #5) > > While the situation has changed, it still differs from what I see on amd64. > > Log attached. > > Hmm. If I had to guess, I'd say that the signal frame that V creates > doesn't have enough details in it to keep the Wine signal handler happy. > > Austin, can you show me the source of the signal handler involved? Yeah, it's here: https://source.winehq.org/git/wine.git/blob/bfad5527acacbdbac623da57413a60c532218423:/dlls/kernel32/heap.c#l741 FYI wine's development IRC channel is #winehackers if you want to get more info (I think you already knew that, but just in case). (In reply to Austin English from comment #7) > > Austin, can you show me the source of the signal handler involved? > > Yeah, it's here: > https://source.winehq.org/git/wine.git/blob/ > bfad5527acacbdbac623da57413a60c532218423:/dlls/kernel32/heap.c#l741 Hmm. While that is probably related, it is not a signal handler, and it also isn't the source of the messages you showed in comment 6. The signal handling magic bits will somehow be hidden in the macros __TRY (line 747), __EXCEPT_PAGE_FAULT (line 785) and __ENDTRY (line 791). Can you show the definitions of those, so we can find the actual handler? A signal handler will have signature "void handler(int signo)" or (more likely) "void handler(int signo, siginfo_t* siginfo, void* context)". (In reply to Julian Seward from comment #8) > (In reply to Austin English from comment #7) > > > Austin, can you show me the source of the signal handler involved? > > > > Yeah, it's here: > > https://source.winehq.org/git/wine.git/blob/ > > bfad5527acacbdbac623da57413a60c532218423:/dlls/kernel32/heap.c#l741 > > Hmm. While that is probably related, it is not a signal handler, > and it also isn't the source of the messages you showed in comment 6. > > The signal handling magic bits will somehow be hidden in the macros > __TRY (line 747), __EXCEPT_PAGE_FAULT (line 785) and __ENDTRY (line 791). > Can you show the definitions of those, so we can find the actual handler? > > A signal handler will have signature "void handler(int signo)" or > (more likely) "void handler(int signo, siginfo_t* siginfo, void* context)". Thanks for the pointer / sorry for the mixup. I believe what you're looking for is in: https://source.winehq.org/git/wine.git/blob/bfad5527acacbdbac623da57413a60c532218423:/include/wine/exception.h#l101 |