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

[Eli Friedman via Phabricator via llvm-commits]

efriedma added a comment.

> If the user wrote a function either with the same identical name as a library function, or it ends up being aliased with such a name, it seems we should here as well trust that the front end has added the mandatory attributes, or?

If we find an alias, I wouldn't trust that the alias points to a function with a real signature.

> Maybe we should change getOrInsertLibFunc() to first check for any existing Function or GlobalAlias matching Name. If there is already one of those, we could then assume that we have either already created it in getOrInsertLibFunc() earlier or that it is an alias created by the front end. So only when we are actually inserting a library function into the Module the first time do we need to add the attributes, right?

We could, sure.  If we want to be conservative, I guess we could mess with isValidProtoForLibFunc to verify the attributes.

> The user could either write a function named e.g. fwrite(), or an alias called fwrite. It seems that an optimizer emitting fwrite() therefore unknowingly either uses the library function, or the user provided function, which is somewhat strange to me: I would have guessed that the libcall emissions only emit libcalls and nothing else. I wonder even if that could lead to problems - what if the user wrote a function that did something entirely different and just happened to give it the same name as a libfunction? Is there something preventing unintended "libcall" emissions in such a case? This seems broken to me unless the user really is supposed to be able to write his own library function and expect it to be used entirely the same as the standard library function would be, as a stand-in. That is a separate topic from this patch, though...

If the user is defining functions that overlap with core libc function names, we expect them to use "-fno-builtin-fwrite" etc.  If the user doesn't specify the right no-builtin attributes, we just sort of best-effort avoid crashing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123198