October 2012 - Wine-Devel - list.winehq.org

Help getting amd64 assembly patch into wine?

by Dan Kegel

As you might know from watching too many of my patches scroll by, I've been working on adding support for OpenMP to wine. (A handful of games, and a lot of serious apps, seem to use that api.) After getting it nicely organized and cleaned up to the point where it passed all the tests I could throw at it, I submitted it as http://source.winehq.org/patches/data/90524, Alexandre rejected the patch, saying it had multiple problems, but declining to provide any further hints. He suggested I find someone to work with to get it in... so here I am, hat in hand, asking for help. The patch adds support for OpenMP programs like this: // Compile with "cl test.c /openmp" #include <omp.h> #include <stdio.h> int main (int argc, char **argv) { #pragma omp parallel { printf("%d\n", omp_get_thread_num()); } } Later patches add support for other OpenMP constructs. My patch doesn't actually parallelize the app, but it does let it run rather than crash on unimplemented _vcomp_fork. Visual C will happily provide an assembly listing of this program showing how _vcomp_fork is called, so understanding that interface well enough to pass simple tests wasn't hard. The main challenge was figuring out how to get the variable list of arguments off the stack, and then put them back onto the stack when calling the provided function pointer. This bit of varargs hackery can't be done in pure C as far as I can tell, so I used assembly. I started programming in machine language long ago, so getting that working wasn't too hard once I realized that's what was needed. Getting it to pass the Alexandre test is another matter. Alexandre did give feedback on one earlier iteration of the patch, telling me I was re-inventing the wheel, so I tossed my old assembly and copied the helper-calling code nearly verbatim from call_method in oleaut32/typelib.c. Maybe he objects to not passing copies of the arguments in the floating point registers on amd64... but the arguments are always pointers, so that shouldn't be needed. I'm definitely not yet comfortable with the ASM_CFI stuff, so perhaps there's a problem there, but I did at least verify that injecting a fault into _test_vcomp_fork_worker5() still yielded a good stack trace on both x86 and amd64 in winedbg. There aren't many comments, and the ones I did add aren't the greatest, but I doubt that's the issue. Perhaps he objects to breaking up _vcomp_fork into three functions, but I couldn't see any other way to write it without making later patches like http://source.winehq.org/patches/data/90527 (and a future patch that adds actual multiple threads) put more logic than seems wise into assembly. So maybe it's none of the above. If someone can suggest what I might be missing, I'd appreciate it. Even just "It will break if..." or "Better compare how this was done in..." would be very helpful. Thanks!

Oct. 4, 2012

Re: shell32/tests: Write proper tests for CommandLineToArgvW(). (try2)

by Marvin

Hi, While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at http://testbot.winehq.org/JobDetails.pl?Key=22000 Your paranoid android. === WINEBUILD (build) === Patch failed to apply

Oct. 4, 2012

Re: shell32/tests: Write proper tests for CommandLineToArgvW(). (try2)

by Marvin

Hi, While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at http://testbot.winehq.org/JobDetails.pl?Key=21999 Your paranoid android. === WINEBUILD (build) === Patch failed to apply

Oct. 4, 2012

Re: shell32/tests: Simplify shlexec's test_argify() and test_lpFile_parsed() and avoid numeric literals.

by Marvin

Hi, While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at http://testbot.winehq.org/JobDetails.pl?Key=21998 Your paranoid android. === WINEBUILD (build) === Patch failed to apply

Oct. 4, 2012

Re: AppDB: Could someone please block this Spammer (UCE)

by Joerg Schiermeier

On Wednesday, October 3, 2012 at 17:13:21 Rosanne DiMesio wrote: > My Preferences screen has an option to "Disable global e-mail > notifications (only send for maintained apps)", but I'm not sure if > that's something only admins have. I (just) filed this under the bug #31880 in wines Bugzilla. -- Kindly regards Joerg Schiermeier Bielefeld/Germany

Oct. 3, 2012

Re: Overriding the check for ownership of a prefix

by Scott Ritchie

On 9/19/12 5:23 PM, Scott Ritchie wrote: > So, I believe I have a legitimate use case for ignoring this, and want > to know what sort of patch would go forward. > > > Imagine a distro package containing a Windows game in the form of a > read-only copy of an installed prefix (into, say, /opt). When the user > launches the app (via desktop file) this in turn launches a script that > does the following: > > 1) Makes a temporary folder > 2) Sets up a unionfs-fuse copy-on-write mount between ~/.appname and the > read-only packaged prefix in /opt, mounted in the temporary folder > 3) Runs the app with WINEPREFIX= the temp folder > 4) When the app is finished, unmounts the temp folder > > This all works quite fine: new (or modified) files within the prefix get > stored in ~/.appname, to be restored the next time the user runs the > app. Distinct users can run the app simultaneously, as they each have > their own prefix. Excess file-copying is avoided, as only the > user-modified files need to be stored in the home folders. > > There is one major snag, however: unionfs displays the owner as root > until the user has modified/copied it. This means Wine refuses to > launch as the user with the "root-owned" prefix. Simply commenting out > this part of the code makes Wine work fine, however I'd like to be able > to have a proper solution. > I have found a better solution: you can use standard FUSE options to mask the true owner such that it is always presented as the mounting user, even if the file is untouched.

Oct. 3, 2012

Re: AppDB: Could someone please block this Spammer (UCE)

by Joerg Schiermeier

Hello Rosanne, you wrote: RD> My Preferences screen has an option to "Disable global e-mail RD> notifications (only send for maintained apps)", but I'm not sure RD> if that's something only admins have. I think you're right: in my prefs I only see 'Send email notifications' with 'yes' and 'no'. Maybe a reason to file a 'bug/missing function in AppDB' in wines Bugzilla? (Or a patch ;) Will do it later to hold this point in mind. Thanks for your respond. -- Kindly regards Joerg Schiermeier Bielefeld/Germany

Oct. 3, 2012

Re: [PATCH] advapi32: avoid memory leaks (Coverity)

by Alexandre Julliard

Marcus Meissner <marcus(a)jet.franken.de> writes: > @@ -530,8 +530,10 @@ static void test_enum_providers(void) > /* alloc provider to half the size required > * cbName holds the size required */ > providerLen = cbName / 2; > - if (!(provider = LocalAlloc(LMEM_ZEROINIT, providerLen))) > + if (!(provider = LocalAlloc(LMEM_ZEROINIT, providerLen))) { > + LocalFree(pszProvName); > return; > + } Actually, checking for allocation failures in tests is not useful, it's not going to happen. -- Alexandre Julliard julliard(a)winehq.org

Oct. 3, 2012

Re: [PATCH] rpcrt4: wait_async_request: return error if we received an error

by Jacek Caban

On 10/03/12 14:57, Marcus Meissner wrote: > On Tue, Oct 02, 2012 at 01:19:37PM +0200, Jacek Caban wrote: >> Hi Marcus, >> >> On 10/01/12 23:00, Marcus Meissner wrote: >>> Hi, >>> >>> Various coverity issues complain about user-after-free scenarios, >>> all involving this code path. >>> >>> I strongly think if call_ret signals error, we also need to return >>> an error condition to avoid the callers from proceeding as if nothing >>> happened. >>> >>> Second reiteration with Jaceks comment applied >> Sorry for not catching this later, I was concentrated on your comment >> instead of the code, but the important thing is that true call_ret means >> success (and wininet doing request asynchronously is signalled as an >> error). It means that in this case we want to return RPC_S_OK. What is >> the exact problem? > Ok, my fix was likely bad. > > Coverity is spotting use-after-free scenarios. > > CID 715805 I restored my login in Coverity and looked at the report. It seems to be false positive. async_data is reference counted structure and we'll never free it in mentioned wait_async_request because we still store the reference in caller. Cheers, Jacek

Oct. 3, 2012

Re: [PATCH 2/2] vbscript: Added Right() implementation

by Marvin

Hi, While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at http://testbot.winehq.org/JobDetails.pl?Key=21987 Your paranoid android. === WINEBUILD (build) === Patch failed to apply

Oct. 3, 2012