[libc] Fix typo and amend restrict qualifier

[Caslyn]

This removes an extraneous ',' in the generated dlfcn header.

This also adds __restrict to dladdr's declaration per POSIX. Another fix is made: the C standard restrict keyword is removed from dlinfo.cpp/dlinfo.h (but note that dlfcn.yaml still annotates __restrict for dlinfo's decl).

[llvmbot]

@llvm/pr-subscribers-libc

Author: Caslyn Tonelli (Caslyn)

Changes

This removes an extraneous ',' in the generated dlfcn header.

This also adds __restrict to dladdr's declaration per POSIX. Another fix is made: the C standard restrict keyword is removed from dlinfo.cpp/dlinfo.h (but note that dlfcn.yaml still annotates __restrict for dlinfo's decl).

---

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

3 Files Affected:

• (modified) libc/include/dlfcn.yaml (+3-3)
• (modified) libc/src/dlfcn/dlinfo.cpp (+1-2)
• (modified) libc/src/dlfcn/dlinfo.h (+1-1)

diff --git a/libc/include/dlfcn.yaml b/libc/include/dlfcn.yaml
index db2893aaff5d9..bf17a11111050 100644
--- a/libc/include/dlfcn.yaml
+++ b/libc/include/dlfcn.yaml
@@ -46,7 +46,7 @@ enums:
     standards:
       - gnu
     value: 2
-  - name: RTLD_DI_CONFIGADDR,
+  - name: RTLD_DI_CONFIGADDR
     standards:
       - gnu
     value: 3
@@ -127,5 +127,5 @@ functions:
       - POSIX
     return_type: int
     arguments:
-      - type: const void *
-      - type: Dl_info *
+      - type: const void *__restrict
+      - type: Dl_info *__restrict
diff --git a/libc/src/dlfcn/dlinfo.cpp b/libc/src/dlfcn/dlinfo.cpp
index d78cade5ea593..ec2b0fe1464bc 100644
--- a/libc/src/dlfcn/dlinfo.cpp
+++ b/libc/src/dlfcn/dlinfo.cpp
@@ -15,8 +15,7 @@
 namespace LIBC_NAMESPACE_DECL {
 
 // TODO: https://github.com/llvm/llvm-project/issues/149911
-LLVM_LIBC_FUNCTION(int, dlinfo,
-                   (void *restrict handle, int request, void *restrict info)) {
+LLVM_LIBC_FUNCTION(int, dlinfo, (void *handle, int request, void *info)) {
   return -1;
 }
 
diff --git a/libc/src/dlfcn/dlinfo.h b/libc/src/dlfcn/dlinfo.h
index c2c34f02bd6f1..98f627636a34c 100644
--- a/libc/src/dlfcn/dlinfo.h
+++ b/libc/src/dlfcn/dlinfo.h
@@ -13,7 +13,7 @@
 
 namespace LIBC_NAMESPACE_DECL {
 
-int dlinfo(void *restrict, int, void *restrict);
+int dlinfo(void *, int, void *);
 
 } // namespace LIBC_NAMESPACE_DECL
 

[lntue]

dlinfo in dlfcn.yaml is declared with:

      - type: void *__restrict
      - type: int
      - type: void *__restrict

[Caslyn]

FWIW I wasn't sure about annotating dlinfo.cpp/dlinfo.h. For example, I looked at how the existing dlsym added __restrict in dlfcn.yaml[0] but omits the qualifiers in its source files[1].

[0] https://github.com/llvm/llvm-project/blob/main/libc/include/dlfcn.yaml#L110-L116
[1] https://github.com/llvm/llvm-project/blob/main/libc/src/dlfcn/dlsym.cpp

Double checking to proceed with adding __restrict on the stub definition? (I can add it on the other dlfcn stubs as well).

[lntue]

Double checking to proceed with adding __restrict on the stub definition? (I can add it on the other dlfcn stubs as well).

Yes please. We should keep the signatures consistent. Thanks,

[frobtech]

For standard qualifiers like this, the declaration is authoritative and if the definition doesn't match and the compiler doesn't make that an error then it means it doesn't change the semantics to have it only on the declaration. (The situation is different for certain attributes.)

However, note that there won't be any cross-checking of the public (extern "C") signature and the namespaced internal signature unless you're doing a production build where the definitions are aliases and the compiler will complain about mismatches. So it's important to take care that the src/fooheader/barfunc.h internal declaration precisely matches the public C signature in the generated public function.

And, notwithstanding the first paragraph, for comprehensibility and maintenance reasons, it should always be our practice to use the full precise signature in the definition just as it is in the definition (modulo function attributes).

Ergo, this is missing dladdr.{cpp,h} changes to match the generated declaration, and dlinfo.{cpp,h}| were better as they were.

[Caslyn]

done in d01192e

[frobtech]

LGTM with changes to keep restrict everywhere for both dladdr and dlinfo

[frobtech]

For standard qualifiers like this, the declaration is authoritative and if the definition doesn't match and the compiler doesn't make that an error then it means it doesn't change the semantics to have it only on the declaration. (The situation is different for certain attributes.)

However, note that there won't be any cross-checking of the public (extern "C") signature and the namespaced internal signature unless you're doing a production build where the definitions are aliases and the compiler will complain about mismatches. So it's important to take care that the src/fooheader/barfunc.h internal declaration precisely matches the public C signature in the generated public function.

And, notwithstanding the first paragraph, for comprehensibility and maintenance reasons, it should always be our practice to use the full precise signature in the definition just as it is in the definition (modulo function attributes).

Ergo, this is missing dladdr.{cpp,h} changes to match the generated declaration, and dlinfo.{cpp,h}| were better as they were.