[PATCH] D123198: [LibCalls] Add argument extension attributes to more functions.

[Jonas Paulsson via Phabricator via llvm-commits]

jonpa added a comment.

> ... An Assert in BuildLibCalls.cpp fails ...

Thanks for reporting this. This is because InstCombiner is replacing the @__stpncpy_chk() call with a call to @stpncpy(), which has a sizeof parameter (see earlier discussion). This went undetected as there was no regression test covering this. I suppose there must be more of these...

I guess I should make a list of all built libcalls with a sizeof parameter and include them as accepted in getOrInsertLibFunc()?

> Even if there is a function, I wouldn't trust it has the right signature. In general, we don't require that function declarations have the right signature. (For example, you can write struct A; struct A f(void), (*g)(void) = f; in C.)

OK - now I understand better: the Function Type may actually be incomplete at this point.

IIUC, this is basically the users responsibility: If user decides to write e.g. a function named fwrite (or a function alias of that name), either

-fno-builtin-fwrite is passed in which case fwrite will not be available to generate so getOrInsertLibFunc() will never be reached.
-fno-builtin-fwrite is *not* passed (as in pr39177.ll) and any problem is then basically the users fault and getOrInsertLibFunc() does not have to care about this.

In getOrInsertLibFunc() we could either

1. Insert into Module a Function with the given Name
2. Get back a previusly inserted Function a. a proper libcall b. a user defined function
3. Get back a GlobalAlias matching Name.

I think we really only need to care about #1. We could check with isValidProtoForLibFunc() and add mandatory arguments also in cases of #2b and #3 where the prototype matches, but maybe that is not worth the effort since this is not really supposed to happen..? (And again, the front end should have emitted those I think).

Would it makes sense then to simply add to the patch to skip adding mandatory attributes in case #3 (when we get a function alias)?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123198