[PATCH] D59523: Thread Safety: also look at ObjC methods

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert added a comment.

> It should consider both because the attributes can be used on Objective-C as well.

Well, it's not really supported that well though. There are known bugs like https://bugs.llvm.org/show_bug.cgi?id=38892, and I don't really have the time to fix that. (You're free to jump in of course.)

> It seems weird how both do pretty similar things and e.g. duplicate getParamDecl.

I have no idea about ObjC at all, but I believe that ObjC methods are substantially different from ordinary functions. (Unlike C++ methods, which just have an additional implicit parameter.)

You could do something like `SExprBuilder::enterCFG`:

  auto Parms = isa<ObjCMethodDecl>(D) ? cast<ObjCMethodDecl>(D)->parameters()
                                      : cast<FunctionDecl>(D)->parameters();



> The repro I have is 20M, creduce chokes on it, and manually reducing failed to repro... ☹️

You could try declaring an ObjC method with a thread safety attribute that refers to a parameter, then somewhere else call that function. Again, I don't know ObjC, but for a C function you would do:

  struct Data { Mutex mu; }
  void f(struct Data *data) __attribute__ ((exclusive_locks_required(data->mu)));
  
  void g() {
    Data d;
    Lock(&d.mu);
    f(&d);  // Should only work with your change.
    Unlock(&d.mu);
  }

Then the SExprBuilder should have to replace the reference to the parameter (here `data`) by the value it is called with (here `&d`). I'll have to admit I don't find it easy to wrap my head around this logic either, so maybe I'm missing something. The tests for ObjC[++] are very brief, but you can look at the C++ tests for hints. (See `test/Sema{ObjC,ObjC++,C++}/warn-thread-safety-analysis.{m,mm,cpp}`.)


Repository:
  rC Clang

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

https://reviews.llvm.org/D59523