[Lldb-commits] [PATCH] D93421: Fix how ValueObject deals with getting unsigned values

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

This seems suspicious to me, on a couple of levels. You claim that BOOL is an unsigned type, but the apple documentation <https://developer.apple.com/documentation/objectivec/bool> says "BOOL is explicitly signed so @encode(BOOL) is c rather than C even if -funsigned-char is used." Which one is true?  Could this have something to do with the fact that the documentation assumes obj-c (which does not have a native `bool` type) but in lldb, we use obj-c++ (which, I guess, inherits `bool` from c++) everywhere?

If BOOL is unsigned, then it's not clear to me why would Scalar::ExtractBitfield be sign-extending anything. OTOH, if it is _signed_, then sign-extension seems to be the right behavior.

I think we may have some deeper problems with the handling bitfields whose underlying types are 1 byte long. Lldb (I think, correctly) sign-extends all signed bitfields, except the signed char one:

  (lldb) p a
  (A) $0 = {
    bool_ = true
    signed_char = '\x01'
    signed_short = -1
    signed_int = -1
    unsigned_char = '\x01'
    unsigned_short = 1
    unsigned_int = 1
  }
  (lldb) p/x a
  (A) $1 = {
    bool_ = 0x01
    signed_char = 0x01
    signed_short = 0x0001
    signed_int = 0x00000001
    unsigned_char = 0x01
    unsigned_short = 0x0001
    unsigned_int = 0x00000001
  }

This patch does not have any impact on this behavior, but the fact that signed char comes out as '\x01' seems suspicious. This is what gdb does for the same input:

  (gdb) p a
  $1 = {bool_ = true, signed_char = -1 '\377', signed_short = -1, signed_int = -1, unsigned_char = 1 '\001', unsigned_short = 1, unsigned_int = 1}
  (gdb) p/x a
  $2 = {bool_ = 0x1, signed_char = 0xff, signed_short = 0xffff, signed_int = 0xffffffff, unsigned_char = 0x1, unsigned_short = 0x1, unsigned_int = 0x1}

Finally, even though I'm not sure this is the right solution, I am sure that `GetValueAsSigned` should get the same fix as `GetValueAsUnsigned`


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D93421/new/

https://reviews.llvm.org/D93421