Commit 51d2e988 authored by Seth Brenith's avatar Seth Brenith Committed by Commit Bot

Fix brittleness of GetObjectProperties test

Part of the GetObjectProperties test case is for verifying the human-
readable brief object description string that GetObjectProperties
returns. That string might look something like this:

"xy" (0x28f038d5 <v8::internal::SeqOneByteString>)

GetObjectProperties also tries to detect known immortal objects by
recognizing their addresses, which is useful in crash dumps with limited
memory. The recognized object name, if it exists, is prepended to the
description string. In order to provide this data accurately (in builds
without pointer compression), GetObjectProperties relies on the caller
to provide the addresses of the first pages in read-only space, map
space, and old space. If the caller doesn't provide those addresses,
then GetObjectProperties does the best it can with limited information
and reports possible matches based on an object's offset within the heap
page that contains it. So the result string might look like this, if the
object happened to get allocated at a lucky offset within its page:

maybe LoadHandler3Map "xy" (0x28f038d5 <v8::internal::SeqOneByteString>)

As a result, when testing these descriptions, we should generally check
that they contain the interesting data rather than that they start with
it, because some incorrect "maybe" match with a known object might be
included at the beginning.

Bug: v8:10034
Change-Id: I0cf5afd67793a239614aba3665ef57cd2d663a47
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1950233Reviewed-by: 's avatarTobias Tebbi <tebbi@chromium.org>
Commit-Queue: Seth Brenith <seth.brenith@microsoft.com>
Cr-Commit-Position: refs/heads/master@{#65432}
parent 83fd3e84
...@@ -112,9 +112,6 @@ ...@@ -112,9 +112,6 @@
# https://crbug.com/v8/8919 # https://crbug.com/v8/8919
'test-platform/StackAlignment': [PASS, ['not is_clang', SKIP]], 'test-platform/StackAlignment': [PASS, ['not is_clang', SKIP]],
# BUG(v8:10034).
'test-debug-helper/GetObjectProperties': [SKIP],
############################################################################ ############################################################################
# Slow tests. # Slow tests.
'test-debug/CallFunctionInDebugger': [PASS, ['mode == debug', SLOW]], 'test-debug/CallFunctionInDebugger': [PASS, ['mode == debug', SLOW]],
......
...@@ -67,10 +67,14 @@ void CheckProp(const d::ObjectProperty& property, const char* expected_type, ...@@ -67,10 +67,14 @@ void CheckProp(const d::ObjectProperty& property, const char* expected_type,
CHECK(*reinterpret_cast<TValue*>(property.address) == expected_value); CHECK(*reinterpret_cast<TValue*>(property.address) == expected_value);
} }
bool StartsWith(std::string full_string, std::string prefix) { bool StartsWith(const std::string& full_string, const std::string& prefix) {
return full_string.substr(0, prefix.size()) == prefix; return full_string.substr(0, prefix.size()) == prefix;
} }
bool Contains(const std::string& full_string, const std::string& substr) {
return full_string.find(substr) != std::string::npos;
}
void CheckStructProp(const d::StructProperty& property, void CheckStructProp(const d::StructProperty& property,
const char* expected_type, const char* expected_name, const char* expected_type, const char* expected_name,
size_t expected_offset) { size_t expected_offset) {
...@@ -155,7 +159,7 @@ TEST(GetObjectProperties) { ...@@ -155,7 +159,7 @@ TEST(GetObjectProperties) {
// deterministic locations within the heap reservation. // deterministic locations within the heap reservation.
CHECK(COMPRESS_POINTERS_BOOL CHECK(COMPRESS_POINTERS_BOOL
? StartsWith(props->brief, "EmptyFixedArray") ? StartsWith(props->brief, "EmptyFixedArray")
: StartsWith(props->brief, "maybe EmptyFixedArray")); : Contains(props->brief, "maybe EmptyFixedArray"));
// Provide a heap first page so the API can be more sure. // Provide a heap first page so the API can be more sure.
heap_addresses.read_only_space_first_page = heap_addresses.read_only_space_first_page =
...@@ -265,7 +269,7 @@ TEST(GetObjectProperties) { ...@@ -265,7 +269,7 @@ TEST(GetObjectProperties) {
alphabet.substr(3,20) + alphabet.toUpperCase().substr(5,15) + "7")"); alphabet.substr(3,20) + alphabet.toUpperCase().substr(5,15) + "7")");
o = v8::Utils::OpenHandle(*v); o = v8::Utils::OpenHandle(*v);
props = d::GetObjectProperties(o->ptr(), &ReadMemory, heap_addresses); props = d::GetObjectProperties(o->ptr(), &ReadMemory, heap_addresses);
CHECK(StartsWith(props->brief, "\"defghijklmnopqrstuvwFGHIJKLMNOPQRST7\"")); CHECK(Contains(props->brief, "\"defghijklmnopqrstuvwFGHIJKLMNOPQRST7\""));
// Cause a failure when reading the "second" pointer within the top-level // Cause a failure when reading the "second" pointer within the top-level
// ConsString. // ConsString.
...@@ -274,15 +278,14 @@ TEST(GetObjectProperties) { ...@@ -274,15 +278,14 @@ TEST(GetObjectProperties) {
uintptr_t second_address = props->properties[4]->address; uintptr_t second_address = props->properties[4]->address;
MemoryFailureRegion failure(second_address, second_address + 4); MemoryFailureRegion failure(second_address, second_address + 4);
props = d::GetObjectProperties(o->ptr(), &ReadMemory, heap_addresses); props = d::GetObjectProperties(o->ptr(), &ReadMemory, heap_addresses);
CHECK( CHECK(Contains(props->brief, "\"defghijklmnopqrstuvwFGHIJKLMNOPQRST...\""));
StartsWith(props->brief, "\"defghijklmnopqrstuvwFGHIJKLMNOPQRST...\""));
} }
// Build a very long string. // Build a very long string.
v = CompileRun("'a'.repeat(1000)"); v = CompileRun("'a'.repeat(1000)");
o = v8::Utils::OpenHandle(*v); o = v8::Utils::OpenHandle(*v);
props = d::GetObjectProperties(o->ptr(), &ReadMemory, heap_addresses); props = d::GetObjectProperties(o->ptr(), &ReadMemory, heap_addresses);
CHECK(std::string(props->brief).substr(79, 7) == std::string("aa...\" ")); CHECK(Contains(props->brief, "\"" + std::string(80, 'a') + "...\""));
// Build a basic JS object and get its properties. // Build a basic JS object and get its properties.
v = CompileRun("({a: 1, b: 2})"); v = CompileRun("({a: 1, b: 2})");
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment