This is an archive of the discontinued LLVM Phabricator instance.

Add support for getting signed ObjC tagged pointer values
ClosedPublic

Authored by jingham on Mar 31 2021, 5:19 PM.

Details

Summary

The ObjC runtime offers both signed & unsigned tagged pointer value
accessors to tagged pointer providers, but lldb's tagged pointer
code only implemented the unsigned one. This patch adds an
emulation of the signed one.

The motivation for doing this is that NSNumbers use the signed
accessor (they are always signed) and we need to follow that in our
summary provider or we will get incorrect values for negative
NSNumbers.

The data-formatter-objc test file had NSNumber examples (along with lots of other
goodies) but the NSNumber values weren't tested. So I also added
checks for those values to the test.

I also did a quick audit of the other types in that main.m file, and
it looks like pretty much all the other values are either intermediates
or are tested.

Diff Detail

Event Timeline

jingham requested review of this revision.Mar 31 2021, 5:19 PM
jingham created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptMar 31 2021, 5:19 PM

LGTM with a little nit. Please clang-format the patch before landing.

lldb/source/Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.h
89–100

Please make this a Doxygen comment and include both methods in a group.

JDevlieghere accepted this revision.Mar 31 2021, 9:46 PM
This revision is now accepted and ready to land.Mar 31 2021, 9:46 PM
This revision was landed with ongoing or failed builds.Apr 1 2021, 10:59 AM
This revision was automatically updated to reflect the committed changes.
shafik added a subscriber: shafik.Apr 1 2021, 11:19 AM
shafik added inline comments.
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
2499

note we can use make_shared.

2586–2587

We can use make_shared here.

The latest change (commit be0ced03) issues the following warning:

/usr/bin/clang++ -DGTEST_HAS_RTTI=0 -DHAVE_ROUND -D_DEBUG -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -Itools/lldb/source/Plugins/Language/ObjC -I/mnt/vss/_work/2/s/lldb/source/Plugins/Language/ObjC -Itools/lldb/source -I/mnt/vss/_work/2/s/lldb/include -Itools/lldb/include -Iinclude -I/mnt/vss/_work/2/s/llvm/include -I/usr/include/python3.8 -I/mnt/vss/_work/2/s/llvm/../clang/include -Itools/lldb/../clang/include -I/usr/include/libxml2 -I/mnt/vss/_work/2/s/lldb/source/. -fPIC -fvisibility-inlines-hidden -Werror -Werror=date-time -Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic -Wno-long-long -Wc++98-compat-extra-semi -Wimplicit-fallthrough -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wstring-conversion -fdiagnostics-color -ffunction-sections -fdata-sections -Wno-deprecated-declarations -Wno-unknown-pragmas -Wno-strict-aliasing -Wno-deprecated-register -Wno-vla-extension -O3   -fno-exceptions -fno-rtti -UNDEBUG -std=c++14 -MD -MT tools/lldb/source/Plugins/Language/ObjC/CMakeFiles/lldbPluginObjCLanguage.dir/Cocoa.cpp.o -MF tools/lldb/source/Plugins/Language/ObjC/CMakeFiles/lldbPluginObjCLanguage.dir/Cocoa.cpp.o.d -o tools/lldb/source/Plugins/Language/ObjC/CMakeFiles/lldbPluginObjCLanguage.dir/Cocoa.cpp.o -c /mnt/vss/_work/2/s/lldb/source/Plugins/Language/ObjC/Cocoa.cpp
/mnt/vss/_work/2/s/lldb/source/Plugins/Language/ObjC/Cocoa.cpp:528:11: error: misleading indentation; statement is not part of the previous 'if' [-Werror,-Wmisleading-indentation]
          return false;
          ^
/mnt/vss/_work/2/s/lldb/source/Plugins/Language/ObjC/Cocoa.cpp:525:9: note: previous statement is here
        if (log)

For this badly indented code:

if (is_preserved_number) {
if (log) 
  log->Printf("Unsupported preserved NSNumber tagged pointer 0x%" 
      PRIu64, valobj_addr);
  return false;
}

It looks like @teemperor fixed it already!

The latest change (commit be0ced03) issues the following warning:

/usr/bin/clang++ -DGTEST_HAS_RTTI=0 -DHAVE_ROUND -D_DEBUG -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -Itools/lldb/source/Plugins/Language/ObjC -I/mnt/vss/_work/2/s/lldb/source/Plugins/Language/ObjC -Itools/lldb/source -I/mnt/vss/_work/2/s/lldb/include -Itools/lldb/include -Iinclude -I/mnt/vss/_work/2/s/llvm/include -I/usr/include/python3.8 -I/mnt/vss/_work/2/s/llvm/../clang/include -Itools/lldb/../clang/include -I/usr/include/libxml2 -I/mnt/vss/_work/2/s/lldb/source/. -fPIC -fvisibility-inlines-hidden -Werror -Werror=date-time -Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic -Wno-long-long -Wc++98-compat-extra-semi -Wimplicit-fallthrough -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wstring-conversion -fdiagnostics-color -ffunction-sections -fdata-sections -Wno-deprecated-declarations -Wno-unknown-pragmas -Wno-strict-aliasing -Wno-deprecated-register -Wno-vla-extension -O3   -fno-exceptions -fno-rtti -UNDEBUG -std=c++14 -MD -MT tools/lldb/source/Plugins/Language/ObjC/CMakeFiles/lldbPluginObjCLanguage.dir/Cocoa.cpp.o -MF tools/lldb/source/Plugins/Language/ObjC/CMakeFiles/lldbPluginObjCLanguage.dir/Cocoa.cpp.o.d -o tools/lldb/source/Plugins/Language/ObjC/CMakeFiles/lldbPluginObjCLanguage.dir/Cocoa.cpp.o -c /mnt/vss/_work/2/s/lldb/source/Plugins/Language/ObjC/Cocoa.cpp
/mnt/vss/_work/2/s/lldb/source/Plugins/Language/ObjC/Cocoa.cpp:528:11: error: misleading indentation; statement is not part of the previous 'if' [-Werror,-Wmisleading-indentation]
          return false;
          ^
/mnt/vss/_work/2/s/lldb/source/Plugins/Language/ObjC/Cocoa.cpp:525:9: note: previous statement is here
        if (log)

For this badly indented code:

if (is_preserved_number) {
if (log) 
  log->Printf("Unsupported preserved NSNumber tagged pointer 0x%" 
      PRIu64, valobj_addr);
  return false;
}