[PATCH] D108481: Avoid nullptr dereferencing of 'Constraint'

[Sindhu Chittireddy via Phabricator via cfe-commits]

schittir created this revision.
schittir added reviewers: erichkeane, saar.raz.
schittir added a project: clang.
schittir requested review of this revision.
Herald added a subscriber: cfe-commits.

  Klocwork static code analysis exposed this bug:
  Pointer 'Constraint' returned from call to function
  'cast_or_null' may
  be NULL and will be dereferenced in the statement following it
  
  Replace 'cast_or_null' with 'cast' so that the latter can assert
  when it encounters a NULL


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D108481

Files:
  clang/lib/Sema/SemaConcept.cpp


Index: clang/lib/Sema/SemaConcept.cpp
===
--- clang/lib/Sema/SemaConcept.cpp
+++ clang/lib/Sema/SemaConcept.cpp
@@ -1065,8 +1065,7 @@
   assert(TC &&
  "TPL must have a template type parameter with a type constraint");
   auto *Constraint =
-  cast_or_null(
-  TC->getImmediatelyDeclaredConstraint());
+  cast(TC->getImmediatelyDeclaredConstraint());
   bool Dependent =
   Constraint->getTemplateArgsAsWritten() &&
   TemplateSpecializationType::anyInstantiationDependentTemplateArguments(


Index: clang/lib/Sema/SemaConcept.cpp
===
--- clang/lib/Sema/SemaConcept.cpp
+++ clang/lib/Sema/SemaConcept.cpp
@@ -1065,8 +1065,7 @@
   assert(TC &&
  "TPL must have a template type parameter with a type constraint");
   auto *Constraint =
-  cast_or_null(
-  TC->getImmediatelyDeclaredConstraint());
+  cast(TC->getImmediatelyDeclaredConstraint());
   bool Dependent =
   Constraint->getTemplateArgsAsWritten() &&
   TemplateSpecializationType::anyInstantiationDependentTemplateArguments(
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D108481: Avoid nullptr dereferencing of 'Constraint'

[Sindhu Chittireddy via Phabricator via cfe-commits]

schittir marked an inline comment as done.
schittir added a comment.

In D108481#2960875 , @aaron.ballman 
wrote:

> LGTM, thanks for the cleanup! Do you need someone to commit on your behalf? 
> If so, what name and email address would you like me to use for patch 
> attribution?

Thank you, Aaron. 
Yes, please! Sindhu Chittireddy; sindhu.chittire...@intel.com


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

https://reviews.llvm.org/D108481

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

[PATCH] D108481: Avoid nullptr dereferencing of 'Constraint'

[Sindhu Chittireddy via Phabricator via cfe-commits]

schittir updated this revision to Diff 368183.
schittir added a comment.

1. Removed the useless assert on TC.
2. Retained the change from `cast_or_null` to `cast`


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

https://reviews.llvm.org/D108481

Files:
  clang/lib/Sema/SemaConcept.cpp


Index: clang/lib/Sema/SemaConcept.cpp
===
--- clang/lib/Sema/SemaConcept.cpp
+++ clang/lib/Sema/SemaConcept.cpp
@@ -1062,11 +1062,8 @@
   assert(TPL->size() == 1);
   const TypeConstraint *TC =
   cast(TPL->getParam(0))->getTypeConstraint();
-  assert(TC &&
- "TPL must have a template type parameter with a type constraint");
   auto *Constraint =
-  cast_or_null(
-  TC->getImmediatelyDeclaredConstraint());
+  cast(TC->getImmediatelyDeclaredConstraint());
   bool Dependent =
   Constraint->getTemplateArgsAsWritten() &&
   TemplateSpecializationType::anyInstantiationDependentTemplateArguments(


Index: clang/lib/Sema/SemaConcept.cpp
===
--- clang/lib/Sema/SemaConcept.cpp
+++ clang/lib/Sema/SemaConcept.cpp
@@ -1062,11 +1062,8 @@
   assert(TPL->size() == 1);
   const TypeConstraint *TC =
   cast(TPL->getParam(0))->getTypeConstraint();
-  assert(TC &&
- "TPL must have a template type parameter with a type constraint");
   auto *Constraint =
-  cast_or_null(
-  TC->getImmediatelyDeclaredConstraint());
+  cast(TC->getImmediatelyDeclaredConstraint());
   bool Dependent =
   Constraint->getTemplateArgsAsWritten() &&
   TemplateSpecializationType::anyInstantiationDependentTemplateArguments(
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D108712: Avoid nullptr dereferencing of 'SubExpr'; NFC

[Sindhu Chittireddy via Phabricator via cfe-commits]

schittir created this revision.
schittir added reviewers: aaron.ballman, erichkeane.
schittir added a project: clang.
schittir requested review of this revision.
Herald added a subscriber: cfe-commits.

Avoid nullptr dereferencing of 'Constraint'; NFC

Klocwork static code analysis exposed this bug:
Pointer 'SubExpr' returned from call to getSubExpr() function 
which may return NULL from 'cast_or_null(Operand)', 
which will be dereferenced in the statement following it

Add an assert on SubExpr to avoid nullptr dereferencing


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D108712

Files:
  clang/lib/CodeGen/MicrosoftCXXABI.cpp


Index: clang/lib/CodeGen/MicrosoftCXXABI.cpp
===
--- clang/lib/CodeGen/MicrosoftCXXABI.cpp
+++ clang/lib/CodeGen/MicrosoftCXXABI.cpp
@@ -4348,6 +4348,7 @@
 
 void MicrosoftCXXABI::emitThrow(CodeGenFunction , const CXXThrowExpr *E) {
   const Expr *SubExpr = E->getSubExpr();
+  assert(SubExpr && "SubExpr cannot be null");
   QualType ThrowType = SubExpr->getType();
   // The exception object lives on the stack and it's address is passed to the
   // runtime function.


Index: clang/lib/CodeGen/MicrosoftCXXABI.cpp
===
--- clang/lib/CodeGen/MicrosoftCXXABI.cpp
+++ clang/lib/CodeGen/MicrosoftCXXABI.cpp
@@ -4348,6 +4348,7 @@
 
 void MicrosoftCXXABI::emitThrow(CodeGenFunction , const CXXThrowExpr *E) {
   const Expr *SubExpr = E->getSubExpr();
+  assert(SubExpr && "SubExpr cannot be null");
   QualType ThrowType = SubExpr->getType();
   // The exception object lives on the stack and it's address is passed to the
   // runtime function.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D108712: Avoid nullptr dereferencing of 'SubExpr'; NFC

[Sindhu Chittireddy via Phabricator via cfe-commits]

schittir added a comment.

In D108712#2965433 , @aaron.ballman 
wrote:

> LGTM! There is no way for this to be null because `emitRethrow()` is what 
> should be called in that case.
>
> I'll land this on your behalf sometime shortly (perhaps as late as tomorrow).

Thank you, Aaron!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108712

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

[PATCH] D115320: Avoid setting tbaa information on return type of call to inline assember

[Sindhu Chittireddy via Phabricator via cfe-commits]

schittir created this revision.
Herald added subscribers: jeroen.dobbelaere, kosarev.
schittir requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

In 32bit mode, attaching TBAA metadata to the store following the call to 
inline assembler results in describing the wrong type by making a fake 
lvalue(i.e., whatever the inline assembler happens to leave in EAX:EDX.) Even 
if inline assembler somehow describes the correct type, setting TBAA 
information on return type of call to inline assembler is likely not correct, 
since TBAA rules need not apply to inline assembler.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D115320

Files:
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenModule.h


Index: clang/lib/CodeGen/CodeGenModule.h
===
--- clang/lib/CodeGen/CodeGenModule.h
+++ clang/lib/CodeGen/CodeGenModule.h
@@ -746,6 +746,9 @@
   /// an object of the given type.
   TBAAAccessInfo getTBAAAccessInfo(QualType AccessType);
 
+  /// returnNullTBAA - Return empty TBAA constructor
+  TBAAAccessInfo returnNullTBAA();
+
   /// getTBAAVTablePtrAccessInfo - Get the TBAA information that describes an
   /// access to a virtual table pointer.
   TBAAAccessInfo getTBAAVTablePtrAccessInfo(llvm::Type *VTablePtrType);
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -935,6 +935,10 @@
   return TBAA->getAccessInfo(AccessType);
 }
 
+TBAAAccessInfo CodeGenModule::returnNullTBAA() {
+  return TBAAAccessInfo();
+}
+
 TBAAAccessInfo
 CodeGenModule::getTBAAVTablePtrAccessInfo(llvm::Type *VTablePtrType) {
   if (!TBAA)
Index: clang/lib/CodeGen/CodeGenFunction.h
===
--- clang/lib/CodeGen/CodeGenFunction.h
+++ clang/lib/CodeGen/CodeGenFunction.h
@@ -2504,6 +2504,12 @@
 BaseInfo, TBAAInfo);
   }
 
+  LValue MakeAddrLValueWithoutTBAA(Address Addr, QualType T,
+AlignmentSource Source = AlignmentSource::Type) {
+return LValue::MakeAddr(Addr, T, getContext(), LValueBaseInfo(Source),
+CGM.returnNullTBAA());
+  }
+
   LValue MakeNaturalAlignPointeeAddrLValue(llvm::Value *V, QualType T);
   LValue MakeNaturalAlignAddrLValue(llvm::Value *V, QualType T);
 
Index: clang/lib/CodeGen/CGStmt.cpp
===
--- clang/lib/CodeGen/CGStmt.cpp
+++ clang/lib/CodeGen/CGStmt.cpp
@@ -2454,7 +2454,7 @@
 const ABIArgInfo  = CurFnInfo->getReturnInfo();
 if (RetAI.isDirect() || RetAI.isExtend()) {
   // Make a fake lvalue for the return value slot.
-  LValue ReturnSlot = MakeAddrLValue(ReturnValue, FnRetTy);
+  LValue ReturnSlot = MakeAddrLValueWithoutTBAA(ReturnValue, FnRetTy);
   CGM.getTargetCodeGenInfo().addReturnRegisterOutputs(
   *this, ReturnSlot, Constraints, ResultRegTypes, ResultTruncRegTypes,
   ResultRegDests, AsmString, S.getNumOutputs());


Index: clang/lib/CodeGen/CodeGenModule.h
===
--- clang/lib/CodeGen/CodeGenModule.h
+++ clang/lib/CodeGen/CodeGenModule.h
@@ -746,6 +746,9 @@
   /// an object of the given type.
   TBAAAccessInfo getTBAAAccessInfo(QualType AccessType);
 
+  /// returnNullTBAA - Return empty TBAA constructor
+  TBAAAccessInfo returnNullTBAA();
+
   /// getTBAAVTablePtrAccessInfo - Get the TBAA information that describes an
   /// access to a virtual table pointer.
   TBAAAccessInfo getTBAAVTablePtrAccessInfo(llvm::Type *VTablePtrType);
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -935,6 +935,10 @@
   return TBAA->getAccessInfo(AccessType);
 }
 
+TBAAAccessInfo CodeGenModule::returnNullTBAA() {
+  return TBAAAccessInfo();
+}
+
 TBAAAccessInfo
 CodeGenModule::getTBAAVTablePtrAccessInfo(llvm::Type *VTablePtrType) {
   if (!TBAA)
Index: clang/lib/CodeGen/CodeGenFunction.h
===
--- clang/lib/CodeGen/CodeGenFunction.h
+++ clang/lib/CodeGen/CodeGenFunction.h
@@ -2504,6 +2504,12 @@
 BaseInfo, TBAAInfo);
   }
 
+  LValue MakeAddrLValueWithoutTBAA(Address Addr, QualType T,
+AlignmentSource Source = AlignmentSource::Type) {
+return LValue::MakeAddr(Addr, T, getContext(), LValueBaseInfo(Source),
+CGM.returnNullTBAA());
+  }
+
   LValue MakeNaturalAlignPointeeAddrLValue(llvm::Value *V, QualType T);
   LValue MakeNaturalAlignAddrLValue(llvm::Value *V, QualType T);
 
Index: 

[PATCH] D115320: Avoid setting tbaa information on return type of call to inline assember

[Sindhu Chittireddy via Phabricator via cfe-commits]

schittir updated this revision to Diff 392840.
schittir added a subscriber: mikerice.
schittir added a comment.

Add test case
Fix formatting


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

https://reviews.llvm.org/D115320

Files:
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/test/CodeGen/avoidTBAAonASMstore.cpp


Index: clang/test/CodeGen/avoidTBAAonASMstore.cpp
===
--- /dev/null
+++ clang/test/CodeGen/avoidTBAAonASMstore.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -triple i386-unknown-linux-gnu -O2 -disable-llvm-passes 
-fasm-blocks %s -emit-llvm -o - | FileCheck %s
+float _Complex foo(float _Complex z) {
+// CHECK-LABEL: define{{.*}} i64 @_Z3fooCf
+   unsigned short ControlWord;
+   __asm { fnstcw word ptr[ControlWord] };
+// CHECK: store i64 %2, i64* %1, align 4
+// CHECK-NOT: !tbaa
+   return z;
+}
Index: clang/lib/CodeGen/CodeGenModule.h
===
--- clang/lib/CodeGen/CodeGenModule.h
+++ clang/lib/CodeGen/CodeGenModule.h
@@ -746,6 +746,9 @@
   /// an object of the given type.
   TBAAAccessInfo getTBAAAccessInfo(QualType AccessType);
 
+  /// returnNullTBAA - Return empty TBAA constructor
+  TBAAAccessInfo returnNullTBAA();
+
   /// getTBAAVTablePtrAccessInfo - Get the TBAA information that describes an
   /// access to a virtual table pointer.
   TBAAAccessInfo getTBAAVTablePtrAccessInfo(llvm::Type *VTablePtrType);
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -935,6 +935,8 @@
   return TBAA->getAccessInfo(AccessType);
 }
 
+TBAAAccessInfo CodeGenModule::returnNullTBAA() { return TBAAAccessInfo(); }
+
 TBAAAccessInfo
 CodeGenModule::getTBAAVTablePtrAccessInfo(llvm::Type *VTablePtrType) {
   if (!TBAA)
Index: clang/lib/CodeGen/CodeGenFunction.h
===
--- clang/lib/CodeGen/CodeGenFunction.h
+++ clang/lib/CodeGen/CodeGenFunction.h
@@ -2504,6 +2504,13 @@
 BaseInfo, TBAAInfo);
   }
 
+  LValue
+  MakeAddrLValueWithoutTBAA(Address Addr, QualType T,
+AlignmentSource Source = AlignmentSource::Type) {
+return LValue::MakeAddr(Addr, T, getContext(), LValueBaseInfo(Source),
+CGM.returnNullTBAA());
+  }
+
   LValue MakeNaturalAlignPointeeAddrLValue(llvm::Value *V, QualType T);
   LValue MakeNaturalAlignAddrLValue(llvm::Value *V, QualType T);
 
Index: clang/lib/CodeGen/CGStmt.cpp
===
--- clang/lib/CodeGen/CGStmt.cpp
+++ clang/lib/CodeGen/CGStmt.cpp
@@ -2454,7 +2454,7 @@
 const ABIArgInfo  = CurFnInfo->getReturnInfo();
 if (RetAI.isDirect() || RetAI.isExtend()) {
   // Make a fake lvalue for the return value slot.
-  LValue ReturnSlot = MakeAddrLValue(ReturnValue, FnRetTy);
+  LValue ReturnSlot = MakeAddrLValueWithoutTBAA(ReturnValue, FnRetTy);
   CGM.getTargetCodeGenInfo().addReturnRegisterOutputs(
   *this, ReturnSlot, Constraints, ResultRegTypes, ResultTruncRegTypes,
   ResultRegDests, AsmString, S.getNumOutputs());


Index: clang/test/CodeGen/avoidTBAAonASMstore.cpp
===
--- /dev/null
+++ clang/test/CodeGen/avoidTBAAonASMstore.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -triple i386-unknown-linux-gnu -O2 -disable-llvm-passes -fasm-blocks %s -emit-llvm -o - | FileCheck %s
+float _Complex foo(float _Complex z) {
+// CHECK-LABEL: define{{.*}} i64 @_Z3fooCf
+	unsigned short ControlWord;
+	__asm { fnstcw word ptr[ControlWord] };
+// CHECK: store i64 %2, i64* %1, align 4
+// CHECK-NOT: !tbaa
+	return z;
+}
Index: clang/lib/CodeGen/CodeGenModule.h
===
--- clang/lib/CodeGen/CodeGenModule.h
+++ clang/lib/CodeGen/CodeGenModule.h
@@ -746,6 +746,9 @@
   /// an object of the given type.
   TBAAAccessInfo getTBAAAccessInfo(QualType AccessType);
 
+  /// returnNullTBAA - Return empty TBAA constructor
+  TBAAAccessInfo returnNullTBAA();
+
   /// getTBAAVTablePtrAccessInfo - Get the TBAA information that describes an
   /// access to a virtual table pointer.
   TBAAAccessInfo getTBAAVTablePtrAccessInfo(llvm::Type *VTablePtrType);
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -935,6 +935,8 @@
   return TBAA->getAccessInfo(AccessType);
 }
 
+TBAAAccessInfo CodeGenModule::returnNullTBAA() { return TBAAAccessInfo(); }
+
 TBAAAccessInfo
 CodeGenModule::getTBAAVTablePtrAccessInfo(llvm::Type *VTablePtrType) {
   if 

[PATCH] D115320: Avoid setting tbaa information on return type of call to inline assember

[Sindhu Chittireddy via Phabricator via cfe-commits]

schittir added a comment.

In D115320#3178682 , 
@jeroen.dobbelaere wrote:

> Do you have a testcase ?

Thank you for the review. Please take a look at the new diff.


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

https://reviews.llvm.org/D115320

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

[PATCH] D115320: Avoid setting tbaa information on store of return type of call to inline assember

[Sindhu Chittireddy via Phabricator via cfe-commits]

schittir updated this revision to Diff 393948.
schittir added a comment.

Fix test format


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

https://reviews.llvm.org/D115320

Files:
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/test/CodeGen/avoidTBAAonASMstore.cpp


Index: clang/test/CodeGen/avoidTBAAonASMstore.cpp
===
--- /dev/null
+++ clang/test/CodeGen/avoidTBAAonASMstore.cpp
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -triple i386-unknown-linux-gnu -O2 -disable-llvm-passes 
-fasm-blocks %s -emit-llvm -o - | FileCheck --check-prefix=STORE-LINE %s
+double foo(double z) {
+  // CHECK-LABEL: define{{.*}} double @_Z3food
+  unsigned short ControlWord;
+  __asm { fnstcw word ptr[ControlWord]}
+  ;
+  // STORE-LINE: store i64 %{{.*}}, i64* %{{.*}},
+  // STORE-LINE-NOT: align 4, !tbaa
+  // STORE-LINE-SAME: align 4
+  return z;
+}
Index: clang/lib/CodeGen/CodeGenFunction.h
===
--- clang/lib/CodeGen/CodeGenFunction.h
+++ clang/lib/CodeGen/CodeGenFunction.h
@@ -2504,6 +2504,13 @@
 BaseInfo, TBAAInfo);
   }
 
+  LValue
+  MakeAddrLValueWithoutTBAA(Address Addr, QualType T,
+AlignmentSource Source = AlignmentSource::Type) {
+return LValue::MakeAddr(Addr, T, getContext(), LValueBaseInfo(Source),
+TBAAAccessInfo());
+  }
+
   LValue MakeNaturalAlignPointeeAddrLValue(llvm::Value *V, QualType T);
   LValue MakeNaturalAlignAddrLValue(llvm::Value *V, QualType T);
 
Index: clang/lib/CodeGen/CGStmt.cpp
===
--- clang/lib/CodeGen/CGStmt.cpp
+++ clang/lib/CodeGen/CGStmt.cpp
@@ -2454,7 +2454,7 @@
 const ABIArgInfo  = CurFnInfo->getReturnInfo();
 if (RetAI.isDirect() || RetAI.isExtend()) {
   // Make a fake lvalue for the return value slot.
-  LValue ReturnSlot = MakeAddrLValue(ReturnValue, FnRetTy);
+  LValue ReturnSlot = MakeAddrLValueWithoutTBAA(ReturnValue, FnRetTy);
   CGM.getTargetCodeGenInfo().addReturnRegisterOutputs(
   *this, ReturnSlot, Constraints, ResultRegTypes, ResultTruncRegTypes,
   ResultRegDests, AsmString, S.getNumOutputs());


Index: clang/test/CodeGen/avoidTBAAonASMstore.cpp
===
--- /dev/null
+++ clang/test/CodeGen/avoidTBAAonASMstore.cpp
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -triple i386-unknown-linux-gnu -O2 -disable-llvm-passes -fasm-blocks %s -emit-llvm -o - | FileCheck --check-prefix=STORE-LINE %s
+double foo(double z) {
+  // CHECK-LABEL: define{{.*}} double @_Z3food
+  unsigned short ControlWord;
+  __asm { fnstcw word ptr[ControlWord]}
+  ;
+  // STORE-LINE: store i64 %{{.*}}, i64* %{{.*}},
+  // STORE-LINE-NOT: align 4, !tbaa
+  // STORE-LINE-SAME: align 4
+  return z;
+}
Index: clang/lib/CodeGen/CodeGenFunction.h
===
--- clang/lib/CodeGen/CodeGenFunction.h
+++ clang/lib/CodeGen/CodeGenFunction.h
@@ -2504,6 +2504,13 @@
 BaseInfo, TBAAInfo);
   }
 
+  LValue
+  MakeAddrLValueWithoutTBAA(Address Addr, QualType T,
+AlignmentSource Source = AlignmentSource::Type) {
+return LValue::MakeAddr(Addr, T, getContext(), LValueBaseInfo(Source),
+TBAAAccessInfo());
+  }
+
   LValue MakeNaturalAlignPointeeAddrLValue(llvm::Value *V, QualType T);
   LValue MakeNaturalAlignAddrLValue(llvm::Value *V, QualType T);
 
Index: clang/lib/CodeGen/CGStmt.cpp
===
--- clang/lib/CodeGen/CGStmt.cpp
+++ clang/lib/CodeGen/CGStmt.cpp
@@ -2454,7 +2454,7 @@
 const ABIArgInfo  = CurFnInfo->getReturnInfo();
 if (RetAI.isDirect() || RetAI.isExtend()) {
   // Make a fake lvalue for the return value slot.
-  LValue ReturnSlot = MakeAddrLValue(ReturnValue, FnRetTy);
+  LValue ReturnSlot = MakeAddrLValueWithoutTBAA(ReturnValue, FnRetTy);
   CGM.getTargetCodeGenInfo().addReturnRegisterOutputs(
   *this, ReturnSlot, Constraints, ResultRegTypes, ResultTruncRegTypes,
   ResultRegDests, AsmString, S.getNumOutputs());
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D115320: Avoid setting tbaa information on store of return type of call to inline assember

[Sindhu Chittireddy via Phabricator via cfe-commits]

schittir updated this revision to Diff 393945.
schittir added a comment.

Changing test's CHECK lines


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

https://reviews.llvm.org/D115320

Files:
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/test/CodeGen/avoidTBAAonASMstore.cpp


Index: clang/test/CodeGen/avoidTBAAonASMstore.cpp
===
--- /dev/null
+++ clang/test/CodeGen/avoidTBAAonASMstore.cpp
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -triple i386-unknown-linux-gnu -O2 -disable-llvm-passes 
-fasm-blocks %s -emit-llvm -o - | FileCheck --check-prefix=STORE-LINE %s
+double foo(double z) {
+// CHECK-LABEL: define{{.*}} double @_Z3food
+   unsigned short ControlWord;
+   __asm { fnstcw word ptr[ControlWord] };
+// STORE-LINE: store i64 %{{.*}}, i64* %{{.*}},
+// STORE-LINE-NOT: align 4, !tbaa
+// STORE-LINE-SAME: align 4
+   return z;
+}
Index: clang/lib/CodeGen/CodeGenFunction.h
===
--- clang/lib/CodeGen/CodeGenFunction.h
+++ clang/lib/CodeGen/CodeGenFunction.h
@@ -2504,6 +2504,13 @@
 BaseInfo, TBAAInfo);
   }
 
+  LValue
+  MakeAddrLValueWithoutTBAA(Address Addr, QualType T,
+AlignmentSource Source = AlignmentSource::Type) {
+return LValue::MakeAddr(Addr, T, getContext(), LValueBaseInfo(Source),
+TBAAAccessInfo());
+  }
+
   LValue MakeNaturalAlignPointeeAddrLValue(llvm::Value *V, QualType T);
   LValue MakeNaturalAlignAddrLValue(llvm::Value *V, QualType T);
 
Index: clang/lib/CodeGen/CGStmt.cpp
===
--- clang/lib/CodeGen/CGStmt.cpp
+++ clang/lib/CodeGen/CGStmt.cpp
@@ -2454,7 +2454,7 @@
 const ABIArgInfo  = CurFnInfo->getReturnInfo();
 if (RetAI.isDirect() || RetAI.isExtend()) {
   // Make a fake lvalue for the return value slot.
-  LValue ReturnSlot = MakeAddrLValue(ReturnValue, FnRetTy);
+  LValue ReturnSlot = MakeAddrLValueWithoutTBAA(ReturnValue, FnRetTy);
   CGM.getTargetCodeGenInfo().addReturnRegisterOutputs(
   *this, ReturnSlot, Constraints, ResultRegTypes, ResultTruncRegTypes,
   ResultRegDests, AsmString, S.getNumOutputs());


Index: clang/test/CodeGen/avoidTBAAonASMstore.cpp
===
--- /dev/null
+++ clang/test/CodeGen/avoidTBAAonASMstore.cpp
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -triple i386-unknown-linux-gnu -O2 -disable-llvm-passes -fasm-blocks %s -emit-llvm -o - | FileCheck --check-prefix=STORE-LINE %s
+double foo(double z) {
+// CHECK-LABEL: define{{.*}} double @_Z3food
+	unsigned short ControlWord;
+	__asm { fnstcw word ptr[ControlWord] };
+// STORE-LINE: store i64 %{{.*}}, i64* %{{.*}},
+// STORE-LINE-NOT: align 4, !tbaa
+// STORE-LINE-SAME: align 4
+	return z;
+}
Index: clang/lib/CodeGen/CodeGenFunction.h
===
--- clang/lib/CodeGen/CodeGenFunction.h
+++ clang/lib/CodeGen/CodeGenFunction.h
@@ -2504,6 +2504,13 @@
 BaseInfo, TBAAInfo);
   }
 
+  LValue
+  MakeAddrLValueWithoutTBAA(Address Addr, QualType T,
+AlignmentSource Source = AlignmentSource::Type) {
+return LValue::MakeAddr(Addr, T, getContext(), LValueBaseInfo(Source),
+TBAAAccessInfo());
+  }
+
   LValue MakeNaturalAlignPointeeAddrLValue(llvm::Value *V, QualType T);
   LValue MakeNaturalAlignAddrLValue(llvm::Value *V, QualType T);
 
Index: clang/lib/CodeGen/CGStmt.cpp
===
--- clang/lib/CodeGen/CGStmt.cpp
+++ clang/lib/CodeGen/CGStmt.cpp
@@ -2454,7 +2454,7 @@
 const ABIArgInfo  = CurFnInfo->getReturnInfo();
 if (RetAI.isDirect() || RetAI.isExtend()) {
   // Make a fake lvalue for the return value slot.
-  LValue ReturnSlot = MakeAddrLValue(ReturnValue, FnRetTy);
+  LValue ReturnSlot = MakeAddrLValueWithoutTBAA(ReturnValue, FnRetTy);
   CGM.getTargetCodeGenInfo().addReturnRegisterOutputs(
   *this, ReturnSlot, Constraints, ResultRegTypes, ResultTruncRegTypes,
   ResultRegDests, AsmString, S.getNumOutputs());
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D115320: Avoid setting tbaa information on store of return type of call to inline assember

[Sindhu Chittireddy via Phabricator via cfe-commits]

schittir updated this revision to Diff 394173.
schittir added a comment.

Rebasing patch to see if the test failure will go away.


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

https://reviews.llvm.org/D115320

Files:
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/test/CodeGen/avoidTBAAonASMstore.cpp


Index: clang/test/CodeGen/avoidTBAAonASMstore.cpp
===
--- /dev/null
+++ clang/test/CodeGen/avoidTBAAonASMstore.cpp
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -triple i386-unknown-linux-gnu -O2 -disable-llvm-passes 
-fasm-blocks %s -emit-llvm -o - | FileCheck --check-prefix=STORE-LINE %s
+double foo(double z) {
+  // CHECK-LABEL: define{{.*}} double @_Z3food
+  unsigned short ControlWord;
+  __asm { fnstcw word ptr[ControlWord]}
+  ;
+  // STORE-LINE: store i64 %{{.*}}, i64* %{{.*}},
+  // STORE-LINE-NOT: align 4, !tbaa
+  // STORE-LINE-SAME: align 4
+  return z;
+}
Index: clang/lib/CodeGen/CodeGenFunction.h
===
--- clang/lib/CodeGen/CodeGenFunction.h
+++ clang/lib/CodeGen/CodeGenFunction.h
@@ -2504,6 +2504,13 @@
 BaseInfo, TBAAInfo);
   }
 
+  LValue
+  MakeAddrLValueWithoutTBAA(Address Addr, QualType T,
+AlignmentSource Source = AlignmentSource::Type) {
+return LValue::MakeAddr(Addr, T, getContext(), LValueBaseInfo(Source),
+TBAAAccessInfo());
+  }
+
   LValue MakeNaturalAlignPointeeAddrLValue(llvm::Value *V, QualType T);
   LValue MakeNaturalAlignAddrLValue(llvm::Value *V, QualType T);
 
Index: clang/lib/CodeGen/CGStmt.cpp
===
--- clang/lib/CodeGen/CGStmt.cpp
+++ clang/lib/CodeGen/CGStmt.cpp
@@ -2454,7 +2454,7 @@
 const ABIArgInfo  = CurFnInfo->getReturnInfo();
 if (RetAI.isDirect() || RetAI.isExtend()) {
   // Make a fake lvalue for the return value slot.
-  LValue ReturnSlot = MakeAddrLValue(ReturnValue, FnRetTy);
+  LValue ReturnSlot = MakeAddrLValueWithoutTBAA(ReturnValue, FnRetTy);
   CGM.getTargetCodeGenInfo().addReturnRegisterOutputs(
   *this, ReturnSlot, Constraints, ResultRegTypes, ResultTruncRegTypes,
   ResultRegDests, AsmString, S.getNumOutputs());


Index: clang/test/CodeGen/avoidTBAAonASMstore.cpp
===
--- /dev/null
+++ clang/test/CodeGen/avoidTBAAonASMstore.cpp
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -triple i386-unknown-linux-gnu -O2 -disable-llvm-passes -fasm-blocks %s -emit-llvm -o - | FileCheck --check-prefix=STORE-LINE %s
+double foo(double z) {
+  // CHECK-LABEL: define{{.*}} double @_Z3food
+  unsigned short ControlWord;
+  __asm { fnstcw word ptr[ControlWord]}
+  ;
+  // STORE-LINE: store i64 %{{.*}}, i64* %{{.*}},
+  // STORE-LINE-NOT: align 4, !tbaa
+  // STORE-LINE-SAME: align 4
+  return z;
+}
Index: clang/lib/CodeGen/CodeGenFunction.h
===
--- clang/lib/CodeGen/CodeGenFunction.h
+++ clang/lib/CodeGen/CodeGenFunction.h
@@ -2504,6 +2504,13 @@
 BaseInfo, TBAAInfo);
   }
 
+  LValue
+  MakeAddrLValueWithoutTBAA(Address Addr, QualType T,
+AlignmentSource Source = AlignmentSource::Type) {
+return LValue::MakeAddr(Addr, T, getContext(), LValueBaseInfo(Source),
+TBAAAccessInfo());
+  }
+
   LValue MakeNaturalAlignPointeeAddrLValue(llvm::Value *V, QualType T);
   LValue MakeNaturalAlignAddrLValue(llvm::Value *V, QualType T);
 
Index: clang/lib/CodeGen/CGStmt.cpp
===
--- clang/lib/CodeGen/CGStmt.cpp
+++ clang/lib/CodeGen/CGStmt.cpp
@@ -2454,7 +2454,7 @@
 const ABIArgInfo  = CurFnInfo->getReturnInfo();
 if (RetAI.isDirect() || RetAI.isExtend()) {
   // Make a fake lvalue for the return value slot.
-  LValue ReturnSlot = MakeAddrLValue(ReturnValue, FnRetTy);
+  LValue ReturnSlot = MakeAddrLValueWithoutTBAA(ReturnValue, FnRetTy);
   CGM.getTargetCodeGenInfo().addReturnRegisterOutputs(
   *this, ReturnSlot, Constraints, ResultRegTypes, ResultTruncRegTypes,
   ResultRegDests, AsmString, S.getNumOutputs());
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D115320: Avoid setting tbaa information on store of return type of call to inline assember

[Sindhu Chittireddy via Phabricator via cfe-commits]

schittir updated this revision to Diff 394176.
schittir added a comment.

Change CHECK-LABEL to STORE-LINE-LABEL


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

https://reviews.llvm.org/D115320

Files:
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/test/CodeGen/avoidTBAAonASMstore.cpp


Index: clang/test/CodeGen/avoidTBAAonASMstore.cpp
===
--- /dev/null
+++ clang/test/CodeGen/avoidTBAAonASMstore.cpp
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -triple i386-unknown-linux-gnu -O2 -disable-llvm-passes 
-fasm-blocks %s -emit-llvm -o - | FileCheck --check-prefix=STORE-LINE %s
+double foo(double z) {
+  // STORE-LINE-LABEL: define{{.*}} double @_Z3food
+  unsigned short ControlWord;
+  __asm { fnstcw word ptr[ControlWord]}
+  ;
+  // STORE-LINE: store i64 %{{.*}}, i64* %{{.*}},
+  // STORE-LINE-NOT: align 4, !tbaa
+  // STORE-LINE-SAME: align 4
+  return z;
+}
Index: clang/lib/CodeGen/CodeGenFunction.h
===
--- clang/lib/CodeGen/CodeGenFunction.h
+++ clang/lib/CodeGen/CodeGenFunction.h
@@ -2504,6 +2504,13 @@
 BaseInfo, TBAAInfo);
   }
 
+  LValue
+  MakeAddrLValueWithoutTBAA(Address Addr, QualType T,
+AlignmentSource Source = AlignmentSource::Type) {
+return LValue::MakeAddr(Addr, T, getContext(), LValueBaseInfo(Source),
+TBAAAccessInfo());
+  }
+
   LValue MakeNaturalAlignPointeeAddrLValue(llvm::Value *V, QualType T);
   LValue MakeNaturalAlignAddrLValue(llvm::Value *V, QualType T);
 
Index: clang/lib/CodeGen/CGStmt.cpp
===
--- clang/lib/CodeGen/CGStmt.cpp
+++ clang/lib/CodeGen/CGStmt.cpp
@@ -2454,7 +2454,7 @@
 const ABIArgInfo  = CurFnInfo->getReturnInfo();
 if (RetAI.isDirect() || RetAI.isExtend()) {
   // Make a fake lvalue for the return value slot.
-  LValue ReturnSlot = MakeAddrLValue(ReturnValue, FnRetTy);
+  LValue ReturnSlot = MakeAddrLValueWithoutTBAA(ReturnValue, FnRetTy);
   CGM.getTargetCodeGenInfo().addReturnRegisterOutputs(
   *this, ReturnSlot, Constraints, ResultRegTypes, ResultTruncRegTypes,
   ResultRegDests, AsmString, S.getNumOutputs());


Index: clang/test/CodeGen/avoidTBAAonASMstore.cpp
===
--- /dev/null
+++ clang/test/CodeGen/avoidTBAAonASMstore.cpp
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -triple i386-unknown-linux-gnu -O2 -disable-llvm-passes -fasm-blocks %s -emit-llvm -o - | FileCheck --check-prefix=STORE-LINE %s
+double foo(double z) {
+  // STORE-LINE-LABEL: define{{.*}} double @_Z3food
+  unsigned short ControlWord;
+  __asm { fnstcw word ptr[ControlWord]}
+  ;
+  // STORE-LINE: store i64 %{{.*}}, i64* %{{.*}},
+  // STORE-LINE-NOT: align 4, !tbaa
+  // STORE-LINE-SAME: align 4
+  return z;
+}
Index: clang/lib/CodeGen/CodeGenFunction.h
===
--- clang/lib/CodeGen/CodeGenFunction.h
+++ clang/lib/CodeGen/CodeGenFunction.h
@@ -2504,6 +2504,13 @@
 BaseInfo, TBAAInfo);
   }
 
+  LValue
+  MakeAddrLValueWithoutTBAA(Address Addr, QualType T,
+AlignmentSource Source = AlignmentSource::Type) {
+return LValue::MakeAddr(Addr, T, getContext(), LValueBaseInfo(Source),
+TBAAAccessInfo());
+  }
+
   LValue MakeNaturalAlignPointeeAddrLValue(llvm::Value *V, QualType T);
   LValue MakeNaturalAlignAddrLValue(llvm::Value *V, QualType T);
 
Index: clang/lib/CodeGen/CGStmt.cpp
===
--- clang/lib/CodeGen/CGStmt.cpp
+++ clang/lib/CodeGen/CGStmt.cpp
@@ -2454,7 +2454,7 @@
 const ABIArgInfo  = CurFnInfo->getReturnInfo();
 if (RetAI.isDirect() || RetAI.isExtend()) {
   // Make a fake lvalue for the return value slot.
-  LValue ReturnSlot = MakeAddrLValue(ReturnValue, FnRetTy);
+  LValue ReturnSlot = MakeAddrLValueWithoutTBAA(ReturnValue, FnRetTy);
   CGM.getTargetCodeGenInfo().addReturnRegisterOutputs(
   *this, ReturnSlot, Constraints, ResultRegTypes, ResultTruncRegTypes,
   ResultRegDests, AsmString, S.getNumOutputs());
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D115320: Avoid setting tbaa information on store of return type of call to inline assember

[Sindhu Chittireddy via Phabricator via cfe-commits]

schittir added a comment.

https://reviews.llvm.org/harbormaster/unit/view/1536685/ 
Test failure seems unrelated to this patch. Unable to reproduce locally.


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

https://reviews.llvm.org/D115320

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

[PATCH] D115320: Avoid setting tbaa information on store of return type of call to inline assember

[Sindhu Chittireddy via Phabricator via cfe-commits]

schittir added a comment.

In D115320#3195090 , @DavidSpickett 
wrote:

> I've gone ahead and added the REQUIRES myself.

Thank you, David!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115320

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

[PATCH] D115320: Avoid setting tbaa information on store of return type of call to inline assember

[Sindhu Chittireddy via Phabricator via cfe-commits]

schittir updated this revision to Diff 393642.
schittir marked an inline comment as done.
schittir added a comment.

1. Removed returnNullTBAA() method
2. Add NO-TBAA check-prefix to the test


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

https://reviews.llvm.org/D115320

Files:
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/test/CodeGen/avoidTBAAonASMstore.cpp


Index: clang/test/CodeGen/avoidTBAAonASMstore.cpp
===
--- /dev/null
+++ clang/test/CodeGen/avoidTBAAonASMstore.cpp
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -triple i386-unknown-linux-gnu -O2 -disable-llvm-passes 
-fasm-blocks %s -emit-llvm -o - | FileCheck --check-prefix=NO-TBAA %s
+// NO-TBAA-NOT: !tbaa
+double foo(double z) {
+// CHECK-LABEL: define{{.*}} double @_Z3food
+   unsigned short ControlWord;
+   __asm { fnstcw word ptr[ControlWord] };
+// CHECK: call i64 asm sideeffect inteldialect
+// NO-TBAA: store i64 %{{.*}}, i64* %{{.*}}, align 4
+   return z;
+}
Index: clang/lib/CodeGen/CodeGenFunction.h
===
--- clang/lib/CodeGen/CodeGenFunction.h
+++ clang/lib/CodeGen/CodeGenFunction.h
@@ -2504,6 +2504,13 @@
 BaseInfo, TBAAInfo);
   }
 
+  LValue
+  MakeAddrLValueWithoutTBAA(Address Addr, QualType T,
+AlignmentSource Source = AlignmentSource::Type) {
+return LValue::MakeAddr(Addr, T, getContext(), LValueBaseInfo(Source),
+TBAAAccessInfo());
+  }
+
   LValue MakeNaturalAlignPointeeAddrLValue(llvm::Value *V, QualType T);
   LValue MakeNaturalAlignAddrLValue(llvm::Value *V, QualType T);
 
Index: clang/lib/CodeGen/CGStmt.cpp
===
--- clang/lib/CodeGen/CGStmt.cpp
+++ clang/lib/CodeGen/CGStmt.cpp
@@ -2454,7 +2454,7 @@
 const ABIArgInfo  = CurFnInfo->getReturnInfo();
 if (RetAI.isDirect() || RetAI.isExtend()) {
   // Make a fake lvalue for the return value slot.
-  LValue ReturnSlot = MakeAddrLValue(ReturnValue, FnRetTy);
+  LValue ReturnSlot = MakeAddrLValueWithoutTBAA(ReturnValue, FnRetTy);
   CGM.getTargetCodeGenInfo().addReturnRegisterOutputs(
   *this, ReturnSlot, Constraints, ResultRegTypes, ResultTruncRegTypes,
   ResultRegDests, AsmString, S.getNumOutputs());


Index: clang/test/CodeGen/avoidTBAAonASMstore.cpp
===
--- /dev/null
+++ clang/test/CodeGen/avoidTBAAonASMstore.cpp
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -triple i386-unknown-linux-gnu -O2 -disable-llvm-passes -fasm-blocks %s -emit-llvm -o - | FileCheck --check-prefix=NO-TBAA %s
+// NO-TBAA-NOT: !tbaa
+double foo(double z) {
+// CHECK-LABEL: define{{.*}} double @_Z3food
+	unsigned short ControlWord;
+	__asm { fnstcw word ptr[ControlWord] };
+// CHECK: call i64 asm sideeffect inteldialect
+// NO-TBAA: store i64 %{{.*}}, i64* %{{.*}}, align 4
+	return z;
+}
Index: clang/lib/CodeGen/CodeGenFunction.h
===
--- clang/lib/CodeGen/CodeGenFunction.h
+++ clang/lib/CodeGen/CodeGenFunction.h
@@ -2504,6 +2504,13 @@
 BaseInfo, TBAAInfo);
   }
 
+  LValue
+  MakeAddrLValueWithoutTBAA(Address Addr, QualType T,
+AlignmentSource Source = AlignmentSource::Type) {
+return LValue::MakeAddr(Addr, T, getContext(), LValueBaseInfo(Source),
+TBAAAccessInfo());
+  }
+
   LValue MakeNaturalAlignPointeeAddrLValue(llvm::Value *V, QualType T);
   LValue MakeNaturalAlignAddrLValue(llvm::Value *V, QualType T);
 
Index: clang/lib/CodeGen/CGStmt.cpp
===
--- clang/lib/CodeGen/CGStmt.cpp
+++ clang/lib/CodeGen/CGStmt.cpp
@@ -2454,7 +2454,7 @@
 const ABIArgInfo  = CurFnInfo->getReturnInfo();
 if (RetAI.isDirect() || RetAI.isExtend()) {
   // Make a fake lvalue for the return value slot.
-  LValue ReturnSlot = MakeAddrLValue(ReturnValue, FnRetTy);
+  LValue ReturnSlot = MakeAddrLValueWithoutTBAA(ReturnValue, FnRetTy);
   CGM.getTargetCodeGenInfo().addReturnRegisterOutputs(
   *this, ReturnSlot, Constraints, ResultRegTypes, ResultTruncRegTypes,
   ResultRegDests, AsmString, S.getNumOutputs());
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D115320: Avoid setting tbaa information on store of return type of call to inline assember

[Sindhu Chittireddy via Phabricator via cfe-commits]

schittir updated this revision to Diff 393381.
schittir added a comment.

Updated test per Jeroen's suggestion


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

https://reviews.llvm.org/D115320

Files:
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/test/CodeGen/avoidTBAAonASMstore.cpp


Index: clang/test/CodeGen/avoidTBAAonASMstore.cpp
===
--- /dev/null
+++ clang/test/CodeGen/avoidTBAAonASMstore.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -triple i386-unknown-linux-gnu -O2 -disable-llvm-passes 
-fasm-blocks %s -emit-llvm -o - | FileCheck %s
+double foo(double z) {
+// CHECK-LABEL: define{{.*}} double @_Z3food
+   unsigned short ControlWord;
+   __asm { fnstcw word ptr[ControlWord] };
+// CHECK: store i64 %2, i64* %1, align 4
+// CHECK-NOT: !tbaa
+   return z;
+}
Index: clang/lib/CodeGen/CodeGenModule.h
===
--- clang/lib/CodeGen/CodeGenModule.h
+++ clang/lib/CodeGen/CodeGenModule.h
@@ -746,6 +746,9 @@
   /// an object of the given type.
   TBAAAccessInfo getTBAAAccessInfo(QualType AccessType);
 
+  /// returnNullTBAA - Return empty TBAA constructor
+  TBAAAccessInfo returnNullTBAA();
+
   /// getTBAAVTablePtrAccessInfo - Get the TBAA information that describes an
   /// access to a virtual table pointer.
   TBAAAccessInfo getTBAAVTablePtrAccessInfo(llvm::Type *VTablePtrType);
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -935,6 +935,8 @@
   return TBAA->getAccessInfo(AccessType);
 }
 
+TBAAAccessInfo CodeGenModule::returnNullTBAA() { return TBAAAccessInfo(); }
+
 TBAAAccessInfo
 CodeGenModule::getTBAAVTablePtrAccessInfo(llvm::Type *VTablePtrType) {
   if (!TBAA)
Index: clang/lib/CodeGen/CodeGenFunction.h
===
--- clang/lib/CodeGen/CodeGenFunction.h
+++ clang/lib/CodeGen/CodeGenFunction.h
@@ -2504,6 +2504,13 @@
 BaseInfo, TBAAInfo);
   }
 
+  LValue
+  MakeAddrLValueWithoutTBAA(Address Addr, QualType T,
+AlignmentSource Source = AlignmentSource::Type) {
+return LValue::MakeAddr(Addr, T, getContext(), LValueBaseInfo(Source),
+CGM.returnNullTBAA());
+  }
+
   LValue MakeNaturalAlignPointeeAddrLValue(llvm::Value *V, QualType T);
   LValue MakeNaturalAlignAddrLValue(llvm::Value *V, QualType T);
 
Index: clang/lib/CodeGen/CGStmt.cpp
===
--- clang/lib/CodeGen/CGStmt.cpp
+++ clang/lib/CodeGen/CGStmt.cpp
@@ -2454,7 +2454,7 @@
 const ABIArgInfo  = CurFnInfo->getReturnInfo();
 if (RetAI.isDirect() || RetAI.isExtend()) {
   // Make a fake lvalue for the return value slot.
-  LValue ReturnSlot = MakeAddrLValue(ReturnValue, FnRetTy);
+  LValue ReturnSlot = MakeAddrLValueWithoutTBAA(ReturnValue, FnRetTy);
   CGM.getTargetCodeGenInfo().addReturnRegisterOutputs(
   *this, ReturnSlot, Constraints, ResultRegTypes, ResultTruncRegTypes,
   ResultRegDests, AsmString, S.getNumOutputs());


Index: clang/test/CodeGen/avoidTBAAonASMstore.cpp
===
--- /dev/null
+++ clang/test/CodeGen/avoidTBAAonASMstore.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -triple i386-unknown-linux-gnu -O2 -disable-llvm-passes -fasm-blocks %s -emit-llvm -o - | FileCheck %s
+double foo(double z) {
+// CHECK-LABEL: define{{.*}} double @_Z3food
+	unsigned short ControlWord;
+	__asm { fnstcw word ptr[ControlWord] };
+// CHECK: store i64 %2, i64* %1, align 4
+// CHECK-NOT: !tbaa
+	return z;
+}
Index: clang/lib/CodeGen/CodeGenModule.h
===
--- clang/lib/CodeGen/CodeGenModule.h
+++ clang/lib/CodeGen/CodeGenModule.h
@@ -746,6 +746,9 @@
   /// an object of the given type.
   TBAAAccessInfo getTBAAAccessInfo(QualType AccessType);
 
+  /// returnNullTBAA - Return empty TBAA constructor
+  TBAAAccessInfo returnNullTBAA();
+
   /// getTBAAVTablePtrAccessInfo - Get the TBAA information that describes an
   /// access to a virtual table pointer.
   TBAAAccessInfo getTBAAVTablePtrAccessInfo(llvm::Type *VTablePtrType);
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -935,6 +935,8 @@
   return TBAA->getAccessInfo(AccessType);
 }
 
+TBAAAccessInfo CodeGenModule::returnNullTBAA() { return TBAAAccessInfo(); }
+
 TBAAAccessInfo
 CodeGenModule::getTBAAVTablePtrAccessInfo(llvm::Type *VTablePtrType) {
   if (!TBAA)
Index: clang/lib/CodeGen/CodeGenFunction.h

[PATCH] D115320: Avoid setting tbaa information on store of return type of call to inline assember

[Sindhu Chittireddy via Phabricator via cfe-commits]

schittir added a comment.

In D115320#3182581 , 
@jeroen.dobbelaere wrote:

> When I try out the example on llvm-13, I get a 'omnipotent char' tbaa 
> description. That should be ok in general. When I replace the 'float 
> _Complex' with 'double', I do get the 'double' tbaa. That might be a better 
> example for the testcase ?

Good point. I updated the test with this suggestion. Thank you.


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

https://reviews.llvm.org/D115320

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

[PATCH] D115320: Avoid setting tbaa information on store of return type of call to inline assember

[Sindhu Chittireddy via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG4706a297fb9e: Avoid setting tbaa on the store of return type 
of call to inline assembler. (authored by schittir).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115320

Files:
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/test/CodeGen/avoidTBAAonASMstore.cpp


Index: clang/test/CodeGen/avoidTBAAonASMstore.cpp
===
--- /dev/null
+++ clang/test/CodeGen/avoidTBAAonASMstore.cpp
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -triple i386-unknown-linux-gnu -O2 -disable-llvm-passes 
-fasm-blocks %s -emit-llvm -o - | FileCheck --check-prefix=STORE-LINE %s
+double foo(double z) {
+  // STORE-LINE-LABEL: define{{.*}} double @_Z3food
+  unsigned short ControlWord;
+  __asm { fnstcw word ptr[ControlWord]}
+  ;
+  // STORE-LINE: store i64 %{{.*}}, i64* %{{.*}},
+  // STORE-LINE-NOT: align 4, !tbaa
+  // STORE-LINE-SAME: align 4
+  return z;
+}
Index: clang/lib/CodeGen/CodeGenFunction.h
===
--- clang/lib/CodeGen/CodeGenFunction.h
+++ clang/lib/CodeGen/CodeGenFunction.h
@@ -2504,6 +2504,13 @@
 BaseInfo, TBAAInfo);
   }
 
+  LValue
+  MakeAddrLValueWithoutTBAA(Address Addr, QualType T,
+AlignmentSource Source = AlignmentSource::Type) {
+return LValue::MakeAddr(Addr, T, getContext(), LValueBaseInfo(Source),
+TBAAAccessInfo());
+  }
+
   LValue MakeNaturalAlignPointeeAddrLValue(llvm::Value *V, QualType T);
   LValue MakeNaturalAlignAddrLValue(llvm::Value *V, QualType T);
 
Index: clang/lib/CodeGen/CGStmt.cpp
===
--- clang/lib/CodeGen/CGStmt.cpp
+++ clang/lib/CodeGen/CGStmt.cpp
@@ -2454,7 +2454,7 @@
 const ABIArgInfo  = CurFnInfo->getReturnInfo();
 if (RetAI.isDirect() || RetAI.isExtend()) {
   // Make a fake lvalue for the return value slot.
-  LValue ReturnSlot = MakeAddrLValue(ReturnValue, FnRetTy);
+  LValue ReturnSlot = MakeAddrLValueWithoutTBAA(ReturnValue, FnRetTy);
   CGM.getTargetCodeGenInfo().addReturnRegisterOutputs(
   *this, ReturnSlot, Constraints, ResultRegTypes, ResultTruncRegTypes,
   ResultRegDests, AsmString, S.getNumOutputs());


Index: clang/test/CodeGen/avoidTBAAonASMstore.cpp
===
--- /dev/null
+++ clang/test/CodeGen/avoidTBAAonASMstore.cpp
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -triple i386-unknown-linux-gnu -O2 -disable-llvm-passes -fasm-blocks %s -emit-llvm -o - | FileCheck --check-prefix=STORE-LINE %s
+double foo(double z) {
+  // STORE-LINE-LABEL: define{{.*}} double @_Z3food
+  unsigned short ControlWord;
+  __asm { fnstcw word ptr[ControlWord]}
+  ;
+  // STORE-LINE: store i64 %{{.*}}, i64* %{{.*}},
+  // STORE-LINE-NOT: align 4, !tbaa
+  // STORE-LINE-SAME: align 4
+  return z;
+}
Index: clang/lib/CodeGen/CodeGenFunction.h
===
--- clang/lib/CodeGen/CodeGenFunction.h
+++ clang/lib/CodeGen/CodeGenFunction.h
@@ -2504,6 +2504,13 @@
 BaseInfo, TBAAInfo);
   }
 
+  LValue
+  MakeAddrLValueWithoutTBAA(Address Addr, QualType T,
+AlignmentSource Source = AlignmentSource::Type) {
+return LValue::MakeAddr(Addr, T, getContext(), LValueBaseInfo(Source),
+TBAAAccessInfo());
+  }
+
   LValue MakeNaturalAlignPointeeAddrLValue(llvm::Value *V, QualType T);
   LValue MakeNaturalAlignAddrLValue(llvm::Value *V, QualType T);
 
Index: clang/lib/CodeGen/CGStmt.cpp
===
--- clang/lib/CodeGen/CGStmt.cpp
+++ clang/lib/CodeGen/CGStmt.cpp
@@ -2454,7 +2454,7 @@
 const ABIArgInfo  = CurFnInfo->getReturnInfo();
 if (RetAI.isDirect() || RetAI.isExtend()) {
   // Make a fake lvalue for the return value slot.
-  LValue ReturnSlot = MakeAddrLValue(ReturnValue, FnRetTy);
+  LValue ReturnSlot = MakeAddrLValueWithoutTBAA(ReturnValue, FnRetTy);
   CGM.getTargetCodeGenInfo().addReturnRegisterOutputs(
   *this, ReturnSlot, Constraints, ResultRegTypes, ResultTruncRegTypes,
   ResultRegDests, AsmString, S.getNumOutputs());
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158775: [NFC] Initialize member pointer and avoid potential null dereference

[Sindhu Chittireddy via Phabricator via cfe-commits]

schittir created this revision.
schittir added reviewers: aaron.ballman, tahonermann.
Herald added a reviewer: NoQ.
Herald added a project: All.
schittir requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D158775

Files:
  clang/lib/AST/Interp/Interp.h
  clang/lib/Analysis/ThreadSafety.cpp


Index: clang/lib/Analysis/ThreadSafety.cpp
===
--- clang/lib/Analysis/ThreadSafety.cpp
+++ clang/lib/Analysis/ThreadSafety.cpp
@@ -1008,7 +1008,7 @@
   threadSafety::SExprBuilder SxBuilder;
 
   ThreadSafetyHandler 
-  const CXXMethodDecl *CurrentMethod;
+  const CXXMethodDecl *CurrentMethod = nullptr;
   LocalVariableMap LocalVarMap;
   FactManager FactMan;
   std::vector BlockInfo;
Index: clang/lib/AST/Interp/Interp.h
===
--- clang/lib/AST/Interp/Interp.h
+++ clang/lib/AST/Interp/Interp.h
@@ -201,13 +201,14 @@
   return false;
   }
 
+  assert(S.Current);
   assert(S.Current->getFrameOffset() == S.Stk.size() && "Invalid frame");
   if (!S.checkingPotentialConstantExpression() || S.Current->Caller) {
 // Certain builtin functions are declared as func-name(...), so the
 // parameters are checked in Sema and only available through the CallExpr.
 // The interp::Function we create for them has 0 parameters, so we need to
 // remove them from the stack by checking the CallExpr.
-if (S.Current && S.Current->getFunction()->needsRuntimeArgPop(S.getCtx()))
+if (S.Current->getFunction()->needsRuntimeArgPop(S.getCtx()))
   popBuiltinArgs(S, PC);
 else
   S.Current->popArgs();


Index: clang/lib/Analysis/ThreadSafety.cpp
===
--- clang/lib/Analysis/ThreadSafety.cpp
+++ clang/lib/Analysis/ThreadSafety.cpp
@@ -1008,7 +1008,7 @@
   threadSafety::SExprBuilder SxBuilder;
 
   ThreadSafetyHandler 
-  const CXXMethodDecl *CurrentMethod;
+  const CXXMethodDecl *CurrentMethod = nullptr;
   LocalVariableMap LocalVarMap;
   FactManager FactMan;
   std::vector BlockInfo;
Index: clang/lib/AST/Interp/Interp.h
===
--- clang/lib/AST/Interp/Interp.h
+++ clang/lib/AST/Interp/Interp.h
@@ -201,13 +201,14 @@
   return false;
   }
 
+  assert(S.Current);
   assert(S.Current->getFrameOffset() == S.Stk.size() && "Invalid frame");
   if (!S.checkingPotentialConstantExpression() || S.Current->Caller) {
 // Certain builtin functions are declared as func-name(...), so the
 // parameters are checked in Sema and only available through the CallExpr.
 // The interp::Function we create for them has 0 parameters, so we need to
 // remove them from the stack by checking the CallExpr.
-if (S.Current && S.Current->getFunction()->needsRuntimeArgPop(S.getCtx()))
+if (S.Current->getFunction()->needsRuntimeArgPop(S.getCtx()))
   popBuiltinArgs(S, PC);
 else
   S.Current->popArgs();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158775: [NFC] Initialize member pointer and avoid potential null dereference

[Sindhu Chittireddy via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGbbcc7c5614cd: [NFC] Initialize member pointer and avoid 
potential null dereference (authored by schittir).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158775

Files:
  clang/lib/AST/Interp/Interp.h
  clang/lib/Analysis/ThreadSafety.cpp


Index: clang/lib/Analysis/ThreadSafety.cpp
===
--- clang/lib/Analysis/ThreadSafety.cpp
+++ clang/lib/Analysis/ThreadSafety.cpp
@@ -1008,7 +1008,7 @@
   threadSafety::SExprBuilder SxBuilder;
 
   ThreadSafetyHandler 
-  const CXXMethodDecl *CurrentMethod;
+  const CXXMethodDecl *CurrentMethod = nullptr;
   LocalVariableMap LocalVarMap;
   FactManager FactMan;
   std::vector BlockInfo;
Index: clang/lib/AST/Interp/Interp.h
===
--- clang/lib/AST/Interp/Interp.h
+++ clang/lib/AST/Interp/Interp.h
@@ -201,13 +201,14 @@
   return false;
   }
 
+  assert(S.Current);
   assert(S.Current->getFrameOffset() == S.Stk.size() && "Invalid frame");
   if (!S.checkingPotentialConstantExpression() || S.Current->Caller) {
 // Certain builtin functions are declared as func-name(...), so the
 // parameters are checked in Sema and only available through the CallExpr.
 // The interp::Function we create for them has 0 parameters, so we need to
 // remove them from the stack by checking the CallExpr.
-if (S.Current && S.Current->getFunction()->needsRuntimeArgPop(S.getCtx()))
+if (S.Current->getFunction()->needsRuntimeArgPop(S.getCtx()))
   popBuiltinArgs(S, PC);
 else
   S.Current->popArgs();


Index: clang/lib/Analysis/ThreadSafety.cpp
===
--- clang/lib/Analysis/ThreadSafety.cpp
+++ clang/lib/Analysis/ThreadSafety.cpp
@@ -1008,7 +1008,7 @@
   threadSafety::SExprBuilder SxBuilder;
 
   ThreadSafetyHandler 
-  const CXXMethodDecl *CurrentMethod;
+  const CXXMethodDecl *CurrentMethod = nullptr;
   LocalVariableMap LocalVarMap;
   FactManager FactMan;
   std::vector BlockInfo;
Index: clang/lib/AST/Interp/Interp.h
===
--- clang/lib/AST/Interp/Interp.h
+++ clang/lib/AST/Interp/Interp.h
@@ -201,13 +201,14 @@
   return false;
   }
 
+  assert(S.Current);
   assert(S.Current->getFrameOffset() == S.Stk.size() && "Invalid frame");
   if (!S.checkingPotentialConstantExpression() || S.Current->Caller) {
 // Certain builtin functions are declared as func-name(...), so the
 // parameters are checked in Sema and only available through the CallExpr.
 // The interp::Function we create for them has 0 parameters, so we need to
 // remove them from the stack by checking the CallExpr.
-if (S.Current && S.Current->getFunction()->needsRuntimeArgPop(S.getCtx()))
+if (S.Current->getFunction()->needsRuntimeArgPop(S.getCtx()))
   popBuiltinArgs(S, PC);
 else
   S.Current->popArgs();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158775: [NFC] Initialize member pointer and avoid potential null dereference

[Sindhu Chittireddy via Phabricator via cfe-commits]

schittir added a comment.

Thank you for the reviews, Aaron and Aaron! 
All tests pass. Landing the patch now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158775

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

[PATCH] D158488: [NFC] Initialize member pointers to nullptr.

[Sindhu Chittireddy via Phabricator via cfe-commits]

schittir created this revision.
schittir added reviewers: aaron.ballman, tahonermann.
Herald added a subscriber: hiraditya.
Herald added a project: All.
schittir requested review of this revision.
Herald added subscribers: llvm-commits, cfe-commits, wangpc.
Herald added projects: clang, LLVM.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D158488

Files:
  clang/include/clang/AST/Stmt.h
  llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp


Index: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
===
--- llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
+++ llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
@@ -931,7 +931,7 @@
   (void)OutlinedFnID;
 
   // Return value of the runtime offloading call.
-  Value *Return;
+  Value *Return = nullptr;
 
   // Arguments for the target kernel.
   SmallVector ArgsVector;
Index: clang/include/clang/AST/Stmt.h
===
--- clang/include/clang/AST/Stmt.h
+++ clang/include/clang/AST/Stmt.h
@@ -2603,7 +2603,8 @@
   friend class ASTStmtReader;
 
   enum { INIT, CONDVAR, COND, INC, BODY, END_EXPR };
-  Stmt* SubExprs[END_EXPR]; // SubExprs[INIT] is an expression or declstmt.
+  Stmt *SubExprs[END_EXPR] = {
+  nullptr}; // SubExprs[INIT] is an expression or declstmt.
   SourceLocation LParenLoc, RParenLoc;
 
 public:


Index: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
===
--- llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
+++ llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
@@ -931,7 +931,7 @@
   (void)OutlinedFnID;
 
   // Return value of the runtime offloading call.
-  Value *Return;
+  Value *Return = nullptr;
 
   // Arguments for the target kernel.
   SmallVector ArgsVector;
Index: clang/include/clang/AST/Stmt.h
===
--- clang/include/clang/AST/Stmt.h
+++ clang/include/clang/AST/Stmt.h
@@ -2603,7 +2603,8 @@
   friend class ASTStmtReader;
 
   enum { INIT, CONDVAR, COND, INC, BODY, END_EXPR };
-  Stmt* SubExprs[END_EXPR]; // SubExprs[INIT] is an expression or declstmt.
+  Stmt *SubExprs[END_EXPR] = {
+  nullptr}; // SubExprs[INIT] is an expression or declstmt.
   SourceLocation LParenLoc, RParenLoc;
 
 public:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D155776: [NFC] Add checks for self-assignment.

[Sindhu Chittireddy via Phabricator via cfe-commits]

schittir updated this revision to Diff 552243.
schittir added a comment.

Fix errors.


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

https://reviews.llvm.org/D155776

Files:
  clang/lib/AST/APValue.cpp
  clang/lib/CodeGen/CGDebugInfo.h
  clang/lib/Interpreter/Value.cpp


Index: clang/lib/Interpreter/Value.cpp
===
--- clang/lib/Interpreter/Value.cpp
+++ clang/lib/Interpreter/Value.cpp
@@ -201,16 +201,17 @@
 }
 
 Value ::operator=(Value &) noexcept {
-  if (IsManuallyAlloc)
-ValueStorage::getFromPayload(getPtr())->Release();
+  if (this != ) {
+if (IsManuallyAlloc)
+  ValueStorage::getFromPayload(getPtr())->Release();
 
-  Interp = std::exchange(RHS.Interp, nullptr);
-  OpaqueType = std::exchange(RHS.OpaqueType, nullptr);
-  ValueKind = std::exchange(RHS.ValueKind, K_Unspecified);
-  IsManuallyAlloc = std::exchange(RHS.IsManuallyAlloc, false);
-
-  Data = RHS.Data;
+Interp = std::exchange(RHS.Interp, nullptr);
+OpaqueType = std::exchange(RHS.OpaqueType, nullptr);
+ValueKind = std::exchange(RHS.ValueKind, K_Unspecified);
+IsManuallyAlloc = std::exchange(RHS.IsManuallyAlloc, false);
 
+Data = RHS.Data;
+  }
   return *this;
 }
 
Index: clang/lib/CodeGen/CGDebugInfo.h
===
--- clang/lib/CodeGen/CGDebugInfo.h
+++ clang/lib/CodeGen/CGDebugInfo.h
@@ -832,8 +832,10 @@
 
   // Define copy assignment operator.
   ApplyDebugLocation =(ApplyDebugLocation &) {
-CGF = Other.CGF;
-Other.CGF = nullptr;
+if (this != ) {
+  CGF = Other.CGF;
+  Other.CGF = nullptr;
+}
 return *this;
   }
 
Index: clang/lib/AST/APValue.cpp
===
--- clang/lib/AST/APValue.cpp
+++ clang/lib/AST/APValue.cpp
@@ -390,11 +390,13 @@
 }
 
 APValue ::operator=(APValue &) {
-  if (Kind != None && Kind != Indeterminate)
-DestroyDataAndMakeUninit();
-  Kind = RHS.Kind;
-  Data = RHS.Data;
-  RHS.Kind = None;
+  if (this != ) {
+if (Kind != None && Kind != Indeterminate)
+  DestroyDataAndMakeUninit();
+Kind = RHS.Kind;
+Data = RHS.Data;
+RHS.Kind = None;
+  }
   return *this;
 }
 


Index: clang/lib/Interpreter/Value.cpp
===
--- clang/lib/Interpreter/Value.cpp
+++ clang/lib/Interpreter/Value.cpp
@@ -201,16 +201,17 @@
 }
 
 Value ::operator=(Value &) noexcept {
-  if (IsManuallyAlloc)
-ValueStorage::getFromPayload(getPtr())->Release();
+  if (this != ) {
+if (IsManuallyAlloc)
+  ValueStorage::getFromPayload(getPtr())->Release();
 
-  Interp = std::exchange(RHS.Interp, nullptr);
-  OpaqueType = std::exchange(RHS.OpaqueType, nullptr);
-  ValueKind = std::exchange(RHS.ValueKind, K_Unspecified);
-  IsManuallyAlloc = std::exchange(RHS.IsManuallyAlloc, false);
-
-  Data = RHS.Data;
+Interp = std::exchange(RHS.Interp, nullptr);
+OpaqueType = std::exchange(RHS.OpaqueType, nullptr);
+ValueKind = std::exchange(RHS.ValueKind, K_Unspecified);
+IsManuallyAlloc = std::exchange(RHS.IsManuallyAlloc, false);
 
+Data = RHS.Data;
+  }
   return *this;
 }
 
Index: clang/lib/CodeGen/CGDebugInfo.h
===
--- clang/lib/CodeGen/CGDebugInfo.h
+++ clang/lib/CodeGen/CGDebugInfo.h
@@ -832,8 +832,10 @@
 
   // Define copy assignment operator.
   ApplyDebugLocation =(ApplyDebugLocation &) {
-CGF = Other.CGF;
-Other.CGF = nullptr;
+if (this != ) {
+  CGF = Other.CGF;
+  Other.CGF = nullptr;
+}
 return *this;
   }
 
Index: clang/lib/AST/APValue.cpp
===
--- clang/lib/AST/APValue.cpp
+++ clang/lib/AST/APValue.cpp
@@ -390,11 +390,13 @@
 }
 
 APValue ::operator=(APValue &) {
-  if (Kind != None && Kind != Indeterminate)
-DestroyDataAndMakeUninit();
-  Kind = RHS.Kind;
-  Data = RHS.Data;
-  RHS.Kind = None;
+  if (this != ) {
+if (Kind != None && Kind != Indeterminate)
+  DestroyDataAndMakeUninit();
+Kind = RHS.Kind;
+Data = RHS.Data;
+RHS.Kind = None;
+  }
   return *this;
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D155776: [NFC] Add checks for self-assignment.

[Sindhu Chittireddy via Phabricator via cfe-commits]

schittir added a comment.

@jplehr @michaelplatings @MitalAshok @tahonermann - sorry this one slipped 
through the cracks. I fixed the errors now. Thank you for your review.


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

https://reviews.llvm.org/D155776

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

[PATCH] D158488: [NFC] Initialize member pointers to nullptr.

[Sindhu Chittireddy via Phabricator via cfe-commits]

schittir marked 2 inline comments as done.
schittir added inline comments.



Comment at: clang/include/clang/AST/Stmt.h:2606-2607
   enum { INIT, CONDVAR, COND, INC, BODY, END_EXPR };
-  Stmt* SubExprs[END_EXPR]; // SubExprs[INIT] is an expression or declstmt.
+  Stmt *SubExprs[END_EXPR] = {
+  nullptr}; // SubExprs[INIT] is an expression or declstmt.
   SourceLocation LParenLoc, RParenLoc;

tahonermann wrote:
> aaron.ballman wrote:
> > I don't think this initialization is necessary. The constructor for 
> > `ForStmt` initializes all of the valid elements: 
> > https://github.com/llvm/llvm-project/blob/87c5a3e203ae3643bc13c9a13917b92a657f4358/clang/lib/AST/Stmt.cpp#L1024
> > 
> > The `EmptyShell` constructor does not initialize anything but that's 
> > because it is piecemeal initialized by the AST importer, but all of its 
> > fields are also initialized: 
> > https://github.com/llvm/llvm-project/blob/87c5a3e203ae3643bc13c9a13917b92a657f4358/clang/lib/Serialization/ASTReaderStmt.cpp#L296
> Declarations like this are common in the AST. I'm curious while static 
> analysis flagged this one in particular. Perhaps it identified a path where 
> one or more of the elements don't get initialized? If so, this would be a 
> good find and a fix should be applied to that path.
I will follow-up with this one more and open a different PR if necessary. 
Removing this change. 



Comment at: clang/include/clang/AST/Stmt.h:2606-2607
   enum { INIT, CONDVAR, COND, INC, BODY, END_EXPR };
-  Stmt* SubExprs[END_EXPR]; // SubExprs[INIT] is an expression or declstmt.
+  Stmt *SubExprs[END_EXPR] = {
+  nullptr}; // SubExprs[INIT] is an expression or declstmt.
   SourceLocation LParenLoc, RParenLoc;

schittir wrote:
> tahonermann wrote:
> > aaron.ballman wrote:
> > > I don't think this initialization is necessary. The constructor for 
> > > `ForStmt` initializes all of the valid elements: 
> > > https://github.com/llvm/llvm-project/blob/87c5a3e203ae3643bc13c9a13917b92a657f4358/clang/lib/AST/Stmt.cpp#L1024
> > > 
> > > The `EmptyShell` constructor does not initialize anything but that's 
> > > because it is piecemeal initialized by the AST importer, but all of its 
> > > fields are also initialized: 
> > > https://github.com/llvm/llvm-project/blob/87c5a3e203ae3643bc13c9a13917b92a657f4358/clang/lib/Serialization/ASTReaderStmt.cpp#L296
> > Declarations like this are common in the AST. I'm curious while static 
> > analysis flagged this one in particular. Perhaps it identified a path where 
> > one or more of the elements don't get initialized? If so, this would be a 
> > good find and a fix should be applied to that path.
> I will follow-up with this one more and open a different PR if necessary. 
> Removing this change. 
Thank you for this analysis!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158488

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

[PATCH] D158488: [NFC] Initialize member pointers to nullptr.

[Sindhu Chittireddy via Phabricator via cfe-commits]

schittir updated this revision to Diff 552380.
schittir added a comment.

Remove unnecessary initialization.


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

https://reviews.llvm.org/D158488

Files:
  llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp


Index: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
===
--- llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
+++ llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
@@ -931,7 +931,7 @@
   (void)OutlinedFnID;
 
   // Return value of the runtime offloading call.
-  Value *Return;
+  Value *Return = nullptr;
 
   // Arguments for the target kernel.
   SmallVector ArgsVector;


Index: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
===
--- llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
+++ llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
@@ -931,7 +931,7 @@
   (void)OutlinedFnID;
 
   // Return value of the runtime offloading call.
-  Value *Return;
+  Value *Return = nullptr;
 
   // Arguments for the target kernel.
   SmallVector ArgsVector;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D155776: [NFC] Add checks for self-assignment.

[Sindhu Chittireddy via Phabricator via cfe-commits]

schittir marked an inline comment as done.
schittir added a comment.

Thank you for the review, @aaron.ballman and @tahonermann 
Investigating the build failure...


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

https://reviews.llvm.org/D155776

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

[PATCH] D158488: [NFC] Initialize member pointers to nullptr.

[Sindhu Chittireddy via Phabricator via cfe-commits]

schittir abandoned this revision.
schittir added a comment.

In D158488#4607754 , @Manna wrote:

> This has already been addressed by https://reviews.llvm.org/D157989

Thank you for letting me know. Abandoning this change.


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

https://reviews.llvm.org/D158488

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

[PATCH] D155776: [NFC] Add checks for self-assignment.

[Sindhu Chittireddy via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG4ad89131e0de: [NFC] Add checks for self-assignment. 
(authored by schittir).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155776

Files:
  clang/lib/AST/APValue.cpp
  clang/lib/CodeGen/CGDebugInfo.h
  clang/lib/Interpreter/Value.cpp


Index: clang/lib/Interpreter/Value.cpp
===
--- clang/lib/Interpreter/Value.cpp
+++ clang/lib/Interpreter/Value.cpp
@@ -201,16 +201,17 @@
 }
 
 Value ::operator=(Value &) noexcept {
-  if (IsManuallyAlloc)
-ValueStorage::getFromPayload(getPtr())->Release();
+  if (this != ) {
+if (IsManuallyAlloc)
+  ValueStorage::getFromPayload(getPtr())->Release();
 
-  Interp = std::exchange(RHS.Interp, nullptr);
-  OpaqueType = std::exchange(RHS.OpaqueType, nullptr);
-  ValueKind = std::exchange(RHS.ValueKind, K_Unspecified);
-  IsManuallyAlloc = std::exchange(RHS.IsManuallyAlloc, false);
-
-  Data = RHS.Data;
+Interp = std::exchange(RHS.Interp, nullptr);
+OpaqueType = std::exchange(RHS.OpaqueType, nullptr);
+ValueKind = std::exchange(RHS.ValueKind, K_Unspecified);
+IsManuallyAlloc = std::exchange(RHS.IsManuallyAlloc, false);
 
+Data = RHS.Data;
+  }
   return *this;
 }
 
Index: clang/lib/CodeGen/CGDebugInfo.h
===
--- clang/lib/CodeGen/CGDebugInfo.h
+++ clang/lib/CodeGen/CGDebugInfo.h
@@ -835,8 +835,10 @@
 
   // Define copy assignment operator.
   ApplyDebugLocation =(ApplyDebugLocation &) {
-CGF = Other.CGF;
-Other.CGF = nullptr;
+if (this != ) {
+  CGF = Other.CGF;
+  Other.CGF = nullptr;
+}
 return *this;
   }
 
Index: clang/lib/AST/APValue.cpp
===
--- clang/lib/AST/APValue.cpp
+++ clang/lib/AST/APValue.cpp
@@ -390,11 +390,13 @@
 }
 
 APValue ::operator=(APValue &) {
-  if (Kind != None && Kind != Indeterminate)
-DestroyDataAndMakeUninit();
-  Kind = RHS.Kind;
-  Data = RHS.Data;
-  RHS.Kind = None;
+  if (this != ) {
+if (Kind != None && Kind != Indeterminate)
+  DestroyDataAndMakeUninit();
+Kind = RHS.Kind;
+Data = RHS.Data;
+RHS.Kind = None;
+  }
   return *this;
 }
 


Index: clang/lib/Interpreter/Value.cpp
===
--- clang/lib/Interpreter/Value.cpp
+++ clang/lib/Interpreter/Value.cpp
@@ -201,16 +201,17 @@
 }
 
 Value ::operator=(Value &) noexcept {
-  if (IsManuallyAlloc)
-ValueStorage::getFromPayload(getPtr())->Release();
+  if (this != ) {
+if (IsManuallyAlloc)
+  ValueStorage::getFromPayload(getPtr())->Release();
 
-  Interp = std::exchange(RHS.Interp, nullptr);
-  OpaqueType = std::exchange(RHS.OpaqueType, nullptr);
-  ValueKind = std::exchange(RHS.ValueKind, K_Unspecified);
-  IsManuallyAlloc = std::exchange(RHS.IsManuallyAlloc, false);
-
-  Data = RHS.Data;
+Interp = std::exchange(RHS.Interp, nullptr);
+OpaqueType = std::exchange(RHS.OpaqueType, nullptr);
+ValueKind = std::exchange(RHS.ValueKind, K_Unspecified);
+IsManuallyAlloc = std::exchange(RHS.IsManuallyAlloc, false);
 
+Data = RHS.Data;
+  }
   return *this;
 }
 
Index: clang/lib/CodeGen/CGDebugInfo.h
===
--- clang/lib/CodeGen/CGDebugInfo.h
+++ clang/lib/CodeGen/CGDebugInfo.h
@@ -835,8 +835,10 @@
 
   // Define copy assignment operator.
   ApplyDebugLocation =(ApplyDebugLocation &) {
-CGF = Other.CGF;
-Other.CGF = nullptr;
+if (this != ) {
+  CGF = Other.CGF;
+  Other.CGF = nullptr;
+}
 return *this;
   }
 
Index: clang/lib/AST/APValue.cpp
===
--- clang/lib/AST/APValue.cpp
+++ clang/lib/AST/APValue.cpp
@@ -390,11 +390,13 @@
 }
 
 APValue ::operator=(APValue &) {
-  if (Kind != None && Kind != Indeterminate)
-DestroyDataAndMakeUninit();
-  Kind = RHS.Kind;
-  Data = RHS.Data;
-  RHS.Kind = None;
+  if (this != ) {
+if (Kind != None && Kind != Indeterminate)
+  DestroyDataAndMakeUninit();
+Kind = RHS.Kind;
+Data = RHS.Data;
+RHS.Kind = None;
+  }
   return *this;
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D139148: Fix nullptr dereference found by Coverity static analysis tool

[Sindhu Chittireddy via Phabricator via cfe-commits]

schittir added inline comments.



Comment at: clang/lib/Sema/SemaInit.cpp:5824-5828
   // Handle default initialization.
   if (Kind.getKind() == InitializationKind::IK_Default) {
 TryDefaultInitialization(S, Entity, Kind, *this);
 return;
   }

tahonermann wrote:
> This block handles default initialization and unconditionally performs a 
> return. I wonder if this effectively guarantees that `Initializer` is 
> non-null if this block is not entered.
Thank you for the review. I am trying to find an explicit connection between 
this block and `Initializer` being non-null, but it isn't clear to me yet. 
How about adding the assert right after this block?



Comment at: clang/lib/Sema/SemaInit.cpp:5941
 if (allowObjCWritebackConversion &&
 tryObjCWritebackConversion(S, *this, Entity, Initializer)) {
   return;

tahonermann wrote:
> This use of `Initializer` is also questionable; 
> `tryObjCWritebackConversion()` will unconditionally dereference it.
Indeed. This set of calls should be addressed too. 


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

https://reviews.llvm.org/D139148

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

[PATCH] D139148: Fix nullptr dereference found by Coverity static analysis tool

[Sindhu Chittireddy via Phabricator via cfe-commits]

schittir updated this revision to Diff 479731.
schittir added a comment.

Add assert instead of multiple checks in if conditions.


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

https://reviews.llvm.org/D139148

Files:
  clang/lib/Sema/SemaInit.cpp


Index: clang/lib/Sema/SemaInit.cpp
===
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -5827,6 +5827,8 @@
 return;
   }
 
+  assert(Initializer && "Intializer must be non-null");
+
   // - If the destination type is an array of characters, an array of
   //   char16_t, an array of char32_t, or an array of wchar_t, and the
   //   initializer is a string literal, see 8.5.2.


Index: clang/lib/Sema/SemaInit.cpp
===
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -5827,6 +5827,8 @@
 return;
   }
 
+  assert(Initializer && "Intializer must be non-null");
+
   // - If the destination type is an array of characters, an array of
   //   char16_t, an array of char32_t, or an array of wchar_t, and the
   //   initializer is a string literal, see 8.5.2.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D139148: Fix nullptr dereference found by Coverity static analysis tool

[Sindhu Chittireddy via Phabricator via cfe-commits]

schittir updated this revision to Diff 481974.
schittir marked an inline comment as done.
schittir added a comment.

Fixed typo. Sorry!


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

https://reviews.llvm.org/D139148

Files:
  clang/lib/Sema/SemaInit.cpp


Index: clang/lib/Sema/SemaInit.cpp
===
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -5936,6 +5936,7 @@
   // We're at the end of the line for C: it's either a write-back conversion
   // or it's a C assignment. There's no need to check anything else.
   if (!S.getLangOpts().CPlusPlus) {
+assert(Initializer && "Intializer must be non-null");
 // If allowed, check whether this is an Objective-C writeback conversion.
 if (allowObjCWritebackConversion &&
 tryObjCWritebackConversion(S, *this, Entity, Initializer)) {
@@ -5962,7 +5963,8 @@
 if (Kind.getKind() == InitializationKind::IK_Direct ||
 (Kind.getKind() == InitializationKind::IK_Copy &&
  (Context.hasSameUnqualifiedType(SourceType, DestType) ||
-  S.IsDerivedFrom(Initializer->getBeginLoc(), SourceType, DestType
+  (Initializer &&
+   S.IsDerivedFrom(Initializer->getBeginLoc(), SourceType, 
DestType)
   TryConstructorInitialization(S, Entity, Kind, Args,
DestType, DestType, *this);
 // - Otherwise (i.e., for the remaining copy-initialization cases),
@@ -5971,9 +5973,11 @@
 //   used) to a derived class thereof are enumerated as described in
 //   13.3.1.4, and the best one is chosen through overload resolution
 //   (13.3).
-else
+else {
+  assert(Initializer && "Intializer must be non-null");
   TryUserDefinedConversion(S, DestType, Kind, Initializer, *this,
TopLevelOfInitList);
+}
 return;
   }
 
@@ -6022,6 +6026,7 @@
   //- Otherwise, if the source type is a (possibly cv-qualified) class
   //  type, conversion functions are considered.
   if (!SourceType.isNull() && SourceType->isRecordType()) {
+assert(Initializer && "Intializer must be non-null");
 // For a conversion to _Atomic(T) from either T or a class type derived
 // from T, initialize the T object then convert to _Atomic type.
 bool NeedAtomicConversion = false;


Index: clang/lib/Sema/SemaInit.cpp
===
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -5936,6 +5936,7 @@
   // We're at the end of the line for C: it's either a write-back conversion
   // or it's a C assignment. There's no need to check anything else.
   if (!S.getLangOpts().CPlusPlus) {
+assert(Initializer && "Intializer must be non-null");
 // If allowed, check whether this is an Objective-C writeback conversion.
 if (allowObjCWritebackConversion &&
 tryObjCWritebackConversion(S, *this, Entity, Initializer)) {
@@ -5962,7 +5963,8 @@
 if (Kind.getKind() == InitializationKind::IK_Direct ||
 (Kind.getKind() == InitializationKind::IK_Copy &&
  (Context.hasSameUnqualifiedType(SourceType, DestType) ||
-  S.IsDerivedFrom(Initializer->getBeginLoc(), SourceType, DestType
+  (Initializer &&
+   S.IsDerivedFrom(Initializer->getBeginLoc(), SourceType, DestType)
   TryConstructorInitialization(S, Entity, Kind, Args,
DestType, DestType, *this);
 // - Otherwise (i.e., for the remaining copy-initialization cases),
@@ -5971,9 +5973,11 @@
 //   used) to a derived class thereof are enumerated as described in
 //   13.3.1.4, and the best one is chosen through overload resolution
 //   (13.3).
-else
+else {
+  assert(Initializer && "Intializer must be non-null");
   TryUserDefinedConversion(S, DestType, Kind, Initializer, *this,
TopLevelOfInitList);
+}
 return;
   }
 
@@ -6022,6 +6026,7 @@
   //- Otherwise, if the source type is a (possibly cv-qualified) class
   //  type, conversion functions are considered.
   if (!SourceType.isNull() && SourceType->isRecordType()) {
+assert(Initializer && "Intializer must be non-null");
 // For a conversion to _Atomic(T) from either T or a class type derived
 // from T, initialize the T object then convert to _Atomic type.
 bool NeedAtomicConversion = false;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D139148: Fix nullptr dereference found by Coverity static analysis tool

[Sindhu Chittireddy via Phabricator via cfe-commits]

schittir updated this revision to Diff 481989.
schittir added a comment.

Fix typos


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

https://reviews.llvm.org/D139148

Files:
  clang/lib/Sema/SemaInit.cpp


Index: clang/lib/Sema/SemaInit.cpp
===
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -5936,6 +5936,7 @@
   // We're at the end of the line for C: it's either a write-back conversion
   // or it's a C assignment. There's no need to check anything else.
   if (!S.getLangOpts().CPlusPlus) {
+assert(Initializer && "Initializer must be non-null");
 // If allowed, check whether this is an Objective-C writeback conversion.
 if (allowObjCWritebackConversion &&
 tryObjCWritebackConversion(S, *this, Entity, Initializer)) {
@@ -5962,7 +5963,8 @@
 if (Kind.getKind() == InitializationKind::IK_Direct ||
 (Kind.getKind() == InitializationKind::IK_Copy &&
  (Context.hasSameUnqualifiedType(SourceType, DestType) ||
-  S.IsDerivedFrom(Initializer->getBeginLoc(), SourceType, DestType
+  (Initializer &&
+   S.IsDerivedFrom(Initializer->getBeginLoc(), SourceType, 
DestType)
   TryConstructorInitialization(S, Entity, Kind, Args,
DestType, DestType, *this);
 // - Otherwise (i.e., for the remaining copy-initialization cases),
@@ -5971,9 +5973,11 @@
 //   used) to a derived class thereof are enumerated as described in
 //   13.3.1.4, and the best one is chosen through overload resolution
 //   (13.3).
-else
+else {
+  assert(Initializer && "Initializer must be non-null");
   TryUserDefinedConversion(S, DestType, Kind, Initializer, *this,
TopLevelOfInitList);
+}
 return;
   }
 
@@ -6022,6 +6026,7 @@
   //- Otherwise, if the source type is a (possibly cv-qualified) class
   //  type, conversion functions are considered.
   if (!SourceType.isNull() && SourceType->isRecordType()) {
+assert(Initializer && "Initializer must be non-null");
 // For a conversion to _Atomic(T) from either T or a class type derived
 // from T, initialize the T object then convert to _Atomic type.
 bool NeedAtomicConversion = false;


Index: clang/lib/Sema/SemaInit.cpp
===
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -5936,6 +5936,7 @@
   // We're at the end of the line for C: it's either a write-back conversion
   // or it's a C assignment. There's no need to check anything else.
   if (!S.getLangOpts().CPlusPlus) {
+assert(Initializer && "Initializer must be non-null");
 // If allowed, check whether this is an Objective-C writeback conversion.
 if (allowObjCWritebackConversion &&
 tryObjCWritebackConversion(S, *this, Entity, Initializer)) {
@@ -5962,7 +5963,8 @@
 if (Kind.getKind() == InitializationKind::IK_Direct ||
 (Kind.getKind() == InitializationKind::IK_Copy &&
  (Context.hasSameUnqualifiedType(SourceType, DestType) ||
-  S.IsDerivedFrom(Initializer->getBeginLoc(), SourceType, DestType
+  (Initializer &&
+   S.IsDerivedFrom(Initializer->getBeginLoc(), SourceType, DestType)
   TryConstructorInitialization(S, Entity, Kind, Args,
DestType, DestType, *this);
 // - Otherwise (i.e., for the remaining copy-initialization cases),
@@ -5971,9 +5973,11 @@
 //   used) to a derived class thereof are enumerated as described in
 //   13.3.1.4, and the best one is chosen through overload resolution
 //   (13.3).
-else
+else {
+  assert(Initializer && "Initializer must be non-null");
   TryUserDefinedConversion(S, DestType, Kind, Initializer, *this,
TopLevelOfInitList);
+}
 return;
   }
 
@@ -6022,6 +6026,7 @@
   //- Otherwise, if the source type is a (possibly cv-qualified) class
   //  type, conversion functions are considered.
   if (!SourceType.isNull() && SourceType->isRecordType()) {
+assert(Initializer && "Initializer must be non-null");
 // For a conversion to _Atomic(T) from either T or a class type derived
 // from T, initialize the T object then convert to _Atomic type.
 bool NeedAtomicConversion = false;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D139148: Fix nullptr dereference found by Coverity static analysis tool

[Sindhu Chittireddy via Phabricator via cfe-commits]

schittir updated this revision to Diff 481732.
schittir added a comment.

Add `(Initializer && ` and assert to else branch per Tom's comments.


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

https://reviews.llvm.org/D139148

Files:
  clang/lib/Sema/SemaInit.cpp


Index: clang/lib/Sema/SemaInit.cpp
===
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -5936,6 +5936,7 @@
   // We're at the end of the line for C: it's either a write-back conversion
   // or it's a C assignment. There's no need to check anything else.
   if (!S.getLangOpts().CPlusPlus) {
+assert(Initializer && "Intializer must be non-null");
 // If allowed, check whether this is an Objective-C writeback conversion.
 if (allowObjCWritebackConversion &&
 tryObjCWritebackConversion(S, *this, Entity, Initializer)) {
@@ -5962,7 +5963,8 @@
 if (Kind.getKind() == InitializationKind::IK_Direct ||
 (Kind.getKind() == InitializationKind::IK_Copy &&
  (Context.hasSameUnqualifiedType(SourceType, DestType) ||
-  S.IsDerivedFrom(Initializer->getBeginLoc(), SourceType, DestType
+  (Initilializer &&
+   S.IsDerivedFrom(Initializer->getBeginLoc(), SourceType, 
DestType)
   TryConstructorInitialization(S, Entity, Kind, Args,
DestType, DestType, *this);
 // - Otherwise (i.e., for the remaining copy-initialization cases),
@@ -5971,9 +5973,11 @@
 //   used) to a derived class thereof are enumerated as described in
 //   13.3.1.4, and the best one is chosen through overload resolution
 //   (13.3).
-else
+else {
+  assert(Initializer && "Intializer must be non-null");
   TryUserDefinedConversion(S, DestType, Kind, Initializer, *this,
TopLevelOfInitList);
+}
 return;
   }
 
@@ -6022,6 +6026,7 @@
   //- Otherwise, if the source type is a (possibly cv-qualified) class
   //  type, conversion functions are considered.
   if (!SourceType.isNull() && SourceType->isRecordType()) {
+assert(Initializer && "Intializer must be non-null");
 // For a conversion to _Atomic(T) from either T or a class type derived
 // from T, initialize the T object then convert to _Atomic type.
 bool NeedAtomicConversion = false;


Index: clang/lib/Sema/SemaInit.cpp
===
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -5936,6 +5936,7 @@
   // We're at the end of the line for C: it's either a write-back conversion
   // or it's a C assignment. There's no need to check anything else.
   if (!S.getLangOpts().CPlusPlus) {
+assert(Initializer && "Intializer must be non-null");
 // If allowed, check whether this is an Objective-C writeback conversion.
 if (allowObjCWritebackConversion &&
 tryObjCWritebackConversion(S, *this, Entity, Initializer)) {
@@ -5962,7 +5963,8 @@
 if (Kind.getKind() == InitializationKind::IK_Direct ||
 (Kind.getKind() == InitializationKind::IK_Copy &&
  (Context.hasSameUnqualifiedType(SourceType, DestType) ||
-  S.IsDerivedFrom(Initializer->getBeginLoc(), SourceType, DestType
+  (Initilializer &&
+   S.IsDerivedFrom(Initializer->getBeginLoc(), SourceType, DestType)
   TryConstructorInitialization(S, Entity, Kind, Args,
DestType, DestType, *this);
 // - Otherwise (i.e., for the remaining copy-initialization cases),
@@ -5971,9 +5973,11 @@
 //   used) to a derived class thereof are enumerated as described in
 //   13.3.1.4, and the best one is chosen through overload resolution
 //   (13.3).
-else
+else {
+  assert(Initializer && "Intializer must be non-null");
   TryUserDefinedConversion(S, DestType, Kind, Initializer, *this,
TopLevelOfInitList);
+}
 return;
   }
 
@@ -6022,6 +6026,7 @@
   //- Otherwise, if the source type is a (possibly cv-qualified) class
   //  type, conversion functions are considered.
   if (!SourceType.isNull() && SourceType->isRecordType()) {
+assert(Initializer && "Intializer must be non-null");
 // For a conversion to _Atomic(T) from either T or a class type derived
 // from T, initialize the T object then convert to _Atomic type.
 bool NeedAtomicConversion = false;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D139148: Fix nullptr dereference found by Coverity static analysis tool

[Sindhu Chittireddy via Phabricator via cfe-commits]

schittir added a comment.






Comment at: clang/lib/Sema/SemaInit.cpp:5947
 
 if (TryOCLZeroOpaqueTypeInitialization(S, *this, DestType, Initializer))
   return;

#1/5 Coverity reported explicit null dereference of 'Initializer'



Comment at: clang/lib/Sema/SemaInit.cpp:5967
  (Context.hasSameUnqualifiedType(SourceType, DestType) ||
   S.IsDerivedFrom(Initializer->getBeginLoc(), SourceType, DestType
   TryConstructorInitialization(S, Entity, Kind, Args,

#2/5 Coverity reported explicit null dereference of 'Initializer'



Comment at: clang/lib/Sema/SemaInit.cpp:5977
 else
   TryUserDefinedConversion(S, DestType, Kind, Initializer, *this,
TopLevelOfInitList);

#3/5 Coverity reported explicit null dereference of 'Initializer'



Comment at: clang/lib/Sema/SemaInit.cpp:6032
   if (Context.hasSameUnqualifiedType(SourceType, Atomic->getValueType()) ||
   S.IsDerivedFrom(Initializer->getBeginLoc(), SourceType,
   Atomic->getValueType())) {

#4/5 Coverity reported explicit null dereference of 'Initializer'



Comment at: clang/lib/Sema/SemaInit.cpp:6039
 
 TryUserDefinedConversion(S, DestType, Kind, Initializer, *this,
  TopLevelOfInitList);

#5/5 Coverity reported explicit null dereference of 'Initializer'


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

https://reviews.llvm.org/D139148

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

[PATCH] D139148: Fix nullptr dereference found by Coverity static analysis tool

[Sindhu Chittireddy via Phabricator via cfe-commits]

schittir added a comment.

In D139148#3969383 , @tahonermann 
wrote:

> When committing changes to address Coverity reported issues, I think it would 
> be useful to include the Coverity analysis in the commit message. A textual 
> representation can be obtained using `cov-format-errors --emacs-style` from 
> the command line with access to the intermediate directory. I'm not sure if a 
> textual representation can be obtained through the Coverity UI though.

Couldn't find a way to do this via the UI, so I am trying the command line way.


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

https://reviews.llvm.org/D139148

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

[PATCH] D139148: Fix nullptr dereference found by Coverity static analysis tool

[Sindhu Chittireddy via Phabricator via cfe-commits]

schittir updated this revision to Diff 480217.
schittir added a comment.

Move the assert to after the branches that handle the cases where Initializer 
may be null


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

https://reviews.llvm.org/D139148

Files:
  clang/lib/Sema/SemaInit.cpp


Index: clang/lib/Sema/SemaInit.cpp
===
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -5925,6 +5925,8 @@
 return;
   }
 
+  assert(Initializer && "Intializer must be non-null");
+
   // Determine whether we should consider writeback conversions for
   // Objective-C ARC.
   bool allowObjCWritebackConversion = S.getLangOpts().ObjCAutoRefCount &&


Index: clang/lib/Sema/SemaInit.cpp
===
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -5925,6 +5925,8 @@
 return;
   }
 
+  assert(Initializer && "Intializer must be non-null");
+
   // Determine whether we should consider writeback conversions for
   // Objective-C ARC.
   bool allowObjCWritebackConversion = S.getLangOpts().ObjCAutoRefCount &&
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D139148: Fix nullptr dereference found by Coverity static analysis tool

[Sindhu Chittireddy via Phabricator via cfe-commits]

schittir added a comment.



> Perhaps the` assert` should be added after line 5926 above.

Done.

> I think the checks for `Initializer` being non-null following the addition of 
> an `assert` should be removed.

I don't see any after this point.


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

https://reviews.llvm.org/D139148

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

[PATCH] D139148: Fix nullptr dereference found by Coverity static analysis tool

[Sindhu Chittireddy via Phabricator via cfe-commits]

schittir updated this revision to Diff 481475.
schittir edited the summary of this revision.
schittir added a comment.

Add assert at the beginning of blocks pointed out by Coverity


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

https://reviews.llvm.org/D139148

Files:
  clang/lib/Sema/SemaInit.cpp


Index: clang/lib/Sema/SemaInit.cpp
===
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -5936,6 +5936,7 @@
   // We're at the end of the line for C: it's either a write-back conversion
   // or it's a C assignment. There's no need to check anything else.
   if (!S.getLangOpts().CPlusPlus) {
+assert(Initializer && "Intializer must be non-null");
 // If allowed, check whether this is an Objective-C writeback conversion.
 if (allowObjCWritebackConversion &&
 tryObjCWritebackConversion(S, *this, Entity, Initializer)) {
@@ -5955,6 +5956,7 @@
 
   // - If the destination type is a (possibly cv-qualified) class type:
   if (DestType->isRecordType()) {
+assert(Initializer && "Intializer must be non-null");
 // - If the initialization is direct-initialization, or if it is
 //   copy-initialization where the cv-unqualified version of the
 //   source type is the same class as, or a derived class of, the
@@ -6022,6 +6024,7 @@
   //- Otherwise, if the source type is a (possibly cv-qualified) class
   //  type, conversion functions are considered.
   if (!SourceType.isNull() && SourceType->isRecordType()) {
+assert(Initializer && "Intializer must be non-null");
 // For a conversion to _Atomic(T) from either T or a class type derived
 // from T, initialize the T object then convert to _Atomic type.
 bool NeedAtomicConversion = false;


Index: clang/lib/Sema/SemaInit.cpp
===
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -5936,6 +5936,7 @@
   // We're at the end of the line for C: it's either a write-back conversion
   // or it's a C assignment. There's no need to check anything else.
   if (!S.getLangOpts().CPlusPlus) {
+assert(Initializer && "Intializer must be non-null");
 // If allowed, check whether this is an Objective-C writeback conversion.
 if (allowObjCWritebackConversion &&
 tryObjCWritebackConversion(S, *this, Entity, Initializer)) {
@@ -5955,6 +5956,7 @@
 
   // - If the destination type is a (possibly cv-qualified) class type:
   if (DestType->isRecordType()) {
+assert(Initializer && "Intializer must be non-null");
 // - If the initialization is direct-initialization, or if it is
 //   copy-initialization where the cv-unqualified version of the
 //   source type is the same class as, or a derived class of, the
@@ -6022,6 +6024,7 @@
   //- Otherwise, if the source type is a (possibly cv-qualified) class
   //  type, conversion functions are considered.
   if (!SourceType.isNull() && SourceType->isRecordType()) {
+assert(Initializer && "Intializer must be non-null");
 // For a conversion to _Atomic(T) from either T or a class type derived
 // from T, initialize the T object then convert to _Atomic type.
 bool NeedAtomicConversion = false;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D139148: Fix nullptr dereference found by Coverity static analysis tool

[Sindhu Chittireddy via Phabricator via cfe-commits]

schittir created this revision.
Herald added a project: All.
schittir requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Adding nullptr check for 'Initializer'


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D139148

Files:
  clang/lib/Sema/SemaInit.cpp


Index: clang/lib/Sema/SemaInit.cpp
===
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -5962,9 +5962,10 @@
 if (Kind.getKind() == InitializationKind::IK_Direct ||
 (Kind.getKind() == InitializationKind::IK_Copy &&
  (Context.hasSameUnqualifiedType(SourceType, DestType) ||
-  S.IsDerivedFrom(Initializer->getBeginLoc(), SourceType, DestType
-  TryConstructorInitialization(S, Entity, Kind, Args,
-   DestType, DestType, *this);
+  (Initializer &&
+   S.IsDerivedFrom(Initializer->getBeginLoc(), SourceType, 
DestType)
+  TryConstructorInitialization(S, Entity, Kind, Args, DestType, DestType,
+   *this);
 // - Otherwise (i.e., for the remaining copy-initialization cases),
 //   user-defined conversion sequences that can convert from the source
 //   type to the destination type or (when a conversion function is


Index: clang/lib/Sema/SemaInit.cpp
===
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -5962,9 +5962,10 @@
 if (Kind.getKind() == InitializationKind::IK_Direct ||
 (Kind.getKind() == InitializationKind::IK_Copy &&
  (Context.hasSameUnqualifiedType(SourceType, DestType) ||
-  S.IsDerivedFrom(Initializer->getBeginLoc(), SourceType, DestType
-  TryConstructorInitialization(S, Entity, Kind, Args,
-   DestType, DestType, *this);
+  (Initializer &&
+   S.IsDerivedFrom(Initializer->getBeginLoc(), SourceType, DestType)
+  TryConstructorInitialization(S, Entity, Kind, Args, DestType, DestType,
+   *this);
 // - Otherwise (i.e., for the remaining copy-initialization cases),
 //   user-defined conversion sequences that can convert from the source
 //   type to the destination type or (when a conversion function is
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D139148: Fix nullptr dereference found by Coverity static analysis tool

[Sindhu Chittireddy via Phabricator via cfe-commits]

schittir added a comment.

In D139148#3965404 , @shafik wrote:

> This looks like the correct thing to do but I see further down there are also 
> unconditional uses of `Initializer` that should also be guarded with a check. 
> It would be good to fix those as well since we are here.

Yes, it does make sense.

> I am guessing the answer is no since this was found using a static analysis 
> tool but do you have a test case that triggers this failure?

Indeed, the answer is no.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139148

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

[PATCH] D139148: Fix nullptr dereference found by Coverity static analysis tool

[Sindhu Chittireddy via Phabricator via cfe-commits]

schittir updated this revision to Diff 479541.
schittir added a comment.

Add more nullptr checks per Shafik's comments


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

https://reviews.llvm.org/D139148

Files:
  clang/lib/Sema/SemaInit.cpp


Index: clang/lib/Sema/SemaInit.cpp
===
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -5962,9 +5962,10 @@
 if (Kind.getKind() == InitializationKind::IK_Direct ||
 (Kind.getKind() == InitializationKind::IK_Copy &&
  (Context.hasSameUnqualifiedType(SourceType, DestType) ||
-  S.IsDerivedFrom(Initializer->getBeginLoc(), SourceType, DestType
-  TryConstructorInitialization(S, Entity, Kind, Args,
-   DestType, DestType, *this);
+  (Initializer &&
+   S.IsDerivedFrom(Initializer->getBeginLoc(), SourceType, 
DestType)
+  TryConstructorInitialization(S, Entity, Kind, Args, DestType, DestType,
+   *this);
 // - Otherwise (i.e., for the remaining copy-initialization cases),
 //   user-defined conversion sequences that can convert from the source
 //   type to the destination type or (when a conversion function is
@@ -6027,8 +6028,8 @@
 bool NeedAtomicConversion = false;
 if (const AtomicType *Atomic = DestType->getAs()) {
   if (Context.hasSameUnqualifiedType(SourceType, Atomic->getValueType()) ||
-  S.IsDerivedFrom(Initializer->getBeginLoc(), SourceType,
-  Atomic->getValueType())) {
+  (Initializer && S.IsDerivedFrom(Initializer->getBeginLoc(),
+  SourceType, Atomic->getValueType())) 
{
 DestType = Atomic->getValueType();
 NeedAtomicConversion = true;
   }
@@ -6045,7 +6046,7 @@
   //- Otherwise, if the initialization is direct-initialization, the source
   //type is std::nullptr_t, and the destination type is bool, the initial
   //value of the object being initialized is false.
-  if (!SourceType.isNull() && SourceType->isNullPtrType() &&
+  if (!SourceType.isNull() && SourceType->isNullPtrType() && Initializer &&
   DestType->isBooleanType() &&
   Kind.getKind() == InitializationKind::IK_Direct) {
 AddConversionSequenceStep(
@@ -6095,11 +6096,11 @@
 DeclAccessPair dap;
 if (isLibstdcxxPointerReturnFalseHack(S, Entity, Initializer)) {
   AddZeroInitializationStep(Entity.getType());
-} else if (Initializer->getType() == Context.OverloadTy &&
+} else if (Initializer && Initializer->getType() == Context.OverloadTy &&
!S.ResolveAddressOfOverloadedFunction(Initializer, DestType,
  false, dap))
   SetFailed(InitializationSequence::FK_AddressOfOverloadFailed);
-else if (Initializer->getType()->isFunctionType() &&
+else if (Initializer && Initializer->getType()->isFunctionType() &&
  isExprAnUnaddressableFunction(S, Initializer))
   SetFailed(InitializationSequence::FK_AddressOfUnaddressableFunction);
 else


Index: clang/lib/Sema/SemaInit.cpp
===
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -5962,9 +5962,10 @@
 if (Kind.getKind() == InitializationKind::IK_Direct ||
 (Kind.getKind() == InitializationKind::IK_Copy &&
  (Context.hasSameUnqualifiedType(SourceType, DestType) ||
-  S.IsDerivedFrom(Initializer->getBeginLoc(), SourceType, DestType
-  TryConstructorInitialization(S, Entity, Kind, Args,
-   DestType, DestType, *this);
+  (Initializer &&
+   S.IsDerivedFrom(Initializer->getBeginLoc(), SourceType, DestType)
+  TryConstructorInitialization(S, Entity, Kind, Args, DestType, DestType,
+   *this);
 // - Otherwise (i.e., for the remaining copy-initialization cases),
 //   user-defined conversion sequences that can convert from the source
 //   type to the destination type or (when a conversion function is
@@ -6027,8 +6028,8 @@
 bool NeedAtomicConversion = false;
 if (const AtomicType *Atomic = DestType->getAs()) {
   if (Context.hasSameUnqualifiedType(SourceType, Atomic->getValueType()) ||
-  S.IsDerivedFrom(Initializer->getBeginLoc(), SourceType,
-  Atomic->getValueType())) {
+  (Initializer && S.IsDerivedFrom(Initializer->getBeginLoc(),
+  SourceType, Atomic->getValueType())) {
 DestType = Atomic->getValueType();
 NeedAtomicConversion = true;
   }
@@ -6045,7 +6046,7 @@
   //- Otherwise, if the initialization is direct-initialization, the source
   //type is std::nullptr_t, and the destination type is bool, the initial
   //value of 

[PATCH] D146604: [NFC] Fix uninitalized member variable use in ASTReader::ParseTargetOptions()

[Sindhu Chittireddy via Phabricator via cfe-commits]

schittir updated this revision to Diff 507964.
schittir added a comment.

Changed CodeObjectVersion default to COV_None


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

https://reviews.llvm.org/D146604

Files:
  clang/include/clang/Basic/TargetOptions.h


Index: clang/include/clang/Basic/TargetOptions.h
===
--- clang/include/clang/Basic/TargetOptions.h
+++ clang/include/clang/Basic/TargetOptions.h
@@ -45,7 +45,7 @@
   std::string ABI;
 
   /// The EABI version to use
-  llvm::EABI EABIVersion;
+  llvm::EABI EABIVersion = llvm::EABI::Default;
 
   /// If given, the version string of the linker in use.
   std::string LinkerVersion;
@@ -88,7 +88,7 @@
 COV_5 = 500,
   };
   /// \brief Code object version for AMDGPU.
-  CodeObjectVersionKind CodeObjectVersion;
+  CodeObjectVersionKind CodeObjectVersion = CodeObjectVersionKind::COV_None;
 
   // The code model to be used as specified by the user. Corresponds to
   // CodeModel::Model enum defined in include/llvm/Support/CodeGen.h, plus


Index: clang/include/clang/Basic/TargetOptions.h
===
--- clang/include/clang/Basic/TargetOptions.h
+++ clang/include/clang/Basic/TargetOptions.h
@@ -45,7 +45,7 @@
   std::string ABI;
 
   /// The EABI version to use
-  llvm::EABI EABIVersion;
+  llvm::EABI EABIVersion = llvm::EABI::Default;
 
   /// If given, the version string of the linker in use.
   std::string LinkerVersion;
@@ -88,7 +88,7 @@
 COV_5 = 500,
   };
   /// \brief Code object version for AMDGPU.
-  CodeObjectVersionKind CodeObjectVersion;
+  CodeObjectVersionKind CodeObjectVersion = CodeObjectVersionKind::COV_None;
 
   // The code model to be used as specified by the user. Corresponds to
   // CodeModel::Model enum defined in include/llvm/Support/CodeGen.h, plus
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D146847: [NFC] Fix uninitialized member variable use in RVVEmitter::createRVVIntrinsics()

[Sindhu Chittireddy via Phabricator via cfe-commits]

schittir added a comment.

In D146847#4220697 , @jrtc27 wrote:

> None of the other fields are initialised, so blindly initialising it alone to 
> 0 here seems highly suspect. What's the actual case in which it's used whilst 
> uninitialised?

Coverity complains about SR.PolicyBitMask being uninitialized when calling 
SemaRecords->push_back(SR)


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

https://reviews.llvm.org/D146847

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

[PATCH] D146847: [NFC] Fix uninitialized member variable use in RVVEmitter::createRVVIntrinsics()

[Sindhu Chittireddy via Phabricator via cfe-commits]

schittir updated this revision to Diff 508221.
schittir added a comment.

Deleting the member PolicyBitMask


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

https://reviews.llvm.org/D146847

Files:
  clang/utils/TableGen/RISCVVEmitter.cpp


Index: clang/utils/TableGen/RISCVVEmitter.cpp
===
--- clang/utils/TableGen/RISCVVEmitter.cpp
+++ clang/utils/TableGen/RISCVVEmitter.cpp
@@ -57,9 +57,6 @@
   // Suffix of overloaded intrinsic name.
   SmallVector OverloadedSuffix;
 
-  // BitMask for supported policies.
-  uint16_t PolicyBitMask;
-
   // Number of field, large than 1 if it's segment load/store.
   unsigned NF;
 


Index: clang/utils/TableGen/RISCVVEmitter.cpp
===
--- clang/utils/TableGen/RISCVVEmitter.cpp
+++ clang/utils/TableGen/RISCVVEmitter.cpp
@@ -57,9 +57,6 @@
   // Suffix of overloaded intrinsic name.
   SmallVector OverloadedSuffix;
 
-  // BitMask for supported policies.
-  uint16_t PolicyBitMask;
-
   // Number of field, large than 1 if it's segment load/store.
   unsigned NF;
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D146847: [NFC] Fix uninitialized member variable use in RVVEmitter::createRVVIntrinsics()

[Sindhu Chittireddy via Phabricator via cfe-commits]

schittir updated this revision to Diff 508215.

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

https://reviews.llvm.org/D146847

Files:
  clang/utils/TableGen/RISCVVEmitter.cpp


Index: clang/utils/TableGen/RISCVVEmitter.cpp
===
--- clang/utils/TableGen/RISCVVEmitter.cpp
+++ clang/utils/TableGen/RISCVVEmitter.cpp
@@ -641,6 +641,7 @@
   SR.RequiredExtensions |= RequireExt;
 }
 
+SR.PolicyBitMask = 0;
 SR.NF = NF;
 SR.HasMasked = HasMasked;
 SR.HasVL = HasVL;


Index: clang/utils/TableGen/RISCVVEmitter.cpp
===
--- clang/utils/TableGen/RISCVVEmitter.cpp
+++ clang/utils/TableGen/RISCVVEmitter.cpp
@@ -641,6 +641,7 @@
   SR.RequiredExtensions |= RequireExt;
 }
 
+SR.PolicyBitMask = 0;
 SR.NF = NF;
 SR.HasMasked = HasMasked;
 SR.HasVL = HasVL;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D146847: [NFC] Fix uninitialized member variable use in RVVEmitter::createRVVIntrinsics()

[Sindhu Chittireddy via Phabricator via cfe-commits]

schittir added a comment.

In D146847#4220717 , @jrtc27 wrote:

> The field should just be deleted. D126742  
> added it without any uses, and I don't know what the original intent was. 
> Maybe it was used before the bitfields were added and it stuck around rather 
> than being GC'ed. I don't know why Coverity would care about this though when 
> it's never read from, there's nothing wrong with that, unless the warning is 
> in fact that the field is entirely unreferenced?

Yea, that would make sense. I am up for deleting the field. Coverity does 
indeed complaining about uninitialized use of the SR.PolicyBitMask when calling 
push_back - surprisingly nothing about the field being unreferenced.


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

https://reviews.llvm.org/D146847

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

[PATCH] D146847: [NFC] Fix uninitialized member variable use in RVVEmitter::createRVVIntrinsics()

[Sindhu Chittireddy via Phabricator via cfe-commits]

schittir created this revision.
schittir added reviewers: tahonermann, shafik, aaron.ballman.
Herald added subscribers: luke, frasercrmck, luismarques, apazos, 
sameer.abuasal, s.egerton, Jim, jocewei, PkmX, the_o, brucehoult, 
MartinMosbeck, rogfer01, edward-jones, zzheng, jrtc27, niosHD, sabuasal, 
simoncook, johnrusso, rbar, asb.
Herald added a project: All.
schittir requested review of this revision.
Herald added subscribers: pcwang-thead, MaskRay.
Herald added a project: clang.

Found by Coverity static analysis tool.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D146847

Files:
  clang/utils/TableGen/RISCVVEmitter.cpp


Index: clang/utils/TableGen/RISCVVEmitter.cpp
===
--- clang/utils/TableGen/RISCVVEmitter.cpp
+++ clang/utils/TableGen/RISCVVEmitter.cpp
@@ -58,7 +58,7 @@
   SmallVector OverloadedSuffix;
 
   // BitMask for supported policies.
-  uint16_t PolicyBitMask;
+  uint16_t PolicyBitMask = 0;
 
   // Number of field, large than 1 if it's segment load/store.
   unsigned NF;


Index: clang/utils/TableGen/RISCVVEmitter.cpp
===
--- clang/utils/TableGen/RISCVVEmitter.cpp
+++ clang/utils/TableGen/RISCVVEmitter.cpp
@@ -58,7 +58,7 @@
   SmallVector OverloadedSuffix;
 
   // BitMask for supported policies.
-  uint16_t PolicyBitMask;
+  uint16_t PolicyBitMask = 0;
 
   // Number of field, large than 1 if it's segment load/store.
   unsigned NF;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D146604: [NFC] Fix uninitalized member variable use in ASTReader::ParseTargetOptions()

[Sindhu Chittireddy via Phabricator via cfe-commits]

schittir created this revision.
schittir added reviewers: tahonermann, shafik.
Herald added a project: All.
schittir requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Found by Coverity static analysis tool.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D146604

Files:
  clang/lib/Serialization/ASTReader.cpp


Index: clang/lib/Serialization/ASTReader.cpp
===
--- clang/lib/Serialization/ASTReader.cpp
+++ clang/lib/Serialization/ASTReader.cpp
@@ -5887,6 +5887,9 @@
   TargetOpts.CPU = ReadString(Record, Idx);
   TargetOpts.TuneCPU = ReadString(Record, Idx);
   TargetOpts.ABI = ReadString(Record, Idx);
+  TargetOpts.EABIVersion = llvm::EABI::Default;
+  // Initialize CodeObjectVersion with default i.e., 4
+  TargetOpts.CodeObjectVersion = TargetOptions::CodeObjectVersionKind::COV_4;
   for (unsigned N = Record[Idx++]; N; --N) {
 TargetOpts.FeaturesAsWritten.push_back(ReadString(Record, Idx));
   }


Index: clang/lib/Serialization/ASTReader.cpp
===
--- clang/lib/Serialization/ASTReader.cpp
+++ clang/lib/Serialization/ASTReader.cpp
@@ -5887,6 +5887,9 @@
   TargetOpts.CPU = ReadString(Record, Idx);
   TargetOpts.TuneCPU = ReadString(Record, Idx);
   TargetOpts.ABI = ReadString(Record, Idx);
+  TargetOpts.EABIVersion = llvm::EABI::Default;
+  // Initialize CodeObjectVersion with default i.e., 4
+  TargetOpts.CodeObjectVersion = TargetOptions::CodeObjectVersionKind::COV_4;
   for (unsigned N = Record[Idx++]; N; --N) {
 TargetOpts.FeaturesAsWritten.push_back(ReadString(Record, Idx));
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D146604: [NFC] Fix uninitalized member variable use in ASTReader::ParseTargetOptions()

[Sindhu Chittireddy via Phabricator via cfe-commits]

schittir updated this revision to Diff 507425.

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

https://reviews.llvm.org/D146604

Files:
  clang/include/clang/Basic/TargetOptions.h


Index: clang/include/clang/Basic/TargetOptions.h
===
--- clang/include/clang/Basic/TargetOptions.h
+++ clang/include/clang/Basic/TargetOptions.h
@@ -45,7 +45,7 @@
   std::string ABI;
 
   /// The EABI version to use
-  llvm::EABI EABIVersion;
+  llvm::EABI EABIVersion = llvm::EABI::Default;
 
   /// If given, the version string of the linker in use.
   std::string LinkerVersion;
@@ -88,7 +88,7 @@
 COV_5 = 500,
   };
   /// \brief Code object version for AMDGPU.
-  CodeObjectVersionKind CodeObjectVersion;
+  CodeObjectVersionKind CodeObjectVersion = CodeObjectVersionKind::COV_4;
 
   // The code model to be used as specified by the user. Corresponds to
   // CodeModel::Model enum defined in include/llvm/Support/CodeGen.h, plus


Index: clang/include/clang/Basic/TargetOptions.h
===
--- clang/include/clang/Basic/TargetOptions.h
+++ clang/include/clang/Basic/TargetOptions.h
@@ -45,7 +45,7 @@
   std::string ABI;
 
   /// The EABI version to use
-  llvm::EABI EABIVersion;
+  llvm::EABI EABIVersion = llvm::EABI::Default;
 
   /// If given, the version string of the linker in use.
   std::string LinkerVersion;
@@ -88,7 +88,7 @@
 COV_5 = 500,
   };
   /// \brief Code object version for AMDGPU.
-  CodeObjectVersionKind CodeObjectVersion;
+  CodeObjectVersionKind CodeObjectVersion = CodeObjectVersionKind::COV_4;
 
   // The code model to be used as specified by the user. Corresponds to
   // CodeModel::Model enum defined in include/llvm/Support/CodeGen.h, plus
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D146604: [NFC] Fix uninitalized member variable use in ASTReader::ParseTargetOptions()

[Sindhu Chittireddy via Phabricator via cfe-commits]

schittir added inline comments.



Comment at: clang/include/clang/Basic/TargetOptions.h:91
   /// \brief Code object version for AMDGPU.
-  CodeObjectVersionKind CodeObjectVersion;
+  CodeObjectVersionKind CodeObjectVersion = CodeObjectVersionKind::COV_4;
 

aaron.ballman wrote:
> tahonermann wrote:
> > I see that `COV_4` is the default as specified in 
> > `clang/include/clang/Driver/Options.td`, but I wonder if we want to 
> > duplicate that value here. I'm a bit more inclined to default to `COV_None` 
> > so that we're not trying to maintain the same default in multiple places. I 
> > don't know that it matters though.
> > 
> > Perhaps it would make sense to add a "default" option value as is done for 
> > EABI? I'm not sure.
> I think `none` makes the most sense to me. This is set via a CC1 option, and 
> the driver doesn't unconditionally set it 
> (https://github.com/llvm/llvm-project/blob/main/clang/lib/Driver/ToolChains/Clang.cpp#L1073),
>  so I think `none` is safer than assuming `CodeObjectVersion` is meaningful 
> when it's not set by -cc1. WDYT?
I agree, and that's why I made the change to none.  @tahonermann


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

https://reviews.llvm.org/D146604

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

[PATCH] D146847: [NFC] Fix uninitialized member variable use in RVVEmitter::createRVVIntrinsics()

[Sindhu Chittireddy via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG9972c9a89347: [NFC] Remove unused member variable 
`PolicyBitMask` in SemaRecord (authored by schittir).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146847

Files:
  clang/utils/TableGen/RISCVVEmitter.cpp


Index: clang/utils/TableGen/RISCVVEmitter.cpp
===
--- clang/utils/TableGen/RISCVVEmitter.cpp
+++ clang/utils/TableGen/RISCVVEmitter.cpp
@@ -57,9 +57,6 @@
   // Suffix of overloaded intrinsic name.
   SmallVector OverloadedSuffix;
 
-  // BitMask for supported policies.
-  uint16_t PolicyBitMask;
-
   // Number of field, large than 1 if it's segment load/store.
   unsigned NF;
 


Index: clang/utils/TableGen/RISCVVEmitter.cpp
===
--- clang/utils/TableGen/RISCVVEmitter.cpp
+++ clang/utils/TableGen/RISCVVEmitter.cpp
@@ -57,9 +57,6 @@
   // Suffix of overloaded intrinsic name.
   SmallVector OverloadedSuffix;
 
-  // BitMask for supported policies.
-  uint16_t PolicyBitMask;
-
   // Number of field, large than 1 if it's segment load/store.
   unsigned NF;
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D146604: [NFC] Fix uninitalized member variable use in ASTReader::ParseTargetOptions()

[Sindhu Chittireddy via Phabricator via cfe-commits]

schittir added a comment.

Thank you, @aaron.ballman and @tahonermann  for your comments!
This was committed to https://github.com/llvm/llvm-project.git with hash 
6f2a865d2f6bc426a61939a0a1acfcb25d5c1a18 



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

https://reviews.llvm.org/D146604

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

[PATCH] D152689: [NFC] Remove dead conditionals

[Sindhu Chittireddy via Phabricator via cfe-commits]

schittir updated this revision to Diff 530702.
schittir added a comment.

Fix clang-format issue in SemaOverload.cpp


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

https://reviews.llvm.org/D152689

Files:
  clang/lib/CodeGen/CGObjCMac.cpp
  clang/lib/Sema/SemaOverload.cpp
  llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp


Index: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
===
--- llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
+++ llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
@@ -2120,8 +2120,7 @@
 // Avoid incrementing past stop since it could overflow.
 Value *CountIfTwo = Builder.CreateAdd(
 Builder.CreateUDiv(Builder.CreateSub(Span, One), Incr), One);
-Value *OneCmp = Builder.CreateICmp(
-InclusiveStop ? CmpInst::ICMP_ULT : CmpInst::ICMP_ULE, Span, Incr);
+Value *OneCmp = Builder.CreateICmp(CmpInst::ICMP_ULE, Span, Incr);
 CountIfLooping = Builder.CreateSelect(OneCmp, One, CountIfTwo);
   }
   Value *TripCount = Builder.CreateSelect(ZeroCmp, Zero, CountIfLooping,
Index: clang/lib/Sema/SemaOverload.cpp
===
--- clang/lib/Sema/SemaOverload.cpp
+++ clang/lib/Sema/SemaOverload.cpp
@@ -10865,8 +10865,8 @@
   if (FromExpr && isa(FromExpr)) {
 S.Diag(Fn->getLocation(), diag::note_ovl_candidate_bad_list_argument)
 << (unsigned)FnKindPair.first << (unsigned)FnKindPair.second << FnDesc
-<< (FromExpr ? FromExpr->getSourceRange() : SourceRange()) << FromTy
-<< ToTy << (unsigned)isObjectArgument << I + 1
+<< FromExpr->getSourceRange() << FromTy << ToTy
+<< (unsigned)isObjectArgument << I + 1
 << (Conv.Bad.Kind == BadConversionSequence::too_few_initializers ? 1
 : Conv.Bad.Kind == BadConversionSequence::too_many_initializers
 ? 2
Index: clang/lib/CodeGen/CGObjCMac.cpp
===
--- clang/lib/CodeGen/CGObjCMac.cpp
+++ clang/lib/CodeGen/CGObjCMac.cpp
@@ -3809,15 +3809,9 @@
   ivarList.fillPlaceholderWithInt(countSlot, ObjCTypes.IntTy, count);
 
   llvm::GlobalVariable *GV;
-  if (ForClass)
-GV =
-CreateMetadataVar("OBJC_CLASS_VARIABLES_" + ID->getName(), ivarList,
-  "__OBJC,__class_vars,regular,no_dead_strip",
-  CGM.getPointerAlign(), true);
-  else
-GV = CreateMetadataVar("OBJC_INSTANCE_VARIABLES_" + ID->getName(), 
ivarList,
-   "__OBJC,__instance_vars,regular,no_dead_strip",
-   CGM.getPointerAlign(), true);
+  GV = CreateMetadataVar("OBJC_INSTANCE_VARIABLES_" + ID->getName(), ivarList,
+ "__OBJC,__instance_vars,regular,no_dead_strip",
+ CGM.getPointerAlign(), true);
   return llvm::ConstantExpr::getBitCast(GV, ObjCTypes.IvarListPtrTy);
 }
 


Index: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
===
--- llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
+++ llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
@@ -2120,8 +2120,7 @@
 // Avoid incrementing past stop since it could overflow.
 Value *CountIfTwo = Builder.CreateAdd(
 Builder.CreateUDiv(Builder.CreateSub(Span, One), Incr), One);
-Value *OneCmp = Builder.CreateICmp(
-InclusiveStop ? CmpInst::ICMP_ULT : CmpInst::ICMP_ULE, Span, Incr);
+Value *OneCmp = Builder.CreateICmp(CmpInst::ICMP_ULE, Span, Incr);
 CountIfLooping = Builder.CreateSelect(OneCmp, One, CountIfTwo);
   }
   Value *TripCount = Builder.CreateSelect(ZeroCmp, Zero, CountIfLooping,
Index: clang/lib/Sema/SemaOverload.cpp
===
--- clang/lib/Sema/SemaOverload.cpp
+++ clang/lib/Sema/SemaOverload.cpp
@@ -10865,8 +10865,8 @@
   if (FromExpr && isa(FromExpr)) {
 S.Diag(Fn->getLocation(), diag::note_ovl_candidate_bad_list_argument)
 << (unsigned)FnKindPair.first << (unsigned)FnKindPair.second << FnDesc
-<< (FromExpr ? FromExpr->getSourceRange() : SourceRange()) << FromTy
-<< ToTy << (unsigned)isObjectArgument << I + 1
+<< FromExpr->getSourceRange() << FromTy << ToTy
+<< (unsigned)isObjectArgument << I + 1
 << (Conv.Bad.Kind == BadConversionSequence::too_few_initializers ? 1
 : Conv.Bad.Kind == BadConversionSequence::too_many_initializers
 ? 2
Index: clang/lib/CodeGen/CGObjCMac.cpp
===
--- clang/lib/CodeGen/CGObjCMac.cpp
+++ clang/lib/CodeGen/CGObjCMac.cpp
@@ -3809,15 +3809,9 @@
   ivarList.fillPlaceholderWithInt(countSlot, ObjCTypes.IntTy, count);
 
   llvm::GlobalVariable *GV;
-  if (ForClass)
-GV =
-CreateMetadataVar("OBJC_CLASS_VARIABLES_" + ID->getName(), ivarList,
-  "__OBJC,__class_vars,regular,no_dead_strip",
-   

[PATCH] D152689: [NFC] Remove dead conditionals

[Sindhu Chittireddy via Phabricator via cfe-commits]

schittir added a comment.

Thank you for the review, @aaron.ballman and @rjmccall 
The latest patch fixes clang-format issue causing build failure. Could you 
please take a look at this again? Thanks!


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

https://reviews.llvm.org/D152689

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

[PATCH] D152977: [NFC] Fix potential dereferencing of null return value.

[Sindhu Chittireddy via Phabricator via cfe-commits]

schittir created this revision.
schittir added reviewers: aaron.ballman, tahonermann, erichkeane.
Herald added subscribers: steakhal, martong.
Herald added a reviewer: NoQ.
Herald added a project: All.
schittir requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Replace getAs with castAs and add assert if needed.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D152977

Files:
  clang/lib/AST/ASTContext.cpp
  clang/lib/Frontend/FrontendActions.cpp
  clang/lib/Sema/SemaType.cpp
  clang/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp
  clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp


Index: clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp
@@ -291,8 +291,8 @@
   // Existence in DestroyRetVal ensures existence in LockMap.
   // Existence in Destroyed also ensures that the lock state for lockR is 
either
   // UntouchedAndPossiblyDestroyed or UnlockedAndPossiblyDestroyed.
-  assert(lstate->isUntouchedAndPossiblyDestroyed() ||
- lstate->isUnlockedAndPossiblyDestroyed());
+  assert(lstate && (lstate->isUntouchedAndPossiblyDestroyed() ||
+lstate->isUnlockedAndPossiblyDestroyed()));
 
   ConstraintManager  = state->getConstraintManager();
   ConditionTruthVal retZero = CMgr.isNull(state, *sym);
Index: clang/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp
+++ clang/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp
@@ -321,7 +321,9 @@
   if (!IvarRegion)
 return nullptr;
 
-  return IvarRegion->getSymbolicBase()->getSymbol();
+  const SymbolicRegion *SR = IvarRegion->getSymbolicBase();
+  assert(SR && "Symbolic base should not be nullptr");
+  return SR->getSymbol();
 }
 
 /// If we are in -dealloc or -dealloc is on the stack, handle the call if it is
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -2790,7 +2790,7 @@
 
   // Only support _BitInt elements with byte-sized power of 2 NumBits.
   if (T->isBitIntType()) {
-unsigned NumBits = T->getAs()->getNumBits();
+unsigned NumBits = T->castAs()->getNumBits();
 if (!llvm::isPowerOf2_32(NumBits) || NumBits < 8) {
   Diag(AttrLoc, diag::err_attribute_invalid_bitint_vector_type)
   << (NumBits < 8);
Index: clang/lib/Frontend/FrontendActions.cpp
===
--- clang/lib/Frontend/FrontendActions.cpp
+++ clang/lib/Frontend/FrontendActions.cpp
@@ -458,6 +458,8 @@
   return;
 }
 
+assert(NamedCtx, "NamedCtx cannot be null");
+
 if (const auto *Decl = dyn_cast(NamedTemplate)) {
   OS << "unnamed function parameter " << Decl->getFunctionScopeIndex()
  << " ";
Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -4141,8 +4141,8 @@
   assert(vecType->isBuiltinType() || vecType->isDependentType() ||
  (vecType->isBitIntType() &&
   // Only support _BitInt elements with byte-sized power of 2 NumBits.
-  llvm::isPowerOf2_32(vecType->getAs()->getNumBits()) &&
-  vecType->getAs()->getNumBits() >= 8));
+  llvm::isPowerOf2_32(vecType->castAs()->getNumBits()) &&
+  vecType->castAs()->getNumBits() >= 8));
 
   // Check if we've already instantiated a vector of this type.
   llvm::FoldingSetNodeID ID;


Index: clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp
@@ -291,8 +291,8 @@
   // Existence in DestroyRetVal ensures existence in LockMap.
   // Existence in Destroyed also ensures that the lock state for lockR is either
   // UntouchedAndPossiblyDestroyed or UnlockedAndPossiblyDestroyed.
-  assert(lstate->isUntouchedAndPossiblyDestroyed() ||
- lstate->isUnlockedAndPossiblyDestroyed());
+  assert(lstate && (lstate->isUntouchedAndPossiblyDestroyed() ||
+lstate->isUnlockedAndPossiblyDestroyed()));
 
   ConstraintManager  = state->getConstraintManager();
   ConditionTruthVal retZero = CMgr.isNull(state, *sym);
Index: clang/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp
+++ clang/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp
@@ -321,7 +321,9 @@
   if (!IvarRegion)
 return nullptr;
 
-  return 

[PATCH] D153892: [NFC] Initialize class member pointers to nullptr.

[Sindhu Chittireddy via Phabricator via cfe-commits]

schittir updated this revision to Diff 538851.
schittir marked an inline comment as done.
schittir added a comment.

Fix format per comments - indent class members and avoid indenting the class 
definition line.


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

https://reviews.llvm.org/D153892

Files:
  clang/lib/ARCMigrate/TransProperties.cpp
  clang/lib/AST/ExprConstant.cpp
  clang/lib/Frontend/ASTConsumers.cpp
  clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp


Index: clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
@@ -29,7 +29,7 @@
 class StackAddrEscapeChecker
 : public Checker,
  check::EndFunction> {
-  mutable IdentifierInfo *dispatch_semaphore_tII;
+  mutable IdentifierInfo *dispatch_semaphore_tII = nullptr;
   mutable std::unique_ptr BT_stackleak;
   mutable std::unique_ptr BT_returnstack;
   mutable std::unique_ptr BT_capturedstackasync;
Index: clang/lib/Frontend/ASTConsumers.cpp
===
--- clang/lib/Frontend/ASTConsumers.cpp
+++ clang/lib/Frontend/ASTConsumers.cpp
@@ -183,21 +183,20 @@
 /// ASTViewer - AST Visualization
 
 namespace {
-  class ASTViewer : public ASTConsumer {
-ASTContext *Context;
-  public:
-void Initialize(ASTContext ) override {
-  this->Context = 
-}
+class ASTViewer : public ASTConsumer {
+  ASTContext *Context = nullptr;
 
-bool HandleTopLevelDecl(DeclGroupRef D) override {
-  for (DeclGroupRef::iterator I = D.begin(), E = D.end(); I != E; ++I)
-HandleTopLevelSingleDecl(*I);
-  return true;
-}
+public:
+  void Initialize(ASTContext ) override { this->Context =  }
 
-void HandleTopLevelSingleDecl(Decl *D);
-  };
+  bool HandleTopLevelDecl(DeclGroupRef D) override {
+for (DeclGroupRef::iterator I = D.begin(), E = D.end(); I != E; ++I)
+  HandleTopLevelSingleDecl(*I);
+return true;
+  }
+
+  void HandleTopLevelSingleDecl(Decl *D);
+};
 }
 
 void ASTViewer::HandleTopLevelSingleDecl(Decl *D) {
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -580,7 +580,7 @@
 /// LambdaCaptureFields - Mapping from captured variables/this to
 /// corresponding data members in the closure class.
 llvm::DenseMap LambdaCaptureFields;
-FieldDecl *LambdaThisCaptureField;
+FieldDecl *LambdaThisCaptureField = nullptr;
 
 CallStackFrame(EvalInfo , SourceLocation CallLoc,
const FunctionDecl *Callee, const LValue *This,
Index: clang/lib/ARCMigrate/TransProperties.cpp
===
--- clang/lib/ARCMigrate/TransProperties.cpp
+++ clang/lib/ARCMigrate/TransProperties.cpp
@@ -45,7 +45,7 @@
 class PropertiesRewriter {
   MigrationContext 
   MigrationPass 
-  ObjCImplementationDecl *CurImplD;
+  ObjCImplementationDecl *CurImplD = nullptr;
 
   enum PropActionKind {
 PropAction_None,


Index: clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
@@ -29,7 +29,7 @@
 class StackAddrEscapeChecker
 : public Checker,
  check::EndFunction> {
-  mutable IdentifierInfo *dispatch_semaphore_tII;
+  mutable IdentifierInfo *dispatch_semaphore_tII = nullptr;
   mutable std::unique_ptr BT_stackleak;
   mutable std::unique_ptr BT_returnstack;
   mutable std::unique_ptr BT_capturedstackasync;
Index: clang/lib/Frontend/ASTConsumers.cpp
===
--- clang/lib/Frontend/ASTConsumers.cpp
+++ clang/lib/Frontend/ASTConsumers.cpp
@@ -183,21 +183,20 @@
 /// ASTViewer - AST Visualization
 
 namespace {
-  class ASTViewer : public ASTConsumer {
-ASTContext *Context;
-  public:
-void Initialize(ASTContext ) override {
-  this->Context = 
-}
+class ASTViewer : public ASTConsumer {
+  ASTContext *Context = nullptr;
 
-bool HandleTopLevelDecl(DeclGroupRef D) override {
-  for (DeclGroupRef::iterator I = D.begin(), E = D.end(); I != E; ++I)
-HandleTopLevelSingleDecl(*I);
-  return true;
-}
+public:
+  void Initialize(ASTContext ) override { this->Context =  }
 
-void HandleTopLevelSingleDecl(Decl *D);
-  };
+  bool HandleTopLevelDecl(DeclGroupRef D) override {
+for (DeclGroupRef::iterator I = D.begin(), E = D.end(); I != E; ++I)
+  HandleTopLevelSingleDecl(*I);
+return true;
+  }
+
+  void HandleTopLevelSingleDecl(Decl *D);
+};
 }
 
 void ASTViewer::HandleTopLevelSingleDecl(Decl *D) {
Index: 

[PATCH] D139148: Fix nullptr dereference found by Coverity static analysis tool

[Sindhu Chittireddy via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG472232a80d03: [NFC] Fix potential dereferencing of nullptr 
(authored by schittir).
Herald added a subscriber: wangpc.

Changed prior to commit:
  https://reviews.llvm.org/D139148?vs=481989=539705#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139148

Files:
  clang/lib/Sema/SemaInit.cpp


Index: clang/lib/Sema/SemaInit.cpp
===
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -6336,6 +6336,7 @@
   // We're at the end of the line for C: it's either a write-back conversion
   // or it's a C assignment. There's no need to check anything else.
   if (!S.getLangOpts().CPlusPlus) {
+assert(Initializer && "Initializer must be non-null");
 // If allowed, check whether this is an Objective-C writeback conversion.
 if (allowObjCWritebackConversion &&
 tryObjCWritebackConversion(S, *this, Entity, Initializer)) {
@@ -6362,7 +6363,8 @@
 if (Kind.getKind() == InitializationKind::IK_Direct ||
 (Kind.getKind() == InitializationKind::IK_Copy &&
  (Context.hasSameUnqualifiedType(SourceType, DestType) ||
-  S.IsDerivedFrom(Initializer->getBeginLoc(), SourceType, DestType 
{
+  (Initializer && S.IsDerivedFrom(Initializer->getBeginLoc(),
+  SourceType, DestType) {
   TryConstructorInitialization(S, Entity, Kind, Args, DestType, DestType,
*this);
 
@@ -6406,6 +6408,7 @@
   //   function is used) to a derived class thereof are enumerated as
   //   described in 13.3.1.4, and the best one is chosen through
   //   overload resolution (13.3).
+  assert(Initializer && "Initializer must be non-null");
   TryUserDefinedConversion(S, DestType, Kind, Initializer, *this,
TopLevelOfInitList);
 }
@@ -6457,6 +6460,7 @@
   //- Otherwise, if the source type is a (possibly cv-qualified) class
   //  type, conversion functions are considered.
   if (!SourceType.isNull() && SourceType->isRecordType()) {
+assert(Initializer && "Initializer must be non-null");
 // For a conversion to _Atomic(T) from either T or a class type derived
 // from T, initialize the T object then convert to _Atomic type.
 bool NeedAtomicConversion = false;


Index: clang/lib/Sema/SemaInit.cpp
===
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -6336,6 +6336,7 @@
   // We're at the end of the line for C: it's either a write-back conversion
   // or it's a C assignment. There's no need to check anything else.
   if (!S.getLangOpts().CPlusPlus) {
+assert(Initializer && "Initializer must be non-null");
 // If allowed, check whether this is an Objective-C writeback conversion.
 if (allowObjCWritebackConversion &&
 tryObjCWritebackConversion(S, *this, Entity, Initializer)) {
@@ -6362,7 +6363,8 @@
 if (Kind.getKind() == InitializationKind::IK_Direct ||
 (Kind.getKind() == InitializationKind::IK_Copy &&
  (Context.hasSameUnqualifiedType(SourceType, DestType) ||
-  S.IsDerivedFrom(Initializer->getBeginLoc(), SourceType, DestType {
+  (Initializer && S.IsDerivedFrom(Initializer->getBeginLoc(),
+  SourceType, DestType) {
   TryConstructorInitialization(S, Entity, Kind, Args, DestType, DestType,
*this);
 
@@ -6406,6 +6408,7 @@
   //   function is used) to a derived class thereof are enumerated as
   //   described in 13.3.1.4, and the best one is chosen through
   //   overload resolution (13.3).
+  assert(Initializer && "Initializer must be non-null");
   TryUserDefinedConversion(S, DestType, Kind, Initializer, *this,
TopLevelOfInitList);
 }
@@ -6457,6 +6460,7 @@
   //- Otherwise, if the source type is a (possibly cv-qualified) class
   //  type, conversion functions are considered.
   if (!SourceType.isNull() && SourceType->isRecordType()) {
+assert(Initializer && "Initializer must be non-null");
 // For a conversion to _Atomic(T) from either T or a class type derived
 // from T, initialize the T object then convert to _Atomic type.
 bool NeedAtomicConversion = false;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D153892: [NFC] Initialize class member pointers to nullptr.

[Sindhu Chittireddy via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGe9877eca408e: [NFC] Initialize class member pointers to 
nullptr. (authored by schittir).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153892

Files:
  clang/lib/ARCMigrate/TransProperties.cpp
  clang/lib/AST/ExprConstant.cpp
  clang/lib/Frontend/ASTConsumers.cpp
  clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp


Index: clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
@@ -29,7 +29,7 @@
 class StackAddrEscapeChecker
 : public Checker,
  check::EndFunction> {
-  mutable IdentifierInfo *dispatch_semaphore_tII;
+  mutable IdentifierInfo *dispatch_semaphore_tII = nullptr;
   mutable std::unique_ptr BT_stackleak;
   mutable std::unique_ptr BT_returnstack;
   mutable std::unique_ptr BT_capturedstackasync;
Index: clang/lib/Frontend/ASTConsumers.cpp
===
--- clang/lib/Frontend/ASTConsumers.cpp
+++ clang/lib/Frontend/ASTConsumers.cpp
@@ -183,21 +183,20 @@
 /// ASTViewer - AST Visualization
 
 namespace {
-  class ASTViewer : public ASTConsumer {
-ASTContext *Context;
-  public:
-void Initialize(ASTContext ) override {
-  this->Context = 
-}
+class ASTViewer : public ASTConsumer {
+  ASTContext *Context = nullptr;
 
-bool HandleTopLevelDecl(DeclGroupRef D) override {
-  for (DeclGroupRef::iterator I = D.begin(), E = D.end(); I != E; ++I)
-HandleTopLevelSingleDecl(*I);
-  return true;
-}
+public:
+  void Initialize(ASTContext ) override { this->Context =  }
 
-void HandleTopLevelSingleDecl(Decl *D);
-  };
+  bool HandleTopLevelDecl(DeclGroupRef D) override {
+for (DeclGroupRef::iterator I = D.begin(), E = D.end(); I != E; ++I)
+  HandleTopLevelSingleDecl(*I);
+return true;
+  }
+
+  void HandleTopLevelSingleDecl(Decl *D);
+};
 }
 
 void ASTViewer::HandleTopLevelSingleDecl(Decl *D) {
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -583,7 +583,7 @@
 /// LambdaCaptureFields - Mapping from captured variables/this to
 /// corresponding data members in the closure class.
 llvm::DenseMap LambdaCaptureFields;
-FieldDecl *LambdaThisCaptureField;
+FieldDecl *LambdaThisCaptureField = nullptr;
 
 CallStackFrame(EvalInfo , SourceLocation CallLoc,
const FunctionDecl *Callee, const LValue *This,
Index: clang/lib/ARCMigrate/TransProperties.cpp
===
--- clang/lib/ARCMigrate/TransProperties.cpp
+++ clang/lib/ARCMigrate/TransProperties.cpp
@@ -45,7 +45,7 @@
 class PropertiesRewriter {
   MigrationContext 
   MigrationPass 
-  ObjCImplementationDecl *CurImplD;
+  ObjCImplementationDecl *CurImplD = nullptr;
 
   enum PropActionKind {
 PropAction_None,


Index: clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
@@ -29,7 +29,7 @@
 class StackAddrEscapeChecker
 : public Checker,
  check::EndFunction> {
-  mutable IdentifierInfo *dispatch_semaphore_tII;
+  mutable IdentifierInfo *dispatch_semaphore_tII = nullptr;
   mutable std::unique_ptr BT_stackleak;
   mutable std::unique_ptr BT_returnstack;
   mutable std::unique_ptr BT_capturedstackasync;
Index: clang/lib/Frontend/ASTConsumers.cpp
===
--- clang/lib/Frontend/ASTConsumers.cpp
+++ clang/lib/Frontend/ASTConsumers.cpp
@@ -183,21 +183,20 @@
 /// ASTViewer - AST Visualization
 
 namespace {
-  class ASTViewer : public ASTConsumer {
-ASTContext *Context;
-  public:
-void Initialize(ASTContext ) override {
-  this->Context = 
-}
+class ASTViewer : public ASTConsumer {
+  ASTContext *Context = nullptr;
 
-bool HandleTopLevelDecl(DeclGroupRef D) override {
-  for (DeclGroupRef::iterator I = D.begin(), E = D.end(); I != E; ++I)
-HandleTopLevelSingleDecl(*I);
-  return true;
-}
+public:
+  void Initialize(ASTContext ) override { this->Context =  }
 
-void HandleTopLevelSingleDecl(Decl *D);
-  };
+  bool HandleTopLevelDecl(DeclGroupRef D) override {
+for (DeclGroupRef::iterator I = D.begin(), E = D.end(); I != E; ++I)
+  HandleTopLevelSingleDecl(*I);
+return true;
+  }
+
+  void HandleTopLevelSingleDecl(Decl *D);
+};
 }
 
 

[PATCH] D139148: Fix nullptr dereference found by Coverity static analysis tool

[Sindhu Chittireddy via Phabricator via cfe-commits]

schittir added a comment.

Landing this one fell through the cracks, hence the delay between accepting and 
committing the patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139148

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

[PATCH] D153589: [NFC] Initialize pointer fields and remove a needless null check.

[Sindhu Chittireddy via Phabricator via cfe-commits]

schittir closed this revision.
schittir added a comment.

In D153589#4493187 , 
@HazardyKnusperkeks wrote:

> Can you mark this change as closed?
> If you'd put "Differential Revision: https://reviews.llvm.org/D153589; in 
> your commit message this would have happened automatically.

I put the link to the review but worded the commit message differently. Thanks, 
I will close this.


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

https://reviews.llvm.org/D153589

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

[PATCH] D155776: [NFC] Add checks for self-assignment.

[Sindhu Chittireddy via Phabricator via cfe-commits]

schittir updated this revision to Diff 542333.
schittir added a comment.

Please consider these changes for review.


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

https://reviews.llvm.org/D155776

Files:
  clang/lib/AST/APValue.cpp
  clang/lib/CodeGen/CGDebugInfo.h
  clang/lib/Interpreter/Value.cpp


Index: clang/lib/Interpreter/Value.cpp
===
--- clang/lib/Interpreter/Value.cpp
+++ clang/lib/Interpreter/Value.cpp
@@ -201,16 +201,17 @@
 }
 
 Value ::operator=(Value &) noexcept {
-  if (IsManuallyAlloc)
-ValueStorage::getFromPayload(getPtr())->Release();
+  if (this != RHS) {
+if (IsManuallyAlloc)
+  ValueStorage::getFromPayload(getPtr())->Release();
 
-  Interp = std::exchange(RHS.Interp, nullptr);
-  OpaqueType = std::exchange(RHS.OpaqueType, nullptr);
-  ValueKind = std::exchange(RHS.ValueKind, K_Unspecified);
-  IsManuallyAlloc = std::exchange(RHS.IsManuallyAlloc, false);
-
-  Data = RHS.Data;
+Interp = std::exchange(RHS.Interp, nullptr);
+OpaqueType = std::exchange(RHS.OpaqueType, nullptr);
+ValueKind = std::exchange(RHS.ValueKind, K_Unspecified);
+IsManuallyAlloc = std::exchange(RHS.IsManuallyAlloc, false);
 
+Data = RHS.Data;
+  }
   return *this;
 }
 
Index: clang/lib/CodeGen/CGDebugInfo.h
===
--- clang/lib/CodeGen/CGDebugInfo.h
+++ clang/lib/CodeGen/CGDebugInfo.h
@@ -832,8 +832,10 @@
 
   // Define copy assignment operator.
   ApplyDebugLocation =(ApplyDebugLocation &) {
-CGF = Other.CGF;
-Other.CGF = nullptr;
+if (this != Other) {
+  CGF = Other.CGF;
+  Other.CGF = nullptr;
+}
 return *this;
   }
 
Index: clang/lib/AST/APValue.cpp
===
--- clang/lib/AST/APValue.cpp
+++ clang/lib/AST/APValue.cpp
@@ -390,11 +390,13 @@
 }
 
 APValue ::operator=(APValue &) {
-  if (Kind != None && Kind != Indeterminate)
-DestroyDataAndMakeUninit();
-  Kind = RHS.Kind;
-  Data = RHS.Data;
-  RHS.Kind = None;
+  if (this != RHS) {
+if (Kind != None && Kind != Indeterminate)
+  DestroyDataAndMakeUninit();
+Kind = RHS.Kind;
+Data = RHS.Data;
+RHS.Kind = None;
+  }
   return *this;
 }
 


Index: clang/lib/Interpreter/Value.cpp
===
--- clang/lib/Interpreter/Value.cpp
+++ clang/lib/Interpreter/Value.cpp
@@ -201,16 +201,17 @@
 }
 
 Value ::operator=(Value &) noexcept {
-  if (IsManuallyAlloc)
-ValueStorage::getFromPayload(getPtr())->Release();
+  if (this != RHS) {
+if (IsManuallyAlloc)
+  ValueStorage::getFromPayload(getPtr())->Release();
 
-  Interp = std::exchange(RHS.Interp, nullptr);
-  OpaqueType = std::exchange(RHS.OpaqueType, nullptr);
-  ValueKind = std::exchange(RHS.ValueKind, K_Unspecified);
-  IsManuallyAlloc = std::exchange(RHS.IsManuallyAlloc, false);
-
-  Data = RHS.Data;
+Interp = std::exchange(RHS.Interp, nullptr);
+OpaqueType = std::exchange(RHS.OpaqueType, nullptr);
+ValueKind = std::exchange(RHS.ValueKind, K_Unspecified);
+IsManuallyAlloc = std::exchange(RHS.IsManuallyAlloc, false);
 
+Data = RHS.Data;
+  }
   return *this;
 }
 
Index: clang/lib/CodeGen/CGDebugInfo.h
===
--- clang/lib/CodeGen/CGDebugInfo.h
+++ clang/lib/CodeGen/CGDebugInfo.h
@@ -832,8 +832,10 @@
 
   // Define copy assignment operator.
   ApplyDebugLocation =(ApplyDebugLocation &) {
-CGF = Other.CGF;
-Other.CGF = nullptr;
+if (this != Other) {
+  CGF = Other.CGF;
+  Other.CGF = nullptr;
+}
 return *this;
   }
 
Index: clang/lib/AST/APValue.cpp
===
--- clang/lib/AST/APValue.cpp
+++ clang/lib/AST/APValue.cpp
@@ -390,11 +390,13 @@
 }
 
 APValue ::operator=(APValue &) {
-  if (Kind != None && Kind != Indeterminate)
-DestroyDataAndMakeUninit();
-  Kind = RHS.Kind;
-  Data = RHS.Data;
-  RHS.Kind = None;
+  if (this != RHS) {
+if (Kind != None && Kind != Indeterminate)
+  DestroyDataAndMakeUninit();
+Kind = RHS.Kind;
+Data = RHS.Data;
+RHS.Kind = None;
+  }
   return *this;
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D155774: [NFC] Remove needless nullchecks.

[Sindhu Chittireddy via Phabricator via cfe-commits]

schittir created this revision.
schittir added reviewers: aaron.ballman, tahonermann.
Herald added a project: All.
schittir requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D155774

Files:
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Lex/ModuleMap.cpp
  clang/lib/Sema/SemaExpr.cpp


Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -10926,11 +10926,9 @@
 return true;
 
   // Adjust scalar if desired.
-  if (Scalar) {
-if (ScalarCast != CK_NoOp)
-  *Scalar = S.ImpCastExprToType(Scalar->get(), VectorEltTy, ScalarCast);
-*Scalar = S.ImpCastExprToType(Scalar->get(), VectorTy, CK_VectorSplat);
-  }
+  if (ScalarCast != CK_NoOp)
+*Scalar = S.ImpCastExprToType(Scalar->get(), VectorEltTy, ScalarCast);
+  *Scalar = S.ImpCastExprToType(Scalar->get(), VectorTy, CK_VectorSplat);
   return false;
 }
 
Index: clang/lib/Lex/ModuleMap.cpp
===
--- clang/lib/Lex/ModuleMap.cpp
+++ clang/lib/Lex/ModuleMap.cpp
@@ -2475,7 +2475,7 @@
   bool NeedsFramework = false;
   Map.addUnresolvedHeader(ActiveModule, std::move(Header), NeedsFramework);
 
-  if (NeedsFramework && ActiveModule)
+  if (NeedsFramework)
 Diags.Report(CurrModuleDeclLoc, diag::note_mmap_add_framework_keyword)
   << ActiveModule->getFullModuleName()
   << FixItHint::CreateReplacement(CurrModuleDeclLoc, "framework module");
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -5234,7 +5234,7 @@
   // Is accessible from all the threads within the grid and from the host
   // through the runtime library (cudaGetSymbolAddress() / cudaGetSymbolSize()
   // / cudaMemcpyToSymbol() / cudaMemcpyFromSymbol())."
-  if (GV && LangOpts.CUDA) {
+  if (LangOpts.CUDA) {
 if (LangOpts.CUDAIsDevice) {
   if (Linkage != llvm::GlobalValue::InternalLinkage &&
   (D->hasAttr() || D->hasAttr() ||


Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -10926,11 +10926,9 @@
 return true;
 
   // Adjust scalar if desired.
-  if (Scalar) {
-if (ScalarCast != CK_NoOp)
-  *Scalar = S.ImpCastExprToType(Scalar->get(), VectorEltTy, ScalarCast);
-*Scalar = S.ImpCastExprToType(Scalar->get(), VectorTy, CK_VectorSplat);
-  }
+  if (ScalarCast != CK_NoOp)
+*Scalar = S.ImpCastExprToType(Scalar->get(), VectorEltTy, ScalarCast);
+  *Scalar = S.ImpCastExprToType(Scalar->get(), VectorTy, CK_VectorSplat);
   return false;
 }
 
Index: clang/lib/Lex/ModuleMap.cpp
===
--- clang/lib/Lex/ModuleMap.cpp
+++ clang/lib/Lex/ModuleMap.cpp
@@ -2475,7 +2475,7 @@
   bool NeedsFramework = false;
   Map.addUnresolvedHeader(ActiveModule, std::move(Header), NeedsFramework);
 
-  if (NeedsFramework && ActiveModule)
+  if (NeedsFramework)
 Diags.Report(CurrModuleDeclLoc, diag::note_mmap_add_framework_keyword)
   << ActiveModule->getFullModuleName()
   << FixItHint::CreateReplacement(CurrModuleDeclLoc, "framework module");
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -5234,7 +5234,7 @@
   // Is accessible from all the threads within the grid and from the host
   // through the runtime library (cudaGetSymbolAddress() / cudaGetSymbolSize()
   // / cudaMemcpyToSymbol() / cudaMemcpyFromSymbol())."
-  if (GV && LangOpts.CUDA) {
+  if (LangOpts.CUDA) {
 if (LangOpts.CUDAIsDevice) {
   if (Linkage != llvm::GlobalValue::InternalLinkage &&
   (D->hasAttr() || D->hasAttr() ||
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D155776: [NFC] Add checks for self-assignment.

[Sindhu Chittireddy via Phabricator via cfe-commits]

schittir created this revision.
Herald added a project: All.
schittir requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D155776

Files:
  clang/include/clang/Sema/ObjCMethodList.h
  clang/lib/AST/APValue.cpp
  clang/lib/CodeGen/CGDebugInfo.h
  clang/lib/Interpreter/Value.cpp


Index: clang/lib/Interpreter/Value.cpp
===
--- clang/lib/Interpreter/Value.cpp
+++ clang/lib/Interpreter/Value.cpp
@@ -201,16 +201,17 @@
 }
 
 Value ::operator=(Value &) noexcept {
-  if (IsManuallyAlloc)
-ValueStorage::getFromPayload(getPtr())->Release();
+  if (this != RHS) {
+if (IsManuallyAlloc)
+  ValueStorage::getFromPayload(getPtr())->Release();
 
-  Interp = std::exchange(RHS.Interp, nullptr);
-  OpaqueType = std::exchange(RHS.OpaqueType, nullptr);
-  ValueKind = std::exchange(RHS.ValueKind, K_Unspecified);
-  IsManuallyAlloc = std::exchange(RHS.IsManuallyAlloc, false);
-
-  Data = RHS.Data;
+Interp = std::exchange(RHS.Interp, nullptr);
+OpaqueType = std::exchange(RHS.OpaqueType, nullptr);
+ValueKind = std::exchange(RHS.ValueKind, K_Unspecified);
+IsManuallyAlloc = std::exchange(RHS.IsManuallyAlloc, false);
 
+Data = RHS.Data;
+  }
   return *this;
 }
 
Index: clang/lib/CodeGen/CGDebugInfo.h
===
--- clang/lib/CodeGen/CGDebugInfo.h
+++ clang/lib/CodeGen/CGDebugInfo.h
@@ -832,8 +832,10 @@
 
   // Define copy assignment operator.
   ApplyDebugLocation =(ApplyDebugLocation &) {
-CGF = Other.CGF;
-Other.CGF = nullptr;
+if (this != Other) {
+  CGF = Other.CGF;
+  Other.CGF = nullptr;
+}
 return *this;
   }
 
Index: clang/lib/AST/APValue.cpp
===
--- clang/lib/AST/APValue.cpp
+++ clang/lib/AST/APValue.cpp
@@ -390,11 +390,13 @@
 }
 
 APValue ::operator=(APValue &) {
-  if (Kind != None && Kind != Indeterminate)
-DestroyDataAndMakeUninit();
-  Kind = RHS.Kind;
-  Data = RHS.Data;
-  RHS.Kind = None;
+  if (this != RHS) {
+if (Kind != None && Kind != Indeterminate)
+  DestroyDataAndMakeUninit();
+Kind = RHS.Kind;
+Data = RHS.Data;
+RHS.Kind = None;
+  }
   return *this;
 }
 
Index: clang/include/clang/Sema/ObjCMethodList.h
===
--- clang/include/clang/Sema/ObjCMethodList.h
+++ clang/include/clang/Sema/ObjCMethodList.h
@@ -37,8 +37,10 @@
 NextAndExtraBits(L.NextAndExtraBits) {}
 
   ObjCMethodList =(const ObjCMethodList ) {
-MethodAndHasMoreThanOneDecl = L.MethodAndHasMoreThanOneDecl;
-NextAndExtraBits = L.NextAndExtraBits;
+if (this != L) {
+  MethodAndHasMoreThanOneDecl = L.MethodAndHasMoreThanOneDecl;
+  NextAndExtraBits = L.NextAndExtraBits;
+}
 return *this;
   }
 


Index: clang/lib/Interpreter/Value.cpp
===
--- clang/lib/Interpreter/Value.cpp
+++ clang/lib/Interpreter/Value.cpp
@@ -201,16 +201,17 @@
 }
 
 Value ::operator=(Value &) noexcept {
-  if (IsManuallyAlloc)
-ValueStorage::getFromPayload(getPtr())->Release();
+  if (this != RHS) {
+if (IsManuallyAlloc)
+  ValueStorage::getFromPayload(getPtr())->Release();
 
-  Interp = std::exchange(RHS.Interp, nullptr);
-  OpaqueType = std::exchange(RHS.OpaqueType, nullptr);
-  ValueKind = std::exchange(RHS.ValueKind, K_Unspecified);
-  IsManuallyAlloc = std::exchange(RHS.IsManuallyAlloc, false);
-
-  Data = RHS.Data;
+Interp = std::exchange(RHS.Interp, nullptr);
+OpaqueType = std::exchange(RHS.OpaqueType, nullptr);
+ValueKind = std::exchange(RHS.ValueKind, K_Unspecified);
+IsManuallyAlloc = std::exchange(RHS.IsManuallyAlloc, false);
 
+Data = RHS.Data;
+  }
   return *this;
 }
 
Index: clang/lib/CodeGen/CGDebugInfo.h
===
--- clang/lib/CodeGen/CGDebugInfo.h
+++ clang/lib/CodeGen/CGDebugInfo.h
@@ -832,8 +832,10 @@
 
   // Define copy assignment operator.
   ApplyDebugLocation =(ApplyDebugLocation &) {
-CGF = Other.CGF;
-Other.CGF = nullptr;
+if (this != Other) {
+  CGF = Other.CGF;
+  Other.CGF = nullptr;
+}
 return *this;
   }
 
Index: clang/lib/AST/APValue.cpp
===
--- clang/lib/AST/APValue.cpp
+++ clang/lib/AST/APValue.cpp
@@ -390,11 +390,13 @@
 }
 
 APValue ::operator=(APValue &) {
-  if (Kind != None && Kind != Indeterminate)
-DestroyDataAndMakeUninit();
-  Kind = RHS.Kind;
-  Data = RHS.Data;
-  RHS.Kind = None;
+  if (this != RHS) {
+if (Kind != None && Kind != Indeterminate)
+  DestroyDataAndMakeUninit();
+Kind = RHS.Kind;
+Data = RHS.Data;
+RHS.Kind = None;
+  }
   return *this;
 }
 
Index: 

[PATCH] D153589: [NFC] Initialize pointer fields and remove a needless null check.

[Sindhu Chittireddy via Phabricator via cfe-commits]

schittir added a comment.

Thank you all for reviewing.


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

https://reviews.llvm.org/D153589

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

[PATCH] D153926: [NFC] Initialize class member pointers to nullptr.

[Sindhu Chittireddy via Phabricator via cfe-commits]

schittir added a comment.

Thank you for reviewing. This patch is now landed.


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

https://reviews.llvm.org/D153926

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

[PATCH] D152689: [NFC] Remove dead conditionals

[Sindhu Chittireddy via Phabricator via cfe-commits]

schittir created this revision.
schittir added reviewers: tahonermann, aaron.ballman, erichkeane.
Herald added a subscriber: hiraditya.
Herald added a project: All.
schittir requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

Patch contains changes to remove obvious dead code.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D152689

Files:
  clang/lib/CodeGen/CGObjCMac.cpp
  clang/lib/Sema/SemaOverload.cpp
  llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp


Index: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
===
--- llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
+++ llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
@@ -2120,8 +2120,7 @@
 // Avoid incrementing past stop since it could overflow.
 Value *CountIfTwo = Builder.CreateAdd(
 Builder.CreateUDiv(Builder.CreateSub(Span, One), Incr), One);
-Value *OneCmp = Builder.CreateICmp(
-InclusiveStop ? CmpInst::ICMP_ULT : CmpInst::ICMP_ULE, Span, Incr);
+Value *OneCmp = Builder.CreateICmp(CmpInst::ICMP_ULE, Span, Incr);
 CountIfLooping = Builder.CreateSelect(OneCmp, One, CountIfTwo);
   }
   Value *TripCount = Builder.CreateSelect(ZeroCmp, Zero, CountIfLooping,
Index: clang/lib/Sema/SemaOverload.cpp
===
--- clang/lib/Sema/SemaOverload.cpp
+++ clang/lib/Sema/SemaOverload.cpp
@@ -10865,7 +10865,7 @@
   if (FromExpr && isa(FromExpr)) {
 S.Diag(Fn->getLocation(), diag::note_ovl_candidate_bad_list_argument)
 << (unsigned)FnKindPair.first << (unsigned)FnKindPair.second << FnDesc
-<< (FromExpr ? FromExpr->getSourceRange() : SourceRange()) << FromTy
+<< FromExpr->getSourceRange() << FromTy
 << ToTy << (unsigned)isObjectArgument << I + 1
 << (Conv.Bad.Kind == BadConversionSequence::too_few_initializers ? 1
 : Conv.Bad.Kind == BadConversionSequence::too_many_initializers
Index: clang/lib/CodeGen/CGObjCMac.cpp
===
--- clang/lib/CodeGen/CGObjCMac.cpp
+++ clang/lib/CodeGen/CGObjCMac.cpp
@@ -3809,15 +3809,9 @@
   ivarList.fillPlaceholderWithInt(countSlot, ObjCTypes.IntTy, count);
 
   llvm::GlobalVariable *GV;
-  if (ForClass)
-GV =
-CreateMetadataVar("OBJC_CLASS_VARIABLES_" + ID->getName(), ivarList,
-  "__OBJC,__class_vars,regular,no_dead_strip",
-  CGM.getPointerAlign(), true);
-  else
-GV = CreateMetadataVar("OBJC_INSTANCE_VARIABLES_" + ID->getName(), 
ivarList,
-   "__OBJC,__instance_vars,regular,no_dead_strip",
-   CGM.getPointerAlign(), true);
+  GV = CreateMetadataVar("OBJC_INSTANCE_VARIABLES_" + ID->getName(), ivarList,
+ "__OBJC,__instance_vars,regular,no_dead_strip",
+ CGM.getPointerAlign(), true);
   return llvm::ConstantExpr::getBitCast(GV, ObjCTypes.IvarListPtrTy);
 }
 


Index: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
===
--- llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
+++ llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
@@ -2120,8 +2120,7 @@
 // Avoid incrementing past stop since it could overflow.
 Value *CountIfTwo = Builder.CreateAdd(
 Builder.CreateUDiv(Builder.CreateSub(Span, One), Incr), One);
-Value *OneCmp = Builder.CreateICmp(
-InclusiveStop ? CmpInst::ICMP_ULT : CmpInst::ICMP_ULE, Span, Incr);
+Value *OneCmp = Builder.CreateICmp(CmpInst::ICMP_ULE, Span, Incr);
 CountIfLooping = Builder.CreateSelect(OneCmp, One, CountIfTwo);
   }
   Value *TripCount = Builder.CreateSelect(ZeroCmp, Zero, CountIfLooping,
Index: clang/lib/Sema/SemaOverload.cpp
===
--- clang/lib/Sema/SemaOverload.cpp
+++ clang/lib/Sema/SemaOverload.cpp
@@ -10865,7 +10865,7 @@
   if (FromExpr && isa(FromExpr)) {
 S.Diag(Fn->getLocation(), diag::note_ovl_candidate_bad_list_argument)
 << (unsigned)FnKindPair.first << (unsigned)FnKindPair.second << FnDesc
-<< (FromExpr ? FromExpr->getSourceRange() : SourceRange()) << FromTy
+<< FromExpr->getSourceRange() << FromTy
 << ToTy << (unsigned)isObjectArgument << I + 1
 << (Conv.Bad.Kind == BadConversionSequence::too_few_initializers ? 1
 : Conv.Bad.Kind == BadConversionSequence::too_many_initializers
Index: clang/lib/CodeGen/CGObjCMac.cpp
===
--- clang/lib/CodeGen/CGObjCMac.cpp
+++ clang/lib/CodeGen/CGObjCMac.cpp
@@ -3809,15 +3809,9 @@
   ivarList.fillPlaceholderWithInt(countSlot, ObjCTypes.IntTy, count);
 
   llvm::GlobalVariable *GV;
-  if (ForClass)
-GV =
-CreateMetadataVar("OBJC_CLASS_VARIABLES_" + ID->getName(), ivarList,
-  

[PATCH] D155776: [NFC] Add checks for self-assignment.

[Sindhu Chittireddy via Phabricator via cfe-commits]

schittir added a comment.

@tahonermann Thank you for considering this thoroughly. This patch is now 
landed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155776

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

[PATCH] D155776: [NFC] Add checks for self-assignment.

[Sindhu Chittireddy via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG8ac137acefc0: [NFC] Add checks for self-assignment. 
(authored by schittir).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155776

Files:
  clang/lib/AST/APValue.cpp
  clang/lib/CodeGen/CGDebugInfo.h
  clang/lib/Interpreter/Value.cpp


Index: clang/lib/Interpreter/Value.cpp
===
--- clang/lib/Interpreter/Value.cpp
+++ clang/lib/Interpreter/Value.cpp
@@ -201,16 +201,17 @@
 }
 
 Value ::operator=(Value &) noexcept {
-  if (IsManuallyAlloc)
-ValueStorage::getFromPayload(getPtr())->Release();
+  if (this != RHS) {
+if (IsManuallyAlloc)
+  ValueStorage::getFromPayload(getPtr())->Release();
 
-  Interp = std::exchange(RHS.Interp, nullptr);
-  OpaqueType = std::exchange(RHS.OpaqueType, nullptr);
-  ValueKind = std::exchange(RHS.ValueKind, K_Unspecified);
-  IsManuallyAlloc = std::exchange(RHS.IsManuallyAlloc, false);
-
-  Data = RHS.Data;
+Interp = std::exchange(RHS.Interp, nullptr);
+OpaqueType = std::exchange(RHS.OpaqueType, nullptr);
+ValueKind = std::exchange(RHS.ValueKind, K_Unspecified);
+IsManuallyAlloc = std::exchange(RHS.IsManuallyAlloc, false);
 
+Data = RHS.Data;
+  }
   return *this;
 }
 
Index: clang/lib/CodeGen/CGDebugInfo.h
===
--- clang/lib/CodeGen/CGDebugInfo.h
+++ clang/lib/CodeGen/CGDebugInfo.h
@@ -832,8 +832,10 @@
 
   // Define copy assignment operator.
   ApplyDebugLocation =(ApplyDebugLocation &) {
-CGF = Other.CGF;
-Other.CGF = nullptr;
+if (this != Other) {
+  CGF = Other.CGF;
+  Other.CGF = nullptr;
+}
 return *this;
   }
 
Index: clang/lib/AST/APValue.cpp
===
--- clang/lib/AST/APValue.cpp
+++ clang/lib/AST/APValue.cpp
@@ -390,11 +390,13 @@
 }
 
 APValue ::operator=(APValue &) {
-  if (Kind != None && Kind != Indeterminate)
-DestroyDataAndMakeUninit();
-  Kind = RHS.Kind;
-  Data = RHS.Data;
-  RHS.Kind = None;
+  if (this != RHS) {
+if (Kind != None && Kind != Indeterminate)
+  DestroyDataAndMakeUninit();
+Kind = RHS.Kind;
+Data = RHS.Data;
+RHS.Kind = None;
+  }
   return *this;
 }
 


Index: clang/lib/Interpreter/Value.cpp
===
--- clang/lib/Interpreter/Value.cpp
+++ clang/lib/Interpreter/Value.cpp
@@ -201,16 +201,17 @@
 }
 
 Value ::operator=(Value &) noexcept {
-  if (IsManuallyAlloc)
-ValueStorage::getFromPayload(getPtr())->Release();
+  if (this != RHS) {
+if (IsManuallyAlloc)
+  ValueStorage::getFromPayload(getPtr())->Release();
 
-  Interp = std::exchange(RHS.Interp, nullptr);
-  OpaqueType = std::exchange(RHS.OpaqueType, nullptr);
-  ValueKind = std::exchange(RHS.ValueKind, K_Unspecified);
-  IsManuallyAlloc = std::exchange(RHS.IsManuallyAlloc, false);
-
-  Data = RHS.Data;
+Interp = std::exchange(RHS.Interp, nullptr);
+OpaqueType = std::exchange(RHS.OpaqueType, nullptr);
+ValueKind = std::exchange(RHS.ValueKind, K_Unspecified);
+IsManuallyAlloc = std::exchange(RHS.IsManuallyAlloc, false);
 
+Data = RHS.Data;
+  }
   return *this;
 }
 
Index: clang/lib/CodeGen/CGDebugInfo.h
===
--- clang/lib/CodeGen/CGDebugInfo.h
+++ clang/lib/CodeGen/CGDebugInfo.h
@@ -832,8 +832,10 @@
 
   // Define copy assignment operator.
   ApplyDebugLocation =(ApplyDebugLocation &) {
-CGF = Other.CGF;
-Other.CGF = nullptr;
+if (this != Other) {
+  CGF = Other.CGF;
+  Other.CGF = nullptr;
+}
 return *this;
   }
 
Index: clang/lib/AST/APValue.cpp
===
--- clang/lib/AST/APValue.cpp
+++ clang/lib/AST/APValue.cpp
@@ -390,11 +390,13 @@
 }
 
 APValue ::operator=(APValue &) {
-  if (Kind != None && Kind != Indeterminate)
-DestroyDataAndMakeUninit();
-  Kind = RHS.Kind;
-  Data = RHS.Data;
-  RHS.Kind = None;
+  if (this != RHS) {
+if (Kind != None && Kind != Indeterminate)
+  DestroyDataAndMakeUninit();
+Kind = RHS.Kind;
+Data = RHS.Data;
+RHS.Kind = None;
+  }
   return *this;
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D156274: [NFC] Avoid potential dereferencing of nullptr.

[Sindhu Chittireddy via Phabricator via cfe-commits]

schittir created this revision.
schittir added reviewers: aaron.ballman, tahonermann, erichkeane.
Herald added a project: All.
schittir requested review of this revision.
Herald added subscribers: cfe-commits, wangpc.
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D156274

Files:
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/CodeGen/CGObjCMac.cpp
  clang/lib/Sema/SemaCodeComplete.cpp


Index: clang/lib/Sema/SemaCodeComplete.cpp
===
--- clang/lib/Sema/SemaCodeComplete.cpp
+++ clang/lib/Sema/SemaCodeComplete.cpp
@@ -6601,7 +6601,7 @@
   // The "template" keyword can follow "::" in the grammar, but only
   // put it into the grammar if the nested-name-specifier is dependent.
   // FIXME: results is always empty, this appears to be dead.
-  if (!Results.empty() && NNS->isDependent())
+  if (!Results.empty() && NNS && NNS->isDependent())
 Results.AddResult("template");
 
   // If the scope is a concept-constrained type parameter, infer nested
Index: clang/lib/CodeGen/CGObjCMac.cpp
===
--- clang/lib/CodeGen/CGObjCMac.cpp
+++ clang/lib/CodeGen/CGObjCMac.cpp
@@ -1803,6 +1803,7 @@
 }
 
 // Complex types.
+assert(contBB);
 CGF.EmitBlock(contBB);
 CodeGenFunction::ComplexPairTy callResult = result.getComplexVal();
 
Index: clang/lib/AST/ItaniumMangle.cpp
===
--- clang/lib/AST/ItaniumMangle.cpp
+++ clang/lib/AST/ItaniumMangle.cpp
@@ -1656,6 +1656,7 @@
   // Otherwise, use the complete destructor name. This is relevant if a
   // class with a destructor is declared within a destructor.
   mangleCXXDtorType(Dtor_Complete);
+assert(ND);
 writeAbiTags(ND, AdditionalAbiTags);
 break;
 


Index: clang/lib/Sema/SemaCodeComplete.cpp
===
--- clang/lib/Sema/SemaCodeComplete.cpp
+++ clang/lib/Sema/SemaCodeComplete.cpp
@@ -6601,7 +6601,7 @@
   // The "template" keyword can follow "::" in the grammar, but only
   // put it into the grammar if the nested-name-specifier is dependent.
   // FIXME: results is always empty, this appears to be dead.
-  if (!Results.empty() && NNS->isDependent())
+  if (!Results.empty() && NNS && NNS->isDependent())
 Results.AddResult("template");
 
   // If the scope is a concept-constrained type parameter, infer nested
Index: clang/lib/CodeGen/CGObjCMac.cpp
===
--- clang/lib/CodeGen/CGObjCMac.cpp
+++ clang/lib/CodeGen/CGObjCMac.cpp
@@ -1803,6 +1803,7 @@
 }
 
 // Complex types.
+assert(contBB);
 CGF.EmitBlock(contBB);
 CodeGenFunction::ComplexPairTy callResult = result.getComplexVal();
 
Index: clang/lib/AST/ItaniumMangle.cpp
===
--- clang/lib/AST/ItaniumMangle.cpp
+++ clang/lib/AST/ItaniumMangle.cpp
@@ -1656,6 +1656,7 @@
   // Otherwise, use the complete destructor name. This is relevant if a
   // class with a destructor is declared within a destructor.
   mangleCXXDtorType(Dtor_Complete);
+assert(ND);
 writeAbiTags(ND, AdditionalAbiTags);
 break;
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D156261: [NFC] Cast unchecked return values to void.

[Sindhu Chittireddy via Phabricator via cfe-commits]

schittir created this revision.
schittir added reviewers: tahonermann, aaron.ballman.
Herald added a project: All.
schittir requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D156261

Files:
  clang/lib/Sema/SemaDeclCXX.cpp


Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -2482,7 +2482,7 @@
 << /*Call*/ 1 << /*Function*/ 0 << E->getSourceRange();
   }
   for (auto A : E->arguments()) {
-getDerived().TraverseStmt(A);
+(void)getDerived().TraverseStmt(A);
   }
   return true;
 }
@@ -2516,7 +2516,7 @@
 bool VisitBlockExpr(BlockExpr *T) { return true; }
 
   } Visitor(*this, FD);
-  Visitor.TraverseStmt(FD->getBody());
+  (void)Visitor.TraverseStmt(FD->getBody());
 }
 
 /// Get the class that is directly named by the current context. This is the


Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -2482,7 +2482,7 @@
 << /*Call*/ 1 << /*Function*/ 0 << E->getSourceRange();
   }
   for (auto A : E->arguments()) {
-getDerived().TraverseStmt(A);
+(void)getDerived().TraverseStmt(A);
   }
   return true;
 }
@@ -2516,7 +2516,7 @@
 bool VisitBlockExpr(BlockExpr *T) { return true; }
 
   } Visitor(*this, FD);
-  Visitor.TraverseStmt(FD->getBody());
+  (void)Visitor.TraverseStmt(FD->getBody());
 }
 
 /// Get the class that is directly named by the current context. This is the
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D156274: [NFC] Avoid potential dereferencing of nullptr.

[Sindhu Chittireddy via Phabricator via cfe-commits]

schittir updated this revision to Diff 545410.
schittir added a comment.

Remove changes in CGObjCMac.cpp


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

https://reviews.llvm.org/D156274

Files:
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/Sema/SemaCodeComplete.cpp


Index: clang/lib/Sema/SemaCodeComplete.cpp
===
--- clang/lib/Sema/SemaCodeComplete.cpp
+++ clang/lib/Sema/SemaCodeComplete.cpp
@@ -6601,7 +6601,7 @@
   // The "template" keyword can follow "::" in the grammar, but only
   // put it into the grammar if the nested-name-specifier is dependent.
   // FIXME: results is always empty, this appears to be dead.
-  if (!Results.empty() && NNS->isDependent())
+  if (!Results.empty() && NNS && NNS->isDependent())
 Results.AddResult("template");
 
   // If the scope is a concept-constrained type parameter, infer nested
Index: clang/lib/AST/ItaniumMangle.cpp
===
--- clang/lib/AST/ItaniumMangle.cpp
+++ clang/lib/AST/ItaniumMangle.cpp
@@ -1656,6 +1656,7 @@
   // Otherwise, use the complete destructor name. This is relevant if a
   // class with a destructor is declared within a destructor.
   mangleCXXDtorType(Dtor_Complete);
+assert(ND);
 writeAbiTags(ND, AdditionalAbiTags);
 break;
 


Index: clang/lib/Sema/SemaCodeComplete.cpp
===
--- clang/lib/Sema/SemaCodeComplete.cpp
+++ clang/lib/Sema/SemaCodeComplete.cpp
@@ -6601,7 +6601,7 @@
   // The "template" keyword can follow "::" in the grammar, but only
   // put it into the grammar if the nested-name-specifier is dependent.
   // FIXME: results is always empty, this appears to be dead.
-  if (!Results.empty() && NNS->isDependent())
+  if (!Results.empty() && NNS && NNS->isDependent())
 Results.AddResult("template");
 
   // If the scope is a concept-constrained type parameter, infer nested
Index: clang/lib/AST/ItaniumMangle.cpp
===
--- clang/lib/AST/ItaniumMangle.cpp
+++ clang/lib/AST/ItaniumMangle.cpp
@@ -1656,6 +1656,7 @@
   // Otherwise, use the complete destructor name. This is relevant if a
   // class with a destructor is declared within a destructor.
   mangleCXXDtorType(Dtor_Complete);
+assert(ND);
 writeAbiTags(ND, AdditionalAbiTags);
 break;
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D156261: [NFC] Cast unchecked return values to void.

[Sindhu Chittireddy via Phabricator via cfe-commits]

schittir added a comment.

In D156261#4535220 , @aaron.ballman 
wrote:

> I think the cast to void is unneeded in these cases. We typically add a cast 
> to void when the function exists to compute a result but also has side 
> effects. e.g., you typically call `malloc()` because you want the returned 
> pointer, but if for some reason you don't want the pointer but still want the 
> side effect of the memory allocation, you'd cast the result of the call to 
> `void`. In this case, the `Traverse` functions exist mostly to perform side 
> effects and the return value is only used to indicate "should we keep 
> going?". It's reasonable to ignore the return value in this case if you don't 
> intend to stop the traversal (and in this particular case, nothing will 
> return `false` from the traversal and so there's really no need to check the 
> return values here).

I see. Thank you for the explanation. I had mixed thoughts about whether 
casting to void is considered a best practice in general, in addition to the 
use of [[nodiscard]] wherever applicable. 
This seemed like a harmless change. I don't have strong opinions here, so I 
will go ahead and abandon these changes unless any one else disagrees.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156261

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

[PATCH] D156274: [NFC] Avoid potential dereferencing of nullptr.

[Sindhu Chittireddy via Phabricator via cfe-commits]

schittir added a comment.

Thank you for the review, @aaron.ballman and @efriedma
Do you recommend any changes here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156274

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

[PATCH] D156274: [NFC] Avoid potential dereferencing of nullptr.

[Sindhu Chittireddy via Phabricator via cfe-commits]

schittir added a comment.

Test failure seems unrelated to change. Driver/fsanitize.c lit test passes in 
my local testing. Restarting build.


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

https://reviews.llvm.org/D156274

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

[PATCH] D156274: [NFC] Avoid potential dereferencing of nullptr.

[Sindhu Chittireddy via Phabricator via cfe-commits]

schittir added a comment.

In D156274#4544804 , @aaron.ballman 
wrote:

> LGTM! Are you planning to work on the ObjC issues separately? (Not suggesting 
> you have to! But if you don't intend to, can you file an issue in GitHub so 
> we don't lose track of the problem?)

Hi Aaron, 
I will make sure to file a bug report. 
Thank you!


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

https://reviews.llvm.org/D156274

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

[PATCH] D156274: [NFC] Avoid potential dereferencing of nullptr.

[Sindhu Chittireddy via Phabricator via cfe-commits]

schittir added a comment.

In D156274#4545581 , @schittir wrote:

> Test failure seems unrelated to change. Driver/fsanitize.c lit test passes in 
> my local testing. Restarting build.

The build is now green. Landing the patch.


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

https://reviews.llvm.org/D156274

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

[PATCH] D156274: [NFC] Avoid potential dereferencing of nullptr.

[Sindhu Chittireddy via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG235390d930be: [NFC] Avoid potential dereferencing of 
nullptr. (authored by schittir).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156274

Files:
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/Sema/SemaCodeComplete.cpp


Index: clang/lib/Sema/SemaCodeComplete.cpp
===
--- clang/lib/Sema/SemaCodeComplete.cpp
+++ clang/lib/Sema/SemaCodeComplete.cpp
@@ -6601,7 +6601,7 @@
   // The "template" keyword can follow "::" in the grammar, but only
   // put it into the grammar if the nested-name-specifier is dependent.
   // FIXME: results is always empty, this appears to be dead.
-  if (!Results.empty() && NNS->isDependent())
+  if (!Results.empty() && NNS && NNS->isDependent())
 Results.AddResult("template");
 
   // If the scope is a concept-constrained type parameter, infer nested
Index: clang/lib/AST/ItaniumMangle.cpp
===
--- clang/lib/AST/ItaniumMangle.cpp
+++ clang/lib/AST/ItaniumMangle.cpp
@@ -1656,6 +1656,7 @@
   // Otherwise, use the complete destructor name. This is relevant if a
   // class with a destructor is declared within a destructor.
   mangleCXXDtorType(Dtor_Complete);
+assert(ND);
 writeAbiTags(ND, AdditionalAbiTags);
 break;
 


Index: clang/lib/Sema/SemaCodeComplete.cpp
===
--- clang/lib/Sema/SemaCodeComplete.cpp
+++ clang/lib/Sema/SemaCodeComplete.cpp
@@ -6601,7 +6601,7 @@
   // The "template" keyword can follow "::" in the grammar, but only
   // put it into the grammar if the nested-name-specifier is dependent.
   // FIXME: results is always empty, this appears to be dead.
-  if (!Results.empty() && NNS->isDependent())
+  if (!Results.empty() && NNS && NNS->isDependent())
 Results.AddResult("template");
 
   // If the scope is a concept-constrained type parameter, infer nested
Index: clang/lib/AST/ItaniumMangle.cpp
===
--- clang/lib/AST/ItaniumMangle.cpp
+++ clang/lib/AST/ItaniumMangle.cpp
@@ -1656,6 +1656,7 @@
   // Otherwise, use the complete destructor name. This is relevant if a
   // class with a destructor is declared within a destructor.
   mangleCXXDtorType(Dtor_Complete);
+assert(ND);
 writeAbiTags(ND, AdditionalAbiTags);
 break;
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D155774: [NFC] Remove needless nullchecks.

[Sindhu Chittireddy via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG2ce662c5d596: [NFC] Remove needless nullchecks. (authored by 
schittir).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155774

Files:
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Lex/ModuleMap.cpp
  clang/lib/Sema/SemaExpr.cpp


Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -10926,11 +10926,9 @@
 return true;
 
   // Adjust scalar if desired.
-  if (Scalar) {
-if (ScalarCast != CK_NoOp)
-  *Scalar = S.ImpCastExprToType(Scalar->get(), VectorEltTy, ScalarCast);
-*Scalar = S.ImpCastExprToType(Scalar->get(), VectorTy, CK_VectorSplat);
-  }
+  if (ScalarCast != CK_NoOp)
+*Scalar = S.ImpCastExprToType(Scalar->get(), VectorEltTy, ScalarCast);
+  *Scalar = S.ImpCastExprToType(Scalar->get(), VectorTy, CK_VectorSplat);
   return false;
 }
 
Index: clang/lib/Lex/ModuleMap.cpp
===
--- clang/lib/Lex/ModuleMap.cpp
+++ clang/lib/Lex/ModuleMap.cpp
@@ -2475,7 +2475,7 @@
   bool NeedsFramework = false;
   Map.addUnresolvedHeader(ActiveModule, std::move(Header), NeedsFramework);
 
-  if (NeedsFramework && ActiveModule)
+  if (NeedsFramework)
 Diags.Report(CurrModuleDeclLoc, diag::note_mmap_add_framework_keyword)
   << ActiveModule->getFullModuleName()
   << FixItHint::CreateReplacement(CurrModuleDeclLoc, "framework module");
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -5234,7 +5234,7 @@
   // Is accessible from all the threads within the grid and from the host
   // through the runtime library (cudaGetSymbolAddress() / cudaGetSymbolSize()
   // / cudaMemcpyToSymbol() / cudaMemcpyFromSymbol())."
-  if (GV && LangOpts.CUDA) {
+  if (LangOpts.CUDA) {
 if (LangOpts.CUDAIsDevice) {
   if (Linkage != llvm::GlobalValue::InternalLinkage &&
   (D->hasAttr() || D->hasAttr() ||


Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -10926,11 +10926,9 @@
 return true;
 
   // Adjust scalar if desired.
-  if (Scalar) {
-if (ScalarCast != CK_NoOp)
-  *Scalar = S.ImpCastExprToType(Scalar->get(), VectorEltTy, ScalarCast);
-*Scalar = S.ImpCastExprToType(Scalar->get(), VectorTy, CK_VectorSplat);
-  }
+  if (ScalarCast != CK_NoOp)
+*Scalar = S.ImpCastExprToType(Scalar->get(), VectorEltTy, ScalarCast);
+  *Scalar = S.ImpCastExprToType(Scalar->get(), VectorTy, CK_VectorSplat);
   return false;
 }
 
Index: clang/lib/Lex/ModuleMap.cpp
===
--- clang/lib/Lex/ModuleMap.cpp
+++ clang/lib/Lex/ModuleMap.cpp
@@ -2475,7 +2475,7 @@
   bool NeedsFramework = false;
   Map.addUnresolvedHeader(ActiveModule, std::move(Header), NeedsFramework);
 
-  if (NeedsFramework && ActiveModule)
+  if (NeedsFramework)
 Diags.Report(CurrModuleDeclLoc, diag::note_mmap_add_framework_keyword)
   << ActiveModule->getFullModuleName()
   << FixItHint::CreateReplacement(CurrModuleDeclLoc, "framework module");
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -5234,7 +5234,7 @@
   // Is accessible from all the threads within the grid and from the host
   // through the runtime library (cudaGetSymbolAddress() / cudaGetSymbolSize()
   // / cudaMemcpyToSymbol() / cudaMemcpyFromSymbol())."
-  if (GV && LangOpts.CUDA) {
+  if (LangOpts.CUDA) {
 if (LangOpts.CUDAIsDevice) {
   if (Linkage != llvm::GlobalValue::InternalLinkage &&
   (D->hasAttr() || D->hasAttr() ||
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D153236: [NFC] Fix potential dereferencing of nullptr.

[Sindhu Chittireddy via Phabricator via cfe-commits]

schittir added inline comments.



Comment at: clang/lib/Parse/ParseStmt.cpp:886-887
 TopLevelCase = Case;
-  else
+  else {
+assert(DeepestParsedCaseStmt && "DeepestParsedCaseStmt cannot be 
null");
 Actions.ActOnCaseStmtBody(DeepestParsedCaseStmt, Case.get());

aaron.ballman wrote:
> The `assert` doesn't add much value, this seems to be a false positive in the 
> analysis.
> 
> If we have no top-level case statement yet, then we get into the `if` branch 
> on line 884, but that then sets `DeepestParsedCaseStmt` on line 890, and that 
> must be non-null because we verified that `Case` is valid above on line 876. 
> So by the time we get into the `else` branch, `DeepestParsedCaseStmt` must be 
> nonnull.
> 
> I don't think changes are needed here at all; WDYT?
This makes sense. I didn't thoroughly consider the Case.isInvalid() check above 
in my analysis. Thank you for catching this! I removed this change. 


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

https://reviews.llvm.org/D153236

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

[PATCH] D153236: [NFC] Fix potential dereferencing of nullptr.

[Sindhu Chittireddy via Phabricator via cfe-commits]

schittir updated this revision to Diff 533386.
schittir marked 3 inline comments as done.
schittir added a comment.

Changes to address comments.


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

https://reviews.llvm.org/D153236

Files:
  clang/lib/Sema/SemaExprObjC.cpp
  clang/lib/Sema/SemaObjCProperty.cpp
  clang/lib/Sema/SemaType.cpp


Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -2708,8 +2708,8 @@
 return QualType();
   }
   // Only support _BitInt elements with byte-sized power of 2 NumBits.
-  if (CurType->isBitIntType()) {
-unsigned NumBits = CurType->getAs()->getNumBits();
+  if (const auto *BIT = CurType->getAs()) {
+unsigned NumBits = BIT->getNumBits();
 if (!llvm::isPowerOf2_32(NumBits) || NumBits < 8) {
   Diag(AttrLoc, diag::err_attribute_invalid_bitint_vector_type)
   << (NumBits < 8);
Index: clang/lib/Sema/SemaObjCProperty.cpp
===
--- clang/lib/Sema/SemaObjCProperty.cpp
+++ clang/lib/Sema/SemaObjCProperty.cpp
@@ -1363,10 +1363,9 @@
 if (!Context.hasSameType(PropertyIvarType, IvarType)) {
   if (isa(PropertyIvarType)
   && isa(IvarType))
-compat =
-  Context.canAssignObjCInterfaces(
-  
PropertyIvarType->getAs(),
-  IvarType->getAs());
+compat = Context.canAssignObjCInterfaces(
+PropertyIvarType->castAs(),
+IvarType->castAs());
   else {
 compat = (CheckAssignmentConstraints(PropertyIvarLoc, PropertyIvarType,
  IvarType)
Index: clang/lib/Sema/SemaExprObjC.cpp
===
--- clang/lib/Sema/SemaExprObjC.cpp
+++ clang/lib/Sema/SemaExprObjC.cpp
@@ -2438,6 +2438,9 @@
   if (!ReceiverType.isNull())
 receiverTypeInfo = Context.getTrivialTypeSourceInfo(ReceiverType);
 
+  assert(((isSuperReceiver && Loc.isValid()) || receiverTypeInfo) &&
+ "Either the super receiver location needs to be valid or the receiver 
"
+ "needs valid type source information");
   return BuildClassMessage(receiverTypeInfo, ReceiverType,
   /*SuperLoc=*/isSuperReceiver ? Loc : 
SourceLocation(),
Sel, Method, Loc, Loc, Loc, Args,


Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -2708,8 +2708,8 @@
 return QualType();
   }
   // Only support _BitInt elements with byte-sized power of 2 NumBits.
-  if (CurType->isBitIntType()) {
-unsigned NumBits = CurType->getAs()->getNumBits();
+  if (const auto *BIT = CurType->getAs()) {
+unsigned NumBits = BIT->getNumBits();
 if (!llvm::isPowerOf2_32(NumBits) || NumBits < 8) {
   Diag(AttrLoc, diag::err_attribute_invalid_bitint_vector_type)
   << (NumBits < 8);
Index: clang/lib/Sema/SemaObjCProperty.cpp
===
--- clang/lib/Sema/SemaObjCProperty.cpp
+++ clang/lib/Sema/SemaObjCProperty.cpp
@@ -1363,10 +1363,9 @@
 if (!Context.hasSameType(PropertyIvarType, IvarType)) {
   if (isa(PropertyIvarType)
   && isa(IvarType))
-compat =
-  Context.canAssignObjCInterfaces(
-  PropertyIvarType->getAs(),
-  IvarType->getAs());
+compat = Context.canAssignObjCInterfaces(
+PropertyIvarType->castAs(),
+IvarType->castAs());
   else {
 compat = (CheckAssignmentConstraints(PropertyIvarLoc, PropertyIvarType,
  IvarType)
Index: clang/lib/Sema/SemaExprObjC.cpp
===
--- clang/lib/Sema/SemaExprObjC.cpp
+++ clang/lib/Sema/SemaExprObjC.cpp
@@ -2438,6 +2438,9 @@
   if (!ReceiverType.isNull())
 receiverTypeInfo = Context.getTrivialTypeSourceInfo(ReceiverType);
 
+  assert(((isSuperReceiver && Loc.isValid()) || receiverTypeInfo) &&
+ "Either the super receiver location needs to be valid or the receiver "
+ "needs valid type source information");
   return BuildClassMessage(receiverTypeInfo, ReceiverType,
   /*SuperLoc=*/isSuperReceiver ? Loc : SourceLocation(),
Sel, Method, Loc, Loc, Loc, Args,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D153236: [NFC] Fix potential dereferencing of nullptr.

[Sindhu Chittireddy via Phabricator via cfe-commits]

schittir added a comment.

Thank you @aaron.ballman and @Fznamznon for reviewing! I landed this patch with 
commit d2fafa79ef08f9ef9cd0108a6caa7fc61a31bdeb 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153236

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

[PATCH] D153236: [NFC] Fix potential dereferencing of nullptr.

[Sindhu Chittireddy via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGd2fafa79ef08: [NFC] Fix potential dereferencing of nullptr. 
(authored by schittir).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153236

Files:
  clang/lib/Sema/SemaExprObjC.cpp
  clang/lib/Sema/SemaObjCProperty.cpp
  clang/lib/Sema/SemaType.cpp


Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -2708,8 +2708,8 @@
 return QualType();
   }
   // Only support _BitInt elements with byte-sized power of 2 NumBits.
-  if (CurType->isBitIntType()) {
-unsigned NumBits = CurType->getAs()->getNumBits();
+  if (const auto *BIT = CurType->getAs()) {
+unsigned NumBits = BIT->getNumBits();
 if (!llvm::isPowerOf2_32(NumBits) || NumBits < 8) {
   Diag(AttrLoc, diag::err_attribute_invalid_bitint_vector_type)
   << (NumBits < 8);
Index: clang/lib/Sema/SemaObjCProperty.cpp
===
--- clang/lib/Sema/SemaObjCProperty.cpp
+++ clang/lib/Sema/SemaObjCProperty.cpp
@@ -1363,10 +1363,9 @@
 if (!Context.hasSameType(PropertyIvarType, IvarType)) {
   if (isa(PropertyIvarType)
   && isa(IvarType))
-compat =
-  Context.canAssignObjCInterfaces(
-  
PropertyIvarType->getAs(),
-  IvarType->getAs());
+compat = Context.canAssignObjCInterfaces(
+PropertyIvarType->castAs(),
+IvarType->castAs());
   else {
 compat = (CheckAssignmentConstraints(PropertyIvarLoc, PropertyIvarType,
  IvarType)
Index: clang/lib/Sema/SemaExprObjC.cpp
===
--- clang/lib/Sema/SemaExprObjC.cpp
+++ clang/lib/Sema/SemaExprObjC.cpp
@@ -2438,6 +2438,9 @@
   if (!ReceiverType.isNull())
 receiverTypeInfo = Context.getTrivialTypeSourceInfo(ReceiverType);
 
+  assert(((isSuperReceiver && Loc.isValid()) || receiverTypeInfo) &&
+ "Either the super receiver location needs to be valid or the receiver 
"
+ "needs valid type source information");
   return BuildClassMessage(receiverTypeInfo, ReceiverType,
   /*SuperLoc=*/isSuperReceiver ? Loc : 
SourceLocation(),
Sel, Method, Loc, Loc, Loc, Args,


Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -2708,8 +2708,8 @@
 return QualType();
   }
   // Only support _BitInt elements with byte-sized power of 2 NumBits.
-  if (CurType->isBitIntType()) {
-unsigned NumBits = CurType->getAs()->getNumBits();
+  if (const auto *BIT = CurType->getAs()) {
+unsigned NumBits = BIT->getNumBits();
 if (!llvm::isPowerOf2_32(NumBits) || NumBits < 8) {
   Diag(AttrLoc, diag::err_attribute_invalid_bitint_vector_type)
   << (NumBits < 8);
Index: clang/lib/Sema/SemaObjCProperty.cpp
===
--- clang/lib/Sema/SemaObjCProperty.cpp
+++ clang/lib/Sema/SemaObjCProperty.cpp
@@ -1363,10 +1363,9 @@
 if (!Context.hasSameType(PropertyIvarType, IvarType)) {
   if (isa(PropertyIvarType)
   && isa(IvarType))
-compat =
-  Context.canAssignObjCInterfaces(
-  PropertyIvarType->getAs(),
-  IvarType->getAs());
+compat = Context.canAssignObjCInterfaces(
+PropertyIvarType->castAs(),
+IvarType->castAs());
   else {
 compat = (CheckAssignmentConstraints(PropertyIvarLoc, PropertyIvarType,
  IvarType)
Index: clang/lib/Sema/SemaExprObjC.cpp
===
--- clang/lib/Sema/SemaExprObjC.cpp
+++ clang/lib/Sema/SemaExprObjC.cpp
@@ -2438,6 +2438,9 @@
   if (!ReceiverType.isNull())
 receiverTypeInfo = Context.getTrivialTypeSourceInfo(ReceiverType);
 
+  assert(((isSuperReceiver && Loc.isValid()) || receiverTypeInfo) &&
+ "Either the super receiver location needs to be valid or the receiver "
+ "needs valid type source information");
   return BuildClassMessage(receiverTypeInfo, ReceiverType,
   /*SuperLoc=*/isSuperReceiver ? Loc : SourceLocation(),
Sel, Method, Loc, Loc, Loc, Args,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D153589: [NFC] Initialize pointer fields and remove a needless null check.

[Sindhu Chittireddy via Phabricator via cfe-commits]

schittir updated this revision to Diff 533794.
schittir added a comment.

Address comment- Use default member initialization instead of initializing in a 
constructor.


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

https://reviews.llvm.org/D153589

Files:
  clang/lib/Analysis/CFG.cpp
  clang/lib/CodeGen/CGObjCGNU.cpp
  clang/lib/Format/UnwrappedLineParser.h
  clang/lib/Lex/PPExpressions.cpp
  clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp


Index: clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -96,7 +96,7 @@
   mutable std::unique_ptr BT_Null, BT_Bounds, BT_Overlap,
   BT_NotCString, BT_AdditionOverflow, BT_UninitRead;
 
-  mutable const char *CurrentFunctionDescription;
+  mutable const char *CurrentFunctionDescription = nullptr;
 
 public:
   /// The filter is used to filter out the diagnostics which are not enabled by
Index: clang/lib/Lex/PPExpressions.cpp
===
--- clang/lib/Lex/PPExpressions.cpp
+++ clang/lib/Lex/PPExpressions.cpp
@@ -44,7 +44,7 @@
 /// conditional and the source range covered by it.
 class PPValue {
   SourceRange Range;
-  IdentifierInfo *II;
+  IdentifierInfo *II = nullptr;
 
 public:
   llvm::APSInt Val;
Index: clang/lib/Format/UnwrappedLineParser.h
===
--- clang/lib/Format/UnwrappedLineParser.h
+++ clang/lib/Format/UnwrappedLineParser.h
@@ -306,7 +306,7 @@
   // Since the next token might already be in a new unwrapped line, we need to
   // store the comments belonging to that token.
   SmallVector CommentsBeforeNextToken;
-  FormatToken *FormatTok;
+  FormatToken *FormatTok = nullptr;
   bool MustBreakBeforeNextToken;
 
   // The parsed lines. Only added to through \c CurrentLines.
Index: clang/lib/CodeGen/CGObjCGNU.cpp
===
--- clang/lib/CodeGen/CGObjCGNU.cpp
+++ clang/lib/CodeGen/CGObjCGNU.cpp
@@ -46,17 +46,16 @@
 /// types and the function declaration into a module if they're not used, and
 /// avoids constructing the type more than once if it's used more than once.
 class LazyRuntimeFunction {
-  CodeGenModule *CGM;
-  llvm::FunctionType *FTy;
-  const char *FunctionName;
-  llvm::FunctionCallee Function;
+  CodeGenModule *CGM = nullptr;
+  llvm::FunctionType *FTy = nullptr;
+  const char *FunctionName = nullptr;
+  llvm::FunctionCallee Function = nullptr;
 
 public:
   /// Constructor leaves this class uninitialized, because it is intended to
   /// be used as a field in another class and not all of the types that are
   /// used as arguments will necessarily be available at construction time.
-  LazyRuntimeFunction()
-  : CGM(nullptr), FunctionName(nullptr), Function(nullptr) {}
+  LazyRuntimeFunction() {}
 
   /// Initialises the lazy function with the name, return type, and the types
   /// of the arguments.
Index: clang/lib/Analysis/CFG.cpp
===
--- clang/lib/Analysis/CFG.cpp
+++ clang/lib/Analysis/CFG.cpp
@@ -4153,7 +4153,7 @@
   //   operand. [...]
   // We add only potentially evaluated statements to the block to avoid
   // CFG generation for unevaluated operands.
-  if (S && !S->isTypeDependent() && S->isPotentiallyEvaluated())
+  if (!S->isTypeDependent() && S->isPotentiallyEvaluated())
 return VisitChildren(S);
 
   // Return block without CFG for unevaluated operands.


Index: clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -96,7 +96,7 @@
   mutable std::unique_ptr BT_Null, BT_Bounds, BT_Overlap,
   BT_NotCString, BT_AdditionOverflow, BT_UninitRead;
 
-  mutable const char *CurrentFunctionDescription;
+  mutable const char *CurrentFunctionDescription = nullptr;
 
 public:
   /// The filter is used to filter out the diagnostics which are not enabled by
Index: clang/lib/Lex/PPExpressions.cpp
===
--- clang/lib/Lex/PPExpressions.cpp
+++ clang/lib/Lex/PPExpressions.cpp
@@ -44,7 +44,7 @@
 /// conditional and the source range covered by it.
 class PPValue {
   SourceRange Range;
-  IdentifierInfo *II;
+  IdentifierInfo *II = nullptr;
 
 public:
   llvm::APSInt Val;
Index: clang/lib/Format/UnwrappedLineParser.h
===
--- clang/lib/Format/UnwrappedLineParser.h
+++ clang/lib/Format/UnwrappedLineParser.h
@@ -306,7 +306,7 @@
   // Since the next token might already be in a new unwrapped line, we need to
   // store the comments belonging to that token.
   SmallVector 

[PATCH] D153589: [NFC] Initialize pointer fields and remove a needless null check.

[Sindhu Chittireddy via Phabricator via cfe-commits]

schittir added inline comments.



Comment at: clang/lib/CodeGen/CGObjCGNU.cpp:58
   /// used as arguments will necessarily be available at construction time.
-  LazyRuntimeFunction()
-  : CGM(nullptr), FunctionName(nullptr), Function(nullptr) {}
+  LazyRuntimeFunction() {}
 

erichkeane wrote:
> 
The comment above needs to be updated?


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

https://reviews.llvm.org/D153589

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

[PATCH] D153589: [NFC] Initialize pointer fields and remove a needless null check.

[Sindhu Chittireddy via Phabricator via cfe-commits]

schittir marked an inline comment as done.
schittir added inline comments.



Comment at: clang/lib/Analysis/CFG.cpp:4156
   // CFG generation for unevaluated operands.
-  if (S && !S->isTypeDependent() && S->isPotentiallyEvaluated())
+  if (!S->isTypeDependent() && S->isPotentiallyEvaluated())
 return VisitChildren(S);

erichkeane wrote:
> I get that we're counting on the dereference on 4145 to have made this check 
> irrelevant, but are we sure that we KNOW that "S" is non-null here otherwise? 
>  That is, is the bug actually 4145 doing 'alwaysAdd' without checking vs the 
> unnecessary check here?
VisitCXXTypeidExpr is used only in one place - here 
https://github.com/llvm/llvm-project/blob/a89c9b35be08b665cc1a01d840bc20349ba1308f/clang/lib/Analysis/CFG.cpp#L2288
 where S is not null. Null check for S already happens at the beginning of the 
method where VisitCXXTypeidExpr is called. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153589

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

[PATCH] D153589: [NFC] Initialize pointer fields and remove a needless null check.

[Sindhu Chittireddy via Phabricator via cfe-commits]

schittir added inline comments.



Comment at: clang/lib/Analysis/CFG.cpp:4156
   // CFG generation for unevaluated operands.
-  if (S && !S->isTypeDependent() && S->isPotentiallyEvaluated())
+  if (!S->isTypeDependent() && S->isPotentiallyEvaluated())
 return VisitChildren(S);

erichkeane wrote:
> schittir wrote:
> > erichkeane wrote:
> > > schittir wrote:
> > > > erichkeane wrote:
> > > > > I get that we're counting on the dereference on 4145 to have made 
> > > > > this check irrelevant, but are we sure that we KNOW that "S" is 
> > > > > non-null here otherwise?  That is, is the bug actually 4145 doing 
> > > > > 'alwaysAdd' without checking vs the unnecessary check here?
> > > > VisitCXXTypeidExpr is used only in one place - here 
> > > > https://github.com/llvm/llvm-project/blob/a89c9b35be08b665cc1a01d840bc20349ba1308f/clang/lib/Analysis/CFG.cpp#L2288
> > > >  where S is not null. Null check for S already happens at the beginning 
> > > > of the method where VisitCXXTypeidExpr is called. 
> > > SG, an assert is perhaps not a bad idea, but LGTM.  I don't know very 
> > > much about the CFG stuff, so aaron might wish to take a final look.
> > What is SG? 
> "Sounds Good"
Haha! 
Thanks for the review. 



Comment at: clang/lib/CodeGen/CGObjCGNU.cpp:58
   /// used as arguments will necessarily be available at construction time.
-  LazyRuntimeFunction()
-  : CGM(nullptr), FunctionName(nullptr), Function(nullptr) {}
+  LazyRuntimeFunction() {}
 

erichkeane wrote:
> schittir wrote:
> > erichkeane wrote:
> > > 
> > The comment above needs to be updated?
> I don't think so?  It still leaves the class uninitialized.
Right! Still valid. 


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

https://reviews.llvm.org/D153589

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

[PATCH] D153589: [NFC] Initialize pointer fields and remove a needless null check.

[Sindhu Chittireddy via Phabricator via cfe-commits]

schittir created this revision.
schittir added reviewers: aaron.ballman, erichkeane, tahonermann.
Herald added subscribers: cfe-commits, steakhal, martong.
Herald added a reviewer: NoQ.
Herald added projects: All, clang, clang-format.
Herald added reviewers: rymiel, HazardyKnusperkeks, owenpan, MyDeveloperDay.
schittir requested review of this revision.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D153589

Files:
  clang/lib/Analysis/CFG.cpp
  clang/lib/CodeGen/CGObjCGNU.cpp
  clang/lib/Format/UnwrappedLineParser.h
  clang/lib/Lex/PPExpressions.cpp
  clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp


Index: clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -96,7 +96,7 @@
   mutable std::unique_ptr BT_Null, BT_Bounds, BT_Overlap,
   BT_NotCString, BT_AdditionOverflow, BT_UninitRead;
 
-  mutable const char *CurrentFunctionDescription;
+  mutable const char *CurrentFunctionDescription = nullptr;
 
 public:
   /// The filter is used to filter out the diagnostics which are not enabled by
Index: clang/lib/Lex/PPExpressions.cpp
===
--- clang/lib/Lex/PPExpressions.cpp
+++ clang/lib/Lex/PPExpressions.cpp
@@ -44,7 +44,7 @@
 /// conditional and the source range covered by it.
 class PPValue {
   SourceRange Range;
-  IdentifierInfo *II;
+  IdentifierInfo *II = nullptr;
 
 public:
   llvm::APSInt Val;
Index: clang/lib/Format/UnwrappedLineParser.h
===
--- clang/lib/Format/UnwrappedLineParser.h
+++ clang/lib/Format/UnwrappedLineParser.h
@@ -306,7 +306,7 @@
   // Since the next token might already be in a new unwrapped line, we need to
   // store the comments belonging to that token.
   SmallVector CommentsBeforeNextToken;
-  FormatToken *FormatTok;
+  FormatToken *FormatTok = nullptr;
   bool MustBreakBeforeNextToken;
 
   // The parsed lines. Only added to through \c CurrentLines.
Index: clang/lib/CodeGen/CGObjCGNU.cpp
===
--- clang/lib/CodeGen/CGObjCGNU.cpp
+++ clang/lib/CodeGen/CGObjCGNU.cpp
@@ -56,7 +56,7 @@
   /// be used as a field in another class and not all of the types that are
   /// used as arguments will necessarily be available at construction time.
   LazyRuntimeFunction()
-  : CGM(nullptr), FunctionName(nullptr), Function(nullptr) {}
+  : CGM(nullptr), FTy(nullptr), FunctionName(nullptr), Function(nullptr) {}
 
   /// Initialises the lazy function with the name, return type, and the types
   /// of the arguments.
Index: clang/lib/Analysis/CFG.cpp
===
--- clang/lib/Analysis/CFG.cpp
+++ clang/lib/Analysis/CFG.cpp
@@ -4153,7 +4153,7 @@
   //   operand. [...]
   // We add only potentially evaluated statements to the block to avoid
   // CFG generation for unevaluated operands.
-  if (S && !S->isTypeDependent() && S->isPotentiallyEvaluated())
+  if (!S->isTypeDependent() && S->isPotentiallyEvaluated())
 return VisitChildren(S);
 
   // Return block without CFG for unevaluated operands.


Index: clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -96,7 +96,7 @@
   mutable std::unique_ptr BT_Null, BT_Bounds, BT_Overlap,
   BT_NotCString, BT_AdditionOverflow, BT_UninitRead;
 
-  mutable const char *CurrentFunctionDescription;
+  mutable const char *CurrentFunctionDescription = nullptr;
 
 public:
   /// The filter is used to filter out the diagnostics which are not enabled by
Index: clang/lib/Lex/PPExpressions.cpp
===
--- clang/lib/Lex/PPExpressions.cpp
+++ clang/lib/Lex/PPExpressions.cpp
@@ -44,7 +44,7 @@
 /// conditional and the source range covered by it.
 class PPValue {
   SourceRange Range;
-  IdentifierInfo *II;
+  IdentifierInfo *II = nullptr;
 
 public:
   llvm::APSInt Val;
Index: clang/lib/Format/UnwrappedLineParser.h
===
--- clang/lib/Format/UnwrappedLineParser.h
+++ clang/lib/Format/UnwrappedLineParser.h
@@ -306,7 +306,7 @@
   // Since the next token might already be in a new unwrapped line, we need to
   // store the comments belonging to that token.
   SmallVector CommentsBeforeNextToken;
-  FormatToken *FormatTok;
+  FormatToken *FormatTok = nullptr;
   bool MustBreakBeforeNextToken;
 
   // The parsed lines. Only added to through \c CurrentLines.
Index: clang/lib/CodeGen/CGObjCGNU.cpp
===
--- clang/lib/CodeGen/CGObjCGNU.cpp
+++ 

[PATCH] D153589: [NFC] Initialize pointer fields and remove a needless null check.

[Sindhu Chittireddy via Phabricator via cfe-commits]

schittir updated this revision to Diff 533798.
schittir added a comment.

Make the constructor default.


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

https://reviews.llvm.org/D153589

Files:
  clang/lib/Analysis/CFG.cpp
  clang/lib/CodeGen/CGObjCGNU.cpp
  clang/lib/Format/UnwrappedLineParser.h
  clang/lib/Lex/PPExpressions.cpp
  clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp


Index: clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -96,7 +96,7 @@
   mutable std::unique_ptr BT_Null, BT_Bounds, BT_Overlap,
   BT_NotCString, BT_AdditionOverflow, BT_UninitRead;
 
-  mutable const char *CurrentFunctionDescription;
+  mutable const char *CurrentFunctionDescription = nullptr;
 
 public:
   /// The filter is used to filter out the diagnostics which are not enabled by
Index: clang/lib/Lex/PPExpressions.cpp
===
--- clang/lib/Lex/PPExpressions.cpp
+++ clang/lib/Lex/PPExpressions.cpp
@@ -44,7 +44,7 @@
 /// conditional and the source range covered by it.
 class PPValue {
   SourceRange Range;
-  IdentifierInfo *II;
+  IdentifierInfo *II = nullptr;
 
 public:
   llvm::APSInt Val;
Index: clang/lib/Format/UnwrappedLineParser.h
===
--- clang/lib/Format/UnwrappedLineParser.h
+++ clang/lib/Format/UnwrappedLineParser.h
@@ -306,7 +306,7 @@
   // Since the next token might already be in a new unwrapped line, we need to
   // store the comments belonging to that token.
   SmallVector CommentsBeforeNextToken;
-  FormatToken *FormatTok;
+  FormatToken *FormatTok = nullptr;
   bool MustBreakBeforeNextToken;
 
   // The parsed lines. Only added to through \c CurrentLines.
Index: clang/lib/CodeGen/CGObjCGNU.cpp
===
--- clang/lib/CodeGen/CGObjCGNU.cpp
+++ clang/lib/CodeGen/CGObjCGNU.cpp
@@ -46,17 +46,16 @@
 /// types and the function declaration into a module if they're not used, and
 /// avoids constructing the type more than once if it's used more than once.
 class LazyRuntimeFunction {
-  CodeGenModule *CGM;
-  llvm::FunctionType *FTy;
-  const char *FunctionName;
-  llvm::FunctionCallee Function;
+  CodeGenModule *CGM = nullptr;
+  llvm::FunctionType *FTy = nullptr;
+  const char *FunctionName = nullptr;
+  llvm::FunctionCallee Function = nullptr;
 
 public:
   /// Constructor leaves this class uninitialized, because it is intended to
   /// be used as a field in another class and not all of the types that are
   /// used as arguments will necessarily be available at construction time.
-  LazyRuntimeFunction()
-  : CGM(nullptr), FunctionName(nullptr), Function(nullptr) {}
+  LazyRuntimeFunction() = default;
 
   /// Initialises the lazy function with the name, return type, and the types
   /// of the arguments.
Index: clang/lib/Analysis/CFG.cpp
===
--- clang/lib/Analysis/CFG.cpp
+++ clang/lib/Analysis/CFG.cpp
@@ -4153,7 +4153,7 @@
   //   operand. [...]
   // We add only potentially evaluated statements to the block to avoid
   // CFG generation for unevaluated operands.
-  if (S && !S->isTypeDependent() && S->isPotentiallyEvaluated())
+  if (!S->isTypeDependent() && S->isPotentiallyEvaluated())
 return VisitChildren(S);
 
   // Return block without CFG for unevaluated operands.


Index: clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -96,7 +96,7 @@
   mutable std::unique_ptr BT_Null, BT_Bounds, BT_Overlap,
   BT_NotCString, BT_AdditionOverflow, BT_UninitRead;
 
-  mutable const char *CurrentFunctionDescription;
+  mutable const char *CurrentFunctionDescription = nullptr;
 
 public:
   /// The filter is used to filter out the diagnostics which are not enabled by
Index: clang/lib/Lex/PPExpressions.cpp
===
--- clang/lib/Lex/PPExpressions.cpp
+++ clang/lib/Lex/PPExpressions.cpp
@@ -44,7 +44,7 @@
 /// conditional and the source range covered by it.
 class PPValue {
   SourceRange Range;
-  IdentifierInfo *II;
+  IdentifierInfo *II = nullptr;
 
 public:
   llvm::APSInt Val;
Index: clang/lib/Format/UnwrappedLineParser.h
===
--- clang/lib/Format/UnwrappedLineParser.h
+++ clang/lib/Format/UnwrappedLineParser.h
@@ -306,7 +306,7 @@
   // Since the next token might already be in a new unwrapped line, we need to
   // store the comments belonging to that token.
   SmallVector CommentsBeforeNextToken;
-  FormatToken *FormatTok;
+  FormatToken 

[PATCH] D153589: [NFC] Initialize pointer fields and remove a needless null check.

[Sindhu Chittireddy via Phabricator via cfe-commits]

schittir added inline comments.



Comment at: clang/lib/Analysis/CFG.cpp:4156
   // CFG generation for unevaluated operands.
-  if (S && !S->isTypeDependent() && S->isPotentiallyEvaluated())
+  if (!S->isTypeDependent() && S->isPotentiallyEvaluated())
 return VisitChildren(S);

erichkeane wrote:
> schittir wrote:
> > erichkeane wrote:
> > > I get that we're counting on the dereference on 4145 to have made this 
> > > check irrelevant, but are we sure that we KNOW that "S" is non-null here 
> > > otherwise?  That is, is the bug actually 4145 doing 'alwaysAdd' without 
> > > checking vs the unnecessary check here?
> > VisitCXXTypeidExpr is used only in one place - here 
> > https://github.com/llvm/llvm-project/blob/a89c9b35be08b665cc1a01d840bc20349ba1308f/clang/lib/Analysis/CFG.cpp#L2288
> >  where S is not null. Null check for S already happens at the beginning of 
> > the method where VisitCXXTypeidExpr is called. 
> SG, an assert is perhaps not a bad idea, but LGTM.  I don't know very much 
> about the CFG stuff, so aaron might wish to take a final look.
What is SG? 


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

https://reviews.llvm.org/D153589

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

[PATCH] D153926: [NFC] Initialize class member pointers to nullptr.

[Sindhu Chittireddy via Phabricator via cfe-commits]

schittir updated this revision to Diff 535466.
schittir added a comment.

Not sure what clang-format wants. I hope it likes this patch.


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

https://reviews.llvm.org/D153926

Files:
  clang/include/clang/AST/RawCommentList.h
  clang/include/clang/AST/Redeclarable.h
  clang/include/clang/CodeGen/CodeGenAction.h
  clang/include/clang/Sema/HLSLExternalSemaSource.h
  clang/include/clang/Sema/Sema.h
  clang/include/clang/Serialization/ModuleFile.h
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/Format/SortJavaScriptImports.cpp
  clang/lib/Serialization/ASTReaderDecl.cpp
  clang/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp
  clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLambdaCapturesChecker.cpp

Index: clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLambdaCapturesChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLambdaCapturesChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLambdaCapturesChecker.cpp
@@ -25,7 +25,7 @@
 private:
   BugType Bug{this, "Lambda capture of uncounted variable",
   "WebKit coding guidelines"};
-  mutable BugReporter *BR;
+  mutable BugReporter *BR = nullptr;
 
 public:
   void checkASTDecl(const TranslationUnitDecl *TUD, AnalysisManager ,
Index: clang/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp
+++ clang/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp
@@ -754,7 +754,7 @@
 
 ObjCDeallocChecker::ObjCDeallocChecker()
 : NSObjectII(nullptr), SenTestCaseII(nullptr), XCTestCaseII(nullptr),
-  CIFilterII(nullptr) {
+  Block_releaseII(nullptr), CIFilterII(nullptr) {
 
   MissingReleaseBugType.reset(
   new BugType(this, "Missing ivar release (leak)",
Index: clang/lib/Serialization/ASTReaderDecl.cpp
===
--- clang/lib/Serialization/ASTReaderDecl.cpp
+++ clang/lib/Serialization/ASTReaderDecl.cpp
@@ -476,9 +476,8 @@
 
 /// Iterator over the redeclarations of a declaration that have already
 /// been merged into the same redeclaration chain.
-template
-class MergedRedeclIterator {
-  DeclT *Start;
+template  class MergedRedeclIterator {
+  DeclT *Start = nullptr;
   DeclT *Canonical = nullptr;
   DeclT *Current = nullptr;
 
Index: clang/lib/Format/SortJavaScriptImports.cpp
===
--- clang/lib/Format/SortJavaScriptImports.cpp
+++ clang/lib/Format/SortJavaScriptImports.cpp
@@ -217,8 +217,8 @@
   }
 
 private:
-  FormatToken *Current;
-  FormatToken *LineEnd;
+  FormatToken *Current = nullptr;
+  FormatToken *LineEnd = nullptr;
 
   FormatToken invalidToken;
 
Index: clang/lib/CodeGen/CodeGenFunction.h
===
--- clang/lib/CodeGen/CodeGenFunction.h
+++ clang/lib/CodeGen/CodeGenFunction.h
@@ -318,10 +318,10 @@
 
   /// CurFuncDecl - Holds the Decl for the current outermost
   /// non-closure context.
-  const Decl *CurFuncDecl;
+  const Decl *CurFuncDecl = nullptr;
   /// CurCodeDecl - This is the inner-most code context, which includes blocks.
-  const Decl *CurCodeDecl;
-  const CGFunctionInfo *CurFnInfo;
+  const Decl *CurCodeDecl = nullptr;
+  const CGFunctionInfo *CurFnInfo = nullptr;
   QualType FnRetTy;
   llvm::Function *CurFn = nullptr;
 
@@ -748,11 +748,11 @@
 
 /// An i1 variable indicating whether or not the @finally is
 /// running for an exception.
-llvm::AllocaInst *ForEHVar;
+llvm::AllocaInst *ForEHVar = nullptr;
 
 /// An i8* variable into which the exception pointer to rethrow
 /// has been saved.
-llvm::AllocaInst *SavedExnVar;
+llvm::AllocaInst *SavedExnVar = nullptr;
 
   public:
 void enter(CodeGenFunction , const Stmt *Finally,
Index: clang/include/clang/Serialization/ModuleFile.h
===
--- clang/include/clang/Serialization/ModuleFile.h
+++ clang/include/clang/Serialization/ModuleFile.h
@@ -196,7 +196,7 @@
 
   /// The memory buffer that stores the data associated with
   /// this AST file, owned by the InMemoryModuleCache.
-  llvm::MemoryBuffer *Buffer;
+  llvm::MemoryBuffer *Buffer = nullptr;
 
   /// The size of this file, in bits.
   uint64_t SizeInBits = 0;
Index: clang/include/clang/Sema/Sema.h
===
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -937,7 +937,7 @@
   class DelayedDiagnostics;
 
   class DelayedDiagnosticsState {
-sema::DelayedDiagnosticPool *SavedPool;
+sema::DelayedDiagnosticPool *SavedPool = nullptr;
 friend class Sema::DelayedDiagnostics;
   };
   typedef DelayedDiagnosticsState ParsingDeclState;
Index: 

[PATCH] D153892: [NFC] Initialize class member pointers to nullptr.

[Sindhu Chittireddy via Phabricator via cfe-commits]

schittir updated this revision to Diff 535459.
schittir added a comment.

Hope clang-format likes this patch!


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

https://reviews.llvm.org/D153892

Files:
  clang/lib/ARCMigrate/TransProperties.cpp
  clang/lib/AST/ExprConstant.cpp
  clang/lib/Frontend/ASTConsumers.cpp
  clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp


Index: clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
@@ -29,7 +29,7 @@
 class StackAddrEscapeChecker
 : public Checker,
  check::EndFunction> {
-  mutable IdentifierInfo *dispatch_semaphore_tII;
+  mutable IdentifierInfo *dispatch_semaphore_tII = nullptr;
   mutable std::unique_ptr BT_stackleak;
   mutable std::unique_ptr BT_returnstack;
   mutable std::unique_ptr BT_capturedstackasync;
Index: clang/lib/Frontend/ASTConsumers.cpp
===
--- clang/lib/Frontend/ASTConsumers.cpp
+++ clang/lib/Frontend/ASTConsumers.cpp
@@ -184,13 +184,12 @@
 
 namespace {
   class ASTViewer : public ASTConsumer {
-ASTContext *Context;
-  public:
-void Initialize(ASTContext ) override {
-  this->Context = 
-}
+  ASTContext *Context = nullptr;
+
+public:
+  void Initialize(ASTContext ) override { this->Context =  }
 
-bool HandleTopLevelDecl(DeclGroupRef D) override {
+  bool HandleTopLevelDecl(DeclGroupRef D) override {
   for (DeclGroupRef::iterator I = D.begin(), E = D.end(); I != E; ++I)
 HandleTopLevelSingleDecl(*I);
   return true;
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -580,7 +580,7 @@
 /// LambdaCaptureFields - Mapping from captured variables/this to
 /// corresponding data members in the closure class.
 llvm::DenseMap LambdaCaptureFields;
-FieldDecl *LambdaThisCaptureField;
+FieldDecl *LambdaThisCaptureField = nullptr;
 
 CallStackFrame(EvalInfo , SourceLocation CallLoc,
const FunctionDecl *Callee, const LValue *This,
Index: clang/lib/ARCMigrate/TransProperties.cpp
===
--- clang/lib/ARCMigrate/TransProperties.cpp
+++ clang/lib/ARCMigrate/TransProperties.cpp
@@ -45,7 +45,7 @@
 class PropertiesRewriter {
   MigrationContext 
   MigrationPass 
-  ObjCImplementationDecl *CurImplD;
+  ObjCImplementationDecl *CurImplD = nullptr;
 
   enum PropActionKind {
 PropAction_None,


Index: clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
@@ -29,7 +29,7 @@
 class StackAddrEscapeChecker
 : public Checker,
  check::EndFunction> {
-  mutable IdentifierInfo *dispatch_semaphore_tII;
+  mutable IdentifierInfo *dispatch_semaphore_tII = nullptr;
   mutable std::unique_ptr BT_stackleak;
   mutable std::unique_ptr BT_returnstack;
   mutable std::unique_ptr BT_capturedstackasync;
Index: clang/lib/Frontend/ASTConsumers.cpp
===
--- clang/lib/Frontend/ASTConsumers.cpp
+++ clang/lib/Frontend/ASTConsumers.cpp
@@ -184,13 +184,12 @@
 
 namespace {
   class ASTViewer : public ASTConsumer {
-ASTContext *Context;
-  public:
-void Initialize(ASTContext ) override {
-  this->Context = 
-}
+  ASTContext *Context = nullptr;
+
+public:
+  void Initialize(ASTContext ) override { this->Context =  }
 
-bool HandleTopLevelDecl(DeclGroupRef D) override {
+  bool HandleTopLevelDecl(DeclGroupRef D) override {
   for (DeclGroupRef::iterator I = D.begin(), E = D.end(); I != E; ++I)
 HandleTopLevelSingleDecl(*I);
   return true;
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -580,7 +580,7 @@
 /// LambdaCaptureFields - Mapping from captured variables/this to
 /// corresponding data members in the closure class.
 llvm::DenseMap LambdaCaptureFields;
-FieldDecl *LambdaThisCaptureField;
+FieldDecl *LambdaThisCaptureField = nullptr;
 
 CallStackFrame(EvalInfo , SourceLocation CallLoc,
const FunctionDecl *Callee, const LValue *This,
Index: clang/lib/ARCMigrate/TransProperties.cpp
===
--- clang/lib/ARCMigrate/TransProperties.cpp
+++ clang/lib/ARCMigrate/TransProperties.cpp
@@ -45,7 +45,7 @@
 class PropertiesRewriter {
   MigrationContext 
   

[PATCH] D153236: [NFC] Fix potential dereferencing of nullptr.

[Sindhu Chittireddy via Phabricator via cfe-commits]

schittir created this revision.
schittir added reviewers: aaron.ballman, tahonermann, erichkeane.
Herald added a project: All.
schittir requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Replace getAs with castAs and add assert if needed.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D153236

Files:
  clang/lib/Parse/ParseStmt.cpp
  clang/lib/Sema/SemaExprObjC.cpp
  clang/lib/Sema/SemaObjCProperty.cpp
  clang/lib/Sema/SemaType.cpp


Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -2709,7 +2709,7 @@
   }
   // Only support _BitInt elements with byte-sized power of 2 NumBits.
   if (CurType->isBitIntType()) {
-unsigned NumBits = CurType->getAs()->getNumBits();
+unsigned NumBits = CurType->casttAs()->getNumBits();
 if (!llvm::isPowerOf2_32(NumBits) || NumBits < 8) {
   Diag(AttrLoc, diag::err_attribute_invalid_bitint_vector_type)
   << (NumBits < 8);
Index: clang/lib/Sema/SemaObjCProperty.cpp
===
--- clang/lib/Sema/SemaObjCProperty.cpp
+++ clang/lib/Sema/SemaObjCProperty.cpp
@@ -1363,10 +1363,9 @@
 if (!Context.hasSameType(PropertyIvarType, IvarType)) {
   if (isa(PropertyIvarType)
   && isa(IvarType))
-compat =
-  Context.canAssignObjCInterfaces(
-  
PropertyIvarType->getAs(),
-  IvarType->getAs());
+compat = Context.canAssignObjCInterfaces(
+PropertyIvarType->castAs(),
+IvarType->castAs());
   else {
 compat = (CheckAssignmentConstraints(PropertyIvarLoc, PropertyIvarType,
  IvarType)
Index: clang/lib/Sema/SemaExprObjC.cpp
===
--- clang/lib/Sema/SemaExprObjC.cpp
+++ clang/lib/Sema/SemaExprObjC.cpp
@@ -2438,6 +2438,7 @@
   if (!ReceiverType.isNull())
 receiverTypeInfo = Context.getTrivialTypeSourceInfo(ReceiverType);
 
+  assert(receiverTypeInfo && "receiverTypeInfo cannot be null");
   return BuildClassMessage(receiverTypeInfo, ReceiverType,
   /*SuperLoc=*/isSuperReceiver ? Loc : 
SourceLocation(),
Sel, Method, Loc, Loc, Loc, Args,
Index: clang/lib/Parse/ParseStmt.cpp
===
--- clang/lib/Parse/ParseStmt.cpp
+++ clang/lib/Parse/ParseStmt.cpp
@@ -883,8 +883,10 @@
   Stmt *NextDeepest = Case.get();
   if (TopLevelCase.isInvalid())
 TopLevelCase = Case;
-  else
+  else {
+assert(DeepestParsedCaseStmt && "DeepestParsedCaseStmt cannot be 
null");
 Actions.ActOnCaseStmtBody(DeepestParsedCaseStmt, Case.get());
+  }
   DeepestParsedCaseStmt = NextDeepest;
 }
 


Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -2709,7 +2709,7 @@
   }
   // Only support _BitInt elements with byte-sized power of 2 NumBits.
   if (CurType->isBitIntType()) {
-unsigned NumBits = CurType->getAs()->getNumBits();
+unsigned NumBits = CurType->casttAs()->getNumBits();
 if (!llvm::isPowerOf2_32(NumBits) || NumBits < 8) {
   Diag(AttrLoc, diag::err_attribute_invalid_bitint_vector_type)
   << (NumBits < 8);
Index: clang/lib/Sema/SemaObjCProperty.cpp
===
--- clang/lib/Sema/SemaObjCProperty.cpp
+++ clang/lib/Sema/SemaObjCProperty.cpp
@@ -1363,10 +1363,9 @@
 if (!Context.hasSameType(PropertyIvarType, IvarType)) {
   if (isa(PropertyIvarType)
   && isa(IvarType))
-compat =
-  Context.canAssignObjCInterfaces(
-  PropertyIvarType->getAs(),
-  IvarType->getAs());
+compat = Context.canAssignObjCInterfaces(
+PropertyIvarType->castAs(),
+IvarType->castAs());
   else {
 compat = (CheckAssignmentConstraints(PropertyIvarLoc, PropertyIvarType,
  IvarType)
Index: clang/lib/Sema/SemaExprObjC.cpp
===
--- clang/lib/Sema/SemaExprObjC.cpp
+++ clang/lib/Sema/SemaExprObjC.cpp
@@ -2438,6 +2438,7 @@
   if (!ReceiverType.isNull())
 receiverTypeInfo = Context.getTrivialTypeSourceInfo(ReceiverType);
 
+  assert(receiverTypeInfo && "receiverTypeInfo cannot be null");
   return BuildClassMessage(receiverTypeInfo, ReceiverType,
   /*SuperLoc=*/isSuperReceiver ? Loc : SourceLocation(),
Sel, Method, Loc, Loc, Loc, Args,
Index: clang/lib/Parse/ParseStmt.cpp

[PATCH] D153236: [NFC] Fix potential dereferencing of nullptr.

[Sindhu Chittireddy via Phabricator via cfe-commits]

schittir updated this revision to Diff 532528.
schittir added a comment.

Fix typo


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

https://reviews.llvm.org/D153236

Files:
  clang/lib/Parse/ParseStmt.cpp
  clang/lib/Sema/SemaExprObjC.cpp
  clang/lib/Sema/SemaObjCProperty.cpp
  clang/lib/Sema/SemaType.cpp


Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -2709,7 +2709,7 @@
   }
   // Only support _BitInt elements with byte-sized power of 2 NumBits.
   if (CurType->isBitIntType()) {
-unsigned NumBits = CurType->getAs()->getNumBits();
+unsigned NumBits = CurType->castAs()->getNumBits();
 if (!llvm::isPowerOf2_32(NumBits) || NumBits < 8) {
   Diag(AttrLoc, diag::err_attribute_invalid_bitint_vector_type)
   << (NumBits < 8);
Index: clang/lib/Sema/SemaObjCProperty.cpp
===
--- clang/lib/Sema/SemaObjCProperty.cpp
+++ clang/lib/Sema/SemaObjCProperty.cpp
@@ -1363,10 +1363,9 @@
 if (!Context.hasSameType(PropertyIvarType, IvarType)) {
   if (isa(PropertyIvarType)
   && isa(IvarType))
-compat =
-  Context.canAssignObjCInterfaces(
-  
PropertyIvarType->getAs(),
-  IvarType->getAs());
+compat = Context.canAssignObjCInterfaces(
+PropertyIvarType->castAs(),
+IvarType->castAs());
   else {
 compat = (CheckAssignmentConstraints(PropertyIvarLoc, PropertyIvarType,
  IvarType)
Index: clang/lib/Sema/SemaExprObjC.cpp
===
--- clang/lib/Sema/SemaExprObjC.cpp
+++ clang/lib/Sema/SemaExprObjC.cpp
@@ -2438,6 +2438,7 @@
   if (!ReceiverType.isNull())
 receiverTypeInfo = Context.getTrivialTypeSourceInfo(ReceiverType);
 
+  assert(receiverTypeInfo && "receiverTypeInfo cannot be null");
   return BuildClassMessage(receiverTypeInfo, ReceiverType,
   /*SuperLoc=*/isSuperReceiver ? Loc : 
SourceLocation(),
Sel, Method, Loc, Loc, Loc, Args,
Index: clang/lib/Parse/ParseStmt.cpp
===
--- clang/lib/Parse/ParseStmt.cpp
+++ clang/lib/Parse/ParseStmt.cpp
@@ -883,8 +883,10 @@
   Stmt *NextDeepest = Case.get();
   if (TopLevelCase.isInvalid())
 TopLevelCase = Case;
-  else
+  else {
+assert(DeepestParsedCaseStmt && "DeepestParsedCaseStmt cannot be 
null");
 Actions.ActOnCaseStmtBody(DeepestParsedCaseStmt, Case.get());
+  }
   DeepestParsedCaseStmt = NextDeepest;
 }
 


Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -2709,7 +2709,7 @@
   }
   // Only support _BitInt elements with byte-sized power of 2 NumBits.
   if (CurType->isBitIntType()) {
-unsigned NumBits = CurType->getAs()->getNumBits();
+unsigned NumBits = CurType->castAs()->getNumBits();
 if (!llvm::isPowerOf2_32(NumBits) || NumBits < 8) {
   Diag(AttrLoc, diag::err_attribute_invalid_bitint_vector_type)
   << (NumBits < 8);
Index: clang/lib/Sema/SemaObjCProperty.cpp
===
--- clang/lib/Sema/SemaObjCProperty.cpp
+++ clang/lib/Sema/SemaObjCProperty.cpp
@@ -1363,10 +1363,9 @@
 if (!Context.hasSameType(PropertyIvarType, IvarType)) {
   if (isa(PropertyIvarType)
   && isa(IvarType))
-compat =
-  Context.canAssignObjCInterfaces(
-  PropertyIvarType->getAs(),
-  IvarType->getAs());
+compat = Context.canAssignObjCInterfaces(
+PropertyIvarType->castAs(),
+IvarType->castAs());
   else {
 compat = (CheckAssignmentConstraints(PropertyIvarLoc, PropertyIvarType,
  IvarType)
Index: clang/lib/Sema/SemaExprObjC.cpp
===
--- clang/lib/Sema/SemaExprObjC.cpp
+++ clang/lib/Sema/SemaExprObjC.cpp
@@ -2438,6 +2438,7 @@
   if (!ReceiverType.isNull())
 receiverTypeInfo = Context.getTrivialTypeSourceInfo(ReceiverType);
 
+  assert(receiverTypeInfo && "receiverTypeInfo cannot be null");
   return BuildClassMessage(receiverTypeInfo, ReceiverType,
   /*SuperLoc=*/isSuperReceiver ? Loc : SourceLocation(),
Sel, Method, Loc, Loc, Loc, Args,
Index: clang/lib/Parse/ParseStmt.cpp
===
--- clang/lib/Parse/ParseStmt.cpp
+++ clang/lib/Parse/ParseStmt.cpp
@@ -883,8 +883,10 @@
   Stmt *NextDeepest = Case.get();
   if 

[PATCH] D153589: [NFC] Initialize pointer fields and remove a needless null check.

[Sindhu Chittireddy via Phabricator via cfe-commits]

schittir added inline comments.



Comment at: clang/lib/Analysis/CFG.cpp:4156
   // CFG generation for unevaluated operands.
-  if (S && !S->isTypeDependent() && S->isPotentiallyEvaluated())
+  if (!S->isTypeDependent() && S->isPotentiallyEvaluated())
 return VisitChildren(S);

schittir wrote:
> erichkeane wrote:
> > schittir wrote:
> > > erichkeane wrote:
> > > > schittir wrote:
> > > > > erichkeane wrote:
> > > > > > I get that we're counting on the dereference on 4145 to have made 
> > > > > > this check irrelevant, but are we sure that we KNOW that "S" is 
> > > > > > non-null here otherwise?  That is, is the bug actually 4145 doing 
> > > > > > 'alwaysAdd' without checking vs the unnecessary check here?
> > > > > VisitCXXTypeidExpr is used only in one place - here 
> > > > > https://github.com/llvm/llvm-project/blob/a89c9b35be08b665cc1a01d840bc20349ba1308f/clang/lib/Analysis/CFG.cpp#L2288
> > > > >  where S is not null. Null check for S already happens at the 
> > > > > beginning of the method where VisitCXXTypeidExpr is called. 
> > > > SG, an assert is perhaps not a bad idea, but LGTM.  I don't know very 
> > > > much about the CFG stuff, so aaron might wish to take a final look.
> > > What is SG? 
> > "Sounds Good"
> Haha! 
> Thanks for the review. 
> SG, an assert is perhaps not a bad idea, but LGTM.  I don't know very much 
> about the CFG stuff, so aaron might wish to take a final look.

Hi @aaron.ballman, could you please comment on this? Thank you!


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

https://reviews.llvm.org/D153589

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

[PATCH] D153926: [NFC] Initialize class member pointers to nullptr.

[Sindhu Chittireddy via Phabricator via cfe-commits]

schittir created this revision.
schittir added reviewers: erichkeane, aaron.ballman, tahonermann, shafik.
Herald added subscribers: cfe-commits, steakhal, martong.
Herald added a reviewer: NoQ.
Herald added projects: All, clang, clang-format.
Herald added reviewers: rymiel, HazardyKnusperkeks, owenpan, MyDeveloperDay.
schittir requested review of this revision.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D153926

Files:
  clang/include/clang/AST/RawCommentList.h
  clang/include/clang/AST/Redeclarable.h
  clang/include/clang/CodeGen/CodeGenAction.h
  clang/include/clang/Sema/HLSLExternalSemaSource.h
  clang/include/clang/Sema/Sema.h
  clang/include/clang/Serialization/ModuleFile.h
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/Format/SortJavaScriptImports.cpp
  clang/lib/Serialization/ASTReaderDecl.cpp
  clang/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp
  clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLambdaCapturesChecker.cpp

Index: clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLambdaCapturesChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLambdaCapturesChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLambdaCapturesChecker.cpp
@@ -25,7 +25,7 @@
 private:
   BugType Bug{this, "Lambda capture of uncounted variable",
   "WebKit coding guidelines"};
-  mutable BugReporter *BR;
+  mutable BugReporter *BR = nullptr;
 
 public:
   void checkASTDecl(const TranslationUnitDecl *TUD, AnalysisManager ,
Index: clang/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp
+++ clang/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp
@@ -754,7 +754,7 @@
 
 ObjCDeallocChecker::ObjCDeallocChecker()
 : NSObjectII(nullptr), SenTestCaseII(nullptr), XCTestCaseII(nullptr),
-  CIFilterII(nullptr) {
+  Block_releaseII(nullptr), CIFilterII(nullptr) {
 
   MissingReleaseBugType.reset(
   new BugType(this, "Missing ivar release (leak)",
Index: clang/lib/Serialization/ASTReaderDecl.cpp
===
--- clang/lib/Serialization/ASTReaderDecl.cpp
+++ clang/lib/Serialization/ASTReaderDecl.cpp
@@ -478,7 +478,7 @@
 /// been merged into the same redeclaration chain.
 template
 class MergedRedeclIterator {
-  DeclT *Start;
+  DeclT *Start = nullptr;
   DeclT *Canonical = nullptr;
   DeclT *Current = nullptr;
 
Index: clang/lib/Format/SortJavaScriptImports.cpp
===
--- clang/lib/Format/SortJavaScriptImports.cpp
+++ clang/lib/Format/SortJavaScriptImports.cpp
@@ -217,8 +217,8 @@
   }
 
 private:
-  FormatToken *Current;
-  FormatToken *LineEnd;
+  FormatToken *Current = nullptr;
+  FormatToken *LineEnd = nullptr;
 
   FormatToken invalidToken;
 
Index: clang/lib/CodeGen/CodeGenFunction.h
===
--- clang/lib/CodeGen/CodeGenFunction.h
+++ clang/lib/CodeGen/CodeGenFunction.h
@@ -318,10 +318,10 @@
 
   /// CurFuncDecl - Holds the Decl for the current outermost
   /// non-closure context.
-  const Decl *CurFuncDecl;
+  const Decl *CurFuncDecl = nullptr;
   /// CurCodeDecl - This is the inner-most code context, which includes blocks.
-  const Decl *CurCodeDecl;
-  const CGFunctionInfo *CurFnInfo;
+  const Decl *CurCodeDecl = nullptr;
+  const CGFunctionInfo *CurFnInfo = nullptr;
   QualType FnRetTy;
   llvm::Function *CurFn = nullptr;
 
@@ -748,11 +748,11 @@
 
 /// An i1 variable indicating whether or not the @finally is
 /// running for an exception.
-llvm::AllocaInst *ForEHVar;
+llvm::AllocaInst *ForEHVar = nullptr;
 
 /// An i8* variable into which the exception pointer to rethrow
 /// has been saved.
-llvm::AllocaInst *SavedExnVar;
+llvm::AllocaInst *SavedExnVar = nullptr;
 
   public:
 void enter(CodeGenFunction , const Stmt *Finally,
Index: clang/include/clang/Serialization/ModuleFile.h
===
--- clang/include/clang/Serialization/ModuleFile.h
+++ clang/include/clang/Serialization/ModuleFile.h
@@ -196,7 +196,7 @@
 
   /// The memory buffer that stores the data associated with
   /// this AST file, owned by the InMemoryModuleCache.
-  llvm::MemoryBuffer *Buffer;
+  llvm::MemoryBuffer *Buffer = nullptr;
 
   /// The size of this file, in bits.
   uint64_t SizeInBits = 0;
Index: clang/include/clang/Sema/Sema.h
===
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -937,7 +937,7 @@
   class DelayedDiagnostics;
 
   class DelayedDiagnosticsState {
-sema::DelayedDiagnosticPool *SavedPool;
+sema::DelayedDiagnosticPool *SavedPool = nullptr;
 friend class Sema::DelayedDiagnostics;
   };
   typedef 

[PATCH] D152689: [NFC] Remove dead conditionals

[Sindhu Chittireddy via Phabricator via cfe-commits]

schittir added a comment.

Patch landed with commit id 0e444c57c85853c86e4f4c94d9a51fb459fd9922 



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

https://reviews.llvm.org/D152689

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

[PATCH] D152977: [NFC] Fix potential dereferencing of null return value.

[Sindhu Chittireddy via Phabricator via cfe-commits]

schittir updated this revision to Diff 531824.
schittir marked an inline comment as done.
schittir added a comment.

Fix the assert causing build failure and address another review comment.


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

https://reviews.llvm.org/D152977

Files:
  clang/lib/AST/ASTContext.cpp
  clang/lib/Frontend/FrontendActions.cpp
  clang/lib/Sema/SemaType.cpp
  clang/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp
  clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp


Index: clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp
@@ -291,6 +291,7 @@
   // Existence in DestroyRetVal ensures existence in LockMap.
   // Existence in Destroyed also ensures that the lock state for lockR is 
either
   // UntouchedAndPossiblyDestroyed or UnlockedAndPossiblyDestroyed.
+  assert(lstate);
   assert(lstate->isUntouchedAndPossiblyDestroyed() ||
  lstate->isUnlockedAndPossiblyDestroyed());
 
Index: clang/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp
+++ clang/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp
@@ -321,7 +321,9 @@
   if (!IvarRegion)
 return nullptr;
 
-  return IvarRegion->getSymbolicBase()->getSymbol();
+  const SymbolicRegion *SR = IvarRegion->getSymbolicBase();
+  assert(SR && "Symbolic base should not be nullptr");
+  return SR->getSymbol();
 }
 
 /// If we are in -dealloc or -dealloc is on the stack, handle the call if it is
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -2790,7 +2790,7 @@
 
   // Only support _BitInt elements with byte-sized power of 2 NumBits.
   if (T->isBitIntType()) {
-unsigned NumBits = T->getAs()->getNumBits();
+unsigned NumBits = T->castAs()->getNumBits();
 if (!llvm::isPowerOf2_32(NumBits) || NumBits < 8) {
   Diag(AttrLoc, diag::err_attribute_invalid_bitint_vector_type)
   << (NumBits < 8);
Index: clang/lib/Frontend/FrontendActions.cpp
===
--- clang/lib/Frontend/FrontendActions.cpp
+++ clang/lib/Frontend/FrontendActions.cpp
@@ -458,6 +458,8 @@
   return;
 }
 
+assert(NamedCtx && "NamedCtx cannot be null");
+
 if (const auto *Decl = dyn_cast(NamedTemplate)) {
   OS << "unnamed function parameter " << Decl->getFunctionScopeIndex()
  << " ";
Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -4141,8 +4141,8 @@
   assert(vecType->isBuiltinType() || vecType->isDependentType() ||
  (vecType->isBitIntType() &&
   // Only support _BitInt elements with byte-sized power of 2 NumBits.
-  llvm::isPowerOf2_32(vecType->getAs()->getNumBits()) &&
-  vecType->getAs()->getNumBits() >= 8));
+  llvm::isPowerOf2_32(vecType->castAs()->getNumBits()) &&
+  vecType->castAs()->getNumBits() >= 8));
 
   // Check if we've already instantiated a vector of this type.
   llvm::FoldingSetNodeID ID;


Index: clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp
@@ -291,6 +291,7 @@
   // Existence in DestroyRetVal ensures existence in LockMap.
   // Existence in Destroyed also ensures that the lock state for lockR is either
   // UntouchedAndPossiblyDestroyed or UnlockedAndPossiblyDestroyed.
+  assert(lstate);
   assert(lstate->isUntouchedAndPossiblyDestroyed() ||
  lstate->isUnlockedAndPossiblyDestroyed());
 
Index: clang/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp
+++ clang/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp
@@ -321,7 +321,9 @@
   if (!IvarRegion)
 return nullptr;
 
-  return IvarRegion->getSymbolicBase()->getSymbol();
+  const SymbolicRegion *SR = IvarRegion->getSymbolicBase();
+  assert(SR && "Symbolic base should not be nullptr");
+  return SR->getSymbol();
 }
 
 /// If we are in -dealloc or -dealloc is on the stack, handle the call if it is
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -2790,7 +2790,7 @@
 
   // Only support _BitInt elements with byte-sized power of 2 NumBits.
   if (T->isBitIntType()) {
-unsigned NumBits = T->getAs()->getNumBits();
+

[PATCH] D152977: [NFC] Fix potential dereferencing of null return value.

[Sindhu Chittireddy via Phabricator via cfe-commits]

schittir added inline comments.



Comment at: clang/lib/Frontend/FrontendActions.cpp:461
 
+assert(NamedCtx, "NamedCtx cannot be null");
+

aaron.ballman wrote:
> This doesn't build. ;-)
Thanks for the catch. Something was wrong with my workspace build setup, and 
didn't diagnose this at all! I changed this in the new patch. 


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

https://reviews.llvm.org/D152977

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

[PATCH] D152977: [NFC] Fix potential dereferencing of null return value.

[Sindhu Chittireddy via Phabricator via cfe-commits]

schittir added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp:294-295
   // UntouchedAndPossiblyDestroyed or UnlockedAndPossiblyDestroyed.
-  assert(lstate->isUntouchedAndPossiblyDestroyed() ||
- lstate->isUnlockedAndPossiblyDestroyed());
+  assert(lstate && (lstate->isUntouchedAndPossiblyDestroyed() ||
+lstate->isUnlockedAndPossiblyDestroyed()));
 

steakhal wrote:
> schittir wrote:
> > steakhal wrote:
> > > 
> > Wouldn't it be better to do an with a comment, like below?  
> > ```
> > assert(lstate && "lstate should not be null");
> > ```
> As a Static Analyzer dev I don't think its necessary. StateRefs are 
> ubiquitous and here we probably know it cannot be null. And if it turns out 
> to be null we would get a segfault. So that sense I don't think its necessary.
> 
> 
> And speaking of a comment like "it should not be null" I think the segfault 
> would sort of imply that.
That makes sense! Thanks!


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

https://reviews.llvm.org/D152977

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