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

[v3 PATCH 06/10] x86/cpufeature: Add User-Mode Instruction Prevention definitions

by Ricardo Neri

User-Mode Instruction Prevention is a security feature present in new Intel processors that, when set, prevents the execution of a subset of instructions if such instructions are executed in user mode (CPL > 0). Attempting to execute such instructions causes a general protection exception. The subset of instructions comprises: * SGDT - Store Global Descriptor Table * SIDT - Store Interrupt Descriptor Table * SLDT - Store Local Descriptor Table * SMSW - Store Machine Status Word * STR - Store Task Register This feature is also added to the list of disabled-features to allow a cleaner handling of build-time configuration. Cc: Andy Lutomirski <luto(a)kernel.org> Cc: Andrew Morton <akpm(a)linux-foundation.org> Cc: H. Peter Anvin <hpa(a)zytor.com> Cc: Borislav Petkov <bp(a)suse.de> Cc: Brian Gerst <brgerst(a)gmail.com> Cc: Chen Yucong <slaoub(a)gmail.com> Cc: Chris Metcalf <cmetcalf(a)mellanox.com> Cc: Dave Hansen <dave.hansen(a)linux.intel.com> Cc: Fenghua Yu <fenghua.yu(a)intel.com> Cc: Huang Rui <ray.huang(a)amd.com> Cc: Jiri Slaby <jslaby(a)suse.cz> Cc: Jonathan Corbet <corbet(a)lwn.net> Cc: Michael S. Tsirkin <mst(a)redhat.com> Cc: Paul Gortmaker <paul.gortmaker(a)windriver.com> Cc: Peter Zijlstra <peterz(a)infradead.org> Cc: Ravi V. Shankar <ravi.v.shankar(a)intel.com> Cc: Shuah Khan <shuah(a)kernel.org> Cc: Vlastimil Babka <vbabka(a)suse.cz> Cc: Tony Luck <tony.luck(a)intel.com> Cc: Paolo Bonzini <pbonzini(a)redhat.com> Cc: Liang Z. Li <liang.z.li(a)intel.com> Cc: Alexandre Julliard <julliard(a)winehq.org> Cc: Stas Sergeev <stsp(a)list.ru> Cc: x86(a)kernel.org Cc: linux-msdos(a)vger.kernel.org Signed-off-by: Ricardo Neri <ricardo.neri-calderon(a)linux.intel.com> --- arch/x86/include/asm/cpufeatures.h | 1 + arch/x86/include/asm/disabled-features.h | 8 +++++++- arch/x86/include/uapi/asm/processor-flags.h | 2 ++ 3 files changed, 10 insertions(+), 1 deletion(-) diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h index d45ab4b..fd04219 100644 --- a/arch/x86/include/asm/cpufeatures.h +++ b/arch/x86/include/asm/cpufeatures.h @@ -286,6 +286,7 @@ /* Intel-defined CPU features, CPUID level 0x00000007:0 (ecx), word 16 */ #define X86_FEATURE_AVX512VBMI (16*32+ 1) /* AVX512 Vector Bit Manipulation instructions*/ +#define X86_FEATURE_UMIP (16*32+ 2) /* User Mode Instruction Protection */ #define X86_FEATURE_PKU (16*32+ 3) /* Protection Keys for Userspace */ #define X86_FEATURE_OSPKE (16*32+ 4) /* OS Protection Keys Enable */ #define X86_FEATURE_AVX512_VPOPCNTDQ (16*32+14) /* POPCNT for vectors of DW/QW */ diff --git a/arch/x86/include/asm/disabled-features.h b/arch/x86/include/asm/disabled-features.h index 85599ad..4707445 100644 --- a/arch/x86/include/asm/disabled-features.h +++ b/arch/x86/include/asm/disabled-features.h @@ -16,6 +16,12 @@ # define DISABLE_MPX (1<<(X86_FEATURE_MPX & 31)) #endif +#ifdef CONFIG_X86_INTEL_UMIP +# define DISABLE_UMIP 0 +#else +# define DISABLE_UMIP (1<<(X86_FEATURE_UMIP & 31)) +#endif + #ifdef CONFIG_X86_64 # define DISABLE_VME (1<<(X86_FEATURE_VME & 31)) # define DISABLE_K6_MTRR (1<<(X86_FEATURE_K6_MTRR & 31)) @@ -55,7 +61,7 @@ #define DISABLED_MASK13 0 #define DISABLED_MASK14 0 #define DISABLED_MASK15 0 -#define DISABLED_MASK16 (DISABLE_PKU|DISABLE_OSPKE) +#define DISABLED_MASK16 (DISABLE_PKU|DISABLE_OSPKE|DISABLE_UMIP) #define DISABLED_MASK17 0 #define DISABLED_MASK_CHECK BUILD_BUG_ON_ZERO(NCAPINTS != 18) diff --git a/arch/x86/include/uapi/asm/processor-flags.h b/arch/x86/include/uapi/asm/processor-flags.h index 567de50..d2c2af8 100644 --- a/arch/x86/include/uapi/asm/processor-flags.h +++ b/arch/x86/include/uapi/asm/processor-flags.h @@ -104,6 +104,8 @@ #define X86_CR4_OSFXSR _BITUL(X86_CR4_OSFXSR_BIT) #define X86_CR4_OSXMMEXCPT_BIT 10 /* enable unmasked SSE exceptions */ #define X86_CR4_OSXMMEXCPT _BITUL(X86_CR4_OSXMMEXCPT_BIT) +#define X86_CR4_UMIP_BIT 11 /* enable UMIP support */ +#define X86_CR4_UMIP _BITUL(X86_CR4_UMIP_BIT) #define X86_CR4_VMXE_BIT 13 /* enable VMX virtualization */ #define X86_CR4_VMXE _BITUL(X86_CR4_VMXE_BIT) #define X86_CR4_SMXE_BIT 14 /* enable safer mode (TXT) */ -- 2.9.3

Jan. 25, 2017

[v3 PATCH 05/10] x86/insn-kernel: Add support to resolve 16-bit addressing encodings

by Ricardo Neri

Tasks running in virtual-8086 mode will use 16-bit addressing form encodings as described in the Intel 64 and IA-32 Architecture Software Developer's Manual Volume 2A Section 2.1.5. 16-bit addressing encodings differ in several ways from the 32-bit/64-bit addressing form encodings: the r/m part of the ModRM byte points to different registers and, in some cases, addresses can be indicated by the addition of the value of two registers. Also, there is no support for SiB bytes. Thus, a separate function is needed to parse this form of addressing. Furthermore, virtual-8086 mode tasks will use real-mode addressing. This implies that the segment selectors do not point to a segment descriptor but are used to compute logical addresses. Hence, there is a need to add support to compute addresses using the segment selectors. If segment- override prefixes are present in the instructions, they take precedence. Lastly, it is important to note that when a tasks is running in virtual- 8086 mode and an interrupt/exception occurs, the CPU pushes to the stack the segment selectors for ds, es, fs and gs. These are accesible via the struct kernel_vm86_regs rather than pt_regs. Code for 16-bit addressing encodings is likely to be used only by virtual- 8086 mode tasks. Thus, this code is wrapped to be built only if the option CONFIG_VM86 is selected. 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: Arnaldo Carvalho de Melo <acme(a)redhat.com> Cc: Masami Hiramatsu <mhiramat(a)kernel.org> Cc: Adrian Hunter <adrian.hunter(a)intel.com> Cc: Kees Cook <keescook(a)chromium.org> Cc: Thomas Garnier <thgarnie(a)google.com> Cc: Peter Zijlstra <peterz(a)infradead.org> Cc: Borislav Petkov <bp(a)suse.de> Cc: Dmitry Vyukov <dvyukov(a)google.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/lib/insn-kernel.c | 192 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 192 insertions(+) diff --git a/arch/x86/lib/insn-kernel.c b/arch/x86/lib/insn-kernel.c index 267cab4..10afdf3 100644 --- a/arch/x86/lib/insn-kernel.c +++ b/arch/x86/lib/insn-kernel.c @@ -8,6 +8,7 @@ #include <asm/inat.h> #include <asm/insn.h> #include <asm/insn-kernel.h> +#include <asm/vm86.h> enum reg_type { REG_TYPE_RM = 0, @@ -95,6 +96,194 @@ static int get_reg_offset(struct insn *insn, struct pt_regs *regs, return regoff[regno]; } +#ifdef CONFIG_VM86 + +/* + * Obtain the segment selector to use based on any prefixes in the instruction + * or in the offset of the register given by the r/m part of the ModRM byte. The + * register offset is as found in struct pt_regs. + */ +static unsigned short __get_segment_selector_16(struct pt_regs *regs, + struct insn *insn, int regoff) +{ + int i; + + struct kernel_vm86_regs *vm86regs = (struct kernel_vm86_regs *)regs; + + /* + * If not in virtual-8086 mode, the segment selector is not used + * to compute addresses but to select the segment descriptor. Return + * 0 to ease the computation of address. + */ + if (!v8086_mode(regs)) + return 0; + + insn_get_prefixes(insn); + + /* Check first if we have selector overrides */ + for (i = 0; i < insn->prefixes.nbytes; i++) { + switch (insn->prefixes.bytes[i]) { + /* + * Code and stack segment selector register are saved in all + * processor modes. Thus, it makes sense to take them + * from pt_regs. + */ + case 0x2e: + return (unsigned short)regs->cs; + case 0x36: + return (unsigned short)regs->ss; + /* + * The rest of the segment selector registers are only saved + * in virtual-8086 mode. Thus, we must obtain them from the + * vm86 register structure. + */ + case 0x3e: + return vm86regs->ds; + case 0x26: + return vm86regs->es; + case 0x64: + return vm86regs->fs; + case 0x65: + return vm86regs->gs; + /* + * No default action needed. We simply did not find any + * relevant prefixes. + */ + } + } + + /* + * If no overrides, use default selectors as described in the + * Intel documentationn. + */ + switch (regoff) { + case -EINVAL: /* no register involved in address computation */ + case offsetof(struct pt_regs, bx): + case offsetof(struct pt_regs, di): + case offsetof(struct pt_regs, si): + return vm86regs->ds; + case offsetof(struct pt_regs, bp): + case offsetof(struct pt_regs, sp): + return (unsigned short)regs->ss; + /* ax, cx, dx are not valid registers for 16-bit addressing*/ + default: + return -EINVAL; + } +} + +/* + * Obtain offsets from pt_regs to the two registers indicated by the + * r/m part of the ModRM byte. A negative offset indicates that the + * register should not be used. + */ +static int get_reg_offset_16(struct insn *insn, struct pt_regs *regs, + int *offs1, int *offs2) +{ + /* 16-bit addressing can use one or two registers */ + static const int regoff1[] = { + offsetof(struct pt_regs, bx), + offsetof(struct pt_regs, bx), + offsetof(struct pt_regs, bp), + offsetof(struct pt_regs, bp), + offsetof(struct pt_regs, si), + offsetof(struct pt_regs, di), + offsetof(struct pt_regs, bp), + offsetof(struct pt_regs, bx), + }; + + static const int regoff2[] = { + offsetof(struct pt_regs, si), + offsetof(struct pt_regs, di), + offsetof(struct pt_regs, si), + offsetof(struct pt_regs, di), + -EINVAL, + -EINVAL, + -EINVAL, + -EINVAL, + }; + + if (!offs1 || !offs2) + return -EINVAL; + + /* operand is a register, use the generic function */ + if (X86_MODRM_MOD(insn->modrm.value) == 3) { + *offs1 = insn_get_reg_offset_rm(insn, regs); + *offs2 = -EINVAL; + return 0; + } + + *offs1 = regoff1[X86_MODRM_RM(insn->modrm.value)]; + *offs2 = regoff2[X86_MODRM_RM(insn->modrm.value)]; + + /* + * If no displacement is indicated in the mod part of the ModRM byte, + * (mod part is 0) and the r/m part of the same byte is 6, no register + * is used caculate the operand address. An r/m part of 6 means that + * the second register offset is already invalid. + */ + if ((X86_MODRM_MOD(insn->modrm.value) == 0) && + (X86_MODRM_RM(insn->modrm.value) == 6)) + *offs1 = -EINVAL; + + return 0; +} + +static void __user *insn_get_addr_ref_16(struct insn *insn, + struct pt_regs *regs) +{ + unsigned long addr; + unsigned short addr1 = 0, addr2 = 0; + int addr_offset1, addr_offset2; + int ret; + unsigned short seg = 0; + + insn_get_displacement(insn); + + /* + * If operand is a register, the layout is the same as in + * 32-bit and 64-bit addressing. + */ + if (X86_MODRM_MOD(insn->modrm.value) == 3) { + addr_offset1 = get_reg_offset(insn, regs, REG_TYPE_RM); + if (addr_offset1 < 0) + goto out_err; + seg = __get_segment_selector_16(regs, insn, addr_offset1); + addr = (seg << 4) + regs_get_register(regs, addr_offset1); + } else { + ret = get_reg_offset_16(insn, regs, &addr_offset1, + &addr_offset2); + if (ret < 0) + goto out_err; + /* + * Don't fail on invalid offset values. They might be invalid + * because they are not supported. Instead, use them in the + * calculation only if they contain a valid value. + */ + if (addr_offset1 >= 0) + addr1 = regs_get_register(regs, addr_offset1); + if (addr_offset2 >= 0) + addr2 = regs_get_register(regs, addr_offset2); + seg = __get_segment_selector_16(regs, insn, addr_offset1); + if (seg < 0) + goto out_err; + addr = (seg << 4) + addr1 + addr2; + } + addr += insn->displacement.value; + + return (void __user *)addr; +out_err: + return (void __user *)-1; +} +#else + +static void __user *insn_get_addr_ref_16(struct insn *insn, + struct pt_regs *regs) +{ + return (void __user *)-1; +} + +#endif /* CONFIG_VM86 */ + int insn_get_reg_offset_rm(struct insn *insn, struct pt_regs *regs) { return get_reg_offset(insn, regs, REG_TYPE_RM); @@ -111,6 +300,9 @@ void __user *insn_get_addr_ref(struct insn *insn, struct pt_regs *regs) int addr_offset, base_offset, indx_offset; insn_byte_t sib; + if (insn->addr_bytes == 2) + return insn_get_addr_ref_16(insn, regs); + insn_get_modrm(insn); insn_get_sib(insn); sib = insn->sib.value; -- 2.9.3

Jan. 25, 2017

[v3 PATCH 04/10] x86/insn-kernel: Add a function to obtain register offset in ModRM

by Ricardo Neri

The function insn_get_reg_offset requires a type to indicate whether the returned offset is that given by by the ModRM or the SIB byte. Callers of this function would need the definition of the type struct. This is not needed. Instead, auxiliary functions can be defined for this purpose. When the operand is a register, the emulation code for User-Mode Instruction Prevention needs to know the offset of the register indicated in the r/m part of the ModRM byte. Thus, start by adding an auxiliary function for this purpose. 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: Arnaldo Carvalho de Melo <acme(a)redhat.com> Cc: Masami Hiramatsu <mhiramat(a)kernel.org> Cc: Adrian Hunter <adrian.hunter(a)intel.com> Cc: Kees Cook <keescook(a)chromium.org> Cc: Thomas Garnier <thgarnie(a)google.com> Cc: Peter Zijlstra <peterz(a)infradead.org> Cc: Borislav Petkov <bp(a)suse.de> Cc: Dmitry Vyukov <dvyukov(a)google.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/include/asm/insn-kernel.h | 1 + arch/x86/lib/insn-kernel.c | 5 +++++ 2 files changed, 6 insertions(+) diff --git a/arch/x86/include/asm/insn-kernel.h b/arch/x86/include/asm/insn-kernel.h index aef416a..3f34649 100644 --- a/arch/x86/include/asm/insn-kernel.h +++ b/arch/x86/include/asm/insn-kernel.h @@ -12,5 +12,6 @@ #include <asm/ptrace.h> void __user *insn_get_addr_ref(struct insn *insn, struct pt_regs *regs); +int insn_get_reg_offset_rm(struct insn *insn, struct pt_regs *regs); #endif /* _ASM_X86_INSN_KERNEL_H */ diff --git a/arch/x86/lib/insn-kernel.c b/arch/x86/lib/insn-kernel.c index 8072abe..267cab4 100644 --- a/arch/x86/lib/insn-kernel.c +++ b/arch/x86/lib/insn-kernel.c @@ -95,6 +95,11 @@ static int get_reg_offset(struct insn *insn, struct pt_regs *regs, return regoff[regno]; } +int insn_get_reg_offset_rm(struct insn *insn, struct pt_regs *regs) +{ + return get_reg_offset(insn, regs, REG_TYPE_RM); +} + /* * return the address being referenced be instruction * for rm=3 returning the content of the rm reg -- 2.9.3

Jan. 25, 2017

[v3 PATCH 03/10] x86/mpx, x86/insn: Relocate insn util functions to a new insn-kernel

by Ricardo Neri

Other kernel submodules can benefit from using the utility functions defined in mpx.c to obtain the addresses and values of operands contained in the general purpose registers. An instance of this is the emulation code used for instructions protected by the Intel User-Mode Instruction Prevention feature. Thus, these functions are relocated to a new insn-kernel.c file. The reason to not relocate these utilities into insn.c is that the latter solely analyses instructions given by a struct insn without any knowledge of the kernel context. This new utilities insn-kernel.c aims to be used within the context of the kernel. For instance, it can be used to determine memory addresses as encoded in the contents of the general purpose registers. These utilities come with a separate header. This is to avoid taking insn.c out of sync from the instructions decoders under tools/obj and tools/perf. This also avoids adding cumbersome #ifdef's for the #include'd files required to decode instructions in a kernel context. Functions are simply relocated. There are not functional or indentation changes. 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: Arnaldo Carvalho de Melo <acme(a)redhat.com> Cc: Masami Hiramatsu <mhiramat(a)kernel.org> Cc: Adrian Hunter <adrian.hunter(a)intel.com> Cc: Kees Cook <keescook(a)chromium.org> Cc: Thomas Garnier <thgarnie(a)google.com> Cc: Peter Zijlstra <peterz(a)infradead.org> Cc: Borislav Petkov <bp(a)suse.de> Cc: Dmitry Vyukov <dvyukov(a)google.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/include/asm/insn-kernel.h | 16 ++++ arch/x86/lib/Makefile | 2 +- arch/x86/lib/insn-kernel.c | 147 +++++++++++++++++++++++++++++++++++++ arch/x86/mm/mpx.c | 140 +---------------------------------- 4 files changed, 166 insertions(+), 139 deletions(-) create mode 100644 arch/x86/include/asm/insn-kernel.h create mode 100644 arch/x86/lib/insn-kernel.c diff --git a/arch/x86/include/asm/insn-kernel.h b/arch/x86/include/asm/insn-kernel.h new file mode 100644 index 0000000..aef416a --- /dev/null +++ b/arch/x86/include/asm/insn-kernel.h @@ -0,0 +1,16 @@ +#ifndef _ASM_X86_INSN_KERNEL_H +#define _ASM_X86_INSN_KERNEL_H +/* + * A collection of utility functions for x86 instruction analysis to be + * used in a kernel context. Useful when, for instance, making sense + * of the registers indicated by operands. + */ + +#include <linux/compiler.h> +#include <linux/bug.h> +#include <linux/err.h> +#include <asm/ptrace.h> + +void __user *insn_get_addr_ref(struct insn *insn, struct pt_regs *regs); + +#endif /* _ASM_X86_INSN_KERNEL_H */ diff --git a/arch/x86/lib/Makefile b/arch/x86/lib/Makefile index 34a7413..d33eff1 100644 --- a/arch/x86/lib/Makefile +++ b/arch/x86/lib/Makefile @@ -23,7 +23,7 @@ lib-y := delay.o misc.o cmdline.o cpu.o lib-y += usercopy_$(BITS).o usercopy.o getuser.o putuser.o lib-y += memcpy_$(BITS).o lib-$(CONFIG_RWSEM_XCHGADD_ALGORITHM) += rwsem.o -lib-$(CONFIG_INSTRUCTION_DECODER) += insn.o inat.o +lib-$(CONFIG_INSTRUCTION_DECODER) += insn.o inat.o insn-kernel.o lib-$(CONFIG_RANDOMIZE_BASE) += kaslr.o obj-y += msr.o msr-reg.o msr-reg-export.o hweight.o diff --git a/arch/x86/lib/insn-kernel.c b/arch/x86/lib/insn-kernel.c new file mode 100644 index 0000000..8072abe --- /dev/null +++ b/arch/x86/lib/insn-kernel.c @@ -0,0 +1,147 @@ +/* + * Utility functions for x86 operand and address decoding + * + * Copyright (C) Intel Corporation 2016 + */ +#include <linux/kernel.h> +#include <linux/string.h> +#include <asm/inat.h> +#include <asm/insn.h> +#include <asm/insn-kernel.h> + +enum reg_type { + REG_TYPE_RM = 0, + REG_TYPE_INDEX, + REG_TYPE_BASE, +}; + +static int get_reg_offset(struct insn *insn, struct pt_regs *regs, + enum reg_type type) +{ + int regno = 0; + + static const int regoff[] = { + offsetof(struct pt_regs, ax), + offsetof(struct pt_regs, cx), + offsetof(struct pt_regs, dx), + offsetof(struct pt_regs, bx), + offsetof(struct pt_regs, sp), + offsetof(struct pt_regs, bp), + offsetof(struct pt_regs, si), + offsetof(struct pt_regs, di), +#ifdef CONFIG_X86_64 + offsetof(struct pt_regs, r8), + offsetof(struct pt_regs, r9), + offsetof(struct pt_regs, r10), + offsetof(struct pt_regs, r11), + offsetof(struct pt_regs, r12), + offsetof(struct pt_regs, r13), + offsetof(struct pt_regs, r14), + offsetof(struct pt_regs, r15), +#endif + }; + int nr_registers = ARRAY_SIZE(regoff); + /* + * Don't possibly decode a 32-bit instructions as + * reading a 64-bit-only register. + */ + if (IS_ENABLED(CONFIG_X86_64) && !insn->x86_64) + nr_registers -= 8; + + switch (type) { + case REG_TYPE_RM: + regno = X86_MODRM_RM(insn->modrm.value); + if (X86_REX_B(insn->rex_prefix.value)) + regno += 8; + break; + + case REG_TYPE_INDEX: + 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 -EINVAL; + break; + + case REG_TYPE_BASE: + regno = X86_SIB_BASE(insn->sib.value); + /* + * If R/EBP (regno = 5) is indicated in the base part of the SIB + * byte, an explicit displacement must be specified. In other + * words, the mod part of the ModRM byte cannot be zero. + */ + if (regno == 5 && X86_MODRM_MOD(insn->modrm.value) == 0) + return -EINVAL; + + if (X86_REX_B(insn->rex_prefix.value)) + regno += 8; + break; + + default: + pr_err("invalid register type"); + BUG(); + break; + } + + if (regno >= nr_registers) { + WARN_ONCE(1, "decoded an instruction with an invalid register"); + return -EINVAL; + } + return regoff[regno]; +} + +/* + * return the address being referenced be instruction + * for rm=3 returning the content of the rm reg + * for rm!=3 calculates the address using SIB and Disp + */ +void __user *insn_get_addr_ref(struct insn *insn, struct pt_regs *regs) +{ + unsigned long addr, base, indx; + int addr_offset, base_offset, indx_offset; + insn_byte_t sib; + + insn_get_modrm(insn); + insn_get_sib(insn); + sib = insn->sib.value; + + if (X86_MODRM_MOD(insn->modrm.value) == 3) { + addr_offset = get_reg_offset(insn, regs, REG_TYPE_RM); + if (addr_offset < 0) + goto out_err; + addr = regs_get_register(regs, addr_offset); + } else { + if (insn->sib.nbytes) { + base_offset = get_reg_offset(insn, regs, REG_TYPE_BASE); + if (base_offset < 0) + goto out_err; + + indx_offset = get_reg_offset(insn, regs, REG_TYPE_INDEX); + /* + * A negative offset means that the register cannot be + * be used as an index. + */ + if (indx_offset < 0) + indx = 0; + else + indx = regs_get_register(regs, indx_offset); + + base = regs_get_register(regs, base_offset); + addr = base + indx * (1 << X86_SIB_SCALE(sib)); + } else { + addr_offset = get_reg_offset(insn, regs, REG_TYPE_RM); + if (addr_offset < 0) + goto out_err; + addr = regs_get_register(regs, addr_offset); + } + addr += insn->displacement.value; + } + return (void __user *)addr; +out_err: + return (void __user *)-1; +} diff --git a/arch/x86/mm/mpx.c b/arch/x86/mm/mpx.c index c59a851..ca6fe13 100644 --- a/arch/x86/mm/mpx.c +++ b/arch/x86/mm/mpx.c @@ -11,6 +11,7 @@ #include <linux/sched/sysctl.h> #include <asm/insn.h> +#include <asm/insn-kernel.h> #include <asm/mman.h> #include <asm/mmu_context.h> #include <asm/mpx.h> @@ -59,143 +60,6 @@ static unsigned long mpx_mmap(unsigned long len) return addr; } -enum reg_type { - REG_TYPE_RM = 0, - REG_TYPE_INDEX, - REG_TYPE_BASE, -}; - -static int get_reg_offset(struct insn *insn, struct pt_regs *regs, - enum reg_type type) -{ - int regno = 0; - - static const int regoff[] = { - offsetof(struct pt_regs, ax), - offsetof(struct pt_regs, cx), - offsetof(struct pt_regs, dx), - offsetof(struct pt_regs, bx), - offsetof(struct pt_regs, sp), - offsetof(struct pt_regs, bp), - offsetof(struct pt_regs, si), - offsetof(struct pt_regs, di), -#ifdef CONFIG_X86_64 - offsetof(struct pt_regs, r8), - offsetof(struct pt_regs, r9), - offsetof(struct pt_regs, r10), - offsetof(struct pt_regs, r11), - offsetof(struct pt_regs, r12), - offsetof(struct pt_regs, r13), - offsetof(struct pt_regs, r14), - offsetof(struct pt_regs, r15), -#endif - }; - int nr_registers = ARRAY_SIZE(regoff); - /* - * Don't possibly decode a 32-bit instructions as - * reading a 64-bit-only register. - */ - if (IS_ENABLED(CONFIG_X86_64) && !insn->x86_64) - nr_registers -= 8; - - switch (type) { - case REG_TYPE_RM: - regno = X86_MODRM_RM(insn->modrm.value); - if (X86_REX_B(insn->rex_prefix.value)) - regno += 8; - break; - - case REG_TYPE_INDEX: - 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 -EINVAL; - break; - - case REG_TYPE_BASE: - regno = X86_SIB_BASE(insn->sib.value); - /* - * If R/EBP (regno = 5) is indicated in the base part of the SIB - * byte, an explicit displacement must be specified. In other - * words, the mod part of the ModRM byte cannot be zero. - */ - if (regno == 5 && X86_MODRM_MOD(insn->modrm.value) == 0) - return -EINVAL; - - if (X86_REX_B(insn->rex_prefix.value)) - regno += 8; - break; - - default: - pr_err("invalid register type"); - BUG(); - break; - } - - if (regno >= nr_registers) { - WARN_ONCE(1, "decoded an instruction with an invalid register"); - return -EINVAL; - } - return regoff[regno]; -} - -/* - * return the address being referenced be instruction - * for rm=3 returning the content of the rm reg - * for rm!=3 calculates the address using SIB and Disp - */ -static void __user *mpx_get_addr_ref(struct insn *insn, struct pt_regs *regs) -{ - unsigned long addr, base, indx; - int addr_offset, base_offset, indx_offset; - insn_byte_t sib; - - insn_get_modrm(insn); - insn_get_sib(insn); - sib = insn->sib.value; - - if (X86_MODRM_MOD(insn->modrm.value) == 3) { - addr_offset = get_reg_offset(insn, regs, REG_TYPE_RM); - if (addr_offset < 0) - goto out_err; - addr = regs_get_register(regs, addr_offset); - } else { - if (insn->sib.nbytes) { - base_offset = get_reg_offset(insn, regs, REG_TYPE_BASE); - if (base_offset < 0) - goto out_err; - - indx_offset = get_reg_offset(insn, regs, REG_TYPE_INDEX); - /* - * A negative offset means that the register cannot be - * be used as an index. - */ - if (indx_offset < 0) - indx = 0; - else - indx = regs_get_register(regs, indx_offset); - - base = regs_get_register(regs, base_offset); - addr = base + indx * (1 << X86_SIB_SCALE(sib)); - } else { - addr_offset = get_reg_offset(insn, regs, REG_TYPE_RM); - if (addr_offset < 0) - goto out_err; - addr = regs_get_register(regs, addr_offset); - } - addr += insn->displacement.value; - } - return (void __user *)addr; -out_err: - return (void __user *)-1; -} - static int mpx_insn_decode(struct insn *insn, struct pt_regs *regs) { @@ -308,7 +172,7 @@ siginfo_t *mpx_generate_siginfo(struct pt_regs *regs) info->si_signo = SIGSEGV; info->si_errno = 0; info->si_code = SEGV_BNDERR; - info->si_addr = mpx_get_addr_ref(&insn, regs); + info->si_addr = insn_get_addr_ref(&insn, regs); /* * We were not able to extract an address from the instruction, * probably because there was something invalid in it. -- 2.9.3

Jan. 25, 2017

[v3 PATCH 02/10] x86/mpx: Fail decoding when SIB baseR/EBP is and no displacement is used

by Ricardo Neri

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 such SIB byte points to R/EBP (i.e., base = 5), an explicit displacement is needed. This is, the mod part of the ModRM byte cannot be zero. In case that no displacement is needed, the mod part of the ModRM byte must be 1 with the displacement byte equal to zero. Make the address decoder to return -EINVAL in the aforementioned 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: 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 | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/arch/x86/mm/mpx.c b/arch/x86/mm/mpx.c index 9d15f6b..c59a851 100644 --- a/arch/x86/mm/mpx.c +++ b/arch/x86/mm/mpx.c @@ -120,6 +120,14 @@ static int get_reg_offset(struct insn *insn, struct pt_regs *regs, case REG_TYPE_BASE: regno = X86_SIB_BASE(insn->sib.value); + /* + * If R/EBP (regno = 5) is indicated in the base part of the SIB + * byte, an explicit displacement must be specified. In other + * words, the mod part of the ModRM byte cannot be zero. + */ + if (regno == 5 && X86_MODRM_MOD(insn->modrm.value) == 0) + return -EINVAL; + if (X86_REX_B(insn->rex_prefix.value)) regno += 8; break; -- 2.9.3

Jan. 25, 2017

[v3 PATCH 01/10] x86/mpx: Do not use SIB index if index points to R/ESP

by Ricardo Neri

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 a negative offset in this particular case. A negative offset indicates callers that they should not use the index to calculate the address. This is equivalent to using a index of zero when multiplying it by the base. 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: 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 | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/arch/x86/mm/mpx.c b/arch/x86/mm/mpx.c index af59f80..9d15f6b 100644 --- a/arch/x86/mm/mpx.c +++ b/arch/x86/mm/mpx.c @@ -109,6 +109,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 -EINVAL; break; case REG_TYPE_BASE: @@ -157,11 +164,16 @@ static void __user *mpx_get_addr_ref(struct insn *insn, struct pt_regs *regs) goto out_err; indx_offset = get_reg_offset(insn, regs, REG_TYPE_INDEX); + /* + * A negative offset means that the register cannot be + * be used as an index. + */ if (indx_offset < 0) - goto out_err; + indx = 0; + else + indx = regs_get_register(regs, indx_offset); base = regs_get_register(regs, base_offset); - indx = regs_get_register(regs, indx_offset); addr = base + indx * (1 << X86_SIB_SCALE(sib)); } else { addr_offset = get_reg_offset(insn, regs, REG_TYPE_RM); -- 2.9.3

Jan. 25, 2017

[v3 PATCH 00/10] x86: Enable User-Mode Instruction Prevention

by Ricardo Neri

This is v3 of this series. Again, it took me a while to complete the updates as support for virtual-8086 mode required extra rework. The two previous submissions can be found here [1] and here [2]. User-Mode Instruction Prevention (UMIP) is a security feature present in new Intel Processors. If enabled, it prevents the execution of certain instructions if the Current Privilege Level (CPL) is greater than 0. If these instructions were executed while in CPL > 0, user space applications could have access to system-wide settings such as the global and local descriptor tables, the segment selectors to the current task state and the local descriptor table. These are the instructions covered by UMIP: * SGDT - Store Global Descriptor Table * SIDT - Store Interrupt Descriptor Table * SLDT - Store Local Descriptor Table * SMSW - Store Machine Status Word * STR - Store Task Register If any of these instructions is executed with CPL > 0, a general protection exception is issued when UMIP is enabled. There is a caveat, however. Certain applications rely on some of these instructions to function. An example of this are applications that use WineHQ[3]. For instance, these applications rely on sidt returning a non- accesible memory location[4]. During the discussions, it was proposed that the fault could be relied to the user-space and perform the emulation in user-mode. However, this would break existing applications until, for instance, they update to a new WineHQ version. However, this approach would require UMIP to be disabled by default. The concensus in this forum is to always enable it. This patchset initially treated tasks running in virtual-8086 mode as a special case. However, I received clarification that DOSEMU[5] does not support applications that use these instructions. It relies on WineHQ for this[6]. Furthermore, the applications for which the concern was raised run in protected mode [4]. This version keeps UMIP enabled at all times and by default. If a general protection fault caused by the instructions protected by UMIP is detected, such fault will be fixed-up by returning dummy values as follows: * SGDT and SIDT return hard-coded dummy values as the base of the global descriptor and interrupt descriptor tables. These hard-coded values are located within a memory map hole in x86_64. For x86_32, the same values are used but truncated to 4 bytes. This is also the case for virtual- 8086 mode tasks, in which the base is truncated to 3 bytes. In all cases, the limit of the table is set to 0. * STR and SLDT return 0 as the segment selector. This looks appropriate since we are providing a dummy value as the base address of the global descriptor table. * SMSW returns the value with which the CR0 register is programmed in head_32/64.S at boot time. This is, the following bits are enabed: CR0.0 for Protection Enable, CR.1 for Monitor Coprocessor, CR.4 for Extension Type, which will always be 1 in recent processors with UMIP; CR.5 for Numeric Error, CR0.16 for Write Protect, CR0.18 for Alignment Mask. Additionally, in x86_64, CR0.31 for Paging is set. The proposed emulation code is handles faults that happens in both protected and virtual-8086 mode. I found very useful the code for Intel MPX (Memory Protection Extensions) used to parse opcodes and the memory locations contained in the general purpose registers when used as operands. I put some of this code in a separate library file that both MPX and UMIP can access and avoid code duplication. While here, I fixed two small bugs that I found in the MPX implementation. This new library was also extended to handle 16-bit address encodings as those found in virtual-8086 mode tasks. A set of representative selftests for virtual-8086 mode tasks are included as part of this series. The tested uses use displacements, registers to indicate memory addresses as well as the use as registers as operands. Extensive test cases were performed to test the page fault that is emulated when memory to write the instructions results is not accesible[7], to tests almost all combinations of operands (ModRM, SiB, REX prefix and displacements) in protected mode[8] and to test almost all the combinations of operands in virtual-8086 mode[9]. If there is interest, this could could also be submitted for the kernel selftests. [1]. https://lwn.net/Articles/705877/ [2]. https://lkml.org/lkml/2016/12/23/265 [3]. https://www.winehq.org/ [4]. https://www.winehq.org/pipermail/wine-devel/2016-November/115320.html [5]. http://www.dosemu.org/ [6]. http://marc.info/?l=linux-kernel&m=147876798717927&w=2 [7]. https://github.com/01org/luv-yocto/blob/rneri/umip/meta-luv/recipes-core/um… [8]. https://github.com/01org/luv-yocto/blob/rneri/umip/meta-luv/recipes-core/um… [9]. https://github.com/01org/luv-yocto/blob/rneri/umip/meta-luv/recipes-kernel/… Thanks and BR, Ricardo Changes since V2: * Added new utility functions to decode the memory addresses contained in registers when the 16-bit addressing encodings are used. This includes code to obtain and compute memory addresses using segment selectors for real-mode address translation. * Added support to emulate UMIP-protected instructions for virtual-8086 tasks. * Added selftests for virtual-8086 mode that contains representative use cases: address represented as a displacement, address in registers and registers as operands. * Instead of maintaining a static variable for the dummy base addresses of the IDT and GDT, a hard-coded value is used. * The emulated SMSW instructions now return the value with which the CR0 register is programmed in head_32/64.S This is: PE | MP | ET | NE | WP | AM. For x86_64, PG is also enabled. * The new file arch/x86/lib/insn-utils.c is now renamed as arch/x86/lib/ insn-kernel.c. It also has its own header. This helps keep in sync the the kernel and objtool instruction decoders. Also, the new insn-kernel.c contains utility functions that are only relevant in a kernel context. * Removed printed warnings for errors that occur when decoding instructions with invalid operands. * Added more comments on fixes in the instruction-decoding MPX functions. * Now user_64bit_mode(regs) is used instead of test_thread_flag(TIF_IA32) to determine if the task is 32-bit or 64-bit. * Found and fixed a bug in insn-decoder in which X86_MODRM_RM was incorrectly used to obtain the mod part of the ModRM byte. * Added more explanatory code in emulation and instruction decoding code. This includes a comment regarding that copy_from_user could fail if there exists a memory protection key in place. * Tested code with CONFIG_X86_DECODER_SELFTEST=y and everything passes now. * Prefixed get_reg_offset_rm with insn_ as this function is exposed via a header file. For clarity, this function was added in a separate patch. Changes since V1: * Virtual-8086 mode tasks are not treated in a special manner. All code for this purpose was removed. * Instead of attempting to disable UMIP during a context switch or when entering virtual-8086 mode, UMIP remains enabled all the time. General protection faults that occur are fixed-up by returning dummy values as detailed above. * Removed umip= kernel parameter in favor of using clearcpuid=514 to disable UMIP. * Removed selftests designed to detect the absence of SIGSEGV signals when running in virtual-8086 mode. * Reused code from MPX to decode instructions operands. For this purpose code was put in a common location. * Fixed two bugs in MPX code that decodes operands. Cc: Andy Lutomirski <luto(a)kernel.org> Cc: Andrew Morton <akpm(a)linux-foundation.org> Cc: Borislav Petkov <bp(a)suse.de> Cc: Brian Gerst <brgerst(a)gmail.com> Cc: Chen Yucong <slaoub(a)gmail.com> Cc: Chris Metcalf <cmetcalf(a)mellanox.com> Cc: Dave Hansen <dave.hansen(a)linux.intel.com> Cc: Fenghua Yu <fenghua.yu(a)intel.com> Cc: Huang Rui <ray.huang(a)amd.com> Cc: Jiri Slaby <jslaby(a)suse.cz> Cc: Jonathan Corbet <corbet(a)lwn.net> Cc: Michael S. Tsirkin <mst(a)redhat.com> Cc: Paul Gortmaker <paul.gortmaker(a)windriver.com> Cc: Peter Zijlstra <peterz(a)infradead.org> Cc: Ravi V. Shankar <ravi.v.shankar(a)intel.com> Cc: Vlastimil Babka <vbabka(a)suse.cz> Cc: Shuah Khan <shuah(a)kernel.org> Cc: Paolo Bonzini <pbonzini(a)redhat.com> Cc: Liang Z Li <liang.z.li(a)intel.com> Cc: x86(a)kernel.org Cc: linux-msdos(a)vger.kernel.org Ricardo Neri (10): x86/mpx: Do not use SIB index if index points to R/ESP x86/mpx: Fail decoding when SIB baseR/EBP is and no displacement is used x86/mpx, x86/insn: Relocate insn util functions to a new insn-kernel x86/insn-kernel: Add a function to obtain register offset in ModRM x86/insn-kernel: Add support to resolve 16-bit addressing encodings x86/cpufeature: Add User-Mode Instruction Prevention definitions x86: Add emulation code for UMIP instructions x86/traps: Fixup general protection faults caused by UMIP x86: Enable User-Mode Instruction Prevention selftests/x86: Add tests for User-Mode Instruction Prevention arch/x86/Kconfig | 10 + arch/x86/include/asm/cpufeatures.h | 1 + arch/x86/include/asm/disabled-features.h | 8 +- arch/x86/include/asm/insn-kernel.h | 17 ++ arch/x86/include/asm/umip.h | 15 ++ arch/x86/include/uapi/asm/processor-flags.h | 2 + arch/x86/kernel/Makefile | 1 + arch/x86/kernel/cpu/common.c | 16 +- arch/x86/kernel/traps.c | 4 + arch/x86/kernel/umip.c | 251 +++++++++++++++++++ arch/x86/lib/Makefile | 2 +- arch/x86/lib/insn-kernel.c | 344 ++++++++++++++++++++++++++ arch/x86/mm/mpx.c | 120 +-------- tools/testing/selftests/x86/entry_from_vm86.c | 39 ++- 14 files changed, 708 insertions(+), 122 deletions(-) create mode 100644 arch/x86/include/asm/insn-kernel.h create mode 100644 arch/x86/include/asm/umip.h create mode 100644 arch/x86/kernel/umip.c create mode 100644 arch/x86/lib/insn-kernel.c -- 2.9.3

Jan. 25, 2017

Re: DirectX 12 in Wine

by Gabríel Arthúr Pétursson

On Wed, 2017-01-25 at 17:12 +0100, Józef Kucia wrote: > The first step for implementing D3D12 in Wine is adding a support for > Vulkan. This is required in order to properly implement DXGI > swapchains which are linked to a given HWND. Generally, I recommend > to > focus on Vulkan support first. Vulkan support in Wine will be > immediately beneficial to Vulkan applications, and it is the starting > point for further D3D12 work. Wine Staging has experimental patches for Vulkan support. Are those patches in a state that can be considered for inclusion into Wine? If not, what further work is needed? There are a number of concerns I have looking at those patches: The API translation is done in the Vulkan loader. This is pr oblematic because applications may ship their own loader, or statically link one. Applications that do that would not work under Wine. There does not appear to be support for instance layers. The Steam overlay uses layers for Vulkan games. Vulkan debuggers also rely on them, e.g. RenderDoc. How about implementing the API translation as a Vulkan ICD and ship the official Khronos Vulkan loader as-is?

Jan. 25, 2017

Re: DirectX 12 in Wine

by Bruno Jesus

On Wed, Jan 25, 2017 at 2:12 PM, Józef Kucia <joseph.kucia(a)gmail.com> wrote: > The first step for implementing D3D12 in Wine is adding a support for > Vulkan. This is required in order to properly implement DXGI > swapchains which are linked to a given HWND. Generally, I recommend to > focus on Vulkan support first. Vulkan support in Wine will be > immediately beneficial to Vulkan applications, and it is the starting > point for further D3D12 work. That is a win-win situation as Vulkan is already implemented in Staging for almost 1 year, so most likely mature enough.

Jan. 25, 2017

Re: DirectX 12 in Wine

by Józef Kucia

Hi, On Tue, Jan 24, 2017 at 7:17 AM, Gabríel Arthúr Pétursson <gabriel(a)system.is> wrote: > With that said, what would an appropriate implementation in Wine look > like? > > DirectX 12 on top of Vulkan seems like an obvious choice. The two > APIs are very similar, whereas the DirectX 12 programming model > deviates significantly from both OpenGL and DirectX 11. Right, Vulkan seems to be the best choice. A hypothetical implementation of D3D12 in Wine is likely to be a separate thing from wined3d. > > Another approach is to leverage wined3d (i.e. OpenGL + extensions). I > don't know to what extent wined3d can be used or needs to be changed in > order to support DirectX 12, or if it'd be useful at all. Can anyone > shed some light on whether this would be a feasible approach? The only thing useful for D3D12 in wined3d seems to be SM5 bytecode parser. The first step for implementing D3D12 in Wine is adding a support for Vulkan. This is required in order to properly implement DXGI swapchains which are linked to a given HWND. Generally, I recommend to focus on Vulkan support first. Vulkan support in Wine will be immediately beneficial to Vulkan applications, and it is the starting point for further D3D12 work. Cheers, Józef

Jan. 25, 2017

Re: A couple unrelated issues

by Christopher Harrington

On Wed, Jan 25, 2017 at 8:16 AM Andrew Eikum <aeikum(a)codeweavers.com> wrote: > On Tue, Jan 24, 2017 at 04:33:58PM +0100, Rafał Mużyło wrote: > > On Tue, Jan 24, 2017 at 08:42:12AM -0600, Andrew Eikum wrote: > > > Are you using PulseAudio via winealsa with the alsa-pulse plugin? Why > > > not just use the winepulse driver? > > > > > > > Well, because reasons... > > > > Also, using winepulse means there's one fewer "part" in the audio > system, and so one fewer place to have bugs. > > And fewer places to introduce latency as audio data flows between threads that may be fighting for scheduling. -Chris Harrington

Jan. 25, 2017

Re: A couple unrelated issues

by Andrew Eikum

On Tue, Jan 24, 2017 at 04:33:58PM +0100, Rafał Mużyło wrote: > On Tue, Jan 24, 2017 at 08:42:12AM -0600, Andrew Eikum wrote: > > On Mon, Jan 23, 2017 at 04:26:14PM +0100, Rafał Mużyło wrote: > > > 2. alsa/pulseaudio and stream recovery: as in the past, it's hard to > > > tell if it's wine or the underlaying stuff that fails; the problem I'm > > > hearing is that apps sometimes without any apparent reason (as CPU usage > > > isn't all that different than the average) go into '(snd_pcm_recover) > > > underrun occurred' mode and start playing the sounds on much higher rate > > > and then just as randomly (as it's again seemingly not related to CPU > > > usage) recover; it's the randomness that bothers me: both going into the > > > state *and* not recovering for several minutes; would it be possible, if > > > not outright fix that, then for example add a helper app, that the user > > > could manually trigger to force recovery ? > > > > > > > Are you using PulseAudio via winealsa with the alsa-pulse plugin? Why > > not just use the winepulse driver? > > > > Well, because reasons... > > OK, a less tongue-in-cheek answer: while I've been working under > pulseaudio for quite awhile (if you recall my older posts/bugs), I'm > still of a strong opinion, that barring very specific needs (like audio > authoring), anything that works with pure alsa, should just as well work with pulseaudio underneath, > unless there's a bug in alsa's pulseaudio plugin or pulseaudio itself - then it's that bug that needs to be adressed (well, at least in the long term). > What's more the random nature of the problem (both triggering it - sometimes things work for hours just fine - and getting cleaned up - again, sometimes it gets cleaned up fast enough it's barely noticable; coupled with the trigger *seemingly* not being tied to actual CPU usage, just happening on a whim...) irks me. > The reason you should use the winepulse driver for a system using PulseAudio is because the ALSA API can't inform the application of latency changes. PA sometimes changes the latency parameters of the stream and informs its clients of this change, which should then compensate. But the ALSA API has no way to communicate this to the application, and things break as we assume the old latency parameters still hold. This particularly presented itself as choppy audio with USB devices, but it can be reproduced with any other latency change. For example, just opening 'pavucontrol' can cause a latency change. I suspect "something" is triggering a latency change on your system. Perhaps PA debug logs could tell you. Also, using winepulse means there's one fewer "part" in the audio system, and so one fewer place to have bugs. Andrew

Jan. 25, 2017

Re: [PATCH 3/7] d3d11/tests: Add test for interleaved Draw() and Dispatch() calls.

by Józef Kucia

On Wed, Jan 25, 2017 at 1:13 PM, Henri Verbeet <hverbeet(a)gmail.com> wrote: > On 25 January 2017 at 11:17, Józef Kucia <jkucia(a)codeweavers.com> wrote: >> Signed-off-by: Józef Kucia <jkucia(a)codeweavers.com> >> --- >> dlls/d3d11/tests/d3d11.c | 186 +++++++++++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 186 insertions(+) >> > This fails here: > > d3d11.c:1310: Feature level 0xb000. > d3d11.c:6487: Tests skipped: Some AMD drivers have a bug affecting the test. > d3d11.c:10104: Compute shader support via SM4 0x1. > d3d11.c:11197: Optional format 0x59 - display supported, feature > level 0xa100. > d3d11.c:11197: Optional format 0x59 - display supported, feature > level 0xa000. > d3d11.c:12628: Test failed: Got 0x00000000, expected 0xffffffff at > (0, 0), sub-resource 0. > d3d11: 7402 tests executed (0 marked as todo, 1 failure), 1 skipped. Right, I've forgot to remove backbuffer clears and backbuffer check_texture_color() calls from the test. The results weren't consistent across GPU drivers. > > It also takes a long time to run; it might be hanging the GPU. I haven't noticed, but I'll try to look into this.

Jan. 25, 2017

Re: [PATCH 3/7] d3d11/tests: Add test for interleaved Draw() and Dispatch() calls.

by Henri Verbeet

On 25 January 2017 at 11:17, Józef Kucia <jkucia(a)codeweavers.com> wrote: > Signed-off-by: Józef Kucia <jkucia(a)codeweavers.com> > --- > dlls/d3d11/tests/d3d11.c | 186 +++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 186 insertions(+) > This fails here: d3d11.c:1310: Feature level 0xb000. d3d11.c:6487: Tests skipped: Some AMD drivers have a bug affecting the test. d3d11.c:10104: Compute shader support via SM4 0x1. d3d11.c:11197: Optional format 0x59 - display supported, feature level 0xa100. d3d11.c:11197: Optional format 0x59 - display supported, feature level 0xa000. d3d11.c:12628: Test failed: Got 0x00000000, expected 0xffffffff at (0, 0), sub-resource 0. d3d11: 7402 tests executed (0 marked as todo, 1 failure), 1 skipped. It also takes a long time to run; it might be hanging the GPU.

Jan. 25, 2017

Post-release

by Alexandre Julliard

Congrats to everybody for the 2.0 release! I'm particularly pleased with the progress we've made on fixing regressions, I didn't expect we'd get that many. Thanks all! The master branch is now open for development again. I'll go through the patches that are still in the queue, but please resend any patch that had been marked Deferred. I'll also resume the biweekly development release cycle, starting with release 2.1 on February 3rd. Despite fierce competition, Michael Stefaniuc once again took the coveted job of Stable Branch Maintainer. He'll be making regular 2.0.x releases. I've also implemented the patch reviewer assignment scheme that we discussed at WineConf. From now on, patches that stay in the queue unreviewed for a week will be assigned a reviewer automatically. The algorithm is prioritizing people who said they have some knowledge about the area (the P: entries in the MAINTAINERS file), before grabbing someone at random from the wildcard entry. If you feel that there are areas where you could help reviewing patches, but don't feel comfortable putting yourself up as maintainer, please consider adding an entry for yourself as a knowledgeable person. If you are asked to review a patch, and you think it looks good but don't feel that you know enough to sign off on it, please just send a reply saying "looks OK to me" or something like that. I may then ask someone else to have a further look, or do it myself. Any review helps, even without a full signoff. Looking forward to 3.0 :-) -- Alexandre Julliard julliard(a)winehq.org

Jan. 25, 2017

Re: A couple unrelated issues

by Rafał Mużyło

On Wed, Jan 25, 2017 at 12:26:24AM +0100, Matteo Bruni wrote: > 2017-01-23 16:26 GMT+01:00 Rafał Mużyło <galtgendo(a)gmail.com>: > > > 3. err:ntdll:RtlpWaitForCriticalSection wait timed out in thread <nr>, blocked by <nr>, retrying (60 sec): this is an annoyance, sometimes a major one; namely, it happens that I terminate an app by pressing ^C; usually, that works just fine, but every now and then app goes into that unterminatable state and not for those 60 sec (which is horribly long anyway), but pretty much indefinitelly; it really shouldn't do that; this requires a taskmgr or wineboot intervention to close it > > You shouldn't terminate applications like that. You might catch the > application in the middle of "something", inside some locked section > or similar. There is no way for Wine to fix things afterwards in > general. > Oh, really ???... The thing is I've seen that happen on closing main app window or even standard app exit. Granted, it's quite rare, still... My point is that when that happens, subsequent ^C has no effect. Would it be possible to (for example) add a generic SIGINT handler to wine sever, so that upon receiving ^C from its terminal the Windows app would go into the standard forced shutdown cycle (that is the one hung apps go upon Windows shutdown) instead of this repeated 60 sec. wait ? As for the regedit patch, it's more of a question, than an answer. I've simply took a few lines from another place in wine (can't recall which anymore), then started thinking out loud. The end result feels a bit messy, even if it solved the problem for me. If you consider it good enough, :shrug: fine by me.

Jan. 25, 2017

Re: rstrtmgr: add RmShutdown stub (resend)

by Sebastian Lackner

On 25.01.2017 06:28, Austin English wrote: > CC: Sebastian Lackner <sebastian(a)fds-team.de> > > Supersedes https://source.winehq.org/patches/data/128639 > > Fixes https://bugs.winehq.org/show_bug.cgi?id=42093 > > Staged, https://github.com/wine-compholio/wine-staging/commit/b013afa66065427110c02… > > -- -Austin GPG: 14FB D7EA A041 937B > > > 0001-rstrtmgr-add-RmShutdown-stub-resend.patch > > > From fab99f2f9e398978461af053b38aa9b06b7bdc1d Mon Sep 17 00:00:00 2001 > From: Austin English <austinenglish(a)gmail.com> > Date: Wed, 28 Dec 2016 02:18:13 -0600 > Subject: [PATCH] rstrtmgr: add RmShutdown stub (resend) > > Signed-off-by: Austin English <austinenglish(a)gmail.com> > --- > dlls/rstrtmgr/main.c | 9 +++++++++ > dlls/rstrtmgr/rstrtmgr.spec | 2 +- > include/restartmanager.h | 3 +++ > 3 files changed, 13 insertions(+), 1 deletion(-) > > diff --git a/dlls/rstrtmgr/main.c b/dlls/rstrtmgr/main.c > index 2d45639..5c9a8dc 100644 > --- a/dlls/rstrtmgr/main.c > +++ b/dlls/rstrtmgr/main.c > @@ -100,3 +100,12 @@ DWORD WINAPI RmEndSession(DWORD handle) > FIXME("%u stub!\n", handle); > return ERROR_CALL_NOT_IMPLEMENTED; > } > + > +/*********************************************************************** > + * RmShutdown (rstrtmgr.@) > + */ > +DWORD WINAPI RmShutdown(DWORD handle, ULONG flags, RM_WRITE_STATUS_CALLBACK status) > +{ > + FIXME("%u, 0x%08x, %p stub!\n", handle, flags, status); > + return ERROR_CALL_NOT_IMPLEMENTED; > +} > diff --git a/dlls/rstrtmgr/rstrtmgr.spec b/dlls/rstrtmgr/rstrtmgr.spec > index 0e1ba79..6c6a9c9 100644 > --- a/dlls/rstrtmgr/rstrtmgr.spec > +++ b/dlls/rstrtmgr/rstrtmgr.spec > @@ -8,5 +8,5 @@ > @ stub RmRemoveFilter > @ stub RmReserveHeap > @ stdcall RmRestart(long long ptr) > -@ stub RmShutdown > +@ stdcall RmShutdown(long long ptr) > @ stdcall RmStartSession(ptr long ptr) > diff --git a/include/restartmanager.h b/include/restartmanager.h > index 27c1a92..51867a3 100644 > --- a/include/restartmanager.h > +++ b/include/restartmanager.h > @@ -64,6 +64,9 @@ typedef struct { > BOOL bRestartable; > } RM_PROCESS_INFO, *PRM_PROCESS_INFO; > > + > +typedef void *RM_WRITE_STATUS_CALLBACK(UINT); The calling convention is missing here, please see the staged version how it should look like. > + > #ifdef __cplusplus > } > #endif > -- 2.10.2 >

Jan. 24, 2017

Re: [resend 2/3] ntdll: Implement FileIdInformation class support in NtQueryInformationFile.

by Sebastian Lackner

On 25.01.2017 04:57, Jonathan Doron wrote: > + ((PLARGE_INTEGER)&info->VolumeSerialNumber)->LowPart = (ULONG)volstats.f_fsid.__val[0]; > + ((PLARGE_INTEGER)&info->VolumeSerialNumber)->HighPart = (ULONG)volstats.f_fsid.__val[1]; Accessing members of f_fsid is not cross-platform compatible - you will either have to find a better way to get this information, or at least add corresponding checks to configure.ac. For more information see: https://linux.die.net/man/2/fstatfs """Nobody knows what f_fsid is supposed to contain (but see below).""" """Solaris, Irix and POSIX have a system call statvfs(2) that returns a struct statvfs (defined in <sys/statvfs.h>) containing an unsigned long f_fsid. Linux, SunOS, HP-UX, 4.4BSD have a system call statfs() that returns a struct statfs (defined in <sys/vfs.h>) containing a fsid_t f_fsid, where fsid_t is defined as struct { int val[2]; }."""

Jan. 24, 2017

Re: [PATCH resend] user32: Also remove WM_CHAR from queue when a menu gets VK_ESCAPE and add tests

by Dmitry Timoshkov

Fabian Maurer <dark.shadow4(a)web.de> wrote: > +/* Test handling of VK_ESCAPE to close the menu */ > +static LRESULT WINAPI esc_test_wnd_proc(HWND hwnd, UINT msg, WPARAM wparam, LPARAM lparam) > +{ > + if(msg == WM_SHOWWINDOW) > + { > + PostMessageW(hwnd, WM_KEYDOWN, VK_ESCAPE, 0); > + HMENU hPopupMenu = CreatePopupMenu(); > + InsertMenuA(hPopupMenu, 0, MF_STRING, 10, "test"); > + TrackPopupMenu(hPopupMenu, 0, 0, 0, 0, hwnd, NULL); > + PostQuitMessage(0); > + } > + > + if(msg == WM_CHAR || msg == WM_KEYDOWN) > + ok(0, "Wrong VK_ESCAPE press detected.\n"); > + > + return DefWindowProcA(hwnd, msg, wparam, lparam); > +} Please add a test to msg.c instead. This should be a normal TrackPopupMenu test not wrapped inside of a parent's WM_SHOWWINDOW message handler. Once there is a message test, and the underlying behaviour is well understood a patch with a fix could be sent separately. -- Dmitry.

Jan. 24, 2017

Re: [PATCH] gdi32: Add traces for painting functions

by Fabian Maurer

On Tuesday, January 24, 2017 9:02:34 AM CET Huw Davies wrote: > I would suggest printing the point-like terms in the same way as > wine_dbgstr_point() and the rect-like terms in the same way as > wine_dbgstr_rect(). Thanks for the feedback, I sent an improved version.

Jan. 24, 2017

Re: Code freeze

by Austin English

On Tue, Jan 24, 2017 at 6:09 PM, Fabian Maurer <dark.shadow4(a)web.de> wrote: > Hello, > > Now that wine 2.0 is released, does this mark the end of code-freeze or should > we still wait before sending in patches like normally? > > Regards, > Fabian The floodgates are open, you can send at will. Note though that there may be a flood, which may mean slower reviews/things getting lost in the noise. -- -Austin GPG: 14FB D7EA A041 937B

Jan. 24, 2017

Code freeze

by Fabian Maurer

Hello, Now that wine 2.0 is released, does this mark the end of code-freeze or should we still wait before sending in patches like normally? Regards, Fabian

Jan. 24, 2017

Re: A couple unrelated issues

by Matteo Bruni

2017-01-23 16:26 GMT+01:00 Rafał Mużyło <galtgendo(a)gmail.com>: > 1. regedit and long REG_BINARY values: the edit control should either be > made resizable or scrollable, cause long values just don't fit in the > box; as a temporary solution, I've made a hackish patch, that changes > the font the control uses (not sure if that's necesarry, but it's just > feels more natural to use monospaced font here), makes sure all glyphs > get the same space and once the value wraps, if it's a non-integer > divide, the last line is correctly placed; I guess this is worthy of a bug report. > 3. err:ntdll:RtlpWaitForCriticalSection wait timed out in thread <nr>, blocked by <nr>, retrying (60 sec): this is an annoyance, sometimes a major one; namely, it happens that I terminate an app by pressing ^C; usually, that works just fine, but every now and then app goes into that unterminatable state and not for those 60 sec (which is horribly long anyway), but pretty much indefinitelly; it really shouldn't do that; this requires a taskmgr or wineboot intervention to close it You shouldn't terminate applications like that. You might catch the application in the middle of "something", inside some locked section or similar. There is no way for Wine to fix things afterwards in general. > 4. (somewhat related to the previous) taskmgr could use a little > enhancement: when running several apps with the same executable name, > it's very hard to tell (impossible ?) how a line from process tab > relates to application tab; usually it's not much of a problem, but if > point 3 strikes, there's no more entry in the app tab, but the process > is still there and you need to guess, which will often be wrong; perhaps > the entries in the process tab should print the app name or have an > option in the context menu to retrieve it ? This also could be an enhancement bug report.

Jan. 24, 2017

Re: [PATCH] shell32: Implement knownfolder_GetRedirectionCapabilities somewhat

by Farmboy0

On 23.01.2017 23:59, Nikolay Sivov wrote: > On 23.01.2017 23:10, Enrico Horn wrote: >> This is the one thing missing to get the config app for Temple+, a >> Temple of Elemental Evil Mod, starting. >> --- >> dlls/shell32/shellpath.c | 5 +++-- >> 1 file changed, 3 insertions(+), 2 deletions(-) >> >> diff --git a/dlls/shell32/shellpath.c b/dlls/shell32/shellpath.c >> index 92306239db7..3c44ecb393a 100644 >> --- a/dlls/shell32/shellpath.c >> +++ b/dlls/shell32/shellpath.c >> @@ -5396,8 +5396,9 @@ static HRESULT WINAPI knownfolder_GetRedirectionCapabilities( >> IKnownFolder *iface, >> KF_REDIRECTION_CAPABILITIES *pCapabilities) >> { >> - FIXME("%p\n", pCapabilities); >> - return E_NOTIMPL; >> + FIXME("%p - always denied\n", pCapabilities); >> + *pCapabilities = KF_REDIRECTION_CAPABILITIES_DENY_ALL; >> + return S_OK; >> } >> >> static HRESULT WINAPI knownfolder_GetFolderDefinition( >> > Do we have an open bug for that? If it's just a matter of returning > static data, it's trivial to fix without any hacks. There is no bug for this. I was just trying to get that app to run. If you have a better way to solve this I can test it.

Jan. 24, 2017

A couple unrelated issues

by Rafał Mużyło

On Tue, Jan 24, 2017 at 08:42:12AM -0600, Andrew Eikum wrote: > On Mon, Jan 23, 2017 at 04:26:14PM +0100, Rafał Mużyło wrote: > > 2. alsa/pulseaudio and stream recovery: as in the past, it's hard to > > tell if it's wine or the underlaying stuff that fails; the problem I'm > > hearing is that apps sometimes without any apparent reason (as CPU usage > > isn't all that different than the average) go into '(snd_pcm_recover) > > underrun occurred' mode and start playing the sounds on much higher rate > > and then just as randomly (as it's again seemingly not related to CPU > > usage) recover; it's the randomness that bothers me: both going into the > > state *and* not recovering for several minutes; would it be possible, if > > not outright fix that, then for example add a helper app, that the user > > could manually trigger to force recovery ? > > > > Are you using PulseAudio via winealsa with the alsa-pulse plugin? Why > not just use the winepulse driver? > Well, because reasons... OK, a less tongue-in-cheek answer: while I've been working under pulseaudio for quite awhile (if you recall my older posts/bugs), I'm still of a strong opinion, that barring very specific needs (like audio authoring), anything that works with pure alsa, should just as well work with pulseaudio underneath, unless there's a bug in alsa's pulseaudio plugin or pulseaudio itself - then it's that bug that needs to be adressed (well, at least in the long term). What's more the random nature of the problem (both triggering it - sometimes things work for hours just fine - and getting cleaned up - again, sometimes it gets cleaned up fast enough it's barely noticable; coupled with the trigger *seemingly* not being tied to actual CPU usage, just happening on a whim...) irks me.

Jan. 24, 2017

RFC: Flushing the test reports

by Francois Gouget

When a test causes Windows to crash the test output is often lost, even if it was redirected to a file : said file is often either 0-byte long or is missing a huge chunk. This is particularly annoying when the crash happens as part of WineTest as then one has to manually run a bunch of tests to try and find which one caused the issue. And it's not always which test caused the crash as shown by the rasapi32:rasapi case (bug 42185). So I have tested a couple of approaches to better pinpoint which test is causing trouble, and possibly even which chunk of code in that test causes the crash. The first approach is simple: flush the WineTest report to disk between each test (in Unix terms sync). It has little impact on performance and in case of a crash the last 'dll:test start' line would clearly identify the test responsible. diff --git a/programs/winetest/main.c b/programs/winetest/main.c index 50ebf4fd52a..df527734bb8 100644 --- a/programs/winetest/main.c +++ b/programs/winetest/main.c @@ -626,6 +626,11 @@ run_ex (char *cmd, HANDLE out_file, const char *tempdir, DWORD ms) PROCESS_INFORMATION pi; DWORD wait, status; + /* Flush to disk so we know which test caused Windows to crash if it does */ + if (out_file) + FlushFileBuffers(out_file); + FlushFileBuffers(GetStdHandle( STD_ERROR_HANDLE )); + GetStartupInfoA (&si); si.dwFlags = STARTF_USESTDHANDLES; si.hStdInput = GetStdHandle( STD_INPUT_HANDLE ); (the second flush is so WineTest's output too is flushed to disk in case stdout is redirected to a file though I did not really check that this part works) Another approach is to add these calls in wine/test.h. This narrows the location of the Windows crash down to an ok()/trace() interval, even more narrow if WINETEST_REPORT_SUCCESS is set. However this has quite an impact on performance: * WineTest normally takes about 19 minutes. * With the extra flushing this goes up to 29 minutes. * With no flushing but report success I get 45 minutes. * With both flushing and report success this shoots up to 315+ minutes. The report success cases can be ignored but even in the normal case the extra 10 minutes are probably not acceptable. So I set the flushing code behind a $WINETEST_FLUSH check. So once you've identified that, for instance, rasapi32:rasapi is causing trouble, you can run it on its own with report success and flushing to pinpoint where it goes wrong. Of course on the TestBot you still won't get the report (until the TestBot is changed appropriately), and you'll have to contend with the 2 minutes timeout but even with those options 88% of the tests completed within the time limit on my machine. That approach could be done with the patch below. commit ef211f6480fe84fd7660314c27bcaa5630dd0c16 Author: Francois Gouget <fgouget(a)codeweavers.com> Date: Sun Jan 22 02:57:23 2017 +0100 tests: Flush after each message if WINETEST_FLUSH is set. Signed-off-by: Francois Gouget <fgouget(a)codeweavers.com> diff --git a/include/wine/test.h b/include/wine/test.h index aa89c55ca07..149b0a9ae14 100644 --- a/include/wine/test.h +++ b/include/wine/test.h @@ -52,6 +52,9 @@ extern int winetest_debug; /* running in interactive mode? */ extern int winetest_interactive; +/* flush after each trace */ +extern int winetest_flush; + /* current platform */ extern const char *winetest_platform; @@ -199,6 +202,9 @@ int winetest_debug = 1; /* interactive mode? */ int winetest_interactive = 0; +/* flush to disk after every message? */ +int winetest_flush = 0; + /* current platform */ const char *winetest_platform = "windows"; @@ -291,6 +297,11 @@ int broken( int condition ) { return (strcmp(winetest_platform, "windows") == 0) && condition; } +static void winetest_do_flush( void ) +{ + if (winetest_flush) + FlushFileBuffers(GetStdHandle( STD_ERROR_HANDLE )); +} /* * Checks condition. @@ -313,6 +324,7 @@ int winetest_vok( int condition, const char *msg, __winetest_va_list args ) printf( "%s:%d: Test succeeded inside todo block: ", data->current_file, data->current_line ); vprintf(msg, args); + winetest_do_flush(); InterlockedIncrement(&todo_failures); return 0; } @@ -323,6 +335,7 @@ int winetest_vok( int condition, const char *msg, __winetest_va_list args ) printf( "%s:%d: Test marked todo: ", data->current_file, data->current_line ); vprintf(msg, args); + winetest_do_flush(); } InterlockedIncrement(&todo_successes); return 1; @@ -335,14 +348,18 @@ int winetest_vok( int condition, const char *msg, __winetest_va_list args ) printf( "%s:%d: Test failed: ", data->current_file, data->current_line ); vprintf(msg, args); + winetest_do_flush(); InterlockedIncrement(&failures); return 0; } else { if (report_success) + { printf( "%s:%d: Test succeeded\n", data->current_file, data->current_line); + winetest_do_flush(); + } InterlockedIncrement(&successes); return 1; } @@ -369,6 +386,7 @@ void __winetest_cdecl winetest_trace( const char *msg, ... ) __winetest_va_start(valist, msg); vprintf(msg, valist); __winetest_va_end(valist); + winetest_do_flush(); } } @@ -378,6 +396,7 @@ void winetest_vskip( const char *msg, __winetest_va_list args ) printf( "%s:%d: Tests skipped: ", data->current_file, data->current_line ); vprintf(msg, args); + winetest_do_flush(); skipped++; } @@ -443,7 +462,10 @@ void winetest_wait_child_process( HANDLE process ) DWORD exit_code = 1; if (WaitForSingleObject( process, 30000 )) + { printf( "%s: child process wait failed\n", current_test->name ); + winetest_do_flush(); + } else GetExitCodeProcess( process, &exit_code ); @@ -452,12 +474,14 @@ void winetest_wait_child_process( HANDLE process ) if (exit_code > 255) { printf( "%s: exception 0x%08x in child process\n", current_test->name, exit_code ); + winetest_do_flush(); InterlockedIncrement( &failures ); } else { printf( "%s: %u failures in child process\n", current_test->name, exit_code ); + winetest_do_flush(); while (exit_code-- > 0) InterlockedIncrement(&failures); } @@ -599,6 +623,7 @@ static int run_test( const char *name ) todo_successes, failures + todo_failures, (failures + todo_failures != 1) ? "failures" : "failure", skipped ); + winetest_do_flush(); } status = (failures + todo_failures < 255) ? failures + todo_failures : 255; return status; @@ -624,6 +649,7 @@ static LONG CALLBACK exc_filter( EXCEPTION_POINTERS *ptrs ) printf( "%s: unhandled exception %08x at %p\n", current_test->name, ptrs->ExceptionRecord->ExceptionCode, ptrs->ExceptionRecord->ExceptionAddress ); fflush( stdout ); + winetest_do_flush(); return EXCEPTION_EXECUTE_HANDLER; } @@ -660,6 +686,7 @@ int main( int argc, char **argv ) if (GetEnvironmentVariableA( "WINETEST_IS_CHILD", p, sizeof(p) )) winetest_is_child = atoi(p); /* Identify any future process as a child of this one */ SetEnvironmentVariableA( "WINETEST_IS_CHILD", "1" ); + if (GetEnvironmentVariableA( "WINETEST_FLUSH", p, sizeof(p) )) winetest_flush = atoi(p); if (!strcmp( winetest_platform, "windows" )) SetUnhandledExceptionFilter( exc_filter ); if (!winetest_interactive) SetErrorMode( SEM_FAILCRITICALERRORS | SEM_NOGPFAULTERRORBOX ); Both approaches could be combined. Comments? Suggestions? -- Francois Gouget <fgouget(a)codeweavers.com>

Jan. 24, 2017

Re: [PATCH] winegstreamer: Avoid zero division in amt_from_gst_caps_video().

by Andrew Eikum

Hi Masanori, Thank you for working on this! We're late in code freeze right now, can you please re-submit this patch after Wine 2.0 is released? Also, it would be better to use the MulDiv function to avoid integer overflow: vih->AvgTimePerFrame = (nom == 0) ? 0 : MulDiv(10000000, denom, nom); Andrew On Mon, Jan 23, 2017 at 10:29:19PM +0900, Masanori Kakura wrote: > Fixes https://bugs.winehq.org/show_bug.cgi?id=42212 > > Signed-off-by: Masanori Kakura <kakurasan(a)gmail.com> > --- > dlls/winegstreamer/gstdemux.c | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) > > diff --git a/dlls/winegstreamer/gstdemux.c b/dlls/winegstreamer/gstdemux.c > index c33ed9e..14ee584 100644 > --- a/dlls/winegstreamer/gstdemux.c > +++ b/dlls/winegstreamer/gstdemux.c > @@ -233,9 +233,7 @@ static gboolean amt_from_gst_caps_video(GstCaps *caps, AM_MEDIA_TYPE *amt) > bih->biCompression = amt->subtype.Data1; > } > bih->biSizeImage = width * height * bih->biBitCount / 8; > - vih->AvgTimePerFrame = 10000000; > - vih->AvgTimePerFrame *= denom; > - vih->AvgTimePerFrame /= nom; > + vih->AvgTimePerFrame = (nom == 0) ? 0 : 10000000 * denom / nom; > vih->rcSource.left = 0; > vih->rcSource.right = width; > vih->rcSource.top = height; > -- > 2.9.3 > > >

Jan. 24, 2017

Re: A couple unrelated issues

by Andrew Eikum

On Mon, Jan 23, 2017 at 04:26:14PM +0100, Rafał Mużyło wrote: > 2. alsa/pulseaudio and stream recovery: as in the past, it's hard to > tell if it's wine or the underlaying stuff that fails; the problem I'm > hearing is that apps sometimes without any apparent reason (as CPU usage > isn't all that different than the average) go into '(snd_pcm_recover) > underrun occurred' mode and start playing the sounds on much higher rate > and then just as randomly (as it's again seemingly not related to CPU > usage) recover; it's the randomness that bothers me: both going into the > state *and* not recovering for several minutes; would it be possible, if > not outright fix that, then for example add a helper app, that the user > could manually trigger to force recovery ? > Are you using PulseAudio via winealsa with the alsa-pulse plugin? Why not just use the winepulse driver? Andrew

Jan. 24, 2017

Re: dinput: Silence some effects feedback in linuxinput

by Bruno Jesus

On Tue, Jan 24, 2017 at 12:35 PM, Andrew Eikum <aeikum(a)codeweavers.com> wrote: > Instead of adding new cases, would you be opposed to just changing the > FIXME into a TRACE or maybe a WARN? I think that would scale better, > and users are likely only interested if they're doing dinput > development anyway. OK, will do after 2.0.

Jan. 24, 2017

Re: dinput: Silence some effects feedback in linuxinput

by Andrew Eikum

Instead of adding new cases, would you be opposed to just changing the FIXME into a TRACE or maybe a WARN? I think that would scale better, and users are likely only interested if they're doing dinput development anyway. Andrew

Jan. 24, 2017

Re: [PATCH] gdi32: Add traces for painting functions

by Huw Davies

On Sat, Jan 21, 2017 at 06:45:31PM +0100, Fabian Maurer wrote: > Signed-off-by: Fabian Maurer <dark.shadow4(a)web.de> > --- > dlls/gdi32/painting.c | 56 +++++++++++++++++++++++++++++++++++++++++++++++---- > 1 file changed, 52 insertions(+), 4 deletions(-) I would suggest printing the point-like terms in the same way as wine_dbgstr_point() and the rect-like terms in the same way as wine_dbgstr_rect(). > @@ -237,6 +237,8 @@ BOOL WINAPI LineTo( HDC hdc, INT x, INT y ) > PHYSDEV physdev; > BOOL ret; > > + TRACE( "%p, %d, %d\n", hdc, x, y ); So this would be: TRACE( "%p, (%d,%d)\n", hdc, x, y ); > @@ -288,6 +292,8 @@ BOOL WINAPI Arc( HDC hdc, INT left, INT top, INT right, > BOOL ret; > DC * dc = get_dc_ptr( hdc ); > > + TRACE( "%p, %d, %d, %d, %d, %d, %d, %d, %d\n", hdc, left, top, right, bottom, xstart, ystart, xend, yend ); And this would be: TRACE( "%p, (%d,%d)-(%d,%d), (%d,%d), (%d,%d)\n", hdc, left, top, right, bottom, xstart, ystart, xend, yend ); > @@ -421,6 +437,8 @@ BOOL WINAPI RoundRect( HDC hdc, INT left, INT top, INT right, > BOOL ret; > DC *dc = get_dc_ptr( hdc ); > > + TRACE( "%p, %d, %d, %d, %d, %d, %d\n", hdc, left, top, right, bottom, ell_width, ell_height ); A size-like term could be: TRACE( "%p, (%d,%d)-(%d,%d), %dx%d\n", hdc, left, top, right, bottom, ell_width, ell_height ); > @@ -702,6 +738,8 @@ BOOL WINAPI ExtFloodFill( HDC hdc, INT x, INT y, COLORREF color, > BOOL ret; > DC * dc = get_dc_ptr( hdc ); > > + TRACE( "%p, %d, %d, %x, %x\n", hdc, x, y, color, fillType ); color should be %08x Huw.

Jan. 24, 2017

DirectX 12 in Wine

by Gabríel Arthúr Pétursson

Hi, I am interested on working on getting DirectX 12 support into Wine. Is there anyone currently working towards that goal that I can get in contact with? With that said, what would an appropriate implementation in Wine look like? DirectX 12 on top of Vulkan seems like an obvious choice. The two APIs are very similar, whereas the DirectX 12 programming model deviates significantly from both OpenGL and DirectX 11. Another approach is to leverage wined3d (i.e. OpenGL + extensions). I don't know to what extent wined3d can be used or needs to be changed in order to support DirectX 12, or if it'd be useful at all. Can anyone shed some light on whether this would be a feasible approach? Thanks.

Jan. 24, 2017

Re: [PATCH] shell32: Implement knownfolder_GetRedirectionCapabilities somewhat

by Nikolay Sivov

On 23.01.2017 23:10, Enrico Horn wrote: > This is the one thing missing to get the config app for Temple+, a > Temple of Elemental Evil Mod, starting. > --- > dlls/shell32/shellpath.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/dlls/shell32/shellpath.c b/dlls/shell32/shellpath.c > index 92306239db7..3c44ecb393a 100644 > --- a/dlls/shell32/shellpath.c > +++ b/dlls/shell32/shellpath.c > @@ -5396,8 +5396,9 @@ static HRESULT WINAPI knownfolder_GetRedirectionCapabilities( > IKnownFolder *iface, > KF_REDIRECTION_CAPABILITIES *pCapabilities) > { > - FIXME("%p\n", pCapabilities); > - return E_NOTIMPL; > + FIXME("%p - always denied\n", pCapabilities); > + *pCapabilities = KF_REDIRECTION_CAPABILITIES_DENY_ALL; > + return S_OK; > } > > static HRESULT WINAPI knownfolder_GetFolderDefinition( > Do we have an open bug for that? If it's just a matter of returning static data, it's trivial to fix without any hacks.

Jan. 23, 2017

Re: [PATCH] Rename Mac OS X to macOS

by André Hentschel

Am 23.01.2017 um 20:32 schrieb Nikolay Sivov: > On 23.01.2017 22:19, André Hentschel wrote: >> Signed-off-by: André Hentschel <nerv(a)dawncrow.de> >> --- >> en/winedev-multimedia.sgml | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/en/winedev-multimedia.sgml b/en/winedev-multimedia.sgml >> index fa04b04..4c95349 100644 >> --- a/en/winedev-multimedia.sgml >> +++ b/en/winedev-multimedia.sgml >> @@ -330,7 +330,7 @@ Kernel space | Client applications >> </listitem> >> <listitem> >> <para> >> - Core Audio as introduced in Mac OS X 10.3. Source code resides in >> + Core Audio as introduced in macOS 10.3. Source code resides in >> <filename class="libraryfile">dlls/winecoreaudio.drv</filename>. >> </para> >> </listitem> >> > > 10.3 was before macOS renaming, so original sounds better to me. You're right, but I thought I simply unify it. Also, CW does somewhat the same: https://www.codeweavers.com/products/crossover-mac/requirements

Jan. 23, 2017

Re: [PATCH] Rename Mac OS X to macOS

by Nikolay Sivov

On 23.01.2017 22:19, André Hentschel wrote: > Signed-off-by: André Hentschel <nerv(a)dawncrow.de> > --- > en/winedev-multimedia.sgml | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/en/winedev-multimedia.sgml b/en/winedev-multimedia.sgml > index fa04b04..4c95349 100644 > --- a/en/winedev-multimedia.sgml > +++ b/en/winedev-multimedia.sgml > @@ -330,7 +330,7 @@ Kernel space | Client applications > </listitem> > <listitem> > <para> > - Core Audio as introduced in Mac OS X 10.3. Source code resides in > + Core Audio as introduced in macOS 10.3. Source code resides in > <filename class="libraryfile">dlls/winecoreaudio.drv</filename>. > </para> > </listitem> > 10.3 was before macOS renaming, so original sounds better to me.

Jan. 23, 2017

Re: [PATCH] programs/winhlp32: Use noyywrap for macro.lex.l

by Alexandre Julliard

Fabian Maurer <dark.shadow4(a)web.de> writes: > On Sunday, January 22, 2017 9:22:46 PM CET Mike Swanson wrote: >> I looked into this, and the source is related to why I made the patch in >> the first place: it seems that flex 2.6.3 just has a bug, and I don't >> think I can reasonably fix it so the warning doesn't appear. >> >> That being said, I believe my patch is still functionally equivalent to >> the old code and having this alternative that works with flex 2.6.3 >> should still be safe for older and newer (hopefully fixed) versions. > > Ah thanks, it indeed is a flex bug. > However, it works for the other flex uses we have in wine. You could make the > warning disappear by using >> %option prefix="winhlp32_" > or something along the lines. It will rename the output files (not sure how > you'd deal with that), but it makes the warning go away. Also, that's the way > it's done in the other .l files. FWIW the initial flex bug is now fixed by https://github.com/westes/flex/commit/078b46c69d063aef1715b11348a2871d6036f… -- Alexandre Julliard julliard(a)winehq.org

Jan. 23, 2017

Re: [PATCH] programs/winhlp32: Use noyywrap for macro.lex.l

by Fabian Maurer

On Monday, January 23, 2017 5:44:53 PM CET Fabian Maurer wrote: > It will rename the output files (not sure how > you'd deal with that), but it makes the warning go away Sorry, forget that, in wine the output is set explicit, so it doesn't change the filename at all. Just changes internal function names. Regards, Fabian

Jan. 23, 2017

Re: [PATCH] programs/winhlp32: Use noyywrap for macro.lex.l

by Fabian Maurer

On Sunday, January 22, 2017 9:22:46 PM CET Mike Swanson wrote: > I looked into this, and the source is related to why I made the patch in > the first place: it seems that flex 2.6.3 just has a bug, and I don't > think I can reasonably fix it so the warning doesn't appear. > > That being said, I believe my patch is still functionally equivalent to > the old code and having this alternative that works with flex 2.6.3 > should still be safe for older and newer (hopefully fixed) versions. Ah thanks, it indeed is a flex bug. However, it works for the other flex uses we have in wine. You could make the warning disappear by using > %option prefix="winhlp32_" or something along the lines. It will rename the output files (not sure how you'd deal with that), but it makes the warning go away. Also, that's the way it's done in the other .l files. Regards, Fabian

Jan. 23, 2017

A couple unrelated issues

by Rafał Mużyło

Just as the title says, I've got a couple problems/questions that don't have much in common except for "happening in wine". 1. regedit and long REG_BINARY values: the edit control should either be made resizable or scrollable, cause long values just don't fit in the box; as a temporary solution, I've made a hackish patch, that changes the font the control uses (not sure if that's necesarry, but it's just feels more natural to use monospaced font here), makes sure all glyphs get the same space and once the value wraps, if it's a non-integer divide, the last line is correctly placed; 2. alsa/pulseaudio and stream recovery: as in the past, it's hard to tell if it's wine or the underlaying stuff that fails; the problem I'm hearing is that apps sometimes without any apparent reason (as CPU usage isn't all that different than the average) go into '(snd_pcm_recover) underrun occurred' mode and start playing the sounds on much higher rate and then just as randomly (as it's again seemingly not related to CPU usage) recover; it's the randomness that bothers me: both going into the state *and* not recovering for several minutes; would it be possible, if not outright fix that, then for example add a helper app, that the user could manually trigger to force recovery ? 3. err:ntdll:RtlpWaitForCriticalSection wait timed out in thread <nr>, blocked by <nr>, retrying (60 sec): this is an annoyance, sometimes a major one; namely, it happens that I terminate an app by pressing ^C; usually, that works just fine, but every now and then app goes into that unterminatable state and not for those 60 sec (which is horribly long anyway), but pretty much indefinitelly; it really shouldn't do that; this requires a taskmgr or wineboot intervention to close it 4. (somewhat related to the previous) taskmgr could use a little enhancement: when running several apps with the same executable name, it's very hard to tell (impossible ?) how a line from process tab relates to application tab; usually it's not much of a problem, but if point 3 strikes, there's no more entry in the app tab, but the process is still there and you need to guess, which will often be wrong; perhaps the entries in the process tab should print the app name or have an option in the context menu to retrieve it ?

Jan. 23, 2017

Re: [PATCH] ws2_32/tests: Fix memcmp size in test_TransmitFile().

by Erich E. Hoover

On Sun, Jan 22, 2017 at 10:34 AM, Bruno Jesus <00cpxxx(a)gmail.com> wrote: > ... > Hi, you also have to change the sending lengths because it is also > using +1. I believe this was a strlen/sizeof misfortune, right? Yes, originally this was using strlen() and I wanted to capture the NULL byte so that it was easy to look at the data for diagnostics. At some point I must have changed it and forgot to remove the "+1", sorry about that :/ Best, Erich

Jan. 23, 2017

Re: [PATCH] programs/winhlp32: Use noyywrap for macro.lex.l

by Dmitry Timoshkov

Fabian Maurer <dark.shadow4(a)web.de> wrote: > I applied the patch and it compiles now, but I get a warning: > > macro.lex.yy.c:398:0: warning: "yywrap" redefined > > macro.lex.yy.c:74:0: note: this is the location of the previous definition > > You probably want to get rid of that, too. That's a flex bug: https://github.com/westes/flex/issues/155 -- Dmitry.

Jan. 22, 2017

Re: [PATCH] programs/winhlp32: Use noyywrap for macro.lex.l

by Mike Swanson

On Mon, 2017-01-23 at 05:31 +0100, Fabian Maurer wrote: > Hello, > > I applied the patch and it compiles now, but I get a warning: > > macro.lex.yy.c:398:0: warning: "yywrap" redefined > > macro.lex.yy.c:74:0: note: this is the location of the previous > > definition > > You probably want to get rid of that, too. > > Regards, > Fabian I looked into this, and the source is related to why I made the patch in the first place: it seems that flex 2.6.3 just has a bug, and I don't think I can reasonably fix it so the warning doesn't appear. That being said, I believe my patch is still functionally equivalent to the old code and having this alternative that works with flex 2.6.3 should still be safe for older and newer (hopefully fixed) versions. Thanks, Mike

Jan. 22, 2017

Re: [PATCH] programs/winhlp32: Use noyywrap for macro.lex.l

by Fabian Maurer

On Sunday, January 22, 2017 11:00:26 AM CET Mike Swanson wrote: > diff --git a/programs/winhlp32/macro.lex.l b/programs/winhlp32/macro.lex.l > index 8f6945ca98..ff8a4832d3 100644 > --- a/programs/winhlp32/macro.lex.l > +++ b/programs/winhlp32/macro.lex.l > @@ -20,7 +20,7 @@ > * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, > USA */ > %} > -%option noinput nounput never-interactive 8bit > +%option noinput nounput noyywrap never-interactive 8bit > %x quote > %{ > #include "config.h" > @@ -367,7 +367,3 @@ WINHELP_WINDOW* MACRO_CurrentWindow(void) > { > return lex_data ? lex_data->window : Globals.active_win; > } > - > -#ifndef yywrap > -int yywrap(void) { return 1; } > -#endif > -- > 2.11.0 Hello, I applied the patch and it compiles now, but I get a warning: > macro.lex.yy.c:398:0: warning: "yywrap" redefined > macro.lex.yy.c:74:0: note: this is the location of the previous definition You probably want to get rid of that, too. Regards, Fabian

Jan. 22, 2017

Re: [PATCH] ws2_32/tests: Fix memcmp size in test_TransmitFile().

by Zebediah Figura

On 01/22/2017 11:34 AM, Bruno Jesus wrote: > On Fri, Jan 20, 2017 at 5:54 PM, Zebediah Figura <z.figura12(a)gmail.com> wrote: >> Fixes https://bugs.winehq.org/show_bug.cgi?id=42221 >> >> Signed-off-by: Zebediah Figura <z.figura12(a)gmail.com> >> --- >> dlls/ws2_32/tests/sock.c | 12 ++++++------ >> 1 file changed, 6 insertions(+), 6 deletions(-) >> >> diff --git a/dlls/ws2_32/tests/sock.c b/dlls/ws2_32/tests/sock.c >> index 5856718..3c3e443 100644 >> --- a/dlls/ws2_32/tests/sock.c >> +++ b/dlls/ws2_32/tests/sock.c >> @@ -8304,9 +8304,9 @@ static void test_TransmitFile(void) >> iret = recv(dest, buf, sizeof(buf), 0); >> ok(iret == sizeof(header_msg)+sizeof(footer_msg)+2, >> "Returned an unexpected buffer from TransmitFile: %d\n", iret ); >> - ok(memcmp(&buf[0], &header_msg[0], sizeof(header_msg)+1) == 0, >> + ok(memcmp(&buf[0], &header_msg[0], sizeof(header_msg)) == 0, >> "TransmitFile header buffer did not match!\n"); >> - ok(memcmp(&buf[sizeof(header_msg)+1], &footer_msg[0], sizeof(footer_msg)+1) == 0, >> + ok(memcmp(&buf[sizeof(header_msg)+1], &footer_msg[0], sizeof(footer_msg)) == 0, >> "TransmitFile footer buffer did not match!\n"); > > Hi, you also have to change the sending lengths because it is also > using +1. I believe this was a strlen/sizeof misfortune, right? > > Best wishes, > Bruno > > Whoops, thanks for the pointer. (And I can only assume so; the code isn't mine.) --Zeb

Jan. 22, 2017

Re: [PATCH] ws2_32/tests: Fix memcmp size in test_TransmitFile().

by Zebediah Figura

On 01/22/2017 11:34 AM, Bruno Jesus wrote: > On Fri, Jan 20, 2017 at 5:54 PM, Zebediah Figura <z.figura12(a)gmail.com> wrote: >> Fixes https://bugs.winehq.org/show_bug.cgi?id=42221 >> >> Signed-off-by: Zebediah Figura <z.figura12(a)gmail.com> >> --- >> dlls/ws2_32/tests/sock.c | 12 ++++++------ >> 1 file changed, 6 insertions(+), 6 deletions(-) >> >> diff --git a/dlls/ws2_32/tests/sock.c b/dlls/ws2_32/tests/sock.c >> index 5856718..3c3e443 100644 >> --- a/dlls/ws2_32/tests/sock.c >> +++ b/dlls/ws2_32/tests/sock.c >> @@ -8304,9 +8304,9 @@ static void test_TransmitFile(void) >> iret = recv(dest, buf, sizeof(buf), 0); >> ok(iret == sizeof(header_msg)+sizeof(footer_msg)+2, >> "Returned an unexpected buffer from TransmitFile: %d\n", iret ); >> - ok(memcmp(&buf[0], &header_msg[0], sizeof(header_msg)+1) == 0, >> + ok(memcmp(&buf[0], &header_msg[0], sizeof(header_msg)) == 0, >> "TransmitFile header buffer did not match!\n"); >> - ok(memcmp(&buf[sizeof(header_msg)+1], &footer_msg[0], sizeof(footer_msg)+1) == 0, >> + ok(memcmp(&buf[sizeof(header_msg)+1], &footer_msg[0], sizeof(footer_msg)) == 0, >> "TransmitFile footer buffer did not match!\n"); > > Hi, you also have to change the sending lengths because it is also > using +1. I believe this was a strlen/sizeof misfortune, right? > > Best wishes, > Bruno > > Whoops, thanks for the pointer. (And I can only assume so; the code isn't mine.) --Zeb

Jan. 22, 2017

Re: [PATCH] ws2_32/tests: Fix memcmp size in test_TransmitFile().

by Zebediah Figura

On 01/22/2017 11:34 AM, Bruno Jesus wrote: > On Fri, Jan 20, 2017 at 5:54 PM, Zebediah Figura <z.figura12(a)gmail.com> wrote: >> Fixes https://bugs.winehq.org/show_bug.cgi?id=42221 >> >> Signed-off-by: Zebediah Figura <z.figura12(a)gmail.com> >> --- >> dlls/ws2_32/tests/sock.c | 12 ++++++------ >> 1 file changed, 6 insertions(+), 6 deletions(-) >> >> diff --git a/dlls/ws2_32/tests/sock.c b/dlls/ws2_32/tests/sock.c >> index 5856718..3c3e443 100644 >> --- a/dlls/ws2_32/tests/sock.c >> +++ b/dlls/ws2_32/tests/sock.c >> @@ -8304,9 +8304,9 @@ static void test_TransmitFile(void) >> iret = recv(dest, buf, sizeof(buf), 0); >> ok(iret == sizeof(header_msg)+sizeof(footer_msg)+2, >> "Returned an unexpected buffer from TransmitFile: %d\n", iret ); >> - ok(memcmp(&buf[0], &header_msg[0], sizeof(header_msg)+1) == 0, >> + ok(memcmp(&buf[0], &header_msg[0], sizeof(header_msg)) == 0, >> "TransmitFile header buffer did not match!\n"); >> - ok(memcmp(&buf[sizeof(header_msg)+1], &footer_msg[0], sizeof(footer_msg)+1) == 0, >> + ok(memcmp(&buf[sizeof(header_msg)+1], &footer_msg[0], sizeof(footer_msg)) == 0, >> "TransmitFile footer buffer did not match!\n"); > > Hi, you also have to change the sending lengths because it is also > using +1. I believe this was a strlen/sizeof misfortune, right? > > Best wishes, > Bruno > > Whoops, thanks for the pointer. (And I can only assume so; the code isn't mine.) --Zeb

Jan. 22, 2017

Re: server: Remove dead check in is_cpu_supported().

by Alexandre Julliard

Henri Verbeet <hverbeet(a)gmail.com> writes: > For what it's worth, > > On 21 January 2017 at 16:25, Gerald Pfeifer <gerald(a)pfeifer.com> wrote: >> I am wondering about the following piece of code in server/thread.c: >> >> int is_cpu_supported( enum cpu_type cpu ) >> { >> unsigned int prefix_cpu_mask = get_prefix_cpu_mask(); >> >> if (CPU_FLAG(cpu) && (supported_cpus & prefix_cpu_mask & CPU_FLAG(cpu))) return 1; >> >> that was added as part of >> >> commit 1e78c99388c61d353cf60787eac6415328f54560 >> Author: Alexandre Julliard <julliard(a)winehq.org> >> Date: Fri Nov 22 12:32:48 2013 +0100 >> >> kernel32: Validate the architecture of newly created processes on the server side. >> > It's actually older than that, and seems to have been introduced by > > commit 279defe66e1265eeae3e2a72126c2b2f75e20c99 > Author: Alexandre Julliard <julliard(a)winehq.org> > Date: Fri Apr 3 14:59:12 2009 +0200 > > server: Verify that the client is using a supported CPU type. > >> That means CPU_FLAG(...) is always positive and different from zero >> -- unless its parameter is 32 or higher on a 32-bit architecture >> (and the 1 shifts out). >> > But the definition of CPU_FLAG doesn't really matter for this. If > "CPU_FLAG(cpu)" evaluates to zero, "supported_cpus & prefix_cpu_mask & > CPU_FLAG(cpu)" evaluates to zero as well. Yes, it's clearly redundant. I seem to remember there was a need to avoid a compiler warning, but that may have been from a previous version that didn't use a shift. -- Alexandre Julliard julliard(a)winehq.org

Jan. 22, 2017

Re: server: Remove dead check in is_cpu_supported().

by Henri Verbeet

For what it's worth, On 21 January 2017 at 16:25, Gerald Pfeifer <gerald(a)pfeifer.com> wrote: > I am wondering about the following piece of code in server/thread.c: > > int is_cpu_supported( enum cpu_type cpu ) > { > unsigned int prefix_cpu_mask = get_prefix_cpu_mask(); > > if (CPU_FLAG(cpu) && (supported_cpus & prefix_cpu_mask & CPU_FLAG(cpu))) return 1; > > that was added as part of > > commit 1e78c99388c61d353cf60787eac6415328f54560 > Author: Alexandre Julliard <julliard(a)winehq.org> > Date: Fri Nov 22 12:32:48 2013 +0100 > > kernel32: Validate the architecture of newly created processes on the server side. > It's actually older than that, and seems to have been introduced by commit 279defe66e1265eeae3e2a72126c2b2f75e20c99 Author: Alexandre Julliard <julliard(a)winehq.org> Date: Fri Apr 3 14:59:12 2009 +0200 server: Verify that the client is using a supported CPU type. > That means CPU_FLAG(...) is always positive and different from zero > -- unless its parameter is 32 or higher on a 32-bit architecture > (and the 1 shifts out). > But the definition of CPU_FLAG doesn't really matter for this. If "CPU_FLAG(cpu)" evaluates to zero, "supported_cpus & prefix_cpu_mask & CPU_FLAG(cpu)" evaluates to zero as well.

Jan. 22, 2017

Re: [PATCH] ws2_32/tests: Fix memcmp size in test_TransmitFile().

by Bruno Jesus

On Fri, Jan 20, 2017 at 5:54 PM, Zebediah Figura <z.figura12(a)gmail.com> wrote: > Fixes https://bugs.winehq.org/show_bug.cgi?id=42221 > > Signed-off-by: Zebediah Figura <z.figura12(a)gmail.com> > --- > dlls/ws2_32/tests/sock.c | 12 ++++++------ > 1 file changed, 6 insertions(+), 6 deletions(-) > > diff --git a/dlls/ws2_32/tests/sock.c b/dlls/ws2_32/tests/sock.c > index 5856718..3c3e443 100644 > --- a/dlls/ws2_32/tests/sock.c > +++ b/dlls/ws2_32/tests/sock.c > @@ -8304,9 +8304,9 @@ static void test_TransmitFile(void) > iret = recv(dest, buf, sizeof(buf), 0); > ok(iret == sizeof(header_msg)+sizeof(footer_msg)+2, > "Returned an unexpected buffer from TransmitFile: %d\n", iret ); > - ok(memcmp(&buf[0], &header_msg[0], sizeof(header_msg)+1) == 0, > + ok(memcmp(&buf[0], &header_msg[0], sizeof(header_msg)) == 0, > "TransmitFile header buffer did not match!\n"); > - ok(memcmp(&buf[sizeof(header_msg)+1], &footer_msg[0], sizeof(footer_msg)+1) == 0, > + ok(memcmp(&buf[sizeof(header_msg)+1], &footer_msg[0], sizeof(footer_msg)) == 0, > "TransmitFile footer buffer did not match!\n"); Hi, you also have to change the sending lengths because it is also using +1. I believe this was a strlen/sizeof misfortune, right? Best wishes, Bruno

Jan. 22, 2017

Re: [PATCH] ws2_32/tests: Fix memcmp size in test_TransmitFile().

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 https://testbot.winehq.org/JobDetails.pl?Key=27913 Your paranoid android. === w8 (32 bit sock) === sock.c:7995: Test failed: bytesReturned isn't supposed to be 1 sock.c:8019: Test failed: GetOverlappedResult failed, error 0

Jan. 22, 2017

Re: GSoC Ideas page

by Austin English

On Sat, Jan 21, 2017 at 7:48 AM, André Hentschel <nerv(a)dawncrow.de> wrote: > Hi, > > I just did the GSoC 2017 application for Wine, > please update https://wiki.winehq.org/Summer_Of_Code with your Ideas and mention yourself as possible mentor where appropriate. > > Thanks! I'm not sure if the size/difficulty is appropriate, but improving wine/valgrind compatibility could be a potential project. E.g., https://bugs.kde.org/show_bug.cgi?id=211031, https://bugs.kde.org/show_bug.cgi?id=335563 and others -- -Austin GPG: 14FB D7EA A041 937B

Jan. 21, 2017

GSoC Ideas page

by André Hentschel

Hi, I just did the GSoC 2017 application for Wine, please update https://wiki.winehq.org/Summer_Of_Code with your Ideas and mention yourself as possible mentor where appropriate. Thanks!

Jan. 21, 2017

Re: TestBot news

by Francois Gouget

This week I tweaked the configuration of some VMs. Specifically: * All Windows 7+ VMs now use Spice for 'display'; remote access really and it goes further than just display like VNC, it also includes forwarding the audio to the client and more. But none of this should matter really since nobody connects to the VMs while they are running the tests. Still switching the remoting method from VNC to Spice fixes a bunch of audio tests! If I remember correctly we only had two holdouts: w7u and w8. So we'll see if their test results improve (those of this friday may not reflect the new state 100% yet). * w7pro64 and w864 seem to be doing reasonably well sound-wise: w864 does have one failure in the 64 bit mmdevapi:capture & render but not in the 32 bit ones. The w1064 results are not so rosy but it's Windows 10 so it's still kind of expected. So maybe removing the soundcard from 64 bit VMs won't be necessary? I'll let Hans and Andrew verify. * All Windows 7+ VMs also got a new beta TestAgentd which is no longer run from an iconized cmd window. Instead it simply detaches from the console and thus runs without a window. This is meant to help with some Direct3D tests and I hope Stefan can remind me of the details. * I have also created a standalone testcase for the Windows 10 64 bit crash and reported it to QEMU in the hope they can do something about it: https://bugs.winehq.org/show_bug.cgi?id=40240 https://bugs.launchpad.net/qemu/+bug/1658141 * I also created a standalone testcase for the Spice audio issue mentionned above. I added it to the bug I created 15 months ago in the hope it can help someone reproduce and fix the bug. But given that there's no indication anyone even looked at this bug that's probably a foolish hope. https://bugs.winehq.org/show_bug.cgi?id=39442 https://bugs.launchpad.net/qemu/+bug/1499908 * But I think bugs sometimes get fixed: I tried to reproduce the rasapi32:rasapi Windows 8 64 bit bug and couldn't. That probably means it got fixed by last week's QEMU and Linux kernel upgrades. Yay! https://bugs.winehq.org/show_bug.cgi?id=42185 * Tonight the build VM failed to rebuild Wine: https://testbot.winehq.org/JobDetails.pl?Key=27894 It turns out that some build processes fell victim to the OOM killer! I guess the issue is that 512 MB is not always enough for parallel builds, particularly when one process is building an ~80 MB winetest.exe (though when investigating this I also got the issue with makedep once!). So I splurged and doubled the RAM allocated to the VM. The balloon driver will hopefully prevent that from having too much of a performance impact on snapshot reverts. -- Francois Gouget <fgouget(a)codeweavers.com>

Jan. 20, 2017

authz.dll

by André Hentschel

Hi, while looking through the output of my wip script for detecting wrong spec file entries I came across authz.dll And here we have two suspicious functions: AuthzInitializeContextFromSid and AuthzInitializeContextFromToken Both have a parameter of type "LUID" (a struct, not a pointer) which is defined as: typedef struct _LUID { DWORD LowPart; LONG HighPart; } LUID, *PLUID; The spec-file says this is a long: @ stdcall AuthzInitializeContextFromSid(long ptr long ptr long ptr ptr) @ stdcall AuthzInitializeContextFromToken(long long long ptr long ptr ptr) I think a struct like this deserves two longs ("long long") in the spec file, right?

Jan. 20, 2017

Re: Performance Testing Wine-1.2-rc4 vs Wine-1.8

by Stefan Dösinger

Am 2017-01-17 um 22:55 schrieb Stefan Dösinger: > The one game that jumps out is Trackmania Nations. It apparently > regressed from approx. 32 fps to 24. I will run a bisect in the next > days to pinpoint the patch that caused this. The game itself wasn't updated. I bisected this on my Nvidia laptop and ended up with 9b7ff40d7e2bf1a6bafffe65a94d2381e8fb1c7d. I don't quite trust the result because the difference between good and bad is relatively small on this device (128 vs 132 fps). I'll verify it on the iMac on Sunday and file a proper bug report once I am more confident about the regression SHA1.

Jan. 20, 2017

Re: [Valgrind-developers] Wine and Valgrind / handling SIGILL

by Austin English

On Fri, Jan 20, 2017 at 4:15 AM, Julian Seward <jseward(a)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 GPG: 14FB D7EA A041 937B

Jan. 20, 2017

Wine-2.0-rc5 source code (Cppcheck 1.72)

by Игорь Чебаненко

Wine source code scanning with Cppcheck.

Jan. 20, 2017

Re: [Valgrind-developers] Wine and Valgrind / handling SIGILL

by Julian Seward

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

Jan. 20, 2017

Re: Wine and Valgrind / handling SIGILL

by Austin English

On Thu, Jan 19, 2017 at 9:40 PM, Sebastian Lackner <sebastian(a)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. -- -Austin GPG: 14FB D7EA A041 937B

Jan. 20, 2017

Re: Wine and Valgrind / handling SIGILL

by Sebastian Lackner

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

Jan. 19, 2017

Re: [PATCH 5/5] wined3d: Recognize another Radeon HD 4650 PCI device id.

by Henri Verbeet

On 19 January 2017 at 17:53, Matteo Bruni <matteo.mystral(a)gmail.com> wrote: > I guess in general we want to avoid adding more device IDs when > unnecessary so yeah, probably we don't want this patch... We'll probably have to think about how to handle multiple PCI IDs for the same GPU at some point, but I'd imagine something along the lines of just mapping 0x9498 to CARD_AMD_RADEON_HD4600 there.

Jan. 19, 2017

Re: Wine and Valgrind / handling SIGILL

by Julian Seward

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

Jan. 19, 2017

Re: [PATCH 5/5] wined3d: Recognize another Radeon HD 4650 PCI device id.

by Matteo Bruni

2017-01-19 12:32 GMT+01:00 Henri Verbeet <hverbeet(a)gmail.com>: > On 18 January 2017 at 21:35, Matteo Bruni <mbruni(a)codeweavers.com> wrote: >> Signed-off-by: Matteo Bruni <mbruni(a)codeweavers.com> >> --- >> For bug 34671, with thanks to winetaste(a)gmx.net for the relevant info. >> > This still doesn't make sense to me. In what cases does this make a > difference? Which of those do we care about? Somehow I missed the patch attached to the bug and just read the last comment there before writing this one... The idea is that it should help for recognizing one more PCI device id as a HD 4650 through the MESA_query_renderer extension. But right, failing to find an entry in gpu_description_table[] it should just fallback to the wined3d_guess_card() codepath and that should still work (or at least there isn't any evidence otherwise), reporting a different device ID belonging to the HD 4600 series is unlikely to matter. I guess in general we want to avoid adding more device IDs when unnecessary so yeah, probably we don't want this patch...

Jan. 19, 2017

Re: [PATCH 5/5] wined3d: Recognize another Radeon HD 4650 PCI device id.

by Henri Verbeet

On 18 January 2017 at 21:35, Matteo Bruni <mbruni(a)codeweavers.com> wrote: > Signed-off-by: Matteo Bruni <mbruni(a)codeweavers.com> > --- > For bug 34671, with thanks to winetaste(a)gmx.net for the relevant info. > This still doesn't make sense to me. In what cases does this make a difference? Which of those do we care about? > @@ -2018,8 +2019,8 @@ cards_amd_binary[] = > - {"HD 4670", CARD_AMD_RADEON_HD4600}, /* Radeon RV730 */ > - {"HD 4650", CARD_AMD_RADEON_HD4600}, /* Radeon RV730 */ > + {"HD 4670", CARD_AMD_RADEON_HD4650}, /* Radeon RV730 */ > + {"HD 4650", CARD_AMD_RADEON_HD4650}, /* Radeon RV730 */ This is arbitrary.

Jan. 19, 2017

Re: Performance Testing Wine-1.2-rc4 vs Wine-1.8

by Aaryaman Vasishta

Awesome! These are really useful, and maybe good material for the next wineconf ;) Looking forward to the HD 5700 tests! Cheers, jam On Thu, Jan 19, 2017 at 3:59 AM, Stefan Dösinger <stefandoesinger(a)gmail.com> wrote: > Puk made me aware of a mistake in my subject: The comparison is 2.0 vs > 1.8, not the ancient 1.2 of course. > > Am 2017-01-17 um 22:55 schrieb Stefan Dösinger: > > Hi, > > > > After more than a year of break I ran my Wine benchmarking scripts again > > on one of my two test machines: The iMac5.1 box with a Radeon X1600 GPU. > > > > You can see the results and the comparison to the last regular run here: > > http://openbenchmarking.org/result/1701170-RI-UPDATETES12, > 1512236-GA-WINE1860G98 > > > > The one game that jumps out is Trackmania Nations. It apparently > > regressed from approx. 32 fps to 24. I will run a bisect in the next > > days to pinpoint the patch that caused this. The game itself wasn't > updated. > > > > Other than TMNations there is nothing too surprising. 3DMark2000 and > > 2001 are slightly slower, Half Life 2 (which surprisingly didn't seem to > > be changed by Valve since December 2015) is slightly faster. Unigine > > Heaven, UT2004 and World in Conflict are unchanged. > > > > As you can see in the results I updated the kernel, Mesa and Wine > > separately to see where changes come from (stable interfaces ftw!). The > > oldkernel_oldmesa_oldwine result differs from the 2015 one in the > > general update of the system (KDE Desktop, X server, DDX driver, etc). > > > > Unfortunately this old machine is the less interesting one of my two > > test boxes. I still don't have a monitor for my Radeon HD 5700 Desktop. > > I ordered one a while ago and it should arrive soon. If I have time I'll > > try to run the tests next week. > > > > Cheers, > > Stefan > > > > > > > >

Jan. 19, 2017

Re: [PATCH] setupapi: Stub CM_Open_DevNode_Key and CM_Get_Child

by Nikolay Sivov

On 18.01.2017 22:40, Christoph Brill wrote: > Both are necessary to start Lord of the Rings Online as of January 2017 > > Signed-off-by: Christoph Brill <egore911(a)gmail.com> > --- > dlls/setupapi/setupapi.spec | 4 ++-- > dlls/setupapi/stubs.c | 22 ++++++++++++++++++++++ > 2 files changed, 24 insertions(+), 2 deletions(-) > > diff --git a/dlls/setupapi/setupapi.spec b/dlls/setupapi/setupapi.spec > index 0eb8359ee1..5c57c23431 100644 > --- a/dlls/setupapi/setupapi.spec > +++ b/dlls/setupapi/setupapi.spec > @@ -53,7 +53,7 @@ > @ stub CM_Free_Res_Des > @ stub CM_Free_Res_Des_Ex > @ stub CM_Free_Res_Des_Handle > -@ stub CM_Get_Child > +@ stdcall CM_Get_Child(ptr long long) > @ stub CM_Get_Child_Ex > @ stub CM_Get_Class_Key_NameA > @ stub CM_Get_Class_Key_NameW > @@ -144,7 +144,7 @@ > @ stub CM_Open_Class_KeyW > @ stub CM_Open_Class_Key_ExA > @ stub CM_Open_Class_Key_ExW > -@ stub CM_Open_DevNode_Key > +@ stdcall CM_Open_DevNode_Key(long long long long ptr long) > @ stub CM_Open_DevNode_Key_Ex > @ stub CM_Query_And_Remove_SubTreeA > @ stub CM_Query_And_Remove_SubTreeW > diff --git a/dlls/setupapi/stubs.c b/dlls/setupapi/stubs.c > index 6ecf240c86..1f76b08a3d 100644 > --- a/dlls/setupapi/stubs.c > +++ b/dlls/setupapi/stubs.c > @@ -79,6 +79,28 @@ CONFIGRET WINAPI CM_Disconnect_Machine(HMACHINE handle) > } > > /*********************************************************************** > + * CM_Open_DevNode_Key (SETUPAPI.@) > + */ > +CONFIGRET WINAPI CM_Open_DevNode_Key( > + DEVINST dnDevInst, REGSAM access, ULONG ulHardwareProfile, ULONG /*REGDISPOSITION*/ disposition, > + PHKEY phkDevice, ULONG ulFlags) > +{ > + FIXME("0x%08x 0x%08x 0x%08x 0x%08x %p 0x%08x : stub\n", dnDevInst, access, ulHardwareProfile, > + disposition, phkDevice, ulFlags); > + return CR_SUCCESS; > +} > + > +/*********************************************************************** > + * CM_Get_Child (SETUPAPI.@) > + */ > +CONFIGRET WINAPI CM_Get_Child( > + PDEVINST pdnDevInst, DEVINST dnDevInst, ULONG ulFlags) > +{ > + FIXME("%p 0x%08x 0x%08x: stub\n", pdnDevInst, dnDevInst, ulFlags); > + return CR_SUCCESS; > +} > + Hi, Christoph. Please add prototypes to corresponding public header too. This will also fix REGDISPOSITION case - no reason not to have proper typedef for it. Also please run ./tools/make_specfiles to update dependent dlls.

Jan. 18, 2017

Re: Performance Testing Wine-1.2-rc4 vs Wine-1.8

by Stefan Dösinger

Puk made me aware of a mistake in my subject: The comparison is 2.0 vs 1.8, not the ancient 1.2 of course. Am 2017-01-17 um 22:55 schrieb Stefan Dösinger: > Hi, > > After more than a year of break I ran my Wine benchmarking scripts again > on one of my two test machines: The iMac5.1 box with a Radeon X1600 GPU. > > You can see the results and the comparison to the last regular run here: > http://openbenchmarking.org/result/1701170-RI-UPDATETES12,1512236-GA-WINE18… > > The one game that jumps out is Trackmania Nations. It apparently > regressed from approx. 32 fps to 24. I will run a bisect in the next > days to pinpoint the patch that caused this. The game itself wasn't updated. > > Other than TMNations there is nothing too surprising. 3DMark2000 and > 2001 are slightly slower, Half Life 2 (which surprisingly didn't seem to > be changed by Valve since December 2015) is slightly faster. Unigine > Heaven, UT2004 and World in Conflict are unchanged. > > As you can see in the results I updated the kernel, Mesa and Wine > separately to see where changes come from (stable interfaces ftw!). The > oldkernel_oldmesa_oldwine result differs from the 2015 one in the > general update of the system (KDE Desktop, X server, DDX driver, etc). > > Unfortunately this old machine is the less interesting one of my two > test boxes. I still don't have a monitor for my Radeon HD 5700 Desktop. > I ordered one a while ago and it should arrive soon. If I have time I'll > try to run the tests next week. > > Cheers, > Stefan >

Jan. 18, 2017

Re: iphlpapi/tests: Add test for AllocateAndGetTcpExTableFromStack

by Sebastian Lackner

On 16.01.2017 17:55, André Hentschel wrote: > + ok( ret == ERROR_INVALID_PARAMETER || broken(ERROR_NOT_SUPPORTED) /* win2k */, "got %u\n", ret ); That is not how broken() works.

Jan. 18, 2017

Re: regedit: Select a treeitem on right mouse click

by Hugh McMaster

On Tuesday, 17 January 2017 11:26 PM, Nikolay Sivov wrote: >This looks like a workaround, according to >https://bugs.winehq.org/show_bug.cgi?id=14606 you should be able to >TVM_GETNEXTITEM with TVGN_CARET, as selection should happen automatically. Sure, but not at the moment. It looks like comctl32/treeview.c is not providing the correct Windows right-click behaviour. I'll look into it.

Jan. 18, 2017

Performance Testing Wine-1.2-rc4 vs Wine-1.8

by Stefan Dösinger

Hi, After more than a year of break I ran my Wine benchmarking scripts again on one of my two test machines: The iMac5.1 box with a Radeon X1600 GPU. You can see the results and the comparison to the last regular run here: http://openbenchmarking.org/result/1701170-RI-UPDATETES12,1512236-GA-WINE18… The one game that jumps out is Trackmania Nations. It apparently regressed from approx. 32 fps to 24. I will run a bisect in the next days to pinpoint the patch that caused this. The game itself wasn't updated. Other than TMNations there is nothing too surprising. 3DMark2000 and 2001 are slightly slower, Half Life 2 (which surprisingly didn't seem to be changed by Valve since December 2015) is slightly faster. Unigine Heaven, UT2004 and World in Conflict are unchanged. As you can see in the results I updated the kernel, Mesa and Wine separately to see where changes come from (stable interfaces ftw!). The oldkernel_oldmesa_oldwine result differs from the 2015 one in the general update of the system (KDE Desktop, X server, DDX driver, etc). Unfortunately this old machine is the less interesting one of my two test boxes. I still don't have a monitor for my Radeon HD 5700 Desktop. I ordered one a while ago and it should arrive soon. If I have time I'll try to run the tests next week. Cheers, Stefan

Jan. 17, 2017

Re: [PATCH v2] shlwapi: Fix the return value of SHAddDataBlock

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 https://testbot.winehq.org/JobDetails.pl?Key=27838 Your paranoid android. === build (build) === Patch failed to apply

Jan. 17, 2017

RE: [PATCH] msvcrt: Implement nan

by Daniel Lehman

> Why can't you just "return NAN;" here? I was following the pattern of forwarding for other math functions and the man page for nan But returning NAN is simpler. I'll submit a new patch

Jan. 17, 2017

Re: Wine po issue page broken?

by Michael Stefaniuc

On 01/10/2017 01:31 PM, Michael Stefaniuc wrote: > On 01/10/2017 12:43 PM, Lauri Kenttä wrote: >> On 2017-01-10 13:11, Julian Rüger wrote: >>> http://source.winehq.org/transl/ is broken for me as well. This page is now fixed too. > This page is from the time when Wine used .rc file for translations. > So a lot of the errors that page caught are not possible now with the > new po based translation. As expected it is not very useful and the statistics are skewed too. It would be more useful if it would take into account the po files too. But that's an exercise left for the interested reader... > I wanted to submit now a patch to just remove the transl from > wine-tools. But as that operates on .res file and those are still > generated during the build it can be made to work again fairly easy. > > parse_resfiles.php just needs to be adapted to work with the new > Makefiles. And then we can decide if that is still useful or not. That wasn't the issue but the introduction of the first MENUEX resource in wine-1.9.20-117-g2878d1e. Thanks to Alexandre for pointing me to that and volunteering me to fix it... bye michael

Jan. 17, 2017

Re: regedit: Select a treeitem on right mouse click

by Nikolay Sivov

On 17.01.2017 12:47, Hugh McMaster wrote: > Fixes https://bugs.winehq.org/show_bug.cgi?id=14606 > > Signed-off-by: Hugh McMaster <hugh.mcmaster(a)outlook.com> > --- > programs/regedit/childwnd.c | 13 ++++++++++++- > 1 file changed, 12 insertions(+), 1 deletion(-) > > diff --git a/programs/regedit/childwnd.c b/programs/regedit/childwnd.c > index 08eeec1..c9a35be 100644 > --- a/programs/regedit/childwnd.c > +++ b/programs/regedit/childwnd.c > @@ -411,8 +411,19 @@ LRESULT CALLBACK ChildWndProc(HWND hWnd, UINT message, WPARAM wParam, LPARAM lPa > g_pChildWnd->nFocusPanel = 0; > break; > case NM_RCLICK: { > - POINT pt; > + POINT pt, client_pt; > + TVHITTESTINFO ht; > + HTREEITEM treeitem; > + HWND hTree = ((NMHDR *)lParam)->hwndFrom; > + > GetCursorPos(&pt); > + client_pt = pt; > + ScreenToClient(hWnd, &client_pt); > + ht.pt = client_pt; > + treeitem = (HTREEITEM)SendMessageA(hTree, TVM_HITTEST, 0, (LPARAM)&ht); > + if (!treeitem) return 1; > + SendMessageW(hTree, TVM_SELECTITEM, TVGN_CARET, (LPARAM)treeitem); > + > TrackPopupMenu(GetSubMenu(hPopupMenus, PM_NEW), > TPM_RIGHTBUTTON, pt.x, pt.y, 0, hFrameWnd, NULL); > break; > This looks like a workaround, according to https://bugs.winehq.org/show_bug.cgi?id=14606 you should be able to TVM_GETNEXTITEM with TVGN_CARET, as selection should happen automatically.

Jan. 17, 2017

Re: [PATCH 09/12] d3d9/tets: Remove redundant state setting calls.

by Józef Kucia

On Fri, Jan 13, 2017 at 4:22 PM, Henri Verbeet <hverbeet(a)gmail.com> wrote: > On 13 January 2017 at 13:39, Marvin <testbot(a)winehq.org> wrote: >> === w8 (32 bit visual) === >> visual.c:7986: Test failed: Got unexpected color 0x00007580 for quad 2 (different colors). >> > This doesn't strike me as something caused by this patch, but between > this and the subject typo, I'll give you a chance to take a look. :) > > This seems to be an intermittent test failure.

Jan. 17, 2017

Re: [2/2] iphlpapi: Use the new version of the IP_ADAPTER_UNICAST_ADDRESS structure (try 2)

by Henri Verbeet

On 16 January 2017 at 18:06, André Hentschel <nerv(a)dawncrow.de> wrote: > m = (m & 0m33333333) + (m >> 2 & 0m33333333); I think you may have replaced a few x's too much.

Jan. 16, 2017

Re: [2/2] iphlpapi: Use the new version of the IP_ADAPTER_UNICAST_ADDRESS structure (try 2)

by André Hentschel

Am 13.01.2017 um 16:02 schrieb Henri Verbeet: > On 13 January 2017 at 15:52, André Hentschel <nerv(a)dawncrow.de> wrote: >> Thank you both for the hints. >> One thing is that popcount would count all 1s even with 0s in between, whereas this function stops at the first 0. > Yeah, the strict equivalent would be something along the lines of > "__builtin_clz(~mask)". But as you mentioned, there's the question > what should happen with broken/special masks. > I would suggest to add those functions, any objections? static DWORD mask_v4_to_prefix(DWORD m) { #ifdef HAVE___BUILTIN_POPCOUNT return __builtin_popcount(m); #else m -= m >> 1 & 0m55555555; m = (m & 0m33333333) + (m >> 2 & 0m33333333); return ((m + (m >> 4)) & 0m0f0f0f0f) * 0m01010101 >> 24; #endif } static DWORD mask_v6_to_prefix(SOCKET_ADDRESS *m) { const IN6_ADDR *mask = &((struct WS_sockaddr_in6 *)m->lpSockaddr)->sin6_addr; DWORD ret = 0, i; for (i = 0; i < 8; i++) ret += mask_v4_to_prefix(mask->u.Word[i]); return ret; }

Jan. 16, 2017

Re: [PATCH] msvcrt: Implement nan

by Piotr Caban

On 01/14/17 00:11, Daniel Lehman wrote: > +double CDECL MSVCR120_nan(const char *tagp) > +{ > + /* Windows ignores input (MSDN) and Linux crashes on NULL */ > + const char *dummy = "NAN"; > +#ifdef HAVE_NAN > + return nan(dummy); > +#else > + return strtod(dummy, NULL); > +#endif > +} Why can't you just "return NAN;" here? Thanks, Piotr

Jan. 16, 2017

Re: [PATCH] msvcrt: Trace clean_type_info_names_internal

by Piotr Caban

On 01/12/17 23:57, Jarkko Korpi wrote: > > Signed-off-by: Jarkko Korpi <jarkko_korpi(a)hotmail.com> > > There are only 2 fixmes in the file, all others are traces. And the other fixme is printed only once. This is printed quite often. There are only 2 FIXMEs in the file because other functions are implemented. I don't think this FIXME is very annoying taking into account that it's mainly printed when the application exits. Is it causing any issues for you? I prefer to have it printed unless it breaks application or makes logs unusable. Thanks, Piotr

Jan. 16, 2017

Re: wined3d: Do not set DDCAPS_ALIGNBOUNDARYSRC caps for ddraw.

by Henri Verbeet

On 16 January 2017 at 15:07, Sebastian Lackner <sebastian(a)fds-team.de> wrote: > The ALIGNBOUNDARYSRC flag suddenly appeared in c8901d6f6253f6c97610eb1068ac4ff89758ed0a, > although dwAlignBoundarySrc is not initialized. > It should be set to 0 by ddraw, although I could certainly see that causing issues.

Jan. 16, 2017

Re: Wine and Valgrind / handling SIGILL

by Sebastian Lackner

On 16.01.2017 04:57, Austin English wrote: > On Dec 5, 2016 2:23 AM, "Austin English" <austinenglish(a)gmail.com> wrote: > > Howdy all, > > Several months ago, a patch from Sebastian: > commit 44fbc018eda12bdee5c2c1e2e40dbdc6a81b27fd > Author: Sebastian Lackner <sebastian(a)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 > 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

Jan. 15, 2017

Re: Wine and Valgrind / handling SIGILL

by Austin English

On Dec 5, 2016 2:23 AM, "Austin English" <austinenglish(a)gmail.com> wrote: Howdy all, Several months ago, a patch from Sebastian: commit 44fbc018eda12bdee5c2c1e2e40dbdc6a81b27fd Author: Sebastian Lackner <sebastian(a)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

Jan. 15, 2017

Re: [PATCH v2 1/2] user32: SetSysColors updates brushes from GetSysColorBrush and add test

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 https://testbot.winehq.org/JobDetails.pl?Key=27774 Your paranoid android. === w8 (32 bit sysparams) === sysparams.c:2485: Test failed: Waiting for the WM_DISPLAYCHANGE message timed out sysparams.c:2496: Test failed: Set bpp 32, but WM_DISPLAYCHANGE reported bpp -1 === w864 (32 bit sysparams) === sysparams.c:259: SPI_SETSCREENSAVETIMEOUT failed for reason: 329. Indicating test failure and skipping remainder of test sysparams.c:260: Test failed: SystemParametersInfoA: rc=0 err=329 sysparams.c:260: Test failed: SystemParametersInfoA: rc=0 err=329 sysparams.c:2485: Test failed: Waiting for the WM_DISPLAYCHANGE message timed out sysparams.c:2496: Test failed: Set bpp 32, but WM_DISPLAYCHANGE reported bpp -1

Jan. 14, 2017

Re: [PATCH 1/2] user32: SetSysColors updates brushes from GetSysColorBrush and add test

by Fabian Maurer

On Saturday, January 14, 2017 11:14:31 PM CET Nikolay Sivov wrote: > This is too much. If you only need to update object property while > keeping same handle, you only need one additional call like > __wine_set_gdi_object_color(), that should work for both brushes and > pens, checking if it's a system object too, as an additional precaution. Thanks, that makes sense. > This looks very dirty. Since it's the first member it's fine as long as the structure doesn't change. I wanted to avoid pulling the struct into the header too, but maybe I can rework it. Fabian

Jan. 14, 2017

Re: [PATCH 1/2] user32: SetSysColors updates brushes from GetSysColorBrush and add test

by Nikolay Sivov

On 14.01.2017 22:33, Fabian Maurer wrote: > Signed-off-by: Fabian Maurer <dark.shadow4(a)web.de> > --- > dlls/gdi32/gdi32.spec | 2 ++ > dlls/gdi32/gdiobj.c | 12 ++++++++++++ > dlls/user32/sysparams.c | 23 +++++++++++++++++------ > dlls/user32/tests/sysparams.c | 16 ++++++++++++++++ > include/wine/gdi_driver.h | 2 ++ > 5 files changed, 49 insertions(+), 6 deletions(-) > > diff --git a/dlls/gdi32/gdi32.spec b/dlls/gdi32/gdi32.spec > index 7792731b9e..f9c3fb9293 100644 > --- a/dlls/gdi32/gdi32.spec > +++ b/dlls/gdi32/gdi32.spec > @@ -518,6 +518,8 @@ > # GDI objects > @ cdecl __wine_make_gdi_object_system(long long) > @ cdecl __wine_set_visible_region(long long ptr ptr ptr) > +@ cdecl __wine_GDI_ReleaseObj(ptr) > +@ cdecl __wine_GDI_GetObjPtr(ptr long) > > # Graphics drivers > @ cdecl __wine_set_display_driver(long) > diff --git a/dlls/gdi32/gdiobj.c b/dlls/gdi32/gdiobj.c > index 46b2241945..aa2ced1025 100644 > --- a/dlls/gdi32/gdiobj.c > +++ b/dlls/gdi32/gdiobj.c > @@ -880,6 +880,18 @@ void GDI_ReleaseObj( HGDIOBJ handle ) > LeaveCriticalSection( &gdi_section ); > } > > +/* Exports for user32, we need to be able to manipulate GDI objects where the official API doesn't allow us */ > + > +void* CDECL __wine_GDI_GetObjPtr( HGDIOBJ handle, WORD type ) > +{ > + return GDI_GetObjPtr( handle, type ); > +} > + > +void CDECL __wine_GDI_ReleaseObj( HGDIOBJ handle ) > +{ > + GDI_ReleaseObj( handle ); > +} > + > This is too much. If you only need to update object property while keeping same handle, you only need one additional call like __wine_set_gdi_object_color(), that should work for both brushes and pens, checking if it's a system object too, as an additional precaution. > +static void set_syscolors_gdi( INT index, COLORREF color ) > +{ > + HBRUSH brush = system_colors[index].brush; > + LOGBRUSH *brush_info; > + > + if(brush) > + { > + /* Since LOGBRUSH is the first member in BRUSHOBJ we just do a cast here */ > + brush_info = __wine_GDI_GetObjPtr( brush, OBJ_BRUSH ); > + if(brush_info->lbColor != color) > + brush_info->lbColor = color; > + __wine_GDI_ReleaseObj( brush ); > + } > +} > This looks very dirty.

Jan. 14, 2017

Re: [PATCH 1/2] user32: SetSysColors updates brushes from GetSysColorBrush and add test

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 https://testbot.winehq.org/JobDetails.pl?Key=27773 Your paranoid android. === w8 (32 bit sysparams) === sysparams.c:2485: Test failed: Waiting for the WM_DISPLAYCHANGE message timed out sysparams.c:2496: Test failed: Set bpp 32, but WM_DISPLAYCHANGE reported bpp -1 === w864 (32 bit sysparams) === sysparams.c:259: SPI_SETSCREENSAVETIMEOUT failed for reason: 329. Indicating test failure and skipping remainder of test sysparams.c:260: Test failed: SystemParametersInfoA: rc=0 err=329 sysparams.c:260: Test failed: SystemParametersInfoA: rc=0 err=329 sysparams.c:2485: Test failed: Waiting for the WM_DISPLAYCHANGE message timed out sysparams.c:2496: Test failed: Set bpp 32, but WM_DISPLAYCHANGE reported bpp -1

Jan. 14, 2017

Re: [PATCH 2/3] winemac: Export HTML Format clipboard data to the public.html pasteboard type.

by Alexandre Julliard

Sebastian Lackner <sebastian(a)fds-team.de> writes: > On 13.01.2017 20:36, Ken Thomases wrote: >> Signed-off-by: Ken Thomases <ken(a)codeweavers.com> >> --- >> dlls/winemac.drv/clipboard.c | 68 +++++++++++++++++++++++++++++++++++++++++++- >> 1 file changed, 67 insertions(+), 1 deletion(-) >> >> diff --git a/dlls/winemac.drv/clipboard.c b/dlls/winemac.drv/clipboard.c >> index fdd1110..49a3ed3 100644 >> --- a/dlls/winemac.drv/clipboard.c >> +++ b/dlls/winemac.drv/clipboard.c >> @@ -82,6 +82,7 @@ static CFDataRef export_bitmap_to_bmp(HANDLE data); >> static CFDataRef export_dib_to_bmp(HANDLE data); >> static CFDataRef export_enhmetafile(HANDLE data); >> static CFDataRef export_hdrop_to_filenames(HANDLE data); >> +static CFDataRef export_html(HANDLE data); >> static CFDataRef export_metafilepict(HANDLE data); >> static CFDataRef export_text_to_utf8(HANDLE data); >> static CFDataRef export_unicodetext_to_utf8(HANDLE data); >> @@ -186,7 +187,7 @@ static const struct >> { wszJFIF, CFSTR("public.jpeg"), import_clipboard_data, export_clipboard_data }, >> { wszPNG, CFSTR("public.png"), import_clipboard_data, export_clipboard_data }, >> { wszHTMLFormat, NULL, import_clipboard_data, export_clipboard_data }, >> - { wszHTMLFormat, CFSTR("public.html"), import_html, NULL, TRUE }, >> + { wszHTMLFormat, CFSTR("public.html"), import_html, export_html, TRUE }, >> { CFSTR_SHELLURLW, CFSTR("public.url"), import_utf8_to_text, export_text_to_utf8 }, >> }; >> >> @@ -522,6 +523,28 @@ static HANDLE create_bitmap_from_dib(HANDLE dib) >> >> >> /************************************************************************** >> + * get_html_description_field >> + * >> + * Find the value of a field in an HTML Format description. >> + */ >> +static const char* get_html_description_field(const char* data, const char* keyword) >> +{ >> + const char* pos = data; >> + >> + while (pos && *pos && *pos != '<') >> + { >> + if (memcmp(pos, keyword, strlen(keyword)) == 0) > > Its not safe to use memcmp, it could read past the terminating \0 of data. That's a copy of the X11 code so it's OK for now, but yes, that code needs some work. I was planning to clean it up after code freeze. -- Alexandre Julliard julliard(a)winehq.org

Jan. 13, 2017

Re: [PATCH 2/3] winemac: Export HTML Format clipboard data to the public.html pasteboard type.

by Sebastian Lackner

On 13.01.2017 20:36, Ken Thomases wrote: > Signed-off-by: Ken Thomases <ken(a)codeweavers.com> > --- > dlls/winemac.drv/clipboard.c | 68 +++++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 67 insertions(+), 1 deletion(-) > > diff --git a/dlls/winemac.drv/clipboard.c b/dlls/winemac.drv/clipboard.c > index fdd1110..49a3ed3 100644 > --- a/dlls/winemac.drv/clipboard.c > +++ b/dlls/winemac.drv/clipboard.c > @@ -82,6 +82,7 @@ static CFDataRef export_bitmap_to_bmp(HANDLE data); > static CFDataRef export_dib_to_bmp(HANDLE data); > static CFDataRef export_enhmetafile(HANDLE data); > static CFDataRef export_hdrop_to_filenames(HANDLE data); > +static CFDataRef export_html(HANDLE data); > static CFDataRef export_metafilepict(HANDLE data); > static CFDataRef export_text_to_utf8(HANDLE data); > static CFDataRef export_unicodetext_to_utf8(HANDLE data); > @@ -186,7 +187,7 @@ static const struct > { wszJFIF, CFSTR("public.jpeg"), import_clipboard_data, export_clipboard_data }, > { wszPNG, CFSTR("public.png"), import_clipboard_data, export_clipboard_data }, > { wszHTMLFormat, NULL, import_clipboard_data, export_clipboard_data }, > - { wszHTMLFormat, CFSTR("public.html"), import_html, NULL, TRUE }, > + { wszHTMLFormat, CFSTR("public.html"), import_html, export_html, TRUE }, > { CFSTR_SHELLURLW, CFSTR("public.url"), import_utf8_to_text, export_text_to_utf8 }, > }; > > @@ -522,6 +523,28 @@ static HANDLE create_bitmap_from_dib(HANDLE dib) > > > /************************************************************************** > + * get_html_description_field > + * > + * Find the value of a field in an HTML Format description. > + */ > +static const char* get_html_description_field(const char* data, const char* keyword) > +{ > + const char* pos = data; > + > + while (pos && *pos && *pos != '<') > + { > + if (memcmp(pos, keyword, strlen(keyword)) == 0) Its not safe to use memcmp, it could read past the terminating \0 of data. > + return pos + strlen(keyword); > + > + pos = strchr(pos, '\n'); > + if (pos) pos++; > + } > + > + return NULL; > +} > + > + > +/************************************************************************** > * import_clipboard_data > * > * Generic import clipboard data routine. > @@ -1141,6 +1164,49 @@ done: > > > /************************************************************************** > + * export_html > + * > + * Export HTML Format to public.html data. > + * > + * FIXME: We should attempt to add an <a base> tag and convert windows paths. > + */ > +static CFDataRef export_html(HANDLE handle) > +{ > + CFDataRef ret; > + const char *data, *field_value; > + int fragmentstart, fragmentend; > + > + data = GlobalLock(handle); > + > + /* read the important fields */ > + field_value = get_html_description_field(data, "StartFragment:"); > + if (!field_value) > + { > + ERR("Couldn't find StartFragment value\n"); > + goto failed; > + } > + fragmentstart = atoi(field_value); > + > + field_value = get_html_description_field(data, "EndFragment:"); > + if (!field_value) > + { > + ERR("Couldn't find EndFragment value\n"); > + goto failed; > + } > + fragmentend = atoi(field_value); > + > + /* export only the fragment */ > + ret = CFDataCreate(NULL, (const UInt8*)&data[fragmentstart], fragmentend - fragmentstart); Some validation of those offsets wouldn't hurt. > + GlobalUnlock(handle); > + return ret; > + > +failed: > + GlobalUnlock(handle); > + return NULL; > +} > + > + > +/************************************************************************** > * export_metafilepict > * > * Export a metafile to data. >

Jan. 13, 2017

Re: [PATCH 09/12] d3d9/tets: Remove redundant state setting calls.

by Henri Verbeet

On 13 January 2017 at 13:39, Marvin <testbot(a)winehq.org> wrote: > === w8 (32 bit visual) === > visual.c:7986: Test failed: Got unexpected color 0x00007580 for quad 2 (different colors). > This doesn't strike me as something caused by this patch, but between this and the subject typo, I'll give you a chance to take a look. :)

Jan. 13, 2017

Re: [PATCH 03/12] d3d8: Return D3D_OK from UnlockRect() for not locked textures.

by Henri Verbeet

On 13 January 2017 at 12:53, Józef Kucia <jkucia(a)codeweavers.com> wrote: > Signed-off-by: Józef Kucia <jkucia(a)codeweavers.com> > --- > > Fixes bug 19862 > https://bugs.winehq.org/show_bug.cgi?id=19862 > Strictly speaking that's not a regression, but I'm willing to risk it.

Jan. 13, 2017

Re: [2/2] iphlpapi: Use the new version of the IP_ADAPTER_UNICAST_ADDRESS structure (try 2)

by Henri Verbeet

On 13 January 2017 at 15:52, André Hentschel <nerv(a)dawncrow.de> wrote: > Thank you both for the hints. > One thing is that popcount would count all 1s even with 0s in between, whereas this function stops at the first 0. Yeah, the strict equivalent would be something along the lines of "__builtin_clz(~mask)". But as you mentioned, there's the question what should happen with broken/special masks.

Jan. 13, 2017

Re: [2/2] iphlpapi: Use the new version of the IP_ADAPTER_UNICAST_ADDRESS structure (try 2)

by Hans Leidekker

On Fri, 2017-01-13 at 15:52 +0100, André Hentschel wrote: > >> Please add a helper to compute prefix length instead of duplicating this code. > >> This should probably wait until the code freeze is over. > >> > > For what it's worth, if you're adding a helper function anyway, note > > that you're really just counting the number of trailing zero-bits. GCC > > has __builtin_ctz() for that, but even without that there are better > > approaches than testing each bit individually. Alternatively, there's > > also __builtin_popcount(). > > > > Thank you both for the hints. > One thing is that popcount would count all 1s even with 0s in between, whereas this function stops at the first 0. > Hans, you wrote it that way, there's no checking if the mask is mangled (0s in between), how should it react to mangled masks? I think we should just trust the OS to give us a valid mask.

Jan. 13, 2017

Re: [2/2] iphlpapi: Use the new version of the IP_ADAPTER_UNICAST_ADDRESS structure (try 2)

by André Hentschel

Am 13.01.2017 um 15:34 schrieb Henri Verbeet: > On 13 January 2017 at 14:43, Hans Leidekker <hans(a)codeweavers.com> wrote: >> On Fri, 2017-01-13 at 13:27 +0100, André Hentschel wrote: >>> diff --git a/dlls/iphlpapi/iphlpapi_main.c b/dlls/iphlpapi/iphlpapi_main.c >>> index 54f7a37..223d980 100644 >>> --- a/dlls/iphlpapi/iphlpapi_main.c >>> +++ b/dlls/iphlpapi/iphlpapi_main.c >>> @@ -1065,6 +1065,13 @@ static ULONG adapterAddressesFromIndex(ULONG family, ULONG flags, IF_INDEX index >>> debugstr_ipv4(&sa->sin_addr.S_un.S_addr, addr_buf)); >>> fill_unicast_addr_data(aa, ua); >>> >>> + ua->OnLinkPrefixLength = 0; >>> + for (j = 0; j < sizeof(*v4masks) * 8; j++) >>> + { >>> + if (v4masks[i] & 1 << j) ua->OnLinkPrefixLength++; >>> + else break; >>> + } >>> + >> >> Please add a helper to compute prefix length instead of duplicating this code. >> This should probably wait until the code freeze is over. >> > For what it's worth, if you're adding a helper function anyway, note > that you're really just counting the number of trailing zero-bits. GCC > has __builtin_ctz() for that, but even without that there are better > approaches than testing each bit individually. Alternatively, there's > also __builtin_popcount(). > Thank you both for the hints. One thing is that popcount would count all 1s even with 0s in between, whereas this function stops at the first 0. Hans, you wrote it that way, there's no checking if the mask is mangled (0s in between), how should it react to mangled masks?

Jan. 13, 2017

Re: [2/2] iphlpapi: Use the new version of the IP_ADAPTER_UNICAST_ADDRESS structure (try 2)

by Henri Verbeet

On 13 January 2017 at 14:43, Hans Leidekker <hans(a)codeweavers.com> wrote: > On Fri, 2017-01-13 at 13:27 +0100, André Hentschel wrote: >> diff --git a/dlls/iphlpapi/iphlpapi_main.c b/dlls/iphlpapi/iphlpapi_main.c >> index 54f7a37..223d980 100644 >> --- a/dlls/iphlpapi/iphlpapi_main.c >> +++ b/dlls/iphlpapi/iphlpapi_main.c >> @@ -1065,6 +1065,13 @@ static ULONG adapterAddressesFromIndex(ULONG family, ULONG flags, IF_INDEX index >> debugstr_ipv4(&sa->sin_addr.S_un.S_addr, addr_buf)); >> fill_unicast_addr_data(aa, ua); >> >> + ua->OnLinkPrefixLength = 0; >> + for (j = 0; j < sizeof(*v4masks) * 8; j++) >> + { >> + if (v4masks[i] & 1 << j) ua->OnLinkPrefixLength++; >> + else break; >> + } >> + > > Please add a helper to compute prefix length instead of duplicating this code. > This should probably wait until the code freeze is over. > For what it's worth, if you're adding a helper function anyway, note that you're really just counting the number of trailing zero-bits. GCC has __builtin_ctz() for that, but even without that there are better approaches than testing each bit individually. Alternatively, there's also __builtin_popcount().

Jan. 13, 2017

Re: [2/2] iphlpapi: Use the new version of the IP_ADAPTER_UNICAST_ADDRESS structure (try 2)

by Hans Leidekker

On Fri, 2017-01-13 at 13:27 +0100, André Hentschel wrote: > diff --git a/dlls/iphlpapi/iphlpapi_main.c b/dlls/iphlpapi/iphlpapi_main.c > index 54f7a37..223d980 100644 > --- a/dlls/iphlpapi/iphlpapi_main.c > +++ b/dlls/iphlpapi/iphlpapi_main.c > @@ -1065,6 +1065,13 @@ static ULONG adapterAddressesFromIndex(ULONG family, ULONG flags, IF_INDEX index > debugstr_ipv4(&sa->sin_addr.S_un.S_addr, addr_buf)); > fill_unicast_addr_data(aa, ua); > > + ua->OnLinkPrefixLength = 0; > + for (j = 0; j < sizeof(*v4masks) * 8; j++) > + { > + if (v4masks[i] & 1 << j) ua->OnLinkPrefixLength++; > + else break; > + } > + Please add a helper to compute prefix length instead of duplicating this code. This should probably wait until the code freeze is over.

Jan. 13, 2017

Re: [PATCH] wined3d: Add Iris Haswell PCI ids and descriptions

by Michael Cronenworth

On 01/13/2017 06:50 AM, André Hentschel wrote: > you forgot to sign off your patch Thanks. Resent. These IDs are now required as some applications are now checking the reported graphics card IDs and string and not allowing rendering unless a "supported" card is detected. My Iris 5100 was being reported as a G45.

Jan. 13, 2017

Re: [PATCH] wined3d: Add Iris Haswell PCI ids and descriptions

by André Hentschel

Am 13.01.2017 um 06:42 schrieb Michael Cronenworth: > --- > dlls/wined3d/directx.c | 11 +++++++++++ > dlls/wined3d/wined3d_private.h | 9 +++++++++ > 2 files changed, 20 insertions(+) > Hi, you forgot to sign off your patch

Jan. 13, 2017

Re: [PATCH 09/12] d3d9/tets: Remove redundant state setting calls.

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 https://testbot.winehq.org/JobDetails.pl?Key=27756 Your paranoid android. === w8 (32 bit visual) === visual.c:7986: Test failed: Got unexpected color 0x00007580 for quad 2 (different colors).

Jan. 13, 2017

Re: [PATCH 03/12] d3d8: Return D3D_OK from UnlockRect() for not locked textures.

by Bruno Jesus

On Fri, Jan 13, 2017 at 9:53 AM, Józef Kucia <jkucia(a)codeweavers.com> wrote: > Signed-off-by: Józef Kucia <jkucia(a)codeweavers.com> > --- > > Fixes bug 19862 > https://bugs.winehq.org/show_bug.cgi?id=19862 Most likely also fixes https://bugs.winehq.org/show_bug.cgi?id=28874

Jan. 13, 2017

Re: [PATCH 09/12] d3d9/tets: Remove redundant state setting calls.

by Józef Kucia

On Fri, Jan 13, 2017 at 12:53 PM, Józef Kucia <jkucia(a)codeweavers.com> wrote: > Signed-off-by: Józef Kucia <jkucia(a)codeweavers.com> > --- > dlls/d3d9/tests/visual.c | 81 ------------------------------------------------ > 1 file changed, 81 deletions(-) There is a typo in the subject prefix. It should read "d3d9/tests:".

Jan. 13, 2017