[libc-commits] [libc] [libc] don't over include stdlib in the hdr declaring bsearch (PR #89471)

[Nick Desaulniers via libc-commits]

https://github.com/nickdesaulniers created https://github.com/llvm/llvm-project/pull/89471

When building overlay mode with GCC in release mode, glibc's stdlib.h contains
an extern inline declaration of bsearch.  This breaks our use of the gnu::alias
function attribute in LLVM_LIBC_FUNCTION with GCC because GCC checks that the
aliasee is defined in the same TU (clang does not).

We're looking at also potentially updating our definition of LLVM_LIBC_FUNCTION
from libc/src/__support/common.h. Upon testing, I was able to get
-Wnonnull-compare diagnostics from GCC in our definition of bsearch because
glibc declares bsearch with the fugly nonnull function attribute.

There's more we can do here though to improve our implementation of bsearch.
7.24.5.1 says:

    Pointer arguments on such a call shall still have valid values, as
    described in 7.1.4.

We could also use either function attributes or parameter attributes to denote
these should not be null (for users/callers) and perhaps still check for
non-null explicitly under some yet to be discussed hardening configurations in
the future.

Fixes: #60481
Link: https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-nonnull-function-attribute


>From 81785d13f79b37f0a5a557d5c8d2b93780e5f394 Mon Sep 17 00:00:00 2001
From: Nick Desaulniers <ndesaulniers at google.com>
Date: Fri, 19 Apr 2024 16:01:43 -0700
Subject: [PATCH] [libc] don't over include stdlib in the hdr declaring bsearch

When building overlay mode with GCC in release mode, glibc's stdlib.h contains
an extern inline declaration of bsearch.  This breaks our use of the gnu::alias
function attribute in LLVM_LIBC_FUNCTION with GCC because GCC checks that the
aliasee is defined in the same TU (clang does not).

We're looking at also potentially updating our definition of LLVM_LIBC_FUNCTION
from libc/src/__support/common.h. Upon testing, I was able to get
-Wnonnull-compare diagnostics from GCC in our definition of bsearch because
glibc declares bsearch with the fugly nonnull function attribute.

There's more we can do here though to improve our implementation of bsearch.
7.24.5.1 says:

    Pointer arguments on such a call shall still have valid values, as
    described in 7.1.4.

We could also use either function attributes or parameter attributes to denote
these should not be null (for users/callers) and perhaps still check for
non-null explicitly under some yet to be discussed hardening configurations in
the future.

Fixes: #60481
Link: https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-nonnull-function-attribute
---
 libc/src/stdlib/bsearch.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libc/src/stdlib/bsearch.h b/libc/src/stdlib/bsearch.h
index 1de7e051ff6c41..3590198ba55704 100644
--- a/libc/src/stdlib/bsearch.h
+++ b/libc/src/stdlib/bsearch.h
@@ -9,7 +9,7 @@
 #ifndef LLVM_LIBC_SRC_STDLIB_BSEARCH_H
 #define LLVM_LIBC_SRC_STDLIB_BSEARCH_H
 
-#include <stdlib.h>
+#include <stddef.h> // size_t
 
 namespace LIBC_NAMESPACE {