This is an archive of the discontinued LLVM Phabricator instance.

[LICM] Check for noalias call instead of alloc like fn

Authored by nikic on Jan 6 2022, 1:34 AM.



When determining whether the memory is local to the function (and we can thus introduce spurious writes without thread-safety issues), check for a noalias call rather than the hardcoded list of memory allocation functions. Noalias calls are the more general way to determine allocation functions, as long as we're only interested in the property that the returned value is distinct from any other accessible memory.

Diff Detail

Event Timeline

nikic created this revision.Jan 6 2022, 1:34 AM
nikic requested review of this revision.Jan 6 2022, 1:34 AM
Herald added a project: Restricted Project. · View Herald TranscriptJan 6 2022, 1:34 AM
fhahn accepted this revision.Jan 6 2022, 3:23 AM

LGTM, thanks!

It seems at least DSE's isInvisibleToCallerAfterRet and isInvisibleToCallerBeforeRet could also use isNoAliasCall instead of isAllocLikeFn


While you are here, maybe also update this comment here?

This revision is now accepted and ready to land.Jan 6 2022, 3:23 AM
This revision was landed with ongoing or failed builds.Jan 6 2022, 5:39 AM
This revision was automatically updated to reflect the committed changes.
aganea added a subscriber: aganea.EditedJan 18 2022, 1:32 PM

Hello @nikic! It seems that rG3cef3cf02f09e397c471cf008060c89b34951959 is breaking some usages. While trying to land D108850, it was breaking on a two stage bot:
I managed to create a reproducer (to be applied on ToT):

The issue can be seen in lld/COFF/DriverUtils_test.cpp, essentially the pointer is moved out of mb but it is not copied in the buffer created for the placement new in make<std::unique_ptr<MemoryBuffer>>. The target buffer pointed by getSpecificAllocSingleton<T>().Allocate() is left untouched, which later fails on deletion.

To repro:

> cd ~/llvm-project/
> git checkout 3cef3cf02f09e397c471cf008060c89b34951959
> mkdir stage1 && cd stage1
> cmake ../llvm -GNinja -DLLVM_ENABLE_PROJECTS='llvm;lld;clang' -DLLVM_ENABLE_ASSERTIONS=ON -DCMAKE_BUILD_TYPE=Release -DCMAKE_C_COMPILER=/usr/bin/clang-13 -DCMAKE_CXX_COMPILER=/usr/bin/clang++-13 -DCMAKE_LINKER=/usr/bin/ld.lld-13
> ninja

and then:

> cd ~/llvm-project/
> git checkout main && git pull && git checkout -b test_bug
> git apply lldCommon_bug.patch
> mkdir stage2 && cd stage2
> cmake ../llvm -GNinja -DLLVM_ENABLE_PROJECTS='llvm;lld' -DLLVM_ENABLE_ASSERTIONS=ON -DCMAKE_BUILD_TYPE=Release -DCMAKE_C_COMPILER=/home/ubuntu/llvm-project/stage1/bin/clang -DCMAKE_CXX_COMPILER=/home/ubuntu/llvm-project/stage1/bin/clang++ -DCMAKE_LINKER=/home/ubuntu/llvm-project/stage1/bin/ld.lld
> ninja check-lld-coff
Testing Time: 7.67s
  Unsupported:   8
  Passed     :  13
  Failed     : 372
FAILED: tools/lld/test/CMakeFiles/check-lld-coff
cd /home/ubuntu/llvm-project/stage2/tools/lld/test && /usr/bin/python3.8 /home/ubuntu/llvm-project/stage2/./bin/llvm-lit -sv /home/ubuntu/llvm-project/lld/test/COFF
ninja: build stopped: subcommand failed.

Would you have a chance to look into it please?

nikic added a comment.Jan 18 2022, 1:42 PM

@aganea Could you please provide either the preprocessed source and cc1 invocation, or the unoptimized IR (-Xclang -disable-llvm-optzns) for the problematic file?

@nikic Please see:

The outcome is that we're not moving the pointer in the target buffer anymore:
(left side is git checkout 3cef3cf02f09e397c471cf008060c89b34951959^, right side is git checkout 3cef3cf02f09e397c471cf008060c89b34951959)

Let me know if there's anything else. Thanks in advance!

nikic added a comment.Jan 19 2022, 5:32 AM

@aganea Thanks. I've looked into this, and believe the DSE optimization is correct -- the issue is an incorrect noalias annotation in The BumpPtrAllocator returns memory that is already otherwise accessible to LLVM, which violates the noalias contract. It only becomes an actual issue in conjunction with DestroyAll in SpecificBumpPtrAllocator, which will scan over all allocations, and thus end up accessing the allocation through a pointer not based on the noalias return value.