From 8214eff467f583309e9fbb971862d3c1cdcc65e4 Mon Sep 17 00:00:00 2001 From: Dmitri Gribenko Date: Wed, 20 May 2020 11:59:47 +0200 Subject: [PATCH] Revert "[lldb/DataFormatter] Check for overflow when finding NSDate epoch" This reverts commit b783f70a42575a5d9147bea1ac97e872370fe55b. This change had multiple issues which required post-commit fixups, and not all issues are fixed yet. In particular, the LLDB build bot for ARM is still broken. There is also an ongoing conversation in the original phabricator review about whether there is undefined behavior in the code. --- lldb/include/lldb/DataFormatters/Mock.h | 26 -------- lldb/source/Plugins/Language/ObjC/Cocoa.cpp | 69 +++++++++------------ lldb/unittests/DataFormatter/CMakeLists.txt | 1 - lldb/unittests/DataFormatter/MockTests.cpp | 40 ------------ 4 files changed, 28 insertions(+), 108 deletions(-) delete mode 100644 lldb/include/lldb/DataFormatters/Mock.h delete mode 100644 lldb/unittests/DataFormatter/MockTests.cpp diff --git a/lldb/include/lldb/DataFormatters/Mock.h b/lldb/include/lldb/DataFormatters/Mock.h deleted file mode 100644 index b3fc10cd2e51..000000000000 --- a/lldb/include/lldb/DataFormatters/Mock.h +++ /dev/null @@ -1,26 +0,0 @@ -//===-- Mock.h --------------------------------------------------*- C++ -*-===// -// -// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. -// See https://llvm.org/LICENSE.txt for license information. -// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception -// -//===----------------------------------------------------------------------===// - -#ifndef LLDB_DATAFORMATTERS_MOCK_H -#define LLDB_DATAFORMATTERS_MOCK_H - -namespace lldb_private { - -class Stream; - -namespace formatters { -namespace NSDate { - -/// Format the date_value field of a NSDate. -bool FormatDateValue(double date_value, Stream &stream); - -} // namespace NSDate -} // namespace formatters -} // namespace lldb_private - -#endif // LLDB_DATAFORMATTERS_MOCK_H diff --git a/lldb/source/Plugins/Language/ObjC/Cocoa.cpp b/lldb/source/Plugins/Language/ObjC/Cocoa.cpp index 1ad443b8b74e..8a44811dd36b 100644 --- a/lldb/source/Plugins/Language/ObjC/Cocoa.cpp +++ b/lldb/source/Plugins/Language/ObjC/Cocoa.cpp @@ -13,7 +13,6 @@ #include "lldb/Core/ValueObject.h" #include "lldb/Core/ValueObjectConstResult.h" #include "lldb/DataFormatters/FormattersHelpers.h" -#include "lldb/DataFormatters/Mock.h" #include "lldb/DataFormatters/StringPrinter.h" #include "lldb/DataFormatters/TypeSummary.h" #include "lldb/Host/Time.h" @@ -28,7 +27,6 @@ #include "llvm/ADT/APInt.h" #include "llvm/ADT/bit.h" -#include "llvm/Support/CheckedArithmetic.h" #include "Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.h" @@ -787,34 +785,6 @@ static double decodeTaggedTimeInterval(uint64_t encodedTimeInterval) { return llvm::bit_cast(decodedBits); } -bool lldb_private::formatters::NSDate::FormatDateValue(double date_value, - Stream &stream) { - if (date_value == -63114076800) { - stream.Printf("0001-12-30 00:00:00 +0000"); - return true; - } - - if (date_value > std::numeric_limits::max() || - date_value < std::numeric_limits::min()) - return false; - - time_t epoch = GetOSXEpoch(); - if (auto osx_epoch = llvm::checkedAdd(epoch, (time_t)date_value)) - epoch = *osx_epoch; - else - return false; - tm *tm_date = gmtime(&epoch); - if (!tm_date) - return false; - std::string buffer(1024, 0); - if (strftime(&buffer[0], 1023, "%Z", tm_date) == 0) - return false; - stream.Printf("%04d-%02d-%02d %02d:%02d:%02d %s", tm_date->tm_year + 1900, - tm_date->tm_mon + 1, tm_date->tm_mday, tm_date->tm_hour, - tm_date->tm_min, tm_date->tm_sec, buffer.c_str()); - return true; -} - bool lldb_private::formatters::NSDateSummaryProvider( ValueObject &valobj, Stream &stream, const TypeSummaryOptions &options) { ProcessSP process_sp = valobj.GetProcessSP(); @@ -858,16 +828,6 @@ bool lldb_private::formatters::NSDateSummaryProvider( if (descriptor->GetTaggedPointerInfo(&info_bits, &value_bits)) { date_value_bits = ((value_bits << 8) | (info_bits << 4)); memcpy(&date_value, &date_value_bits, sizeof(date_value_bits)); - - // Accomodate the __NSTaggedDate format introduced in Foundation 1600. - if (class_name == g___NSTaggedDate) { - auto *apple_runtime = llvm::dyn_cast_or_null( - ObjCLanguageRuntime::Get(*process_sp)); - if (!apple_runtime) - return false; - if (apple_runtime->GetFoundationVersion() >= 1600) - date_value = decodeTaggedTimeInterval(value_bits << 4); - } } else { llvm::Triple triple( process_sp->GetTarget().GetArchitecture().GetTriple()); @@ -890,7 +850,34 @@ bool lldb_private::formatters::NSDateSummaryProvider( } else return false; - return NSDate::FormatDateValue(date_value, stream); + if (date_value == -63114076800) { + stream.Printf("0001-12-30 00:00:00 +0000"); + return true; + } + + // Accomodate for the __NSTaggedDate format introduced in Foundation 1600. + if (class_name == g___NSTaggedDate) { + auto *runtime = llvm::dyn_cast_or_null( + ObjCLanguageRuntime::Get(*process_sp)); + if (runtime && runtime->GetFoundationVersion() >= 1600) + date_value = decodeTaggedTimeInterval(value_bits << 4); + } + + // this snippet of code assumes that time_t == seconds since Jan-1-1970 this + // is generally true and POSIXly happy, but might break if a library vendor + // decides to get creative + time_t epoch = GetOSXEpoch(); + epoch = epoch + (time_t)date_value; + tm *tm_date = gmtime(&epoch); + if (!tm_date) + return false; + std::string buffer(1024, 0); + if (strftime(&buffer[0], 1023, "%Z", tm_date) == 0) + return false; + stream.Printf("%04d-%02d-%02d %02d:%02d:%02d %s", tm_date->tm_year + 1900, + tm_date->tm_mon + 1, tm_date->tm_mday, tm_date->tm_hour, + tm_date->tm_min, tm_date->tm_sec, buffer.c_str()); + return true; } bool lldb_private::formatters::ObjCClassSummaryProvider( diff --git a/lldb/unittests/DataFormatter/CMakeLists.txt b/lldb/unittests/DataFormatter/CMakeLists.txt index 716c8e735287..45011c56b0b0 100644 --- a/lldb/unittests/DataFormatter/CMakeLists.txt +++ b/lldb/unittests/DataFormatter/CMakeLists.txt @@ -1,6 +1,5 @@ add_lldb_unittest(LLDBFormatterTests FormatManagerTests.cpp - MockTests.cpp StringPrinterTests.cpp LINK_LIBS diff --git a/lldb/unittests/DataFormatter/MockTests.cpp b/lldb/unittests/DataFormatter/MockTests.cpp deleted file mode 100644 index 0042888243f7..000000000000 --- a/lldb/unittests/DataFormatter/MockTests.cpp +++ /dev/null @@ -1,40 +0,0 @@ -//===-- MockTests.cpp -----------------------------------------------------===// -// -// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. -// See https://llvm.org/LICENSE.txt for license information. -// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception -// -//===----------------------------------------------------------------------===// - -#include "lldb/DataFormatters/Mock.h" -#include "lldb/Utility/StreamString.h" -#include "llvm/ADT/Optional.h" -#include "gtest/gtest.h" -#include - -using namespace lldb_private; - -// Try to format the date_value field of a NSDate. -static llvm::Optional formatDateValue(double date_value) { - StreamString s; - bool succcess = formatters::NSDate::FormatDateValue(date_value, s); - if (succcess) - return std::string(s.GetData()); - return llvm::None; -} - -TEST(DataFormatterMockTest, NSDate) { - EXPECT_EQ(*formatDateValue(-63114076800), "0001-12-30 00:00:00 +0000"); - - // Can't convert the date_value to a time_t. - EXPECT_EQ(formatDateValue(std::numeric_limits::max() + 1), - llvm::None); - EXPECT_EQ(formatDateValue(std::numeric_limits::min() - 1), - llvm::None); - - // Can't add the macOS epoch to the converted date_value (the add overflows). - EXPECT_EQ(formatDateValue(std::numeric_limits::max()), llvm::None); - EXPECT_EQ(formatDateValue(std::numeric_limits::min()), llvm::None); - - EXPECT_EQ(*formatDateValue(0), "2001-01-01 00:00:00 UTC"); -}