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

[TLI] Add support for reallocarray #114818

Merged
merged 3 commits into from
Nov 13, 2024

Conversation

serge-sans-paille
Copy link
Collaborator

reallocarray is available in glibc since 2.29 under _DEFAULT_SOURCE and under _GNU_SOURCE before, let's model it appropriately.

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" llvm:analysis llvm:transforms labels Nov 4, 2024
@llvmbot
Copy link

llvmbot commented Nov 4, 2024

@llvm/pr-subscribers-llvm-analysis

@llvm/pr-subscribers-llvm-transforms

Author: None (serge-sans-paille)

Changes

reallocarray is available in glibc since 2.29 under _DEFAULT_SOURCE and under _GNU_SOURCE before, let's model it appropriately.


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

7 Files Affected:

  • (modified) clang/include/clang/Basic/Builtins.td (+6)
  • (modified) clang/test/Sema/builtins-gnu-mode.c (+1)
  • (modified) llvm/include/llvm/Analysis/TargetLibraryInfo.def (+5)
  • (modified) llvm/lib/Analysis/TargetLibraryInfo.cpp (+1)
  • (modified) llvm/lib/Transforms/Utils/BuildLibCalls.cpp (+15)
  • (modified) llvm/test/tools/llvm-tli-checker/ps4-tli-check.yaml (+2-2)
  • (modified) llvm/unittests/Analysis/TargetLibraryInfoTest.cpp (+1)
diff --git a/clang/include/clang/Basic/Builtins.td b/clang/include/clang/Basic/Builtins.td
index 9bd67e0cefebc3..546442f843a6b3 100644
--- a/clang/include/clang/Basic/Builtins.td
+++ b/clang/include/clang/Basic/Builtins.td
@@ -3224,6 +3224,12 @@ def AllocA : GNULibBuiltin<"stdlib.h"> {
   let AddBuiltinPrefixedAlias = 1;
 }
 
+def ReallocArray : GNULibBuiltin<"stdlib.h"> {
+  let Spellings = ["reallocarray"];
+  let Prototype = "void*(void*, size_t, size_t)";
+}
+
+
 // POSIX malloc.h
 
 def MemAlign : GNULibBuiltin<"malloc.h"> {
diff --git a/clang/test/Sema/builtins-gnu-mode.c b/clang/test/Sema/builtins-gnu-mode.c
index d93b6fdef027b6..f85a1e5793a703 100644
--- a/clang/test/Sema/builtins-gnu-mode.c
+++ b/clang/test/Sema/builtins-gnu-mode.c
@@ -8,6 +8,7 @@ int stpncpy;
 int strdup;
 int strndup;
 int index;
+int reallocarray;
 int rindex;
 int bzero;
 int strcasecmp;
diff --git a/llvm/include/llvm/Analysis/TargetLibraryInfo.def b/llvm/include/llvm/Analysis/TargetLibraryInfo.def
index fd53a26ef8fc11..db566b8ee610e5 100644
--- a/llvm/include/llvm/Analysis/TargetLibraryInfo.def
+++ b/llvm/include/llvm/Analysis/TargetLibraryInfo.def
@@ -2077,6 +2077,11 @@ TLI_DEFINE_ENUM_INTERNAL(reallocf)
 TLI_DEFINE_STRING_INTERNAL("reallocf")
 TLI_DEFINE_SIG_INTERNAL(Ptr, Ptr, SizeT)
 
+/// void *reallocarray(void *ptr, size_t nmemb, size_t size);
+TLI_DEFINE_ENUM_INTERNAL(reallocarray)
+TLI_DEFINE_STRING_INTERNAL("reallocarray")
+TLI_DEFINE_SIG_INTERNAL(Ptr, Ptr, SizeT, SizeT)
+
 /// char *realpath(const char *file_name, char *resolved_name);
 TLI_DEFINE_ENUM_INTERNAL(realpath)
 TLI_DEFINE_STRING_INTERNAL("realpath")
diff --git a/llvm/lib/Analysis/TargetLibraryInfo.cpp b/llvm/lib/Analysis/TargetLibraryInfo.cpp
index 7f0b98ab3c1514..e4b1dbfc3e7231 100644
--- a/llvm/lib/Analysis/TargetLibraryInfo.cpp
+++ b/llvm/lib/Analysis/TargetLibraryInfo.cpp
@@ -852,6 +852,7 @@ static void initializeLibCalls(TargetLibraryInfoImpl &TLI, const Triple &T,
     TLI.setUnavailable(LibFunc_memrchr);
     TLI.setUnavailable(LibFunc_ntohl);
     TLI.setUnavailable(LibFunc_ntohs);
+    TLI.setUnavailable(LibFunc_reallocarray);
     TLI.setUnavailable(LibFunc_reallocf);
     TLI.setUnavailable(LibFunc_roundeven);
     TLI.setUnavailable(LibFunc_roundevenf);
diff --git a/llvm/lib/Transforms/Utils/BuildLibCalls.cpp b/llvm/lib/Transforms/Utils/BuildLibCalls.cpp
index e039457f313b29..8887f2d7792fee 100644
--- a/llvm/lib/Transforms/Utils/BuildLibCalls.cpp
+++ b/llvm/lib/Transforms/Utils/BuildLibCalls.cpp
@@ -577,6 +577,21 @@ bool llvm::inferNonMandatoryLibFuncAttrs(Function &F,
     Changed |= setDoesNotCapture(F, 0);
     Changed |= setArgNoUndef(F, 1);
     break;
+  case LibFunc_reallocarray:
+    Changed |= setAllocFamily(F, "malloc");
+    Changed |= setAllocKind(F, AllocFnKind::Realloc);
+    Changed |= setAllocatedPointerParam(F, 0);
+    Changed |= setAllocSize(F, 1, std::nullopt);
+    Changed |= setAllocSize(F, 2, std::nullopt);
+    Changed |= setOnlyAccessesInaccessibleMemOrArgMem(F);
+    Changed |= setRetNoUndef(F);
+    Changed |= setDoesNotThrow(F);
+    Changed |= setRetDoesNotAlias(F);
+    Changed |= setWillReturn(F);
+    Changed |= setDoesNotCapture(F, 0);
+    Changed |= setArgNoUndef(F, 1);
+    Changed |= setArgNoUndef(F, 2);
+    break;
   case LibFunc_read:
     // May throw; "read" is a valid pthread cancellation point.
     Changed |= setRetAndArgsNoUndef(F);
diff --git a/llvm/test/tools/llvm-tli-checker/ps4-tli-check.yaml b/llvm/test/tools/llvm-tli-checker/ps4-tli-check.yaml
index 6702725e4fc8a4..2d23b15d74b17d 100644
--- a/llvm/test/tools/llvm-tli-checker/ps4-tli-check.yaml
+++ b/llvm/test/tools/llvm-tli-checker/ps4-tli-check.yaml
@@ -54,10 +54,10 @@
 ## the exact count first; the two directives should add up to that.
 ## Yes, this means additions to TLI will fail this test, but the argument
 ## to -COUNT can't be an expression.
-# AVAIL: TLI knows 522 symbols, 289 available
+# AVAIL: TLI knows 523 symbols, 289 available
 # AVAIL-COUNT-289: {{^}} available
 # AVAIL-NOT:       {{^}} available
-# UNAVAIL-COUNT-233: not available
+# UNAVAIL-COUNT-234: not available
 # UNAVAIL-NOT:       not available
 
 ## This is a large file so it's worth telling lit to stop here.
diff --git a/llvm/unittests/Analysis/TargetLibraryInfoTest.cpp b/llvm/unittests/Analysis/TargetLibraryInfoTest.cpp
index 982d00c5d33593..b9900db68b1420 100644
--- a/llvm/unittests/Analysis/TargetLibraryInfoTest.cpp
+++ b/llvm/unittests/Analysis/TargetLibraryInfoTest.cpp
@@ -318,6 +318,7 @@ TEST_F(TargetLibraryInfoTest, ValidProto) {
       "declare void @qsort(i8*, i64, i64, i32 (i8*, i8*)*)\n"
       "declare i64 @readlink(i8*, i8*, i64)\n"
       "declare i8* @realloc(i8*, i64)\n"
+      "declare i8* @reallocarray(i8*, i64, i64)\n"
       "declare i8* @reallocf(i8*, i64)\n"
       "declare double @remainder(double, double)\n"
       "declare float @remainderf(float, float)\n"

@@ -318,6 +318,7 @@ TEST_F(TargetLibraryInfoTest, ValidProto) {
"declare void @qsort(i8*, i64, i64, i32 (i8*, i8*)*)\n"
"declare i64 @readlink(i8*, i8*, i64)\n"
"declare i8* @realloc(i8*, i64)\n"
"declare i8* @reallocarray(i8*, i64, i64)\n"
Copy link
Contributor

Choose a reason for hiding this comment

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

This test should be converted to opaque pointers

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ack. but that probably belongs to another commit, right?

Copy link
Contributor

Choose a reason for hiding this comment

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

yes

@@ -852,6 +852,7 @@ static void initializeLibCalls(TargetLibraryInfoImpl &TLI, const Triple &T,
TLI.setUnavailable(LibFunc_memrchr);
TLI.setUnavailable(LibFunc_ntohl);
TLI.setUnavailable(LibFunc_ntohs);
TLI.setUnavailable(LibFunc_reallocarray);
Copy link
Contributor

Choose a reason for hiding this comment

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

"Since 2.29 under _DEFAULT_SOURCE and under _GNU_SOURCE"

Can you check the version and comment this somewhere?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That's from the manpage (e.g. https://man.archlinux.org/man/reallocarray.3.en). I'll update the commit message to make it clear.

Copy link
Contributor

Choose a reason for hiding this comment

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

I mean this needs to be documented in the code, not the commit message

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done!

Copy link
Contributor

Choose a reason for hiding this comment

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

How did you determine whether this is available on PS?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I grepped the whole sdk (for PS4 though). Plenty of mention to realloc and none to reallocarray

From the man page (e.g. https://man.archlinux.org/man/reallocarray.3.en):

SYNOPSIS

reallocarray():

    Since glibc 2.29:
        _DEFAULT_SOURCE
    glibc 2.28 and earlier:
        _GNU_SOURCE

STANDARDS

reallocarray()
    None.

HISTORY

reallocarray()
    glibc 2.26. OpenBSD 5.6, FreeBSD 11.0.
@serge-sans-paille
Copy link
Collaborator Author

@arsenm looks good?

@@ -3224,6 +3224,13 @@ def AllocA : GNULibBuiltin<"stdlib.h"> {
let AddBuiltinPrefixedAlias = 1;
}

// Available in glibc by default since since 2.29 and in GNU mode before.
def ReallocArray : GNULibBuiltin<"stdlib.h"> {
Copy link
Contributor

Choose a reason for hiding this comment

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

The clang builtin change should be separate, and is not tested

@@ -8,6 +8,7 @@ int stpncpy;
int strdup;
int strndup;
int index;
int reallocarray;
Copy link
Contributor

Choose a reason for hiding this comment

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

This change probably shouldn't be here anymore?

@@ -852,6 +852,7 @@ static void initializeLibCalls(TargetLibraryInfoImpl &TLI, const Triple &T,
TLI.setUnavailable(LibFunc_memrchr);
TLI.setUnavailable(LibFunc_ntohl);
TLI.setUnavailable(LibFunc_ntohs);
TLI.setUnavailable(LibFunc_reallocarray);
Copy link
Contributor

Choose a reason for hiding this comment

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

How did you determine whether this is available on PS?

@serge-sans-paille
Copy link
Collaborator Author

Should be good now. thanks for the reviews @nikic & @arsenm

Copy link
Contributor

@nikic nikic left a comment

Choose a reason for hiding this comment

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

LGTM

@serge-sans-paille serge-sans-paille merged commit dc4185f into llvm:main Nov 13, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category llvm:analysis llvm:transforms
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants