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.

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

Embed ELF metadata about dynamic runtime dependencies #1482

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Embed ELF metadata about dynamic runtime dependencies #1482

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
37 changes: 37 additions & 0 deletions Macros.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,15 @@ in the source distribution for its full text.
#define IGNORE_WCASTQUAL_END
#endif

#if defined(__clang__)
#define IGNORE_W11EXTENSIONS_BEGIN _Pragma("clang diagnostic push") \
_Pragma("clang diagnostic ignored \"-Wc11-extensions\"")
#define IGNORE_W11EXTENSIONS_END _Pragma("clang diagnostic pop")
#else
#define IGNORE_W11EXTENSIONS_BEGIN
#define IGNORE_W11EXTENSIONS_END
#endif

/* Cheaper function for checking NaNs. Unlike the standard isnan(), this may
throw an FP exception on a "signaling NaN".
(ISO/IEC TS 18661-1 and the C23 standard stated that isnan() throws no
Expand All @@ -142,4 +151,32 @@ static inline unsigned long long saturatingSub(unsigned long long a, unsigned lo
return a > b ? a - b : 0;
}

#ifdef HAVE_ALIGNAS
#include <stdalign.h>

#define ELF_NOTE_DLOPEN_OWNER "FDO"
#define ELF_NOTE_DLOPEN_TYPE UINT32_C(0x407c0c0a)

#define DECLARE_ELF_NOTE_DLOPEN(content) \
IGNORE_W11EXTENSIONS_BEGIN \
__attribute__((used, section(".note.dlopen"))) alignas(sizeof(uint32_t)) static const struct { \
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
__attribute__((used, section(".note.dlopen"))) alignas(sizeof(uint32_t)) static const struct { \
__attribute__((packed, used, section(".note.dlopen"))) static const struct { \

Just byte-align ourselves and round up with some little magic later

Copy link
Contributor

Choose a reason for hiding this comment

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

Another problem: C99 doesn't support anonymous struct yet.

Copy link
Member

Choose a reason for hiding this comment

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

Another problem: C99 doesn't support anonymous struct yet.

The workaround for that is kinda easy though, too …

struct { \
uint32_t n_namesz, n_descsz, n_type; \
} nhdr; \
char name[sizeof(ELF_NOTE_DLOPEN_OWNER)]; \
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
char name[sizeof(ELF_NOTE_DLOPEN_OWNER)]; \
char name[(sizeof(ELF_NOTE_DLOPEN_OWNER)+3)&(~3)]; \

alignas(sizeof(uint32_t)) char dlopen_content[sizeof(content)]; \
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
alignas(sizeof(uint32_t)) char dlopen_content[sizeof(content)]; \
char dlopen_content[(sizeof(content)+3)&(~3)]; \

Although one could argue if this last expansion is still required at the end of the attribute section.

} variable_name = { \
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't be variable_name be dynamic?

.nhdr = { \
.n_namesz = sizeof(ELF_NOTE_DLOPEN_OWNER), \
.n_descsz = sizeof(content), \
.n_type = ELF_NOTE_DLOPEN_TYPE, \
}, \
.name = ELF_NOTE_DLOPEN_OWNER, \
.dlopen_content = content, \
}; \
IGNORE_W11EXTENSIONS_END
#else
#define DECLARE_ELF_NOTE_DLOPEN()
Copy link
Member

Choose a reason for hiding this comment

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

Should name the content parameter for consistency.

#endif

#endif /* HEADER_Macros */
3 changes: 3 additions & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,9 @@ On most BSD systems `kvm` is a requirement to read kernel information.

More information on required and optional dependencies can be found in [configure.ac](configure.ac).

#### ELF Note
The optional runtime dependencies are also embedded in the binary via the ELF note `.note.dlopen`. See the [specification](https://systemd.io/ELF_DLOPEN_METADATA/) for details.

## Usage
See the manual page (`man htop`) or the help menu (**F1** or **h** inside `htop`) for a list of supported key commands.

Expand Down
12 changes: 12 additions & 0 deletions configure.ac
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,18 @@ AC_LINK_IFELSE([
[AC_MSG_RESULT(no)
AC_MSG_ERROR([can not find required macros: NAN, isgreater() and isgreaterequal()])])

AC_MSG_CHECKING(for alignas support)
AC_COMPILE_IFELSE([
AC_LANG_SOURCE(
[[
#include <stdalign.h>
alignas(128) char buffer[128];
]]
)],
AC_DEFINE([HAVE_ALIGNAS], 1, [The alignas C11 feature is supported.])
AC_MSG_RESULT(yes),
AC_MSG_RESULT(no))

Comment on lines +276 to +287
Copy link
Member

Choose a reason for hiding this comment

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

Cf. Macros.h for a way on how to do this without C11 support. If we throw around GCC/Clang compiler extensions, we also could just use packed, which at least is even kinda portable to other compilers too …

# ----------------------------------------------------------------------


Expand Down
6 changes: 6 additions & 0 deletions linux/LibNl.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,12 @@ static void unload_libnl(void) {
}

static int load_libnl(void) {

DECLARE_ELF_NOTE_DLOPEN("[{\"soname\":[\"libnl-3.so\",\"libnl-3.so.200\"],\"feature\":\"delay-accounting\"," \
"\"description:\":\"Enables delay accounting support\",\"priority\":\"recommended\"},{\"soname\":[" \
"\"libnl-genl-3.so\",\"libnl-genl-3.so.200\"],\"feature\":\"delay-accounting\",\"description:\":"\
"\"Enables delay accounting support\",\"priority\":\"recommended\"}]")
Comment on lines +79 to +82
Copy link
Member

Choose a reason for hiding this comment

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

Any objections to have this literal span multiple lines for better maintainability?

Please add proper meta-commands for astyle to leave this block alone afterwards.

Also, this note probably should only contain the optional dependencies we actually have enabled; i.e. if configured without delay accounting, there's no need to declare this as an optional runtime dependency if we would never try to load that library anyway.

Suggested change
DECLARE_ELF_NOTE_DLOPEN("[{\"soname\":[\"libnl-3.so\",\"libnl-3.so.200\"],\"feature\":\"delay-accounting\"," \
"\"description:\":\"Enables delay accounting support\",\"priority\":\"recommended\"},{\"soname\":[" \
"\"libnl-genl-3.so\",\"libnl-genl-3.so.200\"],\"feature\":\"delay-accounting\",\"description:\":"\
"\"Enables delay accounting support\",\"priority\":\"recommended\"}]")
DECLARE_ELF_NOTE_DLOPEN( "["\
"{"\
"\"soname\":[\"libnl-3.so\",\"libnl-3.so.200\"],"\
"\"feature\":\"delay-accounting\"," \
"\"description:\":\"Enables delay accounting support\","\
"\"priority\":\"recommended\""\
"},"\
"{"\
"\"soname\":[\"libnl-genl-3.so\",\"libnl-genl-3.so.200\"],"\
"\"feature\":\"delay-accounting\","\
"\"description:\":\"Enables delay accounting support\","\
"\"priority\":\"recommended\""\
"}"\
"]" )

Copy link
Contributor

@Explorer09 Explorer09 May 19, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I'm not a fan of having a full JSON syntax in a macro caller. Can we rework the macros so that only important fields will be used as parameters?

I expect the usage would ideally work like this:

#define ELF_NOTE_DLOPEN_DEP(sonames, feature_tag, priority) ...
// Example
ELF_NOTE_DLOPEN_DEP(
   "[\"libnl-3.so\",\"libnl-3.so.200\"]", "delay-accounting", "recommended")
ELF_NOTE_DLOPEN_DEP(
   "[\"libnl-genl-3.so\",\"libnl-genl-3.so.200\"]", "delay-accounting", "recommended")

How do you think?

Copy link
Member

Choose a reason for hiding this comment

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

While I also thought about this, including making generating the JSON be split into different macro calls (definitively possible AFAICS), there's one minor issue with the pre-processor IMHO, that prevents this from being really nice syntax. The issue is, that you can't properly auto-terminate lists of arguments to insert the semicolon in list contexts, thus you'd most likely have to resort to something like "[" ELF_NOTE_DEP(foo…) "," ELF_NOTE_DEP(bar…) "]"; which doesn't look too terrible, but in a really great world could work without the explicit "," token in between.

Also from a practcal PoV, I'd prefer such macros to go ELF_NOTE_DLOPEN_DEP(freature, level, description, solist)`, but comparibly that's a minor change to implement.


if (libnlHandle && libnlGenlHandle)
return 0;

Expand Down
3 changes: 3 additions & 0 deletions linux/LibSensors.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,9 @@ int LibSensors_init(void) {

#else

DECLARE_ELF_NOTE_DLOPEN("[{\"soname\":[\"libsensors.so\",\"libsensors.so.5\",\"libsensors.so.4\"],\"feature\":"\
"\"sensors\",\"description:\":\"Enables hardware sensor support\",\"priority\":\"recommended\"}]")
Comment on lines +59 to +60
Copy link
Member

Choose a reason for hiding this comment

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

Multiple lines, example above …


if (!dlopenHandle) {
/* Find the unversioned libsensors.so (symlink) and prefer that, but Debian has .so.5 and Fedora .so.4 without
matching symlinks (unless people install the -dev packages) */
Expand Down
3 changes: 3 additions & 0 deletions linux/SystemdMeter.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,9 @@ static void SystemdMeter_done(ATTR_UNUSED Meter* this) {
static int updateViaLib(bool user) {
SystemdMeterContext_t* ctx = user ? &ctx_user : &ctx_system;
#ifndef BUILD_STATIC
DECLARE_ELF_NOTE_DLOPEN("[{\"soname\":[\"libsystemd.so.0\"],\"feature\":\"systemd\",\"description:\":"\
"\"Enables systemd support\",\"priority\":\"suggested\"}]")
Comment on lines +101 to +102
Copy link
Member

Choose a reason for hiding this comment

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

Multiple lines, example above …


if (!dlopenHandle) {
dlopenHandle = dlopen("libsystemd.so.0", RTLD_LAZY);
if (!dlopenHandle)
Expand Down
Loading