Details

Reviewers

nlopes
aqjune
regehr
hfinkel
reames
lebedev.ri
craig.topper
deadalnix
nikic
RKSimon
whitequark

Commits

rG345fcccb3379: Fix use-of-uninitialized-value in rG75f50e15bf8f
rG75f50e15bf8f: Adding PoisonValue for representing poison value explicitly in IR

Summary

Define ConstantData::PoisonValue.
Add support for poison value to LLLexer/LLParser/BitcodeReader/BitcodeWriter.
Add support for poison value to llvm-c interface.
Add support for poison value to OCaml binding.
Add m_Poison in PatternMatch.

Diff Detail

Repository: rG LLVM Github Monorepo

Event Timeline

liuz created this revision.Dec 6 2019, 8:57 AM

Herald added a reviewer: whitequark. · View Herald TranscriptDec 6 2019, 8:57 AM

Herald added a reviewer: deadalnix. · View Herald Transcript

Herald added a project: Restricted Project. · View Herald Transcript

Herald added subscribers: llvm-commits, dexonsmith, steven_wu and 2 others. · View Herald Transcript

liuz edited the summary of this revision. (Show Details)Dec 6 2019, 9:18 AM

whitequark resigned from this revision.Mar 10 2020, 2:43 PM

RKSimon resigned from this revision.Aug 2 2020, 3:03 AM

liuz updated this revision to Diff 296566.Oct 6 2020, 5:21 PM

Herald added a subscriber: pengfei. · View Herald TranscriptOct 6 2020, 5:21 PM

liuz retitled this revision from Add Poison Value to Adding PoisonValue for representing poison value explicitly in IR.Oct 6 2020, 5:32 PM

Harbormaster completed remote builds in B74201: Diff 296566.Oct 6 2020, 5:46 PM

I would recommend making PoisonValue a subclass of UndefValue. Poison can always be relaxed to undef, and this will prevents lots of regressions when replacing undef with poison values.

In D71126#2316089, @nikic wrote:

I would recommend making PoisonValue a subclass of UndefValue. Poison can always be relaxed to undef, and this will prevents lots of regressions when replacing undef with poison values.

+1 for this. You'll need to update UndefValue::classof.

PoisonValue is now a subclass of UndefValue

liuz edited the summary of this revision. (Show Details)Oct 8 2020, 11:00 PM

Harbormaster completed remote builds in B74530: Diff 297125.Oct 8 2020, 11:25 PM

rebased to upstream

Harbormaster completed remote builds in B80013: Diff 307448.Nov 24 2020, 2:43 PM

LGTM.

p.s. I'm very unsure about the merits of having poison being a sub-class of undef, but I'm also completely fine landing this as is and adjusting later. Being able to spell poison directly in tests is a huge win, and I'm more than comfortable evolving in tree as warranted.

This revision is now accepted and ready to land.Nov 24 2020, 3:46 PM

This revision was landed with ongoing or failed builds.Nov 25 2020, 4:44 PM

Closed by commit rG75f50e15bf8f: Adding PoisonValue for representing poison value explicitly in IR (authored by liuz). · Explain Why

This revision was automatically updated to reflect the committed changes.

liuz added a commit: rG75f50e15bf8f: Adding PoisonValue for representing poison value explicitly in IR.

http://lab.llvm.org:8011/#/builders/5/builds/1690/steps/9/logs/stdio

74536==WARNING: MemorySanitizer: use-of-uninitialized-value

#0 0x7c7b3ed in llvm::Value::setValueName(llvm::StringMapEntry<llvm::Value*>*) /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/lib/IR/Value.cpp:281:3
#1 0x7c78ec6 in destroyValueName /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/lib/IR/Value.cpp:143:3
#2 0x7c78ec6 in llvm::Value::~Value() /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/lib/IR/Value.cpp:104:3
#3 0x7b389e6 in operator() /b/sanitizer-x86_64-linux-fast/build/libcxx_build_msan/include/c++/v1/memory:2122:5
#4 0x7b389e6 in reset /b/sanitizer-x86_64-linux-fast/build/libcxx_build_msan/include/c++/v1/memory:2383:7
#5 0x7b389e6 in ~unique_ptr /b/sanitizer-x86_64-linux-fast/build/libcxx_build_msan/include/c++/v1/memory:2337:19
#6 0x7b389e6 in destroyAll /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/include/llvm/ADT/DenseMap.h:368:25
#7 0x7b389e6 in llvm::DenseMap<llvm::Type*, std::__1::unique_ptr<llvm::PoisonValue, std::__1::default_delete<llvm::PoisonValue> >, llvm::DenseMapInfo<llvm::Type*>, llvm::detail::DenseMapPair<llvm::Type*, std::__1::unique_ptr<llvm::PoisonValue, std::__1::default_delete<llvm::PoisonValue> > > >::~DenseMap() /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/include/llvm/ADT/DenseMap.h:754:11
#8 0x7b2f91d in llvm::LLVMContextImpl::~LLVMContextImpl() /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/lib/IR/LLVMContextImpl.cpp:125:1
#9 0x7b19cff in llvm::LLVMContext::~LLVMContext() /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/lib/IR/LLVMContext.cpp:94:31
#10 0x2561bb6 in main /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/tools/llc/llc.cpp:366:1
#11 0x7f93b5e3309a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2409a)
#12 0x24e0df9 in _start (/b/sanitizer-x86_64-linux-fast/build/llvm_build_msan/bin/llc+0x24e0df9)

liuz added a commit: rG345fcccb3379: Fix use-of-uninitialized-value in rG75f50e15bf8f.Nov 26 2020, 12:39 AM

spatel mentioned this in D90479: [ValueTracking] ComputeKnownBits - minimum leading/trailing zero bits in LSHR/SHL (PR44526).Nov 30 2020, 4:58 AM

cuviper added a subscriber: cuviper.Jan 27 2021, 1:21 PM

cuviper added inline comments.

llvm/include/llvm-c/Core.h
272	This is a breaking change to the C ABI -- can we move it to the end of the enum? https://bugs.llvm.org/show_bug.cgi?id=48905

Please use GitHub pull requests for new patches. Avoid migrating existing patches. Phabricator shutdown timeline

Adding PoisonValue for representing poison value explicitly in IR
ClosedPublic
Actions

Details

Diff Detail

Event Timeline

74536==WARNING: MemorySanitizer: use-of-uninitialized-value

Revision Contents
Changeset List

Diff 307448

llvm/bindings/ocaml/llvm/llvm.ml

llvm/bindings/ocaml/llvm/llvm.mli

llvm/bindings/ocaml/llvm/llvm_ocaml.c

llvm/include/llvm-c/Core.h

llvm/include/llvm/Bitcode/LLVMBitCodes.h

llvm/include/llvm/IR/Constants.h

llvm/include/llvm/IR/PatternMatch.h

llvm/include/llvm/IR/Value.def

llvm/lib/AsmParser/LLLexer.cpp

llvm/lib/AsmParser/LLParser.h

llvm/lib/AsmParser/LLParser.cpp

llvm/lib/AsmParser/LLToken.h

llvm/lib/Bitcode/Reader/BitcodeReader.cpp

llvm/lib/Bitcode/Writer/BitcodeWriter.cpp

llvm/lib/IR/AsmWriter.cpp

llvm/lib/IR/Constants.cpp

llvm/lib/IR/Core.cpp

llvm/lib/IR/LLVMContextImpl.h

llvm/test/Bindings/OCaml/core.ml

llvm/test/Bitcode/compatibility.ll

llvm/test/CodeGen/X86/poison-ops.ll

llvm/tools/llvm-c-test/echo.cpp

Please use GitHub pull requests for new patches. Avoid migrating existing patches. Phabricator shutdown timeline

Adding PoisonValue for representing poison value explicitly in IRClosedPublicActions

Details

Diff Detail

Event Timeline

74536==WARNING: MemorySanitizer: use-of-uninitialized-value

Revision ContentsChangeset List

Diff 307448

llvm/bindings/ocaml/llvm/llvm.ml

llvm/bindings/ocaml/llvm/llvm.mli

llvm/bindings/ocaml/llvm/llvm_ocaml.c

llvm/include/llvm-c/Core.h

llvm/include/llvm/Bitcode/LLVMBitCodes.h

llvm/include/llvm/IR/Constants.h

llvm/include/llvm/IR/PatternMatch.h

llvm/include/llvm/IR/Value.def

llvm/lib/AsmParser/LLLexer.cpp

llvm/lib/AsmParser/LLParser.h

llvm/lib/AsmParser/LLParser.cpp

llvm/lib/AsmParser/LLToken.h

llvm/lib/Bitcode/Reader/BitcodeReader.cpp

llvm/lib/Bitcode/Writer/BitcodeWriter.cpp

llvm/lib/IR/AsmWriter.cpp

llvm/lib/IR/Constants.cpp

llvm/lib/IR/Core.cpp

llvm/lib/IR/LLVMContextImpl.h

llvm/test/Bindings/OCaml/core.ml

llvm/test/Bitcode/compatibility.ll

llvm/test/CodeGen/X86/poison-ops.ll

llvm/tools/llvm-c-test/echo.cpp

Adding PoisonValue for representing poison value explicitly in IR
ClosedPublic
Actions

Revision Contents
Changeset List