[cross-project] Add tests for LLDB data-formatters for llvm::ArrayRef#173238
Conversation
This patch bare-minimum tests for the LLDB `llvm::ArrayRef` formatters. I wanted to keep the test itself minimal and mainly set up/agree on the infrastructure (i.e., CMake machinery, etc.). The setup mimicks that of GDB. The main difference is that the GDB formatter tests put all the test cases in one monolithic test file, whereas I'm planning on having one test-file per LLVM container. Note, the tests currently only get run if LLVM was built with debug-info. Not sure we have any buildbots out there runing this configuration. Though that's what the GDB formatters do too. We could probably get away with relying on just the debug-info from the LLVM headers. We could do that in a follow-up patch, or part of this PR, depending on how reviewers feel.
Michael137
requested review from
JDevlieghere,
adrian-prantl,
bulbazord,
dwblaikie and
kastiglione
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
Michael137
force-pushed
the
cross-project-tests/llvm-lldb-formatter-tests
branch
from
6fb3ab0 to
841e2e5
Compare
Both those things seem fine by me - given the gdb tests have llvm and mlir coverage, you could consider an extra nested directory to group the llvm ones so mlir ones can go in another directory beside it, etc. But can also cross that bridge if/when we come to it. (I think the llvm one was also named after the library - |
Thanks for calling this out. It would be very disappointing if we added the tests but no buildbots were running the right configuration to actually run the test. +1 on the idea overall. I don't have any strong opinions about where you've placed them and you appear to have done your homework. |
…173261) Depends on: * #173238 (only last commit relevant for review) This patch revives the `llvm::PointerIntPair` LLDB data-formatter. The previous version was commented out because it relied on expression evaluation and was hence slow/brittle. The formatter in this PR doesn't rely on evaluating expressions. Drive-by change: * removes the `llvm::PointerUnion` formatter which was also commented out. A future version of it will look very different than it does now, so there's no point in keeping it because the diff won't be helpful in a review.
…erIntPair (#173261) Depends on: * llvm/llvm-project#173238 (only last commit relevant for review) This patch revives the `llvm::PointerIntPair` LLDB data-formatter. The previous version was commented out because it relied on expression evaluation and was hence slow/brittle. The formatter in this PR doesn't rely on evaluating expressions. Drive-by change: * removes the `llvm::PointerUnion` formatter which was also commented out. A future version of it will look very different than it does now, so there's no point in keeping it because the diff won't be helpful in a review.
| debuginfo-tests/llvm-prettyprinters/lldb/arrayref.cpp | ||
| ) | ||
| target_link_libraries(check-lldb-llvm-support-arrayref PRIVATE LLVMSupport) | ||
| target_compile_options(check-lldb-llvm-support-arrayref PRIVATE -g -O0) |
There was a problem hiding this comment.
Hello @Michael137
I noticed that if you compile with -D_FORTIFY_SOURCE=2 we now get
In file included from /app/gcc/14.2.0/include/c++/14.2.0/x86_64-pc-linux-gnu/bits/os_defines.h:39,
from /app/gcc/14.2.0/include/c++/14.2.0/x86_64-pc-linux-gnu/bits/c++config.h:680,
from /app/gcc/14.2.0/include/c++/14.2.0/type_traits:38,
from ../include/llvm/ADT/ADL.h:12,
from ../include/llvm/ADT/Hashing.h:47,
from ../include/llvm/ADT/ArrayRef.h:12,
from ../../cross-project-tests/debuginfo-tests/llvm-prettyprinters/lldb/arrayref.cpp:1:
/usr/include/features.h:381:4: warning: #warning _FORTIFY_SOURCE requires compiling with optimization (-O) [-Wcpp]
381 | # warning _FORTIFY_SOURCE requires compiling with optimization (-O)
| ^~~~~~~
when compiling cross-project-tests. And if compiling with -Werror, the build then fails.
I suppose it's because of the hardcoded "-O0" in the config above.
There was a problem hiding this comment.
Thanks for the ping. Not sure off the top how to best handle this but I'll have a look
There was a problem hiding this comment.
Downstream I worked around it by undefining _FORTIFY_SOURCE when we add -O0 like
-target_compile_options(check-lldb-llvm-support-arrayref PRIVATE -g -O0)
+target_compile_options(check-lldb-llvm-support-arrayref PRIVATE -g -O0 -U_FORTIFY_SOURCE)
but I don't know if there is anything better that can be done.
There was a problem hiding this comment.
Heh yea maybe that's worth a shot, thanks! I'll put up a review
|
|
||
| #--- checks | ||
| # CHECK: (lldb) p ArrayRef | ||
| # CHECK-NEXT: (llvm::ArrayRef<int>) size=3 { |
There was a problem hiding this comment.
Hi @Michael137
I randomly see this testcase failing on a downstream build bot. I've seen failures like this:
/repo/llvm/build-all-builtins/projects/cross-project-tests/debuginfo-tests/llvm-prettyprinters/lldb/Output/arrayref.test.tmp/checks:2:15: error: CHECK-NEXT: is not on the line after the previous match
# CHECK-NEXT: (llvm::ArrayRef<int>) size=3 {
^
<stdin>:20:1: note: 'next' match was here
(llvm::ArrayRef<int>) size=3 {
^
<stdin>:18:18: note: previous match ended here
(lldb) p ArrayRef
^
<stdin>:19:1: note: non-matching line after previous match is here
1 location added to breakpoint 1
^
Input file: <stdin>
Check file: /repo/llvm/build-all-builtins/projects/cross-project-tests/debuginfo-tests/llvm-prettyprinters/lldb/Output/arrayref.test.tmp/checks
-dump-input=help explains the following input dump.
Input was:
<<<<<<
.
.
.
15: 7
16: -> 8 int main() { return 0; }
17: ^
18: (lldb) p ArrayRef
19: 1 location added to breakpoint 1
20: (llvm::ArrayRef<int>) size=3 {
next:2 !~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ error: match on wrong line
21: [0] = 1
22: [1] = 2
23: [2] = 3
24: }
25: (lldb) p MutableArrayRef
.
.
.
>>>>>>
--
********************
Testing: 0.. 10.. 20.. 30.. 40.. 50.. 60.. 70.. 80.. 90..
1 warning(s) in tests
********************
Failed Tests (1):
cross-project-tests :: debuginfo-tests/llvm-prettyprinters/lldb/arrayref.test
so there something unexpected between the expected lines in the output.
It happens now and then. Any idea what the problem is and if it can be fixed?
There was a problem hiding this comment.
Hmm interesting, looks like it's this:
1 location added to breakpoint 1
That would happen on Linux when LLDB finds the breakpoint on main when the process is launched. Might be due to machine load/LLDB sluggishness that the output is printed later than expected. We can probably remove the check for p ArrayRef. Or not make it a CHECK-NEXT.
There was a problem hiding this comment.
Thanks. I'll keep an eye on our build bot and get back if we see any more flakiness after that.
See #173238 (comment) On some downstream CI this was flakey because LLDB's `location added to breakpoint` notification would appear after the commands. Simply remove checking for the command for now.
…tatement See llvm/llvm-project#173238 (comment) On some downstream CI this was flakey because LLDB's `location added to breakpoint` notification would appear after the commands. Simply remove checking for the command for now.
…#173261) Depends on: * llvm/llvm-project#173238 (only last commit relevant for review) This patch revives the `llvm::PointerIntPair` LLDB data-formatter. The previous version was commented out because it relied on expression evaluation and was hence slow/brittle. The formatter in this PR doesn't rely on evaluating expressions. Drive-by change: * removes the `llvm::PointerUnion` formatter which was also commented out. A future version of it will look very different than it does now, so there's no point in keeping it because the diff won't be helpful in a review.
See llvm#173238 (comment) On some downstream CI this was flakey because LLDB's `location added to breakpoint` notification would appear after the commands. Simply remove checking for the command for now.
…#173261) Depends on: * llvm/llvm-project#173238 (only last commit relevant for review) This patch revives the `llvm::PointerIntPair` LLDB data-formatter. The previous version was commented out because it relied on expression evaluation and was hence slow/brittle. The formatter in this PR doesn't rely on evaluating expressions. Drive-by change: * removes the `llvm::PointerUnion` formatter which was also commented out. A future version of it will look very different than it does now, so there's no point in keeping it because the diff won't be helpful in a review. (cherry picked from commit 9471f53)
See llvm/llvm-project#173238 (comment) On some downstream CI this was flakey because LLDB's `location added to breakpoint` notification would appear after the commands. Simply remove checking for the command for now. (cherry picked from commit 85f5007)
See llvm#173238 (comment) On some downstream CI this was flakey because LLDB's `location added to breakpoint` notification would appear after the commands. Simply remove checking for the command for now.
5 participants
This patch adds the bare-minimum tests for the LLDB
llvm::ArrayRefformatters. Since this would be the first LLVM data fromatter test for LLDB, I wanted to keep the test itself minimal and mainly set up/agree on the infrastructure (i.e., CMake machinery, etc.).The setup mimicks that of GDB. The main differences are: