Re: [Valgrind-developers] ppc64le results

[Carl L. <ce...@us...>]

Aaron:

On Wed, 2023-08-30 at 15:09 -0400, Aaron Merey wrote:
> Hi Carl,
> 
> Sorry for the delay.  I'm currently away for the next couple weeks,
> however
> I was able to take a look at these regressions.
> 
> It looks like debuginfo is not always lazily loaded on ppc64le since
> it's
> possible for neither describe_IP or find_DiCfSI to be called before
> symtab
> lookups during stacktrace.  describe_IP and find_DiCfSI contain calls
> to lazily load debuginfo, so if they are not called before stacktrace
> printing
> it results in missing debuginfo and lower quality stacktraces.
> 
> I've attached a patch that fixed the regressions for me when I tested
> this on
> a ppc64le machine.  It adds lazy debuginfo loading during ppc
> get_StackTrace_wrk.
> 

Thanks for taking a look at the issue.  I tested the patch an a variety
of machines and get mixed results.  Here is what I am seeing before the
commit to add the lazy loading, with the current Valgrind mainline
(includes the lazy commit) and with the patch to fix the lazy load on
Power:

machine   	pre-lazy-load		current mainline	with ppc debuginfo fix
Power 8 LE	707 tests,		708 tests, 		708 tests
		4 stderr failures,	280 stderr failures, 	247 stderr failures, 
		0 stdout failures,	54 stdout failures, 	54 stdout failures,
		13 stderrB failures,	16 stderrB failures,	16 stderrB failures,
		0 stdoutB failures,	11 stdoutB failures,	12 stdoutB failures
		9 post failures		13 post failures	9 post failures

Power 8 BE  	742 tests,		743 tests, 		743 tests,	
		2 stderr failures,	671 stderr failures, 	671 stderr failures,
		0 stdout failures,	152 stdout failures, 	152 stdout failures,
		0 stderrB failures,	14 stderrB failures,	14 stderrB failures,
		2 stdoutB failures, 	20 stdoutB failures,	20 stdoutB failures,
		9 post failures		43 post failures	43 post failures

Power 9 LE  	711 tests,		712 tests,		712 tests,
		4 stderr failures,	280 stderr failures,	247 stderr failures,
		0 stdout failures,	54 stdout failures,	54 stdout failures,
		13 stderrB failures,	16 stderrB failures,	16 stderrB failures,
		0 stdoutB failures,	12 stdoutB failures,	12 stdoutB failures
		9 post failures		13 post failures	9 post failures

Power 10 LE 	719 tests		720 tests,		720 tests,
		2 stderr failures,	42 stderr failures,	2 stderr failures,
		0 stdout failures,	0 stdout failures,	0 stdout failures,
		2 stderrB failures,	2 stderrB failures,	2 stderrB failures,
		10 stdoutB failures,	10 stdoutB failures,	10 stdoutB failures,
		0 post failures		3 post failures		0 post failures

So the patch has mixed results in fixing the issue.  It feels like
there is still a timing issue to me.  Perhaps there needs to be a check
to see if the lazy load has completed before the use?  Just throwing
out ideas here.

Anyway, sounds like you are out of the office for awhile.  I am fine
with waiting until you are back to work on this some more.  No need to
mess up you time off.  I don't think there is a release coming soon so
I think we have some time to get this fixed up.

Thanks for the help with the initial patch fix.

                              Carl