HomePhabricator

make TestRegisterVariables slightly more resilient

Description

make TestRegisterVariables slightly more resilient

This test sets the compiler optimization level to -O1 and
makes some assumptions about how local frame vars will be
stored (i.e. in registers). These assumptions are not always
true.

I did a first-pass set of improvements that:
(1) no longer assumes that every one of the target locations has

every variable in a register.  Sometimes the compiler
is even smarter and skips the register entirely.

(2) simply expects one of the 5 or so variables it checks

to be in a register.

This test probably passes on a whole lot more systems than it
used to now. This is certainly true on OS X.

Details

Committed
tfialaApr 5 2016, 6:14 PM
Parents
rL265497: Fix a memory leak found by check-lld asan tests.
Branches
Unknown
Tags
Unknown

Event Timeline

This CL caused a lot of breakage across various Linux/Android builders. I created a fix at rL265527 but please verify it is still working with OSX after that

/lldb/trunk/packages/Python/lldbsuite/test/lang/c/register_variables/Makefile
7–11

This change breaks the test for remote debugging when running from OSX to anything else (e.g. android) as the cross compilers don't need this flag. Why you need this here and not in any of the other tests?

tfiala added a subscriber: tfiala.Apr 6 2016, 7:03 AM

I think it's the -O1 that is screwing it up. The test relies on setting
the optimization level, and then screws up the compiler's ability to find
the framework headers with more recent clangs on OS X (both in-tree built
and Xcode-delivered). Ultimately OS X has been shifting where the
/usr/include files have been delivered for a while, and this then requires
telling clang where to find them one way or another.

Thanks for fixing. I can dig into that more and make it cleaner, I had
some code in the Makefile.rules that normally handles that but wasn't for
some reason. I'll have to look at what failed. The change to the Makefile
was instigated by OS X hitting an error on build when building with the
in-tree clang.

-Todd

My guess is the root cause of the problem is that we are setting CFLAGS instead of appending to CLAFGS_EXTRA (I changed it in my followup CL)

tfiala added a comment.Apr 6 2016, 7:34 AM

CFLAGS_EXTRA didn't work because it then gets both -O0 and -O1 (for the
majority of our tests, Makefile.rules correctly assumes optimizations are
turned off for maximal debug info). If that was the fix, that is
circumventing the check this test is doing.

What we really need (and I'll go back and add) is an explicit optimization
level variable for Makefile.rules that defaults to -O0 but allows
overriding. That would most likely address this. I'll go back to that
once I'm done with another task.

Thanks!

-Todd

tfiala added a comment.Apr 6 2016, 7:35 AM

(And sorry for the breakage! I didn't see any nag mails).