[PATCH] D41575: [index] Return when Parent is null in handleReference

[Fangrui Song via Phabricator via cfe-commits]

MaskRay updated this revision to Diff 128150.
MaskRay added a comment.

Sorry for changing this back and forth. But I do not have a powerful 
workstation and have to reverse engineer this.


Repository:
  rC Clang

https://reviews.llvm.org/D41575

Files:
  tools/libclang/CXIndexDataConsumer.cpp


Index: tools/libclang/CXIndexDataConsumer.cpp
===
--- tools/libclang/CXIndexDataConsumer.cpp
+++ tools/libclang/CXIndexDataConsumer.cpp
@@ -890,7 +890,7 @@
   const DeclContext *DC,
   const Expr *E,
   CXIdxEntityRefKind Kind) {
-  if (!D)
+  if (!D || !DC)
 return false;
 
   CXCursor Cursor = E ? MakeCXCursor(E, cast(DC), CXTU)
@@ -907,7 +907,7 @@
   if (!CB.indexEntityReference)
 return false;
 
-  if (!D)
+  if (!D || !DC)
 return false;
   if (Loc.isInvalid())
 return false;


Index: tools/libclang/CXIndexDataConsumer.cpp
===
--- tools/libclang/CXIndexDataConsumer.cpp
+++ tools/libclang/CXIndexDataConsumer.cpp
@@ -890,7 +890,7 @@
   const DeclContext *DC,
   const Expr *E,
   CXIdxEntityRefKind Kind) {
-  if (!D)
+  if (!D || !DC)
 return false;
 
   CXCursor Cursor = E ? MakeCXCursor(E, cast(DC), CXTU)
@@ -907,7 +907,7 @@
   if (!CB.indexEntityReference)
 return false;
 
-  if (!D)
+  if (!D || !DC)
 return false;
   if (Loc.isInvalid())
 return false;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D41575: [index] Return when DC is null in handleReference

[Fangrui Song via Phabricator via cfe-commits]

MaskRay updated this revision to Diff 128149.
MaskRay added a comment.

DC -> Parent


Repository:
  rC Clang

https://reviews.llvm.org/D41575

Files:
  tools/libclang/CXIndexDataConsumer.cpp


Index: tools/libclang/CXIndexDataConsumer.cpp
===
--- tools/libclang/CXIndexDataConsumer.cpp
+++ tools/libclang/CXIndexDataConsumer.cpp
@@ -907,7 +907,7 @@
   if (!CB.indexEntityReference)
 return false;
 
-  if (!D)
+  if (!D || !Parent)
 return false;
   if (Loc.isInvalid())
 return false;


Index: tools/libclang/CXIndexDataConsumer.cpp
===
--- tools/libclang/CXIndexDataConsumer.cpp
+++ tools/libclang/CXIndexDataConsumer.cpp
@@ -907,7 +907,7 @@
   if (!CB.indexEntityReference)
 return false;
 
-  if (!D)
+  if (!D || !Parent)
 return false;
   if (Loc.isInvalid())
 return false;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D41575: [index] Return when DC is null in handleReference

[Fangrui Song via Phabricator via cfe-commits]

MaskRay updated this revision to Diff 128148.
MaskRay added a comment.

DC -> Parent


Repository:
  rC Clang

https://reviews.llvm.org/D41575

Files:
  tools/libclang/CXIndexDataConsumer.cpp


Index: tools/libclang/CXIndexDataConsumer.cpp
===
--- tools/libclang/CXIndexDataConsumer.cpp
+++ tools/libclang/CXIndexDataConsumer.cpp
@@ -907,7 +907,7 @@
   if (!CB.indexEntityReference)
 return false;
 
-  if (!D)
+  if (!D || !Parent)
 return false;
   if (Loc.isInvalid())
 return false;


Index: tools/libclang/CXIndexDataConsumer.cpp
===
--- tools/libclang/CXIndexDataConsumer.cpp
+++ tools/libclang/CXIndexDataConsumer.cpp
@@ -907,7 +907,7 @@
   if (!CB.indexEntityReference)
 return false;
 
-  if (!D)
+  if (!D || !Parent)
 return false;
   if (Loc.isInvalid())
 return false;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D41575: [index] Return when DC is null in handleReference

[Fangrui Song via Phabricator via cfe-commits]

MaskRay created this revision.
Herald added a subscriber: cfe-commits.

DC may sometimes be NULL and getContainerInfo(DC, Container) will fail.


Repository:
  rC Clang

https://reviews.llvm.org/D41575

Files:
  tools/libclang/CXIndexDataConsumer.cpp


Index: tools/libclang/CXIndexDataConsumer.cpp
===
--- tools/libclang/CXIndexDataConsumer.cpp
+++ tools/libclang/CXIndexDataConsumer.cpp
@@ -890,7 +890,7 @@
   const DeclContext *DC,
   const Expr *E,
   CXIdxEntityRefKind Kind) {
-  if (!D)
+  if (!D || !DC)
 return false;
 
   CXCursor Cursor = E ? MakeCXCursor(E, cast(DC), CXTU)
@@ -907,7 +907,7 @@
   if (!CB.indexEntityReference)
 return false;
 
-  if (!D)
+  if (!D || !DC)
 return false;
   if (Loc.isInvalid())
 return false;


Index: tools/libclang/CXIndexDataConsumer.cpp
===
--- tools/libclang/CXIndexDataConsumer.cpp
+++ tools/libclang/CXIndexDataConsumer.cpp
@@ -890,7 +890,7 @@
   const DeclContext *DC,
   const Expr *E,
   CXIdxEntityRefKind Kind) {
-  if (!D)
+  if (!D || !DC)
 return false;
 
   CXCursor Cursor = E ? MakeCXCursor(E, cast(DC), CXTU)
@@ -907,7 +907,7 @@
   if (!CB.indexEntityReference)
 return false;
 
-  if (!D)
+  if (!D || !DC)
 return false;
   if (Loc.isInvalid())
 return false;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [libunwind] r321440 - [libunwind] Avoid using C++ headers.

[Don Hinton via cfe-commits]

On Mon, Dec 25, 2017 at 1:10 PM, whitequark 
wrote:

> On 2017-12-25 20:54, Don Hinton wrote:
>
>> On Mon, Dec 25, 2017 at 12:43 PM, whitequark
>>  wrote:
>>
>>> On 2017-12-25 20:32, Don Hinton wrote:
>>>
 It should also assert for non-integral types.

>>>
>>> True, but I do not know enough C++ magic to implement that. AIUI
>>> std::numeric_limits implements this by specializing a template for
>>> every
>>> integral type, which is not something I will do here. Of course
>>> there's a standard template for checking whether a type is integral,
>>> but that also lives in libcxx.
>>>
>>
>> Why not just use static_cast(~0) and avoid all these issues?
>>
>
> r321448. Thanks for the review!
>

LGTM, thanks...


>
>
>> You might have a point that this doesn't apply in your case, but it's
>> a good habit to get into.
>>
>> On Mon, Dec 25, 2017 at 12:06 PM, whitequark
>>>  wrote:
>>>
>>> On 2017-12-25 19:43, Don Hinton wrote:
>>>
>>> Here's the patch I applied locally.
>>>
>>> hth...
>>> don
>>> [snip]
>>>
>>> I've committed a slightly beautified version of the patch (below)
>>> as r321446. Cheers!
>>>
>>> From 8a4760bafc1123f09438587ee5432eabdec3d33d Mon Sep 17 00:00:00
>>> 2001
>>> From: whitequark 
>>> Date: Mon, 25 Dec 2017 20:03:40 +
>>> Subject: [PATCH] [libunwind] Unbreak debug builds after r321440.
>>>
>>> ---
>>> src/DwarfParser.hpp | 11 +++
>>> 1 file changed, 7 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/src/DwarfParser.hpp b/src/DwarfParser.hpp
>>> index 518101e..645ac21 100644
>>> --- a/src/DwarfParser.hpp
>>> +++ b/src/DwarfParser.hpp
>>> @@ -17,7 +17,6 @@
>>> #include 
>>> #include 
>>> #include 
>>> -#include 
>>>
>>> #include "libunwind.h"
>>> #include "dwarf2.h"
>>> @@ -26,6 +25,10 @@
>>>
>>> namespace libunwind {
>>>
>>> +// Avoid relying on C++ headers.
>>> +template 
>>> +static constexpr T pint_max_value() { return ~0; }
>>> +
>>> /// CFI_Parser does basic parsing of a CFI (Call Frame Information)
>>> records.
>>> /// See DWARF Spec for details:
>>> ///
>>>
>>
>> http://refspecs.linuxbase.org/LSB_3.1.0/LSB-Core-generic/LSB
>> -Core-generic/ehframechpt.html
>> [5]
>>
>>
>> [1]
>>>
>>> @@ -540,7 +543,7 @@ bool CFI_Parser::parseInstructions(A
>>> , pint_t instructions,
>>> results->cfaRegister = 0;
>>> results->cfaExpression = (int64_t)p;
>>> length = addressSpace.getULEB128(p, instructionsEnd);
>>> -  assert(length < std::numeric_limits::max() &&
>>> "pointer overflow");
>>> +  assert(length < pint_max_value() && "pointer
>>> overflow");
>>> p += static_cast(length);
>>>
>>> _LIBUNWIND_TRACE_DWARF("DW_CFA_def_cfa_expression(expression=0x%"
>>> PRIx64
>>> ", length=%" PRIu64 ")\n",
>>> @@ -556,7 +559,7 @@ bool CFI_Parser::parseInstructions(A
>>> , pint_t instructions,
>>> results->savedRegisters[reg].location =
>>> kRegisterAtExpression;
>>> results->savedRegisters[reg].value = (int64_t)p;
>>> length = addressSpace.getULEB128(p, instructionsEnd);
>>> -  assert(length < std::numeric_limits::max() &&
>>> "pointer overflow");
>>> +  assert(length < pint_max_value() && "pointer
>>> overflow");
>>> p += static_cast(length);
>>> _LIBUNWIND_TRACE_DWARF("DW_CFA_expression(reg=%" PRIu64 ", "
>>> "expression=0x%" PRIx64 ", "
>>> @@ -642,7 +645,7 @@ bool CFI_Parser::parseInstructions(A
>>> , pint_t instructions,
>>> results->savedRegisters[reg].location =
>>> kRegisterIsExpression;
>>> results->savedRegisters[reg].value = (int64_t)p;
>>> length = addressSpace.getULEB128(p, instructionsEnd);
>>> -  assert(length < std::numeric_limits::max() &&
>>> "pointer overflow");
>>> +  assert(length < pint_max_value() && "pointer
>>> overflow");
>>> p += static_cast(length);
>>> _LIBUNWIND_TRACE_DWARF("DW_CFA_val_expression(reg=%" PRIu64
>>> ", "
>>> "expression=0x%" PRIx64 ", length=%"
>>> PRIu64 ")\n",
>>> --
>>> 2.11.0
>>>
>>> On Mon, Dec 25, 2017 at 11:26 AM, Don Hinton 
>>> wrote:
>>>
>>> On Mon, Dec 25, 2017 at 11:09 AM, whitequark
>>>  wrote:
>>> On 2017-12-25 19:04, Don Hinton wrote:
>>> Hi:
>>>
>>> This change breaks in a local debug build, e.g.,:
>>>
>>  /Users/dhinton/projects/llvm_project/libunwind/src/DwarfPar
>> ser.hpp:559:28:
>>
>> error: no member named 'numeric_limits' in namespace 'std'
>>> assert(length < std::numeric_limits::max() && "pointer
>>> overflow");
>>> ~^
>>>
>>> Sorry, I missed this. Any idea on reformulating the assert in a way
>>> that does not require libcxx headers? Not having them significantly
>>> simplifies bare-metal builds...
>>>
>>> Well, assuming pint_t is some unsigned integer type, the max can be
>>> found like this:
>>>
>>> pint_t max_pint_t = ~0;
>>>
>>> So, that could be used in a pinch.
>>>
>>> thanks...
>>> don
>>>
>>> On Mon, Dec 25, 2017 at 5:06 AM, whitequark via cfe-commits
>>>  wrote:
>>>
>>> Author: whitequark
>>> Date: Mon Dec 25 

r321449 - Add a fixit for attributes incorrectly placed prior to 'struct/class/enum' keyword.

[Faisal Vali via cfe-commits]

Author: faisalv
Date: Mon Dec 25 14:23:20 2017
New Revision: 321449

URL: http://llvm.org/viewvc/llvm-project?rev=321449=rev
Log:
Add a fixit for attributes incorrectly placed prior to 'struct/class/enum' 
keyword.

Suggest moving the following erroneous attrib list (based on location)
[[]] struct X;  
to 
struct [[]] X;

Additionally, added a fixme for the current implementation that diagnoses 
misplaced attributes to consider using the newly introduced diagnostic (that I 
think is more user-friendly).






Modified:
cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td
cfe/trunk/include/clang/Parse/Parser.h
cfe/trunk/lib/Parse/ParseDecl.cpp
cfe/trunk/lib/Parse/Parser.cpp
cfe/trunk/test/Parser/c2x-attributes.c
cfe/trunk/test/Parser/cxx-decl.cpp
cfe/trunk/test/Parser/cxx0x-attributes.cpp

Modified: cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td?rev=321449=321448=321449=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td Mon Dec 25 14:23:20 
2017
@@ -587,6 +587,7 @@ def ext_using_attribute_ns : ExtWarn<
 def err_using_attribute_ns_conflict : Error<
   "attribute with scope specifier cannot follow default scope specifier">;
 def err_attributes_not_allowed : Error<"an attribute list cannot appear here">;
+def err_attributes_misplaced : Error<"misplaced attributes; expected 
attributes here">;
 def err_l_square_l_square_not_attribute : Error<
   "C++11 only allows consecutive left square brackets when "
   "introducing an attribute">;

Modified: cfe/trunk/include/clang/Parse/Parser.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Parse/Parser.h?rev=321449=321448=321449=diff
==
--- cfe/trunk/include/clang/Parse/Parser.h (original)
+++ cfe/trunk/include/clang/Parse/Parser.h Mon Dec 25 14:23:20 2017
@@ -2200,13 +2200,16 @@ private:
 
   void stripTypeAttributesOffDeclSpec(ParsedAttributesWithRange ,
   DeclSpec , Sema::TagUseKind TUK);
-
-  void ProhibitAttributes(ParsedAttributesWithRange ) {
+  
+  // FixItLoc = possible correct location for the attributes
+  void ProhibitAttributes(ParsedAttributesWithRange ,
+  SourceLocation FixItLoc = SourceLocation()) {
 if (!attrs.Range.isValid()) return;
-DiagnoseProhibitedAttributes(attrs);
+DiagnoseProhibitedAttributes(attrs, FixItLoc);
 attrs.clear();
   }
-  void DiagnoseProhibitedAttributes(ParsedAttributesWithRange );
+  void DiagnoseProhibitedAttributes(ParsedAttributesWithRange , 
+SourceLocation FixItLoc);
 
   // Forbid C++11 and C2x attributes that appear on certain syntactic locations
   // which standard permits but we don't supported yet, for example, attributes

Modified: cfe/trunk/lib/Parse/ParseDecl.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseDecl.cpp?rev=321449=321448=321449=diff
==
--- cfe/trunk/lib/Parse/ParseDecl.cpp (original)
+++ cfe/trunk/lib/Parse/ParseDecl.cpp Mon Dec 25 14:23:20 2017
@@ -1548,15 +1548,21 @@ void Parser::DiagnoseMisplacedCXX11Attri
   SourceLocation Loc = Tok.getLocation();
   ParseCXX11Attributes(Attrs);
   CharSourceRange AttrRange(SourceRange(Loc, Attrs.Range.getEnd()), true);
-
+  // FIXME: use err_attributes_misplaced
   Diag(Loc, diag::err_attributes_not_allowed)
 << FixItHint::CreateInsertionFromRange(CorrectLocation, AttrRange)
 << FixItHint::CreateRemoval(AttrRange);
 }
 
-void Parser::DiagnoseProhibitedAttributes(ParsedAttributesWithRange ) {
-  Diag(attrs.Range.getBegin(), diag::err_attributes_not_allowed)
-<< attrs.Range;
+void Parser::DiagnoseProhibitedAttributes(ParsedAttributesWithRange ,
+  const SourceLocation 
CorrectLocation) {
+  if (CorrectLocation.isValid()) {
+CharSourceRange AttrRange(attrs.Range, true);
+Diag(CorrectLocation, diag::err_attributes_misplaced)
+<< FixItHint::CreateInsertionFromRange(CorrectLocation, AttrRange)
+<< FixItHint::CreateRemoval(AttrRange);
+  } else
+Diag(attrs.Range.getBegin(), diag::err_attributes_not_allowed) << 
attrs.Range;
 }
 
 void Parser::ProhibitCXX11Attributes(ParsedAttributesWithRange ,

Modified: cfe/trunk/lib/Parse/Parser.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/Parser.cpp?rev=321449=321448=321449=diff
==
--- cfe/trunk/lib/Parse/Parser.cpp (original)
+++ cfe/trunk/lib/Parse/Parser.cpp Mon Dec 25 14:23:20 2017
@@ -930,7 +930,31 @@ Parser::ParseDeclOrFunctionDefInternal(P
   // C99 6.7.2.3p6: Handle "struct-or-union identifier;", 

Re: [libunwind] r321440 - [libunwind] Avoid using C++ headers.

[whitequark via cfe-commits]

On 2017-12-25 20:54, Don Hinton wrote:

On Mon, Dec 25, 2017 at 12:43 PM, whitequark
 wrote:

On 2017-12-25 20:32, Don Hinton wrote:

It should also assert for non-integral types.


True, but I do not know enough C++ magic to implement that. AIUI
std::numeric_limits implements this by specializing a template for
every
integral type, which is not something I will do here. Of course
there's a standard template for checking whether a type is integral,
but that also lives in libcxx.


Why not just use static_cast(~0) and avoid all these issues?


r321448. Thanks for the review!



You might have a point that this doesn't apply in your case, but it's
a good habit to get into.


On Mon, Dec 25, 2017 at 12:06 PM, whitequark
 wrote:

On 2017-12-25 19:43, Don Hinton wrote:

Here's the patch I applied locally.

hth...
don
[snip]

I've committed a slightly beautified version of the patch (below)
as r321446. Cheers!

From 8a4760bafc1123f09438587ee5432eabdec3d33d Mon Sep 17 00:00:00
2001
From: whitequark 
Date: Mon, 25 Dec 2017 20:03:40 +
Subject: [PATCH] [libunwind] Unbreak debug builds after r321440.

---
src/DwarfParser.hpp | 11 +++
1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/src/DwarfParser.hpp b/src/DwarfParser.hpp
index 518101e..645ac21 100644
--- a/src/DwarfParser.hpp
+++ b/src/DwarfParser.hpp
@@ -17,7 +17,6 @@
#include 
#include 
#include 
-#include 

#include "libunwind.h"
#include "dwarf2.h"
@@ -26,6 +25,10 @@

namespace libunwind {

+// Avoid relying on C++ headers.
+template 
+static constexpr T pint_max_value() { return ~0; }
+
/// CFI_Parser does basic parsing of a CFI (Call Frame Information)
records.
/// See DWARF Spec for details:
///


http://refspecs.linuxbase.org/LSB_3.1.0/LSB-Core-generic/LSB-Core-generic/ehframechpt.html
[5]


[1]

@@ -540,7 +543,7 @@ bool CFI_Parser::parseInstructions(A
, pint_t instructions,
results->cfaRegister = 0;
results->cfaExpression = (int64_t)p;
length = addressSpace.getULEB128(p, instructionsEnd);
-  assert(length < std::numeric_limits::max() &&
"pointer overflow");
+  assert(length < pint_max_value() && "pointer
overflow");
p += static_cast(length);

_LIBUNWIND_TRACE_DWARF("DW_CFA_def_cfa_expression(expression=0x%"
PRIx64
", length=%" PRIu64 ")\n",
@@ -556,7 +559,7 @@ bool CFI_Parser::parseInstructions(A
, pint_t instructions,
results->savedRegisters[reg].location =
kRegisterAtExpression;
results->savedRegisters[reg].value = (int64_t)p;
length = addressSpace.getULEB128(p, instructionsEnd);
-  assert(length < std::numeric_limits::max() &&
"pointer overflow");
+  assert(length < pint_max_value() && "pointer
overflow");
p += static_cast(length);
_LIBUNWIND_TRACE_DWARF("DW_CFA_expression(reg=%" PRIu64 ", "
"expression=0x%" PRIx64 ", "
@@ -642,7 +645,7 @@ bool CFI_Parser::parseInstructions(A
, pint_t instructions,
results->savedRegisters[reg].location =
kRegisterIsExpression;
results->savedRegisters[reg].value = (int64_t)p;
length = addressSpace.getULEB128(p, instructionsEnd);
-  assert(length < std::numeric_limits::max() &&
"pointer overflow");
+  assert(length < pint_max_value() && "pointer
overflow");
p += static_cast(length);
_LIBUNWIND_TRACE_DWARF("DW_CFA_val_expression(reg=%" PRIu64
", "
"expression=0x%" PRIx64 ", length=%"
PRIu64 ")\n",
--
2.11.0

On Mon, Dec 25, 2017 at 11:26 AM, Don Hinton 
wrote:

On Mon, Dec 25, 2017 at 11:09 AM, whitequark
 wrote:
On 2017-12-25 19:04, Don Hinton wrote:
Hi:

This change breaks in a local debug build, e.g.,:
 
/Users/dhinton/projects/llvm_project/libunwind/src/DwarfParser.hpp:559:28:



error: no member named 'numeric_limits' in namespace 'std'
assert(length < std::numeric_limits::max() && "pointer
overflow");
~^

Sorry, I missed this. Any idea on reformulating the assert in a way
that does not require libcxx headers? Not having them significantly
simplifies bare-metal builds...

Well, assuming pint_t is some unsigned integer type, the max can be
found like this:

pint_t max_pint_t = ~0;

So, that could be used in a pinch.

thanks...
don

On Mon, Dec 25, 2017 at 5:06 AM, whitequark via cfe-commits
 wrote:

Author: whitequark
Date: Mon Dec 25 05:06:09 2017
New Revision: 321440

URL: http://llvm.org/viewvc/llvm-project?rev=321440=rev [1]
[2] [1]
[1]
Log:
[libunwind] Avoid using C++ headers.

This is useful for building libunwind on libcxx-free systems.

Modified:
libunwind/trunk/src/DwarfParser.hpp

Modified: libunwind/trunk/src/DwarfParser.hpp
URL:


http://llvm.org/viewvc/llvm-project/libunwind/trunk/src/DwarfParser.hpp?rev=321440=321439=321440=diff
[6]


[3]
[2]
[2]
 
==



--- libunwind/trunk/src/DwarfParser.hpp (original)
+++ libunwind/trunk/src/DwarfParser.hpp Mon Dec 25 05:06:09 2017
@@ -17,7 +17,7 @@
#include 
#include 
#include 

[libunwind] r321448 - [libunwind] Remove dubious template function. NFC.

[whitequark via cfe-commits]

Author: whitequark
Date: Mon Dec 25 13:08:41 2017
New Revision: 321448

URL: http://llvm.org/viewvc/llvm-project?rev=321448=rev
Log:
[libunwind] Remove dubious template function. NFC.

Per review by Don Hinton.

Modified:
libunwind/trunk/src/DwarfParser.hpp

Modified: libunwind/trunk/src/DwarfParser.hpp
URL: 
http://llvm.org/viewvc/llvm-project/libunwind/trunk/src/DwarfParser.hpp?rev=321448=321447=321448=diff
==
--- libunwind/trunk/src/DwarfParser.hpp (original)
+++ libunwind/trunk/src/DwarfParser.hpp Mon Dec 25 13:08:41 2017
@@ -25,10 +25,6 @@
 
 namespace libunwind {
 
-// Avoid relying on C++ headers.
-template 
-static constexpr T pint_max_value() { return ~0; }
-
 /// CFI_Parser does basic parsing of a CFI (Call Frame Information) records.
 /// See DWARF Spec for details:
 ///
http://refspecs.linuxbase.org/LSB_3.1.0/LSB-Core-generic/LSB-Core-generic/ehframechpt.html
@@ -543,7 +539,7 @@ bool CFI_Parser::parseInstructions(A
   results->cfaRegister = 0;
   results->cfaExpression = (int64_t)p;
   length = addressSpace.getULEB128(p, instructionsEnd);
-  assert(length < pint_max_value() && "pointer overflow");
+  assert(length < static_cast(~0) && "pointer overflow");
   p += static_cast(length);
   _LIBUNWIND_TRACE_DWARF("DW_CFA_def_cfa_expression(expression=0x%" PRIx64
  ", length=%" PRIu64 ")\n",
@@ -559,7 +555,7 @@ bool CFI_Parser::parseInstructions(A
   results->savedRegisters[reg].location = kRegisterAtExpression;
   results->savedRegisters[reg].value = (int64_t)p;
   length = addressSpace.getULEB128(p, instructionsEnd);
-  assert(length < pint_max_value() && "pointer overflow");
+  assert(length < static_cast(~0) && "pointer overflow");
   p += static_cast(length);
   _LIBUNWIND_TRACE_DWARF("DW_CFA_expression(reg=%" PRIu64 ", "
  "expression=0x%" PRIx64 ", "
@@ -645,7 +641,7 @@ bool CFI_Parser::parseInstructions(A
   results->savedRegisters[reg].location = kRegisterIsExpression;
   results->savedRegisters[reg].value = (int64_t)p;
   length = addressSpace.getULEB128(p, instructionsEnd);
-  assert(length < pint_max_value() && "pointer overflow");
+  assert(length < static_cast(~0) && "pointer overflow");
   p += static_cast(length);
   _LIBUNWIND_TRACE_DWARF("DW_CFA_val_expression(reg=%" PRIu64 ", "
  "expression=0x%" PRIx64 ", length=%" PRIu64 ")\n",


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

Re: [libunwind] r321440 - [libunwind] Avoid using C++ headers.

[Don Hinton via cfe-commits]

On Mon, Dec 25, 2017 at 12:43 PM, whitequark 
wrote:

> On 2017-12-25 20:32, Don Hinton wrote:
>
>> While beauty is in the eye of the beholder, I'm not sure introducing a
>> new template function in a header that's only used in that header is a
>> good idea.  Every file that includes DwarfParser.hpp is going to get
>> that template function -- and someone may try to use it someday.
>>
>
> That header is a header private to the libunwind implementation,
> and the template function is confined to `namespace libunwind`.


Smaller universe, but still the same issue.


>


>
> However, if you did want to do that, you should handle signed types as
>> well, which pint_max_value doesn't do.
>>
>
> `pint_t` is an unsigned type in libunwind, the signed counterpart is
> `sint_t`


The point is that a template can take any type.  Once you introduce it, it
can be used by anyone with any type.


> .
>
> It should also assert for non-integral types.
>>
>
> True, but I do not know enough C++ magic to implement that. AIUI
> std::numeric_limits implements this by specializing a template for every
> integral type, which is not something I will do here. Of course
> there's a standard template for checking whether a type is integral,
> but that also lives in libcxx.
>

Why not just use static_cast(~0) and avoid all these issues?

You might have a point that this doesn't apply in your case, but it's a
good habit to get into.


>
>
>> On Mon, Dec 25, 2017 at 12:06 PM, whitequark
>>  wrote:
>>
>> On 2017-12-25 19:43, Don Hinton wrote:
>>>
>>> Here's the patch I applied locally.

 hth...
 don
 [snip]

>>>
>>> I've committed a slightly beautified version of the patch (below)
>>> as r321446. Cheers!
>>>
>>> From 8a4760bafc1123f09438587ee5432eabdec3d33d Mon Sep 17 00:00:00
>>> 2001
>>> From: whitequark 
>>> Date: Mon, 25 Dec 2017 20:03:40 +
>>> Subject: [PATCH] [libunwind] Unbreak debug builds after r321440.
>>>
>>> ---
>>> src/DwarfParser.hpp | 11 +++
>>> 1 file changed, 7 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/src/DwarfParser.hpp b/src/DwarfParser.hpp
>>> index 518101e..645ac21 100644
>>> --- a/src/DwarfParser.hpp
>>> +++ b/src/DwarfParser.hpp
>>> @@ -17,7 +17,6 @@
>>> #include 
>>> #include 
>>> #include 
>>> -#include 
>>>
>>> #include "libunwind.h"
>>> #include "dwarf2.h"
>>> @@ -26,6 +25,10 @@
>>>
>>> namespace libunwind {
>>>
>>> +// Avoid relying on C++ headers.
>>> +template 
>>> +static constexpr T pint_max_value() { return ~0; }
>>> +
>>> /// CFI_Parser does basic parsing of a CFI (Call Frame Information)
>>> records.
>>> /// See DWARF Spec for details:
>>> ///
>>>
>>> http://refspecs.linuxbase.org/LSB_3.1.0/LSB-Core-generic/LSB
>> -Core-generic/ehframechpt.html
>>
>>> [1]
>>>
>>> @@ -540,7 +543,7 @@ bool CFI_Parser::parseInstructions(A
>>> , pint_t instructions,
>>> results->cfaRegister = 0;
>>> results->cfaExpression = (int64_t)p;
>>> length = addressSpace.getULEB128(p, instructionsEnd);
>>> -  assert(length < std::numeric_limits::max() &&
>>> "pointer overflow");
>>> +  assert(length < pint_max_value() && "pointer
>>> overflow");
>>> p += static_cast(length);
>>>
>>> _LIBUNWIND_TRACE_DWARF("DW_CFA_def_cfa_expression(expression=0x%"
>>> PRIx64
>>> ", length=%" PRIu64 ")\n",
>>> @@ -556,7 +559,7 @@ bool CFI_Parser::parseInstructions(A
>>> , pint_t instructions,
>>> results->savedRegisters[reg].location =
>>> kRegisterAtExpression;
>>> results->savedRegisters[reg].value = (int64_t)p;
>>> length = addressSpace.getULEB128(p, instructionsEnd);
>>> -  assert(length < std::numeric_limits::max() &&
>>> "pointer overflow");
>>> +  assert(length < pint_max_value() && "pointer
>>> overflow");
>>> p += static_cast(length);
>>> _LIBUNWIND_TRACE_DWARF("DW_CFA_expression(reg=%" PRIu64 ", "
>>> "expression=0x%" PRIx64 ", "
>>> @@ -642,7 +645,7 @@ bool CFI_Parser::parseInstructions(A
>>> , pint_t instructions,
>>> results->savedRegisters[reg].location =
>>> kRegisterIsExpression;
>>> results->savedRegisters[reg].value = (int64_t)p;
>>> length = addressSpace.getULEB128(p, instructionsEnd);
>>> -  assert(length < std::numeric_limits::max() &&
>>> "pointer overflow");
>>> +  assert(length < pint_max_value() && "pointer
>>> overflow");
>>> p += static_cast(length);
>>> _LIBUNWIND_TRACE_DWARF("DW_CFA_val_expression(reg=%" PRIu64
>>> ", "
>>> "expression=0x%" PRIx64 ", length=%"
>>> PRIu64 ")\n",
>>> --
>>> 2.11.0
>>>
>>> On Mon, Dec 25, 2017 at 11:26 AM, Don Hinton 
>>> wrote:
>>>
>>> On Mon, Dec 25, 2017 at 11:09 AM, whitequark
>>>  wrote:
>>> On 2017-12-25 19:04, Don Hinton wrote:
>>> Hi:
>>>
>>> This change breaks in a local debug build, e.g.,:
>>>
>>>
>>> /Users/dhinton/projects/llvm_project/libunwind/src/DwarfPars
>> er.hpp:559:28:
>>
>>> error: no member named 'numeric_limits' in namespace 'std'
>>> assert(length < std::numeric_limits::max() 

Re: [libunwind] r321440 - [libunwind] Avoid using C++ headers.

[whitequark via cfe-commits]

On 2017-12-25 20:32, Don Hinton wrote:

While beauty is in the eye of the beholder, I'm not sure introducing a
new template function in a header that's only used in that header is a
good idea.  Every file that includes DwarfParser.hpp is going to get
that template function -- and someone may try to use it someday.


That header is a header private to the libunwind implementation,
and the template function is confined to `namespace libunwind`.


However, if you did want to do that, you should handle signed types as
well, which pint_max_value doesn't do.


`pint_t` is an unsigned type in libunwind, the signed counterpart is 
`sint_t`.



It should also assert for non-integral types.


True, but I do not know enough C++ magic to implement that. AIUI
std::numeric_limits implements this by specializing a template for every
integral type, which is not something I will do here. Of course
there's a standard template for checking whether a type is integral,
but that also lives in libcxx.



On Mon, Dec 25, 2017 at 12:06 PM, whitequark
 wrote:


On 2017-12-25 19:43, Don Hinton wrote:


Here's the patch I applied locally.

hth...
don
[snip]


I've committed a slightly beautified version of the patch (below)
as r321446. Cheers!

From 8a4760bafc1123f09438587ee5432eabdec3d33d Mon Sep 17 00:00:00
2001
From: whitequark 
Date: Mon, 25 Dec 2017 20:03:40 +
Subject: [PATCH] [libunwind] Unbreak debug builds after r321440.

---
src/DwarfParser.hpp | 11 +++
1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/src/DwarfParser.hpp b/src/DwarfParser.hpp
index 518101e..645ac21 100644
--- a/src/DwarfParser.hpp
+++ b/src/DwarfParser.hpp
@@ -17,7 +17,6 @@
#include 
#include 
#include 
-#include 

#include "libunwind.h"
#include "dwarf2.h"
@@ -26,6 +25,10 @@

namespace libunwind {

+// Avoid relying on C++ headers.
+template 
+static constexpr T pint_max_value() { return ~0; }
+
/// CFI_Parser does basic parsing of a CFI (Call Frame Information)
records.
/// See DWARF Spec for details:
///


http://refspecs.linuxbase.org/LSB_3.1.0/LSB-Core-generic/LSB-Core-generic/ehframechpt.html

[1]
@@ -540,7 +543,7 @@ bool CFI_Parser::parseInstructions(A
, pint_t instructions,
results->cfaRegister = 0;
results->cfaExpression = (int64_t)p;
length = addressSpace.getULEB128(p, instructionsEnd);
-  assert(length < std::numeric_limits::max() &&
"pointer overflow");
+  assert(length < pint_max_value() && "pointer
overflow");
p += static_cast(length);

_LIBUNWIND_TRACE_DWARF("DW_CFA_def_cfa_expression(expression=0x%"
PRIx64
", length=%" PRIu64 ")\n",
@@ -556,7 +559,7 @@ bool CFI_Parser::parseInstructions(A
, pint_t instructions,
results->savedRegisters[reg].location =
kRegisterAtExpression;
results->savedRegisters[reg].value = (int64_t)p;
length = addressSpace.getULEB128(p, instructionsEnd);
-  assert(length < std::numeric_limits::max() &&
"pointer overflow");
+  assert(length < pint_max_value() && "pointer
overflow");
p += static_cast(length);
_LIBUNWIND_TRACE_DWARF("DW_CFA_expression(reg=%" PRIu64 ", "
"expression=0x%" PRIx64 ", "
@@ -642,7 +645,7 @@ bool CFI_Parser::parseInstructions(A
, pint_t instructions,
results->savedRegisters[reg].location =
kRegisterIsExpression;
results->savedRegisters[reg].value = (int64_t)p;
length = addressSpace.getULEB128(p, instructionsEnd);
-  assert(length < std::numeric_limits::max() &&
"pointer overflow");
+  assert(length < pint_max_value() && "pointer
overflow");
p += static_cast(length);
_LIBUNWIND_TRACE_DWARF("DW_CFA_val_expression(reg=%" PRIu64
", "
"expression=0x%" PRIx64 ", length=%"
PRIu64 ")\n",
--
2.11.0

On Mon, Dec 25, 2017 at 11:26 AM, Don Hinton 
wrote:

On Mon, Dec 25, 2017 at 11:09 AM, whitequark
 wrote:
On 2017-12-25 19:04, Don Hinton wrote:
Hi:

This change breaks in a local debug build, e.g.,:



/Users/dhinton/projects/llvm_project/libunwind/src/DwarfParser.hpp:559:28:

error: no member named 'numeric_limits' in namespace 'std'
assert(length < std::numeric_limits::max() && "pointer
overflow");
~^

Sorry, I missed this. Any idea on reformulating the assert in a way
that does not require libcxx headers? Not having them significantly
simplifies bare-metal builds...

Well, assuming pint_t is some unsigned integer type, the max can be
found like this:

pint_t max_pint_t = ~0;

So, that could be used in a pinch.

thanks...
don

On Mon, Dec 25, 2017 at 5:06 AM, whitequark via cfe-commits
 wrote:

Author: whitequark
Date: Mon Dec 25 05:06:09 2017
New Revision: 321440

URL: http://llvm.org/viewvc/llvm-project?rev=321440=rev [2] [1]
[1]
Log:
[libunwind] Avoid using C++ headers.

This is useful for building libunwind on libcxx-free systems.

Modified:
libunwind/trunk/src/DwarfParser.hpp

Modified: libunwind/trunk/src/DwarfParser.hpp
URL:

Re: [libunwind] r321440 - [libunwind] Avoid using C++ headers.

[Don Hinton via cfe-commits]

While beauty is in the eye of the beholder, I'm not sure introducing a new
template function in a header that's only used in that header is a good
idea.  Every file that includes DwarfParser.hpp is going to get that
template function -- and someone may try to use it someday.

However, if you did want to do that, you should handle signed types as
well, which pint_max_value doesn't do.  It should also assert for
non-integral types.

On Mon, Dec 25, 2017 at 12:06 PM, whitequark 
wrote:

> On 2017-12-25 19:43, Don Hinton wrote:
>
>> Here's the patch I applied locally.
>>
>> hth...
>> don
>> [snip]
>>
>
> I've committed a slightly beautified version of the patch (below)
> as r321446. Cheers!
>
> From 8a4760bafc1123f09438587ee5432eabdec3d33d Mon Sep 17 00:00:00 2001
> From: whitequark 
> Date: Mon, 25 Dec 2017 20:03:40 +
> Subject: [PATCH] [libunwind] Unbreak debug builds after r321440.
>
> ---
>  src/DwarfParser.hpp | 11 +++
>  1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/src/DwarfParser.hpp b/src/DwarfParser.hpp
> index 518101e..645ac21 100644
> --- a/src/DwarfParser.hpp
> +++ b/src/DwarfParser.hpp
> @@ -17,7 +17,6 @@
>  #include 
>  #include 
>  #include 
> -#include 
>
>  #include "libunwind.h"
>  #include "dwarf2.h"
> @@ -26,6 +25,10 @@
>
>  namespace libunwind {
>
> +// Avoid relying on C++ headers.
> +template 
> +static constexpr T pint_max_value() { return ~0; }
> +
>  /// CFI_Parser does basic parsing of a CFI (Call Frame Information)
> records.
>  /// See DWARF Spec for details:
>  ///http://refspecs.linuxbase.org/LSB_3.1.0/LSB-Core-generic/LSB
> -Core-generic/ehframechpt.html
> @@ -540,7 +543,7 @@ bool CFI_Parser::parseInstructions(A
> , pint_t instructions,
>results->cfaRegister = 0;
>results->cfaExpression = (int64_t)p;
>length = addressSpace.getULEB128(p, instructionsEnd);
> -  assert(length < std::numeric_limits::max() && "pointer
> overflow");
> +  assert(length < pint_max_value() && "pointer overflow");
>p += static_cast(length);
>_LIBUNWIND_TRACE_DWARF("DW_CFA_def_cfa_expression(expression=0x%"
> PRIx64
>   ", length=%" PRIu64 ")\n",
> @@ -556,7 +559,7 @@ bool CFI_Parser::parseInstructions(A
> , pint_t instructions,
>results->savedRegisters[reg].location = kRegisterAtExpression;
>results->savedRegisters[reg].value = (int64_t)p;
>length = addressSpace.getULEB128(p, instructionsEnd);
> -  assert(length < std::numeric_limits::max() && "pointer
> overflow");
> +  assert(length < pint_max_value() && "pointer overflow");
>p += static_cast(length);
>_LIBUNWIND_TRACE_DWARF("DW_CFA_expression(reg=%" PRIu64 ", "
>   "expression=0x%" PRIx64 ", "
> @@ -642,7 +645,7 @@ bool CFI_Parser::parseInstructions(A
> , pint_t instructions,
>results->savedRegisters[reg].location = kRegisterIsExpression;
>results->savedRegisters[reg].value = (int64_t)p;
>length = addressSpace.getULEB128(p, instructionsEnd);
> -  assert(length < std::numeric_limits::max() && "pointer
> overflow");
> +  assert(length < pint_max_value() && "pointer overflow");
>p += static_cast(length);
>_LIBUNWIND_TRACE_DWARF("DW_CFA_val_expression(reg=%" PRIu64 ", "
>   "expression=0x%" PRIx64 ", length=%" PRIu64
> ")\n",
> --
> 2.11.0
>
>
>> On Mon, Dec 25, 2017 at 11:26 AM, Don Hinton 
>> wrote:
>>
>> On Mon, Dec 25, 2017 at 11:09 AM, whitequark
>>>  wrote:
>>> On 2017-12-25 19:04, Don Hinton wrote:
>>> Hi:
>>>
>>> This change breaks in a local debug build, e.g.,:
>>>
>>>
>>> /Users/dhinton/projects/llvm_project/libunwind/src/DwarfPars
>> er.hpp:559:28:
>>
>>> error: no member named 'numeric_limits' in namespace 'std'
>>> assert(length < std::numeric_limits::max() && "pointer
>>> overflow");
>>> ~^
>>>
>>> Sorry, I missed this. Any idea on reformulating the assert in a way
>>> that does not require libcxx headers? Not having them significantly
>>> simplifies bare-metal builds...
>>>
>>
>> Well, assuming pint_t is some unsigned integer type, the max can be
>> found like this:
>>
>>   pint_t max_pint_t = ~0;
>>
>> So, that could be used in a pinch.
>>
>> thanks...
>>> don
>>>
>>> On Mon, Dec 25, 2017 at 5:06 AM, whitequark via cfe-commits
>>>  wrote:
>>>
>>> Author: whitequark
>>> Date: Mon Dec 25 05:06:09 2017
>>> New Revision: 321440
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=321440=rev [1]
>>> [1]
>>> Log:
>>> [libunwind] Avoid using C++ headers.
>>>
>>> This is useful for building libunwind on libcxx-free systems.
>>>
>>> Modified:
>>> libunwind/trunk/src/DwarfParser.hpp
>>>
>>> Modified: libunwind/trunk/src/DwarfParser.hpp
>>> URL:
>>>
>>>
>>> http://llvm.org/viewvc/llvm-project/libunwind/trunk/src/Dwar
>> fParser.hpp?rev=321440=321439=321440=diff

[PATCH] D41573: [x86][icelake][vpclmulqdq]

[Craig Topper via Phabricator via cfe-commits]

craig.topper accepted this revision.
craig.topper added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rC Clang

https://reviews.llvm.org/D41573



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

Re: [libunwind] r321440 - [libunwind] Avoid using C++ headers.

[whitequark via cfe-commits]

On 2017-12-25 19:43, Don Hinton wrote:

Here's the patch I applied locally.

hth...
don
[snip]


I've committed a slightly beautified version of the patch (below)
as r321446. Cheers!

From 8a4760bafc1123f09438587ee5432eabdec3d33d Mon Sep 17 00:00:00 2001
From: whitequark 
Date: Mon, 25 Dec 2017 20:03:40 +
Subject: [PATCH] [libunwind] Unbreak debug builds after r321440.

---
 src/DwarfParser.hpp | 11 +++
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/src/DwarfParser.hpp b/src/DwarfParser.hpp
index 518101e..645ac21 100644
--- a/src/DwarfParser.hpp
+++ b/src/DwarfParser.hpp
@@ -17,7 +17,6 @@
 #include 
 #include 
 #include 
-#include 

 #include "libunwind.h"
 #include "dwarf2.h"
@@ -26,6 +25,10 @@

 namespace libunwind {

+// Avoid relying on C++ headers.
+template 
+static constexpr T pint_max_value() { return ~0; }
+
 /// CFI_Parser does basic parsing of a CFI (Call Frame Information) 
records.

 /// See DWARF Spec for details:
 ///
http://refspecs.linuxbase.org/LSB_3.1.0/LSB-Core-generic/LSB-Core-generic/ehframechpt.html
@@ -540,7 +543,7 @@ bool CFI_Parser::parseInstructions(A 
, pint_t instructions,

   results->cfaRegister = 0;
   results->cfaExpression = (int64_t)p;
   length = addressSpace.getULEB128(p, instructionsEnd);
-  assert(length < std::numeric_limits::max() && "pointer 
overflow");

+  assert(length < pint_max_value() && "pointer overflow");
   p += static_cast(length);
   _LIBUNWIND_TRACE_DWARF("DW_CFA_def_cfa_expression(expression=0x%" 
PRIx64

  ", length=%" PRIu64 ")\n",
@@ -556,7 +559,7 @@ bool CFI_Parser::parseInstructions(A 
, pint_t instructions,

   results->savedRegisters[reg].location = kRegisterAtExpression;
   results->savedRegisters[reg].value = (int64_t)p;
   length = addressSpace.getULEB128(p, instructionsEnd);
-  assert(length < std::numeric_limits::max() && "pointer 
overflow");

+  assert(length < pint_max_value() && "pointer overflow");
   p += static_cast(length);
   _LIBUNWIND_TRACE_DWARF("DW_CFA_expression(reg=%" PRIu64 ", "
  "expression=0x%" PRIx64 ", "
@@ -642,7 +645,7 @@ bool CFI_Parser::parseInstructions(A 
, pint_t instructions,

   results->savedRegisters[reg].location = kRegisterIsExpression;
   results->savedRegisters[reg].value = (int64_t)p;
   length = addressSpace.getULEB128(p, instructionsEnd);
-  assert(length < std::numeric_limits::max() && "pointer 
overflow");

+  assert(length < pint_max_value() && "pointer overflow");
   p += static_cast(length);
   _LIBUNWIND_TRACE_DWARF("DW_CFA_val_expression(reg=%" PRIu64 ", "
  "expression=0x%" PRIx64 ", length=%" 
PRIu64 ")\n",

--
2.11.0



On Mon, Dec 25, 2017 at 11:26 AM, Don Hinton 
wrote:


On Mon, Dec 25, 2017 at 11:09 AM, whitequark
 wrote:
On 2017-12-25 19:04, Don Hinton wrote:
Hi:

This change breaks in a local debug build, e.g.,:



/Users/dhinton/projects/llvm_project/libunwind/src/DwarfParser.hpp:559:28:

error: no member named 'numeric_limits' in namespace 'std'
assert(length < std::numeric_limits::max() && "pointer
overflow");
~^

Sorry, I missed this. Any idea on reformulating the assert in a way
that does not require libcxx headers? Not having them significantly
simplifies bare-metal builds...


Well, assuming pint_t is some unsigned integer type, the max can be
found like this:

  pint_t max_pint_t = ~0;

So, that could be used in a pinch.


thanks...
don

On Mon, Dec 25, 2017 at 5:06 AM, whitequark via cfe-commits
 wrote:

Author: whitequark
Date: Mon Dec 25 05:06:09 2017
New Revision: 321440

URL: http://llvm.org/viewvc/llvm-project?rev=321440=rev [1]
[1]
Log:
[libunwind] Avoid using C++ headers.

This is useful for building libunwind on libcxx-free systems.

Modified:
libunwind/trunk/src/DwarfParser.hpp

Modified: libunwind/trunk/src/DwarfParser.hpp
URL:



http://llvm.org/viewvc/llvm-project/libunwind/trunk/src/DwarfParser.hpp?rev=321440=321439=321440=diff

[2]
[2]



==

--- libunwind/trunk/src/DwarfParser.hpp (original)
+++ libunwind/trunk/src/DwarfParser.hpp Mon Dec 25 05:06:09 2017
@@ -17,7 +17,7 @@
#include 
#include 
#include 
-#include 
+#include 

#include "libunwind.h"
#include "dwarf2.h"

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits [3] [3]

Links:
--
[1] http://llvm.org/viewvc/llvm-project?rev=321440view=rev [4]
[2]


http://llvm.org/viewvc/llvm-project/libunwind/trunk/src/DwarfParser.hpp?rev=321440r1=321439r2=321440view=diff

[5]
[3] http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits [3]


--
whitequark



Links:
--
[1] http://llvm.org/viewvc/llvm-project?rev=321440view=rev

[libunwind] r321446 - [libunwind] Unbreak debug builds after r321440.

[whitequark via cfe-commits]

Author: whitequark
Date: Mon Dec 25 12:04:47 2017
New Revision: 321446

URL: http://llvm.org/viewvc/llvm-project?rev=321446=rev
Log:
[libunwind] Unbreak debug builds after r321440.

Modified:
libunwind/trunk/src/DwarfParser.hpp

Modified: libunwind/trunk/src/DwarfParser.hpp
URL: 
http://llvm.org/viewvc/llvm-project/libunwind/trunk/src/DwarfParser.hpp?rev=321446=321445=321446=diff
==
--- libunwind/trunk/src/DwarfParser.hpp (original)
+++ libunwind/trunk/src/DwarfParser.hpp Mon Dec 25 12:04:47 2017
@@ -17,7 +17,6 @@
 #include 
 #include 
 #include 
-#include 
 
 #include "libunwind.h"
 #include "dwarf2.h"
@@ -26,6 +25,10 @@
 
 namespace libunwind {
 
+// Avoid relying on C++ headers.
+template 
+static constexpr T pint_max_value() { return ~0; }
+
 /// CFI_Parser does basic parsing of a CFI (Call Frame Information) records.
 /// See DWARF Spec for details:
 ///
http://refspecs.linuxbase.org/LSB_3.1.0/LSB-Core-generic/LSB-Core-generic/ehframechpt.html
@@ -540,7 +543,7 @@ bool CFI_Parser::parseInstructions(A
   results->cfaRegister = 0;
   results->cfaExpression = (int64_t)p;
   length = addressSpace.getULEB128(p, instructionsEnd);
-  assert(length < std::numeric_limits::max() && "pointer 
overflow");
+  assert(length < pint_max_value() && "pointer overflow");
   p += static_cast(length);
   _LIBUNWIND_TRACE_DWARF("DW_CFA_def_cfa_expression(expression=0x%" PRIx64
  ", length=%" PRIu64 ")\n",
@@ -556,7 +559,7 @@ bool CFI_Parser::parseInstructions(A
   results->savedRegisters[reg].location = kRegisterAtExpression;
   results->savedRegisters[reg].value = (int64_t)p;
   length = addressSpace.getULEB128(p, instructionsEnd);
-  assert(length < std::numeric_limits::max() && "pointer 
overflow");
+  assert(length < pint_max_value() && "pointer overflow");
   p += static_cast(length);
   _LIBUNWIND_TRACE_DWARF("DW_CFA_expression(reg=%" PRIu64 ", "
  "expression=0x%" PRIx64 ", "
@@ -642,7 +645,7 @@ bool CFI_Parser::parseInstructions(A
   results->savedRegisters[reg].location = kRegisterIsExpression;
   results->savedRegisters[reg].value = (int64_t)p;
   length = addressSpace.getULEB128(p, instructionsEnd);
-  assert(length < std::numeric_limits::max() && "pointer 
overflow");
+  assert(length < pint_max_value() && "pointer overflow");
   p += static_cast(length);
   _LIBUNWIND_TRACE_DWARF("DW_CFA_val_expression(reg=%" PRIu64 ", "
  "expression=0x%" PRIx64 ", length=%" PRIu64 ")\n",


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

Re: [libunwind] r321440 - [libunwind] Avoid using C++ headers.

[Don Hinton via cfe-commits]

Here's the patch I applied locally.

hth...
don


diff --git a/src/DwarfParser.hpp b/src/DwarfParser.hpp
index 518101e..ac4f1c4 100644
--- a/src/DwarfParser.hpp
+++ b/src/DwarfParser.hpp
@@ -540,7 +540,7 @@ bool CFI_Parser::parseInstructions(A ,
pint_t instructions,
   results->cfaRegister = 0;
   results->cfaExpression = (int64_t)p;
   length = addressSpace.getULEB128(p, instructionsEnd);
-  assert(length < std::numeric_limits::max() && "pointer
overflow");
+  assert(length < static_cast(~0) && "pointer overflow");
   p += static_cast(length);
   _LIBUNWIND_TRACE_DWARF("DW_CFA_def_cfa_expression(expression=0x%"
PRIx64
  ", length=%" PRIu64 ")\n",
@@ -556,7 +556,7 @@ bool CFI_Parser::parseInstructions(A ,
pint_t instructions,
   results->savedRegisters[reg].location = kRegisterAtExpression;
   results->savedRegisters[reg].value = (int64_t)p;
   length = addressSpace.getULEB128(p, instructionsEnd);
-  assert(length < std::numeric_limits::max() && "pointer
overflow");
+  assert(length < static_cast(~0) && "pointer overflow");
   p += static_cast(length);
   _LIBUNWIND_TRACE_DWARF("DW_CFA_expression(reg=%" PRIu64 ", "
  "expression=0x%" PRIx64 ", "
@@ -642,7 +642,7 @@ bool CFI_Parser::parseInstructions(A ,
pint_t instructions,
   results->savedRegisters[reg].location = kRegisterIsExpression;
   results->savedRegisters[reg].value = (int64_t)p;
   length = addressSpace.getULEB128(p, instructionsEnd);
-  assert(length < std::numeric_limits::max() && "pointer
overflow");
+  assert(length < static_cast(~0) && "pointer overflow");
   p += static_cast(length);
   _LIBUNWIND_TRACE_DWARF("DW_CFA_val_expression(reg=%" PRIu64 ", "
  "expression=0x%" PRIx64 ", length=%" PRIu64
")\n",

On Mon, Dec 25, 2017 at 11:26 AM, Don Hinton  wrote:

>
> On Mon, Dec 25, 2017 at 11:09 AM, whitequark 
> wrote:
>
>> On 2017-12-25 19:04, Don Hinton wrote:
>>
>>> Hi:
>>>
>>> This change breaks in a local debug build, e.g.,:
>>>
>>> /Users/dhinton/projects/llvm_project/libunwind/src/DwarfPars
>>> er.hpp:559:28:
>>> error: no member named 'numeric_limits' in namespace 'std'
>>>   assert(length < std::numeric_limits::max() && "pointer
>>> overflow");
>>>   ~^
>>>
>>
>> Sorry, I missed this. Any idea on reformulating the assert in a way
>> that does not require libcxx headers? Not having them significantly
>> simplifies bare-metal builds...
>>
>
> Well, assuming pint_t is some unsigned integer type, the max can be found
> like this:
>
>   pint_t max_pint_t = ~0;
>
> So, that could be used in a pinch.
>
>
>>
>>
>>> thanks...
>>> don
>>>
>>> On Mon, Dec 25, 2017 at 5:06 AM, whitequark via cfe-commits
>>>  wrote:
>>>
>>> Author: whitequark
 Date: Mon Dec 25 05:06:09 2017
 New Revision: 321440

 URL: http://llvm.org/viewvc/llvm-project?rev=321440=rev [1]
 Log:
 [libunwind] Avoid using C++ headers.

 This is useful for building libunwind on libcxx-free systems.

 Modified:
 libunwind/trunk/src/DwarfParser.hpp

 Modified: libunwind/trunk/src/DwarfParser.hpp
 URL:

 http://llvm.org/viewvc/llvm-project/libunwind/trunk/src/Dwar
>>> fParser.hpp?rev=321440=321439=321440=diff
>>>
 [2]

 
>>> ==
>>>
 --- libunwind/trunk/src/DwarfParser.hpp (original)
 +++ libunwind/trunk/src/DwarfParser.hpp Mon Dec 25 05:06:09 2017
 @@ -17,7 +17,7 @@
 #include 
 #include 
 #include 
 -#include 
 +#include 

 #include "libunwind.h"
 #include "dwarf2.h"

 ___
 cfe-commits mailing list
 cfe-commits@lists.llvm.org
 http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits [3]

>>>
>>>
>>>
>>> Links:
>>> --
>>> [1] http://llvm.org/viewvc/llvm-project?rev=321440view=rev
>>> [2]
>>> http://llvm.org/viewvc/llvm-project/libunwind/trunk/src/Dwar
>>> fParser.hpp?rev=321440r1=321439r2=321440view=diff
>>> [3] http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>>
>>
>> --
>> whitequark
>>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [libunwind] r321440 - [libunwind] Avoid using C++ headers.

[Don Hinton via cfe-commits]

On Mon, Dec 25, 2017 at 11:09 AM, whitequark 
wrote:

> On 2017-12-25 19:04, Don Hinton wrote:
>
>> Hi:
>>
>> This change breaks in a local debug build, e.g.,:
>>
>> /Users/dhinton/projects/llvm_project/libunwind/src/DwarfPars
>> er.hpp:559:28:
>> error: no member named 'numeric_limits' in namespace 'std'
>>   assert(length < std::numeric_limits::max() && "pointer
>> overflow");
>>   ~^
>>
>
> Sorry, I missed this. Any idea on reformulating the assert in a way
> that does not require libcxx headers? Not having them significantly
> simplifies bare-metal builds...
>

Well, assuming pint_t is some unsigned integer type, the max can be found
like this:

  pint_t max_pint_t = ~0;

So, that could be used in a pinch.


>
>
>> thanks...
>> don
>>
>> On Mon, Dec 25, 2017 at 5:06 AM, whitequark via cfe-commits
>>  wrote:
>>
>> Author: whitequark
>>> Date: Mon Dec 25 05:06:09 2017
>>> New Revision: 321440
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=321440=rev [1]
>>> Log:
>>> [libunwind] Avoid using C++ headers.
>>>
>>> This is useful for building libunwind on libcxx-free systems.
>>>
>>> Modified:
>>> libunwind/trunk/src/DwarfParser.hpp
>>>
>>> Modified: libunwind/trunk/src/DwarfParser.hpp
>>> URL:
>>>
>>> http://llvm.org/viewvc/llvm-project/libunwind/trunk/src/Dwar
>> fParser.hpp?rev=321440=321439=321440=diff
>>
>>> [2]
>>>
>>> 
>> ==
>>
>>> --- libunwind/trunk/src/DwarfParser.hpp (original)
>>> +++ libunwind/trunk/src/DwarfParser.hpp Mon Dec 25 05:06:09 2017
>>> @@ -17,7 +17,7 @@
>>> #include 
>>> #include 
>>> #include 
>>> -#include 
>>> +#include 
>>>
>>> #include "libunwind.h"
>>> #include "dwarf2.h"
>>>
>>> ___
>>> cfe-commits mailing list
>>> cfe-commits@lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits [3]
>>>
>>
>>
>>
>> Links:
>> --
>> [1] http://llvm.org/viewvc/llvm-project?rev=321440view=rev
>> [2]
>> http://llvm.org/viewvc/llvm-project/libunwind/trunk/src/Dwar
>> fParser.hpp?rev=321440r1=321439r2=321440view=diff
>> [3] http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>
>
> --
> whitequark
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D41414: [analyzer] Add keyboard j/k navigation to HTML reports

[David Blaikie via cfe-commits]

any chance this can be implemented based on keyboard layout, so it's good
for dvorak users as well? (maybe it already is, I don't know - just
mentioning it in case)

On Thu, Dec 21, 2017 at 2:58 PM George Karpenkov via Phabricator via
cfe-commits  wrote:

> This revision was automatically updated to reflect the committed changes.
> Closed by commit rC321320: [analyzer] Add Javascript to analyzer HTML
> output to allow keyboard navigation. (authored by george.karpenkov,
> committed by ).
> Herald added a subscriber: cfe-commits.
>
> Changed prior to commit:
>   https://reviews.llvm.org/D41414?vs=127919=127954#toc
>
> Repository:
>   rC Clang
>
> https://reviews.llvm.org/D41414
>
> Files:
>   lib/Rewrite/HTMLRewrite.cpp
>   lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r321319 - [ODRHash] Canonicalize Decl's before processing.

[David Blaikie via cfe-commits]

Test case?

On Thu, Dec 21, 2017 at 2:39 PM Richard Trieu via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: rtrieu
> Date: Thu Dec 21 14:38:29 2017
> New Revision: 321319
>
> URL: http://llvm.org/viewvc/llvm-project?rev=321319=rev
> Log:
> [ODRHash] Canonicalize Decl's before processing.
>
> Canonicalizing the Decl before processing it as part of the hash should
> reduce
> issues with non-canonical types showing up as mismatches.
>
> Modified:
> cfe/trunk/lib/AST/ODRHash.cpp
>
> Modified: cfe/trunk/lib/AST/ODRHash.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ODRHash.cpp?rev=321319=321318=321319=diff
>
> ==
> --- cfe/trunk/lib/AST/ODRHash.cpp (original)
> +++ cfe/trunk/lib/AST/ODRHash.cpp Thu Dec 21 14:38:29 2017
> @@ -468,6 +468,7 @@ void ODRHash::AddCXXRecordDecl(const CXX
>
>  void ODRHash::AddDecl(const Decl *D) {
>assert(D && "Expecting non-null pointer.");
> +  D = D->getCanonicalDecl();
>auto Result = DeclMap.insert(std::make_pair(D, DeclMap.size()));
>ID.AddInteger(Result.first->second);
>// On first encounter of a Decl pointer, process it.  Every time
> afterwards,
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [libunwind] r321440 - [libunwind] Avoid using C++ headers.

[whitequark via cfe-commits]

On 2017-12-25 19:04, Don Hinton wrote:

Hi:

This change breaks in a local debug build, e.g.,:

/Users/dhinton/projects/llvm_project/libunwind/src/DwarfParser.hpp:559:28:
error: no member named 'numeric_limits' in namespace 'std'
  assert(length < std::numeric_limits::max() && "pointer
overflow");
  ~^


Sorry, I missed this. Any idea on reformulating the assert in a way
that does not require libcxx headers? Not having them significantly
simplifies bare-metal builds...



thanks...
don

On Mon, Dec 25, 2017 at 5:06 AM, whitequark via cfe-commits
 wrote:


Author: whitequark
Date: Mon Dec 25 05:06:09 2017
New Revision: 321440

URL: http://llvm.org/viewvc/llvm-project?rev=321440=rev [1]
Log:
[libunwind] Avoid using C++ headers.

This is useful for building libunwind on libcxx-free systems.

Modified:
libunwind/trunk/src/DwarfParser.hpp

Modified: libunwind/trunk/src/DwarfParser.hpp
URL:


http://llvm.org/viewvc/llvm-project/libunwind/trunk/src/DwarfParser.hpp?rev=321440=321439=321440=diff

[2]


==

--- libunwind/trunk/src/DwarfParser.hpp (original)
+++ libunwind/trunk/src/DwarfParser.hpp Mon Dec 25 05:06:09 2017
@@ -17,7 +17,7 @@
#include 
#include 
#include 
-#include 
+#include 

#include "libunwind.h"
#include "dwarf2.h"

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits [3]




Links:
--
[1] http://llvm.org/viewvc/llvm-project?rev=321440view=rev
[2]
http://llvm.org/viewvc/llvm-project/libunwind/trunk/src/DwarfParser.hpp?rev=321440r1=321439r2=321440view=diff
[3] http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

Re: [libunwind] r321440 - [libunwind] Avoid using C++ headers.

[Don Hinton via cfe-commits]

Hi:

This change breaks in a local debug build, e.g.,:

/Users/dhinton/projects/llvm_project/libunwind/src/DwarfParser.hpp:559:28:
error: no member named 'numeric_limits' in namespace 'std'
  assert(length < std::numeric_limits::max() && "pointer
overflow");
  ~^

thanks...
don

On Mon, Dec 25, 2017 at 5:06 AM, whitequark via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: whitequark
> Date: Mon Dec 25 05:06:09 2017
> New Revision: 321440
>
> URL: http://llvm.org/viewvc/llvm-project?rev=321440=rev
> Log:
> [libunwind] Avoid using C++ headers.
>
> This is useful for building libunwind on libcxx-free systems.
>
> Modified:
> libunwind/trunk/src/DwarfParser.hpp
>
> Modified: libunwind/trunk/src/DwarfParser.hpp
> URL: http://llvm.org/viewvc/llvm-project/libunwind/trunk/src/
> DwarfParser.hpp?rev=321440=321439=321440=diff
> 
> ==
> --- libunwind/trunk/src/DwarfParser.hpp (original)
> +++ libunwind/trunk/src/DwarfParser.hpp Mon Dec 25 05:06:09 2017
> @@ -17,7 +17,7 @@
>  #include 
>  #include 
>  #include 
> -#include 
> +#include 
>
>  #include "libunwind.h"
>  #include "dwarf2.h"
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [clang-tools-extra] r321192 - [clangd] Remove an unused lambda capture.

[David Blaikie via cfe-commits]

Generally I'd encourage you/anyone to use [&] for any lambda that doesn't
escape its scope - avoids any issues like this & doesn't really hinder
readability imho.

On Wed, Dec 20, 2017 at 9:23 AM Eric Liu via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: ioeric
> Date: Wed Dec 20 09:22:56 2017
> New Revision: 321192
>
> URL: http://llvm.org/viewvc/llvm-project?rev=321192=rev
> Log:
> [clangd] Remove an unused lambda capture.
>
> Modified:
> clang-tools-extra/trunk/unittests/clangd/Annotations.cpp
>
> Modified: clang-tools-extra/trunk/unittests/clangd/Annotations.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/Annotations.cpp?rev=321192=321191=321192=diff
>
> ==
> --- clang-tools-extra/trunk/unittests/clangd/Annotations.cpp (original)
> +++ clang-tools-extra/trunk/unittests/clangd/Annotations.cpp Wed Dec 20
> 09:22:56 2017
> @@ -24,7 +24,7 @@ static void require(bool Assertion, cons
>
>  Annotations::Annotations(StringRef Text) {
>auto Here = [this] { return offsetToPosition(Code, Code.size()); };
> -  auto Require = [this, Text](bool Assertion, const char *Msg) {
> +  auto Require = [Text](bool Assertion, const char *Msg) {
>  require(Assertion, Msg, Text);
>};
>Optional Name;
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[libunwind] r321445 - [libunwind] Add proper support for DWARF unwind on bare metal.

[whitequark via cfe-commits]

Author: whitequark
Date: Mon Dec 25 09:05:07 2017
New Revision: 321445

URL: http://llvm.org/viewvc/llvm-project?rev=321445=rev
Log:
[libunwind] Add proper support for DWARF unwind on bare metal.

Right now, ARM EHABI unwind on bare metal expects to find the symbols
__exidx_start and __exidx_end defined, and uses those to locate
the EH tables. However, DWARF unwind on bare metal expects to find
dl_iterate_phdr, which, although possible to provide, is inconvenient
and mildly absurd.

This commit provides feature parity with ARM EHABI unwind by looking
for symbols __eh_frame_start, __eh_frame_end, __eh_frame_hdr_start
and __eh_frame_hdr_end, denoting the start and end of the sections
with corresponding names. As far as I know, there is no de jure or
de facto ABI providing any such names, so I chose the obvious ones.

The .eh_frame_hdr support is optional for maximum flexibility and
possible space savings (e.g. if libunwind is only used to provide
backtraces when a device crashes, providing the .eh_frame_hdr, which
is an index for rapid access to EH tables, would be a waste.)

The support for .eh_frame_hdr/DWARF index in the first place is
conditional on defined(_LIBUNWIND_SUPPORT_DWARF_INDEX), although
right now config.h will always define this macro.

The support for DWARF unwind on bare metal has been validated within
the ARTIQ environment[1].

  [1]: https://m-labs.hk/artiq/

Modified:
libunwind/trunk/src/AddressSpace.hpp

Modified: libunwind/trunk/src/AddressSpace.hpp
URL: 
http://llvm.org/viewvc/llvm-project/libunwind/trunk/src/AddressSpace.hpp?rev=321445=321444=321445=diff
==
--- libunwind/trunk/src/AddressSpace.hpp (original)
+++ libunwind/trunk/src/AddressSpace.hpp Mon Dec 25 09:05:07 2017
@@ -83,6 +83,38 @@ namespace libunwind {
 }
   #endif
 
+#elif defined(_LIBUNWIND_SUPPORT_DWARF_UNWIND) && 
defined(_LIBUNWIND_IS_BAREMETAL)
+
+// When statically linked on bare-metal, the symbols for the EH table are 
looked
+// up without going through the dynamic loader.
+
+// The following linker script may be used to produce the necessary sections 
and symbols.
+// Unless the --eh-frame-hdr linker option is provided, the section is not 
generated
+// and does not take space in the output file.
+//
+//   .eh_frame :
+//   {
+//   __eh_frame_start = .;
+//   KEEP(*(.eh_frame))
+//   __eh_frame_end = .;
+//   }
+//
+//   .eh_frame_hdr :
+//   {
+//   KEEP(*(.eh_frame_hdr))
+//   }
+//
+//   __eh_frame_hdr_start = SIZEOF(.eh_frame_hdr) > 0 ? ADDR(.eh_frame_hdr) : 
0;
+//   __eh_frame_hdr_end = SIZEOF(.eh_frame_hdr) > 0 ? . : 0;
+
+extern char __eh_frame_start;
+extern char __eh_frame_end;
+
+#if defined(_LIBUNWIND_SUPPORT_DWARF_INDEX)
+extern char __eh_frame_hdr_start;
+extern char __eh_frame_hdr_end;
+#endif
+
 #elif defined(_LIBUNWIND_ARM_EHABI) && defined(_LIBUNWIND_IS_BAREMETAL)
 
 // When statically linked on bare-metal, the symbols for the EH table are 
looked
@@ -348,6 +380,20 @@ inline bool LocalAddressSpace::findUnwin
 info.compact_unwind_section_length = 
dyldInfo.compact_unwind_section_length;
 return true;
   }
+#elif defined(_LIBUNWIND_SUPPORT_DWARF_UNWIND) && 
defined(_LIBUNWIND_IS_BAREMETAL)
+  // Bare metal is statically linked, so no need to ask the dynamic loader
+  info.dwarf_section_length = (uintptr_t)(&__eh_frame_end - &__eh_frame_start);
+  info.dwarf_section =(uintptr_t)(&__eh_frame_start);
+  _LIBUNWIND_TRACE_UNWINDING("findUnwindSections: section %X length %x",
+ info.dwarf_section, info.dwarf_section_length);
+#if defined(_LIBUNWIND_SUPPORT_DWARF_INDEX)
+  info.dwarf_index_section =(uintptr_t)(&__eh_frame_hdr_start);
+  info.dwarf_index_section_length = (uintptr_t)(&__eh_frame_hdr_end - 
&__eh_frame_hdr_start);
+  _LIBUNWIND_TRACE_UNWINDING("findUnwindSections: index section %X length %x",
+ info.dwarf_index_section, 
info.dwarf_index_section_length);
+#endif
+  if (info.dwarf_section_length)
+return true;
 #elif defined(_LIBUNWIND_ARM_EHABI) && defined(_LIBUNWIND_IS_BAREMETAL)
   // Bare metal is statically linked, so no need to ask the dynamic loader
   info.arm_section =(uintptr_t)(&__exidx_start);


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

[PATCH] D41326: [clang-tidy] Added diagnostics about incorrect usage of NOLINT comment

[Anton via Phabricator via cfe-commits]

xgsa marked 5 inline comments as done.
xgsa added inline comments.



Comment at: clang-tidy/ClangTidyDiagnosticConsumer.cpp:276
+
+  using NolintMap = std::unordered_map xgsa wrote:
> > aaron.ballman wrote:
> > > Is there a better LLVM ADT to use here?
> > This data structure provides the fast lookup by check name+line number and 
> > it's exactly what is necessary. What are the concerns about this data 
> > structure?
> Same as above.
I have reviewed llvm guide [1] and found that it recommends using ordered 
vector in such cases. I have implemented this approach, however I'd like to 
notice that lookup complexity in `reportRedundantNolintComments()` increased 
from average O(1) for `std::unordered_map` to O(log(N)) for binary search. 
However, memory usage become less. As this collection is gathered for each 
translation unit, I don't expect millions of `NOLINT` comments in it, thus this 
approach looks suitable.

[1] - 
http://llvm.org/docs/ProgrammersManual.html#picking-the-right-data-structure-for-a-task



Comment at: clang-tidy/ClangTidyDiagnosticConsumer.cpp:424
+// This method is not const, because it fills cache on first demand.
+// It is possible to fill cache in constructor or make cache volatile
+// and make this method const, but they seem like worse solutions.

aaron.ballman wrote:
> xgsa wrote:
> > aaron.ballman wrote:
> > > Making the cache volatile will have no impact on this.
> > > 
> > > Any reason not to make the cache `mutable`, however? That's quite common 
> > > for implementation details.
> > Sorry, certainly, instead of "volatile" I meant "mutable".
> > 
> > Actually, using of "mutable" violates a constancy contract, as the field is 
> > get modified in a const method. Thus I'd tend to avoid using `mutable`, if 
> > possible, because e.g. in multi-threaded applications these fields require 
> > additional protection/synchronization. Moreover, I see that using of  
> > `mutable` is not very spread in clang-tidy. Thus as, currently, `hasCheck` 
> > is called from the non-constant context, I'd prefer leaving it non-const 
> > instead of making cache `mutable`. Please, let me know if you insist on the 
> > `mutable` option.
> Use of mutable does not violate constancy; the cache is not exposed via any 
> interface; it is purely an implementation detail. Very little of our code 
> base is concerned with multithreaded scenarios (we use bit-fields 
> *everywhere*, for instance).
> 
> I won't insist on using `mutable` if you are set against it, but this is the 
> exact scenario in which it is the correct solution.
I've just tried the `mutalbe`-approach and discovered one more issue: 
`ClangTidyASTConsumerFactory` requires non constant `ClangTidyContext`. Thus, 
if current approach is not suitable, either `ClangTidyASTConsumerFactory` 
should be refactored on const and nonconst parts or caching should be done in 
constructor (which makes  fetching the names not lazy). Because of this, 
currently I am suggesting to leave `hasCheck` nonconstant.



Comment at: clang-tidy/ClangTidyDiagnosticConsumer.cpp:839-840
+case NolintCommentType::Nolint:
+  Message = "there is no diagnostics on this line, "
+"the NOLINT comment is redundant";
+  break;

aaron.ballman wrote:
> xgsa wrote:
> > aaron.ballman wrote:
> > > I don't think the user is going to care about the distinction between no 
> > > diagnostics being triggered and the expected diagnostic not being 
> > > triggered. Also, it's dangerous to claim the lint comment is redundant 
> > > because it's possible the user has NOLINT(foo, bar) and while foo is not 
> > > triggered, bar still is. The NOLINT comment itself isn't redundant, it's 
> > > that the check specified doesn't occur.
> > > 
> > > I would consolidate those scenarios into a single diagnostic: "expected 
> > > diagnostic '%0' not generated" and "expected diagnostic '%0' not 
> > > generated for the following line".
> > > 
> > > One concern I have with this functionality is: how should users silence a 
> > > lint diagnostic that's target sensitive? e.g., a diagnostic that triggers 
> > > based on the underlying type of size_t or the signedness of plain char. 
> > > In that case, the diagnostic may trigger for some targets but not others, 
> > > but on the targets where the diagnostic is not triggered, they now get a 
> > > diagnostic they cannot silence. There should be a way to silence the "bad 
> > > NOLINT" diagnostics.
> > > I don't think the user is going to care about the distinction between no 
> > > diagnostics being triggered and the expected diagnostic not being 
> > > triggered. Also, it's dangerous to claim the lint comment is redundant 
> > > because it's possible the user has NOLINT(foo, bar) and while foo is not 
> > > triggered, 

[PATCH] D41326: [clang-tidy] Added diagnostics about incorrect usage of NOLINT comment

[Anton via Phabricator via cfe-commits]

xgsa updated this revision to Diff 128144.
xgsa marked an inline comment as done.
xgsa added a comment.
Herald added a subscriber: mgrang.

Review comments applied.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D41326

Files:
  clang-tidy/ClangTidy.cpp
  clang-tidy/ClangTidyDiagnosticConsumer.cpp
  clang-tidy/ClangTidyDiagnosticConsumer.h
  test/clang-tidy/nolint-usage.cpp
  test/clang-tidy/nolint.cpp
  test/clang-tidy/nolintnextline.cpp

Index: test/clang-tidy/nolintnextline.cpp
===
--- test/clang-tidy/nolintnextline.cpp
+++ test/clang-tidy/nolintnextline.cpp
@@ -24,6 +24,18 @@
 class C5 { C5(int i); };
 
 
+void f() {
+  int i;
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: unused variable 'i' [clang-diagnostic-unused-variable]
+  // NOLINTNEXTLINE
+  int j;
+  // NOLINTNEXTLINE(clang-diagnostic-unused-variable)
+  int k;
+  // NOLINTNEXTLINE(clang-diagnostic-literal-conversion)
+  int l;
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: unused variable 'l' [clang-diagnostic-unused-variable]
+}
+
 // NOLINTNEXTLINE
 
 class D { D(int i); };
@@ -44,6 +56,6 @@
 // NOLINTNEXTLINE
 MACRO_NOARG
 
-// CHECK-MESSAGES: Suppressed 8 warnings (8 NOLINT)
+// CHECK-MESSAGES: Suppressed 10 warnings (10 NOLINT)
 
-// RUN: %check_clang_tidy %s google-explicit-constructor %t --
+// RUN: %check_clang_tidy %s google-explicit-constructor,clang-diagnostic-unused-variable %t -- -extra-arg=-Wunused-variable --
Index: test/clang-tidy/nolint.cpp
===
--- test/clang-tidy/nolint.cpp
+++ test/clang-tidy/nolint.cpp
@@ -30,6 +30,9 @@
   int i;
 // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: unused variable 'i' [clang-diagnostic-unused-variable]
   int j; // NOLINT
+  int k; // NOLINT(clang-diagnostic-unused-variable)
+  int l; // NOLINT(clang-diagnostic-literal-conversion)
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: unused variable 'l' [clang-diagnostic-unused-variable]
 }
 
 #define MACRO(X) class X { X(int i); };
@@ -46,4 +49,4 @@
 #define DOUBLE_MACRO MACRO(H) // NOLINT
 DOUBLE_MACRO
 
-// CHECK-MESSAGES: Suppressed 12 warnings (12 NOLINT)
+// CHECK-MESSAGES: Suppressed 13 warnings (13 NOLINT)
Index: test/clang-tidy/nolint-usage.cpp
===
--- /dev/null
+++ test/clang-tidy/nolint-usage.cpp
@@ -0,0 +1,171 @@
+// RUN: %check_clang_tidy %s nolint-usage,google-explicit-constructor,misc-unused-parameters %t --
+
+// Case: NO_LINT on line with an error.
+class A1 { A1(int i); }; // NOLINT
+
+// Case: NO_LINT on line without errors.
+class A2 { explicit A2(int i); }; // NOLINT
+// CHECK-MESSAGES: :[[@LINE-1]]:38: warning: there is no diagnostics on this line, the NOLINT comment is redundant
+
+// Case: NO_LINT for the specific check on line with an error on it.
+class A3 { A3(int i); }; // NOLINT(google-explicit-constructor)
+
+// Case: NO_LINT for the specific check on line with an error on another check.
+class A4 { A4(int i); }; // NOLINT(misc-unused-parameters)
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: single-argument constructors must be marked explicit
+// CHECK-MESSAGES: :[[@LINE-2]]:29: warning: there is no diagnostics for the 'misc-unused-parameters' check on this line, the NOLINT comment is redundant
+
+// Case: NO_LINT for the specific check on line without errors.
+class A5 { explicit A5(int i); }; // NOLINT(google-explicit-constructor)
+// CHECK-MESSAGES: :[[@LINE-1]]:38: warning: there is no diagnostics for the 'google-explicit-constructor' check on this line, the NOLINT comment is redundant
+
+// Case: NO_LINT for unknown check on line with an error on some check.
+class A6 { A6(int i); }; // NOLINT(unknown-check)
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: single-argument constructors must be marked explicit
+// CHECK-MESSAGES: :[[@LINE-2]]:29: warning: unknown check name 'unknown-check' specified in NOLINT comment
+
+// Case: NO_LINT for unknown check on line without errors.
+class A7 { explicit A7(int i); }; // NOLINT(unknown-check)
+// CHECK-MESSAGES: :[[@LINE-1]]:38: warning: unknown check name 'unknown-check' specified in NOLINT comment
+
+// Case: NO_LINT with not closed parenthesis without check names.
+class A8 { A8(int i); }; // NOLINT(
+// CHECK-MESSAGES: :[[@LINE-1]]:29: warning: unbalanced parentheses in NOLINT comment; all diagnostics silenced for this line
+
+// Case: NO_LINT with not closed parenthesis with check names.
+class A9 { A9(int i); }; // NOLINT(unknown-check
+// CHECK-MESSAGES: :[[@LINE-1]]:29: warning: unbalanced parentheses in NOLINT comment; all diagnostics silenced for this line
+
+// Case: NO_LINT with clang diagnostics
+int f() {
+  int i = 0; // NOLINT
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: there is no diagnostics on this line, the NOLINT comment is redundant
+  int j = 0; // NOLINT(clang-diagnostic-unused-variable)
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: there is no 

[PATCH] D40478: Added control flow architecture protection Flag

[Oren Ben Simhon via Phabricator via cfe-commits]

oren_ben_simhon updated this revision to Diff 128143.
oren_ben_simhon added a comment.

Reverted clang-format whitespaces updates


Repository:
  rL LLVM

https://reviews.llvm.org/D40478

Files:
  include/clang/Basic/DiagnosticCommonKinds.td
  include/clang/Basic/TargetInfo.h
  include/clang/Driver/Options.td
  include/clang/Frontend/CodeGenOptions.def
  lib/Basic/Targets/X86.cpp
  lib/Basic/Targets/X86.h
  lib/CodeGen/CodeGenFunction.cpp
  lib/CodeGen/CodeGenModule.cpp
  lib/Driver/ToolChains/Clang.cpp
  lib/Frontend/CompilerInvocation.cpp
  test/CodeGen/x86-cf-protection.c
  test/Driver/clang_f_opts.c

Index: test/Driver/clang_f_opts.c
===
--- test/Driver/clang_f_opts.c
+++ test/Driver/clang_f_opts.c
@@ -503,3 +503,17 @@
 // CHECK-WINDOWS-ISO10646: "-fwchar-type=int"
 // CHECK-WINDOWS-ISO10646: "-fsigned-wchar"
 
+// RUN: %clang -### -S -fcf-protection %s 2>&1 | FileCheck -check-prefix=CHECK-CF-PROTECTION-FULL %s
+// RUN: %clang -### -S %s 2>&1 | FileCheck -check-prefix=CHECK-NO-CF-PROTECTION-FULL %s
+// RUN: %clang -### -S -fcf-protection=full %s 2>&1 | FileCheck -check-prefix=CHECK-CF-PROTECTION-FULL %s
+// RUN: %clang -### -S %s 2>&1 | FileCheck -check-prefix=CHECK-NO-CF-PROTECTION-FULL %s
+// CHECK-CF-PROTECTION-FULL: -fcf-protection=full
+// CHECK-NO-CF-PROTECTION-FULL-NOT: -fcf-protection=full
+// RUN: %clang -### -S -fcf-protection=return %s 2>&1 | FileCheck -check-prefix=CHECK-CF-PROTECTION-RETURN %s
+// RUN: %clang -### -S %s 2>&1 | FileCheck -check-prefix=CHECK-NO-CF-PROTECTION-RETURN %s
+// CHECK-CF-PROTECTION-RETURN: -fcf-protection=return
+// CHECK-NO-CF-PROTECTION-RETURN-NOT: -fcf-protection=return
+// RUN: %clang -### -S -fcf-protection=branch %s 2>&1 | FileCheck -check-prefix=CHECK-CF-PROTECTION-BRANCH %s
+// RUN: %clang -### -S %s 2>&1 | FileCheck -check-prefix=CHECK-NO-CF-PROTECTION-BRANCH %s
+// CHECK-CF-PROTECTION-BRANCH: -fcf-protection=branch
+// CHECK-NO-CF-PROTECTION-BRANCH-NOT: -fcf-protection=branch
Index: test/CodeGen/x86-cf-protection.c
===
--- /dev/null
+++ test/CodeGen/x86-cf-protection.c
@@ -0,0 +1,6 @@
+// RUN: not %clang_cc1 -fsyntax-only -S -emit-llvm -triple i386-unknown-unknown -fcf-protection=return %s 2>&1 | FileCheck %s --check-prefix=RETURN
+// RUN: not %clang_cc1 -fsyntax-only -S -emit-llvm -triple i386-unknown-unknown -fcf-protection=branch %s 2>&1 | FileCheck %s --check-prefix=BRANCH
+
+// RETURN: error: option 'cf-protection=return' cannot be specified without '-mshstk'
+// BRANCH: error: option 'cf-protection=branch' cannot be specified without '-mibt'
+void foo() {}
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -791,6 +791,21 @@
   Opts.CallFEntry = Args.hasArg(OPT_mfentry);
   Opts.EmitOpenCLArgMetadata = Args.hasArg(OPT_cl_kernel_arg_info);
 
+  if (const Arg *A = Args.getLastArg(OPT_fcf_protection_EQ)) {
+StringRef Name = A->getValue();
+if (Name == "full") {
+  Opts.CFProtectionReturn = 1;
+  Opts.CFProtectionBranch = 1;
+} else if (Name == "return")
+  Opts.CFProtectionReturn = 1;
+else if (Name == "branch")
+  Opts.CFProtectionBranch = 1;
+else if (Name != "none") {
+  Diags.Report(diag::err_drv_invalid_value) << A->getAsString(Args) << Name;
+  Success = false;
+}
+  }
+
   if (const Arg *A = Args.getLastArg(OPT_compress_debug_sections,
  OPT_compress_debug_sections_EQ)) {
 if (A->getOption().getID() == OPT_compress_debug_sections) {
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -3977,6 +3977,11 @@
   // Forward -cl options to -cc1
   RenderOpenCLOptions(Args, CmdArgs);
 
+  if (Arg *A = Args.getLastArg(options::OPT_fcf_protection_EQ)) {
+CmdArgs.push_back(
+Args.MakeArgString(Twine("-fcf-protection=") + A->getValue()));
+  }
+
   // Forward -f options with positive and negative forms; we translate
   // these by hand.
   if (Arg *A = getLastProfileSampleUseArg(Args)) {
Index: lib/CodeGen/CodeGenModule.cpp
===
--- lib/CodeGen/CodeGenModule.cpp
+++ lib/CodeGen/CodeGenModule.cpp
@@ -497,6 +497,20 @@
 getModule().addModuleFlag(llvm::Module::Override, "Cross-DSO CFI", 1);
   }
 
+  if (CodeGenOpts.CFProtectionReturn) {
+Target.checkCFProtectionReturnSupported(getDiags());
+// Indicate that we want to instrument return control flow protection.
+getModule().addModuleFlag(llvm::Module::Override, "cf-protection-return",
+  1);
+  }
+
+  if (CodeGenOpts.CFProtectionBranch) {
+Target.checkCFProtectionBranchSupported(getDiags());
+// Indicate 

[PATCH] D40478: Added control flow architecture protection Flag

[Oren Ben Simhon via Phabricator via cfe-commits]

oren_ben_simhon updated this revision to Diff 128141.
oren_ben_simhon added a comment.

Implemented comments posted until 12/24 (Thanks Craig)
Moved cf-protection attributes from function attributes to module attributes.


Repository:
  rL LLVM

https://reviews.llvm.org/D40478

Files:
  include/clang/Basic/DiagnosticCommonKinds.td
  include/clang/Basic/TargetInfo.h
  include/clang/Driver/Options.td
  include/clang/Frontend/CodeGenOptions.def
  lib/Basic/Targets/X86.cpp
  lib/Basic/Targets/X86.h
  lib/CodeGen/CodeGenFunction.cpp
  lib/CodeGen/CodeGenModule.cpp
  lib/Driver/ToolChains/Clang.cpp
  lib/Frontend/CompilerInvocation.cpp
  test/CodeGen/complex-builtins.c
  test/CodeGen/complex-libcalls.c
  test/CodeGen/math-builtins.c
  test/CodeGen/math-libcalls.c
  test/CodeGen/x86-cf-protection.c
  test/Driver/clang_f_opts.c

Index: test/Driver/clang_f_opts.c
===
--- test/Driver/clang_f_opts.c
+++ test/Driver/clang_f_opts.c
@@ -503,3 +503,17 @@
 // CHECK-WINDOWS-ISO10646: "-fwchar-type=int"
 // CHECK-WINDOWS-ISO10646: "-fsigned-wchar"
 
+// RUN: %clang -### -S -fcf-protection %s 2>&1 | FileCheck -check-prefix=CHECK-CF-PROTECTION-FULL %s
+// RUN: %clang -### -S %s 2>&1 | FileCheck -check-prefix=CHECK-NO-CF-PROTECTION-FULL %s
+// RUN: %clang -### -S -fcf-protection=full %s 2>&1 | FileCheck -check-prefix=CHECK-CF-PROTECTION-FULL %s
+// RUN: %clang -### -S %s 2>&1 | FileCheck -check-prefix=CHECK-NO-CF-PROTECTION-FULL %s
+// CHECK-CF-PROTECTION-FULL: -fcf-protection=full
+// CHECK-NO-CF-PROTECTION-FULL-NOT: -fcf-protection=full
+// RUN: %clang -### -S -fcf-protection=return %s 2>&1 | FileCheck -check-prefix=CHECK-CF-PROTECTION-RETURN %s
+// RUN: %clang -### -S %s 2>&1 | FileCheck -check-prefix=CHECK-NO-CF-PROTECTION-RETURN %s
+// CHECK-CF-PROTECTION-RETURN: -fcf-protection=return
+// CHECK-NO-CF-PROTECTION-RETURN-NOT: -fcf-protection=return
+// RUN: %clang -### -S -fcf-protection=branch %s 2>&1 | FileCheck -check-prefix=CHECK-CF-PROTECTION-BRANCH %s
+// RUN: %clang -### -S %s 2>&1 | FileCheck -check-prefix=CHECK-NO-CF-PROTECTION-BRANCH %s
+// CHECK-CF-PROTECTION-BRANCH: -fcf-protection=branch
+// CHECK-NO-CF-PROTECTION-BRANCH-NOT: -fcf-protection=branch
Index: test/CodeGen/x86-cf-protection.c
===
--- /dev/null
+++ test/CodeGen/x86-cf-protection.c
@@ -0,0 +1,6 @@
+// RUN: not %clang_cc1 -fsyntax-only -S -emit-llvm -triple i386-unknown-unknown -fcf-protection=return %s 2>&1 | FileCheck %s --check-prefix=RETURN
+// RUN: not %clang_cc1 -fsyntax-only -S -emit-llvm -triple i386-unknown-unknown -fcf-protection=branch %s 2>&1 | FileCheck %s --check-prefix=BRANCH
+
+// RETURN: error: option 'cf-protection=return' cannot be specified without '-mshstk'
+// BRANCH: error: option 'cf-protection=branch' cannot be specified without '-mibt'
+void foo() {}
Index: test/CodeGen/math-libcalls.c
===
--- test/CodeGen/math-libcalls.c
+++ test/CodeGen/math-libcalls.c
@@ -532,7 +532,6 @@
 // HAS_ERRNO: declare x86_fp80 @truncl(x86_fp80) [[READNONE]]
 };
 
-
 // NO__ERRNO: attributes [[READNONE]] = { {{.*}}readnone{{.*}} }
 // NO__ERRNO: attributes [[NOT_READNONE]] = { nounwind "correctly{{.*}} }
 // NO__ERRNO: attributes [[READONLY]] = { {{.*}}readonly{{.*}} }
Index: test/CodeGen/math-builtins.c
===
--- test/CodeGen/math-builtins.c
+++ test/CodeGen/math-builtins.c
@@ -562,7 +562,6 @@
 // HAS_ERRNO: declare x86_fp80 @llvm.trunc.f80(x86_fp80) [[READNONE_INTRINSIC]]
 };
 
-
 // NO__ERRNO: attributes [[READNONE]] = { {{.*}}readnone{{.*}} }
 // NO__ERRNO: attributes [[READNONE_INTRINSIC]] = { {{.*}}readnone{{.*}} }
 // NO__ERRNO: attributes [[NOT_READNONE]] = { nounwind "correctly{{.*}} }
Index: test/CodeGen/complex-libcalls.c
===
--- test/CodeGen/complex-libcalls.c
+++ test/CodeGen/complex-libcalls.c
@@ -199,10 +199,8 @@
 // HAS_ERRNO: declare { x86_fp80, x86_fp80 } @ctanhl({ x86_fp80, x86_fp80 }* byval align 16) [[NOT_READNONE]]
 };
 
-
 // NO__ERRNO: attributes [[READNONE]] = { {{.*}}readnone{{.*}} }
 // NO__ERRNO: attributes [[NOT_READNONE]] = { nounwind "correctly{{.*}} }
 
 // HAS_ERRNO: attributes [[NOT_READNONE]] = { nounwind "correctly{{.*}} }
 // HAS_ERRNO: attributes [[READNONE]] = { {{.*}}readnone{{.*}} }
-
Index: test/CodeGen/complex-builtins.c
===
--- test/CodeGen/complex-builtins.c
+++ test/CodeGen/complex-builtins.c
@@ -197,10 +197,8 @@
 // HAS_ERRNO: declare { x86_fp80, x86_fp80 } @ctanhl({ x86_fp80, x86_fp80 }* byval align 16) [[NOT_READNONE]]
 };
 
-
 // NO__ERRNO: attributes [[READNONE]] = { {{.*}}readnone{{.*}} }
 // NO__ERRNO: attributes [[NOT_READNONE]] = { nounwind "correctly{{.*}} }
 
 // HAS_ERRNO: attributes [[NOT_READNONE]] = { nounwind 

[PATCH] D40478: Added control flow architecture protection Flag

[Oren Ben Simhon via Phabricator via cfe-commits]

oren_ben_simhon added a comment.

In https://reviews.llvm.org/D40478#962348, @craig.topper wrote:

> Are we sure we want a different command line option name from gcc? From our 
> internal conversations with the gcc folks I thought they were suggesting that 
> -fcf-protection could imply a software mechanism if a hardware mechanism was 
> not available thorugh -mibt or -march?
>
> Should we emit an error to the user if -mibt isn't available?  We should be 
> able to add virtual methods on TargetInfo that X86 can customize to check for 
> ibt and shstk.
>
> Can you provide more information about the miscompile on MSVC? I think we 
> should do more to understand that, this sounds like it could be a time bomb 
> waiting to fail somewhere else.


LLVM already has a flag for SW mechanisms "-sanitize=*". Anyway I believe that 
GCC and LLVM should agree first before i change it. I restored cf-protection 
and will create a new patch review after an agreement with GCC will be made.

I added an error message.

I currently don't have more information. After seeing the issue I workaround 
it. I prefer not to open MSVC miscompilation issue in this code review and 
investigate it on a different thread.


Repository:
  rL LLVM

https://reviews.llvm.org/D40478



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

[PATCH] D41573: [x86][icelake][vpclmulqdq]

[coby via Phabricator via cfe-commits]

coby updated this revision to Diff 128139.
coby added a comment.

removing guards to allow better diags


Repository:
  rC Clang

https://reviews.llvm.org/D41573

Files:
  include/clang/Basic/BuiltinsX86.def
  include/clang/Driver/Options.td
  lib/Basic/Targets/X86.cpp
  lib/Basic/Targets/X86.h
  lib/Headers/CMakeLists.txt
  lib/Headers/immintrin.h
  lib/Headers/vpclmulqdqintrin.h
  test/CodeGen/attr-target-x86.c
  test/CodeGen/vpclmulqdq-builtins.c
  test/Driver/x86-target-features.c
  test/Preprocessor/predefined-arch-macros.c
  test/Preprocessor/x86_target_features.c

Index: lib/Headers/CMakeLists.txt
===
--- lib/Headers/CMakeLists.txt
+++ lib/Headers/CMakeLists.txt
@@ -84,6 +84,7 @@
   vadefs.h
   varargs.h
   vecintrin.h
+  vpclmulqdqintrin.h
   wmmintrin.h
   __wmmintrin_aes.h
   __wmmintrin_pclmul.h
Index: lib/Headers/immintrin.h
===
--- lib/Headers/immintrin.h
+++ lib/Headers/immintrin.h
@@ -118,6 +118,10 @@
 }
 #endif /* __AVX2__ */
 
+#if !defined(_MSC_VER) || __has_feature(modules) || defined(__VPCLMULQDQ__)
+#include 
+#endif
+
 #if !defined(_MSC_VER) || __has_feature(modules) || defined(__BMI__)
 #include 
 #endif
Index: lib/Headers/vpclmulqdqintrin.h
===
--- lib/Headers/vpclmulqdqintrin.h
+++ lib/Headers/vpclmulqdqintrin.h
@@ -0,0 +1,42 @@
+/*=== vpclmulqdqintrin.h - VPCLMULQDQ intrinsics ---===
+ *
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ *
+ *===---===
+ */
+#ifndef __IMMINTRIN_H
+#error "Never use  directly; include  instead."
+#endif
+
+#ifndef __VPCLMULQDQINTRIN_H
+#define __VPCLMULQDQINTRIN_H
+
+#define _mm256_clmulepi64_epi128(A, B, I) __extension__ ({\
+  (__m256i)__builtin_ia32_pclmulqdq256((__v4di)(__m256i)(A),  \
+   (__v4di)(__m256i)(B),  \
+   (char)(I)); })
+
+#define _mm512_clmulepi64_epi128(A, B, I) __extension__ ({\
+  (__m512i)__builtin_ia32_pclmulqdq512((__v8di)(__m512i)(A),  \
+   (__v8di)(__m512i)(B),  \
+   (char)(I)); })
+
+#endif // __VPCLMULQDQINTRIN_H
+
Index: lib/Basic/Targets/X86.cpp
===
--- lib/Basic/Targets/X86.cpp
+++ lib/Basic/Targets/X86.cpp
@@ -132,6 +132,7 @@
 break;
 
   case CK_Icelake:
+setFeatureEnabledImpl(Features, "vpclmulqdq", true);
 // TODO: Add icelake features here.
 LLVM_FALLTHROUGH;
   case CK_Cannonlake:
@@ -460,7 +461,7 @@
 LLVM_FALLTHROUGH;
   case AVX:
 Features["fma"] = Features["avx"] = Features["f16c"] = Features["xsave"] =
-Features["xsaveopt"] = false;
+Features["xsaveopt"] = Features["vpclmulqdq"] = false;
 setXOPLevel(Features, FMA4, false);
 LLVM_FALLTHROUGH;
   case AVX2:
@@ -575,6 +576,11 @@
   } else if (Name == "pclmul") {
 if (Enabled)
   setSSELevel(Features, SSE2, Enabled);
+  } else if (Name == "vpclmulqdq") {
+if (Enabled) {
+  setSSELevel(Features, AVX, Enabled);
+  Features["pclmul"] = true;
+}
   } else if (Name == "avx") {
 setSSELevel(Features, AVX, Enabled);
   } else if (Name == "avx2") {
@@ -638,6 +644,8 @@
   HasAES = true;
 } else if (Feature == "+pclmul") {
   HasPCLMUL = true;
+} else if (Feature == "+vpclmulqdq") {
+  HasVPCLMULQDQ = true;
 } else if (Feature == "+lzcnt") {
   HasLZCNT = true;
 } else if (Feature == "+rdrnd") {
@@ -937,6 +945,9 @@
   if (HasPCLMUL)
 Builder.defineMacro("__PCLMUL__");
 
+  if (HasVPCLMULQDQ)
+Builder.defineMacro("__VPCLMULQDQ__");
+
   if (HasLZCNT)
 Builder.defineMacro("__LZCNT__");
 
@@ -1185,6 +1196,7 @@
   .Case("sse4.2", true)
 

[libunwind] r321442 - [libunwind] fix a typo in r321441.

[whitequark via cfe-commits]

Author: whitequark
Date: Mon Dec 25 05:42:41 2017
New Revision: 321442

URL: http://llvm.org/viewvc/llvm-project?rev=321442=rev
Log:
[libunwind] fix a typo in r321441.

Modified:
libunwind/trunk/src/DwarfParser.hpp

Modified: libunwind/trunk/src/DwarfParser.hpp
URL: 
http://llvm.org/viewvc/llvm-project/libunwind/trunk/src/DwarfParser.hpp?rev=321442=321441=321442=diff
==
--- libunwind/trunk/src/DwarfParser.hpp (original)
+++ libunwind/trunk/src/DwarfParser.hpp Mon Dec 25 05:42:41 2017
@@ -549,7 +549,7 @@ bool CFI_Parser::parseInstructions(A
 case DW_CFA_expression:
   reg = addressSpace.getULEB128(p, instructionsEnd);
   if (reg > kMaxRegisterNumber) {
-_LIBUNWIND_LOG(
+_LIBUNWIND_LOG0(
 "malformed DW_CFA_expression DWARF unwind, reg too big");
 return false;
   }


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

[libunwind] r321441 - [libunwind] convert error logs to _LIBUNWIND_LOG/_LIBUNWIND_LOG0.

[whitequark via cfe-commits]

Author: whitequark
Date: Mon Dec 25 05:27:56 2017
New Revision: 321441

URL: http://llvm.org/viewvc/llvm-project?rev=321441=rev
Log:
[libunwind] convert error logs to _LIBUNWIND_LOG/_LIBUNWIND_LOG0.

Use the `_LIBUNWIND_LOG` and `_LIBUNWIND_LOG0` macros instead of
the explicit `fprintf` call.

This was previously done in r292721 as a cleanup and then reverted
in r293257 because the implementation in r292721 relied on a GNU
extension. This implementation avoids the use of an extension
by using a second macro instead, and allows to avoid the dependency
on fprintf if _LIBUNWIND_BARE_METAL is defined.

Modified:
libunwind/trunk/src/DwarfParser.hpp
libunwind/trunk/src/config.h

Modified: libunwind/trunk/src/DwarfParser.hpp
URL: 
http://llvm.org/viewvc/llvm-project/libunwind/trunk/src/DwarfParser.hpp?rev=321441=321440=321441=diff
==
--- libunwind/trunk/src/DwarfParser.hpp (original)
+++ libunwind/trunk/src/DwarfParser.hpp Mon Dec 25 05:27:56 2017
@@ -416,8 +416,8 @@ bool CFI_Parser::parseInstructions(A
   offset = (int64_t)addressSpace.getULEB128(p, instructionsEnd)
   * cieInfo.dataAlignFactor;
   if (reg > kMaxRegisterNumber) {
-fprintf(stderr,
-"malformed DW_CFA_offset_extended DWARF unwind, reg too 
big\n");
+_LIBUNWIND_LOG0(
+"malformed DW_CFA_offset_extended DWARF unwind, reg too big");
 return false;
   }
   results->savedRegisters[reg].location = kRegisterInCFA;
@@ -429,9 +429,8 @@ bool CFI_Parser::parseInstructions(A
 case DW_CFA_restore_extended:
   reg = addressSpace.getULEB128(p, instructionsEnd);
   if (reg > kMaxRegisterNumber) {
-fprintf(
-stderr,
-"malformed DW_CFA_restore_extended DWARF unwind, reg too big\n");
+_LIBUNWIND_LOG0(
+"malformed DW_CFA_restore_extended DWARF unwind, reg too big");
 return false;
   }
   results->savedRegisters[reg] = initialState.savedRegisters[reg];
@@ -440,8 +439,8 @@ bool CFI_Parser::parseInstructions(A
 case DW_CFA_undefined:
   reg = addressSpace.getULEB128(p, instructionsEnd);
   if (reg > kMaxRegisterNumber) {
-fprintf(stderr,
-"malformed DW_CFA_undefined DWARF unwind, reg too big\n");
+_LIBUNWIND_LOG0(
+"malformed DW_CFA_undefined DWARF unwind, reg too big");
 return false;
   }
   results->savedRegisters[reg].location = kRegisterUnused;
@@ -450,8 +449,8 @@ bool CFI_Parser::parseInstructions(A
 case DW_CFA_same_value:
   reg = addressSpace.getULEB128(p, instructionsEnd);
   if (reg > kMaxRegisterNumber) {
-fprintf(stderr,
-"malformed DW_CFA_same_value DWARF unwind, reg too big\n");
+_LIBUNWIND_LOG0(
+"malformed DW_CFA_same_value DWARF unwind, reg too big");
 return false;
   }
   //  DW_CFA_same_value unsupported
@@ -467,13 +466,13 @@ bool CFI_Parser::parseInstructions(A
   reg = addressSpace.getULEB128(p, instructionsEnd);
   reg2 = addressSpace.getULEB128(p, instructionsEnd);
   if (reg > kMaxRegisterNumber) {
-fprintf(stderr,
-"malformed DW_CFA_register DWARF unwind, reg too big\n");
+_LIBUNWIND_LOG0(
+"malformed DW_CFA_register DWARF unwind, reg too big");
 return false;
   }
   if (reg2 > kMaxRegisterNumber) {
-fprintf(stderr,
-"malformed DW_CFA_register DWARF unwind, reg2 too big\n");
+_LIBUNWIND_LOG0(
+"malformed DW_CFA_register DWARF unwind, reg2 too big");
 return false;
   }
   results->savedRegisters[reg].location = kRegisterInRegister;
@@ -512,7 +511,7 @@ bool CFI_Parser::parseInstructions(A
   reg = addressSpace.getULEB128(p, instructionsEnd);
   offset = (int64_t)addressSpace.getULEB128(p, instructionsEnd);
   if (reg > kMaxRegisterNumber) {
-fprintf(stderr, "malformed DW_CFA_def_cfa DWARF unwind, reg too 
big\n");
+_LIBUNWIND_LOG0("malformed DW_CFA_def_cfa DWARF unwind, reg too big");
 return false;
   }
   results->cfaRegister = (uint32_t)reg;
@@ -523,9 +522,8 @@ bool CFI_Parser::parseInstructions(A
 case DW_CFA_def_cfa_register:
   reg = addressSpace.getULEB128(p, instructionsEnd);
   if (reg > kMaxRegisterNumber) {
-fprintf(
-stderr,
-"malformed DW_CFA_def_cfa_register DWARF unwind, reg too big\n");
+_LIBUNWIND_LOG0(
+"malformed DW_CFA_def_cfa_register DWARF unwind, reg too big");
 return false;
   }
   results->cfaRegister = (uint32_t)reg;
@@ -551,8 +549,8 @@ bool CFI_Parser::parseInstructions(A
 case DW_CFA_expression:
   reg = addressSpace.getULEB128(p, instructionsEnd);
   if (reg > kMaxRegisterNumber) {
-fprintf(stderr,
- 

[PATCH] D41492: [Frontend] Correctly handle instantiating ctors with skipped bodies

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff accepted this revision.
sepavloff added a comment.
This revision is now accepted and ready to land.

LGTM.

Thanks!


Repository:
  rC Clang

https://reviews.llvm.org/D41492



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

[libunwind] r321440 - [libunwind] Avoid using C++ headers.

[whitequark via cfe-commits]

Author: whitequark
Date: Mon Dec 25 05:06:09 2017
New Revision: 321440

URL: http://llvm.org/viewvc/llvm-project?rev=321440=rev
Log:
[libunwind] Avoid using C++ headers.

This is useful for building libunwind on libcxx-free systems.

Modified:
libunwind/trunk/src/DwarfParser.hpp

Modified: libunwind/trunk/src/DwarfParser.hpp
URL: 
http://llvm.org/viewvc/llvm-project/libunwind/trunk/src/DwarfParser.hpp?rev=321440=321439=321440=diff
==
--- libunwind/trunk/src/DwarfParser.hpp (original)
+++ libunwind/trunk/src/DwarfParser.hpp Mon Dec 25 05:06:09 2017
@@ -17,7 +17,7 @@
 #include 
 #include 
 #include 
-#include 
+#include 
 
 #include "libunwind.h"
 #include "dwarf2.h"


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

[PATCH] D41573: [x86][icelake][vpclmulqdq]

[coby via Phabricator via cfe-commits]

coby created this revision.
coby added a reviewer: craig.topper.
Herald added a subscriber: mgorny.

added intrinsics support for vpclmulqdq instructions, matching a similar work 
on the backend (https://reviews.llvm.org/D40101)


Repository:
  rC Clang

https://reviews.llvm.org/D41573

Files:
  include/clang/Basic/BuiltinsX86.def
  include/clang/Driver/Options.td
  lib/Basic/Targets/X86.cpp
  lib/Basic/Targets/X86.h
  lib/Headers/CMakeLists.txt
  lib/Headers/immintrin.h
  lib/Headers/vpclmulqdqintrin.h
  test/CodeGen/attr-target-x86.c
  test/CodeGen/vpclmulqdq-builtins.c
  test/Driver/x86-target-features.c
  test/Preprocessor/predefined-arch-macros.c
  test/Preprocessor/x86_target_features.c

Index: lib/Headers/CMakeLists.txt
===
--- lib/Headers/CMakeLists.txt
+++ lib/Headers/CMakeLists.txt
@@ -84,6 +84,7 @@
   vadefs.h
   varargs.h
   vecintrin.h
+  vpclmulqdqintrin.h
   wmmintrin.h
   __wmmintrin_aes.h
   __wmmintrin_pclmul.h
Index: lib/Headers/immintrin.h
===
--- lib/Headers/immintrin.h
+++ lib/Headers/immintrin.h
@@ -118,6 +118,10 @@
 }
 #endif /* __AVX2__ */
 
+#if !defined(_MSC_VER) || __has_feature(modules) || defined(__VPCLMULQDQ__)
+#include 
+#endif
+
 #if !defined(_MSC_VER) || __has_feature(modules) || defined(__BMI__)
 #include 
 #endif
Index: lib/Headers/vpclmulqdqintrin.h
===
--- lib/Headers/vpclmulqdqintrin.h
+++ lib/Headers/vpclmulqdqintrin.h
@@ -0,0 +1,48 @@
+/*=== vpclmulqdqintrin.h - VPCLMULQDQ intrinsics ---===
+ *
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ *
+ *===---===
+ */
+#ifndef __IMMINTRIN_H
+#error "Never use  directly; include  instead."
+#endif
+
+#ifndef __VPCLMULQDQINTRIN_H
+#define __VPCLMULQDQINTRIN_H
+
+#if defined(__VPCLMULQDQ__)
+
+#define _mm256_clmulepi64_epi128(A, B, I) __extension__ ({\
+  (__m256i)__builtin_ia32_pclmulqdq256((__v4di)(__m256i)(A),  \
+   (__v4di)(__m256i)(B),  \
+   (char)(I)); })
+
+#if defined(__AVX512F__)
+
+#define _mm512_clmulepi64_epi128(A, B, I) __extension__ ({\
+  (__m512i)__builtin_ia32_pclmulqdq512((__v8di)(__m512i)(A),  \
+   (__v8di)(__m512i)(B),  \
+   (char)(I)); })
+
+#endif // defined(__AVX512__)
+#endif // defined(__VPCLMULQDQ__)
+#endif // __VPCLMULQDQINTRIN_H
+
Index: lib/Basic/Targets/X86.cpp
===
--- lib/Basic/Targets/X86.cpp
+++ lib/Basic/Targets/X86.cpp
@@ -132,6 +132,7 @@
 break;
 
   case CK_Icelake:
+setFeatureEnabledImpl(Features, "vpclmulqdq", true);
 // TODO: Add icelake features here.
 LLVM_FALLTHROUGH;
   case CK_Cannonlake:
@@ -460,7 +461,7 @@
 LLVM_FALLTHROUGH;
   case AVX:
 Features["fma"] = Features["avx"] = Features["f16c"] = Features["xsave"] =
-Features["xsaveopt"] = false;
+Features["xsaveopt"] = Features["vpclmulqdq"] = false;
 setXOPLevel(Features, FMA4, false);
 LLVM_FALLTHROUGH;
   case AVX2:
@@ -575,6 +576,11 @@
   } else if (Name == "pclmul") {
 if (Enabled)
   setSSELevel(Features, SSE2, Enabled);
+  } else if (Name == "vpclmulqdq") {
+if (Enabled) {
+  setSSELevel(Features, AVX, Enabled);
+  Features["pclmul"] = true;
+}
   } else if (Name == "avx") {
 setSSELevel(Features, AVX, Enabled);
   } else if (Name == "avx2") {
@@ -638,6 +644,8 @@
   HasAES = true;
 } else if (Feature == "+pclmul") {
   HasPCLMUL = true;
+} else if (Feature == "+vpclmulqdq") {
+  HasVPCLMULQDQ = true;
 } else if (Feature == "+lzcnt") {
   HasLZCNT = true;
 } else if (Feature == "+rdrnd") {
@@ 

[PATCH] D41512: [Sema] -Wtautological-constant-compare is too good. Cripple it.

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri updated this revision to Diff 128128.
lebedev.ri added a comment.

Move `TautologicalUnsignedZeroCompare` and 
`TautologicalUnsignedEnumZeroCompare` into `TautologicalRangeCompare` too, and 
enable them only in `-Wextra`.
Please advise re flag name for `warn_tautological_constant_compare`.


Repository:
  rC Clang

https://reviews.llvm.org/D41512

Files:
  include/clang/Basic/DiagnosticGroups.td
  include/clang/Basic/DiagnosticSemaKinds.td
  test/Sema/compare.c
  test/Sema/tautological-constant-compare.c
  test/Sema/tautological-constant-enum-compare.c
  test/Sema/tautological-unsigned-enum-zero-compare.c
  test/Sema/tautological-unsigned-enum-zero-compare.cpp
  test/Sema/tautological-unsigned-zero-compare.c
  test/SemaCXX/compare.cpp

Index: test/SemaCXX/compare.cpp
===
--- test/SemaCXX/compare.cpp
+++ test/SemaCXX/compare.cpp
@@ -1,7 +1,7 @@
 // Force x86-64 because some of our heuristics are actually based
 // on integer sizes.
 
-// RUN: %clang_cc1 -triple x86_64-apple-darwin -fsyntax-only -pedantic -verify -Wsign-compare -std=c++11 %s
+// RUN: %clang_cc1 -triple x86_64-apple-darwin -fsyntax-only -pedantic -verify -Wsign-compare -Wtautological-constant-range-compare -std=c++11 %s
 
 int test0(long a, unsigned long b) {
   enum EnumA {A};
Index: test/Sema/tautological-unsigned-zero-compare.c
===
--- test/Sema/tautological-unsigned-zero-compare.c
+++ test/Sema/tautological-unsigned-zero-compare.c
@@ -1,7 +1,13 @@
-// RUN: %clang_cc1 -fsyntax-only -verify %s
-// RUN: %clang_cc1 -fsyntax-only -Wno-tautological-unsigned-zero-compare -verify=silence %s
-// RUN: %clang_cc1 -fsyntax-only -verify -x c++ %s
-// RUN: %clang_cc1 -fsyntax-only -Wno-tautological-unsigned-zero-compare -verify=silence -x c++ %s
+// RUN: %clang_cc1 -fsyntax-only \
+// RUN:-Wtautological-unsigned-zero-compare \
+// RUN:-verify %s
+// RUN: %clang_cc1 -fsyntax-only \
+// RUN:-verify=silence %s
+// RUN: %clang_cc1 -fsyntax-only \
+// RUN:-Wtautological-unsigned-zero-compare \
+// RUN:-verify -x c++ %s
+// RUN: %clang_cc1 -fsyntax-only \
+// RUN:-verify=silence -x c++ %s
 
 unsigned uvalue(void);
 signed int svalue(void);
Index: test/Sema/tautological-unsigned-enum-zero-compare.cpp
===
--- test/Sema/tautological-unsigned-enum-zero-compare.cpp
+++ test/Sema/tautological-unsigned-enum-zero-compare.cpp
@@ -1,9 +1,10 @@
 // RUN: %clang_cc1 -std=c++11 -triple=x86_64-pc-linux-gnu -fsyntax-only \
+// RUN:-Wtautological-unsigned-enum-zero-compare \
 // RUN:-verify=unsigned,unsigned-signed %s
 // RUN: %clang_cc1 -std=c++11 -triple=x86_64-pc-win32 -fsyntax-only \
+// RUN:-Wtautological-unsigned-enum-zero-compare \
 // RUN:-verify=unsigned-signed %s
 // RUN: %clang_cc1 -std=c++11 -triple=x86_64-pc-win32 -fsyntax-only \
-// RUN:-Wno-tautological-unsigned-enum-zero-compare \
 // RUN:-verify=silence %s
 
 // silence-no-diagnostics
Index: test/Sema/tautological-unsigned-enum-zero-compare.c
===
--- test/Sema/tautological-unsigned-enum-zero-compare.c
+++ test/Sema/tautological-unsigned-enum-zero-compare.c
@@ -1,9 +1,10 @@
 // RUN: %clang_cc1 -triple=x86_64-pc-linux-gnu -fsyntax-only \
+// RUN:-Wtautological-unsigned-enum-zero-compare \
 // RUN:-verify=unsigned,unsigned-signed %s
 // RUN: %clang_cc1 -triple=x86_64-pc-win32 -fsyntax-only \
+// RUN:-Wtautological-unsigned-enum-zero-compare \
 // RUN:-verify=unsigned-signed %s
 // RUN: %clang_cc1 -triple=x86_64-pc-win32 -fsyntax-only \
-// RUN:-Wno-tautological-unsigned-enum-zero-compare \
 // RUN:-verify=silence %s
 
 // Okay, this is where it gets complicated.
Index: test/Sema/tautological-constant-enum-compare.c
===
--- test/Sema/tautological-constant-enum-compare.c
+++ test/Sema/tautological-constant-enum-compare.c
@@ -1,5 +1,5 @@
-// RUN: %clang_cc1 -triple=x86_64-pc-linux-gnu -fsyntax-only -DUNSIGNED -verify %s
-// RUN: %clang_cc1 -triple=x86_64-pc-win32 -fsyntax-only -DSIGNED -verify %s
+// RUN: %clang_cc1 -triple=x86_64-pc-linux-gnu -fsyntax-only -DUNSIGNED -Wtautological-constant-range-compare -verify %s
+// RUN: %clang_cc1 -triple=x86_64-pc-win32 -fsyntax-only -DSIGNED -Wtautological-constant-range-compare -verify %s
 // RUN: %clang_cc1 -triple=x86_64-pc-linux-gnu -fsyntax-only -DUNSIGNED -DSILENCE -Wno-tautological-constant-compare -verify %s
 // RUN: %clang_cc1 -triple=x86_64-pc-win32 -fsyntax-only -DSIGNED -DSILENCE -Wno-tautological-constant-compare -verify %s
 
Index: test/Sema/tautological-constant-compare.c

[PATCH] D41512: [Sema] -Wtautological-constant-compare is too good. Cripple it.

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added inline comments.



Comment at: include/clang/Basic/DiagnosticGroups.td:439
 def TautologicalUnsignedZeroCompare : 
DiagGroup<"tautological-unsigned-zero-compare">;
 def TautologicalUnsignedEnumZeroCompare : 
DiagGroup<"tautological-unsigned-enum-zero-compare">;
+def TautologicalRangeCompare : 
DiagGroup<"tautological-constant-range-compare">;

rsmith wrote:
> I forget, are these in-range-compare warnings new too, or just the 
> non-unsigned-enum form? It would make sense to me for these two to be 
> subgroups of `TautologicalRangeCompare`.
I do believe all three are new: `warn_unsigned_enum_always_true_comparison`, 
`warn_unsigned_always_true_comparison`, `warn_tautological_constant_compare`.
I'll move them into `TautologicalRangeCompare`, and disable them too...

But now we are almost back to square one.
Disabling `TautologicalRangeCompare` will disable all three type-limit-checking 
diagnostic, and only the inner two can be enabled separately.
Is that really the intended result? I think 
`warn_tautological_constant_compare` should have it's own flag.
But i can not come up with a name for it. Please advise.



Comment at: include/clang/Basic/DiagnosticGroups.td:440
 def TautologicalUnsignedEnumZeroCompare : 
DiagGroup<"tautological-unsigned-enum-zero-compare">;
+def TautologicalRangeCompare : 
DiagGroup<"tautological-constant-range-compare">;
 def TautologicalOutOfRangeCompare : 
DiagGroup<"tautological-constant-out-of-range-compare">;

rsmith wrote:
> "tautological-constant-in-range-compare" would make more sense to me, as the 
> complement of "tautological-constant-out-of-range-compare".
I did think about it.
To me "tautological-constant-in-range-compare" may sound as if the constant is 
//somewhere// in the range,
while i'd say "tautological-constant-range-compare" better highlights the fact 
that the constant *is* the range limit.
No?


Repository:
  rC Clang

https://reviews.llvm.org/D41512



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