Skip to content

[libc] Fix typo and amend restrict qualifier #152410

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Aug 7, 2025
Merged

[libc] Fix typo and amend restrict qualifier #152410

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f3ca4fa
[lib] Fix typo and amend restrict qualifier
Caslyn Aug 6, 2025
d01192e
add __restrict qualifiers to source
Caslyn Aug 7, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions libc/include/dlfcn.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ enums:
standards:
- gnu
value: 2
- name: RTLD_DI_CONFIGADDR,
- name: RTLD_DI_CONFIGADDR
standards:
- gnu
value: 3
Expand Down Expand Up @@ -127,5 +127,5 @@ functions:
- POSIX
return_type: int
arguments:
- type: const void *
- type: Dl_info *
- type: const void *__restrict
- type: Dl_info *__restrict
3 changes: 1 addition & 2 deletions libc/src/dlfcn/dlinfo.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
Copy link
Contributor

@lntue lntue Aug 7, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

dlinfo in dlfcn.yaml is declared with:

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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).

Copy link
Contributor

Choose a reason for hiding this comment

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

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,

Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done in d01192e

return -1;
}

Expand Down
2 changes: 1 addition & 1 deletion libc/src/dlfcn/dlinfo.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@

namespace LIBC_NAMESPACE_DECL {

int dlinfo(void *restrict, int, void *restrict);
int dlinfo(void *, int, void *);

} // namespace LIBC_NAMESPACE_DECL

Expand Down
Loading