[PATCH] D109794: [Sanitizer] Allow setting the report path to create directory

Teresa Johnson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Feb 10 15:48:08 PST 2022


tejohnson added a comment.

In D109794#3310049 <https://reviews.llvm.org/D109794#3310049>, @tejohnson wrote:

> In D109794#3310024 <https://reviews.llvm.org/D109794#3310024>, @denik wrote:
>
>> In D109794#3309991 <https://reviews.llvm.org/D109794#3309991>, @tejohnson wrote:
>>
>>> In D109794#3309768 <https://reviews.llvm.org/D109794#3309768>, @denik wrote:
>>>
>>>> This change breaks tests with sanitizers running in a sandbox.
>>>> If `/usr/local/tmp/asan` is in the allow-list for writing, the sandbox will kill the test when it tries to create `/usr`.
>>>>
>>>> This failure is caught on Chrome OS, https://issuetracker.google.com/209296420.
>>>
>>> It sounds like the issue is showing up with some Chrome OS tests built with -fsanitize=address, as opposed to in the unit test added here - is that correct?
>>
>> Correct, I meant Chrome OS tests. Chrome OS uses Gentoo sandboxing for its unit tests.
>>
>>> If the former, by default the report_file is stderr - do you know where/how the report file path is being set to something else? Note the change only creates a directory for the given report_file path, the sanitizer change itself doesn't change the path. Before the report file creation would fail if the directory didn't exist.
>>
>> `log_path` is set in https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/master/profiles/base/profile.bashrc#512
>> Note that the directory was created in the same function. And it's called before entering a sandbox.
>>
>> Can we skip `CreateDir` in `RecursiveCreateParentDirs` if the path exists? So it will be no-op in our case?
>
> Oh I see, we normally assume it will error (but we ignore the error) if the directory already exists. But in the sandboxed case it doesn't like the creation attempt, even though the dir actually exists.
>
> Yeah, we can presumably use "stat" to check before trying to create. Actually, there is already a FileExists() in sanitizer_file.h that has implementations in the various OS specific sanitizer files. Most (except windows) call stat (but look for a regular file). I can just add something like a DirExists() interface that has the equivalent dir check using the same interfaces in each OS implementation. Will work on that tomorrow.

Are you able to patch in D119495 <https://reviews.llvm.org/D119495> and see if that fixes the issue?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D109794/new/

https://reviews.llvm.org/D109794



More information about the llvm-commits mailing list