Re: [PATCH] BUG/MEDIUM: tools: fix build with static only toolchains
Hi Baruch, On Fri, Jul 24, 2020 at 01:05:54PM +0300, Baruch Siach wrote: > I can confirm that the patch below also fixes the same build issue. Great, thank you! I've updated your first patch to use this one instead, adjusted the commit message accordingly, and marked it for backport to 2.1. Thanks! Willy
Re: [PATCH] BUG/MEDIUM: tools: fix build with static only toolchains
Hi Willy, On Fri, Jul 24 2020, Willy Tarreau wrote: > On Fri, Jul 24, 2020 at 07:40:01AM +0200, Willy Tarreau wrote: >> On Fri, Jul 24, 2020 at 07:52:20AM +0300, Baruch Siach wrote: >> > uClibc toolchains built with no dynamic library support don't provide >> > the dlfcn.h header. That leads to build failure: >> > >> > CC src/tools.o >> > src/tools.c:15:10: fatal error: dlfcn.h: No such file or directory >> > #include >> > ^ >> > Enable dladdr only when USE_DL is defined. >> >> Oh thank you, I've encountered it as well a few days ago while working >> on something else and forgot to note it on my todo list! However, I'm >> looking at the original commit which introduced this and it's almost a >> revert. We used to rely on USE_DL and turned it into __ELF__ because >> some platforms don't need USE_DL. >> >> I wanted to turn to sets of WANT/NEED/HAVE instead of the generic USE, >> and I think this one will be one of the first benefiting from this. I'll >> have a look at this. > > In order to make something less intrusive that may be backported to 2.2 > and 2.1, what do you think about this: > > -#ifdef __ELF__ > +#if (defined(__ELF__) && !defined(__linux__)) || defined(USE_DL) > > => __ELF__ is enough on all non-linux platforms >USE_DL is required on linux > > This would not change much from your patch and limit the scope of changes > for stable versions. If you can confirm that it works for you, I can adapt > your patch to do that and commit it (unless you prefer to send a v2, of > course). I can confirm that the patch below also fixes the same build issue. Thanks, baruch diff --git a/src/tools.c b/src/tools.c index 1c664852ad73..21bb1efbeeb0 100644 --- a/src/tools.c +++ b/src/tools.c @@ -10,7 +10,7 @@ * */ -#ifdef __ELF__ +#if (defined(__ELF__) && !defined(__linux__)) || defined(USE_DL) #define _GNU_SOURCE #include #include @@ -4410,7 +4410,7 @@ const char *get_exec_path() return ret; } -#ifdef __ELF__ +#if (defined(__ELF__) && !defined(__linux__)) || defined(USE_DL) /* calls dladdr() or dladdr1() on and . If dladdr1 is available, * also returns the symbol size in , otherwise returns 0 there. */ @@ -,7 +,7 @@ static int dladdr_and_size(const void *addr, Dl_info *dli, size_t *size) * The file name (lib or executable) is limited to what lies between the last * '/' and the first following '.'. An optional prefix is prepended before * the output if not null. The file is not dumped when it's the same as the one - * that contains the "main" symbol, or when __ELF__ is not set. + * that contains the "main" symbol, or when __ELF__ && USE_DL are not set. * * The symbol's base address is returned, or NULL when unresolved, in order to * allow the caller to match it against known ones. @@ -4472,7 +4472,7 @@ const void *resolve_sym_name(struct buffer *buf, const char *pfx, void *addr) #endif }; -#ifdef __ELF__ +#if (defined(__ELF__) && !defined(__linux__)) || defined(USE_DL) Dl_info dli, dli_main; size_t size; const char *fname, *p; @@ -4489,7 +4489,7 @@ const void *resolve_sym_name(struct buffer *buf, const char *pfx, void *addr) } } -#ifdef __ELF__ +#if (defined(__ELF__) && !defined(__linux__)) || defined(USE_DL) /* Now let's try to be smarter */ if (!dladdr_and_size(addr, , )) goto unknown; @@ -4529,7 +4529,7 @@ const void *resolve_sym_name(struct buffer *buf, const char *pfx, void *addr) chunk_appendf(buf, "+%#lx", (long)(addr - dli.dli_fbase)); return NULL; } -#endif /* __ELF__ */ +#endif /* __ELF__ && USE_DL */ unknown: /* unresolved symbol from the main file, report relative offset to main */ if ((void*)addr < (void*)main) -- ~. .~ Tk Open Systems =}ooO--U--Ooo{= - bar...@tkos.co.il - tel: +972.52.368.4656, http://www.tkos.co.il -
Re: [PATCH] BUG/MEDIUM: tools: fix build with static only toolchains
On Fri, Jul 24, 2020 at 07:40:01AM +0200, Willy Tarreau wrote: > On Fri, Jul 24, 2020 at 07:52:20AM +0300, Baruch Siach wrote: > > uClibc toolchains built with no dynamic library support don't provide > > the dlfcn.h header. That leads to build failure: > > > > CC src/tools.o > > src/tools.c:15:10: fatal error: dlfcn.h: No such file or directory > > #include > > ^ > > Enable dladdr only when USE_DL is defined. > > Oh thank you, I've encountered it as well a few days ago while working > on something else and forgot to note it on my todo list! However, I'm > looking at the original commit which introduced this and it's almost a > revert. We used to rely on USE_DL and turned it into __ELF__ because > some platforms don't need USE_DL. > > I wanted to turn to sets of WANT/NEED/HAVE instead of the generic USE, > and I think this one will be one of the first benefiting from this. I'll > have a look at this. In order to make something less intrusive that may be backported to 2.2 and 2.1, what do you think about this: -#ifdef __ELF__ +#if (defined(__ELF__) && !defined(__linux__)) || defined(USE_DL) => __ELF__ is enough on all non-linux platforms USE_DL is required on linux This would not change much from your patch and limit the scope of changes for stable versions. If you can confirm that it works for you, I can adapt your patch to do that and commit it (unless you prefer to send a v2, of course). Thanks, Willy
Re: [PATCH] BUG/MEDIUM: tools: fix build with static only toolchains
On Fri, Jul 24, 2020 at 07:52:20AM +0300, Baruch Siach wrote: > uClibc toolchains built with no dynamic library support don't provide > the dlfcn.h header. That leads to build failure: > > CC src/tools.o > src/tools.c:15:10: fatal error: dlfcn.h: No such file or directory > #include > ^ > Enable dladdr only when USE_DL is defined. Oh thank you, I've encountered it as well a few days ago while working on something else and forgot to note it on my todo list! However, I'm looking at the original commit which introduced this and it's almost a revert. We used to rely on USE_DL and turned it into __ELF__ because some platforms don't need USE_DL. I wanted to turn to sets of WANT/NEED/HAVE instead of the generic USE, and I think this one will be one of the first benefiting from this. I'll have a look at this. Thanks! Willy
[PATCH] BUG/MEDIUM: tools: fix build with static only toolchains
uClibc toolchains built with no dynamic library support don't provide the dlfcn.h header. That leads to build failure: CC src/tools.o src/tools.c:15:10: fatal error: dlfcn.h: No such file or directory #include ^ Enable dladdr only when USE_DL is defined. --- Please Cc me on replies. I'm not subscribed. --- src/tools.c | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/tools.c b/src/tools.c index 1c664852ad73..0bd80f846d05 100644 --- a/src/tools.c +++ b/src/tools.c @@ -10,7 +10,7 @@ * */ -#ifdef __ELF__ +#if defined(__ELF__) && defined(USE_DL) #define _GNU_SOURCE #include #include @@ -4410,7 +4410,7 @@ const char *get_exec_path() return ret; } -#ifdef __ELF__ +#if defined(__ELF__) && defined(USE_DL) /* calls dladdr() or dladdr1() on and . If dladdr1 is available, * also returns the symbol size in , otherwise returns 0 there. */ @@ -,7 +,7 @@ static int dladdr_and_size(const void *addr, Dl_info *dli, size_t *size) * The file name (lib or executable) is limited to what lies between the last * '/' and the first following '.'. An optional prefix is prepended before * the output if not null. The file is not dumped when it's the same as the one - * that contains the "main" symbol, or when __ELF__ is not set. + * that contains the "main" symbol, or when __ELF__ && USE_DL are not set. * * The symbol's base address is returned, or NULL when unresolved, in order to * allow the caller to match it against known ones. @@ -4472,7 +4472,7 @@ const void *resolve_sym_name(struct buffer *buf, const char *pfx, void *addr) #endif }; -#ifdef __ELF__ +#if defined(__ELF__) && defined(USE_DL) Dl_info dli, dli_main; size_t size; const char *fname, *p; @@ -4489,7 +4489,7 @@ const void *resolve_sym_name(struct buffer *buf, const char *pfx, void *addr) } } -#ifdef __ELF__ +#if defined(__ELF__) && defined(USE_DL) /* Now let's try to be smarter */ if (!dladdr_and_size(addr, , )) goto unknown; @@ -4529,7 +4529,7 @@ const void *resolve_sym_name(struct buffer *buf, const char *pfx, void *addr) chunk_appendf(buf, "+%#lx", (long)(addr - dli.dli_fbase)); return NULL; } -#endif /* __ELF__ */ +#endif /* __ELF__ && USE_DL */ unknown: /* unresolved symbol from the main file, report relative offset to main */ if ((void*)addr < (void*)main) -- 2.27.0