ucx icon indicating copy to clipboard operation
ucx copied to clipboard

Clang build error: unused variable nfences in ptmalloc

Open tonycurtis opened this issue 2 years ago • 7 comments

Describe the bug

With (any recent) clang version, I get a "set but not used" error in ptmalloc. Code below. (I understand this is 3rd party code.)

Steps to Reproduce

  • Command line
  • UCX version used (from github branch XX or release YY) + UCX configure flags (can be checked by ucx_info -v)

github master @ 610a5f7684ad6573c6858356c947884426c47bf8 but I guess ptmalloc is pretty much immutable

Setup and versions

  • OS version (e.g Linux distro) + CPU architecture (x86_64/aarch64/ppc64le/...)
    • various

Error Message

libtool: compile:  clang -DHAVE_CONFIG_H -I. -I../.. -DCPU_FLAGS= -I/lustre/home/arcurtis/ucx-shilei/src -I/lustre/home/arcurtis/ucx-shilei -I/lustre/home/arcurtis/ucx-shilei/src -DUCM_MALLOC_PREFIX=ucm_dl -fno-strict-aliasing -DUSE_LOCKS=1 -DMALLINFO_FIELD_TYPE=int -O3 -g -Wall -Werror -fmax-type-align=16 -funwind-tables -Wno-missing-field-initializers -Wno-unused-parameter -Wno-unused-label -Wno-long-long -Wno-endif-labels -Wno-sign-compare -Wno-multichar -Wno-deprecated-declarations -Winvalid-pch -Wno-pointer-sign -Werror-implicit-function-declaration -Wno-format-zero-length -Wnested-externs -Wshadow -Werror=declaration-after-statement -Wno-deprecated-declarations -MT ptmalloc286/libucm_la-malloc.lo -MD -MP -MF ptmalloc286/.deps/libucm_la-malloc.Tpo -c ptmalloc286/malloc.c  -fPIC -DPIC -DUCX_SHARED_LIB -o ptmalloc286/.libs/libucm_la-malloc.o
ptmalloc286/malloc.c:4000:7: error: variable 'nfences' set but not used [-Werror,-Wunused-but-set-variable]
  int nfences = 0;
      ^
1 error generated.

How about this patch as minimal solution:

diff --git a/src/ucm/ptmalloc286/malloc.c b/src/ucm/ptmalloc286/malloc.c
index e17799678..78e198c6c 100644
--- a/src/ucm/ptmalloc286/malloc.c
+++ b/src/ucm/ptmalloc286/malloc.c
@@ -3997,7 +3997,9 @@ static void add_segment(mstate m, char* tbase, size_t tsize, flag_t mmapped) {
   msegmentptr ss = (msegmentptr)(chunk2mem(sp));
   mchunkptr tnext = chunk_plus_offset(sp, ssize);
   mchunkptr p = tnext;
+#ifdef NDEBUG
   int nfences = 0;
+#endif /* NDEBUG */

   /* reset top to new space */
   init_top(m, (mchunkptr)tbase, tsize - TOP_FOOT_SIZE);
@@ -4015,7 +4017,9 @@ static void add_segment(mstate m, char* tbase, size_t tsize, flag_t mmapped) {
   for (;;) {
     mchunkptr nextp = chunk_plus_offset(p, SIZE_T_SIZE);
     p->head = FENCEPOST_HEAD;
+#ifdef NDEBUG
     ++nfences;
+#endif /* NDEBUG */
     if ((char*)(&(nextp->head)) < old_end)
       p = nextp;
     else

tonycurtis avatar Feb 20 '23 21:02 tonycurtis

It also happens with the LLVM-based Intel oneAPI compiler. -Werror shouldn't be enabled in releases.

zzzoom avatar Feb 23 '23 02:02 zzzoom

I also experience this problem!

zerothi avatar Jul 05 '23 08:07 zerothi

@zerothi FWIW the spack compiler wrapper now discards all -Werror parameters so you can build it with spack even if they don't fix ucx.

zzzoom avatar Jul 05 '23 14:07 zzzoom

I am suffering from the same problem. Removing -Werror flag from BASE_CFLAGS="-g -Wall -Werror" in config/m4/compiler.m4 did not fix the problem for me.

wreckdump avatar Sep 01 '23 15:09 wreckdump

Hmm. Removing -Werror flag from BASE_CFLAGS="-g -Wall -Werror" in configure did the trick.

wreckdump avatar Sep 01 '23 15:09 wreckdump

I wondered why I had encountered this problem even without the -DNDEBUG flag. Then, I found the following line that overrides the default behaviour of assert():

https://github.com/openucx/ucx/blob/8f895e8901eaf5aae9edf90e33f6ef41e8a49207/src/ucm/ptmalloc286/malloc.c#L1459

Is it possible to modify ptmalloc code to fix this issue?

s417-lama avatar Feb 07 '24 11:02 s417-lama

Hi @s417-lama @tonycurtis Let me know if this patch solves the issue for you: https://github.com/openucx/ucx/pull/9667

roiedanino avatar Feb 07 '24 14:02 roiedanino