April 2017 - Wine-Devel - list.winehq.org

Re: [v4 PATCH] odbccp32: Handle ODBC_CONFIG_DRIVER request in SQLConfigDriver/W

by Huw Davies

On Wed, Apr 26, 2017 at 12:46:49AM +0000, Alistair Leslie-Hughes wrote: > diff --git a/dlls/odbccp32/odbccp32.c b/dlls/odbccp32/odbccp32.c > index f28d464..475b74e 100644 > --- a/dlls/odbccp32/odbccp32.c > +++ b/dlls/odbccp32/odbccp32.c > @@ -65,6 +65,7 @@ static const WCHAR odbc_error_invalid_param_string[] = {'I','n','v','a','l','i', > static const WCHAR odbc_error_invalid_dsn[] = {'I','n','v','a','l','i','d',' ','D','S','N',0}; > static const WCHAR odbc_error_load_lib_failed[] = {'L','o','a','d',' ','L','i','b','r','a','r','y',' ','F','a','i','l','e','d',0}; > static const WCHAR odbc_error_request_failed[] = {'R','e','q','u','e','s','t',' ','F','a','i','l','e','d',0}; > +static const WCHAR odbc_error_invalid_keyword[] = {'I','n','v','a','l','i','d',' ','k','e','y','w','o','r','d',' ','v','a','l','u','e',0}; > > /* Push an error onto the error stack, taking care of ranges etc. */ > static void push_error(int code, LPCWSTR msg) > @@ -303,6 +304,64 @@ static HMODULE load_config_driver(const WCHAR *driver) > return hmod; > } > > +static BOOL write_config_value(const WCHAR *driver, const WCHAR *args) > +{ > + long ret; > + HKEY hkey, hkeydriver; > + WCHAR *name = NULL; > + > + if (!args) > + return FALSE; > + > + if ((ret = RegOpenKeyW(HKEY_LOCAL_MACHINE, odbcini, &hkey)) == ERROR_SUCCESS) > + { > + if ((ret = RegOpenKeyW(hkey, driver, &hkeydriver)) == ERROR_SUCCESS) > + { > + WCHAR *divider, *value; > + > + name = heap_alloc( (strlenW(args) + 1) * sizeof(WCHAR)); > + if(!name) > + { > + push_error(ODBC_ERROR_OUT_OF_MEM, odbc_error_out_of_mem); > + goto fail; > + } > + lstrcpyW(name, args); > + > + divider = strchrW(name,'='); > + if(!divider) > + { > + push_error(ODBC_ERROR_INVALID_KEYWORD_VALUE, odbc_error_invalid_keyword); > + goto fail; > + } > + > + value = divider + 1; > + *divider = '\0'; > + > + TRACE("Write pair: %s = %s\n", debugstr_w(name), debugstr_w(value)); > + if(RegSetValueExW(hkeydriver, name, 0, REG_SZ, (BYTE*)value, > + strlenW(value) * sizeof(WCHAR)) != ERROR_SUCCESS) The length should be (strlenW(value) + 1) * sizeof(WCHAR). Sorry, I missed that last time. While you're at it, decide whether you're going to use a space after 'if' or not, and then be consistent. Otherwise, this looks good. Huw.

April 26, 2017

Re: [v6 PATCH 03/21] x86/mpx: Do not use R/EBP as base in the SIB byte with Mod = 0

by Borislav Petkov

On Tue, Apr 25, 2017 at 07:04:20PM -0700, Ricardo Neri wrote: > For the specific case of ModRM.mod being 0, I feel I need to clarify > that REX.B is not decoded and if SIB.base is %r13 the base is also 0. Well, that all doesn't matter. The rule is this: ModRM.mod == 00b and ModRM.r/m == 101b -> effective address: disp32 See Table 2-2. "32-Bit Addressing Forms with the ModR/M Byte" in the SDM. So the base register is not used. How that base register is specified then doesn't matter (undecoded REX bits or not). > This comment adds clarity because REX.X is decoded when determining > SIB.index. Well, that's a different thing. The REX bits participating in the SIB fields don't matter about this particular case. We only want to say that we're returning a disp32 without a base register and the comment should keep it simple without extraneous information. I know, you want to mention what Table 2-5. "Special Cases of REX Encodings" says but we should avoid unnecessary content in the comment. People who want details can stare at the manuals - the comment should only document what that particular case is. Btw, you could write it even better: if (!X86_MODRM_MOD(insn->modrm.value) && X86_MODRM_RM(insn->modrm.value) == 5) and then it is basically a 1:1 copy of the rule from Table 2-2. -- Regards/Gruss, Boris. SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg) --

April 26, 2017

Re: po: Update Simplified Chinese translation.

by Alexandre Julliard

Jactry Zeng <jzeng(a)codeweavers.com> writes: > diff --git a/po/zh_CN.po b/po/zh_CN.po > index 45103bf3e4..f2addad1a5 100644 > --- a/po/zh_CN.po > +++ b/po/zh_CN.po > @@ -27,7 +27,7 @@ msgstr "Š»œçŒ¡ç®£çºŒ¯æ'®£'" > > #: appwiz.rc:62 > msgid "&Install..." > -msgstr "®£(&I)" > +msgstr "®£(&I)..." Is it really necessary to change all the ellipsis characters? They were changed the other way around not so long ago. -- Alexandre Julliard julliard(a)winehq.org

April 26, 2017

Re: [PATCH v2 1/2] mountmgr: Create devices and registry entries for serial ports.

by Alex Henrie

2017-04-25 3:09 GMT-06:00 Alexandre Julliard <julliard(a)winehq.org>: > Alex Henrie <alexhenrie24(a)gmail.com> writes: > >> + if (driver == serial_driver) >> + { >> + dos_name_format = comW; >> + nt_name_format = device_serialW; >> + reg_value_format = comW; >> + } >> + else >> + { >> + /* TODO: support parallel ports */ >> + } > > Please avoid introducing dead code. You can add the if() in the second > patch. Okay. >> + /* remove old symlinks */ >> + for (n = 1; n <= MAX_PORTS; n++) >> + { >> + sprintf( p, "%u", n ); >> + if (unlink( dosdevices_path ) != 0 && !errno) >> + break; >> + } > > Why would errno ever be 0 here? I mistakenly thought that unlink set the errno to 0 if the file does not exist. In reality, errno is ENOENT in this case. Thanks for catching that. -Alex

April 26, 2017

Re: [v6 PATCH 04/21] x86/mpx, x86/insn: Relocate insn util functions to a new insn-kernel

by Ricardo Neri

On Wed, 2017-04-12 at 12:03 +0200, Borislav Petkov wrote: > > + * If mod is 0 and register R/EBP (regno=5) is > indicated in the > > + * base part of the SIB byte, the value of such > register should > > + * not be used in the address computation. Also, a > 32-bit > > + * displacement is expected in this case; the > instruction > > + * decoder takes care of it. This is true for both R13 > and > > + * R/EBP as REX.B will not be decoded. > > + */ > > + if (regno == 5 && X86_MODRM_MOD(insn->modrm.value) == > 0) > > + return -EDOM; > > + > > + if (X86_REX_B(insn->rex_prefix.value)) > > + regno += 8; > > + break; > > + > > + default: > > + pr_err("invalid register type"); > > + BUG(); > > WARNING: Avoid crashing the kernel - try using WARN_ON & recovery code > rather than BUG() or BUG_ON() > #211: FILE: arch/x86/lib/insn-eval.c:90: > + BUG(); > > And checkpatch is kinda right. We need to warn here, not explode. Oh > and > that function returns negative values on error... > > Please change that with a patch ontop of the move. Sure, I will change it.

April 26, 2017

Re: [v6 PATCH 03/21] x86/mpx: Do not use R/EBP as base in the SIB byte with Mod = 0

by Ricardo Neri

On Wed, 2017-04-12 at 00:08 +0200, Borislav Petkov wrote: > On Tue, Mar 07, 2017 at 04:32:36PM -0800, Ricardo Neri wrote: > > Section 2.2.1.2 of the Intel 64 and IA-32 Architectures Software > > Developer's Manual volume 2A states that when a SIB byte is used and the > > base of the SIB byte points to R/EBP (i.e., base = 5) and the mod part > > of the ModRM byte is zero, the value of such register will not be used > > as part of the address computation. To signal this, a -EDOM error is > > returned to indicate callers that they should ignore the value. > > > > Also, for this particular case, a displacement of 32-bits should follow > > the SIB byte if the mod part of ModRM is equal to zero. The instruction > > decoder ensures that this is the case. > > > > Cc: Dave Hansen <dave.hansen(a)linux.intel.com> > > Cc: Adam Buchbinder <adam.buchbinder(a)gmail.com> > > Cc: Colin Ian King <colin.king(a)canonical.com> > > Cc: Lorenzo Stoakes <lstoakes(a)gmail.com> > > Cc: Qiaowei Ren <qiaowei.ren(a)intel.com> > > Cc: Peter Zijlstra <peterz(a)infradead.org> > > Cc: Nathan Howard <liverlint(a)gmail.com> > > Cc: Adan Hawthorn <adanhawthorn(a)gmail.com> > > Cc: Joe Perches <joe(a)perches.com> > > Cc: Ravi V. Shankar <ravi.v.shankar(a)intel.com> > > Cc: x86(a)kernel.org > > Signed-off-by: Ricardo Neri <ricardo.neri-calderon(a)linux.intel.com> > > --- > > arch/x86/mm/mpx.c | 29 ++++++++++++++++++++++------- > > 1 file changed, 22 insertions(+), 7 deletions(-) > > > > diff --git a/arch/x86/mm/mpx.c b/arch/x86/mm/mpx.c > > index d9e92d6..ef7eb67 100644 > > --- a/arch/x86/mm/mpx.c > > +++ b/arch/x86/mm/mpx.c > > @@ -121,6 +121,17 @@ static int get_reg_offset(struct insn *insn, struct pt_regs *regs, > > > > case REG_TYPE_BASE: > > regno = X86_SIB_BASE(insn->sib.value); > > + /* > > + * If mod is 0 and register R/EBP (regno=5) is indicated in the > > + * base part of the SIB byte, > > you can simply say here: "if SIB.base == 5, the base of the > register-indirect addressing is 0." This is better wording. I will change it. > > > the value of such register should > > + * not be used in the address computation. Also, a 32-bit > > Not "Also" but "In this case, a 32-bit displacement..." Will change. > > > + * displacement is expected in this case; the instruction > > + * decoder takes care of it. This is true for both R13 and > > + * R/EBP as REX.B will not be decoded. > > You don't need that sentence as the only thing that matters is ModRM.mod > being 0. For the specific case of ModRM.mod being 0, I feel I need to clarify that REX.B is not decoded and if SIB.base is %r13 the base is also 0. This comment adds clarity because REX.X is decoded when determining SIB.index. > > > + */ > > + if (regno == 5 && X86_MODRM_MOD(insn->modrm.value) == 0) > > The 0 test we normally do with the ! (also flip parts of if-condition): > > if (!X86_MODRM_MOD(insn->modrm.value) && regno == 5) Will change it. > > > + return -EDOM; > > + > > if (X86_REX_B(insn->rex_prefix.value)) > > regno += 8; > > break; > > @@ -161,16 +172,21 @@ static void __user *mpx_get_addr_ref(struct insn *insn, struct pt_regs *regs) > > eff_addr = regs_get_register(regs, addr_offset); > > } else { > > if (insn->sib.nbytes) { > > + /* > > + * Negative values in the base and index offset means > > + * an error when decoding the SIB byte. Except -EDOM, > > + * which means that the registers should not be used > > + * in the address computation. > > + */ > > base_offset = get_reg_offset(insn, regs, REG_TYPE_BASE); > > - if (base_offset < 0) > > + if (unlikely(base_offset == -EDOM)) > > + base = 0; > > + else if (unlikely(base_offset < 0)) > > Bah, unlikely's in something which is not really a hot path. They only > encumber readability, no need for them. I will remove them. Thanks and BR, Ricardo

April 26, 2017

Re: [v6 PATCH 01/21] x86/mpx: Use signed variables to compute effective addresses

by Ricardo Neri

On Tue, 2017-04-11 at 23:56 +0200, Borislav Petkov wrote: > On Tue, Mar 07, 2017 at 04:32:34PM -0800, Ricardo Neri wrote: > > Even though memory addresses are unsigned. The operands used to compute the > > ... unsigned, the operands ... Oops! I will correct.

April 26, 2017

Re: [v6 PATCH 02/21] x86/mpx: Do not use SIB index if index points to R/ESP

by Ricardo Neri

Hi Boris, I am sorry I missed your feedback earlier. Thanks for commenting! On Tue, 2017-04-11 at 13:31 +0200, Borislav Petkov wrote: > On Tue, Mar 07, 2017 at 04:32:35PM -0800, Ricardo Neri wrote: > > Section 2.2.1.2 of the Intel 64 and IA-32 Architectures Software > > Developer's Manual volume 2A states that when memory addressing is used > > (i.e., mod part of ModR/M is not 3), a SIB byte is used and the index of > > the SIB byte points to the R/ESP (i.e., index = 4), the index should not be > > used in the computation of the memory address. > > > > In these cases the address is simply the value present in the register > > pointed by the base part of the SIB byte plus the displacement byte. > > > > An example of such instruction could be > > > > insn -0x80(%rsp) > > > > This is represented as: > > > > [opcode] 4c 23 80 > > > > ModR/M=0x4c: mod: 0x1, reg: 0x1: r/m: 0x4(R/ESP) > > SIB=0x23: sc: 0, index: 0x100(R/ESP), base: 0x11(R/EBX): > > Displacement -0x80 > > > > The correct address is (base) + displacement; no index is used. > > > > We can achieve the desired effect of not using the index by making > > get_reg_offset return -EDOM in this particular case. This value indicates > > callers that they should not use the index to calculate the address. > > EINVAL continues to indicate that an error when decoding the SIB byte. > > > > Care is taken to allow R12 to be used as index, which is a valid scenario. > > > > Cc: Dave Hansen <dave.hansen(a)linux.intel.com> > > Cc: Adam Buchbinder <adam.buchbinder(a)gmail.com> > > Cc: Colin Ian King <colin.king(a)canonical.com> > > Cc: Lorenzo Stoakes <lstoakes(a)gmail.com> > > Cc: Qiaowei Ren <qiaowei.ren(a)intel.com> > > Cc: Peter Zijlstra <peterz(a)infradead.org> > > Cc: Nathan Howard <liverlint(a)gmail.com> > > Cc: Adan Hawthorn <adanhawthorn(a)gmail.com> > > Cc: Joe Perches <joe(a)perches.com> > > Cc: Ravi V. Shankar <ravi.v.shankar(a)intel.com> > > Cc: x86(a)kernel.org > > Signed-off-by: Ricardo Neri <ricardo.neri-calderon(a)linux.intel.com> > > --- > > arch/x86/mm/mpx.c | 19 +++++++++++++++++-- > > 1 file changed, 17 insertions(+), 2 deletions(-) > > > > diff --git a/arch/x86/mm/mpx.c b/arch/x86/mm/mpx.c > > index ff112e3..d9e92d6 100644 > > --- a/arch/x86/mm/mpx.c > > +++ b/arch/x86/mm/mpx.c > > @@ -110,6 +110,13 @@ static int get_reg_offset(struct insn *insn, struct pt_regs *regs, > > regno = X86_SIB_INDEX(insn->sib.value); > > if (X86_REX_X(insn->rex_prefix.value)) > > regno += 8; > > + /* > > + * If mod !=3, register R/ESP (regno=4) is not used as index in > > + * the address computation. Check is done after looking at REX.X > > + * This is because R12 (regno=12) can be used as an index. > > + */ > > + if (regno == 4 && X86_MODRM_MOD(insn->modrm.value) != 3) > > + return -EDOM; > > Hmm, ok, so this is a bit confusing, to me at least. Maybe you're saying > the same things but here's how I see it: > > 1. When ModRM.mod != 11b and ModRM.rm == 100b, all that does mean > is that you have a SIB byte following. I.e., you have indexed > register-indirect addressing. Yes, callers of this function already know that there is a SIB byte because they saw ModRM.mod != 11b and ModRM.rm == 100b and struct insn.sib.nbytes is non zero. > > Now, you still need to decode the SIB byte and it goes this way: > > SIB.index == 100b means that the index register specification is > null, i.e., the scale*index portion of that indexed register-indirect > addressing is null, i.e., you have an offset following the SIB byte. > Now, depending on ModRM.mod, that offset is: Yes, for this reason if ModRM.rm != 11b and an index of 100b is found the function return -EDOM to indicate callers to not use the index. We need to return -EDOM because this function returns an offset from the base of struct pt_regs for successful cases. A negative value indicates to not use the offset. Perhaps a better wording is to say as you propose: the scale*index portion that indexed register-indirect addressing is null. I will take your wording! > > ModRM.mod == 01b -> 1 byte offset > ModRM.mod == 10b -> 4 bytes offset Callers will now the size of the offset based on struct insn.displacement.value. > > That's why for an instruction like this one (let's use your example) you > have: > > 8b 4c 23 80 mov -0x80(%rbx,%riz,1),%ecx > > That's basically a binutils hack to state that the SIB index register is > null. > > Another SIB index register works, of course: > > 8b 4c 03 80 mov -0x80(%rbx,%rax,1),%ecx > > Ok, so far so good. > > 2. Now, the %r12 thing is part of the REX implications to those > encodings: That's the REX.X bit which adds a fourth bit to the encoding > of the SIB base register, i.e., if you specify a register with > SIB.index, you want to be able to specify all 16 regs, thus the 4th > bit. That's why it says that the SIB byte is required for %r12-based > addressing. > > I.e., you can still have a SIB.index == 100b addressing with an index > register which is not null but that is only because SIB.index is now > {REX.X=1b, 100b}, i.e.: > > Prefixes: > REX: 0x43 { 4 [w]: 0 [r]: 0 [x]: 1 [b]: 1 } > Opcode: 0x8b > ModRM: 0x4c [mod:1b][.R:0b,reg:1b][.B:1b,r/m:1100b] > register-indirect mode, 1-byte offset in displ. field > SIB: 0x63 [.B:1b,base:1011b][.X:1b,idx:1100b][scale: 1] > > MOV Gv,Ev; MOV reg{16,32,64} reg/mem{16,32,64} > 0: 43 8b 4c 63 80 mov -0x80(%r11,%r12,2),%ecx Correct, that is why we check the value of regno (the value of ModRM.rm) after correcting its value in case we find REX.X set. In this way we can use %r12. > > So, I'm not saying your version is necessarily wrong - I'm just saying > that it could explain the situation a bit more verbose. Sure. I will be more verbose in my commit message. > > Btw, I'd flip the if-test above: > > if (X86_MODRM_MOD(insn->modrm.value) != 3 && regno == 4) > > to make it just like the order the conditions are specified in the > manuals. Will do. Thanks and BR, Ricardo

April 26, 2017

Re: dmscript/tests: Use a void* instead of void** outer IUnknown argument

by Michael Stefaniuc

Alexandre, please ignore this patch as I have submitted a better one. On 04/25/2017 05:17 PM, Marvin wrote: > 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 > https://testbot.winehq.org/JobDetails.pl?Key=30436 > > Your paranoid android. > > > === wxppro (32 bit dmscript) === > dmscript.c:146: Test failed: DirectMusicScriptTrack create failed: 80004003, expected CLASS_E_NOAGGREGATION > > === wvistau64 (32 bit dmscript) === > 01f4:dmscript: unhandled exception c0000005 at 73E47D60 > > === w7u (32 bit dmscript) === > 0ac8:dmscript: unhandled exception c0000005 at 72157EA8 > > === w7pro64 (32 bit dmscript) === > 0978:dmscript: unhandled exception c0000005 at 73907EA8 > > === w8 (32 bit dmscript) === > 0938:dmscript: unhandled exception c0000005 at 69E4804C > > === w864 (32 bit dmscript) === > 0f8c:dmscript: unhandled exception c0000005 at 7451804C > > === w1064 (32 bit dmscript) === > 02bc:dmscript: unhandled exception c0000005 at 7078945E > > bye michael

April 25, 2017

Re: [v2 6/6] d3dx9: Support parameters sharing in effect.

by Matteo Bruni

2017-04-25 18:46 GMT+02:00 Paul Gofman <gofmanp(a)gmail.com>: > Could you please advise how to best add it: just as this patch in if(0) {}, > or a separate test? I prefer the former i.e. no separate test. Thanks!

April 25, 2017