[Lldb-commits] [lldb] 4994890 - [lldb] Add a test for potentially conflicting names for the Objective-C class update utility expression

[Raphael Isemann via lldb-commits]

Author: Raphael Isemann
Date: 2021-08-10T14:42:06+02:00
New Revision: 499489064b7a4519f7c566e76077821ae1f3446d

URL: https://github.com/llvm/llvm-project/commit/499489064b7a4519f7c566e76077821ae1f3446d
DIFF: https://github.com/llvm/llvm-project/commit/499489064b7a4519f7c566e76077821ae1f3446d.diff

LOG: [lldb] Add a test for potentially conflicting names for the Objective-C class update utility expression

We recently had an issue where a user declared a `Class::free` function which
then got picked up by accident by the expression evaluator when calling
`::free`. This was due to a too lax filter in the DWARFIndex (which was fixed by
https://reviews.llvm.org/D73191 ). This broke the Objective-C utility expression
that is trying to update the Objective-C class list (which is calling `:;free`).

This adds a regression test for situations where we have a bunch of functions
defined that share the name of the global functions that this utility function
calls. None of them are actually conflicting with the global functions we are
trying to call (they are all in namespaces, objects or classes).

Reviewed By: JDevlieghere

Differential Revision: https://reviews.llvm.org/D107776

Added: 
    lldb/test/API/lang/objcxx/conflicting-names-class-update-utility-expr/Makefile
    lldb/test/API/lang/objcxx/conflicting-names-class-update-utility-expr/TestObjCConflictingNamesForClassUpdateExpr.py
    lldb/test/API/lang/objcxx/conflicting-names-class-update-utility-expr/main.mm

Modified: 
    

Removed: 
    


################################################################################
diff  --git a/lldb/test/API/lang/objcxx/conflicting-names-class-update-utility-expr/Makefile b/lldb/test/API/lang/objcxx/conflicting-names-class-update-utility-expr/Makefile
new file mode 100644
index 0000000000000..3af75c304c35d
--- /dev/null
+++ b/lldb/test/API/lang/objcxx/conflicting-names-class-update-utility-expr/Makefile
@@ -0,0 +1,4 @@
+OBJCXX_SOURCES := main.mm
+LD_EXTRAS := -lobjc -framework Foundation
+
+include Makefile.rules

diff  --git a/lldb/test/API/lang/objcxx/conflicting-names-class-update-utility-expr/TestObjCConflictingNamesForClassUpdateExpr.py b/lldb/test/API/lang/objcxx/conflicting-names-class-update-utility-expr/TestObjCConflictingNamesForClassUpdateExpr.py
new file mode 100644
index 0000000000000..cac65089a28f1
--- /dev/null
+++ b/lldb/test/API/lang/objcxx/conflicting-names-class-update-utility-expr/TestObjCConflictingNamesForClassUpdateExpr.py
@@ -0,0 +1,42 @@
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class TestCase(TestBase):
+
+    mydir = TestBase.compute_mydir(__file__)
+
+    def test(self):
+        """
+        Tests that running the utility expression that retrieves the Objective-C
+        class list works even when user-code contains functions with apparently
+        conflicting identifiers (e.g. 'free') but that are not in the global
+        scope.
+
+        This is *not* supposed to test what happens when there are actual
+        conflicts such as when a user somehow defined their own '::free'
+        function.
+        """
+
+        self.build()
+        lldbutil.run_to_source_breakpoint(self, "// break here", lldb.SBFileSpec("main.mm"))
+
+        # First check our side effect variable is in its initial state.
+        self.expect_expr("called_function", result_summary='"none"')
+
+        # Get the (dynamic) type of our 'id' variable so that our Objective-C
+        # runtime information is updated.
+        str_val = self.expect_expr("str")
+        dyn_val = str_val.GetDynamicValue(lldb.eDynamicCanRunTarget)
+        dyn_type = dyn_val.GetTypeName()
+
+        # Check our side effect variable which should still be in its initial
+        # state if none of our trap functions were called.
+        # If this is failing, then LLDB called one of the trap functions.
+        self.expect_expr("called_function", result_summary='"none"')
+
+        # Double check that our dynamic type is correct. This is done last
+        # as the assert message from above is the more descriptive one (it
+        # contains the unintentionally called function).
+        self.assertEqual(dyn_type, "__NSCFConstantString *")

diff  --git a/lldb/test/API/lang/objcxx/conflicting-names-class-update-utility-expr/main.mm b/lldb/test/API/lang/objcxx/conflicting-names-class-update-utility-expr/main.mm
new file mode 100644
index 0000000000000..0ec825038c228
--- /dev/null
+++ b/lldb/test/API/lang/objcxx/conflicting-names-class-update-utility-expr/main.mm
@@ -0,0 +1,59 @@
+#import <Foundation/Foundation.h>
+
+// Observable side effect that is changed when one of our trap functions is
+// called. This should always retain its initial value in a successful test run.
+const char *called_function = "none";
+
+// Below several trap functions are declared in 
diff erent scopes that should
+// never be called even though they share the name of some of the utility
+// functions that LLDB has to call when updating the Objective-C class list
+// (i.e. 'free' and 'objc_copyRealizedClassList_nolock').
+// All functions just indicate that they got called by setting 'called_function'
+// to their own name.
+
+namespace N {
+void free(void *) { called_function = "N::free"; }
+void objc_copyRealizedClassList_nolock(unsigned int *) {
+  called_function = "N::objc_copyRealizedClassList_nolock";
+}
+}
+
+struct Context {
+  void free(void *) { called_function = "Context::free"; }
+  void objc_copyRealizedClassList_nolock(unsigned int *) {
+    called_function = "Context::objc_copyRealizedClassList_nolock";
+  }
+};
+
+ at interface ObjCContext : NSObject {
+}
+- (void)free:(void *)p;
+- (void)objc_copyRealizedClassList_nolock:(unsigned int *)outCount;
+ at end
+
+ at implementation ObjCContext
+- (void)free:(void *)p {
+  called_function = "ObjCContext::free";
+}
+
+- (void)objc_copyRealizedClassList_nolock:(unsigned int *)outCount {
+  called_function = "ObjCContext::objc_copyRealizedClassList_nolock";
+}
+ at end
+
+int main(int argc, char **argv) {
+  id str = @"str";
+  // Make sure all our conflicting functions/methods are emitted. The condition
+  // is never executed in the test as the process is launched without args.
+  if (argc == 1234) {
+    Context o;
+    o.free(nullptr);
+    o.objc_copyRealizedClassList_nolock(nullptr);
+    N::free(nullptr);
+    N::objc_copyRealizedClassList_nolock(nullptr);
+    ObjCContext *obj = [[ObjCContext alloc] init];
+    [obj free:nullptr];
+    [obj objc_copyRealizedClassList_nolock:nullptr];
+  }
+  return 0; // break here
+}