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

[Raphael Isemann via Phabricator via lldb-commits]

teemperor added a comment.

In D93421#2459853 <https://reviews.llvm.org/D93421#2459853>, @labath wrote:

> 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`

DumpDataExtractor doesn't know what types its printing, it just gets a `lldb_private::Format` (where we sometimes encode whether a type is signed). That is on my TODO list, but I just didn't get around to refactor this yet.

In short: If you get a `eFormatChar` (which is the default for 'char' stuff) then we always pretend its unsigned when printing the character. If you do 'hex', all ints become unsigned. If you print 'decimal', all types because signed, so there is an 'unsigned decimal' format that works around this:

  (lldb) frame var --format default f
  (F) f = (sc1 = '\x01', uc1 = '\x01', sc2 = '\x01', uc2 = '\x01')
  (lldb) frame var --format 'decimal' f
  (F) f = (sc1 = -1, uc1 = -1, sc2 = 1, uc2 = 1)
  (lldb) frame var --format 'unsigned decimal' f
  (F) f = (sc1 = 1, uc1 = 1, sc2 = 1, uc2 = 1)
  (lldb) frame var --format 'hex' f
  (F) f = (sc1 = 0x01, uc1 = 0x01, sc2 = 0x01, uc2 = 0x01)


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

https://reviews.llvm.org/D93421