[PATCH] D28620: Guard __gnuc_va_list typedef
yaron.keren accepted this revision. yaron.keren added a comment. This revision is now accepted and ready to land. LGTM, matches the code in libstdc++ stdarg,h. You can remove the 'hack' comment in line 46, __GNUC_VA_LIST is just a standard include guard for the typedef. https://reviews.llvm.org/D28620 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28952: [analyzer] Add new Z3 constraint manager backend
zaks.anna added a comment. Thanks for working on this Dominic!!! Can you talk a bit about your motivation for working on this project and what your goals are? Have you compared the performance when using Z3 vs the current builtin solver? I saw that you mention performance issues on large codebases, but could you give some specific numbers of the tradeoffs between performance, code coverage, and the quality of reported bugs. (One very rough idea to use a stronger solver while still keeping the analyzer performant would be to only use it for false positive refutation.) > I looked around for a generic smt-lib interface earlier, but couldn't find > much available, and since I wanted floating-point arithmetic support, I ended > up just directly using the Z3 C interface (the C++ interface uses exceptions, > which causes problems). As far as I know, CVC4 doesn't have built-in > floating-point support. But longer term, I'd agree that this backend should > be made more generic. Ok. Though I'd love to see an interface that supports CVC4! > Another issue is that some testcases will have different results depending on > the constraint manager in use, but I don't believe it's possible to change > the expected testcase output based on the presence of a feature flag. Unless > this changes, there'll need to be (mostly) duplicate testcases for each > constraint manager. How different the results are? Is it the case that you get more warnings with Z3 in most cases? Would it be possible to divide the tests into the ones that work similarly with both solvers and the ones that are only expected to report warnings with Z3? I know this won't work in general, but might be much cleaner than polluting every test with a ton of #ifdefs. > Furthermore, this and the child patches will cause the static analyzer to > generate more complex constraints at runtime. But, I'm not sure if just > having RangeConstraintManager ignore unsupported constraints will be > sufficient, from a performance and correctness perspective. This is probably the most important question to answer. Maintaining the performance and correctness of the analyzer mode that is using RangeConstraintManager is very important as this is the mode most users will use at least for a while. > My personal preference would be to directly merge in this constraint manager > without using the plugin approach, because it would simplify some of the > testing and changes to the build system (e.g. the APFloat linking issue). But > I'm not sure if this would be ok? Most likely that would be possible. I'd also like to second Ryan's suggestion to look at his patch to add support for STP. It was very close to landing other than some build system integration issues. https://reviews.llvm.org/D28952 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: r292562 - clang-format: fix fallback style set to "none" not always formatting
Hello Antonio and Mikael. I've tried reproducing this also after removal of the redundant test (D28943), and I still see the fail for CHECK10. The problem is that we (both me an Mikael) are running this test from inside our llvm repo (with build artifacts ending up in a "build-all" directory inside the llvm clone). So if I run the test with -debug I can see that it is searching for the format file like this: Trying /repo/llvm/build-all/tools/clang/test/Format/Output/.clang-format... Trying /repo/llvm/build-all/tools/clang/test/Format/Output/_clang-format... Trying /repo/llvm/build-all/tools/clang/test/Format/.clang-format... Trying /repo/llvm/build-all/tools/clang/test/Format/_clang-format... Trying /repo/llvm/build-all/tools/clang/test/.clang-format... Trying /repo/llvm/build-all/tools/clang/test/_clang-format... Trying /repo/llvm/build-all/tools/clang/.clang-format... Trying /repo/llvm/build-all/tools/clang/_clang-format... Trying /repo/llvm/build-all/tools/.clang-format... Trying /repo/llvm/build-all/tools/_clang-format... Trying /repo/llvm/build-all/.clang-format... Trying /repo/llvm/build-all/_clang-format... Trying /repo/llvm/.clang-format... Using configuration file /repo/llvm/.clang-format So the test case will find the format file that exists in the llvm repository and it uses that one (and not the one defined as a fallback). I'm not sure that there is a rule against having the build artifacts inside the repo. I think there are both pros and cons to have such a setup. And it is something that has been working well for us so far. Regards /Björn ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28849: [compiler-rt] [test] Fix page address logic in clear_cache_test
This revision was automatically updated to reflect the committed changes.
Closed by commit rL292729: [test] Fix page address logic in clear_cache_test
(authored by mgorny).
Changed prior to commit:
https://reviews.llvm.org/D28849?vs=85208=85248#toc
Repository:
rL LLVM
https://reviews.llvm.org/D28849
Files:
compiler-rt/trunk/test/builtins/Unit/clear_cache_test.c
Index: compiler-rt/trunk/test/builtins/Unit/clear_cache_test.c
===
--- compiler-rt/trunk/test/builtins/Unit/clear_cache_test.c
+++ compiler-rt/trunk/test/builtins/Unit/clear_cache_test.c
@@ -18,9 +18,20 @@
if (!FlushInstructionCache(GetCurrentProcess(), start, end-start))
exit(1);
}
+
+static uintptr_t get_page_size() {
+SYSTEM_INFO si;
+GetSystemInfo();
+return si.dwPageSize;
+}
#else
+#include
#include
extern void __clear_cache(void* start, void* end);
+
+static uintptr_t get_page_size() {
+return sysconf(_SC_PAGE_SIZE);
+}
#endif
@@ -56,8 +67,9 @@
int main()
{
// make executable the page containing execution_buffer
-char* start = (char*)((uintptr_t)execution_buffer & (-4095));
-char* end = (char*)((uintptr_t)(_buffer[128+4096]) & (-4095));
+uintptr_t page_size = get_page_size();
+char* start = (char*)((uintptr_t)execution_buffer & (-page_size));
+char* end = (char*)((uintptr_t)(_buffer[128+page_size]) &
(-page_size));
#if defined(_WIN32)
DWORD dummy_oldProt;
MEMORY_BASIC_INFORMATION b;
Index: compiler-rt/trunk/test/builtins/Unit/clear_cache_test.c
===
--- compiler-rt/trunk/test/builtins/Unit/clear_cache_test.c
+++ compiler-rt/trunk/test/builtins/Unit/clear_cache_test.c
@@ -18,9 +18,20 @@
if (!FlushInstructionCache(GetCurrentProcess(), start, end-start))
exit(1);
}
+
+static uintptr_t get_page_size() {
+SYSTEM_INFO si;
+GetSystemInfo();
+return si.dwPageSize;
+}
#else
+#include
#include
extern void __clear_cache(void* start, void* end);
+
+static uintptr_t get_page_size() {
+return sysconf(_SC_PAGE_SIZE);
+}
#endif
@@ -56,8 +67,9 @@
int main()
{
// make executable the page containing execution_buffer
-char* start = (char*)((uintptr_t)execution_buffer & (-4095));
-char* end = (char*)((uintptr_t)(_buffer[128+4096]) & (-4095));
+uintptr_t page_size = get_page_size();
+char* start = (char*)((uintptr_t)execution_buffer & (-page_size));
+char* end = (char*)((uintptr_t)(_buffer[128+page_size]) & (-page_size));
#if defined(_WIN32)
DWORD dummy_oldProt;
MEMORY_BASIC_INFORMATION b;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28955: [analyzer] Enable support for symbolic extension/truncation
ddcc added a comment.
> We should have expected-warning on 64-bit targets (where `size_t` easily
> overflows `int`) and no-warning on 32-bit targets (where they are of the same
> size and the fix for the original issue
> https://llvm.org/bugs/show_bug.cgi?id=16558 applies). I think we should have
> two run-lines for this test, with two concrete targets specified; it'd be
> great to actually have other tests in this file undergo such trial.
To clarify, you're asking for something like the following, instead of changing
from `int` to `size_t`?
diff --git a/test/Analysis/malloc.c b/test/Analysis/malloc.c
index 42deb9f..80e4184 100644
--- a/test/Analysis/malloc.c
+++ b/test/Analysis/malloc.c
@@ -1,4 +1,5 @@
-// RUN: %clang_cc1 %z3_cc1 -analyze
-analyzer-checker=core,alpha.deadcode.UnreachableCode,alpha.core.CastSize,unix.Malloc,debug.ExprInspection
-analyzer-store=region -verify %s
+// RUN: %clang_cc1 %z3_cc1 -triple i386-unknown-linux -analyze
-analyzer-checker=core,alpha.deadcode.UnreachableCode,alpha.core.CastSize,unix.Malloc,debug.ExprInspection
-analyzer-store=region -verify %s
+// RUN: %clang_cc1 %z3_cc1 -triple x86_64-unknown-linux -Dx86_64 -analyze
-analyzer-checker=core,alpha.deadcode.UnreachableCode,alpha.core.CastSize,unix.Malloc,debug.ExprInspection
-analyzer-store=region -verify %s
#include "Inputs/system-header-simulator.h"
@@ -1705,9 +1706,13 @@ void *smallocNoWarn(size_t size) {
}
char *dupstrNoWarn(const char *s) {
- const size_t len = strlen(s);
+ const int len = strlen(s);
char *p = (char*) smallocNoWarn(len + 1);
- strcpy(p, s); // no-warning
+#ifdef x86_64
+ strcpy(p, s); // expected-warning{{String copy function overflows
destination buffer}}
+#else
+ strcpy(p, s); // no warning
+#endif
return p;
}
https://reviews.llvm.org/D28955
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[libunwind] r292728 - DWARF: correct cast (NFC)
Author: compnerd
Date: Sat Jan 21 15:27:29 2017
New Revision: 292728
URL: http://llvm.org/viewvc/llvm-project?rev=292728=rev
Log:
DWARF: correct cast (NFC)
Change the case of a PRIu64 value from `long` to `uint64_t`. NFC.
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=292728=292727=292728=diff
==
--- libunwind/trunk/src/DwarfParser.hpp (original)
+++ libunwind/trunk/src/DwarfParser.hpp Sat Jan 21 15:27:29 2017
@@ -674,7 +674,7 @@ bool CFI_Parser::parseInstructions(A
reg = operand;
results->savedRegisters[reg] = initialState.savedRegisters[reg];
_LIBUNWIND_TRACE_DWARF("DW_CFA_restore(reg=%" PRIu64 ")\n",
- static_cast(operand));
+ static_cast(operand));
break;
default:
_LIBUNWIND_TRACE_DWARF("unknown CFA opcode 0x%02X\n", opcode);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28952: [analyzer] Add new Z3 constraint manager backend
ddcc added a comment.
> For such test cases i'd consider something like this:
>
> // RUN: ... -analyzer-constraints=range -DRANGE ...
> // RUN: ... -analyzer-constraints=z3 ...
>
> #ifdef RANGE
> foo(); // expected-warning{{}}
> #else
> foo(); // no-warning
> #endif
Would this be assuming that the test setup has Z3 installed and both constraint
managers available? I see that `llvm-lit` supports a `REQUIRES` directive, but
I don't know if that can be scoped to just one `RUN`.
> Yep, the analyzer core has a lot of code that says "we denote this value with
> an incorrect symbol here, because even if we provided the correct symbol it
> would be too hard for the RangeConstraintManager to understand". The most
> obvious place is the integral cast code, which leaves the symbol intact
> *most* of the time (all the time until recently).
>
> I think the right way to address these issues is introduce an API in the base
> ConstraintManager interface to ask the solver if he implements a certain
> feature or is capable of dealing with a particular kind of symbol, and change
> our behavior accordingly. We already have one function that was intended to
> do something like this - `ConstraintManager::canReasonAbout()`; not sure if
> it's working or broken.
`canReasonAbout()` works fine, but in its current form, requires parsing each
constraint to determine whether its supported or not. In
https://reviews.llvm.org/D26061, I've pushed the implementation from
SimpleConstraintManager into RangeConstraintManager itself, since
Z3ConstraintManager will also inherit from SimpleConstraintManager but with a
different feature set. However, even with these changes, `canReasonAbout()` is
only called from within SimpleConstraintManager; the feedback doesn't go all
the way back to e.g. SimpleSValBuilder to dynamically change the constraint
complexity at runtime.
https://reviews.llvm.org/D28952
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D27440: clang-format-vsix: fail when clang-format outputs to stderr
amaiorano abandoned this revision. amaiorano added a comment. This change is no longer needed since clang-format now returns failure status (non-zero) whenever it writes to stderr (e.g. on parse error) since https://llvm.org/svn/llvm-project/cfe/trunk@292174 (https://reviews.llvm.org/D28081) https://reviews.llvm.org/D27440 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28983: clang-format: remove tests that assume no config file will be found as this is not always the case
amaiorano created this revision.
Herald added a subscriber: klimek.
These tests fail for developers who place their build directories under the
llvm root directory because llvm's own .clang-format file will be found.
Anyway these cases are covered by FormatStyle.GetStyleOfFile tests
(FormatTest.cpp).
https://reviews.llvm.org/D28983
Files:
test/Format/style-on-command-line.cpp
Index: test/Format/style-on-command-line.cpp
===
--- test/Format/style-on-command-line.cpp
+++ test/Format/style-on-command-line.cpp
@@ -13,16 +13,11 @@
// RUN: clang-format -style="{BasedOnStyle: WebKit, PointerBindsToType:
false}" %s | FileCheck -strict-whitespace -check-prefix=CHECK9 %s
// Fallback style tests
-// RUN: rm %T/_clang-format
-// Test no config file found, WebKit fallback style is applied
-// RUN: clang-format -style=file -fallback-style=WebKit
-assume-filename=%T/foo.cpp < %s 2>&1 | FileCheck -strict-whitespace
-check-prefix=CHECK10 %s
-// Test no config file and fallback style "none", no formatting is applied
-// RUN: clang-format -style=file -fallback-style=none
-assume-filename=%T/foo.cpp < %s 2>&1 | FileCheck -strict-whitespace
-check-prefix=CHECK11 %s
// Test config file with no based style, and fallback style "none", formatting
is applied
// RUN: printf "IndentWidth: 6\n" > %T/_clang-format
-// RUN: clang-format -style=file -fallback-style=none
-assume-filename=%T/foo.cpp < %s 2>&1 | FileCheck -strict-whitespace
-check-prefix=CHECK12 %s
+// RUN: clang-format -style=file -fallback-style=none
-assume-filename=%T/foo.cpp < %s 2>&1 | FileCheck -strict-whitespace
-check-prefix=CHECK10 %s
// Test yaml with no based style, and fallback style "none", LLVM formatting
applied
-// RUN: clang-format -style="{IndentWidth: 7}" -fallback-style=none %s |
FileCheck -strict-whitespace -check-prefix=CHECK13 %s
+// RUN: clang-format -style="{IndentWidth: 7}" -fallback-style=none %s |
FileCheck -strict-whitespace -check-prefix=CHECK11 %s
void f() {
// CHECK1: {{^int\* i;$}}
@@ -35,10 +30,8 @@
// CHECK7: {{^ int\* i;$}}
// CHECK8: {{^ int\* i;$}}
// CHECK9: {{^int \*i;$}}
-// CHECK10: {{^int\* i;$}}
-// CHECK11: {{^int\*i;$}}
-// CHECK12: {{^ int \*i;$}}
-// CHECK13: {{^ int \*i;$}}
+// CHECK10: {{^ int \*i;$}}
+// CHECK11: {{^ int \*i;$}}
int*i;
int j;
}
Index: test/Format/style-on-command-line.cpp
===
--- test/Format/style-on-command-line.cpp
+++ test/Format/style-on-command-line.cpp
@@ -13,16 +13,11 @@
// RUN: clang-format -style="{BasedOnStyle: WebKit, PointerBindsToType: false}" %s | FileCheck -strict-whitespace -check-prefix=CHECK9 %s
// Fallback style tests
-// RUN: rm %T/_clang-format
-// Test no config file found, WebKit fallback style is applied
-// RUN: clang-format -style=file -fallback-style=WebKit -assume-filename=%T/foo.cpp < %s 2>&1 | FileCheck -strict-whitespace -check-prefix=CHECK10 %s
-// Test no config file and fallback style "none", no formatting is applied
-// RUN: clang-format -style=file -fallback-style=none -assume-filename=%T/foo.cpp < %s 2>&1 | FileCheck -strict-whitespace -check-prefix=CHECK11 %s
// Test config file with no based style, and fallback style "none", formatting is applied
// RUN: printf "IndentWidth: 6\n" > %T/_clang-format
-// RUN: clang-format -style=file -fallback-style=none -assume-filename=%T/foo.cpp < %s 2>&1 | FileCheck -strict-whitespace -check-prefix=CHECK12 %s
+// RUN: clang-format -style=file -fallback-style=none -assume-filename=%T/foo.cpp < %s 2>&1 | FileCheck -strict-whitespace -check-prefix=CHECK10 %s
// Test yaml with no based style, and fallback style "none", LLVM formatting applied
-// RUN: clang-format -style="{IndentWidth: 7}" -fallback-style=none %s | FileCheck -strict-whitespace -check-prefix=CHECK13 %s
+// RUN: clang-format -style="{IndentWidth: 7}" -fallback-style=none %s | FileCheck -strict-whitespace -check-prefix=CHECK11 %s
void f() {
// CHECK1: {{^int\* i;$}}
@@ -35,10 +30,8 @@
// CHECK7: {{^ int\* i;$}}
// CHECK8: {{^ int\* i;$}}
// CHECK9: {{^int \*i;$}}
-// CHECK10: {{^int\* i;$}}
-// CHECK11: {{^int\*i;$}}
-// CHECK12: {{^ int \*i;$}}
-// CHECK13: {{^ int \*i;$}}
+// CHECK10: {{^ int \*i;$}}
+// CHECK11: {{^ int \*i;$}}
int*i;
int j;
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28955: [analyzer] Enable support for symbolic extension/truncation
NoQ added a comment. In https://reviews.llvm.org/D28955#652443, @ddcc wrote: > When I was testing this patch, it was on top of both > https://reviews.llvm.org/D28952 and https://reviews.llvm.org/D28953. For > `malloc.c`, the change on line 1708 from `int` to `size_t` is necessary to > prevent a false positive warning at line 1710. We should have expected-warning on 64-bit targets (where `size_t` easily overflows `int`) and no-warning on 32-bit targets (where they are of the same size and the fix for the original issue https://llvm.org/bugs/show_bug.cgi?id=16558 applies). I think we should have two run-lines for this test, with two concrete targets specified; it'd be great to actually have other tests in this file undergo such trial. https://reviews.llvm.org/D28955 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28955: [analyzer] Enable support for symbolic extension/truncation
ddcc added a comment. When I was testing this patch, it was on top of both https://reviews.llvm.org/D28952 and https://reviews.llvm.org/D28953. For `malloc.c`, the change on line 1708 from `int` to `size_t` is necessary to prevent a false positive warning at line 1710. The other three changes don't affect the testcase output, but I probably made the changes at the same time. Do you want those other three pulled out separately? https://reviews.llvm.org/D28955 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28952: [analyzer] Add new Z3 constraint manager backend
NoQ added a comment.
Late-joining the round of applause for the awesome progression of this project!
Not sure how to deal with the massive test run-line changes at the moment, will
take some extra time to think.
In https://reviews.llvm.org/D28952#652436, @ddcc wrote:
> Another issue is that some testcases will have different results depending on
> the constraint manager in use, but I don't believe it's possible to change
> the expected testcase output based on the presence of a feature flag. Unless
> this changes, there'll need to be (mostly) duplicate testcases for each
> constraint manager.
For such test cases i'd consider something like this:
// RUN: ... -analyzer-constraints=range -DRANGE ...
// RUN: ... -analyzer-constraints=z3 ...
#ifdef RANGE
foo(); // expected-warning{{}}
#else
foo(); // no-warning
#endif
.
In https://reviews.llvm.org/D28952#652436, @ddcc wrote:
> Furthermore, this and the child patches will cause the static analyzer to
> generate more complex constraints at runtime. But, I'm not sure if just
> having RangeConstraintManager ignore unsupported constraints will be
> sufficient, from a performance and correctness perspective.
Yep, the analyzer core has a lot of code that says "we denote this value with
an incorrect symbol here, because even if we provided the correct symbol it
would be too hard for the RangeConstraintManager to understand". The most
obvious place is the integral cast code, which leaves the symbol intact *most*
of the time (all the time until recently).
I think the right way to address these issues is introduce an API in the base
ConstraintManager interface to ask the solver if he implements a certain
feature or is capable of dealing with a particular kind of symbol, and change
our behavior accordingly. We already have one function that was intended to do
something like this - `ConstraintManager::canReasonAbout()`; not sure if it's
working or broken.
https://reviews.llvm.org/D28952
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28952: [analyzer] Add new Z3 constraint manager backend
ddcc added a comment. Do you want me to replace this version of the patch with one that omits the test case changes? The underlying git commit for just the Z3 constraint manager implementation is https://github.com/ddcc/clang/commit/e1414d300882c1459f461424d3e89d1613ecf03c , and https://github.com/ddcc/clang/commit/fb7d558be6492f11a3793f011f364098ddcc9711 is the commit that converts it to a plugin and modifies all the testcases. I looked around for a generic smt-lib interface earlier, but couldn't find much available, and since I wanted floating-point arithmetic support, I ended up just directly using the Z3 C interface (the C++ interface uses exceptions, which causes problems). As far as I know, CVC4 doesn't have built-in floating-point support. But longer term, I'd agree that this backend should be made more generic. I'm not sure what's the easiest way to maintain backwards support with RangeConstraintManager. Right now, I've modified `lit.cfg` to substitute `%z3` and `%z3_cc1` (for `clang` or `clang -cc1`) with the appropriate argument to load the plugin, if the `z3` feature is available. Otherwise, they are substituted with just the empty string. Another approach would be to do this all in `llvm-lit` without modifying the testcases, but then there'd need to be some sort of path-based matching to determine when the argument to load the plugin should be injected (e.g. only for `test/Analysis`). However, if this constraint manager were built into clang instead of as a plugin, then this issue is moot. Another issue is that some testcases will have different results depending on the constraint manager in use, but I don't believe it's possible to change the expected testcase output based on the presence of a feature flag. Unless this changes, there'll need to be (mostly) duplicate testcases for each constraint manager. Furthermore, this and the child patches will cause the static analyzer to generate more complex constraints at runtime. But, I'm not sure if just having RangeConstraintManager ignore unsupported constraints will be sufficient, from a performance and correctness perspective. https://reviews.llvm.org/D28952 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28849: [compiler-rt] [test] Fix page address logic in clear_cache_test
compnerd accepted this revision. compnerd added a comment. This revision is now accepted and ready to land. Would be nice to clang-format those lines, but its no worse than before. https://reviews.llvm.org/D28849 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[libunwind] r292722 - DWARF: allow enabling tracing at runtime
Author: compnerd
Date: Sat Jan 21 10:22:57 2017
New Revision: 292722
URL: http://llvm.org/viewvc/llvm-project?rev=292722=rev
Log:
DWARF: allow enabling tracing at runtime
Introduce `logDWARF` and the associated environment variable
`LIBUNWIND_PRINT_DWARF` to trace the CFI instructions.
Modified:
libunwind/trunk/src/DwarfParser.hpp
libunwind/trunk/src/libunwind.cpp
Modified: libunwind/trunk/src/DwarfParser.hpp
URL:
http://llvm.org/viewvc/llvm-project/libunwind/trunk/src/DwarfParser.hpp?rev=292722=292721=292722=diff
==
--- libunwind/trunk/src/DwarfParser.hpp (original)
+++ libunwind/trunk/src/DwarfParser.hpp Sat Jan 21 10:22:57 2017
@@ -23,6 +23,14 @@
#include "AddressSpace.hpp"
+extern "C" bool logDWARF();
+
+#define _LIBUNWIND_TRACE_DWARF(...)
\
+ do {
\
+if (logDWARF())
\
+ fprintf(stderr, __VA_ARGS__);
\
+ } while (0)
+
namespace libunwind {
/// CFI_Parser does basic parsing of a CFI (Call Frame Information) records.
@@ -364,13 +372,12 @@ bool CFI_Parser::parseInstructions(A
const CIE_Info , pint_t pcoffset,
PrologInfoStackEntry *,
PrologInfo *results) {
- const bool logDwarf = false;
pint_t p = instructions;
pint_t codeOffset = 0;
PrologInfo initialState = *results;
- if (logDwarf)
-fprintf(stderr, "parseInstructions(instructions=0x%0" PRIx64 ")\n",
-(uint64_t)instructionsEnd);
+
+ _LIBUNWIND_TRACE_DWARF("parseInstructions(instructions=0x%0" PRIx64 ")\n",
+ static_cast(instructionsEnd));
// see DWARF Spec, section 6.4.2 for details on unwind opcodes
while ((p < instructionsEnd) && (codeOffset < pcoffset)) {
@@ -386,35 +393,30 @@ bool CFI_Parser::parseInstructions(A
++p;
switch (opcode) {
case DW_CFA_nop:
- if (logDwarf)
-fprintf(stderr, "DW_CFA_nop\n");
+ _LIBUNWIND_TRACE_DWARF("DW_CFA_nop\n");
break;
case DW_CFA_set_loc:
codeOffset =
addressSpace.getEncodedP(p, instructionsEnd,
cieInfo.pointerEncoding);
- if (logDwarf)
-fprintf(stderr, "DW_CFA_set_loc\n");
+ _LIBUNWIND_TRACE_DWARF("DW_CFA_set_loc\n");
break;
case DW_CFA_advance_loc1:
codeOffset += (addressSpace.get8(p) * cieInfo.codeAlignFactor);
p += 1;
- if (logDwarf)
-fprintf(stderr, "DW_CFA_advance_loc1: new offset=%" PRIu64 "\n",
-(uint64_t)codeOffset);
+ _LIBUNWIND_TRACE_DWARF("DW_CFA_advance_loc1: new offset=%" PRIu64 "\n",
+ static_cast(codeOffset));
break;
case DW_CFA_advance_loc2:
codeOffset += (addressSpace.get16(p) * cieInfo.codeAlignFactor);
p += 2;
- if (logDwarf)
-fprintf(stderr, "DW_CFA_advance_loc2: new offset=%" PRIu64 "\n",
-(uint64_t)codeOffset);
+ _LIBUNWIND_TRACE_DWARF("DW_CFA_advance_loc2: new offset=%" PRIu64 "\n",
+ static_cast(codeOffset));
break;
case DW_CFA_advance_loc4:
codeOffset += (addressSpace.get32(p) * cieInfo.codeAlignFactor);
p += 4;
- if (logDwarf)
-fprintf(stderr, "DW_CFA_advance_loc4: new offset=%" PRIu64 "\n",
-(uint64_t)codeOffset);
+ _LIBUNWIND_TRACE_DWARF("DW_CFA_advance_loc4: new offset=%" PRIu64 "\n",
+ static_cast(codeOffset));
break;
case DW_CFA_offset_extended:
reg = addressSpace.getULEB128(p, instructionsEnd);
@@ -426,21 +428,18 @@ bool CFI_Parser::parseInstructions(A
}
results->savedRegisters[reg].location = kRegisterInCFA;
results->savedRegisters[reg].value = offset;
- if (logDwarf)
-fprintf(stderr,
-"DW_CFA_offset_extended(reg=%" PRIu64 ", offset=%" PRId64
")\n",
-reg, offset);
+ _LIBUNWIND_TRACE_DWARF("DW_CFA_offset_extended(reg=%" PRIu64 ", "
+ "offset=%" PRId64 ")\n",
+ reg, offset);
break;
case DW_CFA_restore_extended:
reg = addressSpace.getULEB128(p, instructionsEnd);
- ;
if (reg > kMaxRegisterNumber) {
_LIBUNWIND_LOG("malformed DWARF DW_CFA_restore_extended, reg too big");
return false;
}
results->savedRegisters[reg] = initialState.savedRegisters[reg];
- if (logDwarf)
-fprintf(stderr, "DW_CFA_restore_extended(reg=%" PRIu64 ")\n", reg);
+ _LIBUNWIND_TRACE_DWARF("DW_CFA_restore_extended(reg=%" PRIu64 ")\n",
reg);
break;
case DW_CFA_undefined:
reg = addressSpace.getULEB128(p, instructionsEnd);
@@ -449,8 +448,7 @@
[libunwind] r292723 - X86: swap EBP, ESP on !APPLE
Author: compnerd
Date: Sat Jan 21 10:22:59 2017
New Revision: 292723
URL: http://llvm.org/viewvc/llvm-project?rev=292723=rev
Log:
X86: swap EBP, ESP on !APPLE
Restore the `libunwind.h` enumeration values back to the inverted
values. This diverges from the DWARF definition of the register values.
However, this allows our header to be compatible with other unwind
implementations (e.g. HP, GNU Savannah, GCC).
The register IDs are only swapped in the header and need to be unswapped
when accessing the unwind register file. The flipped EBP and ESP only
applies on non-Apple x86 targets.
When optimizations were enabled, EBP and ESP would no longer be
equivalent. As a result, the incorrect access on Linux would manifest
as a failure to unwind the stack. We can now unwind the stack with and
without FPO on Linux x86.
Resolves PR30879!
Modified:
libunwind/trunk/include/libunwind.h
libunwind/trunk/src/Registers.hpp
Modified: libunwind/trunk/include/libunwind.h
URL:
http://llvm.org/viewvc/llvm-project/libunwind/trunk/include/libunwind.h?rev=292723=292722=292723=diff
==
--- libunwind/trunk/include/libunwind.h (original)
+++ libunwind/trunk/include/libunwind.h Sat Jan 21 10:22:59 2017
@@ -165,13 +165,8 @@ enum {
UNW_X86_ECX = 1,
UNW_X86_EDX = 2,
UNW_X86_EBX = 3,
-#if defined(__CloudABI__) || defined(__FreeBSD__)
- UNW_X86_ESP = 4,
- UNW_X86_EBP = 5,
-#else
UNW_X86_EBP = 4,
UNW_X86_ESP = 5,
-#endif
UNW_X86_ESI = 6,
UNW_X86_EDI = 7
};
Modified: libunwind/trunk/src/Registers.hpp
URL:
http://llvm.org/viewvc/llvm-project/libunwind/trunk/src/Registers.hpp?rev=292723=292722=292723=diff
==
--- libunwind/trunk/src/Registers.hpp (original)
+++ libunwind/trunk/src/Registers.hpp Sat Jan 21 10:22:59 2017
@@ -122,9 +122,17 @@ inline uint32_t Registers_x86::getRegist
return _registers.__edx;
case UNW_X86_EBX:
return _registers.__ebx;
+#if !defined(__APPLE__)
+ case UNW_X86_ESP:
+#else
case UNW_X86_EBP:
+#endif
return _registers.__ebp;
+#if !defined(__APPLE__)
+ case UNW_X86_EBP:
+#else
case UNW_X86_ESP:
+#endif
return _registers.__esp;
case UNW_X86_ESI:
return _registers.__esi;
@@ -154,10 +162,18 @@ inline void Registers_x86::setRegister(i
case UNW_X86_EBX:
_registers.__ebx = value;
return;
+#if !defined(__APPLE__)
+ case UNW_X86_ESP:
+#else
case UNW_X86_EBP:
+#endif
_registers.__ebp = value;
return;
+#if !defined(__APPLE__)
+ case UNW_X86_EBP:
+#else
case UNW_X86_ESP:
+#endif
_registers.__esp = value;
return;
case UNW_X86_ESI:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[libunwind] r292721 - DWARF: convert error logs to _LIBUNWIND_LOG
Author: compnerd
Date: Sat Jan 21 10:22:55 2017
New Revision: 292721
URL: http://llvm.org/viewvc/llvm-project?rev=292721=rev
Log:
DWARF: convert error logs to _LIBUNWIND_LOG
Use the `_LIBUNWIND_LOG` macro instead of the explicit `fprintf` call.
NFC.
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=292721=292720=292721=diff
==
--- libunwind/trunk/src/DwarfParser.hpp (original)
+++ libunwind/trunk/src/DwarfParser.hpp Sat Jan 21 10:22:55 2017
@@ -421,8 +421,7 @@ 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_LOG("malformed DWARF DW_CFA_offset_extended, reg too big");
return false;
}
results->savedRegisters[reg].location = kRegisterInCFA;
@@ -436,9 +435,7 @@ bool CFI_Parser::parseInstructions(A
reg = addressSpace.getULEB128(p, instructionsEnd);
;
if (reg > kMaxRegisterNumber) {
-fprintf(
-stderr,
-"malformed DW_CFA_restore_extended DWARF unwind, reg too big\n");
+_LIBUNWIND_LOG("malformed DWARF DW_CFA_restore_extended, reg too big");
return false;
}
results->savedRegisters[reg] = initialState.savedRegisters[reg];
@@ -448,8 +445,7 @@ 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_LOG("malformed DWARF DW_CFA_undefined, reg too big");
return false;
}
results->savedRegisters[reg].location = kRegisterUnused;
@@ -459,8 +455,7 @@ 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_LOG("malformed DWARF DW_CFA_same_value, reg too big");
return false;
}
// DW_CFA_same_value unsupported
@@ -477,13 +472,11 @@ 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_LOG("malformed DWARF DW_CFA_register, reg too big");
return false;
}
if (reg2 > kMaxRegisterNumber) {
-fprintf(stderr,
-"malformed DW_CFA_register DWARF unwind, reg2 too big\n");
+_LIBUNWIND_LOG("malformed DWARF DW_CFA_register, reg2 too big");
return false;
}
results->savedRegisters[reg].location = kRegisterInRegister;
@@ -525,7 +518,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_LOG("malformed DWARF DW_CFA_def_cfa, reg too big");
return false;
}
results->cfaRegister = (uint32_t)reg;
@@ -537,9 +530,7 @@ 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_LOG("malformed DWARF DW_CFA_def_cfa_register, reg too big");
return false;
}
results->cfaRegister = (uint32_t)reg;
@@ -567,8 +558,7 @@ bool CFI_Parser::parseInstructions(A
case DW_CFA_expression:
reg = addressSpace.getULEB128(p, instructionsEnd);
if (reg > kMaxRegisterNumber) {
-fprintf(stderr,
-"malformed DW_CFA_expression DWARF unwind, reg too big\n");
+_LIBUNWIND_LOG("malformed DWARF DW_CFA_expression, reg too big");
return false;
}
results->savedRegisters[reg].location = kRegisterAtExpression;
@@ -583,9 +573,8 @@ bool CFI_Parser::parseInstructions(A
case DW_CFA_offset_extended_sf:
reg = addressSpace.getULEB128(p, instructionsEnd);
if (reg > kMaxRegisterNumber) {
-fprintf(
-stderr,
-"malformed DW_CFA_offset_extended_sf DWARF unwind, reg too big\n");
+
[libunwind] r292719 - rename OtherAddressSpace to RemoteAddressSpace; NFC
Author: compnerd
Date: Sat Jan 21 10:22:46 2017
New Revision: 292719
URL: http://llvm.org/viewvc/llvm-project?rev=292719=rev
Log:
rename OtherAddressSpace to RemoteAddressSpace; NFC
Modified:
libunwind/trunk/src/AddressSpace.hpp
libunwind/trunk/src/libunwind.cpp
Modified: libunwind/trunk/src/AddressSpace.hpp
URL:
http://llvm.org/viewvc/llvm-project/libunwind/trunk/src/AddressSpace.hpp?rev=292719=292718=292719=diff
==
--- libunwind/trunk/src/AddressSpace.hpp (original)
+++ libunwind/trunk/src/AddressSpace.hpp Sat Jan 21 10:22:46 2017
@@ -485,14 +485,14 @@ inline bool LocalAddressSpace::findFunct
#ifdef UNW_REMOTE
-/// OtherAddressSpace is used as a template parameter to UnwindCursor when
+/// RemoteAddressSpace is used as a template parameter to UnwindCursor when
/// unwinding a thread in the another process. The other process can be a
/// different endianness and a different pointer size which is handled by
/// the P template parameter.
template
-class OtherAddressSpace {
+class RemoteAddressSpace {
public:
- OtherAddressSpace(task_t task) : fTask(task) {}
+ RemoteAddressSpace(task_t task) : fTask(task) {}
typedef typename P::uint_t pint_t;
@@ -515,29 +515,29 @@ private:
task_t fTask;
};
-template uint8_t OtherAddressSpace::get8(pint_t addr) {
+template uint8_t RemoteAddressSpace::get8(pint_t addr) {
return *((uint8_t *)localCopy(addr));
}
-template uint16_t OtherAddressSpace::get16(pint_t addr) {
+template uint16_t RemoteAddressSpace::get16(pint_t addr) {
return P::E::get16(*(uint16_t *)localCopy(addr));
}
-template uint32_t OtherAddressSpace::get32(pint_t addr) {
+template uint32_t RemoteAddressSpace::get32(pint_t addr) {
return P::E::get32(*(uint32_t *)localCopy(addr));
}
-template uint64_t OtherAddressSpace::get64(pint_t addr) {
+template uint64_t RemoteAddressSpace::get64(pint_t addr) {
return P::E::get64(*(uint64_t *)localCopy(addr));
}
template
-typename P::uint_t OtherAddressSpace::getP(pint_t addr) {
+typename P::uint_t RemoteAddressSpace::getP(pint_t addr) {
return P::getP(*(uint64_t *)localCopy(addr));
}
template
-uint64_t OtherAddressSpace::getULEB128(pint_t , pint_t end) {
+uint64_t RemoteAddressSpace::getULEB128(pint_t , pint_t end) {
uintptr_t size = (end - addr);
LocalAddressSpace::pint_t laddr = (LocalAddressSpace::pint_t)
localCopy(addr);
LocalAddressSpace::pint_t sladdr = laddr;
@@ -547,7 +547,7 @@ uint64_t OtherAddressSpace::getULEB12
}
template
-int64_t OtherAddressSpace::getSLEB128(pint_t , pint_t end) {
+int64_t RemoteAddressSpace::getSLEB128(pint_t , pint_t end) {
uintptr_t size = (end - addr);
LocalAddressSpace::pint_t laddr = (LocalAddressSpace::pint_t)
localCopy(addr);
LocalAddressSpace::pint_t sladdr = laddr;
@@ -556,13 +556,14 @@ int64_t OtherAddressSpace::getSLEB128
return result;
}
-template void *OtherAddressSpace::localCopy(pint_t addr) {
+template void *RemoteAddressSpace::localCopy(pint_t addr) {
// FIX ME
}
template
-bool OtherAddressSpace::findFunctionName(pint_t addr, char *buf,
-size_t bufLen, unw_word_t *offset)
{
+bool RemoteAddressSpace::findFunctionName(pint_t addr, char *buf,
+ size_t bufLen,
+ unw_word_t *offset) {
// FIX ME
}
@@ -578,7 +579,7 @@ struct unw_addr_space {
/// a 32-bit intel process.
struct unw_addr_space_i386 : public unw_addr_space {
unw_addr_space_i386(task_t task) : oas(task) {}
- OtherAddressSpace oas;
+ RemoteAddressSpace oas;
};
/// unw_addr_space_x86_64 is the concrete instance that a unw_addr_space_t
@@ -586,7 +587,7 @@ struct unw_addr_space_i386 : public unw_
/// a 64-bit intel process.
struct unw_addr_space_x86_64 : public unw_addr_space {
unw_addr_space_x86_64(task_t task) : oas(task) {}
- OtherAddressSpace oas;
+ RemoteAddressSpace oas;
};
/// unw_addr_space_ppc is the concrete instance that a unw_addr_space_t points
@@ -594,7 +595,7 @@ struct unw_addr_space_x86_64 : public un
/// a 32-bit PowerPC process.
struct unw_addr_space_ppc : public unw_addr_space {
unw_addr_space_ppc(task_t task) : oas(task) {}
- OtherAddressSpace oas;
+ RemoteAddressSpace oas;
};
#endif // UNW_REMOTE
Modified: libunwind/trunk/src/libunwind.cpp
URL:
http://llvm.org/viewvc/llvm-project/libunwind/trunk/src/libunwind.cpp?rev=292719=292718=292719=diff
==
--- libunwind/trunk/src/libunwind.cpp (original)
+++ libunwind/trunk/src/libunwind.cpp Sat Jan 21 10:22:46 2017
@@ -85,18 +85,18 @@ _LIBUNWIND_EXPORT int unw_init_remote_th
switch (as->cpuType) {
case CPU_TYPE_I386:
new ((void *)cursor)
-UnwindCursor,
+
[libunwind] r292720 - config: clean up some of the macro definition
Author: compnerd
Date: Sat Jan 21 10:22:53 2017
New Revision: 292720
URL: http://llvm.org/viewvc/llvm-project?rev=292720=rev
Log:
config: clean up some of the macro definition
Unify the definition of `_LIBUNWIND_LOG_NON_ZERO` to make it clear what
it is defined to and make the definition unconditional. Reorder the
debug and tracing macros to be defined in the same order. NFC.
Modified:
libunwind/trunk/src/config.h
Modified: libunwind/trunk/src/config.h
URL:
http://llvm.org/viewvc/llvm-project/libunwind/trunk/src/config.h?rev=292720=292719=292720=diff
==
--- libunwind/trunk/src/config.h (original)
+++ libunwind/trunk/src/config.h Sat Jan 21 10:22:53 2017
@@ -85,22 +85,32 @@
fflush(stderr);
\
abort();
\
} while (0)
-#define _LIBUNWIND_LOG(msg, ...) fprintf(stderr, "libunwind: " msg "\n",
__VA_ARGS__)
+
+#define _LIBUNWIND_LOG(msg, ...)
\
+ fprintf(stderr, "libunwind: " msg "\n", __VA_ARGS__)
#if defined(_LIBUNWIND_HAS_NO_THREADS)
// only used with pthread calls, not needed for the single-threaded builds
#define _LIBUNWIND_LOG_NON_ZERO(x)
+#else
+ #if defined(NDEBUG)
+#define _LIBUNWIND_LOG_NON_ZERO(x) x
+ #else
+#define _LIBUNWIND_LOG_NON_ZERO(x)
\
+ do {
\
+int _err = x;
\
+if (_err != 0)
\
+ _LIBUNWIND_LOG("" #x "=%d in %s", _err, __FUNCTION__);
\
+ } while (0)
+ #endif
#endif
// Macros that define away in non-Debug builds
#ifdef NDEBUG
#define _LIBUNWIND_DEBUG_LOG(msg, ...)
#define _LIBUNWIND_TRACE_API(msg, ...)
- #define _LIBUNWIND_TRACING_UNWINDING 0
+ #define _LIBUNWIND_TRACING_UNWINDING (0)
#define _LIBUNWIND_TRACE_UNWINDING(msg, ...)
- #ifndef _LIBUNWIND_LOG_NON_ZERO
-#define _LIBUNWIND_LOG_NON_ZERO(x) x
- #endif
#else
#ifdef __cplusplus
extern "C" {
@@ -111,23 +121,17 @@
}
#endif
#define _LIBUNWIND_DEBUG_LOG(msg, ...) _LIBUNWIND_LOG(msg, __VA_ARGS__)
- #ifndef _LIBUNWIND_LOG_NON_ZERO
-#define _LIBUNWIND_LOG_NON_ZERO(x) \
- do { \
-int _err = x; \
-if ( _err != 0 ) \
- _LIBUNWIND_LOG("" #x "=%d in %s", _err, __FUNCTION__); \
- } while (0)
- #endif
- #define _LIBUNWIND_TRACE_API(msg, ...) \
-do { \
- if ( logAPIs() ) _LIBUNWIND_LOG(msg, __VA_ARGS__); \
-} while(0)
- #define _LIBUNWIND_TRACE_UNWINDING(msg, ...) \
-do { \
- if ( logUnwinding() ) _LIBUNWIND_LOG(msg, __VA_ARGS__); \
-} while(0)
+ #define _LIBUNWIND_TRACE_API(msg, ...)
\
+do {
\
+ if (logAPIs())
\
+_LIBUNWIND_LOG(msg, __VA_ARGS__);
\
+} while (0)
#define _LIBUNWIND_TRACING_UNWINDING logUnwinding()
+ #define _LIBUNWIND_TRACE_UNWINDING(msg, ...)
\
+do {
\
+ if (logUnwinding())
\
+_LIBUNWIND_LOG(msg, __VA_ARGS__);
\
+} while (0)
#endif
#ifdef __cplusplus
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28981: Use GNU-style attributes for several __throw_XXX() functions
dim created this revision.
In https://reviews.llvm.org/rL279744, `__throw_XXX()` functions were
introduced, partially for compatibility with software compiled against
libstdc++. `_LIBCPP_NORETURN` is used on all of them, and when C++11
attributes are available, this gets defined as `[[noreturn]]`, instead of the
GNU-style `__attribute__((noreturn))` style.
However, this can cause trouble with some software, for example Mozilla
Firefox, which attempts to redefine several of these `__throw_XXX()` functions,
and but uses the `__attribute__((noreturn))` style exclusively. This then
leads to errors like:
/usr/include/c++/v1/new:130:1: error: function declared '[[noreturn]]' after
its first declaration
_LIBCPP_NORETURN _LIBCPP_FUNC_VIS void __throw_bad_alloc(); // not in C++
spec
^
/usr/include/c++/v1/__config:283:30: note: expanded from macro
'_LIBCPP_NORETURN'
# define _LIBCPP_NORETURN [[noreturn]]
^
../../dist/include/mozilla/throw_gcc.h:35:1: note: declaration missing
'[[noreturn]]' attribute is here
__throw_bad_alloc(void)
^
See also https://bugs.freebsd.org/216186 and
https://bugzilla.mozilla.org/1329520 . In Firefox's case, the following list
of functions is affected:
__throw_bad_alloc(void)
__throw_bad_cast(void)
__throw_bad_exception(void)
__throw_bad_function_call(void)
__throw_bad_typeid(void)
__throw_domain_error(const char*)
__throw_invalid_argument(const char*)
__throw_ios_failure(const char*)
__throw_length_error(const char*)
__throw_logic_error(const char*)
__throw_out_of_range(const char*)
__throw_overflow_error(const char*)
__throw_range_error(const char*)
__throw_runtime_error(const char*)
__throw_system_error(int)
__throw_underflow_error(const char*)
To make it easier on third-party software, let's define a `LIBCPP_NORETURN_GNU`
variant, which always uses the `__attribute__((noreturn))` style, and apply it
specifically to the above list of functions.
This should also be merged to the 4.0 branch, when it has been committed.
https://reviews.llvm.org/D28981
Files:
include/__config
include/__locale
include/functional
include/new
include/stdexcept
include/system_error
include/typeinfo
Index: include/typeinfo
===
--- include/typeinfo
+++ include/typeinfo
@@ -190,7 +190,7 @@
} // std
_LIBCPP_BEGIN_NAMESPACE_STD
-_LIBCPP_NORETURN inline _LIBCPP_ALWAYS_INLINE
+_LIBCPP_NORETURN_GNU inline _LIBCPP_ALWAYS_INLINE
void __throw_bad_cast()
{
#ifndef _LIBCPP_NO_EXCEPTIONS
Index: include/system_error
===
--- include/system_error
+++ include/system_error
@@ -664,7 +664,7 @@
static string __init(const error_code&, string);
};
-_LIBCPP_NORETURN _LIBCPP_FUNC_VIS
+_LIBCPP_NORETURN_GNU _LIBCPP_FUNC_VIS
void __throw_system_error(int ev, const char* what_arg);
_LIBCPP_END_NAMESPACE_STD
Index: include/stdexcept
===
--- include/stdexcept
+++ include/stdexcept
@@ -183,9 +183,9 @@
_LIBCPP_BEGIN_NAMESPACE_STD
// in the dylib
-_LIBCPP_NORETURN _LIBCPP_FUNC_VIS void __throw_runtime_error(const char*);
+_LIBCPP_NORETURN_GNU _LIBCPP_FUNC_VIS void __throw_runtime_error(const char*);
-_LIBCPP_NORETURN inline _LIBCPP_ALWAYS_INLINE
+_LIBCPP_NORETURN_GNU inline _LIBCPP_ALWAYS_INLINE
void __throw_logic_error(const char*__msg)
{
#ifndef _LIBCPP_NO_EXCEPTIONS
@@ -196,7 +196,7 @@
#endif
}
-_LIBCPP_NORETURN inline _LIBCPP_ALWAYS_INLINE
+_LIBCPP_NORETURN_GNU inline _LIBCPP_ALWAYS_INLINE
void __throw_domain_error(const char*__msg)
{
#ifndef _LIBCPP_NO_EXCEPTIONS
@@ -207,7 +207,7 @@
#endif
}
-_LIBCPP_NORETURN inline _LIBCPP_ALWAYS_INLINE
+_LIBCPP_NORETURN_GNU inline _LIBCPP_ALWAYS_INLINE
void __throw_invalid_argument(const char*__msg)
{
#ifndef _LIBCPP_NO_EXCEPTIONS
@@ -218,7 +218,7 @@
#endif
}
-_LIBCPP_NORETURN inline _LIBCPP_ALWAYS_INLINE
+_LIBCPP_NORETURN_GNU inline _LIBCPP_ALWAYS_INLINE
void __throw_length_error(const char*__msg)
{
#ifndef _LIBCPP_NO_EXCEPTIONS
@@ -229,7 +229,7 @@
#endif
}
-_LIBCPP_NORETURN inline _LIBCPP_ALWAYS_INLINE
+_LIBCPP_NORETURN_GNU inline _LIBCPP_ALWAYS_INLINE
void __throw_out_of_range(const char*__msg)
{
#ifndef _LIBCPP_NO_EXCEPTIONS
@@ -240,7 +240,7 @@
#endif
}
-_LIBCPP_NORETURN inline _LIBCPP_ALWAYS_INLINE
+_LIBCPP_NORETURN_GNU inline _LIBCPP_ALWAYS_INLINE
void __throw_range_error(const char*__msg)
{
#ifndef _LIBCPP_NO_EXCEPTIONS
@@ -251,7 +251,7 @@
#endif
}
-_LIBCPP_NORETURN inline _LIBCPP_ALWAYS_INLINE
+_LIBCPP_NORETURN_GNU inline _LIBCPP_ALWAYS_INLINE
void __throw_overflow_error(const char*__msg)
{
#ifndef _LIBCPP_NO_EXCEPTIONS
@@ -262,7 +262,7 @@
#endif
}
-_LIBCPP_NORETURN inline _LIBCPP_ALWAYS_INLINE
+_LIBCPP_NORETURN_GNU inline _LIBCPP_ALWAYS_INLINE
void __throw_underflow_error(const char*__msg)
{
#ifndef
[PATCH] D26753: ASTImporter: improve support for C++ templates
a.sidorin added a comment. I got it. I have hard-coded paths in CHECK-lines so these tests are passed on my machine but not on other. Thank you Kareem! https://reviews.llvm.org/D26753 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28785: Split exception.cpp and new.cpp implementation into different files for different runtimes
EricWF added a comment. In https://reviews.llvm.org/D28785#651156, @EricWF wrote: > In https://reviews.llvm.org/D28785#650101, @compnerd wrote: > > > While I love this direction (the original version really was an > > unintelligible pile of code), I really think that this change may be taking > > on too much. Why not split it up first and do nothing else. We could do > > the MS ABI implementation in a subsequent change. This would improve the > > code and would not be gated on the MS ABI changes. > > > I agree this review is taking on too much, it started out much smaller and I > tried to avoid expanding it, but in the end I had three options: > > A) Regress and remove all support for MSVC, this would break the windows > build. (at least in `exception.cpp` and `new.cpp`). > B) Implement incorrect versions of `support/runtime/_msvc.ipp` > based on w/e we currently have, just to keep Windows building. > C) Implement correct versions of `support/runtime/_msvc.ipp`. > > I choose (C) since I didn't want to regress Windows, or spend time > implementing incorrect `_msvc.ipp` versions. > However I'm willing to try and shrink this down if you think that would be > better. That's dumb, and I'm dumb. I should be perfectly capable of breaking this down into smaller pieces https://reviews.llvm.org/D28785 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[libcxx] r292717 - Revert accidentally changes which reverted r292582
Author: ericwf
Date: Sat Jan 21 08:42:44 2017
New Revision: 292717
URL: http://llvm.org/viewvc/llvm-project?rev=292717=rev
Log:
Revert accidentally changes which reverted r292582
Modified:
libcxx/trunk/include/new
Modified: libcxx/trunk/include/new
URL:
http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/new?rev=292717=292716=292717=diff
==
--- libcxx/trunk/include/new (original)
+++ libcxx/trunk/include/new Sat Jan 21 08:42:44 2017
@@ -145,7 +145,7 @@ public:
#endif // defined(_LIBCPP_BUILDING_NEW) || (_LIBCPP_STD_VER > 11)
-#ifndef _LIBCPP_HAS_NO_ALIGNED_ALLOCATION
+#if !defined(_LIBCPP_HAS_NO_ALIGNED_ALLOCATION) || _LIBCPP_STD_VER > 14
#ifndef _LIBCPP_CXX03_LANG
enum class _LIBCPP_ENUM_VIS align_val_t : size_t { };
#else
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D27621: [clang-tidy] check to find declarations declaring more than one name
firolino marked 7 inline comments as done.
firolino added a comment.
- nothing special, just went through some open comments and marked them as Done.
Comment at: clang-tidy/readability/OneNamePerDeclarationCheck.cpp:246
+
+static std::string getCurrentLineIndent(SourceLocation Loc,
+const SourceManager ) {
aaron.ballman wrote:
> firolino wrote:
> > malcolm.parsons wrote:
> > > Should checks care about formatting, or leave to clang-format?
> > At least, they should not change the formatting and try to preserve it. The
> > user shouldn't be forced to run clang-format multiple times. Moreover, not
> > everyone is using clang-format etc.
> >
> > Having:
> > ```
> > const int myvalue( 42 ), value ( 4 );
> >
> > {
> > int a, b, c;
> > }
> > ```
> > and getting:
> > ```
> > const int myvalue(42);
> > const int value(4);
> >
> > {
> > int a;
> > int b;
> > int c;
> > }
> > ```
> > seems very unsatisfying to me. This would force a non-formatting-tool user
> > to start using one.
> I think what @malcolm.parsons was getting at is that we have talked about
> automatically running clang-format over changes made from the FixIt engine
> instead of trying to add custom logic to all of the places we use FixIts.
Sounds good! Will clang-format be able to detect the code format in the future?
https://reviews.llvm.org/D27621
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D27621: [clang-tidy] check to find declarations declaring more than one name
firolino added a comment. @aaron.ballman I am going to implement `CppCore`, `Cert` and `Everything` and test it on some projects and provide the results next week, to find out which one to set `Default`. https://reviews.llvm.org/D27621 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D27621: [clang-tidy] check to find declarations declaring more than one name
firolino added inline comments.
Comment at: clang-tidy/readability/OneNamePerDeclarationCheck.h:25
+class OneNamePerDeclarationCheck : public ClangTidyCheck {
+private:
+ std::string getUserWrittenType(const DeclStmt *DeclStmt, SourceManager );
firolino wrote:
> alexfh wrote:
> > nit: move the private section to the end.
> [[ http://llvm.org/docs/CodingStandards.html#use-of-class-and-struct-keywords
> | LLVM Coding Standards ]] is putting the private section at the beginning as
> well. Even in the code base, it seems to be the general rule. While you are
> reading code from top to bottom, you need to get the necessary context
> (variables and their types) first, before proceeding to the functional code.
> Otherwise, you would have to scroll all the time down to the private
> variables - when passing its usage in a function - to get its type. This
> would impair the reading fluency and thus the straight-forwarding
> understanding of the code.
Or is it, because there is just a function and not a private variable?
https://reviews.llvm.org/D27621
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D27621: [clang-tidy] check to find declarations declaring more than one name
firolino added inline comments.
Comment at: clang-tidy/readability/OneNamePerDeclarationCheck.cpp:153
+ const SourceRange FVLoc(DeclStmt->getLocStart(), Location);
+ std::string UserWrittenType =
+ Lexer::getSourceText(CharSourceRange::getCharRange(FVLoc), SM,
alexfh wrote:
> I wonder whether re-lexing the whole range and working on tokens instead
> would be a better and more robust approach? You wouldn't have to deal with
> whitespace and comments then. What do you think?
Sounds interesting! I am gonna give this a try.
Comment at: clang-tidy/readability/OneNamePerDeclarationCheck.cpp:163
+ // const int *& -> const int
+ // long **-> long int
+ size_t Pos = std::string::npos;
alexfh wrote:
> Why do we want `long int` and not `long`?
Sorry, misleading comment.
Comment at: clang-tidy/readability/OneNamePerDeclarationCheck.h:25
+class OneNamePerDeclarationCheck : public ClangTidyCheck {
+private:
+ std::string getUserWrittenType(const DeclStmt *DeclStmt, SourceManager );
alexfh wrote:
> nit: move the private section to the end.
[[ http://llvm.org/docs/CodingStandards.html#use-of-class-and-struct-keywords |
LLVM Coding Standards ]] is putting the private section at the beginning as
well. Even in the code base, it seems to be the general rule. While you are
reading code from top to bottom, you need to get the necessary context
(variables and their types) first, before proceeding to the functional code.
Otherwise, you would have to scroll all the time down to the private variables
- when passing its usage in a function - to get its type. This would impair the
reading fluency and thus the straight-forwarding understanding of the code.
Comment at: clang-tidy/utils/LexerUtils.cpp:41-56
+ const auto = Context.getSourceManager();
+ const auto = Context.getLangOpts();
+ auto StartOfFile = SM.getLocForStartOfFile(SM.getFileID(Location));
+
+ Token CurrentToken;
+ CurrentToken.setKind(tok::unknown);
+
alexfh wrote:
> Is it significantly more efficient than
>
> Token Tok;
> do {
> Tok = getPreviousNonCommentToken(Context, Location);
> } while (!Tok.isOneOf(tok::unknown, TokenToFind));
> return Tok.getLocation();
>
> ?
>
> If it's not, then this function might be not so much useful to be here. At
> least, its interface is rather specific to the use case you have. Why, for
> example, it returns a location instead of the token itself?
More efficient? In worst-case (token not found) yes, in my use-case probably
not that significant. You are right, changing the interface to `Token
findTokenBackward(...)` seems to be more generic than returning the location
directly.
https://reviews.llvm.org/D27621
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28952: [analyzer] Add new Z3 constraint manager backend
a.sidorin added a comment. Amazing work, Dominic. That's what I wanted to test for long time. But, personally, I'm not happy with massive changes in tests. 1. I don't think that we need to change run line for tests if they pass with both managers. These changes are pretty noisy, 2. If Z3 is optional, we cannot enforce its usage. 3. How testing will be performed without Z3? I think there should be a way to separate tests between RangeConstraintManager and Z3. Are we going to drop RangeConstraintManager support? If no, we should care about backward compatibility until this happens. https://reviews.llvm.org/D28952 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
