Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

uninits coming from hand assembly that uses OP_psubq with self to zero mm regs #1482

Closed
derekbruening opened this issue Nov 28, 2014 · 1 comment

Comments

@derekbruening
Copy link
Contributor

From bruen...@google.com on March 21, 2014 21:12:37

My attempt to finish off xmm propagation is hitting an uninit in unit_tests:

[ RUN ] ComponentUpdaterTest.InstallCrx
7308
7308 Error #1: UNINITIALIZED READ: reading 0x3ec0f4e0-0x3ec0f4e4 4 byte(s)
7308 # 0 CRNSS.dll!s_mp_mul_d [third_party\nss\nss\lib\freebl\mpi\mpi.c:3314]
7308 # 1 CRNSS.dll!s_mp_div [third_party\nss\nss\lib\freebl\mpi\mpi.c:4254]
7308 # 2 CRNSS.dll!mp_div [third_party\nss\nss\lib\freebl\mpi\mpi.c:1024]
7308 # 3 CRNSS.dll!s_mp_to_mont [third_party\nss\nss\lib\freebl\mpi\mpmontg.c:145]
7308 # 4 CRNSS.dll!mp_exptmod [third_party\nss\nss\lib\freebl\mpi\mpmontg.c:1085]
7308 # 5 CRNSS.dll!RSA_PublicKeyOp [third_party\nss\nss\lib\freebl\rsa.c:858]
7308 # 6 CRNSS.dll!RSA_CheckSignRecover [third_party\nss\nss\lib\freebl\rsapkcs.c:1348]
7308 # 7 CRNSS.dll!NSC_VerifyRecover [third_party\nss\nss\lib\softoken\pkcs11c.c:3462]
7308 # 8 CRNSS.dll!PK11_VerifyRecover [third_party\nss\nss\lib\pk11wrap\pk11obj.c:646]
7308 # 9 CRNSS.dll!DecryptSigBlock [third_party\nss\nss\lib\cryptohi\secvfy.c:45]
7308 #10 CRNSS.dll!vfy_CreateContext [third_party\nss\nss\lib\cryptohi\secvfy.c:384]
7308 #11 CRNSS.dll!VFY_CreateContextWithAlgorithmID [third_party\nss\nss\lib\cryptohi\secvfy.c:458]
7308 #12 crcrypto.dll!crypto::SignatureVerifier::VerifyInit [crypto\signature_verifier_nss.cc:120]
7308 #13 component_updater::anonymous namespace'::CRXValidator::CRXValidator [chrome\browser\component_updater\component_unpacker.cc:65] \~~7308~~#14component_updater::ComponentUnpacker::Verify [chrome\browser\component_updater\component_unpacker.cc:159] \~~7308~~#15component_updater::ComponentUnpacker::UnpackInternal [chrome\browser\component_updater\component_unpacker.cc:137] \~~7308~~#16component_updater::CrxUpdateService::Install [chrome\browser\component_updater\component_updater_service.cc:868] \~~7308~~#17` base::internal::RunnableAdapter<>::Run [base\bind_internal.h:248]
7308 Note: @0:11:25.601 in thread 7308
7308 Note: instruction: cmp (%eax) $0x00000000

CRNSS!s_mp_mul_d+0x6a [e:\derek\chromium\src\third_party\nss\nss\lib\freebl\mpi\mpi.c @ 3314]:
6aa5f41a 8b470c mov eax,dword ptr [edi+0Ch]
6aa5f41d 8d0488 lea eax,[eax+ecx*4]
6aa5f420 83c0fc add eax,0FFFFFFFCh
6aa5f423 833800 cmp dword ptr [eax],0
6aa5f426 7509 jne CRNSS!s_mp_mul_d+0x81 (6aa5f431)

tracking it backward it looks like this is expected to zero mm2:
slow_path 0x6a98d9e6: psubq %mm2 %mm2 -> %mm2

and later:

slow_path 0x6a98da15: movd %mm2[4byte] -> (%edi)[4byte] | 0x0
shadow registers prior to instr:
eax=00 ecx=00 edx=00 ebx=00 esp=00 ebp=00 esi=00 edi=00 efl=0
xmm0=00000000 xmm1=00000000 xmm2=00000000 xmm3=00000000
xmm4=00000000 xmm5=00000000 xmm6=00000000 xmm7=00000000
mm0=0000 mm1=0000 mm2=ffff mm3=ffff mm4=ffff mm5=ffff mm6=ffff mm7=ffff
shadows after src 0 %mm2 : 33330000000000000000000000000000, eflags: 0
memref: write @0x6a98da15 0x0678aa28 0x4 bytes (pre-dword 0x00 0x00)
store @0x6a98da15 to 0x0678aa28 val=0x3
store @0x6a98da15 to 0x0678aa29 val=0x3
store @0x6a98da15 to 0x0678aa2a val=0x3
store @0x6a98da15 to 0x0678aa2b val=0x3
in slow path for unknown reason @0x6a98da15 0x0678aa28
shadow registers after instr:
eax=00 ecx=00 edx=00 ebx=00 esp=00 ebp=00 esi=00 edi=00 efl=0
xmm0=00000000 xmm1=00000000 xmm2=00000000 xmm3=00000000
xmm4=00000000 xmm5=00000000 xmm6=00000000 xmm7=00000000
mm0=0000 mm1=0000 mm2=ffff mm3=ffff mm4=ffff mm5=ffff mm6=ffff mm7=ffff

slow_path 0x6a96f423: cmp (%eax)[4byte] $0x00000000
shadow registers prior to instr:
eax=00 ecx=00 edx=00 ebx=00 esp=00 ebp=00 esi=00 edi=00 efl=0
xmm0=00000000 xmm1=00000000 xmm2=00000000 xmm3=00000000
xmm4=00000000 xmm5=00000000 xmm6=00000000 xmm7=00000000
mm0=0000 mm1=0000 mm2=ffff mm3=ffff mm4=ffff mm5=ffff mm6=ffff mm7=ffff
memref: read @0x6a96f423 0x0678aa28 0x4 bytes (pre-dword 0xff 0x00)

Looks like they mean psubq w/ self to clear it out:

src/third_party/nss/nss/lib/freebl/mpi/mpi_x86_asm.c
s_mpv_mul_d_sse2:
push ebp
mov ebp, esp
push edi
push esi
psubq mm2, mm2 ; carry = 0
mov ecx, [ebp+12] ; ecx = a_len
movd mm1, [ebp+16] ; mm1 = b
mov edi, [ebp+20]
cmp ecx, 0
je L_6 ; jmp if a_len == 0
mov esi, [ebp+8] ; esi = a
cld
L_5:
movd mm0, [esi] ; mm0 = *a++
add esi, 4
pmuludq mm0, mm1 ; mm0 = b * *a++
paddq mm2, mm0 ; add the carry
movd [edi], mm2 ; store the 32bit result
add edi, 4
psrlq mm2, 32 ; save the carry
dec ecx ; --a_len
jnz L_5 ; jmp if a_len != 0
L_6:
movd [edi], mm2 ; *c = carry
emms
pop esi
pop edi
leave
ret

So we should add psubq, subps, and subpd to the list of things to consider
mark-defined when src1==src2.

Original issue: http://code.google.com/p/drmemory/issues/detail?id=1482

@derekbruening
Copy link
Contributor Author

From derek.br...@gmail.com on March 21, 2014 19:12:46

This issue was closed by revision r1809 .

Status: Fixed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant