[Lldb-commits] [lldb] 8214eff - Revert "[lldb/DataFormatter] Check for overflow when finding NSDate epoch"

Dmitri Gribenko via lldb-commits lldb-commits at lists.llvm.org
Wed May 20 03:46:57 PDT 2020


Author: Dmitri Gribenko
Date: 2020-05-20T12:44:19+02:00
New Revision: 8214eff467f583309e9fbb971862d3c1cdcc65e4

URL: https://github.com/llvm/llvm-project/commit/8214eff467f583309e9fbb971862d3c1cdcc65e4
DIFF: https://github.com/llvm/llvm-project/commit/8214eff467f583309e9fbb971862d3c1cdcc65e4.diff

LOG: 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.

Added: 
    

Modified: 
    lldb/source/Plugins/Language/ObjC/Cocoa.cpp
    lldb/unittests/DataFormatter/CMakeLists.txt

Removed: 
    lldb/include/lldb/DataFormatters/Mock.h
    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<double>(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<time_t>::max() ||
-      date_value < std::numeric_limits<time_t>::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<AppleObjCRuntime>(
-            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<AppleObjCRuntime>(
+        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 <string>
-
-using namespace lldb_private;
-
-// Try to format the date_value field of a NSDate.
-static llvm::Optional<std::string> 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<time_t>::max() + 1),
-            llvm::None);
-  EXPECT_EQ(formatDateValue(std::numeric_limits<time_t>::min() - 1),
-            llvm::None);
-
-  // Can't add the macOS epoch to the converted date_value (the add overflows).
-  EXPECT_EQ(formatDateValue(std::numeric_limits<time_t>::max()), llvm::None);
-  EXPECT_EQ(formatDateValue(std::numeric_limits<time_t>::min()), llvm::None);
-
-  EXPECT_EQ(*formatDateValue(0), "2001-01-01 00:00:00 UTC");
-}


        


More information about the lldb-commits mailing list