Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Mon, 17 Aug 2015 19:01:46 +0300
From: Solar Designer <solar@...nwall.com>
To: john-dev@...ts.openwall.com
Subject: Re: FMT_OMP_BAD

Kai, magnum -

On Mon, Aug 17, 2015 at 11:46:04PM +0800, Kai Zhao wrote:
> I find some formats set FMT_OMP flag without judging the value of
> _OPENMP. Such as mysql_fmt_plug.c:
> 
> struct fmt_main fmt_MYSQL_fast =
> {
>         {
>                 [...]
>                 MAX_KEYS_PER_CRYPT,
>                 FMT_CASE | FMT_8_BIT | FMT_SPLIT_UNIFIES_CASE | FMT_OMP,
>                 { NULL },
>                 tests
>         },
>         [...]
> }

That's correct.  Please see formats.h:

#ifdef _OPENMP
/* Parallelized with OpenMP */
#define FMT_OMP                         0x01000000
/* Poor OpenMP scalability */
#define FMT_OMP_BAD                     0x02000000
#else
#define FMT_OMP                         0
#define FMT_OMP_BAD                     0
#endif

> I think it should be:
> 
> struct fmt_main fmt_MYSQL_fast =
> {
>         {
>                 [...]
>                 MAX_KEYS_PER_CRYPT,
> #ifdef _OPENMP
>                 FMT_OMP |
> #endif
>                 FMT_CASE | FMT_8_BIT | FMT_SPLIT_UNIFIES_CASE,
>                 { NULL },
>                 tests
>         },
>         [...]
> }

No, that's redundant.  Some formats do it, but they should actually be
changed to use the simpler approach ...

... Unless this becomes wrong when we introduce FAST_FORMATS_OMP checks.
For example, rawMD5_fmt_plug.c has:

#include "formats.h"

#if !FAST_FORMATS_OMP
#undef _OPENMP
#endif

[...]

#ifdef _OPENMP
                FMT_OMP | FMT_OMP_BAD |
#endif

and this combination makes sense to me.  Listing FMT_OMP | FMT_OMP_BAD
unconditionally is no longer right, because this file may have _OPENMP
undefined even if formats.h had it defined.

Another approach could be to place the #undef before the very first
#include that might result in formats.h getting included.  However, this
might be subtly wrong because formats.h includes params.h, in which we
also have unrelated _OPENMP checks that are better processed in the same
way for the entire program.  I think this simpler approach would work
fine for now (the only _OPENMP check currently in params.h does not
affect format specifics), but it might not if we introduce even more
_OPENMP checks in header files later.

Yet we could give this simpler approach a try: move the #undef to be
before the very first #include in a format source file, and then list
FMT_OMP | FMT_OMP_BAD unconditionally.

I'll leave this decision to magnum.

Alexander

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.