Re: [PATCH] perf build: Suppress openssl v3 deprecation warnings in libcrypto feature test

[Jiri Olsa]

On Mon, Jun 27, 2022 at 11:08:34AM +0800, 谭梓煊 wrote:
> On Sun, Jun 26, 2022 at 10:45 PM Jiri Olsa <olsajiri@xxxxxxxxx> wrote:
> >
> > On Sat, Jun 25, 2022 at 11:34:38PM +0800, Zixuan Tan wrote:
> > > With OpenSSL v3 installed, the libcrypto feature check fails as it use the
> > > deprecated MD5_* API (and is compiled with -Werror). The error message is
> > > as follows.
> > >
> > > $ make tools/perf
> > > ```
> > > Makefile.config:778: No libcrypto.h found, disables jitted code injection,
> > > please install openssl-devel or libssl-dev
> > >
> > > Auto-detecting system features:
> > > ...                         dwarf: [ on  ]
> > > ...            dwarf_getlocations: [ on  ]
> > > ...                         glibc: [ on  ]
> > > ...                        libbfd: [ on  ]
> > > ...                libbfd-buildid: [ on  ]
> > > ...                        libcap: [ on  ]
> > > ...                        libelf: [ on  ]
> > > ...                       libnuma: [ on  ]
> > > ...        numa_num_possible_cpus: [ on  ]
> > > ...                       libperl: [ on  ]
> > > ...                     libpython: [ on  ]
> > > ...                     libcrypto: [ OFF ]
> > > ...                     libunwind: [ on  ]
> > > ...            libdw-dwarf-unwind: [ on  ]
> > > ...                          zlib: [ on  ]
> > > ...                          lzma: [ on  ]
> > > ...                     get_cpuid: [ on  ]
> > > ...                           bpf: [ on  ]
> > > ...                        libaio: [ on  ]
> > > ...                       libzstd: [ on  ]
> > > ...        disassembler-four-args: [ on  ]
> > > ```
> > >
> > > This is very confusing because the suggested library (on my Ubuntu 20.04
> > > it is libssl-dev) is already installed. As the test only checks for the
> > > presence of libcrypto, this commit suppresses the deprecation warning to
> > > allow the test to pass.
> > >
> > > Signed-off-by: Zixuan Tan <tanzixuan.me@xxxxxxxxx>
> > > ---
> > >  tools/build/feature/test-libcrypto.c | 6 ++++++
> > >  1 file changed, 6 insertions(+)
> > >
> > > diff --git a/tools/build/feature/test-libcrypto.c b/tools/build/feature/test-libcrypto.c
> > > index a98174e0569c..31afff093d0b 100644
> > > --- a/tools/build/feature/test-libcrypto.c
> > > +++ b/tools/build/feature/test-libcrypto.c
> > > @@ -2,6 +2,12 @@
> > >  #include <openssl/sha.h>
> > >  #include <openssl/md5.h>
> > >
> > > +/*
> > > + * The MD5_* API have been deprecated since OpenSSL 3.0, which causes the
> > > + * feature test to fail silently. This is a workaround.
> > > + */
> >
> > then we use these deprecated MD5 calls in util/genelf.c if libcrypto is detected,
> > so I wonder how come the rest of the compilation passed for you.. do you have
> > CONFIG_JITDUMP disabled?
> >
> > thanks,
> > jirka
> >
> No, CONFIG_JITDUMP is not disabled. I am using the default configuration.
> 
> Yes, you are right. The rest of the compilation should fail, but it doesn't.
> I checked the verbose build commands. This seems to be the result of another
> inconsistency.
> 
> If libcrypto is detected, the macro "HAVE_LIBCRYPTO_SUPPORT" will be
> defined, but in perf/util/genelf.c, "HAVE_LIBCRYPTO" without the "_SUPPORT"
> prefix is checked. This causes urandom always be used to create build id
> rather than MD5 and SHA1, no matter what the detection result is.
> 
> In perf/Makefile.config, from line 776
> ```
> ifndef NO_LIBCRYPTO
>   ifneq ($(feature-libcrypto), 1)
>     msg := $(warning No libcrypto.h found, disables jitted code injection,
>             please install openssl-devel or libssl-dev);
>     NO_LIBCRYPTO := 1
>   else                                  <-- if libcrypto feature detected
>     CFLAGS += -DHAVE_LIBCRYPTO_SUPPORT  <-- define this
>     EXTLIBS += -lcrypto
>     $(call detected,CONFIG_CRYPTO)
>   endif
> endif
> ```
> 
> In perf/util/genelf.c, from line 33
> ```
> #ifdef HAVE_LIBCRYPTO                <-- but check this, it's always false

nice :)

> 
> #define BUILD_ID_MD5
> #undef BUILD_ID_SHA /* does not seem to work well when linked with Java */
> #undef BUILD_ID_URANDOM /* different uuid for each run */
> 
> #ifdef BUILD_ID_SHA
> #include <openssl/sha.h>
> #endif
> 
> #ifdef BUILD_ID_MD5
> #include <openssl/md5.h>
> #endif
> #endif                               <-- this block will be skipped
> ```
> 
> Maybe we should fix this, to really make use of libcrypto if it is available?

yea, I think that was the original idea, let's keep the variable with
SUPPORT suffix and use the -Wdeprecated-declarations for genelf.c

full fix would be to detect the new API and use it when it's available but..
given that the check was false at least since 2016, perhaps we could remove
that code? ;-) Stephane?

jirka

> 
> Links:
>  This commit include the genelf.c:
>   https://lore.kernel.org/all/1448874143-7269-3-git-send-email-eranian@xxxxxxxxxx/T/#mb6d3e18bee4901b71a4d4ef4f406feaaf48346d9
>  This commit include the feature test:
>   https://lore.kernel.org/all/1448874143-7269-3-git-send-email-eranian@xxxxxxxxxx/T/#m12a2ababf8ad3e366d56d9efab870592e6ff60a5
> 
> Thanks,
> Zixuan
> 
> > > +#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
> > > +
> > >  int main(void)
> > >  {
> > >       MD5_CTX context;
> > > --
> > > 2.34.1
> > >