Closed
Bug 972450
Opened 10 years ago
Closed 10 years ago
Define -DMP_USE_UINT_DIGIT in lib/freebl/Makefile for Linux x86
Categories
(NSS :: Libraries, defect, P2)
Tracking
(Not tracked)
RESOLVED
FIXED
3.16
People
(Reporter: wtc, Assigned: wtc)
Details
Attachments
(4 files)
878 bytes,
patch
|
wtc
:
review+
wtc
:
checked-in+
|
Details | Diff | Splinter Review |
1017 bytes,
patch
|
KaiE
:
review+
wtc
:
checked-in+
|
Details | Diff | Splinter Review |
898 bytes,
patch
|
wtc
:
checked-in+
|
Details | Diff | Splinter Review |
944 bytes,
patch
|
KaiE
:
review+
wtc
:
checked-in+
|
Details | Diff | Splinter Review |
This problem was reported by Stephan Bergmann of Red Hat on the dev-security@lists.mozilla.org mailing list: Stumbled over this when trying to build under linux32 the nss 1.15.3 that we bundle in LibreOffice, but seems the relevant files are still the same on trunk hg.mozilla.org/projects/nss: lib/freebl/Makefile for OS_TARGET=Linux, CPU_ARCH=x86 sets ASFILES=mpi_x86.s but doesn't include -DMP_USE_UINT_DIGIT in DEFINES. That causes lib/freebl/mpi/mpi.h to pick up the #elif !defined(MP_USE_UINT_DIGIT) && defined(MP_ULONG_LONG_MAX) block that does typedef unsigned long long mp_digit; so that the calls to s_mpv_div_2dx1d in lib/freebl/mpi/mpi.c pass 64-bit Nhi, Nlo, divisor arguments, but the implementation of s_mpv_div_2dx1d in lib/freebl/mpi/mpi_x86.s apparently expects mp_digit to be unsinged long and takes 32-bit arguments, garbling the subsequent qp and rp pointer arguments. The below patch solved it for me, unless I'm missing the obvious and walked down the wrong path anyway (not knowing much about the code in question, just seeing it fail in my LibreOffice build). Stephan --- lib/freebl/Makefile +++ lib/freebl/Makefile @@ -195,6 +195,7 @@ ifeq ($(CPU_ARCH),x86) ASFILES = mpi_x86.s DEFINES += -DMP_ASSEMBLY_MULTIPLY -DMP_ASSEMBLY_SQUARE + DEFINES += -DMP_USE_UINT_DIGIT DEFINES += -DMP_ASSEMBLY_DIV_2DX1D DEFINES += -DMP_CHAR_STORE_SLOW -DMP_IS_LITTLE_ENDIAN # The floating point ECC code doesn't work on Linux x86 (bug 311432).
Assignee | ||
Comment 1•10 years ago
|
||
I also define -DMP_NO_MP_WORD to match Windows x86. My understanding is that MP_NO_MP_WORD is a performance-turning parameter. I remember I experimented with it two or three years ago, but I forgot whether it improved or hurt performance. For now let's be consistent on all x86 platforms. Patch checked in: https://hg.mozilla.org/projects/nss/rev/60412aa39b8d
Attachment #8376631 -
Flags: review+
Attachment #8376631 -
Flags: checked-in+
Assignee | ||
Comment 2•10 years ago
|
||
ULLONG_MAX is the standard macro in C99 for the maximum value of unsigned long long, so we should test for it first. I also update the comment to say "C99" instead of "GCC" because ULLONG_MAX is also defined by Visual C++ and Apple's compiler now.
Attachment #8376641 -
Flags: review?(kaie)
Assignee | ||
Comment 3•10 years ago
|
||
I found that we are not defining -DMP_NO_MP_WORD for Mac OS X x86. So I suspect that when I enabled x86 assembly code for Mac OS X, I did some performance measurements but did not find -DMP_NO_MP_WORD to improve performance. So I'm taking a more conservative approach and remove -DMP_NO_MP_WORD for Linux x86 (unless someone shows it improves performance). Patch checked in: https://hg.mozilla.org/projects/nss/rev/fa9a3a970910
Attachment #8377859 -
Flags: checked-in+
Assignee | ||
Comment 4•10 years ago
|
||
While working on bug 966596, I figured out why <limits.h> doesn't define LLONG_MAX when NSS is compiled. The reason is that we are compiling NSS with -D_POSIX_SOURCE -D_BSD_SOURCE -D_XOPEN_SOURCE. These -Dmacro flags were added because we used to compile NSS the -ansi flag, so these *_SOURCE macros were needed to allow NSS to use certain system calls.
Attachment #8379329 -
Flags: review?(kaie)
Comment 5•10 years ago
|
||
Comment on attachment 8376641 [details] [diff] [review] mpi.h should test for the C99 ULLONG_MAX macro first. Update comment. r=kaie
Attachment #8376641 -
Flags: review?(kaie) → review+
Comment 6•10 years ago
|
||
Comment on attachment 8379329 [details] [diff] [review] Don't compile NSS with any *_SOURCE macros defined r=kaie
Attachment #8379329 -
Flags: review?(kaie) → review+
Assignee | ||
Comment 7•10 years ago
|
||
Comment on attachment 8376641 [details] [diff] [review] mpi.h should test for the C99 ULLONG_MAX macro first. Update comment. Patch checked in: https://hg.mozilla.org/projects/nss/rev/444aacba97e4
Attachment #8376641 -
Flags: checked-in+
Assignee | ||
Comment 8•10 years ago
|
||
Comment on attachment 8379329 [details] [diff] [review] Don't compile NSS with any *_SOURCE macros defined Patch checked in: https://hg.mozilla.org/projects/nss/rev/26692da45004
Attachment #8379329 -
Flags: checked-in+
Assignee | ||
Updated•10 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•