All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] glibc: add -fno-builtin-strlen when not using -O2
@ 2016-12-12  5:42 jackie.huang
  2016-12-12  6:37 ` Khem Raj
  0 siblings, 1 reply; 9+ messages in thread
From: jackie.huang @ 2016-12-12  5:42 UTC (permalink / raw)
  To: openembedded-core

From: Jackie Huang <jackie.huang@windriver.com>

The strlen will be inlined when compile with -O, -O1 or -Os,
so there is no symbol for strlen in ld-linux-x86-64.so.2,
causing a fatal error in valgrind:

valgrind: Fatal error at startup: a function redirection
valgrind: which is mandatory for this platform-tool combination
valgrind: cannot be set up. Details of the redirection are:
valgrind:
valgrind: A must-be-redirected function
valgrind: whose name matches the pattern: strlen
valgrind: in an object with soname matching: ld-linux-x86-64.so.2

so add -fno-builtin-strlen when compile with -O, -O1 or -Os.

Signed-off-by: Jackie Huang <jackie.huang@windriver.com>
---
 meta/recipes-core/glibc/glibc.inc | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/meta/recipes-core/glibc/glibc.inc b/meta/recipes-core/glibc/glibc.inc
index e85c704..7f3e0f6 100644
--- a/meta/recipes-core/glibc/glibc.inc
+++ b/meta/recipes-core/glibc/glibc.inc
@@ -16,8 +16,8 @@ python () {
     if opt_effective == "-O0":
         bb.fatal("%s can't be built with %s, try -O1 instead" % (d.getVar('PN', True), opt_effective))
     if opt_effective in ("-O", "-O1", "-Os"):
-        bb.note("%s doesn't build cleanly with %s, adding -Wno-error to SELECTED_OPTIMIZATION" % (d.getVar('PN', True), opt_effective))
-        d.appendVar("SELECTED_OPTIMIZATION", " -Wno-error")
+        bb.note("%s doesn't build cleanly with %s, adding -Wno-error and -fno-builtin-strlen to SELECTED_OPTIMIZATION" % (d.getVar('PN', True), opt_effective))
+        d.appendVar("SELECTED_OPTIMIZATION", " -Wno-error -fno-builtin-strlen")
 }
 
 # siteconfig.bbclass runs configure which needs a working compiler
-- 
2.8.3



^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] glibc: add -fno-builtin-strlen when not using -O2
  2016-12-12  5:42 [PATCH] glibc: add -fno-builtin-strlen when not using -O2 jackie.huang
@ 2016-12-12  6:37 ` Khem Raj
  2016-12-12  7:41   ` Huang, Jie (Jackie)
  0 siblings, 1 reply; 9+ messages in thread
From: Khem Raj @ 2016-12-12  6:37 UTC (permalink / raw)
  To: jhuang0; +Cc: Patches and discussions about the oe-core layer

On Sun, Dec 11, 2016 at 9:42 PM,  <jackie.huang@windriver.com> wrote:
> From: Jackie Huang <jackie.huang@windriver.com>
>
> The strlen will be inlined when compile with -O, -O1 or -Os,
> so there is no symbol for strlen in ld-linux-x86-64.so.2,
> causing a fatal error in valgrind:
>
> valgrind: Fatal error at startup: a function redirection
> valgrind: which is mandatory for this platform-tool combination
> valgrind: cannot be set up. Details of the redirection are:
> valgrind:
> valgrind: A must-be-redirected function
> valgrind: whose name matches the pattern: strlen
> valgrind: in an object with soname matching: ld-linux-x86-64.so.2
>
> so add -fno-builtin-strlen when compile with -O, -O1 or -Os.

This is a bug in valgrind, I read the same on forums. KDE has proposed a fix
https://bugsfiles.kde.org/attachment.cgi?id=82043
can you check if this fixes the issue

>
> Signed-off-by: Jackie Huang <jackie.huang@windriver.com>
> ---
>  meta/recipes-core/glibc/glibc.inc | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/meta/recipes-core/glibc/glibc.inc b/meta/recipes-core/glibc/glibc.inc
> index e85c704..7f3e0f6 100644
> --- a/meta/recipes-core/glibc/glibc.inc
> +++ b/meta/recipes-core/glibc/glibc.inc
> @@ -16,8 +16,8 @@ python () {
>      if opt_effective == "-O0":
>          bb.fatal("%s can't be built with %s, try -O1 instead" % (d.getVar('PN', True), opt_effective))
>      if opt_effective in ("-O", "-O1", "-Os"):
> -        bb.note("%s doesn't build cleanly with %s, adding -Wno-error to SELECTED_OPTIMIZATION" % (d.getVar('PN', True), opt_effective))
> -        d.appendVar("SELECTED_OPTIMIZATION", " -Wno-error")
> +        bb.note("%s doesn't build cleanly with %s, adding -Wno-error and -fno-builtin-strlen to SELECTED_OPTIMIZATION" % (d.getVar('PN', True), opt_effective))
> +        d.appendVar("SELECTED_OPTIMIZATION", " -Wno-error -fno-builtin-strlen")
>  }
>
>  # siteconfig.bbclass runs configure which needs a working compiler
> --
> 2.8.3
>
> --
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.openembedded.org/mailman/listinfo/openembedded-core


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] glibc: add -fno-builtin-strlen when not using -O2
  2016-12-12  6:37 ` Khem Raj
@ 2016-12-12  7:41   ` Huang, Jie (Jackie)
  2016-12-12  9:54     ` Andre McCurdy
  0 siblings, 1 reply; 9+ messages in thread
From: Huang, Jie (Jackie) @ 2016-12-12  7:41 UTC (permalink / raw)
  To: Khem Raj; +Cc: Patches and discussions about the oe-core layer



> -----Original Message-----
> From: Khem Raj [mailto:raj.khem@gmail.com]
> Sent: Monday, December 12, 2016 2:37 PM
> To: Huang, Jie (Jackie)
> Cc: Patches and discussions about the oe-core layer
> Subject: Re: [OE-core] [PATCH] glibc: add -fno-builtin-strlen when not using -O2
> 
> On Sun, Dec 11, 2016 at 9:42 PM,  <jackie.huang@windriver.com> wrote:
> > From: Jackie Huang <jackie.huang@windriver.com>
> >
> > The strlen will be inlined when compile with -O, -O1 or -Os,
> > so there is no symbol for strlen in ld-linux-x86-64.so.2,
> > causing a fatal error in valgrind:
> >
> > valgrind: Fatal error at startup: a function redirection
> > valgrind: which is mandatory for this platform-tool combination
> > valgrind: cannot be set up. Details of the redirection are:
> > valgrind:
> > valgrind: A must-be-redirected function
> > valgrind: whose name matches the pattern: strlen
> > valgrind: in an object with soname matching: ld-linux-x86-64.so.2
> >
> > so add -fno-builtin-strlen when compile with -O, -O1 or -Os.
> 
> This is a bug in valgrind, I read the same on forums. KDE has proposed a fix
> https://bugsfiles.kde.org/attachment.cgi?id=82043
> can you check if this fixes the issue

Thanks, I will check that and change to patch for valgrind if it works.

Thanks,
Jackie

> 
> >
> > Signed-off-by: Jackie Huang <jackie.huang@windriver.com>
> > ---
> >  meta/recipes-core/glibc/glibc.inc | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/meta/recipes-core/glibc/glibc.inc b/meta/recipes-core/glibc/glibc.inc
> > index e85c704..7f3e0f6 100644
> > --- a/meta/recipes-core/glibc/glibc.inc
> > +++ b/meta/recipes-core/glibc/glibc.inc
> > @@ -16,8 +16,8 @@ python () {
> >      if opt_effective == "-O0":
> >          bb.fatal("%s can't be built with %s, try -O1 instead" % (d.getVar('PN', True), opt_effective))
> >      if opt_effective in ("-O", "-O1", "-Os"):
> > -        bb.note("%s doesn't build cleanly with %s, adding -Wno-error to SELECTED_OPTIMIZATION" %
> (d.getVar('PN', True), opt_effective))
> > -        d.appendVar("SELECTED_OPTIMIZATION", " -Wno-error")
> > +        bb.note("%s doesn't build cleanly with %s, adding -Wno-error and -fno-builtin-strlen to
> SELECTED_OPTIMIZATION" % (d.getVar('PN', True), opt_effective))
> > +        d.appendVar("SELECTED_OPTIMIZATION", " -Wno-error -fno-builtin-strlen")
> >  }
> >
> >  # siteconfig.bbclass runs configure which needs a working compiler
> > --
> > 2.8.3
> >
> > --
> > _______________________________________________
> > Openembedded-core mailing list
> > Openembedded-core@lists.openembedded.org
> > http://lists.openembedded.org/mailman/listinfo/openembedded-core

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] glibc: add -fno-builtin-strlen when not using -O2
  2016-12-12  7:41   ` Huang, Jie (Jackie)
@ 2016-12-12  9:54     ` Andre McCurdy
  2016-12-13  5:14       ` Huang, Jie (Jackie)
  0 siblings, 1 reply; 9+ messages in thread
From: Andre McCurdy @ 2016-12-12  9:54 UTC (permalink / raw)
  To: Huang, Jie (Jackie); +Cc: Patches and discussions about the oe-core layer

On Sun, Dec 11, 2016 at 11:41 PM, Huang, Jie (Jackie)
<Jackie.Huang@windriver.com> wrote:
>> -----Original Message-----
>> From: Khem Raj [mailto:raj.khem@gmail.com]
>> Sent: Monday, December 12, 2016 2:37 PM
>> To: Huang, Jie (Jackie)
>> Cc: Patches and discussions about the oe-core layer
>> Subject: Re: [OE-core] [PATCH] glibc: add -fno-builtin-strlen when not using -O2
>>
>> On Sun, Dec 11, 2016 at 9:42 PM,  <jackie.huang@windriver.com> wrote:
>> > From: Jackie Huang <jackie.huang@windriver.com>
>> >
>> > The strlen will be inlined when compile with -O, -O1 or -Os,
>> > so there is no symbol for strlen in ld-linux-x86-64.so.2,
>> > causing a fatal error in valgrind:
>> >
>> > valgrind: Fatal error at startup: a function redirection
>> > valgrind: which is mandatory for this platform-tool combination
>> > valgrind: cannot be set up. Details of the redirection are:
>> > valgrind:
>> > valgrind: A must-be-redirected function
>> > valgrind: whose name matches the pattern: strlen
>> > valgrind: in an object with soname matching: ld-linux-x86-64.so.2
>> >
>> > so add -fno-builtin-strlen when compile with -O, -O1 or -Os.
>>
>> This is a bug in valgrind, I read the same on forums. KDE has proposed a fix
>> https://bugsfiles.kde.org/attachment.cgi?id=82043
>> can you check if this fixes the issue
>
> Thanks, I will check that and change to patch for valgrind if it works.

For reference, here's the patch I've been using. It's a slightly more
generic fix than the one in the KDE bug report.

diff --git a/coregrind/m_redir.c b/coregrind/m_redir.c
index 7e4df8d..640a346 100644
--- a/coregrind/m_redir.c
+++ b/coregrind/m_redir.c
@@ -1220,7 +1220,18 @@ static void add_hardwired_spec (const  HChar*
sopatt, const HChar* fnpatt,
    spec->from_fnpatt = CONST_CAST(HChar *,fnpatt);
    spec->to_addr     = to_addr;
    spec->isWrap      = False;
-   spec->mandatory   = mandatory;
+
+   /* Hack: Depending on how glibc was compiled (e.g. optimised for size or
+      built with _FORTIFY_SOURCE enabled) the strlen symbol might not be found.
+      Therefore although we should still try to intercept it, don't make it
+      mandatory to do so. We over-ride "mandatory" here to avoid the need to
+      patch the many different architecture specific callers to
+      add_hardwired_spec(). */
+   if (0==VG_(strcmp)("strlen", fnpatt))
+      spec->mandatory = NULL;
+   else
+      spec->mandatory = mandatory;
+
    /* VARIABLE PARTS */
    spec->mark        = False; /* not significant */
    spec->done        = False; /* not significant */


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] glibc: add -fno-builtin-strlen when not using -O2
  2016-12-12  9:54     ` Andre McCurdy
@ 2016-12-13  5:14       ` Huang, Jie (Jackie)
  2016-12-13  9:16         ` Andre McCurdy
  0 siblings, 1 reply; 9+ messages in thread
From: Huang, Jie (Jackie) @ 2016-12-13  5:14 UTC (permalink / raw)
  To: Andre McCurdy; +Cc: Patches and discussions about the oe-core layer



> -----Original Message-----
> From: Andre McCurdy [mailto:armccurdy@gmail.com]
> Sent: Monday, December 12, 2016 5:55 PM
> To: Huang, Jie (Jackie)
> Cc: Khem Raj; Patches and discussions about the oe-core layer
> Subject: Re: [OE-core] [PATCH] glibc: add -fno-builtin-strlen when not using -O2
> 
> On Sun, Dec 11, 2016 at 11:41 PM, Huang, Jie (Jackie)
> <Jackie.Huang@windriver.com> wrote:
> >> -----Original Message-----
> >> From: Khem Raj [mailto:raj.khem@gmail.com]
> >> Sent: Monday, December 12, 2016 2:37 PM
> >> To: Huang, Jie (Jackie)
> >> Cc: Patches and discussions about the oe-core layer
> >> Subject: Re: [OE-core] [PATCH] glibc: add -fno-builtin-strlen when not using -O2
> >>
> >> On Sun, Dec 11, 2016 at 9:42 PM,  <jackie.huang@windriver.com> wrote:
> >> > From: Jackie Huang <jackie.huang@windriver.com>
> >> >
> >> > The strlen will be inlined when compile with -O, -O1 or -Os,
> >> > so there is no symbol for strlen in ld-linux-x86-64.so.2,
> >> > causing a fatal error in valgrind:
> >> >
> >> > valgrind: Fatal error at startup: a function redirection
> >> > valgrind: which is mandatory for this platform-tool combination
> >> > valgrind: cannot be set up. Details of the redirection are:
> >> > valgrind:
> >> > valgrind: A must-be-redirected function
> >> > valgrind: whose name matches the pattern: strlen
> >> > valgrind: in an object with soname matching: ld-linux-x86-64.so.2
> >> >
> >> > so add -fno-builtin-strlen when compile with -O, -O1 or -Os.
> >>
> >> This is a bug in valgrind, I read the same on forums. KDE has proposed a fix
> >> https://bugsfiles.kde.org/attachment.cgi?id=82043
> >> can you check if this fixes the issue
> >
> > Thanks, I will check that and change to patch for valgrind if it works.
> 
> For reference, here's the patch I've been using. It's a slightly more
> generic fix than the one in the KDE bug report.

Thanks, It's a better patch and I will take it and send as v2 of this issue if you're
not going to send it yourself, is it fine for you and could you provide extra info
for the patch header like, upstream-status, written by or Signed-off-by?

Thanks,
Jackie

> 
> diff --git a/coregrind/m_redir.c b/coregrind/m_redir.c
> index 7e4df8d..640a346 100644
> --- a/coregrind/m_redir.c
> +++ b/coregrind/m_redir.c
> @@ -1220,7 +1220,18 @@ static void add_hardwired_spec (const  HChar*
> sopatt, const HChar* fnpatt,
>     spec->from_fnpatt = CONST_CAST(HChar *,fnpatt);
>     spec->to_addr     = to_addr;
>     spec->isWrap      = False;
> -   spec->mandatory   = mandatory;
> +
> +   /* Hack: Depending on how glibc was compiled (e.g. optimised for size or
> +      built with _FORTIFY_SOURCE enabled) the strlen symbol might not be found.
> +      Therefore although we should still try to intercept it, don't make it
> +      mandatory to do so. We over-ride "mandatory" here to avoid the need to
> +      patch the many different architecture specific callers to
> +      add_hardwired_spec(). */
> +   if (0==VG_(strcmp)("strlen", fnpatt))
> +      spec->mandatory = NULL;
> +   else
> +      spec->mandatory = mandatory;
> +
>     /* VARIABLE PARTS */
>     spec->mark        = False; /* not significant */
>     spec->done        = False; /* not significant */

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] glibc: add -fno-builtin-strlen when not using -O2
  2016-12-13  5:14       ` Huang, Jie (Jackie)
@ 2016-12-13  9:16         ` Andre McCurdy
  2016-12-14  2:38           ` Huang, Jie (Jackie)
  2016-12-15  1:04           ` Randy MacLeod
  0 siblings, 2 replies; 9+ messages in thread
From: Andre McCurdy @ 2016-12-13  9:16 UTC (permalink / raw)
  To: Huang, Jie (Jackie); +Cc: Patches and discussions about the oe-core layer

On Mon, Dec 12, 2016 at 9:14 PM, Huang, Jie (Jackie)
<Jackie.Huang@windriver.com> wrote:
>> From: Andre McCurdy [mailto:armccurdy@gmail.com]
>> For reference, here's the patch I've been using. It's a slightly more
>> generic fix than the one in the KDE bug report.
>
> Thanks, It's a better patch and I will take it and send as v2 of this issue if you're
> not going to send it yourself, is it fine for you and could you provide extra info
> for the patch header like, upstream-status, written by or Signed-off-by?

Sure. I forget why I didn't submit this at the time. The full patch is:

From d34e2a50ca5493f5a0ce9ccad83a36ac33689266 Mon Sep 17 00:00:00 2001
From: Andre McCurdy <armccurdy@gmail.com>
Date: Fri, 12 Feb 2016 18:22:12 -0800
Subject: [PATCH] make ld-XXX.so strlen intercept optional

Hack: Depending on how glibc was compiled (e.g. optimised for size or
built with _FORTIFY_SOURCE enabled) the strlen symbol might not be
found in ld-XXX.so. Therefore although we should still try to
intercept it, don't make it mandatory to do so.

Upstream-Status: Inappropriate

Signed-off-by: Andre McCurdy <armccurdy@gmail.com>
---
 coregrind/m_redir.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/coregrind/m_redir.c b/coregrind/m_redir.c
index 7e4df8d..640a346 100644
--- a/coregrind/m_redir.c
+++ b/coregrind/m_redir.c
@@ -1220,7 +1220,18 @@ static void add_hardwired_spec (const  HChar*
sopatt, const HChar* fnpatt,
    spec->from_fnpatt = CONST_CAST(HChar *,fnpatt);
    spec->to_addr     = to_addr;
    spec->isWrap      = False;
-   spec->mandatory   = mandatory;
+
+   /* Hack: Depending on how glibc was compiled (e.g. optimised for size or
+      built with _FORTIFY_SOURCE enabled) the strlen symbol might not be found.
+      Therefore although we should still try to intercept it, don't make it
+      mandatory to do so. We over-ride "mandatory" here to avoid the need to
+      patch the many different architecture specific callers to
+      add_hardwired_spec(). */
+   if (0==VG_(strcmp)("strlen", fnpatt))
+      spec->mandatory = NULL;
+   else
+      spec->mandatory = mandatory;
+
    /* VARIABLE PARTS */
    spec->mark        = False; /* not significant */
    spec->done        = False; /* not significant */
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] glibc: add -fno-builtin-strlen when not using -O2
  2016-12-13  9:16         ` Andre McCurdy
@ 2016-12-14  2:38           ` Huang, Jie (Jackie)
  2016-12-15  1:04           ` Randy MacLeod
  1 sibling, 0 replies; 9+ messages in thread
From: Huang, Jie (Jackie) @ 2016-12-14  2:38 UTC (permalink / raw)
  To: Andre McCurdy; +Cc: Patches and discussions about the oe-core layer



> -----Original Message-----
> From: Andre McCurdy [mailto:armccurdy@gmail.com]
> Sent: Tuesday, December 13, 2016 5:17 PM
> To: Huang, Jie (Jackie)
> Cc: Khem Raj; Patches and discussions about the oe-core layer
> Subject: Re: [OE-core] [PATCH] glibc: add -fno-builtin-strlen when not using -O2
> 
> On Mon, Dec 12, 2016 at 9:14 PM, Huang, Jie (Jackie)
> <Jackie.Huang@windriver.com> wrote:
> >> From: Andre McCurdy [mailto:armccurdy@gmail.com]
> >> For reference, here's the patch I've been using. It's a slightly more
> >> generic fix than the one in the KDE bug report.
> >
> > Thanks, It's a better patch and I will take it and send as v2 of this issue if you're
> > not going to send it yourself, is it fine for you and could you provide extra info
> > for the patch header like, upstream-status, written by or Signed-off-by?
> 
> Sure. I forget why I didn't submit this at the time. The full patch is:

Thanks, I rebased the patch since valgrind updated  and sent v2 for this issue:

[OE-core] [v2][PATCH] valgrind: make ld-XXX.so strlen intercept optional

Thanks,
Jackie


> 
> From d34e2a50ca5493f5a0ce9ccad83a36ac33689266 Mon Sep 17 00:00:00 2001
> From: Andre McCurdy <armccurdy@gmail.com>
> Date: Fri, 12 Feb 2016 18:22:12 -0800
> Subject: [PATCH] make ld-XXX.so strlen intercept optional
> 
> Hack: Depending on how glibc was compiled (e.g. optimised for size or
> built with _FORTIFY_SOURCE enabled) the strlen symbol might not be
> found in ld-XXX.so. Therefore although we should still try to
> intercept it, don't make it mandatory to do so.
> 
> Upstream-Status: Inappropriate
> 
> Signed-off-by: Andre McCurdy <armccurdy@gmail.com>
> ---
>  coregrind/m_redir.c | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/coregrind/m_redir.c b/coregrind/m_redir.c
> index 7e4df8d..640a346 100644
> --- a/coregrind/m_redir.c
> +++ b/coregrind/m_redir.c
> @@ -1220,7 +1220,18 @@ static void add_hardwired_spec (const  HChar*
> sopatt, const HChar* fnpatt,
>     spec->from_fnpatt = CONST_CAST(HChar *,fnpatt);
>     spec->to_addr     = to_addr;
>     spec->isWrap      = False;
> -   spec->mandatory   = mandatory;
> +
> +   /* Hack: Depending on how glibc was compiled (e.g. optimised for size or
> +      built with _FORTIFY_SOURCE enabled) the strlen symbol might not be found.
> +      Therefore although we should still try to intercept it, don't make it
> +      mandatory to do so. We over-ride "mandatory" here to avoid the need to
> +      patch the many different architecture specific callers to
> +      add_hardwired_spec(). */
> +   if (0==VG_(strcmp)("strlen", fnpatt))
> +      spec->mandatory = NULL;
> +   else
> +      spec->mandatory = mandatory;
> +
>     /* VARIABLE PARTS */
>     spec->mark        = False; /* not significant */
>     spec->done        = False; /* not significant */
> --
> 1.9.1

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] glibc: add -fno-builtin-strlen when not using -O2
  2016-12-13  9:16         ` Andre McCurdy
  2016-12-14  2:38           ` Huang, Jie (Jackie)
@ 2016-12-15  1:04           ` Randy MacLeod
  2016-12-15  9:28             ` Andre McCurdy
  1 sibling, 1 reply; 9+ messages in thread
From: Randy MacLeod @ 2016-12-15  1:04 UTC (permalink / raw)
  To: Andre McCurdy, Huang, Jie (Jackie)
  Cc: Patches and discussions about the oe-core layer

On 2016-12-13 04:16 AM, Andre McCurdy wrote:
> On Mon, Dec 12, 2016 at 9:14 PM, Huang, Jie (Jackie)
> <Jackie.Huang@windriver.com> wrote:
>>> From: Andre McCurdy [mailto:armccurdy@gmail.com]
>>> For reference, here's the patch I've been using. It's a slightly more
>>> generic fix than the one in the KDE bug report.
>>
>> Thanks, It's a better patch and I will take it and send as v2 of this issue if you're
>> not going to send it yourself, is it fine for you and could you provide extra info
>> for the patch header like, upstream-status, written by or Signed-off-by?
>
> Sure. I forget why I didn't submit this at the time. The full patch is:
>
>>From d34e2a50ca5493f5a0ce9ccad83a36ac33689266 Mon Sep 17 00:00:00 2001
> From: Andre McCurdy <armccurdy@gmail.com>
> Date: Fri, 12 Feb 2016 18:22:12 -0800
> Subject: [PATCH] make ld-XXX.so strlen intercept optional
>
> Hack: Depending on how glibc was compiled (e.g. optimised for size or
> built with _FORTIFY_SOURCE enabled) the strlen symbol might not be
> found in ld-XXX.so. Therefore although we should still try to
> intercept it, don't make it mandatory to do so.
>
> Upstream-Status: Inappropriate

Can you explain why it's not appropriate for upstream?
Does valgrind not support running with different optimizations
of glibc?

>
> Signed-off-by: Andre McCurdy <armccurdy@gmail.com>
> ---
>  coregrind/m_redir.c | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/coregrind/m_redir.c b/coregrind/m_redir.c
> index 7e4df8d..640a346 100644
> --- a/coregrind/m_redir.c
> +++ b/coregrind/m_redir.c
> @@ -1220,7 +1220,18 @@ static void add_hardwired_spec (const  HChar*
> sopatt, const HChar* fnpatt,
>     spec->from_fnpatt = CONST_CAST(HChar *,fnpatt);
>     spec->to_addr     = to_addr;
>     spec->isWrap      = False;
> -   spec->mandatory   = mandatory;
> +
> +   /* Hack: Depending on how glibc was compiled (e.g. optimised for size or
> +      built with _FORTIFY_SOURCE enabled) the strlen symbol might not be found.
> +      Therefore although we should still try to intercept it, don't make it
> +      mandatory to do so. We over-ride "mandatory" here to avoid the need to
> +      patch the many different architecture specific callers to
> +      add_hardwired_spec(). */
> +   if (0==VG_(strcmp)("strlen", fnpatt))
> +      spec->mandatory = NULL;

I know that Jackie has a v2 out but since the interested parties are
CCed here, and since this is marked as a hack, would there
be any value in issuing a warning:

#warning "strlen() will not be tracked due to glibc optimization level"

or something like that. Maybe it's overkill since strlen is (should be?)
side-effect free but I thought I'd share the thought.

What's the right thing to do upstream after we have this hack merged
locally to fix our ptests?

../Randy

> +   else
> +      spec->mandatory = mandatory;
> +
>     /* VARIABLE PARTS */
>     spec->mark        = False; /* not significant */
>     spec->done        = False; /* not significant */
>


-- 
# Randy MacLeod. SMTS, Linux, Wind River
Direct: 613.963.1350 | 350 Terry Fox Drive, Suite 200, Ottawa, ON, 
Canada, K2K 2W5


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] glibc: add -fno-builtin-strlen when not using -O2
  2016-12-15  1:04           ` Randy MacLeod
@ 2016-12-15  9:28             ` Andre McCurdy
  0 siblings, 0 replies; 9+ messages in thread
From: Andre McCurdy @ 2016-12-15  9:28 UTC (permalink / raw)
  To: Randy MacLeod; +Cc: Patches and discussions about the oe-core layer

On Wed, Dec 14, 2016 at 5:04 PM, Randy MacLeod
<randy.macleod@windriver.com> wrote:
> On 2016-12-13 04:16 AM, Andre McCurdy wrote:
>>
>> On Mon, Dec 12, 2016 at 9:14 PM, Huang, Jie (Jackie)
>> <Jackie.Huang@windriver.com> wrote:
>>>>
>>>> From: Andre McCurdy [mailto:armccurdy@gmail.com]
>>>> For reference, here's the patch I've been using. It's a slightly more
>>>> generic fix than the one in the KDE bug report.
>>>
>>> Thanks, It's a better patch and I will take it and send as v2 of this
>>> issue if you're
>>> not going to send it yourself, is it fine for you and could you provide
>>> extra info
>>> for the patch header like, upstream-status, written by or Signed-off-by?
>>
>> Sure. I forget why I didn't submit this at the time. The full patch is:
>>
>>> From d34e2a50ca5493f5a0ce9ccad83a36ac33689266 Mon Sep 17 00:00:00 2001
>>
>> From: Andre McCurdy <armccurdy@gmail.com>
>> Date: Fri, 12 Feb 2016 18:22:12 -0800
>> Subject: [PATCH] make ld-XXX.so strlen intercept optional
>>
>> Hack: Depending on how glibc was compiled (e.g. optimised for size or
>> built with _FORTIFY_SOURCE enabled) the strlen symbol might not be
>> found in ld-XXX.so. Therefore although we should still try to
>> intercept it, don't make it mandatory to do so.
>>
>> Upstream-Status: Inappropriate
>
> Can you explain why it's not appropriate for upstream?

Because it doesn't take into account the main reason why valgrind
tries to find a strlen symbol in ld-XXX.so, ie as a sanity check that
debug symbols for ld-XXX.so are available. In OE core, it's OK to
relax the sanity check because we ensure that symbols are available
via an RRECOMMENDS in the valgrind recipe:

  # valgrind needs debug information for ld.so at runtime in order to
  # redirect functions like strlen.
  RRECOMMENDS_${PN} += "${TCLIBC}-dbg"

However applying my patch (or the KDE patch referenced by Khem - which
is an even bigger hack) in a build environment which doesn't try to
make the same guarantees might be dangerous.

For reference, Buildroot handles the problem by never fully stripping ld-XXX.so

  https://github.com/buildroot/buildroot/commit/1edb4c51dee78d7d26700c037f29558e73d27ee0

> Does valgrind not support running with different optimizations
> of glibc?

Apparently not, but you should probably ask that question on the
valgrind lists rather than here.

A solution which may be more suitable for upstream would be for
valgrind to check for a number of different symbols (rather than just
strlen) and only generate a fatal error if none can be found. That's
not a trivial patch though.

>> Signed-off-by: Andre McCurdy <armccurdy@gmail.com>
>> ---
>>  coregrind/m_redir.c | 13 ++++++++++++-
>>  1 file changed, 12 insertions(+), 1 deletion(-)
>>
>> diff --git a/coregrind/m_redir.c b/coregrind/m_redir.c
>> index 7e4df8d..640a346 100644
>> --- a/coregrind/m_redir.c
>> +++ b/coregrind/m_redir.c
>> @@ -1220,7 +1220,18 @@ static void add_hardwired_spec (const  HChar*
>> sopatt, const HChar* fnpatt,
>>     spec->from_fnpatt = CONST_CAST(HChar *,fnpatt);
>>     spec->to_addr     = to_addr;
>>     spec->isWrap      = False;
>> -   spec->mandatory   = mandatory;
>> +
>> +   /* Hack: Depending on how glibc was compiled (e.g. optimised for size
>> or
>> +      built with _FORTIFY_SOURCE enabled) the strlen symbol might not be
>> found.
>> +      Therefore although we should still try to intercept it, don't make
>> it
>> +      mandatory to do so. We over-ride "mandatory" here to avoid the need
>> to
>> +      patch the many different architecture specific callers to
>> +      add_hardwired_spec(). */
>> +   if (0==VG_(strcmp)("strlen", fnpatt))
>> +      spec->mandatory = NULL;
>
>
> I know that Jackie has a v2 out but since the interested parties are
> CCed here, and since this is marked as a hack, would there
> be any value in issuing a warning:
>
> #warning "strlen() will not be tracked due to glibc optimization level"
>
> or something like that. Maybe it's overkill since strlen is (should be?)
> side-effect free but I thought I'd share the thought.
>
> What's the right thing to do upstream after we have this hack merged
> locally to fix our ptests?
>
> ../Randy
>
>> +   else
>> +      spec->mandatory = mandatory;
>> +
>>     /* VARIABLE PARTS */
>>     spec->mark        = False; /* not significant */
>>     spec->done        = False; /* not significant */
>>
>
>
> --
> # Randy MacLeod. SMTS, Linux, Wind River
> Direct: 613.963.1350 | 350 Terry Fox Drive, Suite 200, Ottawa, ON, Canada,
> K2K 2W5


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2016-12-15  9:28 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-12-12  5:42 [PATCH] glibc: add -fno-builtin-strlen when not using -O2 jackie.huang
2016-12-12  6:37 ` Khem Raj
2016-12-12  7:41   ` Huang, Jie (Jackie)
2016-12-12  9:54     ` Andre McCurdy
2016-12-13  5:14       ` Huang, Jie (Jackie)
2016-12-13  9:16         ` Andre McCurdy
2016-12-14  2:38           ` Huang, Jie (Jackie)
2016-12-15  1:04           ` Randy MacLeod
2016-12-15  9:28             ` Andre McCurdy

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.