[PATCH] D96087: Option to ignore llvm[.compiler].used uses in hasAddressTaken()

[Stanislav Mekhanoshin via Phabricator via llvm-commits]

rampitec added inline comments.


================
Comment at: llvm/lib/IR/Function.cpp:1617
+                     (GV->getName().equals("llvm.compiler.used") ||
+                      GV->getName().equals("llvm.used"));
+            return false;
----------------
madhur13490 wrote:
> rampitec wrote:
> > madhur13490 wrote:
> > > Can you please add a test which has non-bitcast value of function pointers? Something like below;
> > > 
> > > ```
> > > @llvm.used = appending global [2 x float()*] [void()* @foo, void()* @bar]
> > > ```
> > > 
> > > IIRC, the above simple cast to GlobalVariable is not sufficient in this case. You need to handle it with some more extra checks.
> > Does this really happen? My understanding these arrays shall have i8* type, so a function pointer always needs a bitcast.
> Yes, as per the spec, "This array contains a list of pointers to named global variables, functions and aliases **which may optionally have a pointer cast **formed of bitcast or getelementptr". So the pointer cast is optional. You can have plain function pointers in the array. The newly added code will handle only bitcast cast case but not general. You need to handle the general case and find if one of the users if GlobalVariable. 
> 
> ```
> @llvm.used = appending global [2 x void()*] [void()* @foo, void()* @bar]
> ```
This is already done in the previous patch. I still think that does not practically happen because you cannot have elements of different types in an array. The words "may optionally have a pointer cast" shall apply if your pointer is not of the i8* type (since this variable is not only used for functions). For sure one can artificially forge such initializer (like I did in the test), but that does not have much practical sense.


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

https://reviews.llvm.org/D96087