[PATCH] D122608: Fix behavior of ifuncs with 'used' extern "C" static functions

[Tom Honermann via Phabricator via cfe-commits]

tahonermann added inline comments.


================
Comment at: clang/lib/CodeGen/CodeGenModule.cpp:6319
+/// If all uses of the GV are from an IFunc resolver, which can happen when the
+/// IFunc resolver is a static-function, but the name ends up being different,
+/// return the IFunc so it can have its resolver replaced with the correct
----------------
An example of why the names might not match would be helpful here.


================
Comment at: clang/lib/CodeGen/CodeGenModule.cpp:6323-6325
+  // If there is more than 1 use, I don't really know what we can do.
+  // EmitStaticExternCAliases won't be able to do anything, and the ifunc
+  // diagnostic will have to catch this.
----------------
This comment doesn't state what the challenge is when there is more than one use. Is the concern that there may be multiple ifuncs that have the same associated resolver? What might cause that to happen? (presumably multiple functions declared with the `ifunc` attribute that name the same resolver).

A test that exercises this case seems appropriate.


================
Comment at: clang/test/SemaCXX/externc-ifunc-resolver.cpp:8
+  }
+  __attribute__((ifunc("resolve_foo"))) void foo(); // expected-error{{ifunc must point to a defined function}}
+}
----------------
There is considerable subtlety between these two tests and, unfortunately, the diagnostic isn't particularly helpful.

If I'm following correctly, the other test passes because an unmangled name is specified for the `ifunc` attribute and that is resolved against the emitted alias.

In this test, the same code is present as the other test, but with an additional declaration that doesn't seem relevant; one would not expect lookup for "resolve_foo" in the `ifunc` attribute to include the `NS` namespace. Is the error generated because two aliases for "resolve_foo" are emitted? If so, that seems like something that should be diagnosed somewhere. Or perhaps an alias should not be emitted for `used` functions defined at namespace scope.


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

https://reviews.llvm.org/D122608