[Lldb-commits] [lldb] r353191 - [Obj-C] Fix undefined behaviour(s) in the new NSTaggedDate formatter.

[Davide Italiano via lldb-commits]

To clarify, for those listening at home, it was the second UB
(overflow) that exposed a real bug.
Type punning was innocuous here, but given it's technically UB, I
fixed it anyway.

On Tue, Feb 5, 2019 at 9:33 AM Davide Italiano <dccitaliano at gmail.com> wrote:
>
> Shafik, Zachary, I think you reported this a bit ago, but now I got
> around to fix it (also because it was breaking something real).
>
> Thanks,
>
> --
> Davide
>
> On Tue, Feb 5, 2019 at 9:30 AM Davide Italiano via lldb-commits
> <lldb-commits at lists.llvm.org> wrote:
> >
> > Author: davide
> > Date: Tue Feb  5 09:30:53 2019
> > New Revision: 353191
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=353191&view=rev
> > Log:
> > [Obj-C] Fix undefined behaviour(s) in the new NSTaggedDate formatter.
> >
> > Type punning through a union -> no good.
> > double to uint64 to double again -> no good either.
> >
> > The nice side effect, other than silencing the sanitizer bot
> > is that it fixes the formatting of some dates, e.g. Jan 1st 1970.
> >
> > <rdar://problem/47617983>
> >
> > Modified:
> >     lldb/trunk/source/Plugins/Language/ObjC/Cocoa.cpp
> >
> > Modified: lldb/trunk/source/Plugins/Language/ObjC/Cocoa.cpp
> > URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Language/ObjC/Cocoa.cpp?rev=353191&r1=353190&r2=353191&view=diff
> > ==============================================================================
> > --- lldb/trunk/source/Plugins/Language/ObjC/Cocoa.cpp (original)
> > +++ lldb/trunk/source/Plugins/Language/ObjC/Cocoa.cpp Tue Feb  5 09:30:53 2019
> > @@ -27,6 +27,7 @@
> >  #include "lldb/Utility/Stream.h"
> >
> >  #include "llvm/ADT/APInt.h"
> > +#include "llvm/ADT/bit.h"
> >
> >  #include "Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.h"
> >
> > @@ -749,24 +750,18 @@ bool lldb_private::formatters::NSURLSumm
> >  ///   distantFuture, except within about 1e-25 second of the reference date.
> >  const int TAGGED_DATE_EXPONENT_BIAS = 0x3ef;
> >
> > -typedef union {
> > -  struct {
> > -    uint64_t fraction:52;  // unsigned
> > -    uint64_t exponent:11;  // signed
> > -    uint64_t sign:1;
> > -  } repr;
> > -  uint64_t i;
> > -  double d;
> > -} DoubleBits;
> > -typedef union {
> > -  struct {
> > -    uint64_t fraction:52;  // unsigned
> > -    uint64_t exponent:7;   // signed
> > -    uint64_t sign:1;
> > -    uint64_t unused:4;  // placeholder for pointer tag bits
> > -  } repr;
> > -  uint64_t i;
> > -} TaggedDoubleBits;
> > +struct DoubleBits {
> > +  uint64_t fraction : 52; // unsigned
> > +  uint64_t exponent : 11; // signed
> > +  uint64_t sign : 1;
> > +};
> > +
> > +struct TaggedDoubleBits {
> > +  uint64_t fraction : 52; // unsigned
> > +  uint64_t exponent : 7;  // signed
> > +  uint64_t sign : 1;
> > +  uint64_t unused : 4; // placeholder for pointer tag bits
> > +};
> >
> >  static uint64_t decodeExponent(uint64_t exp) {
> >    // Tagged exponent field is 7-bit signed. Sign-extend the value to 64 bits
> > @@ -774,24 +769,24 @@ static uint64_t decodeExponent(uint64_t
> >    return llvm::SignExtend64<7>(exp) + TAGGED_DATE_EXPONENT_BIAS;
> >  }
> >
> > -static uint64_t decodeTaggedTimeInterval(uint64_t encodedTimeInterval) {
> > +static double decodeTaggedTimeInterval(uint64_t encodedTimeInterval) {
> >    if (encodedTimeInterval == 0)
> >      return 0.0;
> >    if (encodedTimeInterval == std::numeric_limits<uint64_t>::max())
> >      return (uint64_t)-0.0;
> >
> > -  TaggedDoubleBits encodedBits = {};
> > -  encodedBits.i = encodedTimeInterval;
> > -  DoubleBits decodedBits;
> > +  TaggedDoubleBits encodedBits =
> > +      llvm::bit_cast<TaggedDoubleBits>(encodedTimeInterval);
> > +  assert(encodedBits.unused == 0);
> >
> >    // Sign and fraction are represented exactly.
> >    // Exponent is encoded.
> > -  assert(encodedBits.repr.unused == 0);
> > -  decodedBits.repr.sign = encodedBits.repr.sign;
> > -  decodedBits.repr.fraction = encodedBits.repr.fraction;
> > -  decodedBits.repr.exponent = decodeExponent(encodedBits.repr.exponent);
> > +  DoubleBits decodedBits;
> > +  decodedBits.sign = encodedBits.sign;
> > +  decodedBits.fraction = encodedBits.fraction;
> > +  decodedBits.exponent = decodeExponent(encodedBits.exponent);
> >
> > -  return decodedBits.d;
> > +  return llvm::bit_cast<double>(decodedBits);
> >  }
> >
> >  bool lldb_private::formatters::NSDateSummaryProvider(
> > @@ -868,7 +863,8 @@ bool lldb_private::formatters::NSDateSum
> >
> >    // Accomodate for the __NSTaggedDate format introduced in Foundation 1600.
> >    if (class_name == g___NSTaggedDate) {
> > -    auto *runtime = llvm::dyn_cast_or_null<AppleObjCRuntime>(process_sp->GetObjCLanguageRuntime());
> > +    auto *runtime = llvm::dyn_cast_or_null<AppleObjCRuntime>(
> > +        process_sp->GetObjCLanguageRuntime());
> >      if (runtime && runtime->GetFoundationVersion() >= 1600)
> >        date_value = decodeTaggedTimeInterval(value_bits << 4);
> >    }
> >
> >
> > _______________________________________________
> > lldb-commits mailing list
> > lldb-commits at lists.llvm.org
> > https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits