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

CSI allocfn compiler crash #267

Open
DanielDeLayo opened this issue Aug 29, 2024 · 2 comments
Open

CSI allocfn compiler crash #267

DanielDeLayo opened this issue Aug 29, 2024 · 2 comments
Labels
bug Something isn't working

Comments

@DanielDeLayo
Copy link

Describe the bug

Enabling allocfn instrumentation using CSI when the code contains strdup causes a compiler crash.
We use cilkscale for a minimal example, but the crash is reproducible on custom tools.
We reproduce the crash with strdup because other allocfns, such as malloc, do not cause a crash but are not instrumented.

Expected behavior

The code to be compiled successfully and strdup and malloc to be instrumented by CSI

OpenCilk version

Built from source:

  • opencilk-project: df80b7d
  • cheetah: 5a80d8a534e8d56a5e354997bbfea23257c1c36b
  • productivity-tools: 4493af865e411dd9288c1af40d3358543281e0d2
  • infrastructure: release ee22572e020e96b0ae1d05c046c201281ee713e3

System information

  • OS: Rocky Linux 9.4 (Blue Onyx)
  • CPU: AMD EPYC 7643 48-Core Processor
  • (Running on a node of SBU's seawulf cluster)

Steps to reproduce (include relevant output)

  1. git clone -b opencilk/v2.1 https://github.com/OpenCilk/infrastructure and infrastructure/tools/get $(pwd)/opencilk

  2. Comment out opencilk/clang/lib/CodeGen/BackendUtil.cpp line 281
    That is, comment out Options.InstrumentAllocFns = false; in the function static CSIOptions getCSIOptionsForCilkscale(bool InstrumentBasicBlocks)

  3. Build with infrastructure/tools/build $(pwd)/opencilk $(pwd)/build

  4. write minimal example to strdup.cpp

#include <cilk/cilk.h>
#include <string.h>

int main()
{
        strdup("a");
        return 0;
}
  1. Compile with build/bin/clang++ -fcilktool=cilkscale -fopencilk strdup.cpp

  2. Compiler crashes
    (.txt added to filenames so github will allow them to be attached)
    out.txt
    strdup-43bc7f.cpp.txt
    strdup-43bc7f.sh.txt

@DanielDeLayo DanielDeLayo added the bug Something isn't working label Aug 29, 2024
@neboat
Copy link
Collaborator

neboat commented Sep 1, 2024

Thanks for the bug report. I managed to reproduce this bug on my end.

I believe I have a draft change to fix this bug. But the change is fairly substantial, so it might be hard to accommodate as a patch to your OpenCilk compiler.

There's a simpler change you can apply as a patch, but with it, you'll need to use something like function interpositioning to instrument strdup.

Alternatively, depending on what you're doing, you might be able to use Cilksan's instrumentation hooks, which are similar to the Cilkscale/CSI instrumentation hooks. Cilksan implements a fix for instrumenting strdup.

Let me know what you would prefer for a short-term workaround, patch, or fix for this issue.

@DanielDeLayo
Copy link
Author

@kennyzzhang and I are happy to accept the draft change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants