Commit f90d7226 authored by Joe Downing's avatar Joe Downing Committed by Commit Bot

Revert "Add start & end markers around |str_stack| for use by Crash service."

This reverts commit 64646172.

Reason for revert: LoggingTest.LogMessageMarkersOnStack is failing on several builders and this test was introduced in this CL:
https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Win7%20Tests%20%28dbg%29%281%29/72386
https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Linux%20MSan%20Tests/12371

Example error message:
https://logs.chromium.org/logs/chromium/buildbucket/cr-buildbucket.appspot.com/8932209218566904752/+/steps/base_unittests/0/logs/LoggingTest.LogMessageMarkersOnStack/0


Original change's description:
> Add start & end markers around |str_stack| for use by Crash service.
> 
> Crash can locate particular entries in a stack by scanning for start &
> end markers with unusual values.  We add start & end markers around the
> |str_stack| copy of the LogMessage string, created when we're about to
> crash the process due to a FATAL level log message.
> 
> Bug: 802393
> Change-Id: I8fd462cc515ddbefb8c7b34363eea848171aa776
> Reviewed-on: https://chromium-review.googlesource.com/c/1201344
> Reviewed-by: Wez <wez@chromium.org>
> Reviewed-by: Albert J. Wong <ajwong@chromium.org>
> Reviewed-by: Daniel Cheng <dcheng@chromium.org>
> Commit-Queue: Wez <wez@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#601236}

TBR=ajwong@chromium.org,dcheng@chromium.org,wez@chromium.org

Change-Id: If00343894caa467887191fbb09a4c80233cc94b1
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 802393
Reviewed-on: https://chromium-review.googlesource.com/c/1292595Reviewed-by: default avatarJoe Downing <joedow@chromium.org>
Commit-Queue: Joe Downing <joedow@chromium.org>
Cr-Commit-Position: refs/heads/master@{#601295}
parent c67f8bc9
...@@ -8,7 +8,6 @@ ...@@ -8,7 +8,6 @@
#include <stdint.h> #include <stdint.h>
#include "base/macros.h" #include "base/macros.h"
#include "base/stl_util.h"
#include "build/build_config.h" #include "build/build_config.h"
#if defined(OS_WIN) #if defined(OS_WIN)
...@@ -832,17 +831,8 @@ LogMessage::~LogMessage() { ...@@ -832,17 +831,8 @@ LogMessage::~LogMessage() {
tracker->RecordLogMessage(str_newline); tracker->RecordLogMessage(str_newline);
// Ensure the first characters of the string are on the stack so they // Ensure the first characters of the string are on the stack so they
// are contained in minidumps for diagnostic purposes. We place start // are contained in minidumps for diagnostic purposes.
// and end marker values at either end, so we can scan captured stacks DEBUG_ALIAS_FOR_CSTR(str_stack, str_newline.c_str(), 1024);
// for the data easily.
struct {
uint32_t start_marker = 0xbedead01;
char data[1024];
uint32_t end_marker = 0x5050dead;
} str_stack;
base::strlcpy(str_stack.data, str_newline.data(),
base::size(str_stack.data));
base::debug::Alias(&str_stack);
if (log_assert_handler_stack.IsCreated() && if (log_assert_handler_stack.IsCreated() &&
!log_assert_handler_stack.Get().empty()) { !log_assert_handler_stack.Get().empty()) {
......
...@@ -829,56 +829,6 @@ TEST_F(LoggingTest, LogPrefix) { ...@@ -829,56 +829,6 @@ TEST_F(LoggingTest, LogPrefix) {
log_string_ptr = nullptr; log_string_ptr = nullptr;
} }
#if !defined(ADDRESS_SANITIZER)
// Since we scan potentially uninitialized portions of the stack, we can't run
// this test under address-sanitizer.
TEST_F(LoggingTest, LogMessageMarkersOnStack) {
const uint32_t kLogStartMarker = 0xbedead01;
const uint32_t kLogEndMarker = 0x5050dead;
const char kTestMessage[] = "Oh noes! I have crashed! 💩";
uint32_t stack_start = 0;
// Install a LogAssertHandler which will scan between |stack_start| and its
// local-scope stack for the start & end markers, and verify the message.
ScopedLogAssertHandler assert_handler(base::BindRepeating(
[](uint32_t* stack_start_ptr, const char* file, int line,
const base::StringPiece message, const base::StringPiece stack_trace) {
uint32_t stack_end;
uint32_t* stack_end_ptr = &stack_end;
// Scan the stack for the expected markers.
uint32_t* start_marker = nullptr;
uint32_t* end_marker = nullptr;
for (uint32_t* ptr = stack_end_ptr; ptr <= stack_start_ptr; ++ptr) {
if (*ptr == kLogStartMarker)
start_marker = ptr;
else if (*ptr == kLogEndMarker)
end_marker = ptr;
}
// Verify that start & end markers were found, somewhere, in-between
// this and the LogAssertHandler scope, in the LogMessage destructor's
// stack frame.
ASSERT_TRUE(start_marker);
ASSERT_TRUE(end_marker);
// Verify that the |message| is found in-between the markers.
const char* start_char_marker =
reinterpret_cast<char*>(start_marker + 1);
const char* end_char_marker = reinterpret_cast<char*>(end_marker);
const base::StringPiece stack_view(start_char_marker,
end_char_marker - start_char_marker);
ASSERT_FALSE(stack_view.find(message) == base::StringPiece::npos);
},
&stack_start));
// Trigger a log assertion, with a test message we can check for.
LOG(FATAL) << kTestMessage;
}
#endif // !defined(ADDRESS_SANITIZER)
} // namespace } // namespace
} // namespace logging } // namespace logging
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