Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[libc] Give more functions restrict qualifiers (NFC) #78061

Merged
merged 1 commit into from
Jan 15, 2024

Conversation

AtariDreams
Copy link
Contributor

@AtariDreams AtariDreams commented Jan 13, 2024

strsep, strtok_r, strlcpy, and strlcat take restricted pointers as parameters.
Add the restrict qualifiers to them.

Sources:
https://man7.org/linux/man-pages/man3/strsep.3.html
https://man7.org/linux/man-pages/man3/strtok_r.3.html
https://man.freebsd.org/cgi/man.cgi?strlcpy

@llvmbot llvmbot added clang Clang issues not falling into any other category libc labels Jan 13, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Jan 13, 2024

@llvm/pr-subscribers-llvm-support
@llvm/pr-subscribers-compiler-rt-sanitizer
@llvm/pr-subscribers-backend-x86
@llvm/pr-subscribers-libc

@llvm/pr-subscribers-clang

Author: AtariDreams (AtariDreams)

Changes

strsep has restrict qualifiers, as well as strtok_r. Add the restrict qualifiers to them.

Source: https://man7.org/linux/man-pages/man3/strsep.3.html


Full diff: https://github.com/llvm/llvm-project/pull/78061.diff

4 Files Affected:

  • (modified) clang/test/Analysis/Inputs/system-header-simulator.h (+3-3)
  • (modified) clang/test/Analysis/string.c (+2-2)
  • (modified) libc/src/string/strsep.cpp (+1-1)
  • (modified) libc/src/string/strsep.h (+1-1)
diff --git a/clang/test/Analysis/Inputs/system-header-simulator.h b/clang/test/Analysis/Inputs/system-header-simulator.h
index cd7ac616bcc67f..5f969adfdd92cd 100644
--- a/clang/test/Analysis/Inputs/system-header-simulator.h
+++ b/clang/test/Analysis/Inputs/system-header-simulator.h
@@ -71,9 +71,9 @@ int fflush(FILE *stream);
 size_t strlen(const char *);
 
 char *strcpy(char *restrict, const char *restrict);
-char *strncpy(char *dst, const char *src, size_t n);
-char *strsep(char **stringp, const char *delim);
-void *memcpy(void *dst, const void *src, size_t n);
+char *strncpy(char *restrict dst, const char * restrict src, size_t n);
+char *strsep(char ** restrict stringp, const char * restrict delim);
+void *memcpy(void * restrict dst, const void * restrict src, size_t n);
 void *memset(void *s, int c, size_t n);
 
 typedef unsigned long __darwin_pthread_key_t;
diff --git a/clang/test/Analysis/string.c b/clang/test/Analysis/string.c
index d47de9db8228e5..85232624160c06 100644
--- a/clang/test/Analysis/string.c
+++ b/clang/test/Analysis/string.c
@@ -71,7 +71,7 @@ void clang_analyzer_eval(int);
 int scanf(const char *restrict format, ...);
 void *malloc(size_t);
 void free(void *);
-void *memcpy(void *dest, const void *src, size_t n);
+void *memcpy(void *restrict dest, const void *restrict src, size_t n);
 
 //===----------------------------------------------------------------------===
 // strlen()
@@ -1252,7 +1252,7 @@ int strncasecmp_null_argument(char *a, size_t n) {
 // strsep()
 //===----------------------------------------------------------------------===
 
-char *strsep(char **stringp, const char *delim);
+char *strsep(char ** restrict stringp, const char * restrict delim);
 
 void strsep_null_delim(char *s) {
   strsep(&s, NULL); // expected-warning{{Null pointer passed as 2nd argument to strsep()}}
diff --git a/libc/src/string/strsep.cpp b/libc/src/string/strsep.cpp
index edd2cf07e20a45..ce74223ee7721d 100644
--- a/libc/src/string/strsep.cpp
+++ b/libc/src/string/strsep.cpp
@@ -12,7 +12,7 @@
 
 namespace LIBC_NAMESPACE {
 
-LLVM_LIBC_FUNCTION(char *, strsep, (char **stringp, const char *delim)) {
+LLVM_LIBC_FUNCTION(char *, strsep, (char ** __restrict stringp, const char * __restrict delim)) {
   if (!*stringp)
     return nullptr;
   return internal::string_token<false>(*stringp, delim, stringp);
diff --git a/libc/src/string/strsep.h b/libc/src/string/strsep.h
index 48f55a899d7f33..d0a22f4af89119 100644
--- a/libc/src/string/strsep.h
+++ b/libc/src/string/strsep.h
@@ -11,7 +11,7 @@
 
 namespace LIBC_NAMESPACE {
 
-char *strsep(char **stringp, const char *delim);
+char *strsep(char ** __restrict stringp, const char * __restrict delim);
 
 } // namespace LIBC_NAMESPACE
 

@AtariDreams AtariDreams force-pushed the restrict branch 3 times, most recently from df4e56a to eafa096 Compare January 14, 2024 01:13
@llvmbot llvmbot added backend:X86 clang:headers Headers provided by Clang, e.g. for intrinsics labels Jan 14, 2024
Copy link
Contributor

@lntue lntue left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM for the libc part.
Thanks for adding the missing qualifiers! They were specified in our own generated header but not in the implementation: https://github.com/llvm/llvm-project/blob/main/libc/spec/bsd_ext.td#L21

Copy link

github-actions bot commented Jan 14, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

@AtariDreams AtariDreams force-pushed the restrict branch 5 times, most recently from c76bc82 to a2c6ba0 Compare January 15, 2024 00:12
@AtariDreams
Copy link
Contributor Author

@lntue Do you think this PR is acceptable?

@jhuber6
Copy link
Contributor

jhuber6 commented Jan 15, 2024

LLVM changes look unrelated, it was originally copied from OpenBSD it seems. But it's not a major issue.

@AtariDreams
Copy link
Contributor Author

LLVM changes look unrelated, it was originally copied from OpenBSD it seems. But it's not a major issue.

I did that for consistency. If needed I could split into other PRs

@AtariDreams
Copy link
Contributor Author

LLVM changes look unrelated, it was originally copied from OpenBSD it seems. But it's not a major issue.

FWIW I opened a few PRs in FreeBSD regarding this.

@jhuber6
Copy link
Contributor

jhuber6 commented Jan 15, 2024

LLVM changes look unrelated, it was originally copied from OpenBSD it seems. But it's not a major issue.

FWIW I opened a few PRs in FreeBSD regarding this.

Yeah, go ahead and move that portion there so the people who know more about LLVM's regex can look at it compared to the libc team.

@AtariDreams
Copy link
Contributor Author

@jhuber6 Oh you meant the formatting. clang-format did that for some reason. I did not edit the body at all.

@AtariDreams AtariDreams force-pushed the restrict branch 3 times, most recently from 14e0a78 to e96e055 Compare January 15, 2024 15:23
@AtariDreams
Copy link
Contributor Author

@jhuber6 Alright I removed the llvm/Support changes for the sake of the PR

Copy link
Contributor

@jhuber6 jhuber6 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks.

@AtariDreams AtariDreams changed the title [Libc] Give more functions restrict qualifiers [Libc] Give more functions restrict qualifiers (NFC) Jan 15, 2024
@AtariDreams AtariDreams changed the title [Libc] Give more functions restrict qualifiers (NFC) [libc] Give more functions restrict qualifiers (NFC) Jan 15, 2024
@AtariDreams
Copy link
Contributor Author

Ready!

@jhuber6 jhuber6 merged commit e06b5a2 into llvm:main Jan 15, 2024
4 checks passed
@AtariDreams AtariDreams deleted the restrict branch January 15, 2024 18:21
justinfargnoli pushed a commit to justinfargnoli/llvm-project that referenced this pull request Jan 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend:X86 clang:headers Headers provided by Clang, e.g. for intrinsics clang Clang issues not falling into any other category compiler-rt:sanitizer compiler-rt libc llvm:support
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants