[PATCH] D44143: [clang-tidy] Create properly seeded random generator check
boga95 updated this revision to Diff 153655.
https://reviews.llvm.org/D44143
Files:
clang-tidy/cert/CERTTidyModule.cpp
clang-tidy/cert/CMakeLists.txt
clang-tidy/cert/ProperlySeededRandomGeneratorCheck.cpp
clang-tidy/cert/ProperlySeededRandomGeneratorCheck.h
docs/ReleaseNotes.rst
docs/clang-tidy/checks/cert-msc32-c.rst
docs/clang-tidy/checks/cert-msc51-cpp.rst
test/clang-tidy/cert-msc32-c.c
test/clang-tidy/cert-msc51-cpp.cpp
Index: test/clang-tidy/cert-msc51-cpp.cpp
===
--- /dev/null
+++ test/clang-tidy/cert-msc51-cpp.cpp
@@ -0,0 +1,210 @@
+// RUN: %check_clang_tidy %s cert-msc51-cpp %t -- -config="{CheckOptions: [{key: cert-msc51-cpp.DisallowedSeedTypes, value: 'some_type,time_t'}]}" -- -std=c++11
+
+namespace std {
+
+void srand(int seed);
+
+template
+struct linear_congruential_engine {
+ linear_congruential_engine(int _ = 0);
+ void seed(int _ = 0);
+};
+using default_random_engine = linear_congruential_engine;
+
+using size_t = int;
+template
+struct mersenne_twister_engine {
+ mersenne_twister_engine(int _ = 0);
+ void seed(int _ = 0);
+};
+using mt19937 = mersenne_twister_engine;
+
+template
+struct subtract_with_carry_engine {
+ subtract_with_carry_engine(int _ = 0);
+ void seed(int _ = 0);
+};
+using ranlux24_base = subtract_with_carry_engine;
+
+template
+struct discard_block_engine {
+ discard_block_engine();
+ discard_block_engine(int _);
+ void seed();
+ void seed(int _);
+};
+using ranlux24 = discard_block_engine;
+
+template
+struct independent_bits_engine {
+ independent_bits_engine();
+ independent_bits_engine(int _);
+ void seed();
+ void seed(int _);
+};
+using independent_bits = independent_bits_engine;
+
+template
+struct shuffle_order_engine {
+ shuffle_order_engine();
+ shuffle_order_engine(int _);
+ void seed();
+ void seed(int _);
+};
+using shuffle_order = shuffle_order_engine;
+
+struct random_device {
+ random_device();
+ int operator()();
+};
+} // namespace std
+
+using time_t = unsigned int;
+time_t time(time_t *t);
+
+void f() {
+ const int seed = 2;
+ time_t t;
+
+ std::srand(0);
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: random number generator seeded with a constant value will generate a predictable sequence of values [cert-msc51-cpp]
+ std::srand(seed);
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: random number generator seeded with a constant value will generate a predictable sequence of values [cert-msc51-cpp]
+ std::srand(time(&t));
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: random number generator seeded with a disallowed source of seed value will generate a predictable sequence of values [cert-msc51-cpp]
+
+ // One instantiation for every engine
+ std::default_random_engine engine1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:30: warning: random number generator seeded with a default argument will generate a predictable sequence of values [cert-msc51-cpp]
+ std::default_random_engine engine2(1);
+ // CHECK-MESSAGES: :[[@LINE-1]]:30: warning: random number generator seeded with a constant value will generate a predictable sequence of values [cert-msc51-cpp]
+ std::default_random_engine engine3(seed);
+ // CHECK-MESSAGES: :[[@LINE-1]]:30: warning: random number generator seeded with a constant value will generate a predictable sequence of values [cert-msc51-cpp]
+ std::default_random_engine engine4(time(&t));
+ // CHECK-MESSAGES: :[[@LINE-1]]:30: warning: random number generator seeded with a disallowed source of seed value will generate a predictable sequence of values [cert-msc51-cpp]
+ engine1.seed();
+ // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: random number generator seeded with a default argument will generate a predictable sequence of values [cert-msc51-cpp]
+ engine1.seed(1);
+ // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: random number generator seeded with a constant value will generate a predictable sequence of values [cert-msc51-cpp]
+ engine1.seed(seed);
+ // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: random number generator seeded with a constant value will generate a predictable sequence of values [cert-msc51-cpp]
+ engine1.seed(time(&t));
+ // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: random number generator seeded with a disallowed source of seed value will generate a predictable sequence of values [cert-msc51-cpp]
+
+ std::mt19937 engine5;
+ // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: random number generator seeded with a default argument will generate a predictable sequence of values [cert-msc51-cpp]
+ std::mt19937 engine6(1);
+ // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: random number generator seeded with a constant value will generate a predictable sequence of values [cert-msc51-cpp]
+ std::mt19937 engine7(seed);
+ // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: random number generator seeded with a constant value will generate a predictable sequence of values [cert-msc51-cpp]
+ std::mt19937 engine8(time(&t));
[PATCH] D48721: Patch to fix pragma metadata for do-while loops
deepak2427 added a comment. Added to Bugzilla, https://bugs.llvm.org/show_bug.cgi?id=38011 Repository: rC Clang https://reviews.llvm.org/D48721 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47103: Implement strip.invariant.group
This revision was automatically updated to reflect the committed changes.
Prazek marked an inline comment as done.
Closed by commit rL336073: Implement strip.invariant.group (authored by Prazek,
committed by ).
Changed prior to commit:
https://reviews.llvm.org/D47103?vs=148313&id=153664#toc
Repository:
rL LLVM
https://reviews.llvm.org/D47103
Files:
llvm/trunk/docs/LangRef.rst
llvm/trunk/include/llvm/IR/IRBuilder.h
llvm/trunk/include/llvm/IR/Intrinsics.td
llvm/trunk/lib/Analysis/BasicAliasAnalysis.cpp
llvm/trunk/lib/Analysis/ConstantFolding.cpp
llvm/trunk/lib/Analysis/ValueTracking.cpp
llvm/trunk/lib/CodeGen/CodeGenPrepare.cpp
llvm/trunk/lib/CodeGen/SelectionDAG/FastISel.cpp
llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
llvm/trunk/lib/IR/Value.cpp
llvm/trunk/lib/Target/AMDGPU/AMDGPUPromoteAlloca.cpp
llvm/trunk/test/Analysis/ValueTracking/invariant.group.ll
llvm/trunk/test/CodeGen/Generic/intrinsics.ll
llvm/trunk/test/Other/invariant.group.ll
llvm/trunk/test/Other/launder.invariant.group.ll
llvm/trunk/test/Transforms/CodeGenPrepare/invariant.group.ll
llvm/trunk/test/Transforms/DeadStoreElimination/launder.invariant.group.ll
llvm/trunk/test/Transforms/FunctionAttrs/nocapture.ll
llvm/trunk/test/Transforms/GVN/invariant.group.ll
llvm/trunk/test/Transforms/GlobalOpt/invariant.group.barrier.ll
llvm/trunk/test/Transforms/GlobalOpt/invariant.group.ll
llvm/trunk/test/Transforms/InstCombine/invariant.group.ll
llvm/trunk/test/Transforms/NewGVN/invariant.group.ll
Index: llvm/trunk/lib/IR/Value.cpp
===
--- llvm/trunk/lib/IR/Value.cpp
+++ llvm/trunk/lib/IR/Value.cpp
@@ -521,7 +521,8 @@
// but it can't be marked with returned attribute, that's why it needs
// special case.
if (StripKind == PSK_ZeroIndicesAndAliasesAndInvariantGroups &&
-CS.getIntrinsicID() == Intrinsic::launder_invariant_group) {
+(CS.getIntrinsicID() == Intrinsic::launder_invariant_group ||
+ CS.getIntrinsicID() == Intrinsic::strip_invariant_group)) {
V = CS.getArgOperand(0);
continue;
}
Index: llvm/trunk/lib/CodeGen/SelectionDAG/FastISel.cpp
===
--- llvm/trunk/lib/CodeGen/SelectionDAG/FastISel.cpp
+++ llvm/trunk/lib/CodeGen/SelectionDAG/FastISel.cpp
@@ -1437,6 +1437,7 @@
return true;
}
case Intrinsic::launder_invariant_group:
+ case Intrinsic::strip_invariant_group:
case Intrinsic::expect: {
unsigned ResultReg = getRegForValue(II->getArgOperand(0));
if (!ResultReg)
Index: llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
===
--- llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
+++ llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
@@ -5768,6 +5768,7 @@
case Intrinsic::annotation:
case Intrinsic::ptr_annotation:
case Intrinsic::launder_invariant_group:
+ case Intrinsic::strip_invariant_group:
// Drop the intrinsic, but forward the value
setValue(&I, getValue(I.getOperand(0)));
return nullptr;
Index: llvm/trunk/lib/CodeGen/CodeGenPrepare.cpp
===
--- llvm/trunk/lib/CodeGen/CodeGenPrepare.cpp
+++ llvm/trunk/lib/CodeGen/CodeGenPrepare.cpp
@@ -1702,6 +1702,7 @@
return true;
}
case Intrinsic::launder_invariant_group:
+case Intrinsic::strip_invariant_group:
II->replaceAllUsesWith(II->getArgOperand(0));
II->eraseFromParent();
return true;
Index: llvm/trunk/lib/Target/AMDGPU/AMDGPUPromoteAlloca.cpp
===
--- llvm/trunk/lib/Target/AMDGPU/AMDGPUPromoteAlloca.cpp
+++ llvm/trunk/lib/Target/AMDGPU/AMDGPUPromoteAlloca.cpp
@@ -457,6 +457,7 @@
case Intrinsic::invariant_start:
case Intrinsic::invariant_end:
case Intrinsic::launder_invariant_group:
+ case Intrinsic::strip_invariant_group:
case Intrinsic::objectsize:
return true;
default:
@@ -882,6 +883,7 @@
case Intrinsic::invariant_start:
case Intrinsic::invariant_end:
case Intrinsic::launder_invariant_group:
+case Intrinsic::strip_invariant_group:
Intr->eraseFromParent();
// FIXME: I think the invariant marker should still theoretically apply,
// but the intrinsics need to be changed to accept pointers with any
Index: llvm/trunk/lib/Analysis/ValueTracking.cpp
===
--- llvm/trunk/lib/Analysis/ValueTracking.cpp
+++ llvm/trunk/lib/Analysis/ValueTracking.cpp
@@ -3404,8 +3404,9 @@
}
bool llvm::isIntrinsicReturningPointerAliasingArgumentWithoutCapturing(
- ImmutableCallSite CS) {
- return CS.getIntrinsicID() == Intrinsic::launder_invariant_group;
+ImmutableCallSite CS) {
+ return CS.getIntrinsicID(
[PATCH] D47299: [CodeGenCXX] Emit strip.invariant.group with -fstrict-vtable-pointers
Prazek marked 10 inline comments as done.
Prazek added inline comments.
Comment at: clang/lib/CodeGen/CGExprScalar.cpp:1647
+ }
+}
+
rsmith wrote:
> Prazek wrote:
> > rjmccall wrote:
> > > Prazek wrote:
> > > > rjmccall wrote:
> > > > > Incidentally, how do you protect against code like this?
> > > > >
> > > > > A *ptr;
> > > > > reinterpret_cast(ptr) = new B();
> > > > > ptr->foo();
> > > > >
> > > > > Presumably there needs to be a launder/strip here, but I guess it
> > > > > would have to be introduced by the middle-end when forwarding the
> > > > > store? The way I've written this is an aliasing violation, but (1) I
> > > > > assume your pass isn't disabled whenever strict-aliasing is disabled
> > > > > and (2) you can do this with a memcpy and still pretty reliably
> > > > > expect that LLVM will be able to eventually forward the store.
> > > > Can you add more info on what is A and B so I can make sure I
> > > > understand it correctly?
> > > > Is the prefix of the layout the same for both, but they are not in the
> > > > hierarchy?
> > > >
> > > > I haven't thought about the strict aliasing. I think the only sane way
> > > > would be to require strict aliasing for the strict vtable pointers.
> > > It's whatever case you're worried about such that you have to launder
> > > member accesses and bitcasts.
> > >
> > > And like I mentioned, relying on strict aliasing isn't enough because you
> > > can do it legally with memcpy. Maybe it's okay to consider it UB? I'm
> > > not sure about that.
> > AFAIK reinterpreting one class as another is UB if they are not in
> > hierarchy (especially calling virtual function on reinterpreted class), not
> > sure if strict aliasing should allow it anyway (if it would be a hand
> > written custom vptr then it should be ok with strict aliasing turned off,
> > but with vptr I don't think it is legal).
> > @rsmith Can you comment on that?
> OK, here's how I think about what we're doing here:
>
> We view the IR-level pointer value for a pointer to a dynamic class type as
> being a fat pointer, containing the actual pointer value and also a tag
> indicating the dynamic type of the object (only notionally, though -- the
> actual bit representation of the pointer doesn't include the extra
> information, but we don't ever emit IR that inspects the bit representation
> of the fat pointer to avoid exposing that fact). In that model, if you try to
> type pun between a pointer to a dynamic class type and a pointer to a
> non-dynamic-class type, that can't be expected to work because the (notional)
> value is different, much as type punning between a derived and base class
> pointer wouldn't work for a pointer to something other than a base class at
> offset zero.
>
> I think @rjmccall's example is OK, because both `A` and `B` would need to be
> dynamic class types in a hierarchy to work, and that means we'd be using the
> same notional pointer representation. A slight variation of that example:
>
> ```
> struct A {};
> struct B : A { virtual void f(); };
> struct C : B { void f(); } c;
> A *p = &c;
> B *q;
> memcpy(&q, &p, sizeof(B*)); // or q = std::bit_cast(p);
> q->f();
> ```
>
> ... would be UB, because the representation of an `A*` and a `B*` are
> different (a `B*` contains a tag and an `A*` does not).
Does this answer satisfy you John? Can I push it to trunk?
Repository:
rL LLVM
https://reviews.llvm.org/D47299
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
