[PATCH] D102822: [Clang][CodeGen] Set the size of llvm.lifetime to unknown for scalable types.

[Hsiangkai Wang via Phabricator via cfe-commits]

HsiangKai updated this revision to Diff 350035.
HsiangKai added a comment.

Simplify the test case.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102822/new/

https://reviews.llvm.org/D102822

Files:
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGDecl.cpp
  clang/lib/CodeGen/CGExprAgg.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/test/CodeGen/RISCV/riscv-v-lifetime.cpp

Index: clang/test/CodeGen/RISCV/riscv-v-lifetime.cpp
===
--- /dev/null
+++ clang/test/CodeGen/RISCV/riscv-v-lifetime.cpp
@@ -0,0 +1,25 @@
+// REQUIRES: riscv-registered-target
+// RUN: %clang_cc1 -std=c++11 -triple riscv64 -target-feature +experimental-v \
+// RUN:   -O1 -disable-llvm-passes -emit-llvm -o - %s | FileCheck %s
+
+#include 
+
+vint32m1_t Baz();
+
+// CHECK-LABEL: @_Z4Testv(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[A:%.*]] = alloca *, align 8
+// CHECK-NEXT:[[REF_TMP:%.*]] = alloca , align 4
+// CHECK-NEXT:[[TMP0:%.*]] = bitcast ** [[A]] to i8*
+// CHECK-NEXT:call void @llvm.lifetime.start.p0i8(i64 8, i8* [[TMP0]]) #[[ATTR3:[0-9]+]]
+// CHECK-NEXT:[[TMP1:%.*]] = bitcast * [[REF_TMP]] to i8*
+// CHECK-NEXT:call void @llvm.lifetime.start.p0i8(i64 -1, i8* [[TMP1]]) #[[ATTR3]]
+// CHECK: [[TMP4:%.*]] = bitcast * [[REF_TMP]] to i8*
+// CHECK-NEXT:call void @llvm.lifetime.end.p0i8(i64 -1, i8* [[TMP4]]) #[[ATTR3]]
+// CHECK-NEXT:[[TMP5:%.*]] = bitcast ** [[A]] to i8*
+// CHECK-NEXT:call void @llvm.lifetime.end.p0i8(i64 8, i8* [[TMP5]]) #[[ATTR3]]
+//
+vint32m1_t Test() {
+  const vint32m1_t &a = Baz();
+  return a;
+}
Index: clang/lib/CodeGen/CodeGenFunction.h
===
--- clang/lib/CodeGen/CodeGenFunction.h
+++ clang/lib/CodeGen/CodeGenFunction.h
@@ -2872,7 +2872,7 @@
   void EmitSehTryScopeBegin();
   void EmitSehTryScopeEnd();
 
-  llvm::Value *EmitLifetimeStart(uint64_t Size, llvm::Value *Addr);
+  llvm::Value *EmitLifetimeStart(llvm::TypeSize Size, llvm::Value *Addr);
   void EmitLifetimeEnd(llvm::Value *Size, llvm::Value *Addr);
 
   llvm::Value *EmitCXXNewExpr(const CXXNewExpr *E);
Index: clang/lib/CodeGen/CGExprAgg.cpp
===
--- clang/lib/CodeGen/CGExprAgg.cpp
+++ clang/lib/CodeGen/CGExprAgg.cpp
@@ -276,7 +276,7 @@
 RetAddr = Dest.getAddress();
   } else {
 RetAddr = CGF.CreateMemTemp(RetTy, "tmp", &RetAllocaAddr);
-uint64_t Size =
+llvm::TypeSize Size =
 CGF.CGM.getDataLayout().getTypeAllocSize(CGF.ConvertTypeForMem(RetTy));
 LifetimeSizePtr = CGF.EmitLifetimeStart(Size, RetAllocaAddr.getPointer());
 if (LifetimeSizePtr) {
Index: clang/lib/CodeGen/CGDecl.cpp
===
--- clang/lib/CodeGen/CGDecl.cpp
+++ clang/lib/CodeGen/CGDecl.cpp
@@ -1317,7 +1317,7 @@
 /// Emit a lifetime.begin marker if some criteria are satisfied.
 /// \return a pointer to the temporary size Value if a marker was emitted, null
 /// otherwise
-llvm::Value *CodeGenFunction::EmitLifetimeStart(uint64_t Size,
+llvm::Value *CodeGenFunction::EmitLifetimeStart(llvm::TypeSize Size,
 llvm::Value *Addr) {
   if (!ShouldEmitLifetimeMarkers)
 return nullptr;
@@ -1325,7 +1325,8 @@
   assert(Addr->getType()->getPointerAddressSpace() ==
  CGM.getDataLayout().getAllocaAddrSpace() &&
  "Pointer should be in alloca address space");
-  llvm::Value *SizeV = llvm::ConstantInt::get(Int64Ty, Size);
+  llvm::Value *SizeV = llvm::ConstantInt::get(
+  Int64Ty, Size.isScalable() ? -1 : Size.getFixedValue());
   Addr = Builder.CreateBitCast(Addr, AllocaInt8PtrTy);
   llvm::CallInst *C =
   Builder.CreateCall(CGM.getLLVMLifetimeStartFn(), {SizeV, Addr});
@@ -1548,12 +1549,9 @@
 // is rare.
 if (!Bypasses.IsBypassed(&D) &&
 !(!getLangOpts().CPlusPlus && hasLabelBeenSeenInCurrentScope())) {
-  llvm::TypeSize size =
-  CGM.getDataLayout().getTypeAllocSize(allocaTy);
+  llvm::TypeSize Size = CGM.getDataLayout().getTypeAllocSize(allocaTy);
   emission.SizeForLifetimeMarkers =
-  size.isScalable() ? EmitLifetimeStart(-1, AllocaAddr.getPointer())
-: EmitLifetimeStart(size.getFixedSize(),
-AllocaAddr.getPointer());
+  EmitLifetimeStart(Size, AllocaAddr.getPointer());
 }
   } else {
 assert(!emission.useLifetimeMarkers());
Index: clang/lib/CodeGen/CGCall.cpp
===
--- clang/lib/CodeGen/CGCall.cpp
+++ clang/lib/CodeGen/CGCall.cpp
@@ -4679,7 +4679,7 @@
 } else {
   SRetPtr = CreateMemTemp(RetTy, "tmp", &SRetAlloca);
   if (HaveInsertPoint() && ReturnValue.isUnused()) {
-uint64

[PATCH] D103440: [WIP][analyzer] Introduce range-based reasoning for addition operator

[Valeriy Savchenko via Phabricator via cfe-commits]

vsavchenko added a comment.

In D103440#2799629 , @xazax.hun wrote:

> I was wondering, if we could try something new with the tests. To increase 
> our confidence that the expected behavior is correct, how about including a 
> Z3 proof with each of the test cases?
>
> For example:
>
>   (declare-const a ( _ BitVec 32 ))
>   (declare-const b ( _ BitVec 32 ))
>   
>   ; Assume 0 <= a, b, <= 5
>   (assert (bvsge a #x))
>   (assert (bvsge b #x))
>   (assert (bvsle a #x0005))
>   (assert (bvsle b #x0005))
>   
>   ; Check if 0 <= a + b  <= 10
>   (assert (not (and (bvsle (bvadd a b) #x000A) (bvsge (bvadd a b) 
> #x
>   
>   (check-sat)
>
> Of course, the example above is obvious, but with overflows and other corner 
> cases it is great to have a tool doublecheck our assumptions, and share how 
> to reproduce the results.

I usually add things like this not about the tests, but about the 
implementation. I think it provides a bit more generality.  But usually I do it 
as a comment to the patchset, maybe it is also good to put it in the code as 
well.

In D103440#2800122 , @manas wrote:

> In D103440#2799629 , @xazax.hun 
> wrote:
>
>> I was wondering, if we could try something new with the tests. To increase 
>> our confidence that the expected behavior is correct, how about including a 
>> Z3 proof with each of the test cases?
>
> We are looking forward to design a unit-test framework for the solver which 
> can compact the test cases and make them much more manageable (unlike 
> `constant-folding.c`). Perhaps, we can incorporate the Z3 proves in that 
> framework, corresponding to test cases.

Hmm, so you mean we can check if the analyzer was compiled with Z3 and if so, 
verify the same things by it?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D103440/new/

https://reviews.llvm.org/D103440

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D103749: [clang][driver] Add -foperator-names

[Markus Böck via Phabricator via cfe-commits]

zero9178 created this revision.
zero9178 added reviewers: MaskRay, thakis, hans.
Herald added a subscriber: dang.
zero9178 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This patch adds the command line option -foperator-names which acts as the 
opposite of -fno-operator-names. With this command line option it is possible 
to reenable C++ operator keywords on the command line if -fno-operator-names 
had previously been passed.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D103749

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/cxx-operator-names.cpp


Index: clang/test/Driver/cxx-operator-names.cpp
===
--- /dev/null
+++ clang/test/Driver/cxx-operator-names.cpp
@@ -0,0 +1,7 @@
+// RUN: %clang -### -S -foperator-names -fno-operator-names %s 2>&1 | \
+// RUN:   FileCheck -check-prefix=CHECK-1 %s
+// CHECK-1: "-fno-operator-names"
+
+// RUN: %clang -### -S -fno-operator-names -foperator-names %s 2>&1 | \
+// RUN:   FileCheck -check-prefix=CHECK-2 %s
+// CHECK-2-NOT: "-fno-operator-names"
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -5655,11 +5655,14 @@
 
   Args.AddLastArg(CmdArgs, options::OPT_ftlsmodel_EQ);
 
+  if (Args.hasFlag(options::OPT_fno_operator_names,
+   options::OPT_foperator_names, false))
+CmdArgs.push_back("-fno-operator-names");
+
   // Forward -f (flag) options which we can pass directly.
   Args.AddLastArg(CmdArgs, options::OPT_femit_all_decls);
   Args.AddLastArg(CmdArgs, options::OPT_fheinous_gnu_extensions);
   Args.AddLastArg(CmdArgs, options::OPT_fdigraphs, options::OPT_fno_digraphs);
-  Args.AddLastArg(CmdArgs, options::OPT_fno_operator_names);
   Args.AddLastArg(CmdArgs, options::OPT_femulated_tls,
   options::OPT_fno_emulated_tls);
 
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -2217,9 +2217,10 @@
 def fno_objc_legacy_dispatch : Flag<["-"], "fno-objc-legacy-dispatch">, 
Group;
 def fno_objc_weak : Flag<["-"], "fno-objc-weak">, Group, 
Flags<[CC1Option]>;
 def fno_omit_frame_pointer : Flag<["-"], "fno-omit-frame-pointer">, 
Group;
-def fno_operator_names : Flag<["-"], "fno-operator-names">, Group,
-  HelpText<"Do not treat C++ operator name keywords as synonyms for 
operators">,
-  Flags<[CC1Option]>, 
MarshallingInfoNegativeFlag, cplusplus.KeyPath>;
+defm operator_names : BoolFOption<"operator-names",
+  LangOpts<"CXXOperatorNames">, Default,
+  NegFlag,
+  PosFlag>;
 def fdiagnostics_absolute_paths : Flag<["-"], "fdiagnostics-absolute-paths">, 
Group,
   Flags<[CC1Option, CoreOption]>, HelpText<"Print absolute paths in 
diagnostics">,
   MarshallingInfoFlag>;


Index: clang/test/Driver/cxx-operator-names.cpp
===
--- /dev/null
+++ clang/test/Driver/cxx-operator-names.cpp
@@ -0,0 +1,7 @@
+// RUN: %clang -### -S -foperator-names -fno-operator-names %s 2>&1 | \
+// RUN:   FileCheck -check-prefix=CHECK-1 %s
+// CHECK-1: "-fno-operator-names"
+
+// RUN: %clang -### -S -fno-operator-names -foperator-names %s 2>&1 | \
+// RUN:   FileCheck -check-prefix=CHECK-2 %s
+// CHECK-2-NOT: "-fno-operator-names"
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -5655,11 +5655,14 @@
 
   Args.AddLastArg(CmdArgs, options::OPT_ftlsmodel_EQ);
 
+  if (Args.hasFlag(options::OPT_fno_operator_names,
+   options::OPT_foperator_names, false))
+CmdArgs.push_back("-fno-operator-names");
+
   // Forward -f (flag) options which we can pass directly.
   Args.AddLastArg(CmdArgs, options::OPT_femit_all_decls);
   Args.AddLastArg(CmdArgs, options::OPT_fheinous_gnu_extensions);
   Args.AddLastArg(CmdArgs, options::OPT_fdigraphs, options::OPT_fno_digraphs);
-  Args.AddLastArg(CmdArgs, options::OPT_fno_operator_names);
   Args.AddLastArg(CmdArgs, options::OPT_femulated_tls,
   options::OPT_fno_emulated_tls);
 
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -2217,9 +2217,10 @@
 def fno_objc_legacy_dispatch : Flag<["-"], "fno-objc-legacy-dispatch">, Group;
 def fno_objc_weak : Flag<["-"], "fno-objc-weak">, Group, Flags<[CC1Option]>;
 def fno_omit_frame_pointer : Flag<["-"], "fno-omit-frame-pointer">, Group;
-def fno_operator_names : Flag<["-"], "fno-operator-names">, Group,
-  HelpText<"

[PATCH] D103750: [analyzer] Handle std::make_unique for SmartPtrModeling

[Deep Majumder via Phabricator via cfe-commits]

RedDocMD created this revision.
RedDocMD added reviewers: NoQ, vsavchenko, xazax.hun, teemperor.
Herald added subscribers: manas, steakhal, ASDenysPetrov, martong, dkrupp, 
donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, rnkovacs, szepet, 
baloghadamsoftware.
RedDocMD requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This patch models the std::make_unique method.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D103750

Files:
  clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp


Index: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
+++ clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
@@ -40,6 +40,7 @@
  check::LiveSymbols> {
 
   bool isBoolConversionMethod(const CallEvent &Call) const;
+  bool isStdMakeUniqueCall(const CallEvent &Call) const;
 
 public:
   // Whether the checker should model for null dereferences of smart pointers.
@@ -68,6 +69,7 @@
   bool updateMovedSmartPointers(CheckerContext &C, const MemRegion *ThisRegion,
 const MemRegion *OtherSmartPtrRegion) const;
   void handleBoolConversion(const CallEvent &Call, CheckerContext &C) const;
+  void handleMakeUnique(const CallEvent &Call, CheckerContext &C) const;
 
   using SmartPtrMethodHandlerFn =
   void (SmartPtrModeling::*)(const CallEvent &Call, CheckerContext &) 
const;
@@ -175,8 +177,31 @@
   return CD && CD->getConversionType()->isBooleanType();
 }
 
+bool SmartPtrModeling::isStdMakeUniqueCall(const CallEvent &Call) const {
+  if (Call.getKind() != CallEventKind::CE_Function)
+return false;
+  const auto *D = Call.getDecl();
+  if (!D)
+return false;
+  const auto *FTD = llvm::dyn_cast(D);
+  if (!FTD)
+return false;
+  if (FTD->getDeclName().isIdentifier()) {
+StringRef Name = FTD->getName();
+if (Name == "make_unique")
+  return true;
+  }
+  return false;
+}
+
 bool SmartPtrModeling::evalCall(const CallEvent &Call,
 CheckerContext &C) const {
+
+  if (isStdMakeUniqueCall(Call)) {
+handleMakeUnique(Call, C);
+return true;
+  }
+
   ProgramStateRef State = C.getState();
   if (!smartptr::isStdSmartPtrCall(Call))
 return false;
@@ -272,6 +297,21 @@
   return C.isDifferent();
 }
 
+void SmartPtrModeling::handleMakeUnique(const CallEvent &Call,
+CheckerContext &C) const {
+  const auto *OriginExpr = Call.getOriginExpr();
+  const auto *LocCtx = C.getLocationContext();
+  auto ExprVal = C.getSValBuilder().conjureSymbolVal(
+  OriginExpr, LocCtx, Call.getResultType(), C.blockCount());
+  ProgramStateRef State = C.getState();
+  State = State->BindExpr(OriginExpr, LocCtx, ExprVal);
+
+  // With make unique it is not possible to make a null smart pointer.
+  // So we can set the SVal for Call.getOriginExpr() to be non-null
+  State = State->assume(ExprVal, true);
+  C.addTransition(State);
+}
+
 void SmartPtrModeling::checkDeadSymbols(SymbolReaper &SymReaper,
 CheckerContext &C) const {
   ProgramStateRef State = C.getState();


Index: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
+++ clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
@@ -40,6 +40,7 @@
  check::LiveSymbols> {
 
   bool isBoolConversionMethod(const CallEvent &Call) const;
+  bool isStdMakeUniqueCall(const CallEvent &Call) const;
 
 public:
   // Whether the checker should model for null dereferences of smart pointers.
@@ -68,6 +69,7 @@
   bool updateMovedSmartPointers(CheckerContext &C, const MemRegion *ThisRegion,
 const MemRegion *OtherSmartPtrRegion) const;
   void handleBoolConversion(const CallEvent &Call, CheckerContext &C) const;
+  void handleMakeUnique(const CallEvent &Call, CheckerContext &C) const;
 
   using SmartPtrMethodHandlerFn =
   void (SmartPtrModeling::*)(const CallEvent &Call, CheckerContext &) const;
@@ -175,8 +177,31 @@
   return CD && CD->getConversionType()->isBooleanType();
 }
 
+bool SmartPtrModeling::isStdMakeUniqueCall(const CallEvent &Call) const {
+  if (Call.getKind() != CallEventKind::CE_Function)
+return false;
+  const auto *D = Call.getDecl();
+  if (!D)
+return false;
+  const auto *FTD = llvm::dyn_cast(D);
+  if (!FTD)
+return false;
+  if (FTD->getDeclName().isIdentifier()) {
+StringRef Name = FTD->getName();
+if (Name == "make_unique")
+  return true;
+  }
+  return false;
+}
+
 bool SmartPtrModeling::evalCall(const CallEvent &Call,
 CheckerContext &C) const {
+
+  if (isStdMakeUniqueCall(Call)) {
+handleMakeUnique(Call, C);
+return true;
+  }
+
   ProgramSta

[PATCH] D103440: [WIP][analyzer] Introduce range-based reasoning for addition operator

[Manas Gupta via Phabricator via cfe-commits]

manas added a comment.

In D103440#2800710 , @vsavchenko 
wrote:

> In D103440#2800122 , @manas wrote:
>
>> In D103440#2799629 , @xazax.hun 
>> wrote:
>>
>>> I was wondering, if we could try something new with the tests. To increase 
>>> our confidence that the expected behavior is correct, how about including a 
>>> Z3 proof with each of the test cases?
>>
>> We are looking forward to design a unit-test framework for the solver which 
>> can compact the test cases and make them much more manageable (unlike 
>> `constant-folding.c`). Perhaps, we can incorporate the Z3 proves in that 
>> framework, corresponding to test cases.
>
> Hmm, so you mean we can check if the analyzer was compiled with Z3 and if so, 
> verify the same things by it?

Yeah in some sense. But I think that having proof for every test case may 
become redundant for certain cases.

For e.g., consider two test cases for addition operator:

1. c == [0, 10] and d == [-10, 0] will result in (c + d) == [-10, 10]
2. c == d ==  [0, 10] will result in (c + d) == [0, 20]

But the first test case can be modeled as `c - (- d)`or `c - D`, that is,

- usage of subtraction binary operator : (c **-** D), and
- symmetrical inversion of range around origin (**-** d) for symbol `d`. This 
will shift the range from `[-10, 0]` to `[0, 10]`.

Considering having proof for every test case will make the proof for test-case 
1 kind of redundant.

So, I think we should go with @vsavchenko 's method of adding Z3 proof with the 
**implementation** (in code), instead of test cases themselves.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D103440/new/

https://reviews.llvm.org/D103440

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D101696: [Matrix] Implement C-style explicit type conversions in CXX for matrix types

[Saurabh Jha via Phabricator via cfe-commits]

SaurabhJha added a comment.

In D101696#2798713 , @SjoerdMeijer 
wrote:

> Perhaps for bonus points, update the Clang documentation in 
> https://clang.llvm.org/docs/LanguageExtensions.html#matrix-types with some 
> examples?

Can you please point me to how to submit patches to the documentation? I could 
only find links like https://clang.llvm.org/hacking.html which talk about code 
patches. Many thanks.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D101696/new/

https://reviews.llvm.org/D101696

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D103745: [dfsan] Add full fast8 support

[stephan.yichao.zhao via Phabricator via cfe-commits]

stephan.yichao.zhao added a comment.

The failed test cases from x64 debian > libFuzzer.libFuzzer::* seem related. 
They still use -dfsan-fast-16-labels.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D103745/new/

https://reviews.llvm.org/D103745

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D103745: [dfsan] Add full fast8 support

[stephan.yichao.zhao via Phabricator via cfe-commits]

stephan.yichao.zhao added inline comments.



Comment at: clang/docs/DataFlowSanitizer.rst:172-178
 assert(ij_label == 3);  // Verifies all of the above
 
+// Or, equivalently:
+assert(dfsan_has_label(ij_label, i_label));
+assert(dfsan_has_label(ij_label, j_label));
+assert(!dfsan_has_label(ij_label, k_label));
+

If we swap assert(ij_label == 3) with the 3 dfsan_has_label, the two equivalent 
blocks are close to each other.



Comment at: clang/docs/DataFlowSanitizerDesign.rst:60
 
-As stated above, the tool must track a large number of taint
-labels. This poses an implementation challenge, as most multiple-label
-tainting systems assign one label per bit to shadow storage, and
-union taint labels using a bitwise or operation. This will not scale
-to clients which use hundreds or thousands of taint labels, as the
-label union operation becomes O(n) in the number of supported labels,
-and data associated with it will quickly dominate the live variable
-set, causing register spills and hampering performance.
-
-Instead, a low overhead approach is proposed which is best-case O(log\
-:sub:`2` n) during execution. The underlying assumption is that
-the required space of label unions is sparse, which is a reasonable
-assumption to make given that we are optimizing for the case where
-applications mostly copy data from one place to another, without often
-invoking the need for an actual union operation. The representation
-of a taint label is a 16-bit integer, and new labels are allocated
-sequentially from a pool. The label identifier 0 is special, and means
-that the data item is unlabelled.
-
-When a label union operation is requested at a join point (any
-arithmetic or logical operation with two or more operands, such as
-addition), the code checks whether a union is required, whether the
-same union has been requested before, and whether one union label
-subsumes the other. If so, it returns the previously allocated union
-label. If not, it allocates a new union label from the same pool used
-for new labels.
-
-Specifically, the instrumentation pass will insert code like this
-to decide the union label ``lu`` for a pair of labels ``l1``
-and ``l2``:
-
-.. code-block:: c
-
-  if (l1 == l2)
-lu = l1;
-  else
-lu = __dfsan_union(l1, l2);
-
-The equality comparison is outlined, to provide an early exit in
-the common cases where the program is processing unlabelled data, or
-where the two data items have the same label.  ``__dfsan_union`` is
-a runtime library function which performs all other union computation.
+We use an 8-bit unsigned integers for the representation of a
+label. The label identifier 0 is special, and means that the data item

integer



Comment at: clang/docs/DataFlowSanitizerDesign.rst:65
+join point (any arithmetic or logical operation with two or more
+operands, such as addition), we can simply OR the two labels in O(1).
 

the labels, and each OR is in O(1).



Comment at: clang/docs/DataFlowSanitizerDesign.rst:68
+Users are responsible for managing the 8 integer labels (i.e., keeping
+track of what labels they have used so far, pick one that is yet
+unused, etc).

picking



Comment at: clang/docs/DataFlowSanitizerDesign.rst:74
 
 The following is the current memory layout for Linux/x86\_64:
 

memory layout



Comment at: clang/docs/DataFlowSanitizerDesign.rst:99
 associated directly with registers.  Loads will result in a union of
-all shadow labels corresponding to bytes loaded (which most of the
-time will be short circuited by the initial comparison) and stores will
-result in a copy of the label to the shadow of all bytes stored to.
+all shadow labels corresponding to bytes loaded and stores will result
+in a copy of the label to the shadow of all bytes stored to.

, and



Comment at: clang/docs/DataFlowSanitizerDesign.rst:100
+all shadow labels corresponding to bytes loaded and stores will result
+in a copy of the label to the shadow of all bytes stored to.
 

the label of a stored value



Comment at: compiler-rt/lib/dfsan/dfsan.cpp:209
 
-// Like __dfsan_union, but for use from the client or custom functions.  Hence
-// the equality comparison is done here before calling __dfsan_union.
+// Resolves the union of two unequal labels.
 SANITIZER_INTERFACE_ATTRIBUTE dfsan_label

After removing legacy mode. if our code does not check l1 != l2 in IR, the 
comments can be updated.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D103745/new/

https://reviews.llvm.org/D103745

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D100733: [clang] NFC: change uses of `Expr->getValueKind` into `is?Value`

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov added a comment.

I created a DR which proposes the renaming as rsmith suggested: 
https://reviews.llvm.org/D103720

In D100733#2773944 , @aaronpuchert 
wrote:

> A new value category feels like a global change for a local problem. We can 
> explain the behavior that we want without introducing a new value category: 
> either as a “combined” overload resolution for xvalue and lvalue, where all 
> candidates for the xvalue are strictly better than any candidate for the 
> lvalue, or as a two-phase resolution that falls back only if there are no 
> candidates. These explanations are equivalent.

Sure, I had it in mind that is not necessarily how that would be explained to 
the users, but perhaps as the standard text is more geared towards the 
toolchain folks, the original explanation might be more suited there, and this 
alternative one is more didactic, as you pointed out it can be explained just 
in the context of implicit moves.

> My impression is that some people didn't like the changes that came with 
> C++11 and that it made the terminology of C and C++ somewhat inconsistent. 
> (Though I think you can work with the C++ terminology in C, there are just no 
> xvalues, but you can't work the other way around.)

Thanks. That makes sense!

In D100733#2773944 , @aaronpuchert 
wrote:

> But perhaps I would not introduce for now an `isRValue` function. (Although C 
> doesn't have xvalues, so maybe it doesn't matter?)



In D100733#2761031 , @rsmith wrote:

> What do we think about renaming `isRValue()` to `isPRValue()` and renaming 
> `VK_RValue` to `VK_PRValue`, adding a "real" `isRValue()`, and then 
> performing this cleanup?

We might want to hold on to the second step there (adding back another 
isRValue) for a little while, to give time for people to rebase patches and 
such, so they get a friendlier build breakage instead of some more subtle issue.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100733/new/

https://reviews.llvm.org/D100733

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D103611: Correct the behavior of va_arg checking in C++

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman updated this revision to Diff 350060.
aaron.ballman added a comment.

Added some more logic and new tests.

The CI pipeline pointed out a valid issue which has now been corrected -- C++ 
was not properly handling the case where one type was `int` and the other was 
`unsigned int`, which are compatible types in C but not the same type in C++. 
The test adds some triples and a new test case to ensure that we are able to 
consistently test this property in both directions.

@efriedma, would you mind taking a second look just to be sure you're still 
happy with the fix?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D103611/new/

https://reviews.llvm.org/D103611

Files:
  clang/lib/Sema/SemaExpr.cpp
  clang/test/SemaCXX/varargs.cpp


Index: clang/test/SemaCXX/varargs.cpp
===
--- clang/test/SemaCXX/varargs.cpp
+++ clang/test/SemaCXX/varargs.cpp
@@ -1,5 +1,5 @@
-// RUN: %clang_cc1 -std=c++03 -verify %s
-// RUN: %clang_cc1 -std=c++11 -verify %s
+// RUN: %clang_cc1 -std=c++03 -Wno-c++11-extensions -triple i386-pc-unknown 
-verify %s
+// RUN: %clang_cc1 -std=c++11 -triple x86_64-apple-darwin9 -verify %s
 
 __builtin_va_list ap;
 
@@ -28,6 +28,33 @@
   };
 }
 
+// Ensure the correct behavior for promotable type UB checking.
+void promotable(int a, ...) {
+  enum Unscoped1 { One = 0x7FFF };
+  (void)__builtin_va_arg(ap, Unscoped1); // ok
+
+  enum Unscoped2 { Two = 0x };
+  (void)__builtin_va_arg(ap, Unscoped2); // ok
+
+  enum class Scoped { Three };
+  (void)__builtin_va_arg(ap, Scoped); // ok
+
+  enum Fixed : int { Four };
+  (void)__builtin_va_arg(ap, Fixed); // ok
+
+  enum FixedSmall : char { Five };
+  (void)__builtin_va_arg(ap, FixedSmall); // expected-warning {{second 
argument to 'va_arg' is of promotable type 'FixedSmall'; this va_arg has 
undefined behavior because arguments will be promoted to 'int'}}
+
+  enum FixedLarge : long long { Six };
+  (void)__builtin_va_arg(ap, FixedLarge); // ok
+
+  // Ensure that qualifiers are ignored.
+  (void)__builtin_va_arg(ap, const volatile int);  // ok
+
+  // Ensure that signed vs unsigned doesn't matter either.
+  (void)__builtin_va_arg(ap, unsigned int);
+}
+
 #if __cplusplus >= 201103L
 // We used to have bugs identifying the correct enclosing function scope in a
 // lambda.
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -15750,8 +15750,48 @@
 QualType PromoteType;
 if (TInfo->getType()->isPromotableIntegerType()) {
   PromoteType = Context.getPromotedIntegerType(TInfo->getType());
-  if (Context.typesAreCompatible(PromoteType, TInfo->getType()))
+  // [cstdarg.syn]p1 defers the C++ behavior to what the C standard says,
+  // and C2x 7.16.1.1p2 says, in part:
+  //   If type is not compatible with the type of the actual next argument
+  //   (as promoted according to the default argument promotions), the
+  //   behavior is undefined, except for the following cases:
+  // - both types are pointers to qualified or unqualified versions of
+  //   compatible types;
+  // - one type is a signed integer type, the other type is the
+  //   corresponding unsigned integer type, and the value is
+  //   representable in both types;
+  // - one type is pointer to qualified or unqualified void and the
+  //   other is a pointer to a qualified or unqualified character type.
+  // Given that type compatibility is the primary requirement (ignoring
+  // qualifications), you would think we could call typesAreCompatible()
+  // directly to test this. However, in C++, that checks for *same type*,
+  // which causes false positives when passing an enumeration type to
+  // va_arg. Instead, get the underlying type of the enumeration and pass
+  // that.
+  QualType UnderlyingType = TInfo->getType();
+  if (const auto *ET = UnderlyingType->getAs())
+UnderlyingType = ET->getDecl()->getIntegerType();
+  if (Context.typesAreCompatible(PromoteType, UnderlyingType,
+ /*CompareUnqualified*/ true))
 PromoteType = QualType();
+
+  // If the types are still not compatible, in C++ we need to test whether
+  // the promoted type and the underlying type are the same except for
+  // signedness. Ask the AST for the correctly corresponding type and see
+  // if that's compatible. We don't need to do this in C because the above
+  // test for typesAreCompatible() will already properly consider those to
+  // be compatible types.
+  if (Context.getLangOpts().CPlusPlus && !PromoteType.isNull() &&
+  PromoteType->isUnsignedIntegerType() !=
+  UnderlyingType->isUnsignedIntegerType()) {
+UnderlyingType =
+UnderlyingType->isUnsignedIntegerTy

[PATCH] D103750: [analyzer][WIP] Handle std::make_unique for SmartPtrModeling

[Deep Majumder via Phabricator via cfe-commits]

RedDocMD updated this revision to Diff 350067.
RedDocMD added a comment.

Accounting for std::make_unique_for_overwrite


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D103750/new/

https://reviews.llvm.org/D103750

Files:
  clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp

Index: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
+++ clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
@@ -35,11 +35,19 @@
 using namespace ento;
 
 namespace {
+
+enum class MakeUniqueKind {
+  Regular, // ie, std::make_unique
+  ForOverwrite, // ie, std::make_unique_for_overwrite
+  None // ie, is neither of the above two
+};
+
 class SmartPtrModeling
 : public Checker {
 
   bool isBoolConversionMethod(const CallEvent &Call) const;
+  MakeUniqueKind isStdMakeUniqueCall(const CallEvent &Call) const;
 
 public:
   // Whether the checker should model for null dereferences of smart pointers.
@@ -68,6 +76,7 @@
   bool updateMovedSmartPointers(CheckerContext &C, const MemRegion *ThisRegion,
 const MemRegion *OtherSmartPtrRegion) const;
   void handleBoolConversion(const CallEvent &Call, CheckerContext &C) const;
+  void handleMakeUnique(const CallEvent &Call, CheckerContext &C, MakeUniqueKind Kind) const;
 
   using SmartPtrMethodHandlerFn =
   void (SmartPtrModeling::*)(const CallEvent &Call, CheckerContext &) const;
@@ -175,8 +184,34 @@
   return CD && CD->getConversionType()->isBooleanType();
 }
 
+MakeUniqueKind SmartPtrModeling::isStdMakeUniqueCall(const CallEvent &Call) const {
+  if (Call.getKind() != CallEventKind::CE_Function)
+return MakeUniqueKind::None;
+  const auto *D = Call.getDecl();
+  if (!D)
+return MakeUniqueKind::None;
+  const auto *FTD = llvm::dyn_cast(D);
+  if (!FTD)
+return MakeUniqueKind::None;
+  if (FTD->getDeclName().isIdentifier()) {
+StringRef Name = FTD->getName();
+if (Name == "make_unique")
+  return MakeUniqueKind::Regular;
+else if (Name == "make_unique_for_overwrite")
+  return MakeUniqueKind::ForOverwrite;
+  }
+  return MakeUniqueKind::None;
+}
+
 bool SmartPtrModeling::evalCall(const CallEvent &Call,
 CheckerContext &C) const {
+
+  MakeUniqueKind Kind = isStdMakeUniqueCall(Call);
+  if (Kind != MakeUniqueKind::None) {
+handleMakeUnique(Call, C, Kind);
+return true;
+  }
+
   ProgramStateRef State = C.getState();
   if (!smartptr::isStdSmartPtrCall(Call))
 return false;
@@ -214,7 +249,6 @@
   if (const auto *CC = dyn_cast(&Call)) {
 if (CC->getDecl()->isCopyConstructor())
   return false;
-
 const MemRegion *ThisRegion = CC->getCXXThisVal().getAsRegion();
 if (!ThisRegion)
   return false;
@@ -272,6 +306,33 @@
   return C.isDifferent();
 }
 
+void SmartPtrModeling::handleMakeUnique(const CallEvent &Call,
+CheckerContext &C, MakeUniqueKind Kind) const {
+  assert(Kind != MakeUniqueKind::None && "Call is not to make_unique or make_unique_for_overwrite");
+
+  const auto *OriginExpr = Call.getOriginExpr();
+  const auto *LocCtx = C.getLocationContext();
+
+  ProgramStateRef State = C.getState();
+
+  if (Kind == MakeUniqueKind::Regular) {
+  // With make unique it is not possible to make a null smart pointer.
+  // So we can set the SVal for Call.getOriginExpr() to be non-null
+  auto ExprVal = C.getSValBuilder().conjureSymbolVal(
+  OriginExpr, LocCtx, Call.getResultType(), C.blockCount());
+  State = State->assume(ExprVal, true);
+  State = State->BindExpr(OriginExpr, LocCtx, ExprVal);
+  } else {
+  // Technically, we are creating an uninitialized unique_ptr.
+  // So the value inside is *not* null, but we might as well set it
+  // so that the user gets better warnings.
+  auto ExprVal = C.getSValBuilder().makeZeroVal(Call.getResultType());
+  State = State->BindExpr(OriginExpr, LocCtx, ExprVal);
+  }
+
+  C.addTransition(State);
+}
+
 void SmartPtrModeling::checkDeadSymbols(SymbolReaper &SymReaper,
 CheckerContext &C) const {
   ProgramStateRef State = C.getState();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D103663: [AMDGPU] Add gfx1013 target

[Brendon Cahoon via Phabricator via cfe-commits]

bcahoon updated this revision to Diff 350075.
bcahoon added a comment.

Addressed review comments. Updated the patch to use the new AEncoding target 
feature
correctly. Added code to report an error for the image intersect intrinsics for
unsupported targets.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D103663/new/

https://reviews.llvm.org/D103663

Files:
  clang/include/clang/Basic/Cuda.h
  clang/lib/Basic/Cuda.cpp
  clang/lib/Basic/Targets/AMDGPU.cpp
  clang/lib/Basic/Targets/NVPTX.cpp
  clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
  clang/test/CodeGenOpenCL/amdgpu-features.cl
  clang/test/Driver/amdgpu-macros.cl
  clang/test/Driver/amdgpu-mcpu.cl
  clang/test/Misc/target-invalid-cpu-note.c
  llvm/docs/AMDGPUUsage.rst
  llvm/include/llvm/BinaryFormat/ELF.h
  llvm/include/llvm/Support/TargetParser.h
  llvm/lib/Object/ELFObjectFile.cpp
  llvm/lib/ObjectYAML/ELFYAML.cpp
  llvm/lib/Support/TargetParser.cpp
  llvm/lib/Target/AMDGPU/AMDGPU.td
  llvm/lib/Target/AMDGPU/AMDGPULegalizerInfo.cpp
  llvm/lib/Target/AMDGPU/AMDGPUSubtarget.cpp
  llvm/lib/Target/AMDGPU/GCNProcessors.td
  llvm/lib/Target/AMDGPU/GCNSubtarget.h
  llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUTargetStreamer.cpp
  llvm/lib/Target/AMDGPU/MIMGInstructions.td
  llvm/lib/Target/AMDGPU/SIISelLowering.cpp
  llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.cpp
  llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.h
  llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.intersect_ray.ll
  llvm/test/CodeGen/AMDGPU/directive-amdgcn-target.ll
  llvm/test/CodeGen/AMDGPU/elf-header-flags-mach.ll
  llvm/test/CodeGen/AMDGPU/llvm.amdgcn.intersect_ray.ll
  llvm/test/MC/AMDGPU/dl-insts-err.s
  llvm/test/MC/AMDGPU/gfx10_unsupported.s
  llvm/test/Object/AMDGPU/elf-header-flags-mach.yaml
  llvm/test/tools/llvm-objdump/ELF/AMDGPU/subtarget.ll
  llvm/test/tools/llvm-readobj/ELF/amdgpu-elf-headers.test
  llvm/tools/llvm-readobj/ELFDumper.cpp
  openmp/libomptarget/plugins/amdgpu/impl/get_elf_mach_gfx_name.cpp

Index: openmp/libomptarget/plugins/amdgpu/impl/get_elf_mach_gfx_name.cpp
===
--- openmp/libomptarget/plugins/amdgpu/impl/get_elf_mach_gfx_name.cpp
+++ openmp/libomptarget/plugins/amdgpu/impl/get_elf_mach_gfx_name.cpp
@@ -39,6 +39,8 @@
 return "gfx1011";
   case EF_AMDGPU_MACH_AMDGCN_GFX1012:
 return "gfx1012";
+  case EF_AMDGPU_MACH_AMDGCN_GFX1013:
+return "gfx1013";
   case EF_AMDGPU_MACH_AMDGCN_GFX1030:
 return "gfx1030";
   case EF_AMDGPU_MACH_AMDGCN_GFX1031:
Index: llvm/tools/llvm-readobj/ELFDumper.cpp
===
--- llvm/tools/llvm-readobj/ELFDumper.cpp
+++ llvm/tools/llvm-readobj/ELFDumper.cpp
@@ -1482,6 +1482,7 @@
   LLVM_READOBJ_ENUM_ENT(ELF, EF_AMDGPU_MACH_AMDGCN_GFX1010),
   LLVM_READOBJ_ENUM_ENT(ELF, EF_AMDGPU_MACH_AMDGCN_GFX1011),
   LLVM_READOBJ_ENUM_ENT(ELF, EF_AMDGPU_MACH_AMDGCN_GFX1012),
+  LLVM_READOBJ_ENUM_ENT(ELF, EF_AMDGPU_MACH_AMDGCN_GFX1013),
   LLVM_READOBJ_ENUM_ENT(ELF, EF_AMDGPU_MACH_AMDGCN_GFX1030),
   LLVM_READOBJ_ENUM_ENT(ELF, EF_AMDGPU_MACH_AMDGCN_GFX1031),
   LLVM_READOBJ_ENUM_ENT(ELF, EF_AMDGPU_MACH_AMDGCN_GFX1032),
@@ -1534,6 +1535,7 @@
   LLVM_READOBJ_ENUM_ENT(ELF, EF_AMDGPU_MACH_AMDGCN_GFX1010),
   LLVM_READOBJ_ENUM_ENT(ELF, EF_AMDGPU_MACH_AMDGCN_GFX1011),
   LLVM_READOBJ_ENUM_ENT(ELF, EF_AMDGPU_MACH_AMDGCN_GFX1012),
+  LLVM_READOBJ_ENUM_ENT(ELF, EF_AMDGPU_MACH_AMDGCN_GFX1013),
   LLVM_READOBJ_ENUM_ENT(ELF, EF_AMDGPU_MACH_AMDGCN_GFX1030),
   LLVM_READOBJ_ENUM_ENT(ELF, EF_AMDGPU_MACH_AMDGCN_GFX1031),
   LLVM_READOBJ_ENUM_ENT(ELF, EF_AMDGPU_MACH_AMDGCN_GFX1032),
Index: llvm/test/tools/llvm-readobj/ELF/amdgpu-elf-headers.test
===
--- llvm/test/tools/llvm-readobj/ELF/amdgpu-elf-headers.test
+++ llvm/test/tools/llvm-readobj/ELF/amdgpu-elf-headers.test
@@ -223,6 +223,15 @@
 # RUN: yaml2obj %s -o %t -DABI_VERSION=2 -DFLAG_NAME=EF_AMDGPU_MACH_AMDGCN_GFX1012
 # RUN: llvm-readobj -h %t | FileCheck %s --check-prefixes=ALL,KNOWN-ABI-VERSION,SINGLE-FLAG --match-full-lines -DABI_VERSION=2 -DFILE=%t -DFLAG_NAME=EF_AMDGPU_MACH_AMDGCN_GFX1012 -DFLAG_VALUE=0x35
 
+# RUN: yaml2obj %s -o %t -DABI_VERSION=0 -DFLAG_NAME=EF_AMDGPU_MACH_AMDGCN_GFX1013
+# RUN: llvm-readobj -h %t | FileCheck %s --check-prefixes=ALL,KNOWN-ABI-VERSION,SINGLE-FLAG --match-full-lines -DABI_VERSION=0 -DFILE=%t -DFLAG_NAME=EF_AMDGPU_MACH_AMDGCN_GFX1013 -DFLAG_VALUE=0x42
+
+# RUN: yaml2obj %s -o %t -DABI_VERSION=1 -DFLAG_NAME=EF_AMDGPU_MACH_AMDGCN_GFX1013
+# RUN: llvm-readobj -h %t | FileCheck %s --check-prefixes=ALL,KNOWN-ABI-VERSION,SINGLE-FLAG --match-full-lines -DABI_VERSION=1 -DFILE=%t -DFLAG_NAME=EF_AMDGPU_MACH_AMDGCN_GFX1013 -DFLAG_VALUE=0x42
+
+# RUN: yaml2obj %s -o %t -DABI_VERSION=2 -DFLAG_NAME=EF_AMDGPU_MACH_AMDGCN_GFX1013
+# RUN: llvm-readobj -h %t | FileCheck %s --check-prefixes=ALL,KNOWN-ABI-VERSION,SINGLE-FLAG --match-full-lines -DABI_VERSION=2 -DFILE=%t -DFLAG_NAME=EF_AMDGPU_MA

[PATCH] D99696: [clang] NRVO: Improvements and handling of more cases.

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov added a comment.

Ping.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D99696/new/

https://reviews.llvm.org/D99696

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D103760: [clang] use correct builtin type for defaulted comparison analyzer

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov created this revision.
Herald added subscribers: manas, steakhal, ASDenysPetrov, dkrupp, donat.nagy, 
Szelethus, a.sidorin, baloghadamsoftware.
mizvekov requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Fixes PR50591.

When analyzing classes with members which have user-defined conversion
operators to builtin types, the defaulted comparison analyzer was
picking the member type instead of the type for the builtin operator
which was selected as the best match.

This could either result in wrong comparison category being selected,
or a crash when runtime checks are enabled.

Signed-off-by: Matheus Izvekov 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D103760

Files:
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/CXX/class/class.compare/class.spaceship/p2.cpp


Index: clang/test/CXX/class/class.compare/class.spaceship/p2.cpp
===
--- clang/test/CXX/class/class.compare/class.spaceship/p2.cpp
+++ clang/test/CXX/class/class.compare/class.spaceship/p2.cpp
@@ -172,3 +172,23 @@
 int C::*x;   // expected-note {{because 
there is no viable three-way comparison function for member 'x'}}
   };
 }
+
+namespace PR50591 {
+  struct a1 {
+operator int() const;
+  };
+  struct b1 {
+auto operator<=>(b1 const &) const = default;
+a1 f;
+  };
+  std::strong_ordering cmp_b1 = b1() <=> b1();
+
+  struct a2 {
+operator float() const;
+  };
+  struct b2 {
+auto operator<=>(b2 const &) const = default;
+a2 f;
+  };
+  std::partial_ordering cmp_b2 = b2() <=> b2();
+}
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -7861,8 +7861,13 @@
 return Result::deleted();
   }
 } else {
+  QualType T = Best->BuiltinParamTypes[0];
+  assert(T == Best->BuiltinParamTypes[1] &&
+ "builtin comparison for different types?");
+  assert(Best->BuiltinParamTypes[2].isNull() &&
+ "invalid builtin comparison");
   Optional Cat =
-  getComparisonCategoryForBuiltinCmp(Args[0]->getType());
+  getComparisonCategoryForBuiltinCmp(T);
   assert(Cat && "no category for builtin comparison?");
   R.Category = *Cat;
 }


Index: clang/test/CXX/class/class.compare/class.spaceship/p2.cpp
===
--- clang/test/CXX/class/class.compare/class.spaceship/p2.cpp
+++ clang/test/CXX/class/class.compare/class.spaceship/p2.cpp
@@ -172,3 +172,23 @@
 int C::*x;   // expected-note {{because there is no viable three-way comparison function for member 'x'}}
   };
 }
+
+namespace PR50591 {
+  struct a1 {
+operator int() const;
+  };
+  struct b1 {
+auto operator<=>(b1 const &) const = default;
+a1 f;
+  };
+  std::strong_ordering cmp_b1 = b1() <=> b1();
+
+  struct a2 {
+operator float() const;
+  };
+  struct b2 {
+auto operator<=>(b2 const &) const = default;
+a2 f;
+  };
+  std::partial_ordering cmp_b2 = b2() <=> b2();
+}
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -7861,8 +7861,13 @@
 return Result::deleted();
   }
 } else {
+  QualType T = Best->BuiltinParamTypes[0];
+  assert(T == Best->BuiltinParamTypes[1] &&
+ "builtin comparison for different types?");
+  assert(Best->BuiltinParamTypes[2].isNull() &&
+ "invalid builtin comparison");
   Optional Cat =
-  getComparisonCategoryForBuiltinCmp(Args[0]->getType());
+  getComparisonCategoryForBuiltinCmp(T);
   assert(Cat && "no category for builtin comparison?");
   R.Category = *Cat;
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D103760: [clang] use correct builtin type for defaulted comparison analyzer

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov added a comment.

Since we can still crash with user defined conversion operators for any builtin 
types not supported by `getComparisonCategoryForBuiltinCmp` (many others), I 
will make a follow up patch to just replace that assert with a deleted result.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D103760/new/

https://reviews.llvm.org/D103760

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D27651: [clang-format] Even with AlignConsecutiveDeclarations, PointerAlignment: Right should keep *s and &s to the right

[Ida Delphine via Phabricator via cfe-commits]

Idadelveloper accepted this revision.
Idadelveloper added a comment.

Hello,
Just wanted to follow up on these changes hoping they can be approved and 
merged. This will help me on a project I'm working on :)


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D27651/new/

https://reviews.llvm.org/D27651

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits