Commit Graph

13 Commits

Author SHA1 Message Date
Jaroslav Sevcik
177dd63c8d Data formatters: fix detection of C strings
Summary:
Detection of C strings does not work well for pointers. If the value object holding a (char*) pointer does not have an address (e.g., if it is a temp), the value is not considered a C string and its formatting is left to DumpDataExtractor rather than the special handling in  ValueObject::DumpPrintableRepresentation. This leads to inconsistent outputs, e.g., in escaping non-ASCII characters. See the test for an example; the second test expectation is not met (without this patch). With this patch, the C string detection only insists that the pointer value is valid. The patch makes the code consistent with how the pointer is obtained in ValueObject::ReadPointedString.

Reviewers: teemperor

Reviewed By: teemperor

Subscribers: lldb-commits

Tags: #lldb

Differential Revision: https://reviews.llvm.org/D76650
2020-03-24 14:25:59 +01:00
shafik
a567d6809e [DataFormatters] Add formatter for libc++ std::unique_ptr
This adds a formatter for libc++ std::unique_ptr.

I also refactored GetValueOfCompressedPair(...) out of LibCxxList.cpp since I need the same functionality and it made sense to share it.

Differential Revision: https://reviews.llvm.org/D76476
2020-03-23 11:48:20 -07:00
Raphael Isemann
467c4902a1 [lldb] Enable now passing part of TestDataFormatterStdString.py
This was fixed by 7b2442584e .
2020-03-20 11:35:15 +01:00
Raphael Isemann
7b2442584e Reland [lldb] Fix string summary of an empty NSPathStore2
(This is D68010 but I also set the new parameter in LibStdcpp.cpp to fix
the Debian tests).

Summary:
Printing a summary for an empty NSPathStore2 string currently prints random bytes behind the empty string pointer from memory (rdar://55575888).

It seems the reason for this is that the SourceSize parameter in the `ReadStringAndDumpToStreamOptions` - which is supposed to contain the string
length - actually uses the length 0 as a magic value for saying "read as much as possible from the buffer" which is clearly wrong for empty strings.

This patch adds another flag that indicates if we have know the string length or not and makes this behaviour dependent on that (which seemingly
was the original purpose of this magic value).

Reviewers: aprantl, JDevlieghere, shafik

Reviewed By: aprantl

Subscribers: christof, abidh, lldb-commits

Tags: #lldb

Differential Revision: https://reviews.llvm.org/D68010
2020-03-19 18:50:26 +01:00
Raphael Isemann
718d94187d Revert "[lldb] Fix string summary of an empty NSPathStore2"
This reverts commit 939ca455e7.

This failed on the debian bot for some reason:
  File "/home/worker/lldb-x86_64-debian/lldb-x86_64-debian/llvm-project/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/TestDataFormatterStdString.py", line 67, in test_with_run_command
    "s summary wrong")
AssertionError: 'L"hello world! מזל טוב!\\0!\\0!!!!\\0\\0A\\0\\U0000fffd\\U0000fffd\\U0000fffd\\ [truncated]... != 'L"hello world! מזל טוב!"'
Diff is 2156 characters long. Set self.maxDiff to None to see it. : s summary wrong
2020-03-19 13:08:39 +01:00
Raphael Isemann
939ca455e7 [lldb] Fix string summary of an empty NSPathStore2
Summary:
Printing a summary for an empty NSPathStore2 string currently prints random bytes behind the empty string pointer from memory (rdar://55575888).

It seems the reason for this is that the SourceSize parameter in the `ReadStringAndDumpToStreamOptions` - which is supposed to contain the string
length - actually uses the length 0 as a magic value for saying "read as much as possible from the buffer" which is clearly wrong for empty strings.

This patch adds another flag that indicates if we have know the string length or not and makes this behaviour dependent on that (which seemingly
was the original purpose of this magic value).

Reviewers: aprantl, JDevlieghere, shafik

Reviewed By: aprantl

Subscribers: christof, abidh, lldb-commits

Tags: #lldb

Differential Revision: https://reviews.llvm.org/D68010
2020-03-19 12:20:35 +01:00
Vedant Kumar
8a0f0e2656 [lldb/test] Tweak libcxx string test on Apple+ARM devices
On Apple platforms, is __arm__ isn't defined and we're not on Intel, we use an
alternate std::string layout. I.e., the libcxx string test fails on phones
because the hand-crafted "garbage" string structs are actually valid strings.

See:

```
  // _LIBCPP_ALTERNATE_STRING_LAYOUT is an old name for
  // _LIBCPP_ABI_ALTERNATE_STRING_LAYOUT left here for backward compatibility.
  #if (defined(__APPLE__) && !defined(__i386__) && !defined(__x86_64__) &&       \
       (!defined(__arm__) || __ARM_ARCH_7K__ >= 2)) ||                           \
      defined(_LIBCPP_ALTERNATE_STRING_LAYOUT)
  #define _LIBCPP_ABI_ALTERNATE_STRING_LAYOUT
  #endif
```

Disable inspection of the garbage structs on Apple+ARM devices.
2020-02-21 15:54:38 -08:00
Raphael Isemann
fdea9a4ec9 [lldb] Remove license headers from all test source files
Summary:
Around a third of our test sources have LLVM license headers. This patch removes those headers from all test
sources and also fixes any tests that depended on the length of the license header.

The reasons for this are:

* A few tests verify line numbers and will start failing if the number of lines in the LLVM license header changes. Once I landed my patch for valid SourceLocations in debug info we will probably have even more tests that verify line numbers.
* No other LLVM project is putting license headers in its test files to my knowledge.
* They make the test sources much more verbose than they have to be. Several tests have longer license headers than the actual test source.

For the record, the following tests had their line numbers changed to pass with the removal of the license header:
    lldb-api :: functionalities/breakpoint/breakpoint_by_line_and_column/TestBreakpointByLineAndColumn.py
    lldb-shell :: Reproducer/TestGDBRemoteRepro.test
    lldb-shell :: Reproducer/TestMultipleTargets.test
    lldb-shell :: Reproducer/TestReuseDirectory.test
    lldb-shell :: ExecControl/StopHook/stop-hook-threads.test
    lldb-shell :: ExecControl/StopHook/stop-hook.test
    lldb-api :: lang/objc/exceptions/TestObjCExceptions.py

Reviewers: #lldb, espindola, JDevlieghere

Reviewed By: #lldb, JDevlieghere

Subscribers: emaste, aprantl, arphaman, JDevlieghere, lldb-commits

Tags: #lldb

Differential Revision: https://reviews.llvm.org/D74839
2020-02-20 08:32:01 +01:00
Raphael Isemann
785df61680 [lldb] Let TypeSystemClang::GetDisplayTypeName remove anonymous and inline namespaces.
Summary:
Currently when printing data types we include implicit scopes such as inline namespaces or anonymous namespaces.
This leads to command output like this (for `std::set<X>` with X being in an anonymous namespace):

```
(lldb) print my_set
(std::__1::set<(anonymous namespace)::X, std::__1::less<(anonymous namespace)::X>, std::__1::allocator<(anonymous namespace)::X> >) $0 = size=0 {}
```

This patch removes all the implicit scopes when printing type names in TypeSystemClang::GetDisplayTypeName
so that our output now looks like this:

```
(lldb) print my_set
(std::set<X, std::less<X>, std::allocator<X> >) $0 = size=0 {}
```

As previously GetDisplayTypeName and GetTypeName had the same output we actually often used the
two as if they are the same method (they were in fact using the same implementation), so this patch also
fixes the places where we actually want the display type name and not the actual type name.

Note that this doesn't touch the `GetTypeName` class that for example the data formatters use, so this patch
is only changes the way we display types to the user. The full type name can also still be found when passing
'-R' to see the raw output of a variable in case someone is somehow interested in that.

Partly fixes rdar://problem/59292534

Reviewers: shafik, jingham

Reviewed By: shafik

Subscribers: christof, JDevlieghere, lldb-commits

Tags: #lldb

Differential Revision: https://reviews.llvm.org/D74478
2020-02-19 10:30:11 +01:00
Raphael Isemann
51d8c59833 [lldb] Don't model std::atomic as a transparent data structure in the data formatter
Summary:
Currently the data formatter is treating `std::atomic` variables as transparent wrappers
around their underlying value type. This causes that when printing `std::atomic<A *>`, the data
formatter will forward all requests for the children of the atomic variable to the `A *` pointer type
which will then return the respective members of `A`. If `A` in turn has a member that contains
the original atomic variable, this causes LLDB to infinitely recurse when printing an object with
such a `std::atomic` pointer member.

We could implement a workaround similar to whatever we do for pointer values but this patch
just implements the `std::atomic` formatter in the same way as we already implement other
formatters (e.g. smart pointers or `std::optional`) that just model the contents of the  as a child
"Value". This way LLDB knows when it actually prints a pointer and can just use its normal
workaround if "Value" is a recursive pointer.

Fixes rdar://59189235

Reviewers: JDevlieghere, jingham, shafik

Reviewed By: shafik

Subscribers: shafik, christof, jfb, abidh, lldb-commits

Tags: #lldb

Differential Revision: https://reviews.llvm.org/D74310
2020-02-18 11:22:12 +01:00
Raphael Isemann
b3a0c4d7dc [lldb] Replace assertTrue(a == b, "msg") with assertEquals(a, b, "msg") in the test suite
Summary:
The error message from the construct `assertTrue(a == b, "msg") ` are nearly always completely useless for actually debugging the issue.
This patch is just replacing this construct (and similar ones like `assertTrue(a != b, ...)` with the proper call to assertEqual or assertNotEquals.

This patch was mostly written by a shell script with some manual verification afterwards:
```
lang=python
import sys

def sanitize_line(line):
  if line.strip().startswith("self.assertTrue(") and " == " in line:
    line = line.replace("self.assertTrue(", "self.assertEquals(")
    line = line.replace(" == ", ", ", 1)
  if line.strip().startswith("self.assertTrue(") and " != " in line:
    line = line.replace("self.assertTrue(", "self.assertNotEqual(")
    line = line.replace(" != ", ", ", 1)
  return line

for a in sys.argv[1:]:
  with open(a, "r") as f:
    lines = f.readlines()
  with open(a, "w") as f:
    for line in lines:
      f.write(sanitize_line(line))
```

Reviewers: labath, JDevlieghere

Reviewed By: labath

Subscribers: abidh, lldb-commits

Tags: #lldb

Differential Revision: https://reviews.llvm.org/D74475
2020-02-13 15:00:55 +01:00
Vedant Kumar
7aabad1312 [lldb/StringPrinter] Avoid reading garbage in uninitialized strings
This patch fixes a few related out-of-bounds read bugs in the
string data formatters. These issues have to do with mishandling of un-
initialized strings. These manifest as ASan exceptions when debugging a
clang binary.

The first issue was that the std::string formatter treated strings in
"short mode" with length greater than the size of the inline buffer as
valid.

The second issue was that the StringPrinter facility did not check that
a full utf8 codepoint sequence can be read from the buffer (i.e. there
are some missing range checks). I took the opportunity here to delete
some untested code that was meant to deal with invalid input and replace
it with fail-on-invalid logic ([1][2][3]). This means we'll give up on
formatting an invalid string instead of guessing our way through it.

The third issue is that StringPrinter did not check that a utf8 sequence
could actually be fully read from the string payload. This one is especially
tricky as we may overflow the buffer pointer while reading the sequence.

I also noticed that the std::string formatter would spew the raw version of
the underlying ValueObject when garbage is detected. I've changed this to
just print "Summary Unavailable" instead, as we do elsewhere.

I've added regression tests for these issues to
test/functionalities/data-formatter/data-formatter-stl/libcxx/string.

[1]
http://lab.llvm.org:8080/coverage/coverage-reports/coverage/Users/buildslave/jenkins/workspace/coverage/llvm-project/lldb/source/DataFormatters/StringPrinter.cpp.html#L136
[2]
http://lab.llvm.org:8080/coverage/coverage-reports/coverage/Users/buildslave/jenkins/workspace/coverage/llvm-project/lldb/source/DataFormatters/StringPrinter.cpp.html#L163
[3]
http://lab.llvm.org:8080/coverage/coverage-reports/coverage/Users/buildslave/jenkins/workspace/coverage/llvm-project/lldb/source/DataFormatters/StringPrinter.cpp.html#L357

rdar://59080026

Differential Revision: https://reviews.llvm.org/D73860
2020-02-12 11:24:03 -08:00
Jordan Rupprecht
99451b4453 [lldb][test] Remove symlink for API tests.
Summary: Moves lldbsuite tests to lldb/test/API.

This is a largely mechanical change, moved with the following steps:

```
rm lldb/test/API/testcases
mkdir -p lldb/test/API/{test_runner/test,tools/lldb-{server,vscode}}
mv lldb/packages/Python/lldbsuite/test/test_runner/test lldb/test/API/test_runner
for d in $(find lldb/packages/Python/lldbsuite/test/* -maxdepth 0 -type d | egrep -v "make|plugins|test_runner|tools"); do mv $d lldb/test/API; done
for d in $(find lldb/packages/Python/lldbsuite/test/tools/lldb-vscode -maxdepth 1 -mindepth 1 | grep -v ".py"); do mv $d lldb/test/API/tools/lldb-vscode; done
for d in $(find lldb/packages/Python/lldbsuite/test/tools/lldb-server -maxdepth 1 -mindepth 1 | egrep -v "gdbremote_testcase.py|lldbgdbserverutils.py|socket_packet_pump.py"); do mv $d lldb/test/API/tools/lldb-server; done
```

lldb/packages/Python/lldbsuite/__init__.py and lldb/test/API/lit.cfg.py were also updated with the new directory structure.

Reviewers: labath, JDevlieghere

Tags: #lldb

Differential Revision: https://reviews.llvm.org/D71151
2020-02-11 10:03:53 -08:00