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

[Bug]: Chapel variables with extern types may not be default initialized #26406

Open
jabraham17 opened this issue Dec 13, 2024 · 1 comment · May be fixed by #26417
Open

[Bug]: Chapel variables with extern types may not be default initialized #26406

jabraham17 opened this issue Dec 13, 2024 · 1 comment · May be fixed by #26417

Comments

@jabraham17
Copy link
Member

The following program demonstrates this issue in 3 different ways

use IO;
import OS.errorCode;

require "foo.h";

extern record R {
  var a,b,c,d,e,f,g,h,i,j,k: int(64);
}
proc printR(r:R) {
  writeln("print R");
  writeln(r.a, r.b, r.c, r.d, r.e, r.f, r.g, r.h, r.i, r.j, r.k);
}

extern type my_int64_t;
extern proc printX(x: my_int64_t);

writeln("print1");
var g: R;
printR(g);
writeln("print2");
var gg: R;
printR(gg);
writeln("print3");
var ggg: R;
printR(ggg);
// all of these are uninitialized

var x: my_int64_t;
printX(x); // uninitialized

writeln("global syserr");
var globSysErr: errorCode;
writeln(globSysErr:int);  // segfaults because this is uninitialized, and we try and dereference a bad pointer

This appears to only be an issue with CHPL_COMM=gasnet and CHPL_MEM=cstdlib. Since the memory is uninitialized and can by chance be zero (it frequently is on my Mac, sometimes not on a linxu64 system), the best way to reproduce this bug is with ASAN. I think in theory CHPL_MEM=jemalloc is susceptible to this, but I think we have never hit it in practice.

I have the following

CHPL_HOST_PLATFORM: darwin
CHPL_HOST_COMPILER: clang
  CHPL_HOST_CC: clang
  CHPL_HOST_CXX: clang++
CHPL_HOST_ARCH: arm64
CHPL_TARGET_PLATFORM: darwin
CHPL_TARGET_COMPILER: clang *
  CHPL_TARGET_CC: clang
  CHPL_TARGET_CXX: clang++
  CHPL_TARGET_LD: /usr/bin/clang++
CHPL_TARGET_ARCH: arm64
CHPL_TARGET_CPU: native *
CHPL_LOCALE_MODEL: flat
CHPL_COMM: gasnet *
  CHPL_COMM_SUBSTRATE: udp
  CHPL_GASNET_SEGMENT: everything
CHPL_TASKS: fifo *
CHPL_LAUNCHER: amudprun
CHPL_TIMERS: generic
CHPL_UNWIND: none
CHPL_HOST_MEM: cstdlib
CHPL_MEM: cstdlib *
CHPL_ATOMICS: cstdlib
  CHPL_NETWORK_ATOMICS: none
CHPL_GMP: system *
CHPL_HWLOC: none *
CHPL_RE2: bundled *
CHPL_LLVM: system
  CHPL_LLVM_SUPPORT: system
  CHPL_LLVM_CONFIG: /opt/homebrew/opt/llvm@19/bin/llvm-config
  CHPL_LLVM_VERSION: 19
CHPL_AUX_FILESYS: none
CHPL_LIB_PIC: none
CHPL_SANITIZE: none
CHPL_SANITIZE_EXE: address *

Furthermore, this is only an issue when the variables are declared as globals. In the above example, wrapping the code inside of an explicit main() function causes the variables to be properly initialized.

My reading of the generated C code is that we intend to memset the variable to 0, but end of not doing so.

Focusing on just printR(g), I compared the generated C;

writeln("print1");
var g: R;
printR(g);

With COMM=none and g at global scope

  writeln_chpl(&_str_literal_2642_chpl /* "print1" */);
  memset(&g_chpl, INT32(0), sizeof(R));
  printR_chpl(&g_chpl);

In this case, g_chpl is declared as static R g_chpl at global scope.

With COMM=gasnet and g at local scope

  writeln_chpl(&_str_literal_2644_chpl /* "print1" */);
  memset(&g_chpl, INT32(0), sizeof(R));
  printR_chpl(&g_chpl);

In this case, g_chpl is declared as R g_chpl at local scope, inside of chpl_user_main.

With COMM=gasnet and g at global scope (i.e. the problem case)

  writeln_chpl(&_str_literal_2644_chpl /* "print1" */);
  chpl_check_nil((&g_chpl)->addr, INT64(19), INT32(93));
  chpl_macro_tmp_4208.locale = (&g_chpl)->locale;
  chpl_macro_tmp_4208.addr = &(((&g_chpl)->addr)->value);
  chpl_gen_comm_get(((void*)(&tmp_chpl)), chpl_nodeFromLocaleID((chpl_macro_tmp_4208).locale, INT64(0), INT32(0)), (chpl_macro_tmp_4208).addr, sizeof(R), COMMID(0), INT64(19), INT64(93));
  memset(&tmp_chpl, INT32(0), sizeof(R));
  chpl_check_nil((&g_chpl)->addr, INT64(19), INT32(93));
  chpl_macro_tmp_4209.locale = (&g_chpl)->locale;
  chpl_macro_tmp_4209.addr = &(((&g_chpl)->addr)->value);
  tmp_chpl2 = chpl_macro_tmp_4209;
  printR_chpl(tmp_chpl2);

Because g: R is at global scope (and we have compiled with COMM!=none), g_chpl is now a wide pointer. And we still have the same memset code to initialize it to zero, but it has no effect. The reason it has not effect is due to chpl_gen_comm_get. We call that on g_chpl to copy the actual R struct value locally and then call memset on that temporary. But we never write that temporary back to the wide pointer (with a put). So the memory pointed to by g_chpl is still uninitialized and the memset effectively does a deadstore.

I confirmed that hacking the C code and recompiling with the following means that g_chpl is no longer uninitialized

// after memset(&tmp_chpl, INT32(0), sizeof(R));
  chpl_gen_comm_put(((void*)(&tmp_chpl)), chpl_nodeFromLocaleID((chpl_macro_tmp_4208).locale, INT64(0), INT32(0)), (chpl_macro_tmp_4208).addr, sizeof(R), COMMID(1), INT64(0), INT64(0));
@jabraham17 jabraham17 changed the title [Bug]: Chapel variables with extern types may not be default intialized [Bug]: Chapel variables with extern types may not be default initialized Dec 13, 2024
@mppf
Copy link
Member

mppf commented Dec 16, 2024

PR #18542 tried to fix this but apparently missed some cases

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants