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

Build most programs without OpenMP #5658

Open
solardiz opened this issue Feb 7, 2025 · 3 comments
Open

Build most programs without OpenMP #5658

solardiz opened this issue Feb 7, 2025 · 3 comments
Labels
enhancement RFC / discussion Help or comments wanted

Comments

@solardiz
Copy link
Member

solardiz commented Feb 7, 2025

As seen in openwall/john-packages#723 and I've just confirmed on Linux, even separate program binaries like dmg2john get linked against OpenMP now. We should ideally avoid that, limiting such compiler/linker flags only to programs that actually use OpenMP.

I understand this can be tricky as some object files are shared between john and some *2john programs, and are built with OpenMP enabled even if unused, so theoretically could require the library at link time. It'd be extra effort for us to get them built with different compiler flags than the rest. It'd probably be less effort to omit OpenMP just at link time of *2john programs, hoping that no actual dependency got in despite of OpenMP enabled at compile time? Also, we somehow do avoid dependencies of *2john on most libraries that john uses, such as zlib and OpenSSL's libcrypto. So maybe not that hard to fix, after all?

@solardiz solardiz added enhancement RFC / discussion Help or comments wanted labels Feb 7, 2025
@ghost
Copy link

ghost commented Feb 7, 2025

I've just confirmed on Linux

It's challenging to reproduce the problem on Linux [1].


Anyway, the patch doesn't look wrong to me:

Date: Fri, 7 Feb 2025 16:51:39 -0300
Subject: [PATCH] +1
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Signed-off-by: Claudio André <[email protected]>
---
 src/Makefile.in | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/src/Makefile.in b/src/Makefile.in
index d8c3d927e..06e48f026 100644
--- a/src/Makefile.in
+++ b/src/Makefile.in
@@ -61,6 +61,8 @@ endif
 
 CPPFLAGS = @CPPFLAGS@
 CFLAGSX = -c @CFLAGS@ @JOHN_NO_SIMD@ @CFLAGS_EXTRA@ @OPENSSL_CFLAGS@ @OPENMP_CFLAGS@ @HAVE_MPI@ @PTHREAD_CFLAGS@ $(CPPFLAGS)
+# CFLAGS for use on 2john tools only
+CFLAGS_CLEAN = -DAC_BUILT -c @CFLAGS@ @CFLAGS_EXTRA@ @PTHREAD_CFLAGS@ $(CPPFLAGS)
 # CFLAGS for use on the main john.c file only
 CFLAGS_MAIN = -DAC_BUILT @CC_MAIN_CPU@ $(CFLAGSX)
 CFLAGS = -DAC_BUILT @CC_CPU@ @CC_MAIN_CPU@ $(CFLAGSX)
@@ -310,6 +312,7 @@ DES_std.o:	DES_std.c arch.h common.h memory.h DES_std.h os.h os-autoconf.h autoc
 detect.o:	detect.c
 
 dmg2john.o:	dmg2john.c autoconfig.h arch.h filevault.h misc.h jumbo.h memory.h johnswap.h os.h os-autoconf.h
+	$(CC) $(CFLAGS_CLEAN) $(OPT_NORMAL) $< -o $@
 
 dummy.o:	dummy.c common.h arch.h memory.h formats.h params.h misc.h jumbo.h autoconfig.h options.h list.h loader.h getopt.h os.h os-autoconf.h
 
@@ -706,7 +709,7 @@ poly1305-donna/poly1305-donna.a:
 	$(CC) -DAC_BUILT -Wall -O2 @CPPFLAGS@ @CFLAGS@ @OPENSSL_CFLAGS@ @CFLAGS_EXTRA@ @PTHREAD_CFLAGS@ @PTHREAD_LIBS@ keepass2john.c jumbo.c base64_convert.c -D_JOHN_MISC_NO_LOG misc.c common.c memory.c sha2.o $(LDFLAGS) @OPENMP_CFLAGS@ @OPENSSL_LIBS@ -o $@
 
 ../run/dmg2john@EXE_EXT@: dmg2john.o jumbo.o
-	$(LD) $(LDFLAGS) @PTHREAD_CFLAGS@ @PTHREAD_LIBS@ dmg2john.o jumbo.o @OPENMP_CFLAGS@ -o $@
+	$(LD) $(LDFLAGS) @PTHREAD_CFLAGS@ @PTHREAD_LIBS@ dmg2john.o jumbo.o -o $@
 
 ../run/bitlocker2john@EXE_EXT@: bitlocker2john.c jumbo.c base64_convert.c misc.c common.o memory.c
 	$(CC) -DAC_BUILT -Wall -O2 @CPPFLAGS@ @CFLAGS@ @OPENSSL_CFLAGS@ @CFLAGS_EXTRA@ @PTHREAD_CFLAGS@ @PTHREAD_LIBS@ bitlocker2john.c jumbo.c base64_convert.c -D_JOHN_MISC_NO_LOG misc.c common.c memory.c  $(LDFLAGS) @OPENMP_CFLAGS@ @OPENSSL_LIBS@ -o $@
-- 
2.43.0

[1]

$ ldd ../run/dmg2john; echo "---"; ldd ../run/john
	linux-vdso.so.1 (0x00007fff5c1a7000)
	libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x000079814a000000)
	/lib64/ld-linux-x86-64.so.2 (0x000079814a38f000)
---
	linux-vdso.so.1 (0x00007fff7bff2000)
	libcrypto.so.3 => /lib/x86_64-linux-gnu/libcrypto.so.3 (0x000074984a800000)
	libgmp.so.10 => /lib/x86_64-linux-gnu/libgmp.so.10 (0x000074984bc0d000)
	libm.so.6 => /lib/x86_64-linux-gnu/libm.so.6 (0x000074984ad17000)
	libz.so.1 => /lib/x86_64-linux-gnu/libz.so.1 (0x000074984bbf1000)
	libcrypt.so.1 => /lib/x86_64-linux-gnu/libcrypt.so.1 (0x000074984bbb7000)
	libbz2.so.1.0 => /lib/x86_64-linux-gnu/libbz2.so.1.0 (0x000074984bba1000)
	libgomp.so.1 => /lib/x86_64-linux-gnu/libgomp.so.1 (0x000074984bb4b000)
	libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x000074984a400000)
	/lib64/ld-linux-x86-64.so.2 (0x000074984bca4000)

@magnumripper
Copy link
Member

magnumripper commented Feb 7, 2025

I understand this can be tricky as some object files are shared between john and some *2john programs, and are built with OpenMP enabled even if unused, so theoretically could require the library at link time.

Very few non-format object files has any OpenMP stuff and none of them are plugins. I only see bench.c, idle.c and john.c (I didn't look further than they do have either an OpenMP pragma or use some omp_ function so I'm not even sure all three need any flags/linking).

Can't we just drop the OpenMP stuff from CFLAGS/LDFLAGS, like Claudio did above but globally. Then add it explicitly for formats/plugins plus the select few that actually need it?

@ghost
Copy link

ghost commented Feb 7, 2025

Something like this is also desired (or demanded, depending on what we commit).

diff --git a/src/configure.ac b/src/configure.ac
index 45c6c5df7..de1121a4d 100644
--- a/src/configure.ac
+++ b/src/configure.ac
@@ -50,6 +50,11 @@ AC_ARG_VAR([CFLAGS_EXTRA], [additional CFLAGS for -Woptions and other things, no
 AC_ARG_VAR([AS], [full pathname of assembler to use])
 AC_ARG_VAR([AR], [full pathname of "ar" to use])
 AC_ARG_VAR([LD], [full pathname of linker to use])
+AC_ARG_VAR([OMP_EXTRAFLAGS], [additional OpenMP flags required by the compiler])
+
+dnl If someone needs to specify an OpenMP flag, they can't use CPPFLAGS because it applies to all binaries.
+dnl So we need a flag that only affects OpenMP binaries.
+OPENMP_CFLAGS="$OMP_EXTRAFLAGS"
 
 dnl Define Packages.
 dnl Use OpenSSL (default: yes).

The syntax may be wrong.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement RFC / discussion Help or comments wanted
Projects
None yet
Development

No branches or pull requests

2 participants