Fix crash in 'malloc' referring to function without a argument

[erichkeane]

As reported in #159080, patch #68059 didn't correctly check for the argument count of the target function from malloc to ensure it has an argument. This patch corrects that check.

Fixes: #159080

[llvmbot]

@llvm/pr-subscribers-clang

Author: Erich Keane (erichkeane)

Changes

As reported in #159080, patch #68059 didn't correctly check for the argument count of the target function from malloc to ensure it has an argument. This patch corrects that check.

---

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

3 Files Affected:

• (modified) clang/docs/ReleaseNotes.rst (+3)
• (modified) clang/lib/Sema/SemaDeclAttr.cpp (+5-1)
• (modified) clang/test/Sema/attr-args.c (+6)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 80f4e83a11b70..5a27bc851eb3a 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -347,6 +347,9 @@ Bug Fixes in This Version
   ``-Wshadow`` and show uncaptured-local warnings with ``-Wshadow-all``. (#GH68605)
 - Fixed a failed assertion with a negative limit parameter value inside of
   ``__has_embed``. (#GH157842)
+- Fixed an assertion when an improper use of the ``malloc`` attribute targetting
+  a function without arguments caused us to try to access a non-existant argument.
+  (#GH159080)
 
 Bug Fixes to Compiler Builtins
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index 44906456f3371..b6ebe54764282 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -1802,7 +1802,11 @@ static void handleRestrictAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
   if (AL.getNumArgs() == 1) {
     DeallocPtrIdx = ParamIdx(1, DeallocFD);
 
-    if (!DeallocPtrIdx.isValid() ||
+    // FIXME: We could probably be better about diagnosing that there IS no
+    // argument, or that the function doesn't have a prototype, but this is how
+    // GCC diagnoses this, and is reasonably clear.
+    if (!DeallocPtrIdx.isValid() || !hasFunctionProto(DeallocFD) ||
+        getFunctionOrMethodNumParams(DeallocFD) < 1 ||
         !getFunctionOrMethodParamType(DeallocFD, DeallocPtrIdx.getASTIndex())
              .getCanonicalType()
              ->isPointerType()) {
diff --git a/clang/test/Sema/attr-args.c b/clang/test/Sema/attr-args.c
index 23815f3a4e675..01bfcc1951cd8 100644
--- a/clang/test/Sema/attr-args.c
+++ b/clang/test/Sema/attr-args.c
@@ -29,3 +29,9 @@ __attribute__ ((__format_arg__(2))) // expected-error {{'__format_arg__' attribu
 void test (int, ...);
 
 void __attribute__ ((alloc_size (2, 3))) *test2(int, ...); // expected-error {{'alloc_size' attribute parameter 1 is out of bounds}}
+
+void gh159080_a(void);
+void *gh159080_b(void) __attribute__((malloc(gh159080_a))); // expected-error{{'malloc' argument 'gh159080_a' must take a pointer type as its first argument}}
+void gh159080_c();
+void *gh159080_d(void) __attribute__((malloc(gh159080_c))); // expected-error{{'malloc' argument 'gh159080_c' must take a pointer type as its first argument}}
+

[Fznamznon]

Suggested change

	- Fixed an assertion when an improper use of the ``malloc`` attribute targetting
	a function without arguments caused us to try to access a non-existant argument.
	(#GH159080)
	- Fixed an assertion when an improper use of the ``malloc`` attribute targeting
	a function without arguments caused us to try to access a non-existent argument.
	(#GH159080)

[Fznamznon]

I was beaten, but there is still targetting vs targeting.

[erichkeane]

Woops! missed my 'push' had failed.