[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:
> > > 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.
> Two questions:
> 1. I don't understand why the below the initialization of llvm.used is invalid? 
> 
> ```
> @llvm.used = appending global [2 x void()*] [void()* @foo, void()* @bar]
> ```
> I don't see verifier complain about it, which make me believe that it is a valid LLVM construct. Are you saying that it is not common so you'd like to skip it?
> 
> 2. Assumming that 1 is valid, does the patch succeed to determine if one of the user of function symbol is GlobalVariable?
1. The initialization is not invalid, it just useless. If you have functions with different types in a module you will not be able to to initialize it. You also will be unable link different modules with such initializations.
2. Yet it works with the patch. See the test for llvm.compiler.used.


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

https://reviews.llvm.org/D96087