[OE-core] [PATCH] glibc: add -fno-builtin-strlen when not using -O2

[Randy MacLeod]

On 2016-12-13 04:16 AM, Andre McCurdy wrote:
> On Mon, Dec 12, 2016 at 9:14 PM, Huang, Jie (Jackie)
> <Jackie.Huang at windriver.com> wrote:
>>> From: Andre McCurdy [mailto:armccurdy at 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 at 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 at 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