Every test fails the same way: (stretch)austin@localhost:~/wine-git/dlls/advapi32/tests$ make service.ok ../../../tools/runtest -q -P wine -T ../../.. -M advapi32.dll -p advapi32_test.exe.so service && touch service.ok ==20504== ==20504== Process terminating with default action of signal 4 (SIGILL) ==20504== Illegal opcode at address 0x4FC2C54 ==20504== at 0x4FC2C54: signal_init_thread (signal_arm.c:974) ==20504== by 0x4FCA7CB: thread_init (thread.c:354) ==20504== by 0x4F9F303: __wine_process_init (loader.c:3341) ==20504== by 0x485FBC3: wine_init (loader.c:979) ==20504== by 0x108A27: main (main.c:258) Illegal instruction (core dumped) Makefile:374: recipe for target 'service.ok' failed make: *** [service.ok] Error 132 I reviewed this with the wine ARM maintainer, who pointed out it's likely a valgrind bug. The code in question: https://source.winehq.org/git/wine.git/blob/40b7831cd80607e42b9e1c910a62f022c45ac884:/dlls/ntdll/signal_arm.c#l959 Removing the TEB/TPIDRURW handling get's past this: diff --git a/dlls/ntdll/signal_arm.c b/dlls/ntdll/signal_arm.c index e5e314049e..df050a96ae 100644 --- a/dlls/ntdll/signal_arm.c +++ b/dlls/ntdll/signal_arm.c @@ -968,12 +968,6 @@ void signal_init_thread( TEB *teb ) pthread_key_create( &teb_key, NULL ); init_done = TRUE; } - -#if defined(__ARM_ARCH_7A__) || defined(__ARM_ARCH_8A__) - /* Win32/ARM applications expect the TEB pointer to be in the TPIDRURW register. */ - __asm__ __volatile__( "mcr p15, 0, %0, c13, c0, 2" : : "r" (teb) ); -#endif - pthread_setspecific( teb_key, teb ); } and now tests work.
(stretch)austin@localhost:~/src/valgrind$ uname -a Linux localhost 3.14.0 #1 SMP PREEMPT Wed Oct 25 21:59:24 PDT 2017 armv7l GNU/Linux (stretch)austin@localhost:~/src/valgrind$ /opt/valgrind/bin/valgrind -v --version valgrind-3.14.0.GIT (confused why that doesn't show the git hash..) (stretch)austin@localhost:~/src/valgrind$ tail -n 1 include/vgversion.h #define VGGIT "2f9cceafa3-20171028"
Forgot to include, some background on why we're doing that: https://patchwork.kernel.org/patch/2536641/
Austin, is this an ARM- or Thumb- encoding that fails? Can you show the part of the message that precedes the SIGILL, that is, whatever the ARM/Thumb front end prints out? IIUC, TPIDRURW is a 32 bit register that can be both read and written from user space. Yes? Does it require any special handling?
(In reply to Julian Seward from comment #3) > IIUC, TPIDRURW is a 32 bit register that can be both read and > written from user space. Yes? Does it require any special handling? To clarify .. what I mean to ask is: does TPIDRURW behave like a "normal" integer register, in that each thread has its own copy and can read and write it independently of other threads? Or does it have some other behaviour?
For the hardware, TPIDRURW has no behaviour except that it holds what you write to it. For the Linux kernel, this register is used to provide per-thread information, and its contents are context-switched when threads are context-switched. Looks like Austin will know better than me the exact details of what happens to the value on fork/clone.
(In reply to Julian Seward from comment #4) > (In reply to Julian Seward from comment #3) > > IIUC, TPIDRURW is a 32 bit register that can be both read and > > written from user space. Yes? Does it require any special handling? > > To clarify .. what I mean to ask is: does TPIDRURW behave like a "normal" > integer register, in that each thread has its own copy and can read and > write it independently of other threads? Or does it have some other > behaviour? From Andre: >> Sure, >> >> it should be ARM encoding. trpidrurw is rw from userspace and needs no permissions > Is it specific per thread or shared across? > per thread maybe https://github.com/AndreRH/tpidrurw-test can help to understand it
Re arm/thumb: <wizardedit> so you said it's arm encoding. I noticed that configure.ac requires thumb? Do both get used? <Andre_H> yes, most of wine should be arm <wizardedit> what's thumb used for? <Andre_H> Windows Apps are Thumb-2, and to call into such functions we need the command "blx" (branch and link while exchanging instruction set if necessary, or something like that), if the compiler targets non-thumb (e.g. arm-only) it doesn't like bx and blx <wizardedit> so it targets both arm and thumb? <Andre_H> the compiler, yes <Andre_H> the instruction.... I'm checking this right now <Andre_H> the instruction encoding seems to be exactly the same for both arm and thumb-2 <Andre_H> it's definitely arm
(In reply to Peter Maydell from comment #5) > [..] its contents are context-switched when threads are context-switched. [..] Peter, do you mean by that that its contents are preserved across context switches like any other GP integer register, so that a thread simply always sees the last value it wrote to that register? Or do you mean that the kernel changes the value in the register in some way?
Yes, I just mean that each thread sees its own copy with the value it last wrote. On exec() a fresh process image starts out with a zero value. On fork() or clone() a new thread or process inherits the value from its parent.
Created attachment 108896 [details] TPIDRURW support for 32-bit arm This runs the test program shown in comment 6, correctly, both for Thumb and ARM encodings. For 32 bit only. Austin, can you test this?
(In reply to Julian Seward from comment #10) > Created attachment 108896 [details] > TPIDRURW support for 32-bit arm > > This runs the test program shown in comment 6, correctly, both for > Thumb and ARM encodings. For 32 bit only. Austin, can you test this? Seems to work for me, thanks! I'm going to attach logs with/without the patch. I used --verbose instead of -q, which then showed the missing info: disInstr(arm): unhandled instruction: 0xEE0D4F50 cond=14(0xE) 27:20=224(0xE0) 4:4=1 3:0=0(0x0) ==4434== valgrind: Unrecognised instruction at address 0x4fc3bb4. ==4434== at 0x4FC3BB4: signal_init_thread (signal_arm.c:974) ==4434== by 0x4FCACF7: thread_init (thread.c:354) ==4434== by 0x4FA1433: __wine_process_init (loader.c:3341) ==4434== by 0x485FBC3: wine_init (loader.c:979) ==4434== by 0x108A27: main (main.c:258)
Created attachment 108908 [details] output without patch
Created attachment 108909 [details] output with patch
Landed, c470e0c23c6c79deec943cb6a111b572fc86dbba.
(In reply to Julian Seward from comment #14) > Landed, c470e0c23c6c79deec943cb6a111b572fc86dbba. Thanks for the quick fix!