[PATCH] D68377: [Builtins] Teach Clang about memccpy

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 marked an inline comment as done.
xbolva00 added inline comments.


================
Comment at: include/clang/Basic/Builtins.def:983
 // POSIX string.h
+LIBBUILTIN(memccpy, "v*v*vC*iz",  "f",     "string.h", ALL_GNU_LANGUAGES)
 LIBBUILTIN(stpcpy, "c*c*cC*",     "f",     "string.h", ALL_GNU_LANGUAGES)
----------------
aaron.ballman wrote:
> xbolva00 wrote:
> > aaron.ballman wrote:
> > > xbolva00 wrote:
> > > > aaron.ballman wrote:
> > > > > xbolva00 wrote:
> > > > > > aaron.ballman wrote:
> > > > > > > Isn't `memccpy` supported by Visual Studio? What should happen there?
> > > > > > ”This POSIX function is deprecated. Use the ISO C++ conformant _memccpy instead.“
> > > > > Thanks for verifying it's still supported (deprecated != unsupported). We do support other Microsoft builtins, so should this one be added?
> > > > Ok, I will add it.
> > > > 
> > > > One last question, should I somehow represent that memccpy is in C 20 (if so, how) ?
> > > You should probably start a new section for C2x library functions and add `memccpy`, `strdup`, and `strndup` to it.
> > > 
> > > Also, just to double-check: we don't encode `restrict` into builtin signatures, do we? (Should we?)
> > >> You should probably start a new section for C2x library functions and add memccpy, strdup, and strndup to it.
> > Okay, thanks - probably not needed for now (should be separate patch to handle new functions in C 2X anyway; maybe more functions to be added?)
> > 
> > >> Also, just to double-check: we don't encode restrict into builtin signatures, do we? (Should we?)
> > Maybe catch very simple overlap issues? I dont think this is worth it (we have sanitizers). LLVM annotates libc functions with noalias anyway.
> > Okay, thanks - probably not needed for now (should be separate patch to handle new functions in C 2X anyway; maybe more functions to be added?)
> 
> Yeah, can totally be done in a separate patch.
> 
> > Maybe catch very simple overlap issues? I dont think this is worth it (we have sanitizers). LLVM annotates libc functions with noalias anyway.
> 
> Sanitizers don't run everywhere, but more importantly, I was wondering about the possible optimization opportunities by noting which arguments cannot overlap with one another (perhaps noalias covers that; I'm less familiar with the llvm side of things).
> 
> However, I looked at the file and see we don't have any encodings for it, so nothing to be done here.
>> I was wondering about the possible optimization opportunities by noting which arguments cannot overlap with one another

This is implemented in LLVM. 

For example: https://godbolt.org/z/E8x2Ev (see line 10).

>> Sanitizers don't run everywhere

GCC emits a lot of warnings from middle-end. While I dont like some implementations which are part of regular pass, but extra warning pass just to check specific thing is nice idea I think (that pass is enabled only with -Wxxxx flag).


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

https://reviews.llvm.org/D68377