Open Bug 1284375 Opened 9 years ago Updated 2 years ago

Make the x86-64 asm in NSS compliant with Windows x86-64 ABI

Categories

(NSS :: Libraries, defect, P3)

Product:
NSS
Component:
Libraries
Platform:
x86_64
Windows
Type:
defect

Tracking

(Not tracked)

Status:
UNCONFIRMED

People

(Reporter: yuhongbao_386, Unassigned)

Details

Attachments

(3 files, 8 obsolete files)

Patch 2 for intel-gcm-x64-masm.asm
6.73 KB, patch
Details | Diff | Splinter Review
Patch 2 for intel-aes-x64-masm.asm
8.90 KB, patch
Details | Diff | Splinter Review
Patch 2 for arcfour-amd64-masm.asm
695 bytes, patch
Details | Diff | Splinter Review
Currently many of the Windows x64 assembly code in NSS has no function tables and has other violations of the Windows x64 ABI.
(Fixed version of intel-gcm-x64-masm.asm is in bug 1283585)
Attached file Fixed version of RC4 assembly code (obsolete) —
Tim, can you recommend someone to review Yuhong's NSS patches here?
Blocks: win64-rollout
Flags: needinfo?(ttaubert)
Cc'ing Franziskus who's working on related things currently and in the near future.
Flags: needinfo?(ttaubert)
Franziskus, can you please review Yuhong's NSS patches? Should this bug block our rollout of 64-bit Firefox builds to more Win64 OS users? (win64-rollout meta bug 1340936)
Flags: needinfo?(franziskuskiefer)
Chris, I don't think this should block any Win64-related tracking bugs. This appears to be mainly a matter of academic correctness.
It matters when debugging and when it crashes.
Should I write a version of intel-gcm-x64-masm.asm with just the Windows x64 SEH fixes.
More precisely, when it crashes Windows x64 SEH would not be able to unwind properly without this fix.
Attached file version with only SEH fixes (obsolete) —
Attached file second version with only SEH fixes (obsolete) —
Attachment #8869666 - Attachment is obsolete: true
Attached file Third version with only SEH fixes (obsolete) —
Attachment #8869667 - Attachment is obsolete: true
No longer blocks: win64-rollout
I don't think this has to block win64 rollout. I haven't seen any crashes related to this code. @Yuhong. I'm happy to look at the code if you provide patches.
Flags: needinfo?(franziskuskiefer)
Note that crash reporting itself may depend on Windows x64 SEH working properly though!
Attached patch Patch for intel-gcm-x64-masm.asm (obsolete) — — Splinter Review
Attachment #8869668 - Attachment is obsolete: true
Attached patch patch for arcfour-amd64-masm.asm (obsolete) — — Splinter Review
Attachment #8767823 - Attachment is obsolete: true
Attached patch Patch for intel-aes-x64-masm.asm (obsolete) — — Splinter Review
Attachment #8767819 - Attachment is obsolete: true
Hm, those are diffs not patches :(
I will see if I can do a better one.
Attachment #8870106 - Attachment is obsolete: true
Attachment #8870111 - Attachment is obsolete: true
Attachment #8870108 - Attachment is obsolete: true
Comment on attachment 8870319 [details] [diff] [review] Patch 2 for intel-gcm-x64-masm.asm Review of attachment 8870319 [details] [diff] [review]: ----------------------------------------------------------------- I don't like all the separate savexmm128 statements. Wrapping them in a macro would make this code much more readable. Same goes for allocstack.
There is ksamd64.inc.
Priority: -- → P3
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: