Re: [PATCH] BUG/MEDIUM: tools: fix build with static only toolchains

2020-07-24 Thread Willy Tarreau
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

2020-07-24 Thread Baruch Siach
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

2020-07-24 Thread Willy Tarreau
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

2020-07-23 Thread Willy Tarreau
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

2020-07-23 Thread Baruch Siach
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