On Wed, Dec 14, 2016 at 5:04 PM, Randy MacLeod
<randy.macl...@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.hu...@windriver.com> wrote:
>>>>
>>>> From: Andre McCurdy [mailto:armccu...@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 <armccu...@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 <armccu...@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
--
_______________________________________________
Openembedded-core mailing list
Openembedded-core@lists.openembedded.org
http://lists.openembedded.org/mailman/listinfo/openembedded-core
