This is an archive of the discontinued LLVM Phabricator instance.

Adapt TestCustomShell and TestMultipleDebuggers to run under ASAN
ClosedPublic

Authored by augusto2112 on Feb 10 2023, 11:24 AM.

Details

Summary

In situations where only LLDB is ASANified, a false positive occurs
unless ASAN_OPTIONS=detect_container_overflow=0 is set in the
environment.

Diff Detail

Event Timeline

augusto2112 created this revision.Feb 10 2023, 11:24 AM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 10 2023, 11:24 AM
augusto2112 requested review of this revision.Feb 10 2023, 11:24 AM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 10 2023, 11:24 AM

Why are only these two tests affected? Should this be something we set globally for all the tests? The API tests already have support for forwarding ASAN_OPTIONS and lit has a similar concept.

I can imagine why TestMultipleSimultaneousDebuggers would be special: it basically build its own driver (so the detest machinery to set the inferior env doesn't work). But I'm a little puzzled by the shell tests, especially since you only updated a single RUN line.

Why are only these two tests affected? Should this be something we set globally for all the tests? The API tests already have support for forwarding ASAN_OPTIONS and lit has a similar concept.

Because they both set their own environment in one way or the other, which doesn't happen in the usual cas.

But I'm a little puzzled by the shell tests, especially since you only updated a single RUN line.

"env -i " means only use the enviroments passed in that line (discard everything else).

Generally LGTM, with one possible improvement inline!

lldb/test/API/api/multiple-debuggers/TestMultipleDebuggers.py
20

IIUC, the python process executing this python code should already have this flag passed in via lit, so if it's possible to say something like 'ASAN_OPTIONS':getEnv('ASAN_OPTIONS' then that would be the cleanest implementation.

lldb/test/Shell/Host/TestCustomShell.test
10

This is as good as we can do.

Why are only these two tests affected? Should this be something we set globally for all the tests? The API tests already have support for forwarding ASAN_OPTIONS and lit has a similar concept.

We do, but only for dotest tests, and our dotest test launches another LLDB.

JDevlieghere accepted this revision.Feb 10 2023, 1:16 PM

"env -i " means only use the enviroments passed in that line (discard everything else).

Thanks, I didn't notice -i.

LGTM

This revision is now accepted and ready to land.Feb 10 2023, 1:16 PM